Messages in this thread Patch in this message | | | From | David Hildenbrand <> | Subject | Re: [PATCH v1 1/2] s390x: KVM: accept STSI for CPU topology information | Date | Thu, 15 Jul 2021 10:51:04 +0200 |
| |
On 14.07.21 17:25, Pierre Morel wrote: > STSI(15.1.x) gives information on the CPU configuration topology. > Let's accept the interception of STSI with the function code 15 and > let the userland part of the hypervisor handle it. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > arch/s390/kvm/priv.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c > index 9928f785c677..4ab5f8b7780e 100644 > --- a/arch/s390/kvm/priv.c > +++ b/arch/s390/kvm/priv.c > @@ -856,7 +856,7 @@ static int handle_stsi(struct kvm_vcpu *vcpu) > if (vcpu->arch.sie_block->gpsw.mask & PSW_MASK_PSTATE) > return kvm_s390_inject_program_int(vcpu, PGM_PRIVILEGED_OP); > > - if (fc > 3) { > + if (fc > 3 && fc != 15) { > kvm_s390_set_psw_cc(vcpu, 3); > return 0; > } > @@ -893,6 +893,15 @@ static int handle_stsi(struct kvm_vcpu *vcpu) > goto out_no_data; > handle_stsi_3_2_2(vcpu, (void *) mem); > break; > + case 15: > + if (sel1 != 1 || sel2 < 2 || sel2 > 6) > + goto out_no_data; > + if (vcpu->kvm->arch.user_stsi) { > + insert_stsi_usr_data(vcpu, operand2, ar, fc, sel1, sel2); > + return -EREMOTE; > + } > + kvm_s390_set_psw_cc(vcpu, 3); > + return 0; > } > if (kvm_s390_pv_cpu_is_protected(vcpu)) { > memcpy((void *)sida_origin(vcpu->arch.sie_block), (void *)mem, >
1. Setting GPRS to 0
I was wondering why we have the "vcpu->run->s.regs.gprs[0] = 0;" for existing fc 1,2,3 in case we set cc=0.
Looking at the doc, all I find is:
"CC 0: Requested configuration-level number placed in general register 0 or requested SYSIB informa- tion stored"
But I don't find where it states that we are supposed to set general register 0 to 0. Wouldn't we also have to do it for fc=15 or for none?
If fc 1,2,3 and 15 are to be handled equally, I suggest the following:
diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c index 9928f785c677..6eb86fa58b0b 100644 --- a/arch/s390/kvm/priv.c +++ b/arch/s390/kvm/priv.c @@ -893,17 +893,23 @@ static int handle_stsi(struct kvm_vcpu *vcpu) goto out_no_data; handle_stsi_3_2_2(vcpu, (void *) mem); break; + case 15: + if (sel1 != 1 || sel2 < 2 || sel2 > 6) + goto out_no_data; + break; } - if (kvm_s390_pv_cpu_is_protected(vcpu)) { - memcpy((void *)sida_origin(vcpu->arch.sie_block), (void *)mem, - PAGE_SIZE); - rc = 0; - } else { - rc = write_guest(vcpu, operand2, ar, (void *)mem, PAGE_SIZE); - } - if (rc) { - rc = kvm_s390_inject_prog_cond(vcpu, rc); - goto out; + if (mem) { + if (kvm_s390_pv_cpu_is_protected(vcpu)) { + memcpy((void *)sida_origin(vcpu->arch.sie_block), + (void *)mem, PAGE_SIZE); + } else { + rc = write_guest(vcpu, operand2, ar, (void *)mem, + PAGE_SIZE); + if (rc) { + rc = kvm_s390_inject_prog_cond(vcpu, rc); + goto out; + } + } } if (vcpu->kvm->arch.user_stsi) { insert_stsi_usr_data(vcpu, operand2, ar, fc, sel1, sel2);
2. maximum-MNest facility
" 1. If the maximum-MNest facility is installed and selector 2 exceeds the nonzero model-depen- dent maximum-selector-2 value."
2. If the maximum-MNest facility is not installed and selector 2 is not specified as two. "
We will we be handling the presence/absence of the maximum-MNest facility (for our guest?) in QEMU, corect?
I do wonder if we should just let any fc=15 go to user space let the whole sel1 / sel2 checking be handled there. I don't think it's a fast path after all. But no strong opinion.
How do we identify availability of maximum-MNest facility?
3. User space awareness
How can user space identify that we actually forward these intercepts? How can it enable them? The old KVM_CAP_S390_USER_STSI capability is not sufficient.
I do wonder if we want KVM_CAP_S390_USER_STSI_15 or sth like that to change the behavior once enabled by user space.
4. Without vcpu->kvm->arch.user_stsi, we indicate cc=0 to our guest, also for fc 1,2,3. Is that actually what we want? (or do we simply not care because the guest is not supposed to use stsi?)
-- Thanks,
David / dhildenb
| |