On 8/20/19 3:21 PM, Thomas Huth wrote:
> On 8/20/19 12:55 PM, Janosch Frank wrote:
>> For now let's concentrate on the error conditions.
>>
>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>> ---
>> s390x/Makefile | 1 +
>> s390x/stsi.c | 123 ++++++++++++++++++++++++++++++++++++++++++++
>> s390x/unittests.cfg | 5 +-
>> 3 files changed, 128 insertions(+), 1 deletion(-)
>> create mode 100644 s390x/stsi.c
>>
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index b654c56..311ab77 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -12,6 +12,7 @@ tests += $(TEST_DIR)/vector.elf
>> tests += $(TEST_DIR)/gs.elf
>> tests += $(TEST_DIR)/iep.elf
>> tests += $(TEST_DIR)/diag288.elf
>> +tests += $(TEST_DIR)/stsi.elf
>> tests_binary = $(patsubst %.elf,%.bin,$(tests))
>>
>> all: directories test_cases test_cases_binary
>> diff --git a/s390x/stsi.c b/s390x/stsi.c
>> new file mode 100644
>> index 0000000..005f337
>> --- /dev/null
>> +++ b/s390x/stsi.c
>> @@ -0,0 +1,123 @@
>> +/*
>> + * Store System Information tests
>> + *
>> + * Copyright (c) 2019 IBM Corp
>> + *
>> + * Authors:
>> + * Janosch Frank <frankja@xxxxxxxxxxxxx>
>> + *
>> + * This code is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU Library General Public License version 2.
>> + */
>> +
>> +#include <libcflat.h>
>> +#include <asm/page.h>
>> +#include <asm/asm-offsets.h>
>> +#include <asm/interrupt.h>
>> +
>> +static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
>> +
>> +static inline unsigned long stsi(unsigned long *addr,
>> + unsigned long fc, uint8_t sel1, uint8_t sel2)
>
> Return code should be "int", not "long".
>
> I'd also suggest to use "void *addr" instead of "unsigned long *addr",
> then you don't have to cast the pagebuf when you're calling this function.
Ok
>
>> +{
>> + register unsigned long r0 asm("0") = (fc << 28) | sel1;
>> + register unsigned long r1 asm("1") = sel2;
>> + int cc;
>> +
>> + asm volatile("stsi 0(%3)\n"
>> + "ipm %[cc]\n"
>> + "srl %[cc],28\n"
>> + : "+d" (r0), [cc] "=d" (cc)
>> + : "d" (r1), "a" (addr)
>> + : "cc", "memory");
>> + return cc;
>> +}
>
> Bonus points for putting that function into a header and re-use it in
> skey.c (maybe in a separate patch, though).
I forgot that you added that...
How about moving it to lib/s390/asm/arch_def.h ?
[...]
>> +static void test_fc(void)
>> +{
>> + report("cc == 3", stsi((void *)pagebuf, 7, 0, 0));
>
> Shouldn't that line look like this instead:
>
> report("cc == 3", stsi((void *)pagebuf, 7, 0, 0) == 3);
>
> ?
Yes
>
>> + report("r0 == 3", stsi_get_fc((void *)pagebuf));
>
> report("r0 >= 3", stsi_get_fc((void *)pagebuf) >= 3);
>
> ?
Well rather >= 2 because we can also run on lpar with some additional
patches applied. Time to test this under lpar...
>
>> +}
>> +
>> +int main(void)
>> +{
>> + report_prefix_push("stsi");
>> + test_priv();
>> + test_specs();
>> + test_fc();
>> + return report_summary();
>> +}
>
> How about adding another test for access exceptions? Activate low
> address protection, then store to address 4096 ... and/or check
> "stsi((void *)-0xdeadadd, 1, 0, 0);" ?
Sounds good
>
> Thomas
>
Attachment:
signature.asc
Description: OpenPGP digital signature
