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

[Julien Grall <Julien.Grall@xxxxxxx>]

Hi,

On 27/04/2019 00:47, Stefano Stabellini wrote:
> 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).

Well, I asked to use 0 as a mask to understand a bit more the issue in 
the algorithm. Now, we need to understand whether this is the right 
thing to do everytime.

In this case, Jan has pointed out that on x86 they don't compute the PDX 
compression using any RAM below 4GB.

For Arm, there are limited reason to use any RAM below 1GB to work out 
the PDX compression because they will not be ignored by the PDX allocator.

So maybe a more sensible choice (and potentially requiring less brain 
work :)) is to set a boundary below which there are no point to use for 
bits to compact.

My suggestion would be to follow x86 and use 4GB.

> 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.

While the change makes perfect sense to me,  I still don't understand 
how this has ever worked on Arm. Surely if we passed a size rather than 
a PFN then the computation will be wrong. How do we get away today? Is 
it because the frametable will always be a multiple of 2MB (or 32MB if 
bigger than 32MB)? But then it would mean that some page_info may not be 
initialized correctly.

I have seen report regarding memory issue on Xen recently [1] (data 
abort on page_info). This seems to happen because we stop relocating Xen 
towards the end of the memory. I am wondering whether the issue is 
because of the way we setup the frametable as well.

[1] 
https://lists.xenproject.org/archives/html/xen-devel/2019-03/msg01483.html

Cheers,

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