Re: [PATCH v6 09/13] xen/arm: Extract MMU-specific MM code

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 8 Sep 2023, Henry Wang wrote:
> Hi Ayan,
> 
> > On Sep 7, 2023, at 19:34, Ayan Kumar Halder <ayankuma@xxxxxxx> wrote:
> > 
> > Hi Henry,
> > 
> >> +
> >> +extern mfn_t directmap_mfn_start, directmap_mfn_end;
> > 
> > As you are declaring them for MMU specific , you also need this change :-
> > 
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index 89ecb54be2..19b60c5d1b 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -670,7 +670,7 @@ void __init populate_boot_allocator(void)
> > 
> >              s = bootinfo.reserved_mem.bank[i].start;
> >              e = s + bootinfo.reserved_mem.bank[i].size;
> > -#ifdef CONFIG_ARM_32
> > +#if (CONFIG_ARM_32 && CONFIG_MMU)
> >              /* Avoid the xenheap, note that the xenheap cannot across a 
> > bank */
> >              if ( s <= mfn_to_maddr(directmap_mfn_start) &&
> >                   e >= mfn_to_maddr(directmap_mfn_end) )
> > @@ -708,7 +708,7 @@ void __init populate_boot_allocator(void)
> >              if ( e > bank_end )
> >                  e = bank_end;
> > 
> > -#ifdef CONFIG_ARM_32
> > +#if (CONFIG_ARM_32 && CONFIG_MMU)
> >              /* Avoid the xenheap */
> >              if ( s < mfn_to_maddr(directmap_mfn_end) &&
> >                   mfn_to_maddr(directmap_mfn_start) < e )
> > 
> > So that directmap_mfn_end and directmap_mfn_start is used only when MMU is 
> > enabled.
> 
> I am not 100% sure on this, because currently there is no MPU code at
> all, indicating all setup.c is MMU specific. In this case adding “&& 
> CONFIG_MMU”
> seems a little bit redundant to me. But I agree you made a point and it is 
> correct
> that when the MPU code is in, these “directmap” part should be gated with
> CONFIG_MMU (or maybe split the code between arm32/arm64 to different helpers
> to avoid #ifdef). Hence I would prefer doing these change when the MPU code 
> is added.
> 
> Let’s see what maintainers will say. I am happy to do the change once we have
> an agreement.

It might be wiser to add && CONFIG_MMU when the MPU code is added in
case we decide to move it / shape it differently.