Re: [PATCH 2/3] selftests: kvm: s390: Add uc_skey VM test case

["Christoph Schlameuss" <schlameuss@linux.ibm.com>]

On Fri Aug 23, 2024 at 9:59 AM CEST, Janosch Frank wrote:
> On 8/19/24 6:00 PM, Christoph Schlameuss wrote:
> > On Fri Aug 16, 2024 at 4:36 PM CEST, Janosch Frank wrote:
> >> On 8/15/24 5:45 PM, Christoph Schlameuss wrote:
> [...]
> >>> +TEST_F(uc_kvm, uc_skey)
> >>> +{
> >>> +	u64 test_vaddr = self->base_gpa + VM_MEM_SIZE - (SZ_1M / 2);
> >>> +	struct kvm_sync_regs *sync_regs = &self->run->s.regs;
> >>> +	struct kvm_run *run = self->run;
> >>> +	u8 skeyvalue = 0x34;
> >>> +
> >>> +	/* copy test_skey_asm to code_hva / code_gpa */
> >>> +	TH_LOG("copy code %p to vm mapped memory %p / %p",
> >>> +	       &test_skey_asm, (void *)self->code_hva, (void *)self->code_gpa);
> >>> +	memcpy((void *)self->code_hva, &test_skey_asm, PAGE_SIZE);
> >>> +
> >>> +	/* set register content for test_skey_asm to access not mapped memory */
> >>> +	sync_regs->gprs[1] = skeyvalue;
> >>> +	sync_regs->gprs[5] = self->base_gpa;
> >>> +	sync_regs->gprs[6] = test_vaddr;
> >>> +	run->kvm_dirty_regs |= KVM_SYNC_GPRS;
> >>> +
> >>> +	self->sie_block->ictl |= ICTL_OPEREXC | ICTL_PINT;
> >>> +	self->sie_block->cpuflags &= ~CPUSTAT_KSS;
> >>
> >> So you don't want KVM to initialize skeys?
> >> Or am I missing a ucontrol skey interaction?
> >>
> >> What about the ICTLs if KSS is not available on the machine?
> > 
> > This is explicitly disabling KSS, not enabling it.
> > Doing that explicitly might not strictly be necessary but I thought this does
> > provide some clarity about the state.
> > 
>
> The 3 skey ICTLs and KSS are used by KVM to get an intercept on the 
> first skey instruction that the guest issues. KVM uses that intercept to 
> initialize the keys and setup skey handling since it's an edge case 
> because Linux doesn't use skeys.
>
> If KSS is available KVM will not set the skey ICTLs but KSS is a 
> "recent" addition (my guess would be ~z13). So if you want to disable 
> skey intercepts regardless of the machine you need to clear all 4 bits.
>
> Are you sure that disabling KSS makes sense and does what you think it does?

I did revisit the normal skey initialization. It is as you say triggered by the
first KSS intercept. But this is where it actually differs in uncontrol VMs. 
kvm_handle_sie_intercept() would normally call kvm_s390_skey_check_enable(). But
in the ucontrol case it exists early and sets KVM_EXIT_S390_SIEIC with the KSS
intercept code.

So I think the best coverage we can produce here is to mimic that within the
tests userspace code. I will restore the KSS interception and handle it in the
next patch version.