Re: [RFC PATCH v2 6/8] arm/mem_access: Add long-descriptor based gpt

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

On 12/06/17 11:12, Sergej Proskurin wrote:
> Hi Julien,

Hello Sergej,

>>
>>> +
>>> +    const unsigned int strides[3] = {
>>> +        LPAE_SHIFT_4K,
>>> +        LPAE_SHIFT_16K,
>>> +        LPAE_SHIFT_64K
>>> +    };
>>
>> Also, the stride can be found from the page shift. So I am not
>> convinced you need that.
>
> Sure, but don't you think it is cleaner doing it that way, than
> subtracting a value from PAGE_SHIFT_* although we have already a
> suitable define for that? Apart from that, we need to consider different
> strides for different granularities, which would make it harder to
> read/review if we don't use an array of strides at this point. For
> instance, see the following formula to compute the starting level of the
> translation tables:
>
> level = 4 - DIV_ROUND_UP((input_size - grainsizes[gran]), strides[gran]);

Looking at AArch64.TranslationTableWalk the stride is basically:

stride = grainsize - 3; // Log2(page size / 8 bytes)

So I still don't see how this would make the code less readable. This 
would also avoid to introduce yet another array on the stack (though it 
should really have been static) for limited purpose.

Cheers,

-- 
Julien Grall

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