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.
[...]
