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

Janis Schoetterl-Glausch
2021-10-05T14:35:00Z

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