[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] xen/arm: skip first page when RAM starts at 0x0
On Fri, 26 Apr 2019, Julien Grall wrote: > On 4/26/19 4:48 PM, Jan Beulich wrote: > > > > > On 26.04.19 at 17:38, <julien.grall@xxxxxxx> wrote: > > > On 26/04/2019 10:42, Jan Beulich wrote: > > > > > > > On 26.04.19 at 11:19, <Julien.Grall@xxxxxxx> wrote: > > > > > So how does the PDX work for memory below 4GB? Do you still call > > > > > pfn_to_pdx or those pages? > > > > > > > > Of course. We merely never compress any of the low 32 address > > > > bits, i.e. our choice is limited to address bits 32 ... 51. > > > > > > Ah, the code makes a bit more sense on the x86 side. Is there any reason > > > to > > > limit to address bit 32 .. 51? > > > > Well, there being various special things immediately below 4Gb > > there's simply little point in trying to squash any of the lower bits. > > > > > For Arm, we can't compress bits 0 ... 30 due to the buddy allocator (see > > > pfn_pdx_hole_setup). So we could ignore the first 1GB of RAM. > > > > Bits 0 ... 30 would be the first 2Gb afaict. > > Sorry I messed us my maths. I meant 0 ... 29 (MAX_ORDER = 18). > > @Stefano, do you think you can have a look at it? After an extensive online debugging session with Julien, we found a fix to the PDX code that makes the masks and nr_pdxs calculation correct. See the appended patch below. Mask is initialized to 0 to the let the algorithm compute the right mask based on the RAM banks. pdx_init_mask doesn't cope with a start address of 0x0 because it does `base_addr - 1' which causes a wrap around when base_addr is 0x0. That works OK as far as I can tell and the resulting values of `pfn_pdx_bottom_mask', `pfn_top_mask', and `pfn_pdx_hole_shift' seems correct (they match the case where I force ram_start = PAGE_SIZE). The other change to nr_pdxs is less obvious. It is clear that nr_pdxs is calculated wrongly in this case (memory 0-0x80000000, 0x800000000-0x880000000, ps=0, pe=0x880000000): nr_pdxs=524288 which is half the number we need (the correct number is 1048575). Taking a line from the x86 code xen/arch/x86/setup.c:setup_max_pdx (Julien's suggestion): max_pdx = pfn_to_pdx(top_page - 1) + 1; I changed nr_pdxs to nr_pdxs = pfn_to_pdx((pe >> PAGE_SHIFT) - 1) + 1; and it works. I think the change is correct because looking at the implementation of pfn_to_pdx it is certainly meant to operate on a pfn masking bits on it to compensate for the holes. It is not meant to work on a size. Jan, does it look correct to you too? diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 01ae2cc..f711eef 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -874,8 +874,7 @@ void __init setup_xenheap_mappings(unsigned long base_mfn, /* Map a frame table to cover physical addresses ps through pe */ void __init setup_frametable_mappings(paddr_t ps, paddr_t pe) { - unsigned long nr_pages = (pe - ps) >> PAGE_SHIFT; - unsigned long nr_pdxs = pfn_to_pdx(nr_pages); + unsigned long nr_pdxs = pfn_to_pdx((pe >> PAGE_SHIFT) - 1) + 1; unsigned long frametable_size = nr_pdxs * sizeof(struct page_info); mfn_t base_mfn; const unsigned long mapping_size = frametable_size < MB(32) ? MB(2) : MB(32); diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index ccb0f18..9e7da41 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -482,7 +482,7 @@ static void __init init_pdx(void) { paddr_t bank_start, bank_size, bank_end; - u64 mask = pdx_init_mask(bootinfo.mem.bank[0].start); + u64 mask = 0; int bank; for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ ) _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |