Re: [Xen-devel] [PATCH 03/24] xen/arm: setup: Remove bogus xenheap_mfn_end in setup_mm for arm64

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 06/16/2017 06:33 PM, Stefano Stabellini wrote:
> On Fri, 16 Jun 2017, Julien Grall wrote:
> > Hi Stefano,
> >
> > On 15/06/2017 23:28, Stefano Stabellini wrote:
> > > On Tue, 13 Jun 2017, Julien Grall wrote:
> > > > xenheap_mfn_end is storing an MFN and not a physical address. Thankfully
> > > > xenheap_mfn_end is not used in the arm64 code. So drop it.
> > > That's fine, but in that case I would prefer to move the definition of
> > > xenheap_mfn_end under #ifdef CONFIG_ARM_32. In fact, there is another
> > > assignment of xenheap_mfn_end few lines below in the arm64 version of
> > > setup_mm: don't we need to remove that too?
> > The other xenheap_mfn_end contains valid mfn that point to the end and I
> > didn't want to #ifdef it because:
> >         1) It complexify the code
> >         2) All regions should be bound with start/end to simplify potential
> > use.
> I am only suggesting to move its definition and declaration under #ifdef
> CONFIG_ARM_32 in xen/include/asm-arm/mm.h and xen/arch/arm/mm.c.
>
> After that, all users of xenheap_mfn_end are already #ifdef
> CONFIG_ARM_32, except for xen/arch/arm/setup.c:setup_mm. The setup_mm
> under #ifdef CONFIG_ARM_32 will be fine. The setup_mm under
> #ifdef CONFIG_ARM_64, doesn't need xenheap_mfn_end and we could just
> remove it from there.
>
> Does it make sense? Am I missing something?
To be honest, I really want to limit the ifdefery in the mm code. This 
is a bit complex to follow. One of my side project is to look at that.
Also, even if xenheap_mfn_end today is not used, I think the current 
value is valid and could be helpful to have in hand. For instance, it 
does not seem justify to have different implementation of at least 
is_xen_heap_page for arm32 and arm64.
So I am not in favor of dropping xenheap_mfn_end at the moment.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel