Re: [Xen-devel] [PATCH v2 16/24] xen/arm: page: Use ARMv8 naming to improve readability

[Andre Przywara <andre.przywara@xxxxxxx>]

Hi,

On 21/09/17 16:46, Stefano Stabellini wrote:
> On Thu, 21 Sep 2017, Julien Grall wrote:
>> Hi,
>>
>> On 20/09/17 00:45, Stefano Stabellini wrote:
>>>> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
>>>> index 30fcfa0778..899fd1801a 100644
>>>> --- a/xen/include/asm-arm/page.h
>>>> +++ b/xen/include/asm-arm/page.h
>>>> @@ -26,14 +26,14 @@
>>>>    * LPAE entry; the 8-bit fields are packed little-endian into MAIR0 and
>>>> MAIR1.
>>>>    *
>>>>    *                    ai    encoding
>>>> - *   MT_UNCACHED      000   0000 0000  -- Strongly Ordered
>>>> - *   MT_BUFFERABLE    001   0100 0100  -- Non-Cacheable
>>>> - *   MT_WRITETHROUGH  010   1010 1010  -- Write-through
>>>> - *   MT_WRITEBACK     011   1110 1110  -- Write-back
>>>> - *   MT_DEV_SHARED    100   0000 0100  -- Device
>>>> + *   MT_DEVICE_nGnRE  000   0000 0000  -- Strongly Ordered/Device nGnRnE
>>>
>>> I admit I always hated the "nGnRE" acronym. However, it is on the ARM
>>> ARM too, so if you'd like to introduce it here, I'll accept it. But
>>> please at least expand the acronym in the comment to make it
>>> understandable (same with nGnRnE).
>>
>> "nGnRE" acronym are not great but convey the meaning of what would be the
>> resulting attribute.

I agree they are hideous to read, but easy to break down once you got
the idea ...

> This is an honest question, no pun intended: how do they convey the
> meaning? Personally, I have to look it up every time on the ARM ARM...

ARMv8 ARM B2.7.2  Device memory

G -> Gathering (can merge multiple accesses into one transfer)
R -> Reordering
E -> Early Write acknowledgement (other agents than the endpoint
(caches) can acknowledge the transfer).

n means not.

Done. More details in the ARM ARM.

Cheers,
Andre.

>> For instance MT_UNCACHED does not really say if it is for
>> device or memory. Lets not even mention MT_BUFFERABLE which is in fact
>> non-cacheable memory :).
>>
>>>
>>> Also, the comment say "nGnRnE" while the definition is MT_DEVICE_nGnRE.
>>
>> Actually, the comment is correct but not the naming. It should
>> MT_DEVICE_nGnRnE. I will rename it.
>>
>> Aside that, I think the comment is understandable. nGnRnE is equivalent to
>> Strongly ordered. I could expand nGnRnE (non-Gatherable, non-Reordering, No
>> Early write acknowledgment) but I feel at this stage you can just search the
>> name in the ARM ARM...
> 
> I am not asking to expland the name, only to expand nGnRnE in the
> comment on the side. Searching through that pdf is not really a fun
> activity.
> 

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