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

[Janosch Frank <frankja@linux.ibm.com>]

On 4/26/22 09:53, Janosch Frank wrote:
> On 4/25/22 18:17, Janis Schoetterl-Glausch 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@linux.ibm.com>
[...]

We need to:
a) Fence this in our CI environments
b) Fence this for the gitlab ci s390x kvm entry

Could you please add a patch that removes the skey test from 
.gitlab-ci.yml? It's at the very end of that file.

If the nit below is fixed:
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>


> Please add a comment that we're testing the fetch access of the operand
> since the prefix is only checked for addressing.
> 
>> +static void test_set_prefix(void)
>> +{
>> +	uint32_t *prefix_ptr = (uint32_t *)pagebuf;
>> +	uint32_t *no_override_prefix_ptr;
>> +	uint32_t old_prefix;
>> +	pgd_t *root;
>> +
>> +	report_prefix_push("SET PREFIX");
>> +	root = (pgd_t *)(stctg(1) & PAGE_MASK);
>> +	old_prefix = get_prefix();
>> +	memcpy(lowcore_tmp, 0, sizeof(lowcore_tmp));
>> +	assert(((uint64_t)&lowcore_tmp >> 31) == 0);
>> +	*prefix_ptr = (uint32_t)(uint64_t)&lowcore_tmp;
>> +
>> +	report_prefix_push("zero key");
>> +	set_prefix(old_prefix);
>> +	set_storage_key(prefix_ptr, 0x20, 0);
>> +	set_prefix(*prefix_ptr);
>> +	report(get_prefix() == *prefix_ptr, "set prefix");
>> +	report_prefix_pop();
>> +
>> +	report_prefix_push("matching key");
>> +	set_prefix(old_prefix);
>> +	set_storage_key(pagebuf, 0x10, 0);
>> +	set_prefix_key_1(prefix_ptr);
>> +	report(get_prefix() == *prefix_ptr, "set prefix");
>> +	report_prefix_pop();
>> +
>> +	report_prefix_push("mismatching key");
>> +
>> +	report_prefix_push("no fetch protection");
>> +	set_prefix(old_prefix);
>> +	set_storage_key(pagebuf, 0x20, 0);
>> +	set_prefix_key_1(prefix_ptr);
>> +	report(get_prefix() == *prefix_ptr, "set prefix");
>> +	report_prefix_pop();
>> +
>> +	report_prefix_push("fetch protection");
>> +	set_prefix(old_prefix);
>> +	set_storage_key(pagebuf, 0x28, 0);
>> +	expect_pgm_int();
>> +	set_prefix_key_1(prefix_ptr);
>> +	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
>> +	report(get_prefix() == old_prefix, "did not set prefix");
>> +	report_prefix_pop();
>> +
>> +	register_pgm_cleanup_func(dat_fixup_pgm_int);
>> +
>> +	report_prefix_push("remapped page, fetch protection");
>> +	set_prefix(old_prefix);
>> +	set_storage_key(pagebuf, 0x28, 0);
>> +	expect_pgm_int();
>> +	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
>> +	set_prefix_key_1((uint32_t *)0);
>> +	install_page(root, 0, 0);
>> +	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
>> +	report(get_prefix() == old_prefix, "did not set prefix");
>> +	report_prefix_pop();
>> +
>> +	ctl_set_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
>> +
>> +	report_prefix_push("fetch protection override applies");
>> +	set_prefix(old_prefix);
>> +	set_storage_key(pagebuf, 0x28, 0);
>> +	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
>> +	set_prefix_key_1((uint32_t *)0);
>> +	install_page(root, 0, 0);
>> +	report(get_prefix() == *prefix_ptr, "set prefix");
>> +	report_prefix_pop();
>> +
>> +	no_override_prefix_ptr = (uint32_t *)(pagebuf + 2048);
>> +	WRITE_ONCE(*no_override_prefix_ptr, (uint32_t)(uint64_t)&lowcore_tmp);
>> +	report_prefix_push("fetch protection override does not apply");
>> +	set_prefix(old_prefix);
>> +	set_storage_key(pagebuf, 0x28, 0);
>> +	expect_pgm_int();
>> +	install_page(root, virt_to_pte_phys(root, pagebuf), 0);
>> +	set_prefix_key_1((uint32_t *)2048);
>> +	install_page(root, 0, 0);
>> +	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
>> +	report(get_prefix() == old_prefix, "did not set prefix");
>> +	report_prefix_pop();
>> +
>> +	ctl_clear_bit(0, CTL0_FETCH_PROTECTION_OVERRIDE);
>> +	register_pgm_cleanup_func(NULL);
>> +	report_prefix_pop();
>> +	set_storage_key(pagebuf, 0x00, 0);
>> +	report_prefix_pop();
>> +}
>> +
>>    int main(void)
>>    {
>>    	report_prefix_push("skey");
>> @@ -130,6 +352,11 @@ int main(void)
>>    	test_set();
>>    	test_set_mb();
>>    	test_chg();
>> +	test_test_protection();
>> +	test_store_cpu_address();
>> +
>> +	setup_vm();
>> +	test_set_prefix();
>>    done:
>>    	report_prefix_pop();
>>    	return report_summary();
>>
>> base-commit: 6a7a83ed106211fc0ee530a3a05f171f6a4c4e66
>