[kvm-unit-tests PATCH v2 0/3] s390x: Cleanup and maintenance

[kvm-unit-tests PATCH v2 0/3] s390x: Cleanup and maintenance

[Janosch Frank]

A bit more cleanup and some extensions before I start adding the PV
SIE support.

https://gitlab.com/frankja/kvm-unit-tests/-/tree/lib_clean_ext

v2:
	* Some of the small patches have been part of the pull and are hence dropped
	* Dropped the bitops patch, I'll add set_bit in the next series
	* Now using BIT_ULL
	* Added comment to decode_pgm_prot() stating we only decode
          the exceptions that are most likely relevant to tests
	* Moved PGM address translation functions to fault.c


Janosch Frank (3):
  lib: s390x: Print addressing related exception information
  s390x: uv-host: Explain why we set up the home space and remove the
    space change
  lib: s390x: Control register constant cleanup

 lib/s390x/asm/arch_def.h | 33 ++++++++---------
 lib/s390x/fault.c        | 76 ++++++++++++++++++++++++++++++++++++++++
 lib/s390x/fault.h        | 44 +++++++++++++++++++++++
 lib/s390x/interrupt.c    | 29 +++++++++++++--
 lib/s390x/smp.c          |  3 +-
 s390x/Makefile           |  1 +
 s390x/skrf.c             |  3 +-
 s390x/uv-host.c          | 11 ++++--
 8 files changed, 177 insertions(+), 23 deletions(-)
 create mode 100644 lib/s390x/fault.c
 create mode 100644 lib/s390x/fault.h

-- 
2.30.2

[kvm-unit-tests PATCH v2 1/3] lib: s390x: Print addressing related exception information

[Janosch Frank]

Right now we only get told the kind of program exception as well as
the PSW at the point where it happened.

For addressing exceptions the PSW is not always enough so let's print
the TEID which contains the failing address and flags that tell us
more about the kind of address exception.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/asm/arch_def.h |  5 +++
 lib/s390x/fault.c        | 76 ++++++++++++++++++++++++++++++++++++++++
 lib/s390x/fault.h        | 44 +++++++++++++++++++++++
 lib/s390x/interrupt.c    | 29 +++++++++++++--
 s390x/Makefile           |  1 +
 5 files changed, 153 insertions(+), 2 deletions(-)
 create mode 100644 lib/s390x/fault.c
 create mode 100644 lib/s390x/fault.h

diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 302ef1ff..ab5a9043 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -41,6 +41,11 @@ struct psw {
 	uint64_t	addr;
 };
 
+#define AS_PRIM				0
+#define AS_ACCR				1
+#define AS_SECN				2
+#define AS_HOME				3
+
 #define PSW_MASK_EXT			0x0100000000000000UL
 #define PSW_MASK_IO			0x0200000000000000UL
 #define PSW_MASK_DAT			0x0400000000000000UL
diff --git a/lib/s390x/fault.c b/lib/s390x/fault.c
new file mode 100644
index 00000000..fd1490e3
--- /dev/null
+++ b/lib/s390x/fault.c
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Library to decode addressing related exceptions
+ *
+ * Copyright 2021 IBM Corp.
+ *
+ * Authors:
+ *    Janosch Frank <frankja@linux.ibm.com>
+ */
+#include <libcflat.h>
+#include <bitops.h>
+#include <asm/arch_def.h>
+#include <asm/page.h>
+#include <fault.h>
+
+static struct lowcore *lc = (struct lowcore *)0x0;
+
+/* Decodes the protection exceptions we'll most likely see */
+static void decode_pgm_prot(uint64_t teid)
+{
+	if (prot_is_lap(teid)) {
+		printf("Type: LAP\n");
+		return;
+	}
+
+	if (prot_is_iep(teid)) {
+		printf("Type: IEP\n");
+		return;
+	}
+
+	if (prot_is_datp(teid)) {
+		printf("Type: DAT\n");
+		return;
+	}
+}
+
+void print_decode_teid(uint64_t teid)
+{
+	int asce_id = teid & 3;
+	bool dat = lc->pgm_old_psw.mask & PSW_MASK_DAT;
+
+	printf("Memory exception information:\n");
+	printf("DAT: %s\n", dat ? "on" : "off");
+
+	printf("AS: ");
+	switch (asce_id) {
+	case AS_PRIM:
+		printf("Primary\n");
+		break;
+	case AS_ACCR:
+		printf("Access Register\n");
+		break;
+	case AS_SECN:
+		printf("Secondary\n");
+		break;
+	case AS_HOME:
+		printf("Home\n");
+		break;
+	}
+
+	if (lc->pgm_int_code == PGM_INT_CODE_PROTECTION)
+		decode_pgm_prot(teid);
+
+	/*
+	 * If teid bit 61 is off for these two exception the reported
+	 * address is unpredictable.
+	 */
+	if ((lc->pgm_int_code == PGM_INT_CODE_SECURE_STOR_ACCESS ||
+	     lc->pgm_int_code == PGM_INT_CODE_SECURE_STOR_VIOLATION) &&
+	    !test_bit_inv(61, &teid)) {
+		printf("Address: %lx, unpredictable\n ", teid & PAGE_MASK);
+		return;
+	}
+	printf("TEID: %lx\n", teid);
+	printf("Address: %lx\n\n", teid & PAGE_MASK);
+}
diff --git a/lib/s390x/fault.h b/lib/s390x/fault.h
new file mode 100644
index 00000000..726da2f0
--- /dev/null
+++ b/lib/s390x/fault.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Headers for fault.c
+ *
+ * Copyright 2021 IBM Corp.
+ *
+ * Authors:
+ *    Janosch Frank <frankja@linux.ibm.com>
+ */
+#ifndef _S390X_FAULT_H_
+#define _S390X_FAULT_H_
+
+#include <bitops.h>
+
+/* Instruction execution prevention, i.e. no-execute, 101 */
+static inline bool prot_is_iep(uint64_t teid)
+{
+	if (test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && test_bit_inv(61, &teid))
+		return true;
+
+	return false;
+}
+
+/* Standard DAT exception, 001 */
+static inline bool prot_is_datp(uint64_t teid)
+{
+	if (!test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && test_bit_inv(61, &teid))
+		return true;
+
+	return false;
+}
+
+/* Low-address protection exception, 100 */
+static inline bool prot_is_lap(uint64_t teid)
+{
+	if (test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && !test_bit_inv(61, &teid))
+		return true;
+
+	return false;
+}
+
+void print_decode_teid(uint64_t teid);
+
+#endif /* _S390X_FAULT_H_ */
diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index 01ded49d..721e6758 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -12,6 +12,8 @@
 #include <sclp.h>
 #include <interrupt.h>
 #include <sie.h>
+#include <fault.h>
+#include <asm/page.h>
 
 static bool pgm_int_expected;
 static bool ext_int_expected;
@@ -76,8 +78,7 @@ static void fixup_pgm_int(struct stack_frame_int *stack)
 		break;
 	case PGM_INT_CODE_PROTECTION:
 		/* Handling for iep.c test case. */
-		if (lc->trans_exc_id & 0x80UL && lc->trans_exc_id & 0x04UL &&
-		    !(lc->trans_exc_id & 0x08UL))
+		if (prot_is_iep(lc->trans_exc_id))
 			/*
 			 * We branched to the instruction that caused
 			 * the exception so we can use the return
@@ -126,6 +127,26 @@ static void fixup_pgm_int(struct stack_frame_int *stack)
 	/* suppressed/terminated/completed point already at the next address */
 }
 
+static void print_storage_exception_information(void)
+{
+	switch (lc->pgm_int_code) {
+	case PGM_INT_CODE_PROTECTION:
+	case PGM_INT_CODE_PAGE_TRANSLATION:
+	case PGM_INT_CODE_SEGMENT_TRANSLATION:
+	case PGM_INT_CODE_ASCE_TYPE:
+	case PGM_INT_CODE_REGION_FIRST_TRANS:
+	case PGM_INT_CODE_REGION_SECOND_TRANS:
+	case PGM_INT_CODE_REGION_THIRD_TRANS:
+	case PGM_INT_CODE_SECURE_STOR_ACCESS:
+	case PGM_INT_CODE_NON_SECURE_STOR_ACCESS:
+	case PGM_INT_CODE_SECURE_STOR_VIOLATION:
+		print_decode_teid(lc->trans_exc_id);
+		break;
+	default:
+		return;
+	}
+}
+
 static void print_int_regs(struct stack_frame_int *stack)
 {
 	printf("\n");
@@ -155,6 +176,10 @@ static void print_pgm_info(struct stack_frame_int *stack)
 	       lc->pgm_int_code, stap(), lc->pgm_old_psw.addr, lc->pgm_int_id);
 	print_int_regs(stack);
 	dump_stack();
+
+	/* Dump stack doesn't end with a \n so we add it here instead */
+	printf("\n");
+	print_storage_exception_information();
 	report_summary();
 	abort();
 }
diff --git a/s390x/Makefile b/s390x/Makefile
index ef8041a6..5d1a33a0 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -72,6 +72,7 @@ cflatobjs += lib/s390x/css_lib.o
 cflatobjs += lib/s390x/malloc_io.o
 cflatobjs += lib/s390x/uv.o
 cflatobjs += lib/s390x/sie.o
+cflatobjs += lib/s390x/fault.o
 
 OBJDIRS += lib/s390x
 
-- 
2.30.2

Re: [kvm-unit-tests PATCH v2 1/3] lib: s390x: Print addressing related exception information

[Claudio Imbrenda]

On Fri, 20 Aug 2021 11:39:58 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> Right now we only get told the kind of program exception as well as
> the PSW at the point where it happened.
> 
> For addressing exceptions the PSW is not always enough so let's print
> the TEID which contains the failing address and flags that tell us
> more about the kind of address exception.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

(but see a small nit below)

> ---
>  lib/s390x/asm/arch_def.h |  5 +++
>  lib/s390x/fault.c        | 76 ++++++++++++++++++++++++++++++++++++++++
>  lib/s390x/fault.h        | 44 +++++++++++++++++++++++
>  lib/s390x/interrupt.c    | 29 +++++++++++++--
>  s390x/Makefile           |  1 +
>  5 files changed, 153 insertions(+), 2 deletions(-)
>  create mode 100644 lib/s390x/fault.c
>  create mode 100644 lib/s390x/fault.h
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 302ef1ff..ab5a9043 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -41,6 +41,11 @@ struct psw {
>  	uint64_t	addr;
>  };
>  
> +#define AS_PRIM				0
> +#define AS_ACCR				1
> +#define AS_SECN				2
> +#define AS_HOME				3
> +
>  #define PSW_MASK_EXT			0x0100000000000000UL
>  #define PSW_MASK_IO			0x0200000000000000UL
>  #define PSW_MASK_DAT			0x0400000000000000UL
> diff --git a/lib/s390x/fault.c b/lib/s390x/fault.c
> new file mode 100644
> index 00000000..fd1490e3
> --- /dev/null
> +++ b/lib/s390x/fault.c
> @@ -0,0 +1,76 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Library to decode addressing related exceptions
> + *
> + * Copyright 2021 IBM Corp.
> + *
> + * Authors:
> + *    Janosch Frank <frankja@linux.ibm.com>
> + */
> +#include <libcflat.h>
> +#include <bitops.h>
> +#include <asm/arch_def.h>
> +#include <asm/page.h>
> +#include <fault.h>
> +
> +static struct lowcore *lc = (struct lowcore *)0x0;
> +
> +/* Decodes the protection exceptions we'll most likely see */
> +static void decode_pgm_prot(uint64_t teid)

I think this is also a "print_decode_pgm_prot", but it's static
so I guess we don't really care too much

> +{
> +	if (prot_is_lap(teid)) {
> +		printf("Type: LAP\n");
> +		return;
> +	}
> +
> +	if (prot_is_iep(teid)) {
> +		printf("Type: IEP\n");
> +		return;
> +	}
> +
> +	if (prot_is_datp(teid)) {
> +		printf("Type: DAT\n");
> +		return;
> +	}
> +}
> +
> +void print_decode_teid(uint64_t teid)
> +{
> +	int asce_id = teid & 3;
> +	bool dat = lc->pgm_old_psw.mask & PSW_MASK_DAT;
> +
> +	printf("Memory exception information:\n");
> +	printf("DAT: %s\n", dat ? "on" : "off");
> +
> +	printf("AS: ");
> +	switch (asce_id) {
> +	case AS_PRIM:
> +		printf("Primary\n");
> +		break;
> +	case AS_ACCR:
> +		printf("Access Register\n");
> +		break;
> +	case AS_SECN:
> +		printf("Secondary\n");
> +		break;
> +	case AS_HOME:
> +		printf("Home\n");
> +		break;
> +	}
> +
> +	if (lc->pgm_int_code == PGM_INT_CODE_PROTECTION)
> +		decode_pgm_prot(teid);
> +
> +	/*
> +	 * If teid bit 61 is off for these two exception the reported
> +	 * address is unpredictable.
> +	 */
> +	if ((lc->pgm_int_code == PGM_INT_CODE_SECURE_STOR_ACCESS ||
> +	     lc->pgm_int_code == PGM_INT_CODE_SECURE_STOR_VIOLATION) &&
> +	    !test_bit_inv(61, &teid)) {
> +		printf("Address: %lx, unpredictable\n ", teid & PAGE_MASK);
> +		return;
> +	}
> +	printf("TEID: %lx\n", teid);
> +	printf("Address: %lx\n\n", teid & PAGE_MASK);
> +}
> diff --git a/lib/s390x/fault.h b/lib/s390x/fault.h
> new file mode 100644
> index 00000000..726da2f0
> --- /dev/null
> +++ b/lib/s390x/fault.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Headers for fault.c
> + *
> + * Copyright 2021 IBM Corp.
> + *
> + * Authors:
> + *    Janosch Frank <frankja@linux.ibm.com>
> + */
> +#ifndef _S390X_FAULT_H_
> +#define _S390X_FAULT_H_
> +
> +#include <bitops.h>
> +
> +/* Instruction execution prevention, i.e. no-execute, 101 */
> +static inline bool prot_is_iep(uint64_t teid)
> +{
> +	if (test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && test_bit_inv(61, &teid))
> +		return true;
> +
> +	return false;
> +}
> +
> +/* Standard DAT exception, 001 */
> +static inline bool prot_is_datp(uint64_t teid)
> +{
> +	if (!test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && test_bit_inv(61, &teid))
> +		return true;
> +
> +	return false;
> +}
> +
> +/* Low-address protection exception, 100 */
> +static inline bool prot_is_lap(uint64_t teid)
> +{
> +	if (test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && !test_bit_inv(61, &teid))
> +		return true;
> +
> +	return false;
> +}
> +
> +void print_decode_teid(uint64_t teid);
> +
> +#endif /* _S390X_FAULT_H_ */
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index 01ded49d..721e6758 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -12,6 +12,8 @@
>  #include <sclp.h>
>  #include <interrupt.h>
>  #include <sie.h>
> +#include <fault.h>
> +#include <asm/page.h>
>  
>  static bool pgm_int_expected;
>  static bool ext_int_expected;
> @@ -76,8 +78,7 @@ static void fixup_pgm_int(struct stack_frame_int *stack)
>  		break;
>  	case PGM_INT_CODE_PROTECTION:
>  		/* Handling for iep.c test case. */
> -		if (lc->trans_exc_id & 0x80UL && lc->trans_exc_id & 0x04UL &&
> -		    !(lc->trans_exc_id & 0x08UL))
> +		if (prot_is_iep(lc->trans_exc_id))
>  			/*
>  			 * We branched to the instruction that caused
>  			 * the exception so we can use the return
> @@ -126,6 +127,26 @@ static void fixup_pgm_int(struct stack_frame_int *stack)
>  	/* suppressed/terminated/completed point already at the next address */
>  }
>  
> +static void print_storage_exception_information(void)
> +{
> +	switch (lc->pgm_int_code) {
> +	case PGM_INT_CODE_PROTECTION:
> +	case PGM_INT_CODE_PAGE_TRANSLATION:
> +	case PGM_INT_CODE_SEGMENT_TRANSLATION:
> +	case PGM_INT_CODE_ASCE_TYPE:
> +	case PGM_INT_CODE_REGION_FIRST_TRANS:
> +	case PGM_INT_CODE_REGION_SECOND_TRANS:
> +	case PGM_INT_CODE_REGION_THIRD_TRANS:
> +	case PGM_INT_CODE_SECURE_STOR_ACCESS:
> +	case PGM_INT_CODE_NON_SECURE_STOR_ACCESS:
> +	case PGM_INT_CODE_SECURE_STOR_VIOLATION:
> +		print_decode_teid(lc->trans_exc_id);
> +		break;
> +	default:
> +		return;
> +	}
> +}
> +
>  static void print_int_regs(struct stack_frame_int *stack)
>  {
>  	printf("\n");
> @@ -155,6 +176,10 @@ static void print_pgm_info(struct stack_frame_int *stack)
>  	       lc->pgm_int_code, stap(), lc->pgm_old_psw.addr, lc->pgm_int_id);
>  	print_int_regs(stack);
>  	dump_stack();
> +
> +	/* Dump stack doesn't end with a \n so we add it here instead */
> +	printf("\n");
> +	print_storage_exception_information();
>  	report_summary();
>  	abort();
>  }
> diff --git a/s390x/Makefile b/s390x/Makefile
> index ef8041a6..5d1a33a0 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -72,6 +72,7 @@ cflatobjs += lib/s390x/css_lib.o
>  cflatobjs += lib/s390x/malloc_io.o
>  cflatobjs += lib/s390x/uv.o
>  cflatobjs += lib/s390x/sie.o
> +cflatobjs += lib/s390x/fault.o
>  
>  OBJDIRS += lib/s390x
>  

Re: [kvm-unit-tests PATCH v2 1/3] lib: s390x: Print addressing related exception information

[Janosch Frank]

On 8/20/21 1:48 PM, Claudio Imbrenda wrote:
> On Fri, 20 Aug 2021 11:39:58 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Right now we only get told the kind of program exception as well as
>> the PSW at the point where it happened.
>>
>> For addressing exceptions the PSW is not always enough so let's print
>> the TEID which contains the failing address and flags that tell us
>> more about the kind of address exception.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> 
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Thanks

> 
> (but see a small nit below)
> 
>> ---
>>  lib/s390x/asm/arch_def.h |  5 +++
>>  lib/s390x/fault.c        | 76 ++++++++++++++++++++++++++++++++++++++++
>>  lib/s390x/fault.h        | 44 +++++++++++++++++++++++
>>  lib/s390x/interrupt.c    | 29 +++++++++++++--
>>  s390x/Makefile           |  1 +
>>  5 files changed, 153 insertions(+), 2 deletions(-)
>>  create mode 100644 lib/s390x/fault.c
>>  create mode 100644 lib/s390x/fault.h
>>
>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
>> index 302ef1ff..ab5a9043 100644
>> --- a/lib/s390x/asm/arch_def.h
>> +++ b/lib/s390x/asm/arch_def.h
>> @@ -41,6 +41,11 @@ struct psw {
>>  	uint64_t	addr;
>>  };
>>  
>> +#define AS_PRIM				0
>> +#define AS_ACCR				1
>> +#define AS_SECN				2
>> +#define AS_HOME				3
>> +
>>  #define PSW_MASK_EXT			0x0100000000000000UL
>>  #define PSW_MASK_IO			0x0200000000000000UL
>>  #define PSW_MASK_DAT			0x0400000000000000UL
>> diff --git a/lib/s390x/fault.c b/lib/s390x/fault.c
>> new file mode 100644
>> index 00000000..fd1490e3
>> --- /dev/null
>> +++ b/lib/s390x/fault.c
>> @@ -0,0 +1,76 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Library to decode addressing related exceptions
>> + *
>> + * Copyright 2021 IBM Corp.
>> + *
>> + * Authors:
>> + *    Janosch Frank <frankja@linux.ibm.com>
>> + */
>> +#include <libcflat.h>
>> +#include <bitops.h>
>> +#include <asm/arch_def.h>
>> +#include <asm/page.h>
>> +#include <fault.h>
>> +
>> +static struct lowcore *lc = (struct lowcore *)0x0;
>> +
>> +/* Decodes the protection exceptions we'll most likely see */
>> +static void decode_pgm_prot(uint64_t teid)
> 
> I think this is also a "print_decode_pgm_prot", but it's static
> so I guess we don't really care too much

Can do, sounds reasonable

> 
>> +{
>> +	if (prot_is_lap(teid)) {
>> +		printf("Type: LAP\n");
>> +		return;
>> +	}
>> +
>> +	if (prot_is_iep(teid)) {
>> +		printf("Type: IEP\n");
>> +		return;
>> +	}
>> +
>> +	if (prot_is_datp(teid)) {
>> +		printf("Type: DAT\n");
>> +		return;
>> +	}
>> +}
>> +
>> +void print_decode_teid(uint64_t teid)
>> +{
>> +	int asce_id = teid & 3;
>> +	bool dat = lc->pgm_old_psw.mask & PSW_MASK_DAT;
>> +
>> +	printf("Memory exception information:\n");
>> +	printf("DAT: %s\n", dat ? "on" : "off");
>> +
>> +	printf("AS: ");
>> +	switch (asce_id) {
>> +	case AS_PRIM:
>> +		printf("Primary\n");
>> +		break;
>> +	case AS_ACCR:
>> +		printf("Access Register\n");
>> +		break;
>> +	case AS_SECN:
>> +		printf("Secondary\n");
>> +		break;
>> +	case AS_HOME:
>> +		printf("Home\n");
>> +		break;
>> +	}
>> +
>> +	if (lc->pgm_int_code == PGM_INT_CODE_PROTECTION)
>> +		decode_pgm_prot(teid);
>> +
>> +	/*
>> +	 * If teid bit 61 is off for these two exception the reported
>> +	 * address is unpredictable.
>> +	 */
>> +	if ((lc->pgm_int_code == PGM_INT_CODE_SECURE_STOR_ACCESS ||
>> +	     lc->pgm_int_code == PGM_INT_CODE_SECURE_STOR_VIOLATION) &&
>> +	    !test_bit_inv(61, &teid)) {
>> +		printf("Address: %lx, unpredictable\n ", teid & PAGE_MASK);
>> +		return;
>> +	}
>> +	printf("TEID: %lx\n", teid);
>> +	printf("Address: %lx\n\n", teid & PAGE_MASK);
>> +}
>> diff --git a/lib/s390x/fault.h b/lib/s390x/fault.h
>> new file mode 100644
>> index 00000000..726da2f0
>> --- /dev/null
>> +++ b/lib/s390x/fault.h
>> @@ -0,0 +1,44 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Headers for fault.c
>> + *
>> + * Copyright 2021 IBM Corp.
>> + *
>> + * Authors:
>> + *    Janosch Frank <frankja@linux.ibm.com>
>> + */
>> +#ifndef _S390X_FAULT_H_
>> +#define _S390X_FAULT_H_
>> +
>> +#include <bitops.h>
>> +
>> +/* Instruction execution prevention, i.e. no-execute, 101 */
>> +static inline bool prot_is_iep(uint64_t teid)
>> +{
>> +	if (test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && test_bit_inv(61, &teid))
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>> +/* Standard DAT exception, 001 */
>> +static inline bool prot_is_datp(uint64_t teid)
>> +{
>> +	if (!test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && test_bit_inv(61, &teid))
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>> +/* Low-address protection exception, 100 */
>> +static inline bool prot_is_lap(uint64_t teid)
>> +{
>> +	if (test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && !test_bit_inv(61, &teid))
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>> +void print_decode_teid(uint64_t teid);
>> +
>> +#endif /* _S390X_FAULT_H_ */
>> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
>> index 01ded49d..721e6758 100644
>> --- a/lib/s390x/interrupt.c
>> +++ b/lib/s390x/interrupt.c
>> @@ -12,6 +12,8 @@
>>  #include <sclp.h>
>>  #include <interrupt.h>
>>  #include <sie.h>
>> +#include <fault.h>
>> +#include <asm/page.h>
>>  
>>  static bool pgm_int_expected;
>>  static bool ext_int_expected;
>> @@ -76,8 +78,7 @@ static void fixup_pgm_int(struct stack_frame_int *stack)
>>  		break;
>>  	case PGM_INT_CODE_PROTECTION:
>>  		/* Handling for iep.c test case. */
>> -		if (lc->trans_exc_id & 0x80UL && lc->trans_exc_id & 0x04UL &&
>> -		    !(lc->trans_exc_id & 0x08UL))
>> +		if (prot_is_iep(lc->trans_exc_id))
>>  			/*
>>  			 * We branched to the instruction that caused
>>  			 * the exception so we can use the return
>> @@ -126,6 +127,26 @@ static void fixup_pgm_int(struct stack_frame_int *stack)
>>  	/* suppressed/terminated/completed point already at the next address */
>>  }
>>  
>> +static void print_storage_exception_information(void)
>> +{
>> +	switch (lc->pgm_int_code) {
>> +	case PGM_INT_CODE_PROTECTION:
>> +	case PGM_INT_CODE_PAGE_TRANSLATION:
>> +	case PGM_INT_CODE_SEGMENT_TRANSLATION:
>> +	case PGM_INT_CODE_ASCE_TYPE:
>> +	case PGM_INT_CODE_REGION_FIRST_TRANS:
>> +	case PGM_INT_CODE_REGION_SECOND_TRANS:
>> +	case PGM_INT_CODE_REGION_THIRD_TRANS:
>> +	case PGM_INT_CODE_SECURE_STOR_ACCESS:
>> +	case PGM_INT_CODE_NON_SECURE_STOR_ACCESS:
>> +	case PGM_INT_CODE_SECURE_STOR_VIOLATION:
>> +		print_decode_teid(lc->trans_exc_id);
>> +		break;
>> +	default:
>> +		return;
>> +	}
>> +}
>> +
>>  static void print_int_regs(struct stack_frame_int *stack)
>>  {
>>  	printf("\n");
>> @@ -155,6 +176,10 @@ static void print_pgm_info(struct stack_frame_int *stack)
>>  	       lc->pgm_int_code, stap(), lc->pgm_old_psw.addr, lc->pgm_int_id);
>>  	print_int_regs(stack);
>>  	dump_stack();
>> +
>> +	/* Dump stack doesn't end with a \n so we add it here instead */
>> +	printf("\n");
>> +	print_storage_exception_information();
>>  	report_summary();
>>  	abort();
>>  }
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index ef8041a6..5d1a33a0 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -72,6 +72,7 @@ cflatobjs += lib/s390x/css_lib.o
>>  cflatobjs += lib/s390x/malloc_io.o
>>  cflatobjs += lib/s390x/uv.o
>>  cflatobjs += lib/s390x/sie.o
>> +cflatobjs += lib/s390x/fault.o
>>  
>>  OBJDIRS += lib/s390x
>>  
> 

Re: [kvm-unit-tests PATCH v2 1/3] lib: s390x: Print addressing related exception information

[Thomas Huth]

On 20/08/2021 13.39, Janosch Frank wrote:
> Right now we only get told the kind of program exception as well as
> the PSW at the point where it happened.
> 
> For addressing exceptions the PSW is not always enough so let's print
> the TEID which contains the failing address and flags that tell us
> more about the kind of address exception.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
...
> --- /dev/null
> +++ b/lib/s390x/fault.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Headers for fault.c
> + *
> + * Copyright 2021 IBM Corp.
> + *
> + * Authors:
> + *    Janosch Frank <frankja@linux.ibm.com>
> + */
> +#ifndef _S390X_FAULT_H_
> +#define _S390X_FAULT_H_
> +
> +#include <bitops.h>
> +
> +/* Instruction execution prevention, i.e. no-execute, 101 */
> +static inline bool prot_is_iep(uint64_t teid)
> +{
> +	if (test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && test_bit_inv(61, &teid))
> +		return true;
> +
> +	return false;

You could simplify this into:

	return test_bit_inv(56, &teid) &&
                !test_bit_inv(60, &teid) &&
                test_bit_inv(61, &teid);

... but I don't mind too much if you keep the current version.

> +}
> +
> +/* Standard DAT exception, 001 */
> +static inline bool prot_is_datp(uint64_t teid)
> +{
> +	if (!test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && test_bit_inv(61, &teid))
> +		return true;
> +
> +	return false;

dito

> +}
> +
> +/* Low-address protection exception, 100 */
> +static inline bool prot_is_lap(uint64_t teid)
> +{
> +	if (test_bit_inv(56, &teid) && !test_bit_inv(60, &teid) && !test_bit_inv(61, &teid))
> +		return true;
> +
> +	return false;

dito

> +}
> +
> +void print_decode_teid(uint64_t teid);
> +
> +#endif /* _S390X_FAULT_H_ */
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index 01ded49d..721e6758 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -12,6 +12,8 @@
>   #include <sclp.h>
>   #include <interrupt.h>
>   #include <sie.h>
> +#include <fault.h>
> +#include <asm/page.h>
>   
>   static bool pgm_int_expected;
>   static bool ext_int_expected;
> @@ -76,8 +78,7 @@ static void fixup_pgm_int(struct stack_frame_int *stack)
>   		break;
>   	case PGM_INT_CODE_PROTECTION:
>   		/* Handling for iep.c test case. */
> -		if (lc->trans_exc_id & 0x80UL && lc->trans_exc_id & 0x04UL &&
> -		    !(lc->trans_exc_id & 0x08UL))
> +		if (prot_is_iep(lc->trans_exc_id))
>   			/*
>   			 * We branched to the instruction that caused
>   			 * the exception so we can use the return
> @@ -126,6 +127,26 @@ static void fixup_pgm_int(struct stack_frame_int *stack)
>   	/* suppressed/terminated/completed point already at the next address */
>   }
>   
> +static void print_storage_exception_information(void)
> +{
> +	switch (lc->pgm_int_code) {
> +	case PGM_INT_CODE_PROTECTION:
> +	case PGM_INT_CODE_PAGE_TRANSLATION:
> +	case PGM_INT_CODE_SEGMENT_TRANSLATION:
> +	case PGM_INT_CODE_ASCE_TYPE:
> +	case PGM_INT_CODE_REGION_FIRST_TRANS:
> +	case PGM_INT_CODE_REGION_SECOND_TRANS:
> +	case PGM_INT_CODE_REGION_THIRD_TRANS:
> +	case PGM_INT_CODE_SECURE_STOR_ACCESS:
> +	case PGM_INT_CODE_NON_SECURE_STOR_ACCESS:
> +	case PGM_INT_CODE_SECURE_STOR_VIOLATION:
> +		print_decode_teid(lc->trans_exc_id);
> +		break;
> +	default:
> +		return;

Drop the default case?

> +	}
> +}
> +
>   static void print_int_regs(struct stack_frame_int *stack)
>   {
>   	printf("\n");
> @@ -155,6 +176,10 @@ static void print_pgm_info(struct stack_frame_int *stack)
>   	       lc->pgm_int_code, stap(), lc->pgm_old_psw.addr, lc->pgm_int_id);
>   	print_int_regs(stack);
>   	dump_stack();
> +
> +	/* Dump stack doesn't end with a \n so we add it here instead */
> +	printf("\n");
> +	print_storage_exception_information();
>   	report_summary();
>   	abort();
>   }
> diff --git a/s390x/Makefile b/s390x/Makefile
> index ef8041a6..5d1a33a0 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -72,6 +72,7 @@ cflatobjs += lib/s390x/css_lib.o
>   cflatobjs += lib/s390x/malloc_io.o
>   cflatobjs += lib/s390x/uv.o
>   cflatobjs += lib/s390x/sie.o
> +cflatobjs += lib/s390x/fault.o
>   
>   OBJDIRS += lib/s390x
>   
> 

Some nits, but looks fine to me anyway:

Reviewed-by: Thomas Huth <thuth@redhat.com>

[kvm-unit-tests PATCH v2 2/3] s390x: uv-host: Explain why we set up the home space and remove the space change

[Janosch Frank]

UV home addresses don't require us to be in home space but we need to
have it set up so hw/fw can use the home asce to translate home
virtual addresses.

Hence we add a comment why we're setting up the home asce and remove
the address space since it's unneeded.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 s390x/uv-host.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/s390x/uv-host.c b/s390x/uv-host.c
index 426a67f6..28035707 100644
--- a/s390x/uv-host.c
+++ b/s390x/uv-host.c
@@ -444,13 +444,18 @@ static void test_clear(void)
 
 static void setup_vmem(void)
 {
-	uint64_t asce, mask;
+	uint64_t asce;
 
 	setup_mmu(get_max_ram_size(), NULL);
+	/*
+	 * setup_mmu() will enable DAT and set the primary address
+	 * space but we need to have a valid home space since UV calls
+	 * take home space virtual addresses.
+	 *
+	 * Hence we just copy the primary asce into the home space.
+	 */
 	asce = stctg(1);
 	lctlg(13, asce);
-	mask = extract_psw_mask() | 0x0000C00000000000UL;
-	load_psw_mask(mask);
 }
 
 int main(void)
-- 
2.30.2

Re: [kvm-unit-tests PATCH v2 2/3] s390x: uv-host: Explain why we set up the home space and remove the space change

[Thomas Huth]

On 20/08/2021 13.39, Janosch Frank wrote:
> UV home addresses don't require us to be in home space but we need to
> have it set up so hw/fw can use the home asce to translate home
> virtual addresses.
> 
> Hence we add a comment why we're setting up the home asce and remove
> the address space since it's unneeded.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
>   s390x/uv-host.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/s390x/uv-host.c b/s390x/uv-host.c
> index 426a67f6..28035707 100644
> --- a/s390x/uv-host.c
> +++ b/s390x/uv-host.c
> @@ -444,13 +444,18 @@ static void test_clear(void)
>   
>   static void setup_vmem(void)
>   {
> -	uint64_t asce, mask;
> +	uint64_t asce;
>   
>   	setup_mmu(get_max_ram_size(), NULL);
> +	/*
> +	 * setup_mmu() will enable DAT and set the primary address
> +	 * space but we need to have a valid home space since UV calls
> +	 * take home space virtual addresses.
> +	 *
> +	 * Hence we just copy the primary asce into the home space.
> +	 */
>   	asce = stctg(1);
>   	lctlg(13, asce);
> -	mask = extract_psw_mask() | 0x0000C00000000000UL;
> -	load_psw_mask(mask);
>   }
>   
>   int main(void)
> 

Acked-by: Thomas Huth <thuth@redhat.com>

[kvm-unit-tests PATCH v2 3/3] lib: s390x: Control register constant cleanup

[Janosch Frank]

We had bits and masks defined and don't necessarily need both.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/asm/arch_def.h | 28 ++++++++++++----------------
 lib/s390x/smp.c          |  3 ++-
 s390x/skrf.c             |  3 ++-
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index ab5a9043..aa80d840 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -55,10 +55,18 @@ struct psw {
 #define PSW_MASK_BA			0x0000000080000000UL
 #define PSW_MASK_64			PSW_MASK_BA | PSW_MASK_EA;
 
-#define CR0_EXTM_SCLP			0x0000000000000200UL
-#define CR0_EXTM_EXTC			0x0000000000002000UL
-#define CR0_EXTM_EMGC			0x0000000000004000UL
-#define CR0_EXTM_MASK			0x0000000000006200UL
+#define CTL0_LOW_ADDR_PROT		(63 - 35)
+#define CTL0_EDAT			(63 - 40)
+#define CTL0_IEP			(63 - 43)
+#define CTL0_AFP			(63 - 45)
+#define CTL0_VECTOR			(63 - 46)
+#define CTL0_EMERGENCY_SIGNAL		(63 - 49)
+#define CTL0_EXTERNAL_CALL		(63 - 50)
+#define CTL0_CLOCK_COMPARATOR		(63 - 52)
+#define CTL0_SERVICE_SIGNAL		(63 - 54)
+#define CR0_EXTM_MASK			0x0000000000006200UL /* Combined external masks */
+
+#define CTL2_GUARDED_STORAGE		(63 - 59)
 
 struct lowcore {
 	uint8_t		pad_0x0000[0x0080 - 0x0000];	/* 0x0000 */
@@ -240,18 +248,6 @@ static inline uint64_t stctg(int cr)
 	return value;
 }
 
-#define CTL0_LOW_ADDR_PROT	(63 - 35)
-#define CTL0_EDAT		(63 - 40)
-#define CTL0_IEP		(63 - 43)
-#define CTL0_AFP		(63 - 45)
-#define CTL0_VECTOR		(63 - 46)
-#define CTL0_EMERGENCY_SIGNAL	(63 - 49)
-#define CTL0_EXTERNAL_CALL	(63 - 50)
-#define CTL0_CLOCK_COMPARATOR	(63 - 52)
-#define CTL0_SERVICE_SIGNAL	(63 - 54)
-
-#define CTL2_GUARDED_STORAGE	(63 - 59)
-
 static inline void ctl_set_bit(int cr, unsigned int bit)
 {
         uint64_t reg;
diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
index 228fe667..da6d32f3 100644
--- a/lib/s390x/smp.c
+++ b/lib/s390x/smp.c
@@ -10,6 +10,7 @@
  *  Janosch Frank <frankja@linux.ibm.com>
  */
 #include <libcflat.h>
+#include <bitops.h>
 #include <asm/arch_def.h>
 #include <asm/sigp.h>
 #include <asm/page.h>
@@ -204,7 +205,7 @@ int smp_cpu_setup(uint16_t addr, struct psw psw)
 	cpu->lowcore->sw_int_grs[15] = (uint64_t)cpu->stack + (PAGE_SIZE * 4);
 	lc->restart_new_psw.mask = PSW_MASK_64;
 	lc->restart_new_psw.addr = (uint64_t)smp_cpu_setup_state;
-	lc->sw_int_crs[0] = 0x0000000000040000UL;
+	lc->sw_int_crs[0] = BIT_ULL(CTL0_AFP);
 
 	/* Start processing */
 	smp_cpu_restart_nolock(addr, NULL);
diff --git a/s390x/skrf.c b/s390x/skrf.c
index 9488c32b..8ca7588c 100644
--- a/s390x/skrf.c
+++ b/s390x/skrf.c
@@ -8,6 +8,7 @@
  *  Janosch Frank <frankja@linux.ibm.com>
  */
 #include <libcflat.h>
+#include <bitops.h>
 #include <asm/asm-offsets.h>
 #include <asm-generic/barrier.h>
 #include <asm/interrupt.h>
@@ -125,8 +126,8 @@ static void ecall_cleanup(void)
 {
 	struct lowcore *lc = (void *)0x0;
 
-	lc->sw_int_crs[0] = 0x0000000000040000;
 	lc->ext_new_psw.mask = PSW_MASK_64;
+	lc->sw_int_crs[0] = BIT_ULL(CTL0_AFP);
 
 	/*
 	 * PGM old contains the ext new PSW, we need to clean it up,
-- 
2.30.2

Re: [kvm-unit-tests PATCH v2 3/3] lib: s390x: Control register constant cleanup

[Claudio Imbrenda]

On Fri, 20 Aug 2021 11:40:00 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> We had bits and masks defined and don't necessarily need both.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  lib/s390x/asm/arch_def.h | 28 ++++++++++++----------------
>  lib/s390x/smp.c          |  3 ++-
>  s390x/skrf.c             |  3 ++-
>  3 files changed, 16 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index ab5a9043..aa80d840 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -55,10 +55,18 @@ struct psw {
>  #define PSW_MASK_BA			0x0000000080000000UL
>  #define PSW_MASK_64			PSW_MASK_BA | PSW_MASK_EA;
>  
> -#define CR0_EXTM_SCLP			0x0000000000000200UL
> -#define CR0_EXTM_EXTC			0x0000000000002000UL
> -#define CR0_EXTM_EMGC			0x0000000000004000UL
> -#define CR0_EXTM_MASK			0x0000000000006200UL
> +#define CTL0_LOW_ADDR_PROT		(63 - 35)
> +#define CTL0_EDAT			(63 - 40)
> +#define CTL0_IEP			(63 - 43)
> +#define CTL0_AFP			(63 - 45)
> +#define CTL0_VECTOR			(63 - 46)
> +#define CTL0_EMERGENCY_SIGNAL		(63 - 49)
> +#define CTL0_EXTERNAL_CALL		(63 - 50)
> +#define CTL0_CLOCK_COMPARATOR		(63 - 52)
> +#define CTL0_SERVICE_SIGNAL		(63 - 54)
> +#define CR0_EXTM_MASK			0x0000000000006200UL /* Combined external masks */
> +
> +#define CTL2_GUARDED_STORAGE		(63 - 59)
>  
>  struct lowcore {
>  	uint8_t		pad_0x0000[0x0080 - 0x0000];	/* 0x0000 */
> @@ -240,18 +248,6 @@ static inline uint64_t stctg(int cr)
>  	return value;
>  }
>  
> -#define CTL0_LOW_ADDR_PROT	(63 - 35)
> -#define CTL0_EDAT		(63 - 40)
> -#define CTL0_IEP		(63 - 43)
> -#define CTL0_AFP		(63 - 45)
> -#define CTL0_VECTOR		(63 - 46)
> -#define CTL0_EMERGENCY_SIGNAL	(63 - 49)
> -#define CTL0_EXTERNAL_CALL	(63 - 50)
> -#define CTL0_CLOCK_COMPARATOR	(63 - 52)
> -#define CTL0_SERVICE_SIGNAL	(63 - 54)
> -
> -#define CTL2_GUARDED_STORAGE	(63 - 59)
> -
>  static inline void ctl_set_bit(int cr, unsigned int bit)
>  {
>          uint64_t reg;
> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
> index 228fe667..da6d32f3 100644
> --- a/lib/s390x/smp.c
> +++ b/lib/s390x/smp.c
> @@ -10,6 +10,7 @@
>   *  Janosch Frank <frankja@linux.ibm.com>
>   */
>  #include <libcflat.h>
> +#include <bitops.h>
>  #include <asm/arch_def.h>
>  #include <asm/sigp.h>
>  #include <asm/page.h>
> @@ -204,7 +205,7 @@ int smp_cpu_setup(uint16_t addr, struct psw psw)
>  	cpu->lowcore->sw_int_grs[15] = (uint64_t)cpu->stack + (PAGE_SIZE * 4);
>  	lc->restart_new_psw.mask = PSW_MASK_64;
>  	lc->restart_new_psw.addr = (uint64_t)smp_cpu_setup_state;
> -	lc->sw_int_crs[0] = 0x0000000000040000UL;
> +	lc->sw_int_crs[0] = BIT_ULL(CTL0_AFP);
>  
>  	/* Start processing */
>  	smp_cpu_restart_nolock(addr, NULL);
> diff --git a/s390x/skrf.c b/s390x/skrf.c
> index 9488c32b..8ca7588c 100644
> --- a/s390x/skrf.c
> +++ b/s390x/skrf.c
> @@ -8,6 +8,7 @@
>   *  Janosch Frank <frankja@linux.ibm.com>
>   */
>  #include <libcflat.h>
> +#include <bitops.h>
>  #include <asm/asm-offsets.h>
>  #include <asm-generic/barrier.h>
>  #include <asm/interrupt.h>
> @@ -125,8 +126,8 @@ static void ecall_cleanup(void)
>  {
>  	struct lowcore *lc = (void *)0x0;
>  
> -	lc->sw_int_crs[0] = 0x0000000000040000;
>  	lc->ext_new_psw.mask = PSW_MASK_64;
> +	lc->sw_int_crs[0] = BIT_ULL(CTL0_AFP);
>  
>  	/*
>  	 * PGM old contains the ext new PSW, we need to clean it up,

Re: [kvm-unit-tests PATCH v2 3/3] lib: s390x: Control register constant cleanup

[Thomas Huth]

On 20/08/2021 13.40, Janosch Frank wrote:
> We had bits and masks defined and don't necessarily need both.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   lib/s390x/asm/arch_def.h | 28 ++++++++++++----------------
>   lib/s390x/smp.c          |  3 ++-
>   s390x/skrf.c             |  3 ++-
>   3 files changed, 16 insertions(+), 18 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>