All of lore.kernel.org
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH v2 1/2] s390x: Remove assert from arch_def.h
       [not found] <20211005091153.1863139-1-scgl@linux.ibm.com>
@ 2021-10-05  9:11 ` Janis Schoetterl-Glausch
  2021-10-05 11:19   ` [kvm-unit-tests PATCH v2 1/2] [kvm-unit-tests PATCH v2 0/2] Test specification exception Janis Schoetterl-Glausch
                     ` (2 more replies)
  2021-10-05  9:11 ` [kvm-unit-tests PATCH v2 2/2] s390x: Add specification exception interception test Janis Schoetterl-Glausch
  1 sibling, 3 replies; 12+ messages in thread
From: Janis Schoetterl-Glausch @ 2021-10-05  9:11 UTC (permalink / raw)
  To: Thomas Huth, Janosch Frank
  Cc: Janis Schoetterl-Glausch, Pierre Morel, Cornelia Huck,
	Claudio Imbrenda, David Hildenbrand, kvm, linux-s390

Do not use asserts in arch_def.h so it can be included by snippets.
The caller in stsi.c does not need to be adjusted, returning -1 causes
the test to fail.

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---
 lib/s390x/asm/arch_def.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
index 302ef1f..4167e2b 100644
--- a/lib/s390x/asm/arch_def.h
+++ b/lib/s390x/asm/arch_def.h
@@ -334,7 +334,7 @@ static inline int stsi(void *addr, int fc, int sel1, int sel2)
 	return cc;
 }
 
-static inline unsigned long stsi_get_fc(void)
+static inline int stsi_get_fc(void)
 {
 	register unsigned long r0 asm("0") = 0;
 	register unsigned long r1 asm("1") = 0;
@@ -346,7 +346,8 @@ static inline unsigned long stsi_get_fc(void)
 		     : "+d" (r0), [cc] "=d" (cc)
 		     : "d" (r1)
 		     : "cc", "memory");
-	assert(!cc);
+	if (cc != 0)
+		return -1;
 	return r0 >> 28;
 }
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [kvm-unit-tests PATCH v2 2/2] s390x: Add specification exception interception test
       [not found] <20211005091153.1863139-1-scgl@linux.ibm.com>
  2021-10-05  9:11 ` [kvm-unit-tests PATCH v2 1/2] s390x: Remove assert from arch_def.h Janis Schoetterl-Glausch
@ 2021-10-05  9:11 ` Janis Schoetterl-Glausch
  2021-10-05 13:09   ` Claudio Imbrenda
                     ` (2 more replies)
  1 sibling, 3 replies; 12+ messages in thread
From: Janis Schoetterl-Glausch @ 2021-10-05  9:11 UTC (permalink / raw)
  To: Thomas Huth, Janosch Frank
  Cc: Janis Schoetterl-Glausch, Cornelia Huck, Claudio Imbrenda,
	David Hildenbrand, kvm, linux-s390

Check that specification exceptions cause intercepts when
specification exception interpretation is off.
Check that specification exceptions caused by program new PSWs
cause interceptions.
We cannot assert that non program new PSW specification exceptions
are interpreted because whether interpretation occurs or not is
configuration dependent.

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---
 s390x/Makefile             |  2 +
 lib/s390x/sie.h            |  1 +
 s390x/snippets/c/spec_ex.c | 20 +++++++++
 s390x/spec_ex-sie.c        | 83 ++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg        |  3 ++
 5 files changed, 109 insertions(+)
 create mode 100644 s390x/snippets/c/spec_ex.c
 create mode 100644 s390x/spec_ex-sie.c

diff --git a/s390x/Makefile b/s390x/Makefile
index ef8041a..7198882 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -24,6 +24,7 @@ tests += $(TEST_DIR)/mvpg.elf
 tests += $(TEST_DIR)/uv-host.elf
 tests += $(TEST_DIR)/edat.elf
 tests += $(TEST_DIR)/mvpg-sie.elf
+tests += $(TEST_DIR)/spec_ex-sie.elf
 
 tests_binary = $(patsubst %.elf,%.bin,$(tests))
 ifneq ($(HOST_KEY_DOCUMENT),)
@@ -85,6 +86,7 @@ snippet_asmlib = $(SNIPPET_DIR)/c/cstart.o
 # perquisites (=guests) for the snippet hosts.
 # $(TEST_DIR)/<snippet-host>.elf: snippets = $(SNIPPET_DIR)/<c/asm>/<snippet>.gbin
 $(TEST_DIR)/mvpg-sie.elf: snippets = $(SNIPPET_DIR)/c/mvpg-snippet.gbin
+$(TEST_DIR)/spec_ex-sie.elf: snippets = $(SNIPPET_DIR)/c/spec_ex.gbin
 
 $(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o $(FLATLIBS)
 	$(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
index ca514ef..7ef7251 100644
--- a/lib/s390x/sie.h
+++ b/lib/s390x/sie.h
@@ -98,6 +98,7 @@ struct kvm_s390_sie_block {
 	uint8_t		fpf;			/* 0x0060 */
 #define ECB_GS		0x40
 #define ECB_TE		0x10
+#define ECB_SPECI	0x08
 #define ECB_SRSI	0x04
 #define ECB_HOSTPROTINT	0x02
 	uint8_t		ecb;			/* 0x0061 */
diff --git a/s390x/snippets/c/spec_ex.c b/s390x/snippets/c/spec_ex.c
new file mode 100644
index 0000000..bdba4f4
--- /dev/null
+++ b/s390x/snippets/c/spec_ex.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * © Copyright IBM Corp. 2021
+ *
+ * Snippet used by specification exception interception test.
+ */
+#include <stdint.h>
+#include <asm/arch_def.h>
+
+__attribute__((section(".text"))) int main(void)
+{
+	struct lowcore *lowcore = (struct lowcore *) 0;
+	uint64_t bad_psw = 0;
+
+	/* PSW bit 12 has no name or meaning and must be 0 */
+	lowcore->pgm_new_psw.mask = 1UL << (63 - 12);
+	lowcore->pgm_new_psw.addr = 0xdeadbeee;
+	asm volatile ("lpsw %0" :: "Q"(bad_psw));
+	return 0;
+}
diff --git a/s390x/spec_ex-sie.c b/s390x/spec_ex-sie.c
new file mode 100644
index 0000000..b7e79de
--- /dev/null
+++ b/s390x/spec_ex-sie.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * © Copyright IBM Corp. 2021
+ *
+ * Specification exception interception test.
+ * Checks that specification exception interceptions occur as expected when
+ * specification exception interpretation is off/on.
+ */
+#include <libcflat.h>
+#include <sclp.h>
+#include <asm/page.h>
+#include <asm/arch_def.h>
+#include <alloc_page.h>
+#include <vm.h>
+#include <sie.h>
+
+static struct vm vm;
+extern const char _binary_s390x_snippets_c_spec_ex_gbin_start[];
+extern const char _binary_s390x_snippets_c_spec_ex_gbin_end[];
+
+static void setup_guest(void)
+{
+	char *guest;
+	int binary_size = ((uintptr_t)_binary_s390x_snippets_c_spec_ex_gbin_end -
+			   (uintptr_t)_binary_s390x_snippets_c_spec_ex_gbin_start);
+
+	setup_vm();
+	guest = alloc_pages(8);
+	memcpy(guest, _binary_s390x_snippets_c_spec_ex_gbin_start, binary_size);
+	sie_guest_create(&vm, (uint64_t) guest, HPAGE_SIZE);
+}
+
+static void reset_guest(void)
+{
+	vm.sblk->gpsw.addr = PAGE_SIZE * 4;
+	vm.sblk->gpsw.mask = PSW_MASK_64;
+	vm.sblk->icptcode = 0;
+}
+
+static void test_spec_ex_sie(void)
+{
+	setup_guest();
+
+	report_prefix_push("SIE spec ex interpretation");
+	report_prefix_push("off");
+	reset_guest();
+	sie(&vm);
+	/* interpretation off -> initial exception must cause interception */
+	report(vm.sblk->icptcode == ICPT_PROGI
+	       && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION
+	       && vm.sblk->gpsw.addr != 0xdeadbeee,
+	       "Received specification exception intercept for initial exception");
+	report_prefix_pop();
+
+	report_prefix_push("on");
+	vm.sblk->ecb |= ECB_SPECI;
+	reset_guest();
+	sie(&vm);
+	/* interpretation on -> configuration dependent if initial exception causes
+	 * interception, but invalid new program PSW must
+	 */
+	report(vm.sblk->icptcode == ICPT_PROGI
+	       && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION,
+	       "Received specification exception intercept");
+	if (vm.sblk->gpsw.addr == 0xdeadbeee)
+		report_info("Interpreted initial exception, intercepted invalid program new PSW exception");
+	else
+		report_info("Did not interpret initial exception");
+	report_prefix_pop();
+	report_prefix_pop();
+}
+
+int main(int argc, char **argv)
+{
+	if (!sclp_facilities.has_sief2) {
+		report_skip("SIEF2 facility unavailable");
+		goto out;
+	}
+
+	test_spec_ex_sie();
+out:
+	return report_summary();
+}
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index 9e1802f..3b454b7 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -109,3 +109,6 @@ file = edat.elf
 
 [mvpg-sie]
 file = mvpg-sie.elf
+
+[spec_ex-sie]
+file = spec_ex-sie.elf
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH v2 1/2] [kvm-unit-tests PATCH v2 0/2] Test specification exception
  2021-10-05  9:11 ` [kvm-unit-tests PATCH v2 1/2] s390x: Remove assert from arch_def.h Janis Schoetterl-Glausch
@ 2021-10-05 11:19   ` Janis Schoetterl-Glausch
  2021-10-05 12:46   ` [kvm-unit-tests PATCH v2 1/2] s390x: Remove assert from arch_def.h Claudio Imbrenda
  2021-10-05 12:51   ` Janosch Frank
  2 siblings, 0 replies; 12+ messages in thread
From: Janis Schoetterl-Glausch @ 2021-10-05 11:19 UTC (permalink / raw)
  To: Janis Schoetterl-Glausch; +Cc: kvm, linux-s390

Oops, forgot to Cc the lists on the cover letter, see below.


When specification exception interpretation is enabled, specification
exceptions need not result in interceptions.
However, if the exception is due to an invalid program new PSW,
interception must occur.
Test this.
Also test that interpretation does occur if interpretation is disabled.

v1 -> v2
	Add license and test description
	Use lowcore pointer instead of magic value for program new PSW
		-> need to get rid of assert in arch_def.h
	Do not use odd program new PSW, even if irrelevant
	Use SIE lib
	Reword messages
	Fix nits


Janis Schoetterl-Glausch (2):
  s390x: Remove assert from arch_def.h
  s390x: Add specification exception interception test

 s390x/Makefile             |  2 +
 lib/s390x/asm/arch_def.h   |  5 ++-
 lib/s390x/sie.h            |  1 +
 s390x/snippets/c/spec_ex.c | 20 +++++++++
 s390x/spec_ex-sie.c        | 83 ++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg        |  3 ++
 6 files changed, 112 insertions(+), 2 deletions(-)
 create mode 100644 s390x/snippets/c/spec_ex.c
 create mode 100644 s390x/spec_ex-sie.c

Range-diff against v1:
-:  ------- > 1:  8ad817c s390x: Remove assert from arch_def.h
1:  9e5e161 ! 2:  f8abbae s390x: Add specification exception interception test
    @@ lib/s390x/sie.h: struct kvm_s390_sie_block {
     
      ## s390x/snippets/c/spec_ex.c (new) ##
     @@
    ++// SPDX-License-Identifier: GPL-2.0-only
    ++/*
    ++ * © Copyright IBM Corp. 2021
    ++ *
    ++ * Snippet used by specification exception interception test.
    ++ */
     +#include <stdint.h>
     +#include <asm/arch_def.h>
     +
     +__attribute__((section(".text"))) int main(void)
     +{
    ++	struct lowcore *lowcore = (struct lowcore *) 0;
     +	uint64_t bad_psw = 0;
    -+	struct psw *pgm_new = (struct psw *)464;
     +
    -+	pgm_new->mask = 1UL << (63 - 12); //invalid program new PSW
    -+	pgm_new->addr = 0xdeadbeef;
    ++	/* PSW bit 12 has no name or meaning and must be 0 */
    ++	lowcore->pgm_new_psw.mask = 1UL << (63 - 12);
    ++	lowcore->pgm_new_psw.addr = 0xdeadbeee;
     +	asm volatile ("lpsw %0" :: "Q"(bad_psw));
     +	return 0;
     +}
     
      ## s390x/spec_ex-sie.c (new) ##
     @@
    ++// SPDX-License-Identifier: GPL-2.0-only
    ++/*
    ++ * © Copyright IBM Corp. 2021
    ++ *
    ++ * Specification exception interception test.
    ++ * Checks that specification exception interceptions occur as expected when
    ++ * specification exception interpretation is off/on.
    ++ */
     +#include <libcflat.h>
     +#include <sclp.h>
     +#include <asm/page.h>
    @@ s390x/spec_ex-sie.c (new)
     +			   (uintptr_t)_binary_s390x_snippets_c_spec_ex_gbin_start);
     +
     +	setup_vm();
    -+
    -+	/* Allocate 1MB as guest memory */
     +	guest = alloc_pages(8);
    -+	/* The first two pages are the lowcore */
    -+
    -+	vm.sblk = alloc_page();
    -+
    -+	vm.sblk->cpuflags = CPUSTAT_ZARCH | CPUSTAT_RUNNING;
    -+	vm.sblk->prefix = 0;
    -+	/*
    -+	 * Pageable guest with the same ASCE as the test program, but
    -+	 * the guest memory 0x0 is offset to start at the allocated
    -+	 * guest pages and end after 1MB.
    -+	 *
    -+	 * It's not pretty but faster and easier than managing guest ASCEs.
    -+	 */
    -+	vm.sblk->mso = (u64)guest;
    -+	vm.sblk->msl = (u64)guest;
    -+	vm.sblk->ihcpu = 0xffff;
    -+
    -+	vm.sblk->crycbd = (uint64_t)alloc_page();
    -+
     +	memcpy(guest, _binary_s390x_snippets_c_spec_ex_gbin_start, binary_size);
    ++	sie_guest_create(&vm, (uint64_t) guest, HPAGE_SIZE);
     +}
     +
     +static void reset_guest(void)
     +{
     +	vm.sblk->gpsw.addr = PAGE_SIZE * 4;
    -+	vm.sblk->gpsw.mask = 0x0000000180000000ULL;
    ++	vm.sblk->gpsw.mask = PSW_MASK_64;
    ++	vm.sblk->icptcode = 0;
     +}
     +
     +static void test_spec_ex_sie(void)
     +{
     +	setup_guest();
     +
    -+	report_prefix_push("spec ex interpretation off");
    ++	report_prefix_push("SIE spec ex interpretation");
    ++	report_prefix_push("off");
     +	reset_guest();
    -+	sie64a(vm.sblk, &vm.save_area);
    -+	//interpretation off -> initial exception must cause interception
    ++	sie(&vm);
    ++	/* interpretation off -> initial exception must cause interception */
     +	report(vm.sblk->icptcode == ICPT_PROGI
     +	       && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION
    -+	       && vm.sblk->gpsw.addr != 0xdeadbeef,
    -+	       "Received specification exception intercept for non program new PSW");
    ++	       && vm.sblk->gpsw.addr != 0xdeadbeee,
    ++	       "Received specification exception intercept for initial exception");
     +	report_prefix_pop();
     +
    -+	report_prefix_push("spec ex interpretation on");
    ++	report_prefix_push("on");
     +	vm.sblk->ecb |= ECB_SPECI;
     +	reset_guest();
    -+	sie64a(vm.sblk, &vm.save_area);
    -+	// interpretation on -> configuration dependent if initial exception causes
    -+	// interception, but invalid new program PSW must
    ++	sie(&vm);
    ++	/* interpretation on -> configuration dependent if initial exception causes
    ++	 * interception, but invalid new program PSW must
    ++	 */
     +	report(vm.sblk->icptcode == ICPT_PROGI
     +	       && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION,
     +	       "Received specification exception intercept");
    -+	if (vm.sblk->gpsw.addr == 0xdeadbeef)
    -+		report_info("Interpreted non program new PSW specification exception");
    ++	if (vm.sblk->gpsw.addr == 0xdeadbeee)
    ++		report_info("Interpreted initial exception, intercepted invalid program new PSW exception");
     +	else
    -+		report_info("Did not interpret non program new PSW specification exception");
    ++		report_info("Did not interpret initial exception");
    ++	report_prefix_pop();
     +	report_prefix_pop();
     +}
     +

base-commit: fe26131eec769cef7ad7e2e1e4e192aa0bdb2bba
-- 
2.31.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH v2 1/2] s390x: Remove assert from arch_def.h
  2021-10-05  9:11 ` [kvm-unit-tests PATCH v2 1/2] s390x: Remove assert from arch_def.h Janis Schoetterl-Glausch
  2021-10-05 11:19   ` [kvm-unit-tests PATCH v2 1/2] [kvm-unit-tests PATCH v2 0/2] Test specification exception Janis Schoetterl-Glausch
@ 2021-10-05 12:46   ` Claudio Imbrenda
  2021-10-05 12:51   ` Janosch Frank
  2 siblings, 0 replies; 12+ messages in thread
From: Claudio Imbrenda @ 2021-10-05 12:46 UTC (permalink / raw)
  To: Janis Schoetterl-Glausch
  Cc: Thomas Huth, Janosch Frank, Pierre Morel, Cornelia Huck,
	David Hildenbrand, kvm, linux-s390

On Tue,  5 Oct 2021 11:11:52 +0200
Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:

> Do not use asserts in arch_def.h so it can be included by snippets.
> The caller in stsi.c does not need to be adjusted, returning -1 causes
> the test to fail.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>

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

> ---
>  lib/s390x/asm/arch_def.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 302ef1f..4167e2b 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -334,7 +334,7 @@ static inline int stsi(void *addr, int fc, int sel1, int sel2)
>  	return cc;
>  }
>  
> -static inline unsigned long stsi_get_fc(void)
> +static inline int stsi_get_fc(void)
>  {
>  	register unsigned long r0 asm("0") = 0;
>  	register unsigned long r1 asm("1") = 0;
> @@ -346,7 +346,8 @@ static inline unsigned long stsi_get_fc(void)
>  		     : "+d" (r0), [cc] "=d" (cc)
>  		     : "d" (r1)
>  		     : "cc", "memory");
> -	assert(!cc);
> +	if (cc != 0)
> +		return -1;
>  	return r0 >> 28;
>  }
>  


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH v2 1/2] s390x: Remove assert from arch_def.h
  2021-10-05  9:11 ` [kvm-unit-tests PATCH v2 1/2] s390x: Remove assert from arch_def.h Janis Schoetterl-Glausch
  2021-10-05 11:19   ` [kvm-unit-tests PATCH v2 1/2] [kvm-unit-tests PATCH v2 0/2] Test specification exception Janis Schoetterl-Glausch
  2021-10-05 12:46   ` [kvm-unit-tests PATCH v2 1/2] s390x: Remove assert from arch_def.h Claudio Imbrenda
@ 2021-10-05 12:51   ` Janosch Frank
  2021-10-05 13:51     ` Janis Schoetterl-Glausch
  2 siblings, 1 reply; 12+ messages in thread
From: Janosch Frank @ 2021-10-05 12:51 UTC (permalink / raw)
  To: Janis Schoetterl-Glausch, Thomas Huth
  Cc: Pierre Morel, Cornelia Huck, Claudio Imbrenda, David Hildenbrand,
	kvm, linux-s390

On 10/5/21 11:11, Janis Schoetterl-Glausch wrote:
> Do not use asserts in arch_def.h so it can be included by snippets.
> The caller in stsi.c does not need to be adjusted, returning -1 causes
> the test to fail.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>

A few days ago I had a minute to investigate what needed to be added to 
be able to link the libcflat. Fortunately it wasn't a lot and I'll try 
to post it this week so this patch can hopefully be dropped.

> ---
>   lib/s390x/asm/arch_def.h | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 302ef1f..4167e2b 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -334,7 +334,7 @@ static inline int stsi(void *addr, int fc, int sel1, int sel2)
>   	return cc;
>   }
>   
> -static inline unsigned long stsi_get_fc(void)
> +static inline int stsi_get_fc(void)
>   {
>   	register unsigned long r0 asm("0") = 0;
>   	register unsigned long r1 asm("1") = 0;
> @@ -346,7 +346,8 @@ static inline unsigned long stsi_get_fc(void)
>   		     : "+d" (r0), [cc] "=d" (cc)
>   		     : "d" (r1)
>   		     : "cc", "memory");
> -	assert(!cc);
> +	if (cc != 0)
> +		return -1;
>   	return r0 >> 28;
>   }
>   
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH v2 2/2] s390x: Add specification exception interception test
  2021-10-05  9:11 ` [kvm-unit-tests PATCH v2 2/2] s390x: Add specification exception interception test Janis Schoetterl-Glausch
@ 2021-10-05 13:09   ` Claudio Imbrenda
  2021-10-05 14:12     ` Janis Schoetterl-Glausch
       [not found]   ` <d49ea774-2e3b-aea7-6c7a-9dd3cc744283@linux.ibm.com>
  2021-10-05 16:52   ` Thomas Huth
  2 siblings, 1 reply; 12+ messages in thread
From: Claudio Imbrenda @ 2021-10-05 13:09 UTC (permalink / raw)
  To: Janis Schoetterl-Glausch
  Cc: Thomas Huth, Janosch Frank, Cornelia Huck, David Hildenbrand,
	kvm, linux-s390

On Tue,  5 Oct 2021 11:11:53 +0200
Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:

> Check that specification exceptions cause intercepts when
> specification exception interpretation is off.
> Check that specification exceptions caused by program new PSWs
> cause interceptions.
> We cannot assert that non program new PSW specification exceptions
> are interpreted because whether interpretation occurs or not is
> configuration dependent.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> ---
>  s390x/Makefile             |  2 +
>  lib/s390x/sie.h            |  1 +
>  s390x/snippets/c/spec_ex.c | 20 +++++++++
>  s390x/spec_ex-sie.c        | 83 ++++++++++++++++++++++++++++++++++++++
>  s390x/unittests.cfg        |  3 ++
>  5 files changed, 109 insertions(+)
>  create mode 100644 s390x/snippets/c/spec_ex.c
>  create mode 100644 s390x/spec_ex-sie.c
> 
> diff --git a/s390x/Makefile b/s390x/Makefile
> index ef8041a..7198882 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -24,6 +24,7 @@ tests += $(TEST_DIR)/mvpg.elf
>  tests += $(TEST_DIR)/uv-host.elf
>  tests += $(TEST_DIR)/edat.elf
>  tests += $(TEST_DIR)/mvpg-sie.elf
> +tests += $(TEST_DIR)/spec_ex-sie.elf
>  
>  tests_binary = $(patsubst %.elf,%.bin,$(tests))
>  ifneq ($(HOST_KEY_DOCUMENT),)
> @@ -85,6 +86,7 @@ snippet_asmlib = $(SNIPPET_DIR)/c/cstart.o
>  # perquisites (=guests) for the snippet hosts.
>  # $(TEST_DIR)/<snippet-host>.elf: snippets = $(SNIPPET_DIR)/<c/asm>/<snippet>.gbin
>  $(TEST_DIR)/mvpg-sie.elf: snippets = $(SNIPPET_DIR)/c/mvpg-snippet.gbin
> +$(TEST_DIR)/spec_ex-sie.elf: snippets = $(SNIPPET_DIR)/c/spec_ex.gbin
>  
>  $(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o $(FLATLIBS)
>  	$(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
> index ca514ef..7ef7251 100644
> --- a/lib/s390x/sie.h
> +++ b/lib/s390x/sie.h
> @@ -98,6 +98,7 @@ struct kvm_s390_sie_block {
>  	uint8_t		fpf;			/* 0x0060 */
>  #define ECB_GS		0x40
>  #define ECB_TE		0x10
> +#define ECB_SPECI	0x08
>  #define ECB_SRSI	0x04
>  #define ECB_HOSTPROTINT	0x02
>  	uint8_t		ecb;			/* 0x0061 */
> diff --git a/s390x/snippets/c/spec_ex.c b/s390x/snippets/c/spec_ex.c
> new file mode 100644
> index 0000000..bdba4f4
> --- /dev/null
> +++ b/s390x/snippets/c/spec_ex.c
> @@ -0,0 +1,20 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * © Copyright IBM Corp. 2021
> + *
> + * Snippet used by specification exception interception test.
> + */
> +#include <stdint.h>
> +#include <asm/arch_def.h>
> +
> +__attribute__((section(".text"))) int main(void)
> +{
> +	struct lowcore *lowcore = (struct lowcore *) 0;
> +	uint64_t bad_psw = 0;
> +
> +	/* PSW bit 12 has no name or meaning and must be 0 */
> +	lowcore->pgm_new_psw.mask = 1UL << (63 - 12);

you can use the BIT or BIT_ULL macro

> +	lowcore->pgm_new_psw.addr = 0xdeadbeee;

if the system is broken, it might actually jump at that address; in
that case, will the test fail?

> +	asm volatile ("lpsw %0" :: "Q"(bad_psw));
> +	return 0;
> +}
> diff --git a/s390x/spec_ex-sie.c b/s390x/spec_ex-sie.c
> new file mode 100644
> index 0000000..b7e79de
> --- /dev/null
> +++ b/s390x/spec_ex-sie.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * © Copyright IBM Corp. 2021
> + *
> + * Specification exception interception test.
> + * Checks that specification exception interceptions occur as expected when
> + * specification exception interpretation is off/on.
> + */
> +#include <libcflat.h>
> +#include <sclp.h>
> +#include <asm/page.h>
> +#include <asm/arch_def.h>
> +#include <alloc_page.h>
> +#include <vm.h>
> +#include <sie.h>
> +
> +static struct vm vm;
> +extern const char _binary_s390x_snippets_c_spec_ex_gbin_start[];
> +extern const char _binary_s390x_snippets_c_spec_ex_gbin_end[];
> +
> +static void setup_guest(void)
> +{
> +	char *guest;
> +	int binary_size = ((uintptr_t)_binary_s390x_snippets_c_spec_ex_gbin_end -
> +			   (uintptr_t)_binary_s390x_snippets_c_spec_ex_gbin_start);
> +
> +	setup_vm();
> +	guest = alloc_pages(8);
> +	memcpy(guest, _binary_s390x_snippets_c_spec_ex_gbin_start, binary_size);
> +	sie_guest_create(&vm, (uint64_t) guest, HPAGE_SIZE);
> +}
> +
> +static void reset_guest(void)
> +{
> +	vm.sblk->gpsw.addr = PAGE_SIZE * 4;
> +	vm.sblk->gpsw.mask = PSW_MASK_64;
> +	vm.sblk->icptcode = 0;
> +}
> +
> +static void test_spec_ex_sie(void)
> +{
> +	setup_guest();
> +
> +	report_prefix_push("SIE spec ex interpretation");
> +	report_prefix_push("off");
> +	reset_guest();
> +	sie(&vm);
> +	/* interpretation off -> initial exception must cause interception */
> +	report(vm.sblk->icptcode == ICPT_PROGI
> +	       && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION
> +	       && vm.sblk->gpsw.addr != 0xdeadbeee,
> +	       "Received specification exception intercept for initial exception");
> +	report_prefix_pop();
> +
> +	report_prefix_push("on");
> +	vm.sblk->ecb |= ECB_SPECI;
> +	reset_guest();
> +	sie(&vm);
> +	/* interpretation on -> configuration dependent if initial exception causes
> +	 * interception, but invalid new program PSW must
> +	 */
> +	report(vm.sblk->icptcode == ICPT_PROGI
> +	       && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION,
> +	       "Received specification exception intercept");
> +	if (vm.sblk->gpsw.addr == 0xdeadbeee)
> +		report_info("Interpreted initial exception, intercepted invalid program new PSW exception");
> +	else
> +		report_info("Did not interpret initial exception");
> +	report_prefix_pop();
> +	report_prefix_pop();
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	if (!sclp_facilities.has_sief2) {
> +		report_skip("SIEF2 facility unavailable");
> +		goto out;
> +	}
> +
> +	test_spec_ex_sie();
> +out:
> +	return report_summary();
> +}
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index 9e1802f..3b454b7 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -109,3 +109,6 @@ file = edat.elf
>  
>  [mvpg-sie]
>  file = mvpg-sie.elf
> +
> +[spec_ex-sie]
> +file = spec_ex-sie.elf


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH v2 1/2] s390x: Remove assert from arch_def.h
  2021-10-05 12:51   ` Janosch Frank
@ 2021-10-05 13:51     ` Janis Schoetterl-Glausch
  2021-10-05 13:56       ` Janosch Frank
  0 siblings, 1 reply; 12+ messages in thread
From: Janis Schoetterl-Glausch @ 2021-10-05 13:51 UTC (permalink / raw)
  To: Janosch Frank, Janis Schoetterl-Glausch, Thomas Huth
  Cc: Pierre Morel, Cornelia Huck, Claudio Imbrenda, David Hildenbrand,
	kvm, linux-s390

On 10/5/21 2:51 PM, Janosch Frank wrote:
> On 10/5/21 11:11, Janis Schoetterl-Glausch wrote:
>> Do not use asserts in arch_def.h so it can be included by snippets.
>> The caller in stsi.c does not need to be adjusted, returning -1 causes
>> the test to fail.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> 
> A few days ago I had a minute to investigate what needed to be added to be able to link the libcflat. Fortunately it wasn't a lot and I'll try to post it this week so this patch can hopefully be dropped.

One could argue that cc being != 0 is part of the test and so should go through report() and not assert().
Which happens naturally, since the caller will likely compare it to some positive expected value.
> 
>> ---
>>   lib/s390x/asm/arch_def.h | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
>> index 302ef1f..4167e2b 100644
>> --- a/lib/s390x/asm/arch_def.h
>> +++ b/lib/s390x/asm/arch_def.h
>> @@ -334,7 +334,7 @@ static inline int stsi(void *addr, int fc, int sel1, int sel2)
>>       return cc;
>>   }
>>   -static inline unsigned long stsi_get_fc(void)
>> +static inline int stsi_get_fc(void)
>>   {
>>       register unsigned long r0 asm("0") = 0;
>>       register unsigned long r1 asm("1") = 0;
>> @@ -346,7 +346,8 @@ static inline unsigned long stsi_get_fc(void)
>>                : "+d" (r0), [cc] "=d" (cc)
>>                : "d" (r1)
>>                : "cc", "memory");
>> -    assert(!cc);
>> +    if (cc != 0)
>> +        return -1;
>>       return r0 >> 28;
>>   }
>>  
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH v2 1/2] s390x: Remove assert from arch_def.h
  2021-10-05 13:51     ` Janis Schoetterl-Glausch
@ 2021-10-05 13:56       ` Janosch Frank
  0 siblings, 0 replies; 12+ messages in thread
From: Janosch Frank @ 2021-10-05 13:56 UTC (permalink / raw)
  To: Janis Schoetterl-Glausch, Janis Schoetterl-Glausch, Thomas Huth
  Cc: Pierre Morel, Cornelia Huck, Claudio Imbrenda, David Hildenbrand,
	kvm, linux-s390

On 10/5/21 15:51, Janis Schoetterl-Glausch wrote:
> On 10/5/21 2:51 PM, Janosch Frank wrote:
>> On 10/5/21 11:11, Janis Schoetterl-Glausch wrote:
>>> Do not use asserts in arch_def.h so it can be included by snippets.
>>> The caller in stsi.c does not need to be adjusted, returning -1 causes
>>> the test to fail.
>>>
>>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
>>
>> A few days ago I had a minute to investigate what needed to be added to be able to link the libcflat. Fortunately it wasn't a lot and I'll try to post it this week so this patch can hopefully be dropped.
> 
> One could argue that cc being != 0 is part of the test and so should go through report() and not assert().
> Which happens naturally, since the caller will likely compare it to some positive expected value.

Yes, I can also live with that if you change the commit message then :)

>>
>>> ---
>>>    lib/s390x/asm/arch_def.h | 5 +++--
>>>    1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
>>> index 302ef1f..4167e2b 100644
>>> --- a/lib/s390x/asm/arch_def.h
>>> +++ b/lib/s390x/asm/arch_def.h
>>> @@ -334,7 +334,7 @@ static inline int stsi(void *addr, int fc, int sel1, int sel2)
>>>        return cc;
>>>    }
>>>    -static inline unsigned long stsi_get_fc(void)
>>> +static inline int stsi_get_fc(void)
>>>    {
>>>        register unsigned long r0 asm("0") = 0;
>>>        register unsigned long r1 asm("1") = 0;
>>> @@ -346,7 +346,8 @@ static inline unsigned long stsi_get_fc(void)
>>>                 : "+d" (r0), [cc] "=d" (cc)
>>>                 : "d" (r1)
>>>                 : "cc", "memory");
>>> -    assert(!cc);
>>> +    if (cc != 0)
>>> +        return -1;
>>>        return r0 >> 28;
>>>    }
>>>   
>>
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH v2 2/2] s390x: Add specification exception interception test
  2021-10-05 13:09   ` Claudio Imbrenda
@ 2021-10-05 14:12     ` Janis Schoetterl-Glausch
  0 siblings, 0 replies; 12+ messages in thread
From: Janis Schoetterl-Glausch @ 2021-10-05 14:12 UTC (permalink / raw)
  To: Claudio Imbrenda, Janis Schoetterl-Glausch
  Cc: Thomas Huth, Janosch Frank, Cornelia Huck, David Hildenbrand,
	kvm, linux-s390

On 10/5/21 3:09 PM, Claudio Imbrenda wrote:
> On Tue,  5 Oct 2021 11:11:53 +0200
> Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:
> 
>> Check that specification exceptions cause intercepts when
>> specification exception interpretation is off.
>> Check that specification exceptions caused by program new PSWs
>> cause interceptions.
>> We cannot assert that non program new PSW specification exceptions
>> are interpreted because whether interpretation occurs or not is
>> configuration dependent.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
>> ---
>>  s390x/Makefile             |  2 +
>>  lib/s390x/sie.h            |  1 +
>>  s390x/snippets/c/spec_ex.c | 20 +++++++++
>>  s390x/spec_ex-sie.c        | 83 ++++++++++++++++++++++++++++++++++++++
>>  s390x/unittests.cfg        |  3 ++
>>  5 files changed, 109 insertions(+)
>>  create mode 100644 s390x/snippets/c/spec_ex.c
>>  create mode 100644 s390x/spec_ex-sie.c
>>
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index ef8041a..7198882 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -24,6 +24,7 @@ tests += $(TEST_DIR)/mvpg.elf
>>  tests += $(TEST_DIR)/uv-host.elf
>>  tests += $(TEST_DIR)/edat.elf
>>  tests += $(TEST_DIR)/mvpg-sie.elf
>> +tests += $(TEST_DIR)/spec_ex-sie.elf
>>  
>>  tests_binary = $(patsubst %.elf,%.bin,$(tests))
>>  ifneq ($(HOST_KEY_DOCUMENT),)
>> @@ -85,6 +86,7 @@ snippet_asmlib = $(SNIPPET_DIR)/c/cstart.o
>>  # perquisites (=guests) for the snippet hosts.
>>  # $(TEST_DIR)/<snippet-host>.elf: snippets = $(SNIPPET_DIR)/<c/asm>/<snippet>.gbin
>>  $(TEST_DIR)/mvpg-sie.elf: snippets = $(SNIPPET_DIR)/c/mvpg-snippet.gbin
>> +$(TEST_DIR)/spec_ex-sie.elf: snippets = $(SNIPPET_DIR)/c/spec_ex.gbin
>>  
>>  $(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o $(FLATLIBS)
>>  	$(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
>> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
>> index ca514ef..7ef7251 100644
>> --- a/lib/s390x/sie.h
>> +++ b/lib/s390x/sie.h
>> @@ -98,6 +98,7 @@ struct kvm_s390_sie_block {
>>  	uint8_t		fpf;			/* 0x0060 */
>>  #define ECB_GS		0x40
>>  #define ECB_TE		0x10
>> +#define ECB_SPECI	0x08
>>  #define ECB_SRSI	0x04
>>  #define ECB_HOSTPROTINT	0x02
>>  	uint8_t		ecb;			/* 0x0061 */
>> diff --git a/s390x/snippets/c/spec_ex.c b/s390x/snippets/c/spec_ex.c
>> new file mode 100644
>> index 0000000..bdba4f4
>> --- /dev/null
>> +++ b/s390x/snippets/c/spec_ex.c
>> @@ -0,0 +1,20 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * © Copyright IBM Corp. 2021
>> + *
>> + * Snippet used by specification exception interception test.
>> + */
>> +#include <stdint.h>
>> +#include <asm/arch_def.h>
>> +
>> +__attribute__((section(".text"))) int main(void)
>> +{
>> +	struct lowcore *lowcore = (struct lowcore *) 0;
>> +	uint64_t bad_psw = 0;
>> +
>> +	/* PSW bit 12 has no name or meaning and must be 0 */
>> +	lowcore->pgm_new_psw.mask = 1UL << (63 - 12);
> 
> you can use the BIT or BIT_ULL macro
> 
>> +	lowcore->pgm_new_psw.addr = 0xdeadbeee;
> 
> if the system is broken, it might actually jump at that address; in
> that case, will the test fail?

Broken how? If interpretation is overzealous the test might hang.

[...]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH v2 2/2] s390x: Add specification exception interception test
       [not found]   ` <d49ea774-2e3b-aea7-6c7a-9dd3cc744283@linux.ibm.com>
@ 2021-10-05 14:34     ` Janis Schoetterl-Glausch
  0 siblings, 0 replies; 12+ messages in thread
From: Janis Schoetterl-Glausch @ 2021-10-05 14:34 UTC (permalink / raw)
  To: Janosch Frank, Janis Schoetterl-Glausch, Thomas Huth
  Cc: Cornelia Huck, Claudio Imbrenda, David Hildenbrand, kvm, linux-s390

On 10/5/21 2:49 PM, Janosch Frank wrote:
> On 10/5/21 11:11, Janis Schoetterl-Glausch wrote:
>> Check that specification exceptions cause intercepts when
>> specification exception interpretation is off.
>> Check that specification exceptions caused by program new PSWs
>> cause interceptions.
>> We cannot assert that non program new PSW specification exceptions
>> are interpreted because whether interpretation occurs or not is
>> configuration dependent.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> 
> Reviewed-by: Janosch Frank <frankja@de.ibm.com>
> Minor nit below.
> 
>> ---
>>   s390x/Makefile             |  2 +
>>   lib/s390x/sie.h            |  1 +
>>   s390x/snippets/c/spec_ex.c | 20 +++++++++
>>   s390x/spec_ex-sie.c        | 83 ++++++++++++++++++++++++++++++++++++++
>>   s390x/unittests.cfg        |  3 ++
>>   5 files changed, 109 insertions(+)
>>   create mode 100644 s390x/snippets/c/spec_ex.c
>>   create mode 100644 s390x/spec_ex-sie.c
>>
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index ef8041a..7198882 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -24,6 +24,7 @@ tests += $(TEST_DIR)/mvpg.elf
>>   tests += $(TEST_DIR)/uv-host.elf
>>   tests += $(TEST_DIR)/edat.elf
>>   tests += $(TEST_DIR)/mvpg-sie.elf
>> +tests += $(TEST_DIR)/spec_ex-sie.elf
>>     tests_binary = $(patsubst %.elf,%.bin,$(tests))
>>   ifneq ($(HOST_KEY_DOCUMENT),)
>> @@ -85,6 +86,7 @@ snippet_asmlib = $(SNIPPET_DIR)/c/cstart.o
>>   # perquisites (=guests) for the snippet hosts.
>>   # $(TEST_DIR)/<snippet-host>.elf: snippets = $(SNIPPET_DIR)/<c/asm>/<snippet>.gbin
>>   $(TEST_DIR)/mvpg-sie.elf: snippets = $(SNIPPET_DIR)/c/mvpg-snippet.gbin
>> +$(TEST_DIR)/spec_ex-sie.elf: snippets = $(SNIPPET_DIR)/c/spec_ex.gbin
>>     $(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o $(FLATLIBS)
>>       $(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
>> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
>> index ca514ef..7ef7251 100644
>> --- a/lib/s390x/sie.h
>> +++ b/lib/s390x/sie.h
>> @@ -98,6 +98,7 @@ struct kvm_s390_sie_block {
>>       uint8_t        fpf;            /* 0x0060 */
>>   #define ECB_GS        0x40
>>   #define ECB_TE        0x10
>> +#define ECB_SPECI    0x08
>>   #define ECB_SRSI    0x04
>>   #define ECB_HOSTPROTINT    0x02
>>       uint8_t        ecb;            /* 0x0061 */
>> diff --git a/s390x/snippets/c/spec_ex.c b/s390x/snippets/c/spec_ex.c
>> new file mode 100644
>> index 0000000..bdba4f4
>> --- /dev/null
>> +++ b/s390x/snippets/c/spec_ex.c
>> @@ -0,0 +1,20 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + *Copyright IBM Corp. 2021
> 
> I'd replace that copyright symbol with the (C) that we have usually.

IBM guidelines say to just drop it if not available. Will do that.

> Also, don't you want to add your name/mail in an authors list?

Eh, I don't see the point. Git tracks authorship and having a single source
of truth is preferable IMO. Thanks for the reminder, tho.

[...]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH v2 2/2] s390x: Add specification exception interception test
  2021-10-05  9:11 ` [kvm-unit-tests PATCH v2 2/2] s390x: Add specification exception interception test Janis Schoetterl-Glausch
  2021-10-05 13:09   ` Claudio Imbrenda
       [not found]   ` <d49ea774-2e3b-aea7-6c7a-9dd3cc744283@linux.ibm.com>
@ 2021-10-05 16:52   ` Thomas Huth
  2021-10-06  9:47     ` Janosch Frank
  2 siblings, 1 reply; 12+ messages in thread
From: Thomas Huth @ 2021-10-05 16:52 UTC (permalink / raw)
  To: Janis Schoetterl-Glausch, Janosch Frank
  Cc: Cornelia Huck, Claudio Imbrenda, David Hildenbrand, kvm, linux-s390

On 05/10/2021 11.11, Janis Schoetterl-Glausch wrote:
> Check that specification exceptions cause intercepts when
> specification exception interpretation is off.
> Check that specification exceptions caused by program new PSWs
> cause interceptions.
> We cannot assert that non program new PSW specification exceptions
> are interpreted because whether interpretation occurs or not is
> configuration dependent.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> ---
>   s390x/Makefile             |  2 +
>   lib/s390x/sie.h            |  1 +
>   s390x/snippets/c/spec_ex.c | 20 +++++++++
>   s390x/spec_ex-sie.c        | 83 ++++++++++++++++++++++++++++++++++++++
>   s390x/unittests.cfg        |  3 ++
>   5 files changed, 109 insertions(+)
>   create mode 100644 s390x/snippets/c/spec_ex.c
>   create mode 100644 s390x/spec_ex-sie.c
> 
> diff --git a/s390x/Makefile b/s390x/Makefile
> index ef8041a..7198882 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -24,6 +24,7 @@ tests += $(TEST_DIR)/mvpg.elf
>   tests += $(TEST_DIR)/uv-host.elf
>   tests += $(TEST_DIR)/edat.elf
>   tests += $(TEST_DIR)/mvpg-sie.elf
> +tests += $(TEST_DIR)/spec_ex-sie.elf
>   
>   tests_binary = $(patsubst %.elf,%.bin,$(tests))
>   ifneq ($(HOST_KEY_DOCUMENT),)
> @@ -85,6 +86,7 @@ snippet_asmlib = $(SNIPPET_DIR)/c/cstart.o
>   # perquisites (=guests) for the snippet hosts.
>   # $(TEST_DIR)/<snippet-host>.elf: snippets = $(SNIPPET_DIR)/<c/asm>/<snippet>.gbin
>   $(TEST_DIR)/mvpg-sie.elf: snippets = $(SNIPPET_DIR)/c/mvpg-snippet.gbin
> +$(TEST_DIR)/spec_ex-sie.elf: snippets = $(SNIPPET_DIR)/c/spec_ex.gbin
>   
>   $(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o $(FLATLIBS)
>   	$(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
> index ca514ef..7ef7251 100644
> --- a/lib/s390x/sie.h
> +++ b/lib/s390x/sie.h
> @@ -98,6 +98,7 @@ struct kvm_s390_sie_block {
>   	uint8_t		fpf;			/* 0x0060 */
>   #define ECB_GS		0x40
>   #define ECB_TE		0x10
> +#define ECB_SPECI	0x08
>   #define ECB_SRSI	0x04
>   #define ECB_HOSTPROTINT	0x02
>   	uint8_t		ecb;			/* 0x0061 */
> diff --git a/s390x/snippets/c/spec_ex.c b/s390x/snippets/c/spec_ex.c
> new file mode 100644
> index 0000000..bdba4f4
> --- /dev/null
> +++ b/s390x/snippets/c/spec_ex.c
> @@ -0,0 +1,20 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * © Copyright IBM Corp. 2021
> + *
> + * Snippet used by specification exception interception test.
> + */
> +#include <stdint.h>
> +#include <asm/arch_def.h>
> +
> +__attribute__((section(".text"))) int main(void)
> +{
> +	struct lowcore *lowcore = (struct lowcore *) 0;
> +	uint64_t bad_psw = 0;
> +
> +	/* PSW bit 12 has no name or meaning and must be 0 */
> +	lowcore->pgm_new_psw.mask = 1UL << (63 - 12);
> +	lowcore->pgm_new_psw.addr = 0xdeadbeee;
> +	asm volatile ("lpsw %0" :: "Q"(bad_psw));
> +	return 0;
> +}
> diff --git a/s390x/spec_ex-sie.c b/s390x/spec_ex-sie.c
> new file mode 100644
> index 0000000..b7e79de
> --- /dev/null
> +++ b/s390x/spec_ex-sie.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * © Copyright IBM Corp. 2021
> + *
> + * Specification exception interception test.
> + * Checks that specification exception interceptions occur as expected when
> + * specification exception interpretation is off/on.
> + */
> +#include <libcflat.h>
> +#include <sclp.h>
> +#include <asm/page.h>
> +#include <asm/arch_def.h>
> +#include <alloc_page.h>
> +#include <vm.h>
> +#include <sie.h>
> +
> +static struct vm vm;
> +extern const char _binary_s390x_snippets_c_spec_ex_gbin_start[];
> +extern const char _binary_s390x_snippets_c_spec_ex_gbin_end[];
> +
> +static void setup_guest(void)
> +{
> +	char *guest;
> +	int binary_size = ((uintptr_t)_binary_s390x_snippets_c_spec_ex_gbin_end -
> +			   (uintptr_t)_binary_s390x_snippets_c_spec_ex_gbin_start);
> +
> +	setup_vm();
> +	guest = alloc_pages(8);
> +	memcpy(guest, _binary_s390x_snippets_c_spec_ex_gbin_start, binary_size);
> +	sie_guest_create(&vm, (uint64_t) guest, HPAGE_SIZE);
> +}
> +
> +static void reset_guest(void)
> +{
> +	vm.sblk->gpsw.addr = PAGE_SIZE * 4;

Could we please get a #define for this magic PAGE_SIZE * 4 value, with a 
comment mentioning that the value comes from s390x/snippets/c/flat.lds ?
I know it's already like this in the mvpg-sie.c test, so this could also be 
done in a follow-up patch instead, to fix mvpg-sie.c, too.

By the way, Janosch, do you remember why it s PAGE_SIZE * 4 and not 
PAGE_SIZE * 2 ? Space for additional SMP lowcores?

> +	vm.sblk->gpsw.mask = PSW_MASK_64;
> +	vm.sblk->icptcode = 0;
> +}
> +
> +static void test_spec_ex_sie(void)
> +{
> +	setup_guest();
> +
> +	report_prefix_push("SIE spec ex interpretation");
> +	report_prefix_push("off");
> +	reset_guest();
> +	sie(&vm);
> +	/* interpretation off -> initial exception must cause interception */
> +	report(vm.sblk->icptcode == ICPT_PROGI
> +	       && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION
> +	       && vm.sblk->gpsw.addr != 0xdeadbeee,
> +	       "Received specification exception intercept for initial exception");
> +	report_prefix_pop();
> +
> +	report_prefix_push("on");
> +	vm.sblk->ecb |= ECB_SPECI;
> +	reset_guest();
> +	sie(&vm);
> +	/* interpretation on -> configuration dependent if initial exception causes
> +	 * interception, but invalid new program PSW must
> +	 */
> +	report(vm.sblk->icptcode == ICPT_PROGI
> +	       && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION,
> +	       "Received specification exception intercept");
> +	if (vm.sblk->gpsw.addr == 0xdeadbeee)
> +		report_info("Interpreted initial exception, intercepted invalid program new PSW exception");
> +	else
> +		report_info("Did not interpret initial exception");
> +	report_prefix_pop();
> +	report_prefix_pop();
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	if (!sclp_facilities.has_sief2) {
> +		report_skip("SIEF2 facility unavailable");
> +		goto out;
> +	}
> +
> +	test_spec_ex_sie();
> +out:
> +	return report_summary();
> +}
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index 9e1802f..3b454b7 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -109,3 +109,6 @@ file = edat.elf
>   
>   [mvpg-sie]
>   file = mvpg-sie.elf
> +
> +[spec_ex-sie]
> +file = spec_ex-sie.elf
> 

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [kvm-unit-tests PATCH v2 2/2] s390x: Add specification exception interception test
  2021-10-05 16:52   ` Thomas Huth
@ 2021-10-06  9:47     ` Janosch Frank
  0 siblings, 0 replies; 12+ messages in thread
From: Janosch Frank @ 2021-10-06  9:47 UTC (permalink / raw)
  To: Thomas Huth, Janis Schoetterl-Glausch
  Cc: Cornelia Huck, Claudio Imbrenda, David Hildenbrand, kvm, linux-s390

On 10/5/21 18:52, Thomas Huth wrote:
> On 05/10/2021 11.11, Janis Schoetterl-Glausch wrote:
>> Check that specification exceptions cause intercepts when
>> specification exception interpretation is off.
>> Check that specification exceptions caused by program new PSWs
>> cause interceptions.
>> We cannot assert that non program new PSW specification exceptions
>> are interpreted because whether interpretation occurs or not is
>> configuration dependent.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
>> ---
>>    s390x/Makefile             |  2 +
>>    lib/s390x/sie.h            |  1 +
>>    s390x/snippets/c/spec_ex.c | 20 +++++++++
>>    s390x/spec_ex-sie.c        | 83 ++++++++++++++++++++++++++++++++++++++
>>    s390x/unittests.cfg        |  3 ++
>>    5 files changed, 109 insertions(+)
>>    create mode 100644 s390x/snippets/c/spec_ex.c
>>    create mode 100644 s390x/spec_ex-sie.c
>>
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index ef8041a..7198882 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -24,6 +24,7 @@ tests += $(TEST_DIR)/mvpg.elf
>>    tests += $(TEST_DIR)/uv-host.elf
>>    tests += $(TEST_DIR)/edat.elf
>>    tests += $(TEST_DIR)/mvpg-sie.elf
>> +tests += $(TEST_DIR)/spec_ex-sie.elf
>>    
>>    tests_binary = $(patsubst %.elf,%.bin,$(tests))
>>    ifneq ($(HOST_KEY_DOCUMENT),)
>> @@ -85,6 +86,7 @@ snippet_asmlib = $(SNIPPET_DIR)/c/cstart.o
>>    # perquisites (=guests) for the snippet hosts.
>>    # $(TEST_DIR)/<snippet-host>.elf: snippets = $(SNIPPET_DIR)/<c/asm>/<snippet>.gbin
>>    $(TEST_DIR)/mvpg-sie.elf: snippets = $(SNIPPET_DIR)/c/mvpg-snippet.gbin
>> +$(TEST_DIR)/spec_ex-sie.elf: snippets = $(SNIPPET_DIR)/c/spec_ex.gbin
>>    
>>    $(SNIPPET_DIR)/asm/%.gbin: $(SNIPPET_DIR)/asm/%.o $(FLATLIBS)
>>    	$(OBJCOPY) -O binary $(patsubst %.gbin,%.o,$@) $@
>> diff --git a/lib/s390x/sie.h b/lib/s390x/sie.h
>> index ca514ef..7ef7251 100644
>> --- a/lib/s390x/sie.h
>> +++ b/lib/s390x/sie.h
>> @@ -98,6 +98,7 @@ struct kvm_s390_sie_block {
>>    	uint8_t		fpf;			/* 0x0060 */
>>    #define ECB_GS		0x40
>>    #define ECB_TE		0x10
>> +#define ECB_SPECI	0x08
>>    #define ECB_SRSI	0x04
>>    #define ECB_HOSTPROTINT	0x02
>>    	uint8_t		ecb;			/* 0x0061 */
>> diff --git a/s390x/snippets/c/spec_ex.c b/s390x/snippets/c/spec_ex.c
>> new file mode 100644
>> index 0000000..bdba4f4
>> --- /dev/null
>> +++ b/s390x/snippets/c/spec_ex.c
>> @@ -0,0 +1,20 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*

>> + *
>> + * Snippet used by specification exception interception test.
>> + */
>> +#include <stdint.h>
>> +#include <asm/arch_def.h>
>> +
>> +__attribute__((section(".text"))) int main(void)
>> +{
>> +	struct lowcore *lowcore = (struct lowcore *) 0;
>> +	uint64_t bad_psw = 0;
>> +
>> +	/* PSW bit 12 has no name or meaning and must be 0 */
>> +	lowcore->pgm_new_psw.mask = 1UL << (63 - 12);
>> +	lowcore->pgm_new_psw.addr = 0xdeadbeee;
>> +	asm volatile ("lpsw %0" :: "Q"(bad_psw));
>> +	return 0;
>> +}
>> diff --git a/s390x/spec_ex-sie.c b/s390x/spec_ex-sie.c
>> new file mode 100644
>> index 0000000..b7e79de
>> --- /dev/null
>> +++ b/s390x/spec_ex-sie.c
>> @@ -0,0 +1,83 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*

>> + *
>> + * Specification exception interception test.
>> + * Checks that specification exception interceptions occur as expected when
>> + * specification exception interpretation is off/on.
>> + */
>> +#include <libcflat.h>
>> +#include <sclp.h>
>> +#include <asm/page.h>
>> +#include <asm/arch_def.h>
>> +#include <alloc_page.h>
>> +#include <vm.h>
>> +#include <sie.h>
>> +
>> +static struct vm vm;
>> +extern const char _binary_s390x_snippets_c_spec_ex_gbin_start[];
>> +extern const char _binary_s390x_snippets_c_spec_ex_gbin_end[];
>> +
>> +static void setup_guest(void)
>> +{
>> +	char *guest;
>> +	int binary_size = ((uintptr_t)_binary_s390x_snippets_c_spec_ex_gbin_end -
>> +			   (uintptr_t)_binary_s390x_snippets_c_spec_ex_gbin_start);
>> +
>> +	setup_vm();
>> +	guest = alloc_pages(8);
>> +	memcpy(guest, _binary_s390x_snippets_c_spec_ex_gbin_start, binary_size);
>> +	sie_guest_create(&vm, (uint64_t) guest, HPAGE_SIZE);
>> +}
>> +
>> +static void reset_guest(void)
>> +{
>> +	vm.sblk->gpsw.addr = PAGE_SIZE * 4;
> 
> Could we please get a #define for this magic PAGE_SIZE * 4 value, with a
> comment mentioning that the value comes from s390x/snippets/c/flat.lds ?
> I know it's already like this in the mvpg-sie.c test, so this could also be
> done in a follow-up patch instead, to fix mvpg-sie.c, too.
> 
> By the way, Janosch, do you remember why it s PAGE_SIZE * 4 and not
> PAGE_SIZE * 2 ? Space for additional SMP lowcores?

I've put the stack on Page 4 because I can't guarantee what people do on 
higher pages so I hope we don't need more than a page or two for the 
stack. It's very much a hack :)

> 
>> +	vm.sblk->gpsw.mask = PSW_MASK_64;
>> +	vm.sblk->icptcode = 0;
>> +}
>> +
>> +static void test_spec_ex_sie(void)
>> +{
>> +	setup_guest();
>> +
>> +	report_prefix_push("SIE spec ex interpretation");
>> +	report_prefix_push("off");
>> +	reset_guest();
>> +	sie(&vm);
>> +	/* interpretation off -> initial exception must cause interception */
>> +	report(vm.sblk->icptcode == ICPT_PROGI
>> +	       && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION
>> +	       && vm.sblk->gpsw.addr != 0xdeadbeee,
>> +	       "Received specification exception intercept for initial exception");
>> +	report_prefix_pop();
>> +
>> +	report_prefix_push("on");
>> +	vm.sblk->ecb |= ECB_SPECI;
>> +	reset_guest();
>> +	sie(&vm);
>> +	/* interpretation on -> configuration dependent if initial exception causes
>> +	 * interception, but invalid new program PSW must
>> +	 */
>> +	report(vm.sblk->icptcode == ICPT_PROGI
>> +	       && vm.sblk->iprcc == PGM_INT_CODE_SPECIFICATION,
>> +	       "Received specification exception intercept");
>> +	if (vm.sblk->gpsw.addr == 0xdeadbeee)
>> +		report_info("Interpreted initial exception, intercepted invalid program new PSW exception");
>> +	else
>> +		report_info("Did not interpret initial exception");
>> +	report_prefix_pop();
>> +	report_prefix_pop();
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> +	if (!sclp_facilities.has_sief2) {
>> +		report_skip("SIEF2 facility unavailable");
>> +		goto out;
>> +	}
>> +
>> +	test_spec_ex_sie();
>> +out:
>> +	return report_summary();
>> +}
>> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
>> index 9e1802f..3b454b7 100644
>> --- a/s390x/unittests.cfg
>> +++ b/s390x/unittests.cfg
>> @@ -109,3 +109,6 @@ file = edat.elf
>>    
>>    [mvpg-sie]
>>    file = mvpg-sie.elf
>> +
>> +[spec_ex-sie]
>> +file = spec_ex-sie.elf
>>
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-10-06  9:47 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20211005091153.1863139-1-scgl@linux.ibm.com>
2021-10-05  9:11 ` [kvm-unit-tests PATCH v2 1/2] s390x: Remove assert from arch_def.h Janis Schoetterl-Glausch
2021-10-05 11:19   ` [kvm-unit-tests PATCH v2 1/2] [kvm-unit-tests PATCH v2 0/2] Test specification exception Janis Schoetterl-Glausch
2021-10-05 12:46   ` [kvm-unit-tests PATCH v2 1/2] s390x: Remove assert from arch_def.h Claudio Imbrenda
2021-10-05 12:51   ` Janosch Frank
2021-10-05 13:51     ` Janis Schoetterl-Glausch
2021-10-05 13:56       ` Janosch Frank
2021-10-05  9:11 ` [kvm-unit-tests PATCH v2 2/2] s390x: Add specification exception interception test Janis Schoetterl-Glausch
2021-10-05 13:09   ` Claudio Imbrenda
2021-10-05 14:12     ` Janis Schoetterl-Glausch
     [not found]   ` <d49ea774-2e3b-aea7-6c7a-9dd3cc744283@linux.ibm.com>
2021-10-05 14:34     ` Janis Schoetterl-Glausch
2021-10-05 16:52   ` Thomas Huth
2021-10-06  9:47     ` Janosch Frank

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.