[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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.

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


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.

They are really easy to find compare to other bits of the specs :). Expanding is not going to be very useful without looking at the definition. I would prefer to add the section of the ARM ARM on top.

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.