Re: [PATCH v1 2/3] linux-user: deal with address wrap for ARM_COMMPAGE on 32 bit

[Alex Bennée]

Alex Bennée <alex.bennee@linaro.org> writes:

> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> On 5/27/20 3:05 AM, Alex Bennée wrote:
>>> @@ -2145,7 +2145,7 @@ static uintptr_t pgd_find_hole_fallback(uintptr_t 
>>> guest_size, uintptr_t brk, lon
>>>  
>>>  /* Return value for guest_base, or -1 if no hole found. */
>>>  static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t 
>>> guest_size,
>>> -                               long align)
>>> +                               long align, uintptr_t offset)
>>>  {
>>>      GSList *maps, *iter;
>>>      uintptr_t this_start, this_end, next_start, brk;
>>> @@ -2171,7 +2171,7 @@ static uintptr_t pgb_find_hole(uintptr_t 
>>> guest_loaddr, uintptr_t guest_size,
>>>  
>>>          this_end = ((MapInfo *)iter->data)->start;
>>>          next_start = ((MapInfo *)iter->data)->end;
>>> -        align_start = ROUND_UP(this_start, align);
>>> +        align_start = ROUND_UP(this_start + offset, align);
>>>  
>>>          /* Skip holes that are too small. */
>>
>> I suppose offset is supposed to mean we start from -offset?
>
> Well guest_base will start higher meaning we have space for the
> commpage beneath it.
>
>> You didn't update
>> pgb_find_hole_fallback.
>
> Fixed.
>
>>
>>> -            loaddr = ARM_COMMPAGE & -align;
>>> +            offset = (128 * KiB);
>>
>> Why 128K?  Surely this should be an expression against ARM_COMMPAGE.
>
> In theory:
>
>             offset = -(ARM_COMMPAGE & -align);
>
> should do the trick but I found it failed every now and again.
> Frustratingly putting printfs in made it go away so in frustration I
> just upped the offset until it stopped happening.
>
> I do kinda wish rr worked on i386 :-/

Ahh all I needed was a MAP_FIXED for init_commpage

-- 
Alex Bennée