On Tue, 17 May 2022 13:56:06 +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 | 285 ++++++++++++++++++++++++++++++++++++++++++++
> s390x/unittests.cfg | 1 +
> 2 files changed, 286 insertions(+)
>
> diff --git a/s390x/skey.c b/s390x/skey.c
> index 19fa5721..60ae8158 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,115 @@ 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 chsc_key_1(void *comm_block)
> +{
> + uint32_t program_mask;
> +
> + asm volatile (
> + "spka 0x10\n\t"
> + ".insn rre,0xb25f0000,%[comm_block],0\n\t"
> + "spka 0\n\t"
> + "ipm %[program_mask]\n"
> + : [program_mask] "=d" (program_mask)
> + : [comm_block] "d" (comm_block)
> + : "memory"
> + );
> + return program_mask >> 28;
> +}
> +
> +static const char chsc_msg[] = "Performed store-channel-subsystem-characteristics";
> +static void init_comm_block(uint16_t *comm_block)
> +{
> + memset(comm_block, 0, PAGE_SIZE);
> + /* store-channel-subsystem-characteristics command */
> + comm_block[0] = 0x10;
> + comm_block[1] = 0x10;
> + comm_block[9] = 0;
> +}
> +
> +static void test_channel_subsystem_call(void)
> +{
> + uint16_t *comm_block = (uint16_t *)&pagebuf;
> + unsigned int cc;
> +
> + report_prefix_push("CHANNEL SUBSYSTEM CALL");
> +
> + report_prefix_push("zero key");
> + init_comm_block(comm_block);
> + set_storage_key(comm_block, 0x10, 0);
> + asm volatile (
> + ".insn rre,0xb25f0000,%[comm_block],0\n\t"
> + "ipm %[cc]\n"
> + : [cc] "=d" (cc)
> + : [comm_block] "d" (comm_block)
> + : "memory"
> + );
> + cc = cc >> 28;
> + report(cc == 0 && comm_block[9], chsc_msg);
> + report_prefix_pop();
> +
> + report_prefix_push("matching key");
> + init_comm_block(comm_block);
> + set_storage_key(comm_block, 0x10, 0);
> + cc = chsc_key_1(comm_block);
> + report(cc == 0 && comm_block[9], chsc_msg);
> + report_prefix_pop();
> +
> + report_prefix_push("mismatching key");
> +
> + report_prefix_push("no fetch protection");
> + init_comm_block(comm_block);
> + set_storage_key(comm_block, 0x20, 0);
> + expect_pgm_int();
> + chsc_key_1(comm_block);
> + check_key_prot_exc(ACC_UPDATE, PROT_STORE);
I wonder if ACC_UPDATE is really needed here? you should clearly never
get a read error, right?
> + report_prefix_pop();
> +
> + report_prefix_push("fetch protection");
> + init_comm_block(comm_block);
> + set_storage_key(comm_block, 0x28, 0);
> + expect_pgm_int();
> + chsc_key_1(comm_block);
> + check_key_prot_exc(ACC_UPDATE, PROT_FETCH_STORE);
and here, I guess you would wait for a read error? or is it actually
defined as unpredictable?
(same for all ACC_UPDATE below)
> + report_prefix_pop();
> +
> + ctl_set_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
> +
> + report_prefix_push("storage-protection override, invalid key");
> + set_storage_key(comm_block, 0x20, 0);
> + init_comm_block(comm_block);
> + expect_pgm_int();
> + chsc_key_1(comm_block);
> + check_key_prot_exc(ACC_UPDATE, PROT_STORE);
> + report_prefix_pop();
> +
> + report_prefix_push("storage-protection override, override key");
> + init_comm_block(comm_block);
> + set_storage_key(comm_block, 0x90, 0);
> + cc = chsc_key_1(comm_block);
> + report(cc == 0 && comm_block[9], chsc_msg);
> + report_prefix_pop();
> +
> + ctl_clear_bit(0, CTL0_STORAGE_PROTECTION_OVERRIDE);
> +
> + report_prefix_push("storage-protection override disabled, override key");
> + init_comm_block(comm_block);
> + set_storage_key(comm_block, 0x90, 0);
> + expect_pgm_int();
> + chsc_key_1(comm_block);
> + check_key_prot_exc(ACC_UPDATE, PROT_STORE);
> + report_prefix_pop();
> +
> + report_prefix_pop();
> +
> + set_storage_key(comm_block, 0x00, 0);
> + report_prefix_pop();
> +}
> +
> /*
> * Perform SET PREFIX (SPX) instruction while temporarily executing
> * with access key 1.
> @@ -417,6 +527,179 @@ 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;
> +
> +/*
> + * gcc 12.0.1 warns if schib is < 4k.
> + * We need such addresses to test fetch protection override.
> + */
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Warray-bounds"
I really dislike these pragmas
can we find a nicer way?
> + 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"
> + );
> +#pragma GCC diagnostic pop
> + 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);
> + 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");
> @@ -431,9 +714,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 b456b288..1280ff0f 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
