| Message ID | 1520942503-6163-4-git-send-email-frankja@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13.03.2018 13:01, Janosch Frank wrote: > Storage keys are not used by Linux anymore, so let's show them some > love and test if the basics still work. > > Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> > --- > lib/s390x/asm/mem.h | 61 ++++++++++++++++++++++++++++++++++ > s390x/Makefile | 1 + > s390x/skey.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > s390x/unittests.cfg | 3 ++ > 4 files changed, 159 insertions(+) > create mode 100644 lib/s390x/asm/mem.h > create mode 100644 s390x/skey.c > > diff --git a/lib/s390x/asm/mem.h b/lib/s390x/asm/mem.h > new file mode 100644 > index 0000000..28772b0 > --- /dev/null > +++ b/lib/s390x/asm/mem.h > @@ -0,0 +1,61 @@ > +/* > + * Physical memory management related functions and definitions. > + * > + * Copyright IBM Corp. 2017 2018 ? > + * Author(s): Janosch Frank <frankja@de.ibm.com> > + * > + * 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. > + */ > +#ifndef _ASM_S390_MEM_H > +#define _ASM_S390_MEM_H > + > +union skey { > + struct { > + uint8_t acc : 4; > + uint8_t fp : 1; > + uint8_t rf : 1; > + uint8_t ch : 1; > + uint8_t pad : 1; > + } str; > + uint8_t val; > +}; > + > +static inline void set_storage_key(unsigned long addr, > + unsigned char skey, > + int nq) I did not spot any occurance of set_storage_key(..., true) in this patch, so do you really need this parameter? > +{ > + if (nq && test_facility(14)) I think you could simply drop the test_facility check here since the bit is ignored accoring to the PoP if the facility is not installed. > + asm volatile(".insn rrf,0xb22b0000,%0,%1,8,0" > + : : "d" (skey), "a" (addr)); > + else > + asm volatile("sske %0,%1" : : "d" (skey), "a" (addr)); > +} > + > +static inline unsigned long set_storage_key_mb(unsigned long addr, > + unsigned char skey) > +{ > + if (!test_facility(8)) > + return 0; You check that already in test_set_mb() ... so I'd either remove this if-statement or turn it into a assert(test_facility(8)). > + /* As we only have one cpu and no concurrent skey changes, > + * we're able to use the non-quescing option (if available) to > + * speed things up a little. > + */ > + if (test_facility(14)) Again, no need to explicitly check for that facility here - the bit is ignored if the facility is not available. > + asm volatile(".insn rrf,0xb22b0000,%[skey],%[addr],9,0" > + : [addr] "+a" (addr) : [skey] "d" (skey)); > + else > + asm volatile(".insn rrf,0xb22b0000,%[skey],%[addr],1,0" > + : [addr] "+a" (addr) : [skey] "d" (skey)); > + return addr; > +} > + > +static inline unsigned char get_storage_key(unsigned long addr) > +{ > + unsigned char skey; > + > + asm volatile("iske %0,%1" : "=d" (skey) : "a" (addr)); > + return skey; > +} > +#endif The other parts of the patch look fine to me. Thomas
On 13.03.2018 19:32, Thomas Huth wrote: > On 13.03.2018 13:01, Janosch Frank wrote: >> Storage keys are not used by Linux anymore, so let's show them some >> love and test if the basics still work. >> >> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> >> --- >> lib/s390x/asm/mem.h | 61 ++++++++++++++++++++++++++++++++++ >> s390x/Makefile | 1 + >> s390x/skey.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++ >> s390x/unittests.cfg | 3 ++ >> 4 files changed, 159 insertions(+) >> create mode 100644 lib/s390x/asm/mem.h >> create mode 100644 s390x/skey.c >> >> diff --git a/lib/s390x/asm/mem.h b/lib/s390x/asm/mem.h >> new file mode 100644 >> index 0000000..28772b0 >> --- /dev/null >> +++ b/lib/s390x/asm/mem.h >> @@ -0,0 +1,61 @@ >> +/* >> + * Physical memory management related functions and definitions. >> + * >> + * Copyright IBM Corp. 2017 > > 2018 ? Sure > >> + * Author(s): Janosch Frank <frankja@de.ibm.com> >> + * >> + * 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. >> + */ >> +#ifndef _ASM_S390_MEM_H >> +#define _ASM_S390_MEM_H >> + >> +union skey { >> + struct { >> + uint8_t acc : 4; >> + uint8_t fp : 1; >> + uint8_t rf : 1; >> + uint8_t ch : 1; >> + uint8_t pad : 1; >> + } str; >> + uint8_t val; >> +}; >> + >> +static inline void set_storage_key(unsigned long addr, >> + unsigned char skey, >> + int nq) > > I did not spot any occurance of set_storage_key(..., true) in this > patch, so do you really need this parameter? No, I just added nq setting because I was at it, if you want I can remove it. > >> +{ >> + if (nq && test_facility(14)) > > I think you could simply drop the test_facility check here since the bit > is ignored accoring to the PoP if the facility is not installed. > >> + asm volatile(".insn rrf,0xb22b0000,%0,%1,8,0" >> + : : "d" (skey), "a" (addr)); >> + else >> + asm volatile("sske %0,%1" : : "d" (skey), "a" (addr)); >> +} >> + >> +static inline unsigned long set_storage_key_mb(unsigned long addr, >> + unsigned char skey) >> +{ >> + if (!test_facility(8)) >> + return 0; > > You check that already in test_set_mb() ... so I'd either remove this > if-statement or turn it into a assert(test_facility(8)). Assert it is. > >> + /* As we only have one cpu and no concurrent skey changes, >> + * we're able to use the non-quescing option (if available) to >> + * speed things up a little. >> + */ >> + if (test_facility(14)) > > Again, no need to explicitly check for that facility here - the bit is > ignored if the facility is not available. Did not know that. > >> + asm volatile(".insn rrf,0xb22b0000,%[skey],%[addr],9,0" >> + : [addr] "+a" (addr) : [skey] "d" (skey)); >> + else >> + asm volatile(".insn rrf,0xb22b0000,%[skey],%[addr],1,0" >> + : [addr] "+a" (addr) : [skey] "d" (skey)); >> + return addr; >> +} >> + >> +static inline unsigned char get_storage_key(unsigned long addr) >> +{ >> + unsigned char skey; >> + >> + asm volatile("iske %0,%1" : "=d" (skey) : "a" (addr)); >> + return skey; >> +} >> +#endif > > The other parts of the patch look fine to me. > > Thomas >
diff --git a/lib/s390x/asm/mem.h b/lib/s390x/asm/mem.h new file mode 100644 index 0000000..28772b0 --- /dev/null +++ b/lib/s390x/asm/mem.h @@ -0,0 +1,61 @@ +/* + * Physical memory management related functions and definitions. + * + * Copyright IBM Corp. 2017 + * Author(s): Janosch Frank <frankja@de.ibm.com> + * + * 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. + */ +#ifndef _ASM_S390_MEM_H +#define _ASM_S390_MEM_H + +union skey { + struct { + uint8_t acc : 4; + uint8_t fp : 1; + uint8_t rf : 1; + uint8_t ch : 1; + uint8_t pad : 1; + } str; + uint8_t val; +}; + +static inline void set_storage_key(unsigned long addr, + unsigned char skey, + int nq) +{ + if (nq && test_facility(14)) + asm volatile(".insn rrf,0xb22b0000,%0,%1,8,0" + : : "d" (skey), "a" (addr)); + else + asm volatile("sske %0,%1" : : "d" (skey), "a" (addr)); +} + +static inline unsigned long set_storage_key_mb(unsigned long addr, + unsigned char skey) +{ + if (!test_facility(8)) + return 0; + + /* As we only have one cpu and no concurrent skey changes, + * we're able to use the non-quescing option (if available) to + * speed things up a little. + */ + if (test_facility(14)) + asm volatile(".insn rrf,0xb22b0000,%[skey],%[addr],9,0" + : [addr] "+a" (addr) : [skey] "d" (skey)); + else + asm volatile(".insn rrf,0xb22b0000,%[skey],%[addr],1,0" + : [addr] "+a" (addr) : [skey] "d" (skey)); + return addr; +} + +static inline unsigned char get_storage_key(unsigned long addr) +{ + unsigned char skey; + + asm volatile("iske %0,%1" : "=d" (skey) : "a" (addr)); + return skey; +} +#endif diff --git a/s390x/Makefile b/s390x/Makefile index e062727..b73031d 100644 --- a/s390x/Makefile +++ b/s390x/Makefile @@ -3,6 +3,7 @@ tests += $(TEST_DIR)/intercept.elf tests += $(TEST_DIR)/emulator.elf tests += $(TEST_DIR)/sieve.elf tests += $(TEST_DIR)/sthyi.elf +tests += $(TEST_DIR)/skey.elf all: directories test_cases diff --git a/s390x/skey.c b/s390x/skey.c new file mode 100644 index 0000000..e33e0e3 --- /dev/null +++ b/s390x/skey.c @@ -0,0 +1,94 @@ +/* + * Storage key tests + * + * Copyright (c) 2017 IBM Corp + * + * Authors: + * Janosch Frank <frankja@linux.vnet.ibm.com> + * + * 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/asm-offsets.h> +#include <asm/interrupt.h> +#include <asm/page.h> +#include <asm/facility.h> +#include <asm/mem.h> + + +static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2))); +const unsigned long page0 = (unsigned long)pagebuf; +const unsigned long page1 = (unsigned long)(pagebuf + PAGE_SIZE); + +static void test_set_mb(void) +{ + union skey skey, ret1, ret2; + unsigned long addr = 0x10000 - 2 * PAGE_SIZE; + unsigned long end = 0x10000; + + /* Multi block support came with EDAT 1 */ + if (!test_facility(8)) + return; + + skey.val = 0x30; + while (addr < end) + addr = set_storage_key_mb(addr, skey.val); + + ret1.val = get_storage_key(end - PAGE_SIZE); + ret2.val = get_storage_key(end - PAGE_SIZE * 2); + report("multi block", ret1.val == ret2.val && ret1.val == skey.val); +} + +static void test_chg(void) +{ + union skey skey1, skey2; + + skey1.val = 0x30; + set_storage_key(page0, skey1.val, 0); + skey1.val = get_storage_key(page0); + pagebuf[0] = 3; + skey2.val = get_storage_key(page0); + report("chg bit test", !skey1.str.ch && skey2.str.ch); +} + +static void test_set(void) +{ + union skey skey, ret; + + skey.val = 0x30; + ret.val = get_storage_key(page0); + set_storage_key(page0, skey.val, 0); + ret.val = get_storage_key(page0); + report("set key test", skey.val == ret.val); +} + +static void test_priv(void) +{ + union skey skey; + + memset(pagebuf, 0, PAGE_SIZE * 2); + expect_pgm_int(); + enter_pstate(); + set_storage_key(page0, 0x30, 0); + check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION); + + skey.val = get_storage_key(page0); + report("skey did not change on exception", skey.str.acc != 3); + + expect_pgm_int(); + enter_pstate(); + get_storage_key(page0); + check_pgm_int_code(PGM_INT_CODE_PRIVILEGED_OPERATION); +} + +int main(void) +{ + report_prefix_push("skey"); + test_priv(); + test_set(); + test_set_mb(); + test_chg(); + report_prefix_pop(); + return report_summary(); +} diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg index 506891a..8321e9b 100644 --- a/s390x/unittests.cfg +++ b/s390x/unittests.cfg @@ -38,3 +38,6 @@ timeout = 600 [sthyi] file = sthyi.elf accel = kvm + +[skey] +file = skey.elf
Storage keys are not used by Linux anymore, so let's show them some love and test if the basics still work. Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> --- lib/s390x/asm/mem.h | 61 ++++++++++++++++++++++++++++++++++ s390x/Makefile | 1 + s390x/skey.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++ s390x/unittests.cfg | 3 ++ 4 files changed, 159 insertions(+) create mode 100644 lib/s390x/asm/mem.h create mode 100644 s390x/skey.c