On 5/13/22 14:33, Claudio Imbrenda wrote: > On Fri, 13 May 2022 13:04:34 +0200 > Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx> wrote: > >> On 5/12/22 16:01, Nico Boehr wrote: >>> Upon migration, we expect storage keys being set by the guest to be preserved, >>> so add a test for it. >>> >>> We keep 128 pages and set predictable storage keys. Then, we migrate and check >>> they can be read back and the respective access restrictions are in place when >>> the access key in the PSW doesn't match. >>> >>> TCG currently doesn't implement key-controlled protection, see >>> target/s390x/mmu_helper.c, function mmu_handle_skey(), hence add the relevant >>> tests as xfails. >>> >>> Signed-off-by: Nico Boehr <nrb@xxxxxxxxxxxxx> >>> --- >>> s390x/Makefile | 1 + >>> s390x/migration-skey.c | 98 ++++++++++++++++++++++++++++++++++++++++++ >>> s390x/unittests.cfg | 4 ++ >>> 3 files changed, 103 insertions(+) >>> create mode 100644 s390x/migration-skey.c >>> [...] >>> + for (i = 0; i < NUM_PAGES; i++) { >>> + report_prefix_pushf("page %d", i); >>> + >>> + page = &pagebuf[i][0]; >>> + actual_key.val = get_storage_key(page); >>> + expected_key.val = i * 2; >>> + >>> + /* ignore reference bit */ >>> + actual_key.str.rf = 0; >>> + expected_key.str.rf = 0; >>> + >>> + report(actual_key.val == expected_key.val, "expected_key=0x%x actual_key=0x%x", expected_key.val, actual_key.val); >>> + >>> + /* ensure access key doesn't match storage key and is never zero */ >>> + mismatching_key.str.acc = expected_key.str.acc < 15 ? expected_key.str.acc + 1 : 1; >>> + *page = 0xff; >>> + >>> + expect_pgm_int(); >>> + asm volatile ( >>> + /* set access key */ >>> + "spka 0(%[mismatching_key])\n" >>> + /* try to write page */ >>> + "mvi 0(%[page]), 42\n" >>> + /* reset access key */ >>> + "spka 0\n" >>> + : >>> + : [mismatching_key] "a"(mismatching_key.val), >>> + [page] "a"(page) >>> + : "memory" >>> + ); >>> + check_pgm_int_code_xfail(host_is_tcg(), PGM_INT_CODE_PROTECTION); >>> + report_xfail(host_is_tcg(), *page == 0xff, "no store occured"); >> >> What are you testing with this bit? If storage keys are really effective after the migration? >> I'm wondering if using tprot would not be better, it should simplify the code a lot. >> Plus you'd easily test for fetch protection, too. > > on the other hand you could have tprot successful, but then not honour > the protection it indicates (I don't know how TPROT is implemented in > TCG) Not at all with regards to skeys. But neither is checking the keys on access. And for kvm, both TPROT and checking is handled by SIE. > > to be fair, this test is only about checking that storage keys are > correctly migrated, maybe the check for actual protection is out of > scope > Having more tests does no harm and might uncover things nobody thought of, but I'd also be fine with keeping it short and sweet. [...]