Re: [PATCH] xen/arm: skip first page when RAM starts at 0x0

[Julien Grall <julien.grall@arm.com>]

Hi,

On 29/04/2019 08:15, Jan Beulich wrote:
>>>> On 27.04.19 at 01:47, <sstabellini@kernel.org> wrote:
>> 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?
> 
> Yes. pfn_to_pdx() and friends may only ever be passed actual
> PFNs / PDXes, not something that's one above or one below a
> valid range. I share Julien's question though: Was it really mere
> luck that things have been working to date?

TLDR; unexplained to not say mere luck

We define the size of the frametable using:

nr_pdxs = pfn_to_pdx(pe - ps);
frametable_size = nr_pdxs * sizeof (struct page_info);

mfn_to_page is implemented the following way:

frame_table + (mfn_to_pdx(mfn) - frametable_base_pdx)

Where frametable_base_pdx = pfn_to_pdx(ps >> PAGE_SHIFT)

On the two platforms (Juno-r2 + Foundation Model) I tested today, somehow

pfn_to_pdx(pe - ps) == (pfn_to_pdx((pe >> PAGE_SHIFT) - 1) + 1) - 
frametable_base_pdx)

So the frametable is correctly sized but I honestly I have no idea why it works.

Anyway, I also tested the change suggested by Stefano. This will substantially 
increase the size of the frametable on platform where the RAM does not start at 0.

For instance, on Foundation Model the RAM starts at 2GB. As we don't compress 
any of the first 31 bits, the frametable will now be 28MB bigger than we 
currently have (112MB up from 84MB).

So I think what we want is:

nr_pdxs = pfn_to_pdx(end - 1) - pfn_to_pdx(start) + 1;
frame_table_base_pdx = pfn_to_pdx(start);

On a side note, I noticed that the table is still much bigger than it should be. 
4GB of RAM should only require a frametable of 56MB, but it is 84MB. Anyway, 
that's probably a separate discussion.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel