Re: [PATCH 7/7] xen/arm: mm: Reduce the area that xen_second covers

[Julien Grall <julien@xxxxxxx>]

Hi Michal,

On 27/06/2022 08:51, Michal Orzel wrote:
> On 24.06.2022 11:11, Julien Grall wrote:
> > From: Julien Grall <jgrall@xxxxxxxxxx>
> >
> > At the moment, xen_second is used to cover the first 2GB of the
> > virtual address space. With the recent rework of the page-tables,
> > only the first 1GB region (where Xen resides) is effectively used.
> >
> > In addition to that, I would like to reshuffle the memory layout.
> > So Xen mappings may not be anymore in the first 2GB of the virtual
> > address space.
> >
> > Therefore, rework xen_second so it only covers the 1GB region where
> > Xen will reside.
> >
> > With this change, xen_second doesn't cover anymore the xenheap area
> > on arm32. So, we first need to add memory to the boot allocator before
> > setting up the xenheap mappings.
> >
> > Take the opportunity to update the comments on top of xen_fixmap and
> > xen_xenmap.
> >
> > Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> > ---
> >   xen/arch/arm/mm.c    | 32 +++++++++++---------------------
> >   xen/arch/arm/setup.c | 13 +++++++++++--
> >   2 files changed, 22 insertions(+), 23 deletions(-)
> >
> > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> > index 74666b2e720a..f87a7c32d07d 100644
> > --- a/xen/arch/arm/mm.c
> > +++ b/xen/arch/arm/mm.c
> > @@ -116,17 +116,14 @@ static DEFINE_PAGE_TABLE(cpu0_pgtable);
> >   #endif
> >   
> >   /* Common pagetable leaves */
> > -/* Second level page tables.
> > - *
> > - * The second-level table is 2 contiguous pages long, and covers all
> > - * addresses from 0 to 0x7fffffff. Offsets into it are calculated
> > - * with second_linear_offset(), not second_table_offset().
> > - */
> > -static DEFINE_PAGE_TABLES(xen_second, 2);
> > -/* First level page table used for fixmap */
> > +/* Second level page table used to cover Xen virtual address space */
> > +static DEFINE_PAGE_TABLE(xen_second);
> > +/* Third level page table used for fixmap */
> >   DEFINE_BOOT_PAGE_TABLE(xen_fixmap);
> > -/* First level page table used to map Xen itself with the XN bit set
> > - * as appropriate. */
> > +/*
> > + * Third level page table used to map Xen itself with the XN bit set
> > + * as appropriate.
> > + */
> >   static DEFINE_PAGE_TABLE(xen_xenmap);
> >   
> >   /* Non-boot CPUs use this to find the correct pagetables. */
> > @@ -168,7 +165,6 @@ static void __init __maybe_unused build_assertions(void)
> >       BUILD_BUG_ON(zeroeth_table_offset(XEN_VIRT_START));
> >   #endif
> >       BUILD_BUG_ON(first_table_offset(XEN_VIRT_START));
> > -    BUILD_BUG_ON(second_linear_offset(XEN_VIRT_START) >= XEN_PT_LPAE_ENTRIES);
> Instead of removing this completely, shouldn't you change it to:
> BUILD_BUG_ON(second_table_offset(XEN_VIRT_START)); ?
This would be wrong because the 2nd table offset is 1 today (Xen starts 
at 2MB).
Furthermore, setup_pagetables() doesn't make any assumption regarding 
the 2nd table offset. So I don't think we should have a BUILD_BUG_ON().
Cheers,

--
Julien Grall