On Thu, 5 May 2022 14:46:56 +0200 Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx> wrote: > Test correctness of some instructions handled by user space instead of > KVM with regards to storage keys. > Test success and error conditions, including coverage of storage and > fetch protection override. > > Signed-off-by: Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx> > --- > s390x/skey.c | 277 ++++++++++++++++++++++++++++++++++++++++++++ > s390x/unittests.cfg | 1 + > 2 files changed, 278 insertions(+) > > diff --git a/s390x/skey.c b/s390x/skey.c > index 56bf5f45..d50470a8 100644 > --- a/s390x/skey.c > +++ b/s390x/skey.c > @@ -12,6 +12,7 @@ > #include <asm/asm-offsets.h> > #include <asm/interrupt.h> > #include <vmalloc.h> > +#include <css.h> > #include <asm/page.h> > #include <asm/facility.h> > #include <asm/mem.h> > @@ -284,6 +285,114 @@ static void test_store_cpu_address(void) > report_prefix_pop(); > } > > +/* > + * Perform CHANNEL SUBSYSTEM CALL (CHSC) instruction while temporarily executing > + * with access key 1. > + */ > +static unsigned int channel_subsystem_call_key_1(void *communication_block) this function name is very long (maybe chsc_with_key_1 instead?) > +{ > + uint32_t program_mask; > + > + asm volatile ( > + "spka 0x10\n\t" > + ".insn rre,0xb25f0000,%[communication_block],0\n\t" > + "spka 0\n\t" > + "ipm %[program_mask]\n" > + : [program_mask] "=d" (program_mask) > + : [communication_block] "d" (communication_block) > + : "memory" > + ); > + return program_mask >> 28; > +} > + > +static void init_store_channel_subsystem_characteristics(uint16_t *communication_block) same here (init_comm_block?) > +{ > + memset(communication_block, 0, PAGE_SIZE); > + communication_block[0] = 0x10; > + communication_block[1] = 0x10; > + communication_block[9] = 0; > +} > + > +static void test_channel_subsystem_call(void) > +{ > + static const char request_name[] = "Store channel-subsystem-characteristics"; so this "request_name" is for when CHSC succeeds? why not just "Success" then? > + uint16_t *communication_block = (uint16_t *)&pagebuf; long name (consider comm_block, or even cb) > + unsigned int cc; > + > + report_prefix_push("CHANNEL SUBSYSTEM CALL"); > + > + report_prefix_push("zero key"); > + init_store_channel_subsystem_characteristics(communication_block); see what I mean when I say that the names are too long? ^ > + set_storage_key(communication_block, 0x10, 0); > + asm volatile ( > + ".insn rre,0xb25f0000,%[communication_block],0\n\t" > + "ipm %[cc]\n" > + : [cc] "=d" (cc) > + : [communication_block] "d" (communication_block) > + : "memory" > + ); > + cc = cc >> 28; > + report(cc == 0 && communication_block[9], request_name); > + report_prefix_pop(); > + > + report_prefix_push("matching key"); > + init_store_channel_subsystem_characteristics(communication_block); > + set_storage_key(communication_block, 0x10, 0); you just set the storage key in the previous test, and you did not set it back to 0, why do you need to set it again? > + cc = channel_subsystem_call_key_1(communication_block); > + report(cc == 0 && communication_block[9], request_name); > + report_prefix_pop(); > + > + report_prefix_push("mismatching key"); > + > + report_prefix_push("no fetch protection"); > + init_store_channel_subsystem_characteristics(communication_block); > + set_storage_key(communication_block, 0x20, 0); > + expect_pgm_int(); > + channel_subsystem_call_key_1(communication_block); > + check_key_prot_exc(ACC_UPDATE, PROT_STORE); > + report_prefix_pop(); > + > + report_prefix_push("fetch protection"); > + init_store_channel_subsystem_characteristics(communication_block); > + set_storage_key(communication_block, 0x28, 0); > + expect_pgm_int(); > + channel_subsystem_call_key_1(communication_block); > + check_key_prot_exc(ACC_UPDATE, PROT_FETCH_STORE); > + report_prefix_pop(); > + > + ctl_set_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE); > + > + report_prefix_push("storage-protection override, invalid key"); > + set_storage_key(communication_block, 0x20, 0); > + init_store_channel_subsystem_characteristics(communication_block); > + expect_pgm_int(); > + channel_subsystem_call_key_1(communication_block); > + check_key_prot_exc(ACC_UPDATE, PROT_STORE); > + report_prefix_pop(); > + > + report_prefix_push("storage-protection override, override key"); > + init_store_channel_subsystem_characteristics(communication_block); > + set_storage_key(communication_block, 0x90, 0); > + cc = channel_subsystem_call_key_1(communication_block); > + report(cc == 0 && communication_block[9], request_name); > + report_prefix_pop(); > + > + ctl_clear_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE); > + > + report_prefix_push("storage-protection override disabled, override key"); > + init_store_channel_subsystem_characteristics(communication_block); > + set_storage_key(communication_block, 0x90, 0); > + expect_pgm_int(); > + channel_subsystem_call_key_1(communication_block); > + check_key_prot_exc(ACC_UPDATE, PROT_STORE); > + report_prefix_pop(); > + > + report_prefix_pop(); > + > + set_storage_key(communication_block, 0x00, 0); > + report_prefix_pop(); > +} > + > /* > * Perform SET PREFIX (SPX) instruction while temporarily executing > * with access key 1. > @@ -410,6 +519,172 @@ static void test_set_prefix(void) > report_prefix_pop(); > } > > +/* > + * Perform MODIFY SUBCHANNEL (MSCH) instruction while temporarily executing > + * with access key 1. > + */ > +static uint32_t modify_subchannel_key_1(uint32_t sid, struct schib *schib) > +{ > + uint32_t program_mask; > + > + asm volatile ( > + "lr %%r1,%[sid]\n\t" > + "spka 0x10\n\t" > + "msch %[schib]\n\t" > + "spka 0\n\t" > + "ipm %[program_mask]\n" > + : [program_mask] "=d" (program_mask) > + : [sid] "d" (sid), > + [schib] "Q" (*schib) > + : "%r1" > + ); > + return program_mask >> 28; > +} > + > +static void test_msch(void) > +{ > + struct schib *schib = (struct schib *)pagebuf; > + struct schib *no_override_schib; > + int test_device_sid; > + pgd_t *root; > + int cc; > + > + report_prefix_push("MSCH"); > + root = (pgd_t *)(stctg(1) & PAGE_MASK); > + test_device_sid = css_enumerate(); > + > + if (!(test_device_sid & SCHID_ONE)) { > + report_fail("no I/O device found"); > + return; > + } > + > + cc = stsch(test_device_sid, schib); > + if (cc) { > + report_fail("could not store SCHIB"); > + return; > + } > + > + report_prefix_push("zero key"); > + schib->pmcw.intparm = 100; > + set_storage_key(schib, 0x28, 0); > + cc = msch(test_device_sid, schib); > + if (!cc) { > + WRITE_ONCE(schib->pmcw.intparm, 0); why are you using WRITE_ONCE here? > + cc = stsch(test_device_sid, schib); > + report(!cc && schib->pmcw.intparm == 100, "fetched from SCHIB"); > + } else { > + report_fail("MSCH cc != 0"); > + } > + report_prefix_pop(); > + > + report_prefix_push("matching key"); > + schib->pmcw.intparm = 200; > + set_storage_key(schib, 0x18, 0); > + cc = modify_subchannel_key_1(test_device_sid, schib); > + if (!cc) { > + WRITE_ONCE(schib->pmcw.intparm, 0); > + cc = stsch(test_device_sid, schib); > + report(!cc && schib->pmcw.intparm == 200, "fetched from SCHIB"); > + } else { > + report_fail("MSCH cc != 0"); > + } > + report_prefix_pop(); > + > + report_prefix_push("mismatching key"); > + > + report_prefix_push("no fetch protection"); > + schib->pmcw.intparm = 300; > + set_storage_key(schib, 0x20, 0); > + cc = modify_subchannel_key_1(test_device_sid, schib); > + if (!cc) { > + WRITE_ONCE(schib->pmcw.intparm, 0); > + cc = stsch(test_device_sid, schib); > + report(!cc && schib->pmcw.intparm == 300, "fetched from SCHIB"); > + } else { > + report_fail("MSCH cc != 0"); > + } > + report_prefix_pop(); > + > + schib->pmcw.intparm = 0; > + if (!msch(test_device_sid, schib)) { > + report_prefix_push("fetch protection"); > + schib->pmcw.intparm = 400; > + set_storage_key(schib, 0x28, 0); > + expect_pgm_int(); > + modify_subchannel_key_1(test_device_sid, schib); > + check_key_prot_exc(ACC_FETCH, PROT_FETCH_STORE); > + WRITE_ONCE(schib->pmcw.intparm, 0); > + cc = stsch(test_device_sid, schib); > + report(!cc && schib->pmcw.intparm == 0, "did not modify subchannel"); > + report_prefix_pop(); > + } else { > + report_fail("could not reset SCHIB"); > + } > + > + register_pgm_cleanup_func(dat_fixup_pgm_int); > + > + schib->pmcw.intparm = 0; > + if (!msch(test_device_sid, schib)) { > + report_prefix_push("remapped page, fetch protection"); > + schib->pmcw.intparm = 500; > + set_storage_key(pagebuf, 0x28, 0); > + expect_pgm_int(); > + install_page(root, virt_to_pte_phys(root, pagebuf), 0); > + modify_subchannel_key_1(test_device_sid, (struct schib *)0); > + install_page(root, 0, 0); > + check_key_prot_exc(ACC_FETCH, PROT_FETCH_STORE); > + WRITE_ONCE(schib->pmcw.intparm, 0); > + cc = stsch(test_device_sid, schib); > + report(!cc && schib->pmcw.intparm == 0, "did not modify subchannel"); > + report_prefix_pop(); > + } else { > + report_fail("could not reset SCHIB"); > + } > + > + ctl_set_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE); > + > + report_prefix_push("fetch-protection override applies"); > + schib->pmcw.intparm = 600; > + set_storage_key(pagebuf, 0x28, 0); > + install_page(root, virt_to_pte_phys(root, pagebuf), 0); > + cc = modify_subchannel_key_1(test_device_sid, (struct schib *)0); > + install_page(root, 0, 0); > + if (!cc) { > + WRITE_ONCE(schib->pmcw.intparm, 0); > + cc = stsch(test_device_sid, schib); > + report(!cc && schib->pmcw.intparm == 600, "fetched from SCHIB"); > + } else { > + report_fail("MSCH cc != 0"); > + } > + report_prefix_pop(); > + > + schib->pmcw.intparm = 0; > + if (!msch(test_device_sid, schib)) { > + report_prefix_push("fetch-protection override does not apply"); > + schib->pmcw.intparm = 700; > + no_override_schib = (struct schib *)(pagebuf + 2048); > + memcpy(no_override_schib, schib, sizeof(struct schib)); > + set_storage_key(pagebuf, 0x28, 0); > + expect_pgm_int(); > + install_page(root, virt_to_pte_phys(root, pagebuf), 0); > + modify_subchannel_key_1(test_device_sid, (struct schib *)2048); > + install_page(root, 0, 0); > + check_key_prot_exc(ACC_FETCH, PROT_FETCH_STORE); > + WRITE_ONCE(schib->pmcw.intparm, 0); > + cc = stsch(test_device_sid, schib); > + report(!cc && schib->pmcw.intparm == 0, "did not modify subchannel"); > + report_prefix_pop(); > + } else { > + report_fail("could not reset SCHIB"); > + } > + > + ctl_clear_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE); > + register_pgm_cleanup_func(NULL); > + report_prefix_pop(); > + set_storage_key(schib, 0x00, 0); > + report_prefix_pop(); > +} > + > int main(void) > { > report_prefix_push("skey"); > @@ -424,9 +699,11 @@ int main(void) > test_chg(); > test_test_protection(); > test_store_cpu_address(); > + test_channel_subsystem_call(); > > setup_vm(); > test_set_prefix(); > + test_msch(); > done: > report_prefix_pop(); > return report_summary(); > diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg > index 743013b2..069c41a7 100644 > --- a/s390x/unittests.cfg > +++ b/s390x/unittests.cfg > @@ -41,6 +41,7 @@ file = sthyi.elf > > [skey] > file = skey.elf > +extra_params = -device virtio-net-ccw > > [diag10] > file = diag10.elf