| Message ID | 20220711084148.25017-3-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: KVM: CPU Topology | expand |
On 7/11/22 10:41, Pierre Morel wrote: > We report a topology change to the guest for any CPU hotplug. > > The reporting to the guest is done using the Multiprocessor > Topology-Change-Report (MTCR) bit of the utility entry in the guest's > SCA which will be cleared during the interpretation of PTF. > > On every vCPU creation we set the MCTR bit to let the guest know the > next time it uses the PTF with command 2 instruction that the > topology changed and that it should use the STSI(15.1.x) instruction > to get the topology details. > > 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 when userland > supports the CPU Topology facility. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > Reviewed-by: Nico Boehr <nrb@linux.ibm.com> Reviewed-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> See nit below. > --- > arch/s390/include/asm/kvm_host.h | 18 +++++++++++++++--- > arch/s390/kvm/kvm-s390.c | 31 +++++++++++++++++++++++++++++++ > arch/s390/kvm/priv.c | 22 ++++++++++++++++++---- > arch/s390/kvm/vsie.c | 8 ++++++++ > 4 files changed, 72 insertions(+), 7 deletions(-) > [...] > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index 8fcb56141689..70436bfff53a 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -1691,6 +1691,32 @@ static int kvm_s390_get_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr) > return ret; > } > > +/** > + * kvm_s390_update_topology_change_report - update CPU topology change report > + * @kvm: guest KVM description > + * @val: set or clear the MTCR bit > + * > + * Updates the Multiprocessor Topology-Change-Report bit to signal > + * the guest with a topology change. > + * This is only relevant if the topology facility is present. > + * > + * The SCA version, bsca or esca, doesn't matter as offset is the same. > + */ > +static void kvm_s390_update_topology_change_report(struct kvm *kvm, bool val) > +{ > + union sca_utility new, old; > + struct bsca_block *sca; > + > + read_lock(&kvm->arch.sca_lock); > + do { > + sca = kvm->arch.sca; I find this assignment being in the loop unintuitive, but it should not make a difference. > + old = READ_ONCE(sca->utility); > + new = old; > + new.mtcr = val; > + } while (cmpxchg(&sca->utility.val, old.val, new.val) != old.val); > + read_unlock(&kvm->arch.sca_lock); > +} > + [...]
On 7/11/22 14:30, Janis Schoetterl-Glausch wrote: > On 7/11/22 10:41, Pierre Morel wrote: >> We report a topology change to the guest for any CPU hotplug. >> >> The reporting to the guest is done using the Multiprocessor >> Topology-Change-Report (MTCR) bit of the utility entry in the guest's >> SCA which will be cleared during the interpretation of PTF. >> >> On every vCPU creation we set the MCTR bit to let the guest know the >> next time it uses the PTF with command 2 instruction that the >> topology changed and that it should use the STSI(15.1.x) instruction >> to get the topology details. >> >> 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 when userland >> supports the CPU Topology facility. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> Reviewed-by: Nico Boehr <nrb@linux.ibm.com> > > Reviewed-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> Thanks. > See nit below. >> --- >> arch/s390/include/asm/kvm_host.h | 18 +++++++++++++++--- >> arch/s390/kvm/kvm-s390.c | 31 +++++++++++++++++++++++++++++++ >> arch/s390/kvm/priv.c | 22 ++++++++++++++++++---- >> arch/s390/kvm/vsie.c | 8 ++++++++ >> 4 files changed, 72 insertions(+), 7 deletions(-) >> > > [...] > >> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >> index 8fcb56141689..70436bfff53a 100644 >> --- a/arch/s390/kvm/kvm-s390.c >> +++ b/arch/s390/kvm/kvm-s390.c >> @@ -1691,6 +1691,32 @@ static int kvm_s390_get_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr) >> return ret; >> } >> >> +/** >> + * kvm_s390_update_topology_change_report - update CPU topology change report >> + * @kvm: guest KVM description >> + * @val: set or clear the MTCR bit >> + * >> + * Updates the Multiprocessor Topology-Change-Report bit to signal >> + * the guest with a topology change. >> + * This is only relevant if the topology facility is present. >> + * >> + * The SCA version, bsca or esca, doesn't matter as offset is the same. >> + */ >> +static void kvm_s390_update_topology_change_report(struct kvm *kvm, bool val) >> +{ >> + union sca_utility new, old; >> + struct bsca_block *sca; >> + >> + read_lock(&kvm->arch.sca_lock); >> + do { >> + sca = kvm->arch.sca; > > I find this assignment being in the loop unintuitive, but it should not make a difference. The price would be an ugly cast. > >> + old = READ_ONCE(sca->utility); >> + new = old; >> + new.mtcr = val; >> + } while (cmpxchg(&sca->utility.val, old.val, new.val) != old.val); >> + read_unlock(&kvm->arch.sca_lock); >> +} >> + > [...] >
On 7/12/22 09:45, Pierre Morel wrote: > > > On 7/11/22 14:30, Janis Schoetterl-Glausch wrote: >> On 7/11/22 10:41, Pierre Morel wrote: >>> We report a topology change to the guest for any CPU hotplug. >>> >>> The reporting to the guest is done using the Multiprocessor >>> Topology-Change-Report (MTCR) bit of the utility entry in the guest's >>> SCA which will be cleared during the interpretation of PTF. >>> >>> On every vCPU creation we set the MCTR bit to let the guest know the >>> next time it uses the PTF with command 2 instruction that the >>> topology changed and that it should use the STSI(15.1.x) instruction >>> to get the topology details. >>> >>> 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 when userland >>> supports the CPU Topology facility. >>> >>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>> Reviewed-by: Nico Boehr <nrb@linux.ibm.com> >> >> Reviewed-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> > > Thanks. > > >> See nit below. >>> --- >>> arch/s390/include/asm/kvm_host.h | 18 +++++++++++++++--- >>> arch/s390/kvm/kvm-s390.c | 31 +++++++++++++++++++++++++++++++ >>> arch/s390/kvm/priv.c | 22 ++++++++++++++++++---- >>> arch/s390/kvm/vsie.c | 8 ++++++++ >>> 4 files changed, 72 insertions(+), 7 deletions(-) >>> >> >> [...] >> >>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >>> index 8fcb56141689..70436bfff53a 100644 >>> --- a/arch/s390/kvm/kvm-s390.c >>> +++ b/arch/s390/kvm/kvm-s390.c >>> @@ -1691,6 +1691,32 @@ static int kvm_s390_get_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr) >>> return ret; >>> } >>> +/** >>> + * kvm_s390_update_topology_change_report - update CPU topology change report >>> + * @kvm: guest KVM description >>> + * @val: set or clear the MTCR bit >>> + * >>> + * Updates the Multiprocessor Topology-Change-Report bit to signal >>> + * the guest with a topology change. >>> + * This is only relevant if the topology facility is present. >>> + * >>> + * The SCA version, bsca or esca, doesn't matter as offset is the same. >>> + */ >>> +static void kvm_s390_update_topology_change_report(struct kvm *kvm, bool val) >>> +{ >>> + union sca_utility new, old; >>> + struct bsca_block *sca; >>> + >>> + read_lock(&kvm->arch.sca_lock); >>> + do { >>> + sca = kvm->arch.sca; >> >> I find this assignment being in the loop unintuitive, but it should not make a difference. > > The price would be an ugly cast. I don't get what you mean. Nothing about the types changes if you move it before the loop. > > >> >>> + old = READ_ONCE(sca->utility); >>> + new = old; >>> + new.mtcr = val; >>> + } while (cmpxchg(&sca->utility.val, old.val, new.val) != old.val); >>> + read_unlock(&kvm->arch.sca_lock); >>> +} >>> + >> [...] >> > >
On 7/12/22 10:50, Janis Schoetterl-Glausch wrote: > On 7/12/22 09:45, Pierre Morel wrote: >> >> >> On 7/11/22 14:30, Janis Schoetterl-Glausch wrote: >>> On 7/11/22 10:41, Pierre Morel wrote: >>>> We report a topology change to the guest for any CPU hotplug. >>>> >>>> The reporting to the guest is done using the Multiprocessor >>>> Topology-Change-Report (MTCR) bit of the utility entry in the guest's >>>> SCA which will be cleared during the interpretation of PTF. >>>> >>>> On every vCPU creation we set the MCTR bit to let the guest know the >>>> next time it uses the PTF with command 2 instruction that the >>>> topology changed and that it should use the STSI(15.1.x) instruction >>>> to get the topology details. >>>> >>>> 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 when userland >>>> supports the CPU Topology facility. >>>> >>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>>> Reviewed-by: Nico Boehr <nrb@linux.ibm.com> >>> >>> Reviewed-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> >> >> Thanks. >> >> >>> See nit below. >>>> --- >>>> arch/s390/include/asm/kvm_host.h | 18 +++++++++++++++--- >>>> arch/s390/kvm/kvm-s390.c | 31 +++++++++++++++++++++++++++++++ >>>> arch/s390/kvm/priv.c | 22 ++++++++++++++++++---- >>>> arch/s390/kvm/vsie.c | 8 ++++++++ >>>> 4 files changed, 72 insertions(+), 7 deletions(-) >>>> >>> >>> [...] >>> >>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >>>> index 8fcb56141689..70436bfff53a 100644 >>>> --- a/arch/s390/kvm/kvm-s390.c >>>> +++ b/arch/s390/kvm/kvm-s390.c >>>> @@ -1691,6 +1691,32 @@ static int kvm_s390_get_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr) >>>> return ret; >>>> } >>>> +/** >>>> + * kvm_s390_update_topology_change_report - update CPU topology change report >>>> + * @kvm: guest KVM description >>>> + * @val: set or clear the MTCR bit >>>> + * >>>> + * Updates the Multiprocessor Topology-Change-Report bit to signal >>>> + * the guest with a topology change. >>>> + * This is only relevant if the topology facility is present. >>>> + * >>>> + * The SCA version, bsca or esca, doesn't matter as offset is the same. >>>> + */ >>>> +static void kvm_s390_update_topology_change_report(struct kvm *kvm, bool val) >>>> +{ >>>> + union sca_utility new, old; >>>> + struct bsca_block *sca; >>>> + >>>> + read_lock(&kvm->arch.sca_lock); >>>> + do { >>>> + sca = kvm->arch.sca; >>> >>> I find this assignment being in the loop unintuitive, but it should not make a difference. >> >> The price would be an ugly cast. > > I don't get what you mean. Nothing about the types changes if you move it before the loop. Yes right, did wrong understand. It is better before. >> >> >>> >>>> + old = READ_ONCE(sca->utility); >>>> + new = old; >>>> + new.mtcr = val; >>>> + } while (cmpxchg(&sca->utility.val, old.val, new.val) != old.val); >>>> + read_unlock(&kvm->arch.sca_lock); >>>> +} >>>> + >>> [...] >>> >> >> >
[...] >>>>> +/** >>>>> + * kvm_s390_update_topology_change_report - update CPU topology change report >>>>> + * @kvm: guest KVM description >>>>> + * @val: set or clear the MTCR bit >>>>> + * >>>>> + * Updates the Multiprocessor Topology-Change-Report bit to signal >>>>> + * the guest with a topology change. >>>>> + * This is only relevant if the topology facility is present. >>>>> + * >>>>> + * The SCA version, bsca or esca, doesn't matter as offset is the same. >>>>> + */ >>>>> +static void kvm_s390_update_topology_change_report(struct kvm *kvm, bool val) >>>>> +{ >>>>> + union sca_utility new, old; >>>>> + struct bsca_block *sca; >>>>> + >>>>> + read_lock(&kvm->arch.sca_lock); >>>>> + do { >>>>> + sca = kvm->arch.sca; >>>> >>>> I find this assignment being in the loop unintuitive, but it should not make a difference. >>> >>> The price would be an ugly cast. >> >> I don't get what you mean. Nothing about the types changes if you move it before the loop. > > Yes right, did wrong understand. > It is better before. With the assignment moved one line up: Reviewed-by: Janosch Frank <frankja@linux.ibm.com> > >>> >>> >>>> >>>>> + old = READ_ONCE(sca->utility); >>>>> + new = old; >>>>> + new.mtcr = val; >>>>> + } while (cmpxchg(&sca->utility.val, old.val, new.val) != old.val); >>>>> + read_unlock(&kvm->arch.sca_lock); >>>>> +} >>>>> + >>>> [...] >>>> >>> >>> >> >
On 7/13/22 10:34, Janosch Frank wrote: > [...] >>>>>> +/** >>>>>> + * kvm_s390_update_topology_change_report - update CPU topology >>>>>> change report >>>>>> + * @kvm: guest KVM description >>>>>> + * @val: set or clear the MTCR bit >>>>>> + * >>>>>> + * Updates the Multiprocessor Topology-Change-Report bit to signal >>>>>> + * the guest with a topology change. >>>>>> + * This is only relevant if the topology facility is present. >>>>>> + * >>>>>> + * The SCA version, bsca or esca, doesn't matter as offset is the >>>>>> same. >>>>>> + */ >>>>>> +static void kvm_s390_update_topology_change_report(struct kvm >>>>>> *kvm, bool val) >>>>>> +{ >>>>>> + union sca_utility new, old; >>>>>> + struct bsca_block *sca; >>>>>> + >>>>>> + read_lock(&kvm->arch.sca_lock); >>>>>> + do { >>>>>> + sca = kvm->arch.sca; >>>>> >>>>> I find this assignment being in the loop unintuitive, but it should >>>>> not make a difference. >>>> >>>> The price would be an ugly cast. >>> >>> I don't get what you mean. Nothing about the types changes if you >>> move it before the loop. >> >> Yes right, did wrong understand. >> It is better before. > With the assignment moved one line up: > Reviewed-by: Janosch Frank <frankja@linux.ibm.com> Thanks > >> >>>> >>>> >>>>> >>>>>> + old = READ_ONCE(sca->utility); >>>>>> + new = old; >>>>>> + new.mtcr = val; >>>>>> + } while (cmpxchg(&sca->utility.val, old.val, new.val) != >>>>>> old.val); >>>>>> + read_unlock(&kvm->arch.sca_lock); >>>>>> +} >>>>>> + >>>>> [...] >>>>> >>>> >>>> >>> >> >
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h index 766028d54a3e..ae6bd3d607de 100644 --- a/arch/s390/include/asm/kvm_host.h +++ b/arch/s390/include/asm/kvm_host.h @@ -93,19 +93,30 @@ union ipte_control { }; }; +union sca_utility { + __u16 val; + struct { + __u16 mtcr : 1; + __u16 reserved : 15; + }; +}; + struct bsca_block { union ipte_control ipte_control; __u64 reserved[5]; __u64 mcn; - __u64 reserved2; + union sca_utility utility; + __u8 reserved2[6]; struct bsca_entry cpu[KVM_S390_BSCA_CPU_SLOTS]; }; struct esca_block { union ipte_control ipte_control; - __u64 reserved1[7]; + __u64 reserved1[6]; + union sca_utility utility; + __u8 reserved2[6]; __u64 mcn[4]; - __u64 reserved2[20]; + __u64 reserved3[20]; struct esca_entry cpu[KVM_S390_ESCA_CPU_SLOTS]; }; @@ -249,6 +260,7 @@ struct kvm_s390_sie_block { #define ECB_SPECI 0x08 #define ECB_SRSI 0x04 #define ECB_HOSTPROTINT 0x02 +#define ECB_PTF 0x01 __u8 ecb; /* 0x0061 */ #define ECB2_CMMA 0x80 #define ECB2_IEP 0x20 diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 8fcb56141689..70436bfff53a 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -1691,6 +1691,32 @@ static int kvm_s390_get_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr) return ret; } +/** + * kvm_s390_update_topology_change_report - update CPU topology change report + * @kvm: guest KVM description + * @val: set or clear the MTCR bit + * + * Updates the Multiprocessor Topology-Change-Report bit to signal + * the guest with a topology change. + * This is only relevant if the topology facility is present. + * + * The SCA version, bsca or esca, doesn't matter as offset is the same. + */ +static void kvm_s390_update_topology_change_report(struct kvm *kvm, bool val) +{ + union sca_utility new, old; + struct bsca_block *sca; + + read_lock(&kvm->arch.sca_lock); + do { + sca = kvm->arch.sca; + old = READ_ONCE(sca->utility); + new = old; + new.mtcr = val; + } while (cmpxchg(&sca->utility.val, old.val, new.val) != old.val); + read_unlock(&kvm->arch.sca_lock); +} + static int kvm_s390_vm_set_attr(struct kvm *kvm, struct kvm_device_attr *attr) { int ret; @@ -2877,6 +2903,7 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu) kvm_clear_async_pf_completion_queue(vcpu); if (!kvm_is_ucontrol(vcpu->kvm)) sca_del_vcpu(vcpu); + kvm_s390_update_topology_change_report(vcpu->kvm, 1); if (kvm_is_ucontrol(vcpu->kvm)) gmap_remove(vcpu->arch.gmap); @@ -3272,6 +3299,8 @@ static int kvm_s390_vcpu_setup(struct kvm_vcpu *vcpu) vcpu->arch.sie_block->ecb |= ECB_HOSTPROTINT; if (test_kvm_facility(vcpu->kvm, 9)) vcpu->arch.sie_block->ecb |= ECB_SRSI; + if (test_kvm_facility(vcpu->kvm, 11)) + vcpu->arch.sie_block->ecb |= ECB_PTF; if (test_kvm_facility(vcpu->kvm, 73)) vcpu->arch.sie_block->ecb |= ECB_TE; if (!kvm_is_ucontrol(vcpu->kvm)) @@ -3403,6 +3432,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) rc = kvm_s390_vcpu_setup(vcpu); if (rc) goto out_ucontrol_uninit; + + kvm_s390_update_topology_change_report(vcpu->kvm, 1); return 0; out_ucontrol_uninit: diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c index 12c464c7cddf..a0f41f65a4f1 100644 --- a/arch/s390/kvm/priv.c +++ b/arch/s390/kvm/priv.c @@ -873,10 +873,20 @@ 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) { - kvm_s390_set_psw_cc(vcpu, 3); - return 0; - } + /* Bailout forbidden function codes */ + if (fc > 3 && fc != 15) + goto out_no_data; + + /* + * fc 15 is fully provided only with + * - PTF/CPU topology support through facility 15 + * - KVM_CAP_S390_USER_STSI + * - and is not provided with protected virtualization + */ + if (fc == 15 && (!test_kvm_facility(vcpu->kvm, 11) || + !vcpu->kvm->arch.user_stsi || + kvm_s390_pv_cpu_is_protected(vcpu))) + goto out_no_data; if (vcpu->run->s.regs.gprs[0] & 0x0fffff00 || vcpu->run->s.regs.gprs[1] & 0xffff0000) @@ -910,6 +920,10 @@ static int handle_stsi(struct kvm_vcpu *vcpu) goto out_no_data; handle_stsi_3_2_2(vcpu, (void *) mem); break; + case 15: /* fc 15 is fully handled in userspace */ + insert_stsi_usr_data(vcpu, operand2, ar, fc, sel1, sel2); + trace_kvm_s390_handle_stsi(vcpu, fc, sel1, sel2, operand2); + return -EREMOTE; } if (kvm_s390_pv_cpu_is_protected(vcpu)) { memcpy((void *)sida_origin(vcpu->arch.sie_block), (void *)mem, diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c index dada78b92691..94138f8f0c1c 100644 --- a/arch/s390/kvm/vsie.c +++ b/arch/s390/kvm/vsie.c @@ -503,6 +503,14 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) /* Host-protection-interruption introduced with ESOP */ if (test_kvm_cpu_feat(vcpu->kvm, KVM_S390_VM_CPU_FEAT_ESOP)) scb_s->ecb |= scb_o->ecb & ECB_HOSTPROTINT; + /* + * CPU Topology + * This facility only uses the utility field of the SCA and none of + * the cpu entries that are problematic with the other interpretation + * facilities so we can pass it through + */ + if (test_kvm_facility(vcpu->kvm, 11)) + scb_s->ecb |= scb_o->ecb & ECB_PTF; /* transactional execution */ if (test_kvm_facility(vcpu->kvm, 73) && wants_tx) { /* remap the prefix is tx is toggled on */