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

[Janosch Frank <frankja@linux.ibm.com>]

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