Re: [Qemu-devel] [PATCH] target/s390x: Remove leading underscores from #defines

2018-03-05 Thread Cornelia Huck 
On Mon,  5 Mar 2018 06:16:58 +0100
Thomas Huth  wrote:

> We should not use leading underscores followed by a capital letter
> in #defines since such identifiers are reserved by the C standard.
> 
> For ASCE_ORIGIN, REGION_ENTRY_ORIGIN and SEGMENT_ENTRY_ORIGIN I also
> added parentheses around the value to silence an error message from
> checkpatch.pl.
> 
> Signed-off-by: Thomas Huth 
> ---
>  target/s390x/cpu.h| 66 
> +++
>  target/s390x/mem_helper.c | 20 +++---
>  target/s390x/mmu_helper.c | 54 +++---
>  3 files changed, 70 insertions(+), 70 deletions(-)

Thanks, applied.

Re: [Qemu-devel] [PATCH] target/s390x: Remove leading underscores from #defines

2018-03-05 Thread David Hildenbrand 
On 05.03.2018 06:16, Thomas Huth wrote:
> We should not use leading underscores followed by a capital letter
> in #defines since such identifiers are reserved by the C standard.
> 

Didn't know about that but seems to be true.

> For ASCE_ORIGIN, REGION_ENTRY_ORIGIN and SEGMENT_ENTRY_ORIGIN I also
> added parentheses around the value to silence an error message from
> checkpatch.pl.
> 
> Signed-off-by: Thomas Huth 

I have a patch lying around that does basically the same thing but in
addition cleans up the names and definitions to match what the PoP
specifies. But this is fine for now - I'll just have to do some extra
work whenever I have time to rewrite the MMU code :)

Reviewed-by: David Hildenbrand 

> ---
>  target/s390x/cpu.h| 66 
> +++
>  target/s390x/mem_helper.c | 20 +++---
>  target/s390x/mmu_helper.c | 54 +++---
>  3 files changed, 70 insertions(+), 70 deletions(-)
> 
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index c5ef930..5f357a4e 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -538,39 +538,39 @@ typedef union SysIB {
>  QEMU_BUILD_BUG_ON(sizeof(SysIB) != 4096);
>  
>  /* MMU defines */
> -#define _ASCE_ORIGIN~0xfffULL /* segment table origin
>  */
> -#define _ASCE_SUBSPACE  0x200 /* subspace group control  
>  */
> -#define _ASCE_PRIVATE_SPACE 0x100 /* private space control   
>  */
> -#define _ASCE_ALT_EVENT 0x80  /* storage alteration event 
> control */
> -#define _ASCE_SPACE_SWITCH  0x40  /* space switch event  
>  */
> -#define _ASCE_REAL_SPACE0x20  /* real space control  
>  */
> -#define _ASCE_TYPE_MASK 0x0c  /* asce table type mask
>  */
> -#define _ASCE_TYPE_REGION1  0x0c  /* region first table type 
>  */
> -#define _ASCE_TYPE_REGION2  0x08  /* region second table type
>  */
> -#define _ASCE_TYPE_REGION3  0x04  /* region third table type 
>  */
> -#define _ASCE_TYPE_SEGMENT  0x00  /* segment table type  
>  */
> -#define _ASCE_TABLE_LENGTH  0x03  /* region table length 
>  */
> -
> -#define _REGION_ENTRY_ORIGIN~0xfffULL /* region/segment table origin 
>  */
> -#define _REGION_ENTRY_RO0x200 /* region/segment protection bit   
>  */
> -#define _REGION_ENTRY_TF0xc0  /* region/segment table offset 
>  */
> -#define _REGION_ENTRY_INV   0x20  /* invalid region table entry  
>  */
> -#define _REGION_ENTRY_TYPE_MASK 0x0c  /* region/segment table type mask  
>  */
> -#define _REGION_ENTRY_TYPE_R1   0x0c  /* region first table type 
>  */
> -#define _REGION_ENTRY_TYPE_R2   0x08  /* region second table type
>  */
> -#define _REGION_ENTRY_TYPE_R3   0x04  /* region third table type 
>  */
> -#define _REGION_ENTRY_LENGTH0x03  /* region third length 
>  */
> -
> -#define _SEGMENT_ENTRY_ORIGIN   ~0x7ffULL /* segment table origin
>  */
> -#define _SEGMENT_ENTRY_FC   0x400 /* format control  
>  */
> -#define _SEGMENT_ENTRY_RO   0x200 /* page protection bit 
>  */
> -#define _SEGMENT_ENTRY_INV  0x20  /* invalid segment table entry 
>  */
> -
> -#define VADDR_PX0xff000   /* page index bits 
>  */
> -
> -#define _PAGE_RO0x200/* HW read-only bit  */
> -#define _PAGE_INVALID   0x400/* HW invalid bit*/
> -#define _PAGE_RES0  0x800/* bit must be zero  */
> +#define ASCE_ORIGIN   (~0xfffULL) /* segment table origin
>  */
> +#define ASCE_SUBSPACE 0x200   /* subspace group control  
>  */
> +#define ASCE_PRIVATE_SPACE0x100   /* private space control   
>  */
> +#define ASCE_ALT_EVENT0x80/* storage alteration event 
> control */
> +#define ASCE_SPACE_SWITCH 0x40/* space switch event  
>  */
> +#define ASCE_REAL_SPACE   0x20/* real space control  
>  */
> +#define ASCE_TYPE_MASK0x0c/* asce table type mask
>  */
> +#define ASCE_TYPE_REGION1 0x0c/* region first table type 
>  */
> +#define ASCE_TYPE_REGION2 0x08/* region second table type
>  */
> +#define ASCE_TYPE_REGION3 0x04/* region third table type 
>  */
> +#define ASCE_TYPE_SEGMENT 0x00/* segment table type  
>  */
> +#define ASCE_TABLE_LENGTH 0x03/* region table length 
>  */
> +
> +#define REGION_ENTRY_ORIGIN   (~0xfffULL) /* region/segment table origin
> */
> +#define REGION_ENTRY_RO   0x200   /* region/segment protection bit  
> */
> +#define

[Qemu-devel] [PATCH] target/s390x: Remove leading underscores from #defines

2018-03-04 Thread Thomas Huth 
We should not use leading underscores followed by a capital letter
in #defines since such identifiers are reserved by the C standard.

For ASCE_ORIGIN, REGION_ENTRY_ORIGIN and SEGMENT_ENTRY_ORIGIN I also
added parentheses around the value to silence an error message from
checkpatch.pl.

Signed-off-by: Thomas Huth 
---
 target/s390x/cpu.h| 66 +++
 target/s390x/mem_helper.c | 20 +++---
 target/s390x/mmu_helper.c | 54 +++---
 3 files changed, 70 insertions(+), 70 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index c5ef930..5f357a4e 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -538,39 +538,39 @@ typedef union SysIB {
 QEMU_BUILD_BUG_ON(sizeof(SysIB) != 4096);
 
 /* MMU defines */
-#define _ASCE_ORIGIN~0xfffULL /* segment table origin 
*/
-#define _ASCE_SUBSPACE  0x200 /* subspace group control   
*/
-#define _ASCE_PRIVATE_SPACE 0x100 /* private space control
*/
-#define _ASCE_ALT_EVENT 0x80  /* storage alteration event control 
*/
-#define _ASCE_SPACE_SWITCH  0x40  /* space switch event   
*/
-#define _ASCE_REAL_SPACE0x20  /* real space control   
*/
-#define _ASCE_TYPE_MASK 0x0c  /* asce table type mask 
*/
-#define _ASCE_TYPE_REGION1  0x0c  /* region first table type  
*/
-#define _ASCE_TYPE_REGION2  0x08  /* region second table type 
*/
-#define _ASCE_TYPE_REGION3  0x04  /* region third table type  
*/
-#define _ASCE_TYPE_SEGMENT  0x00  /* segment table type   
*/
-#define _ASCE_TABLE_LENGTH  0x03  /* region table length  
*/
-
-#define _REGION_ENTRY_ORIGIN~0xfffULL /* region/segment table origin  
*/
-#define _REGION_ENTRY_RO0x200 /* region/segment protection bit
*/
-#define _REGION_ENTRY_TF0xc0  /* region/segment table offset  
*/
-#define _REGION_ENTRY_INV   0x20  /* invalid region table entry   
*/
-#define _REGION_ENTRY_TYPE_MASK 0x0c  /* region/segment table type mask   
*/
-#define _REGION_ENTRY_TYPE_R1   0x0c  /* region first table type  
*/
-#define _REGION_ENTRY_TYPE_R2   0x08  /* region second table type 
*/
-#define _REGION_ENTRY_TYPE_R3   0x04  /* region third table type  
*/
-#define _REGION_ENTRY_LENGTH0x03  /* region third length  
*/
-
-#define _SEGMENT_ENTRY_ORIGIN   ~0x7ffULL /* segment table origin 
*/
-#define _SEGMENT_ENTRY_FC   0x400 /* format control   
*/
-#define _SEGMENT_ENTRY_RO   0x200 /* page protection bit  
*/
-#define _SEGMENT_ENTRY_INV  0x20  /* invalid segment table entry  
*/
-
-#define VADDR_PX0xff000   /* page index bits  
*/
-
-#define _PAGE_RO0x200/* HW read-only bit  */
-#define _PAGE_INVALID   0x400/* HW invalid bit*/
-#define _PAGE_RES0  0x800/* bit must be zero  */
+#define ASCE_ORIGIN   (~0xfffULL) /* segment table origin 
*/
+#define ASCE_SUBSPACE 0x200   /* subspace group control   
*/
+#define ASCE_PRIVATE_SPACE0x100   /* private space control
*/
+#define ASCE_ALT_EVENT0x80/* storage alteration event control 
*/
+#define ASCE_SPACE_SWITCH 0x40/* space switch event   
*/
+#define ASCE_REAL_SPACE   0x20/* real space control   
*/
+#define ASCE_TYPE_MASK0x0c/* asce table type mask 
*/
+#define ASCE_TYPE_REGION1 0x0c/* region first table type  
*/
+#define ASCE_TYPE_REGION2 0x08/* region second table type 
*/
+#define ASCE_TYPE_REGION3 0x04/* region third table type  
*/
+#define ASCE_TYPE_SEGMENT 0x00/* segment table type   
*/
+#define ASCE_TABLE_LENGTH 0x03/* region table length  
*/
+
+#define REGION_ENTRY_ORIGIN   (~0xfffULL) /* region/segment table origin*/
+#define REGION_ENTRY_RO   0x200   /* region/segment protection bit  */
+#define REGION_ENTRY_TF   0xc0/* region/segment table offset*/
+#define REGION_ENTRY_INV  0x20/* invalid region table entry */
+#define REGION_ENTRY_TYPE_MASK 0x0c   /* region/segment table type mask */
+#define REGION_ENTRY_TYPE_R1  0x0c/* region first table type*/
+#define REGION_ENTRY_TYPE_R2  0x08/* region second table type   */
+#define REGION_ENTRY_TYPE_R3  0x04/* region third table type*/
+#define REGION_ENTRY_LENGTH   0x03/* region third length*/
+
+#define SEGMENT_ENTRY_ORIGIN  (~0x7ffULL) /* segment table origin*/