Re: [kvm-unit-tests PATCH v2 2/2] s390x: Add specification exception interception test

Janosch Frank
2021-10-06T09:47:34Z

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