Re: [Xen-devel] [PATCH 20/27] xen/arm: page: Use ARMv8 naming to improve readability

2017-08-23 Thread Andre Przywara
Hi,

On 14/08/17 15:24, Julien Grall wrote:
> This is based on the Linux ARMv8 naming scheme (see arch/arm64/mm/proc.S). 
> Each
> type will contain "NORMAL" or "DEVICE" to make clear whether each attribute
> targets device or normal memory.

Yes, that makes sense and improves readability as the naming matches the
spec and is more intuitive. Also it looks correct to me.

However I feel it would be more helpful is this patches comes before the
previous one which reworks the MAIR construction.

However for this patch:

> Signed-off-by: Julien Grall 

Reviewed-by: Andre Przywara 

Cheers,
Andre.

> ---
>  xen/arch/arm/kernel.c |  2 +-
>  xen/arch/arm/mm.c | 28 +-
>  xen/arch/arm/platforms/vexpress.c |  2 +-
>  xen/drivers/video/arm_hdlcd.c |  2 +-
>  xen/include/asm-arm/page.h| 42 
> +++
>  5 files changed, 38 insertions(+), 38 deletions(-)
> 
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 9c183f96da..a12baa86e7 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -54,7 +54,7 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned 
> long len)
>  s = paddr & (PAGE_SIZE-1);
>  l = min(PAGE_SIZE - s, len);
>  
> -set_fixmap(FIXMAP_MISC, maddr_to_mfn(paddr), MT_BUFFERABLE);
> +set_fixmap(FIXMAP_MISC, maddr_to_mfn(paddr), MT_NORMAL_NC);
>  memcpy(dst, src + s, l);
>  clean_dcache_va_range(dst, l);
>  
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 45974846a9..ce1858fbf3 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -290,7 +290,7 @@ static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned 
> attr)
>  
>  switch ( attr )
>  {
> -case MT_BUFFERABLE:
> +case MT_NORMAL_NC:
>  /*
>   * ARM ARM: Overlaying the shareability attribute (DDI
>   * 0406C.b B3-1376 to 1377)
> @@ -305,8 +305,8 @@ static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned 
> attr)
>   */
>  e.pt.sh = LPAE_SH_OUTER;
>  break;
> -case MT_UNCACHED:
> -case MT_DEV_SHARED:
> +case MT_DEVICE_nGnRnE:
> +case MT_DEVICE_nGnRE:
>  /*
>   * Shareability is ignored for non-Normal memory, Outer is as
>   * good as anything.
> @@ -369,7 +369,7 @@ static void __init create_mappings(lpae_t *second,
>  
>  count = nr_mfns / LPAE_ENTRIES;
>  p = second + second_linear_offset(virt_offset);
> -pte = mfn_to_xen_entry(_mfn(base_mfn), MT_WRITEALLOC);
> +pte = mfn_to_xen_entry(_mfn(base_mfn), MT_NORMAL);
>  if ( granularity == 16 * LPAE_ENTRIES )
>  pte.pt.contig = 1;  /* These maps are in 16-entry contiguous chunks. 
> */
>  for ( i = 0; i < count; i++ )
> @@ -422,7 +422,7 @@ void *map_domain_page(mfn_t mfn)
>  else if ( map[slot].pt.avail == 0 )
>  {
>  /* Commandeer this 2MB slot */
> -pte = mfn_to_xen_entry(_mfn(slot_mfn), MT_WRITEALLOC);
> +pte = mfn_to_xen_entry(_mfn(slot_mfn), MT_NORMAL);
>  pte.pt.avail = 1;
>  write_pte(map + slot, pte);
>  break;
> @@ -543,7 +543,7 @@ static inline lpae_t pte_of_xenaddr(vaddr_t va)
>  {
>  paddr_t ma = va + phys_offset;
>  
> -return mfn_to_xen_entry(maddr_to_mfn(ma), MT_WRITEALLOC);
> +return mfn_to_xen_entry(maddr_to_mfn(ma), MT_NORMAL);
>  }
>  
>  /* Map the FDT in the early boot page table */
> @@ -652,7 +652,7 @@ void __init setup_pagetables(unsigned long 
> boot_phys_offset, paddr_t xen_paddr)
>  /* Initialise xen second level entries ... */
>  /* ... Xen's text etc */
>  
> -pte = mfn_to_xen_entry(maddr_to_mfn(xen_paddr), MT_WRITEALLOC);
> +pte = mfn_to_xen_entry(maddr_to_mfn(xen_paddr), MT_NORMAL);
>  pte.pt.xn = 0;/* Contains our text mapping! */
>  xen_second[second_table_offset(XEN_VIRT_START)] = pte;
>  
> @@ -669,7 +669,7 @@ void __init setup_pagetables(unsigned long 
> boot_phys_offset, paddr_t xen_paddr)
>  
>  /* ... Boot Misc area for xen relocation */
>  dest_va = BOOT_RELOC_VIRT_START;
> -pte = mfn_to_xen_entry(maddr_to_mfn(xen_paddr), MT_WRITEALLOC);
> +pte = mfn_to_xen_entry(maddr_to_mfn(xen_paddr), MT_NORMAL);
>  /* Map the destination in xen_second. */
>  xen_second[second_table_offset(dest_va)] = pte;
>  /* Map the destination in boot_second. */
> @@ -700,7 +700,7 @@ void __init setup_pagetables(unsigned long 
> boot_phys_offset, paddr_t xen_paddr)
>  unsigned long va = XEN_VIRT_START + (i << PAGE_SHIFT);
>  if ( !is_kernel(va) )
>  break;
> -pte = mfn_to_xen_entry(mfn, MT_WRITEALLOC);
> +pte = mfn_to_xen_entry(mfn, MT_NORMAL);
>  pte.pt.table = 1; /* 4k mappings always have this bit set */
>  if ( is_kernel_text(va) || is_kernel_inittext(va) )
>  {
> @@ -771,7 +771,7 @@ int 

[Xen-devel] [PATCH 20/27] xen/arm: page: Use ARMv8 naming to improve readability

2017-08-14 Thread Julien Grall
This is based on the Linux ARMv8 naming scheme (see arch/arm64/mm/proc.S). Each
type will contain "NORMAL" or "DEVICE" to make clear whether each attribute
targets device or normal memory.

Signed-off-by: Julien Grall 
---
 xen/arch/arm/kernel.c |  2 +-
 xen/arch/arm/mm.c | 28 +-
 xen/arch/arm/platforms/vexpress.c |  2 +-
 xen/drivers/video/arm_hdlcd.c |  2 +-
 xen/include/asm-arm/page.h| 42 +++
 5 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 9c183f96da..a12baa86e7 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -54,7 +54,7 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long 
len)
 s = paddr & (PAGE_SIZE-1);
 l = min(PAGE_SIZE - s, len);
 
-set_fixmap(FIXMAP_MISC, maddr_to_mfn(paddr), MT_BUFFERABLE);
+set_fixmap(FIXMAP_MISC, maddr_to_mfn(paddr), MT_NORMAL_NC);
 memcpy(dst, src + s, l);
 clean_dcache_va_range(dst, l);
 
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 45974846a9..ce1858fbf3 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -290,7 +290,7 @@ static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned 
attr)
 
 switch ( attr )
 {
-case MT_BUFFERABLE:
+case MT_NORMAL_NC:
 /*
  * ARM ARM: Overlaying the shareability attribute (DDI
  * 0406C.b B3-1376 to 1377)
@@ -305,8 +305,8 @@ static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned 
attr)
  */
 e.pt.sh = LPAE_SH_OUTER;
 break;
-case MT_UNCACHED:
-case MT_DEV_SHARED:
+case MT_DEVICE_nGnRnE:
+case MT_DEVICE_nGnRE:
 /*
  * Shareability is ignored for non-Normal memory, Outer is as
  * good as anything.
@@ -369,7 +369,7 @@ static void __init create_mappings(lpae_t *second,
 
 count = nr_mfns / LPAE_ENTRIES;
 p = second + second_linear_offset(virt_offset);
-pte = mfn_to_xen_entry(_mfn(base_mfn), MT_WRITEALLOC);
+pte = mfn_to_xen_entry(_mfn(base_mfn), MT_NORMAL);
 if ( granularity == 16 * LPAE_ENTRIES )
 pte.pt.contig = 1;  /* These maps are in 16-entry contiguous chunks. */
 for ( i = 0; i < count; i++ )
@@ -422,7 +422,7 @@ void *map_domain_page(mfn_t mfn)
 else if ( map[slot].pt.avail == 0 )
 {
 /* Commandeer this 2MB slot */
-pte = mfn_to_xen_entry(_mfn(slot_mfn), MT_WRITEALLOC);
+pte = mfn_to_xen_entry(_mfn(slot_mfn), MT_NORMAL);
 pte.pt.avail = 1;
 write_pte(map + slot, pte);
 break;
@@ -543,7 +543,7 @@ static inline lpae_t pte_of_xenaddr(vaddr_t va)
 {
 paddr_t ma = va + phys_offset;
 
-return mfn_to_xen_entry(maddr_to_mfn(ma), MT_WRITEALLOC);
+return mfn_to_xen_entry(maddr_to_mfn(ma), MT_NORMAL);
 }
 
 /* Map the FDT in the early boot page table */
@@ -652,7 +652,7 @@ void __init setup_pagetables(unsigned long 
boot_phys_offset, paddr_t xen_paddr)
 /* Initialise xen second level entries ... */
 /* ... Xen's text etc */
 
-pte = mfn_to_xen_entry(maddr_to_mfn(xen_paddr), MT_WRITEALLOC);
+pte = mfn_to_xen_entry(maddr_to_mfn(xen_paddr), MT_NORMAL);
 pte.pt.xn = 0;/* Contains our text mapping! */
 xen_second[second_table_offset(XEN_VIRT_START)] = pte;
 
@@ -669,7 +669,7 @@ void __init setup_pagetables(unsigned long 
boot_phys_offset, paddr_t xen_paddr)
 
 /* ... Boot Misc area for xen relocation */
 dest_va = BOOT_RELOC_VIRT_START;
-pte = mfn_to_xen_entry(maddr_to_mfn(xen_paddr), MT_WRITEALLOC);
+pte = mfn_to_xen_entry(maddr_to_mfn(xen_paddr), MT_NORMAL);
 /* Map the destination in xen_second. */
 xen_second[second_table_offset(dest_va)] = pte;
 /* Map the destination in boot_second. */
@@ -700,7 +700,7 @@ void __init setup_pagetables(unsigned long 
boot_phys_offset, paddr_t xen_paddr)
 unsigned long va = XEN_VIRT_START + (i << PAGE_SHIFT);
 if ( !is_kernel(va) )
 break;
-pte = mfn_to_xen_entry(mfn, MT_WRITEALLOC);
+pte = mfn_to_xen_entry(mfn, MT_NORMAL);
 pte.pt.table = 1; /* 4k mappings always have this bit set */
 if ( is_kernel_text(va) || is_kernel_inittext(va) )
 {
@@ -771,7 +771,7 @@ int init_secondary_pagetables(int cpu)
 for ( i = 0; i < DOMHEAP_SECOND_PAGES; i++ )
 {
 pte = mfn_to_xen_entry(virt_to_mfn(domheap+i*LPAE_ENTRIES),
-   MT_WRITEALLOC);
+   MT_NORMAL);
 pte.pt.table = 1;
 write_pte([first_table_offset(DOMHEAP_VIRT_START+i*FIRST_SIZE)], 
pte);
 }
@@ -869,13 +869,13 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
 mfn_t first_mfn = alloc_boot_pages(1, 1);
 
 clear_page(mfn_to_virt(first_mfn));
-pte = mfn_to_xen_entry(first_mfn, MT_WRITEALLOC);
+pte