Re: [kvm-unit-tests PATCH v3] s390x: Test effect of storage keys on some instructions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/22/22 13:56, Claudio Imbrenda wrote:
> On Thu, 21 Apr 2022 11:04:21 +0200
> Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx> wrote:
> 
>> Some instructions are emulated by KVM. Test that KVM correctly emulates
>> storage key checking for two of those instructions (STORE CPU ADDRESS,
>> SET PREFIX).
>> Test success and error conditions, including coverage of storage and
>> fetch protection override.
>> Also add test for TEST PROTECTION, even if that instruction will not be
>> emulated by KVM under normal conditions.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx>
>> ---
>> v2 -> v3:
>>  * fix asm for SET PREFIX zero key test: make input
>>  * implement Thomas' suggestions:
>>    https://lore.kernel.org/kvm/f050da01-4d50-5da5-7f08-6da30f5dbbbe@xxxxxxxxxx/
>>
>> v1 -> v2:
>>  * use install_page instead of manual page table entry manipulation
>>  * check that no store occurred if none is expected
>>  * try to check that no fetch occured if not expected, although in
>>    practice a fetch would probably cause the test to crash
>>  * reset storage key to 0 after test
>>

[...]

>> +static void test_set_prefix(void)
>> +{
>> +	uint32_t *out = (uint32_t *)pagebuf;
>> +	pgd_t *root;
>> +
>> +	report_prefix_push("SET PREFIX");
>> +	root = (pgd_t *)(stctg(1) & PAGE_MASK);
>> +
>> +	asm volatile("stpx	%0" : "=Q"(*out));
>> +
>> +	report_prefix_push("zero key");
>> +	set_storage_key(pagebuf, 0x20, 0);
>> +	asm volatile("spx	%0" :: "Q" (*out));
> 
> so you are changing the prefix to... the old prefix (so nothing
> changes). how do you know that something happened at all?

Basically, I'm only checking that no exception occurs, which would
crash the test. 
> 
> (see my longer comment below)
> 
>> +	report_pass("no exception");
>> +	report_prefix_pop();
>> +
>> +	report_prefix_push("matching key");
>> +	set_storage_key(pagebuf, 0x10, 0);
>> +	set_prefix_key_1(out);
>> +	report_pass("no exception");
>> +	report_prefix_pop();
>> +
>> +	report_prefix_push("mismatching key, no fetch protection");
>> +	set_storage_key(pagebuf, 0x20, 0);
>> +	set_prefix_key_1(out);
>> +	report_pass("no exception");
>> +	report_prefix_pop();
>> +
>> +	report_prefix_push("mismatching key, fetch protection");
>> +	set_storage_key(pagebuf, 0x28, 0);
>> +	expect_pgm_int();
>> +	*out = 0xdeadbeef;
> 
> so here you are trying to set 0xdeadbeef as prefix, right?

In the sense that I'm executing SPX with that as operand, yes, but
the hypervisor is not supposed to ever read that value, so the
content should not matter.

> which would fail for other reasons I guess, since that would be outside
> memory (unless otherwise specified, kvm unit tests run with 128M of ram)
> 
>> +	set_prefix_key_1(out);
>> +	check_pgm_int_code(PGM_INT_CODE_PROTECTION);

This ^ is the important check, the two lines below indeed questionable,
I considered just dropping them.

>> +	asm volatile("stpx	%0" : "=Q"(*out));
>> +	report(*out != 0xdeadbeef, "no fetch occurred");
> 
> and here you check that the prefix has not changed to that "wrong
> value", which would be impossible anyway because it would be outside of
> memory. moreover the address you give is not even in the lower 2G, so
> in case the address has been changed, it would not match your magic
> value anyway.
> 
> for this (and the following) tests, I propose the following:
> 
> add a new
> char tmplowcore[2 * PAGE_SIZE] __attribute((aligned(2*PAGE_SIZE)));
> 
> initialize it properly (memcpy the actual lowcore into it), and use
> that address for SPX. this way if SPX does not fail, at least you would
> have a valid lowcore. and at that point you can check against that
> address instead of your magic

I'll try it out, maybe it will make tcg not crash and just fail the test.

[...]




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux