| Message ID | 20230201132051.126868-7-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: CPU Topology | expand |
On 01/02/2023 14.20, Pierre Morel wrote: > When the host supports the CPU topology facility, the PTF > instruction with function code 2 is interpreted by the SIE, > provided that the userland hypervizor activates the interpretation s/hypervizor/hypervisor/ > by using the KVM_CAP_S390_CPU_TOPOLOGY KVM extension. > > The PTF instructions with function code 0 and 1 are intercepted > and must be emulated by the userland hypervizor. dito > During RESET all CPU of the configuration are placed in > horizontal polarity. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- ... > /** > * s390_topology_reset: > * > * Generic reset for CPU topology, calls s390_topology_reset() > * s390_topology_reset() to reset the kernel Modified Topology > * change record. > + * Then set global and all CPUs polarity to POLARITY_HORIZONTAL. You describe here already what's going to happen... > */ > void s390_topology_reset(void) > { > s390_cpu_topology_reset(); > + /* Set global polarity to POLARITY_HORIZONTAL */ ... then here again ... > + s390_topology.polarity = POLARITY_HORIZONTAL; ... and the code is (fortunately) also very self-exaplaining... > + /* Set all CPU polarity to POLARITY_HORIZONTAL */ > + s390_topology_set_cpus_polarity(POLARITY_HORIZONTAL); ... so I'd rather drop the two comments within the function body. > } (rest of the patch looks fine to me) Thomas
On 2/6/23 12:38, Thomas Huth wrote: > On 01/02/2023 14.20, Pierre Morel wrote: >> When the host supports the CPU topology facility, the PTF >> instruction with function code 2 is interpreted by the SIE, >> provided that the userland hypervizor activates the interpretation > > s/hypervizor/hypervisor/ grr, I was pretty sure you already said that to me and I did change it... seems I did not. done now. > >> by using the KVM_CAP_S390_CPU_TOPOLOGY KVM extension. >> >> The PTF instructions with function code 0 and 1 are intercepted >> and must be emulated by the userland hypervizor. > > dito yes, thx. > >> During RESET all CPU of the configuration are placed in >> horizontal polarity. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- > ... >> /** >> * s390_topology_reset: >> * >> * Generic reset for CPU topology, calls s390_topology_reset() >> * s390_topology_reset() to reset the kernel Modified Topology >> * change record. >> + * Then set global and all CPUs polarity to POLARITY_HORIZONTAL. > > You describe here already what's going to happen... > >> */ >> void s390_topology_reset(void) >> { >> s390_cpu_topology_reset(); >> + /* Set global polarity to POLARITY_HORIZONTAL */ > > ... then here again ... > >> + s390_topology.polarity = POLARITY_HORIZONTAL; > > ... and the code is (fortunately) also very self-exaplaining... > >> + /* Set all CPU polarity to POLARITY_HORIZONTAL */ >> + s390_topology_set_cpus_polarity(POLARITY_HORIZONTAL); > > ... so I'd rather drop the two comments within the function body. OK > >> } > > (rest of the patch looks fine to me) > > Thomas > Thanks. Regards, Pierre
On Wed, 2023-02-01 at 14:20 +0100, Pierre Morel wrote: > When the host supports the CPU topology facility, the PTF > instruction with function code 2 is interpreted by the SIE, > provided that the userland hypervizor activates the interpretation > by using the KVM_CAP_S390_CPU_TOPOLOGY KVM extension. > > The PTF instructions with function code 0 and 1 are intercepted > and must be emulated by the userland hypervizor. > > During RESET all CPU of the configuration are placed in > horizontal polarity. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > include/hw/s390x/s390-virtio-ccw.h | 6 ++ > target/s390x/cpu.h | 1 + > hw/s390x/cpu-topology.c | 103 +++++++++++++++++++++++++++++ > target/s390x/cpu-sysemu.c | 14 ++++ > target/s390x/kvm/kvm.c | 11 +++ > 5 files changed, 135 insertions(+) > [...] > diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c > index cf63f3dd01..1028bf4476 100644 > --- a/hw/s390x/cpu-topology.c > +++ b/hw/s390x/cpu-topology.c > @@ -85,16 +85,104 @@ static void s390_topology_init(MachineState *ms) > QTAILQ_INSERT_HEAD(&s390_topology.list, entry, next); > } > > +/** > + * s390_topology_set_cpus_polarity: > + * @polarity: polarity requested by the caller > + * > + * Set all CPU entitlement according to polarity and > + * dedication. > + * Default vertical entitlement is POLARITY_VERTICAL_MEDIUM as > + * it does not require host modification of the CPU provisioning > + * until the host decide to modify individual CPU provisioning > + * using QAPI interface. > + * However a dedicated vCPU will have a POLARITY_VERTICAL_HIGH > + * entitlement. > + */ > +static void s390_topology_set_cpus_polarity(int polarity) Since you set the entitlement field I'd prefer _set_cpus_entitlement or similar. > +{ > + CPUState *cs; > + > + CPU_FOREACH(cs) { > + if (polarity == POLARITY_HORIZONTAL) { > + S390_CPU(cs)->env.entitlement = 0; > + } else if (S390_CPU(cs)->env.dedicated) { > + S390_CPU(cs)->env.entitlement = POLARITY_VERTICAL_HIGH; > + } else { > + S390_CPU(cs)->env.entitlement = POLARITY_VERTICAL_MEDIUM; > + } > + } > +} > + [...] > > /** > @@ -137,6 +225,21 @@ static void s390_topology_cpu_default(S390CPU *cpu, Error **errp) > (smp->books * smp->sockets * smp->cores)) % > smp->drawers; > } Why are the changes below in this patch? > + > + /* > + * Machine polarity is set inside the global s390_topology structure. > + * In the case the polarity is set as horizontal set the entitlement > + * to POLARITY_VERTICAL_MEDIUM which is the better equivalent when > + * machine polarity is set to vertical or POLARITY_VERTICAL_HIGH if > + * the vCPU is dedicated. > + */ > + if (s390_topology.polarity && !env->entitlement) { It'd be more readable if you compared against enum values by name. I don't see why you check s390_topology.polarity. If it is horizontal then the value of the entitlement doesn't matter at all, so you can set it to whatever. All you want to do is enforce dedicated -> VERTICAL_HIGH, right? So why don't you just add + if (cpu->env.dedicated && cpu->env.entitlement != POLARITY_VERTICAL_HIGH) { + error_setg(errp, "A dedicated cpu implies high entitlement"); + return; + } to s390_topology_check? > + if (env->dedicated) { > + env->entitlement = POLARITY_VERTICAL_HIGH; > + } else { > + env->entitlement = POLARITY_VERTICAL_MEDIUM; > + } If it is horizontal, then setting the entitlement is pointless as it will be reset to medium on PTF. So the current polarization is vertical and a cpu is being hotplugged, but setting the entitlement of the cpu being added is also pointless, because it's determined by the dedication. That seems weird. > + } > } > [...]
On 2/6/23 19:34, Nina Schoetterl-Glausch wrote: > On Wed, 2023-02-01 at 14:20 +0100, Pierre Morel wrote: >> When the host supports the CPU topology facility, the PTF >> instruction with function code 2 is interpreted by the SIE, >> provided that the userland hypervizor activates the interpretation >> by using the KVM_CAP_S390_CPU_TOPOLOGY KVM extension. >> >> The PTF instructions with function code 0 and 1 are intercepted >> and must be emulated by the userland hypervizor. >> >> During RESET all CPU of the configuration are placed in >> horizontal polarity. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> include/hw/s390x/s390-virtio-ccw.h | 6 ++ >> target/s390x/cpu.h | 1 + >> hw/s390x/cpu-topology.c | 103 +++++++++++++++++++++++++++++ >> target/s390x/cpu-sysemu.c | 14 ++++ >> target/s390x/kvm/kvm.c | 11 +++ >> 5 files changed, 135 insertions(+) >> > [...] > >> diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c >> index cf63f3dd01..1028bf4476 100644 >> --- a/hw/s390x/cpu-topology.c >> +++ b/hw/s390x/cpu-topology.c >> @@ -85,16 +85,104 @@ static void s390_topology_init(MachineState *ms) >> QTAILQ_INSERT_HEAD(&s390_topology.list, entry, next); >> } >> >> +/** >> + * s390_topology_set_cpus_polarity: >> + * @polarity: polarity requested by the caller >> + * >> + * Set all CPU entitlement according to polarity and >> + * dedication. >> + * Default vertical entitlement is POLARITY_VERTICAL_MEDIUM as >> + * it does not require host modification of the CPU provisioning >> + * until the host decide to modify individual CPU provisioning >> + * using QAPI interface. >> + * However a dedicated vCPU will have a POLARITY_VERTICAL_HIGH >> + * entitlement. >> + */ >> +static void s390_topology_set_cpus_polarity(int polarity) > > Since you set the entitlement field I'd prefer _set_cpus_entitlement or similar. OK if you prefer. > >> +{ >> + CPUState *cs; >> + >> + CPU_FOREACH(cs) { >> + if (polarity == POLARITY_HORIZONTAL) { >> + S390_CPU(cs)->env.entitlement = 0; >> + } else if (S390_CPU(cs)->env.dedicated) { >> + S390_CPU(cs)->env.entitlement = POLARITY_VERTICAL_HIGH; >> + } else { >> + S390_CPU(cs)->env.entitlement = POLARITY_VERTICAL_MEDIUM; >> + } >> + } >> +} >> + > [...] >> >> /** >> @@ -137,6 +225,21 @@ static void s390_topology_cpu_default(S390CPU *cpu, Error **errp) >> (smp->books * smp->sockets * smp->cores)) % >> smp->drawers; >> } > > Why are the changes below in this patch? Because before thos patch we have only horizontal polarization. > >> + >> + /* >> + * Machine polarity is set inside the global s390_topology structure. >> + * In the case the polarity is set as horizontal set the entitlement Sorry here an error in the comment should be : "In the case the polarity is NOT set as horizontal..." >> + * to POLARITY_VERTICAL_MEDIUM which is the better equivalent when >> + * machine polarity is set to vertical or POLARITY_VERTICAL_HIGH if >> + * the vCPU is dedicated. >> + */ >> + if (s390_topology.polarity && !env->entitlement) { > > It'd be more readable if you compared against enum values by name. Right, I will change this to if (s390_topology.polarity != S390_POLARITY_HORIZONTAL && env->entitlement == S390_ENTITLEMENT_UNSET) { > > I don't see why you check s390_topology.polarity. If it is horizontal > then the value of the entitlement doesn't matter at all, so you can set it > to whatever. Right, that is why it is done only for vertical polarization (sorry for the wrong comment) > All you want to do is enforce dedicated -> VERTICAL_HIGH, right? > So why don't you just add > > + if (cpu->env.dedicated && cpu->env.entitlement != POLARITY_VERTICAL_HIGH) { > + error_setg(errp, "A dedicated cpu implies high entitlement"); > + return; > + } > > to s390_topology_check? Here it is to set the default in the case the values are not provided. But where you are right is that I should add a verification to the check function. > >> + if (env->dedicated) { >> + env->entitlement = POLARITY_VERTICAL_HIGH; >> + } else { >> + env->entitlement = POLARITY_VERTICAL_MEDIUM; >> + } > > If it is horizontal, then setting the entitlement is pointless as it will be > reset to medium on PTF. That is why the polarity is tested (sorry for the bad comment) > So the current polarization is vertical and a cpu is being hotplugged, > but setting the entitlement of the cpu being added is also pointless, because > it's determined by the dedication. That seems weird. No it is not determined by the dedication, if there is no dedication the 3 vertical values are possible. Regards, Pierre > >> + } >> } >> > > [...]
On Tue, 2023-02-07 at 10:59 +0100, Pierre Morel wrote: > > On 2/6/23 19:34, Nina Schoetterl-Glausch wrote: > > On Wed, 2023-02-01 at 14:20 +0100, Pierre Morel wrote: > > > When the host supports the CPU topology facility, the PTF > > > instruction with function code 2 is interpreted by the SIE, > > > provided that the userland hypervizor activates the interpretation > > > by using the KVM_CAP_S390_CPU_TOPOLOGY KVM extension. > > > > > > The PTF instructions with function code 0 and 1 are intercepted > > > and must be emulated by the userland hypervizor. > > > > > > During RESET all CPU of the configuration are placed in > > > horizontal polarity. > > > > > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > > > --- > > > include/hw/s390x/s390-virtio-ccw.h | 6 ++ > > > target/s390x/cpu.h | 1 + > > > hw/s390x/cpu-topology.c | 103 +++++++++++++++++++++++++++++ > > > target/s390x/cpu-sysemu.c | 14 ++++ > > > target/s390x/kvm/kvm.c | 11 +++ > > > 5 files changed, 135 insertions(+) > > > > > [...] > > > > > diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c > > > index cf63f3dd01..1028bf4476 100644 > > > --- a/hw/s390x/cpu-topology.c > > > +++ b/hw/s390x/cpu-topology.c > > > @@ -85,16 +85,104 @@ static void s390_topology_init(MachineState *ms) > > > QTAILQ_INSERT_HEAD(&s390_topology.list, entry, next); > > > } > > > > > > +/** > > > + * s390_topology_set_cpus_polarity: > > > + * @polarity: polarity requested by the caller > > > + * > > > + * Set all CPU entitlement according to polarity and > > > + * dedication. > > > + * Default vertical entitlement is POLARITY_VERTICAL_MEDIUM as > > > + * it does not require host modification of the CPU provisioning > > > + * until the host decide to modify individual CPU provisioning > > > + * using QAPI interface. > > > + * However a dedicated vCPU will have a POLARITY_VERTICAL_HIGH > > > + * entitlement. > > > + */ > > > +static void s390_topology_set_cpus_polarity(int polarity) > > > > Since you set the entitlement field I'd prefer _set_cpus_entitlement or similar. > > OK if you prefer. > > > > > > +{ > > > + CPUState *cs; > > > + > > > + CPU_FOREACH(cs) { > > > + if (polarity == POLARITY_HORIZONTAL) { > > > + S390_CPU(cs)->env.entitlement = 0; > > > + } else if (S390_CPU(cs)->env.dedicated) { > > > + S390_CPU(cs)->env.entitlement = POLARITY_VERTICAL_HIGH; > > > + } else { > > > + S390_CPU(cs)->env.entitlement = POLARITY_VERTICAL_MEDIUM; > > > + } > > > + } > > > +} > > > + > > [...] > > > > > > /** > > > @@ -137,6 +225,21 @@ static void s390_topology_cpu_default(S390CPU *cpu, Error **errp) > > > (smp->books * smp->sockets * smp->cores)) % > > > smp->drawers; > > > } > > > > Why are the changes below in this patch? > > Because before thos patch we have only horizontal polarization. Not really since you only enable topology in the next patch. > > > > > > + > > > + /* > > > + * Machine polarity is set inside the global s390_topology structure. > > > + * In the case the polarity is set as horizontal set the entitlement > > Sorry here an error in the comment should be : > "In the case the polarity is NOT set as horizontal..." > > > > + * to POLARITY_VERTICAL_MEDIUM which is the better equivalent when > > > + * machine polarity is set to vertical or POLARITY_VERTICAL_HIGH if > > > + * the vCPU is dedicated. > > > + */ > > > + if (s390_topology.polarity && !env->entitlement) { > > > > It'd be more readable if you compared against enum values by name. > > Right, I will change this to > > if (s390_topology.polarity != S390_POLARITY_HORIZONTAL && > env->entitlement == S390_ENTITLEMENT_UNSET) { > > > > > I don't see why you check s390_topology.polarity. If it is horizontal > > then the value of the entitlement doesn't matter at all, so you can set it > > to whatever. > > Right, that is why it is done only for vertical polarization (sorry for > the wrong comment) I'm saying you don't need to check it at all. You adjust the values for vertical polarization, but you could just always do that since the values don't matter at all if the polarization is horizontal. > > > All you want to do is enforce dedicated -> VERTICAL_HIGH, right? > > So why don't you just add > > > > + if (cpu->env.dedicated && cpu->env.entitlement != POLARITY_VERTICAL_HIGH) { > > + error_setg(errp, "A dedicated cpu implies high entitlement"); > > + return; > > + } > > > to s390_topology_check? > > Here it is to set the default in the case the values are not provided. If no values are provided, they default to dedication=false and entitlement=medium, as defined by patch 1, which are fine and don't need to be adjusted. > > But where you are right is that I should add a verification to the check > function. > > > > > > + if (env->dedicated) { > > > + env->entitlement = POLARITY_VERTICAL_HIGH; > > > + } else { > > > + env->entitlement = POLARITY_VERTICAL_MEDIUM; > > > + } > > > > If it is horizontal, then setting the entitlement is pointless as it will be > > reset to medium on PTF. > > That is why the polarity is tested (sorry for the bad comment) I said this because I'm fine with setting it pointlessly. > > So the current polarization is vertical and a cpu is being hotplugged, > > but setting the entitlement of the cpu being added is also pointless, because > > it's determined by the dedication. That seems weird. > > No it is not determined by the dedication, if there is no dedication the > 3 vertical values are possible. You set it to either high or medium based on the dedication. And for horizontal polarization it obviously doesn't matter. As far as I understand you don't need this because the default values are fine. You should add the check that if a dedicated cpu is hotplugged, then the entitlement must be high, to patch 2 and that's it, no additional changes necessary. > > > Regards, > Pierre > > > > > > + } > > > } > > > > > > > [...] >
On 2/7/23 12:27, Nina Schoetterl-Glausch wrote: > On Tue, 2023-02-07 at 10:59 +0100, Pierre Morel wrote: >> >> On 2/6/23 19:34, Nina Schoetterl-Glausch wrote: >>> On Wed, 2023-02-01 at 14:20 +0100, Pierre Morel wrote: ... >>> [...] >>>> >>>> /** >>>> @@ -137,6 +225,21 @@ static void s390_topology_cpu_default(S390CPU *cpu, Error **errp) >>>> (smp->books * smp->sockets * smp->cores)) % >>>> smp->drawers; >>>> } >>> >>> Why are the changes below in this patch? >> >> Because before thos patch we have only horizontal polarization. > > Not really since you only enable topology in the next patch. >> >>> >>>> + >>>> + /* >>>> + * Machine polarity is set inside the global s390_topology structure. >>>> + * In the case the polarity is set as horizontal set the entitlement >> >> Sorry here an error in the comment should be : >> "In the case the polarity is NOT set as horizontal..." >> >>>> + * to POLARITY_VERTICAL_MEDIUM which is the better equivalent when >>>> + * machine polarity is set to vertical or POLARITY_VERTICAL_HIGH if >>>> + * the vCPU is dedicated. >>>> + */ >>>> + if (s390_topology.polarity && !env->entitlement) { >>> >>> It'd be more readable if you compared against enum values by name. >> >> Right, I will change this to >> >> if (s390_topology.polarity != S390_POLARITY_HORIZONTAL && >> env->entitlement == S390_ENTITLEMENT_UNSET) { >> >>> >>> I don't see why you check s390_topology.polarity. If it is horizontal >>> then the value of the entitlement doesn't matter at all, so you can set it >>> to whatever. >> >> Right, that is why it is done only for vertical polarization (sorry for >> the wrong comment) > > I'm saying you don't need to check it at all. You adjust the values for > vertical polarization, but you could just always do that since the values > don't matter at all if the polarization is horizontal. OK right >> >>> All you want to do is enforce dedicated -> VERTICAL_HIGH, right? >>> So why don't you just add >>> >>> + if (cpu->env.dedicated && cpu->env.entitlement != POLARITY_VERTICAL_HIGH) { >>> + error_setg(errp, "A dedicated cpu implies high entitlement"); >>> + return; >>> + } > >>> to s390_topology_check? >> >> Here it is to set the default in the case the values are not provided. > > If no values are provided, they default to dedication=false and entitlement=medium, > as defined by patch 1, which are fine and don't need to be adjusted. Right, I think I added this when working on modifying the attributes in QAPI and rebased it in the wrong patch. Here it has no sense. > >> >> But where you are right is that I should add a verification to the check >> function. >> >>> >>>> + if (env->dedicated) { >>>> + env->entitlement = POLARITY_VERTICAL_HIGH; >>>> + } else { >>>> + env->entitlement = POLARITY_VERTICAL_MEDIUM; >>>> + } >>> >>> If it is horizontal, then setting the entitlement is pointless as it will be >>> reset to medium on PTF. >> >> That is why the polarity is tested (sorry for the bad comment) > > I said this because I'm fine with setting it pointlessly. > >>> So the current polarization is vertical and a cpu is being hotplugged, >>> but setting the entitlement of the cpu being added is also pointless, because >>> it's determined by the dedication. That seems weird. >> >> No it is not determined by the dedication, if there is no dedication the >> 3 vertical values are possible. > > You set it to either high or medium based on the dedication. And for horizontal > polarization it obviously doesn't matter. on PTF yes but not on hotplug, on hotplug all 3 values are possible. > > As far as I understand you don't need this because the default values are fine. > You should add the check that if a dedicated cpu is hotplugged, then the entitlement > must be high, to patch 2 and that's it, no additional changes necessary. Yes, right. Regards, Pierre
diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h index 9bba21a916..c1d46e78af 100644 --- a/include/hw/s390x/s390-virtio-ccw.h +++ b/include/hw/s390x/s390-virtio-ccw.h @@ -30,6 +30,12 @@ struct S390CcwMachineState { uint8_t loadparm[8]; }; +#define S390_PTF_REASON_NONE (0x00 << 8) +#define S390_PTF_REASON_DONE (0x01 << 8) +#define S390_PTF_REASON_BUSY (0x02 << 8) +#define S390_TOPO_FC_MASK 0xffUL +void s390_handle_ptf(S390CPU *cpu, uint8_t r1, uintptr_t ra); + struct S390CcwMachineClass { /*< private >*/ MachineClass parent_class; diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index 848314d2a9..f6e207afde 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -857,6 +857,7 @@ void s390_enable_css_support(S390CPU *cpu); void s390_do_cpu_set_diag318(CPUState *cs, run_on_cpu_data arg); int s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch_id, int vq, bool assign); +void s390_cpu_topology_set_modified(void); #ifndef CONFIG_USER_ONLY unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu); #else diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c index cf63f3dd01..1028bf4476 100644 --- a/hw/s390x/cpu-topology.c +++ b/hw/s390x/cpu-topology.c @@ -85,16 +85,104 @@ static void s390_topology_init(MachineState *ms) QTAILQ_INSERT_HEAD(&s390_topology.list, entry, next); } +/** + * s390_topology_set_cpus_polarity: + * @polarity: polarity requested by the caller + * + * Set all CPU entitlement according to polarity and + * dedication. + * Default vertical entitlement is POLARITY_VERTICAL_MEDIUM as + * it does not require host modification of the CPU provisioning + * until the host decide to modify individual CPU provisioning + * using QAPI interface. + * However a dedicated vCPU will have a POLARITY_VERTICAL_HIGH + * entitlement. + */ +static void s390_topology_set_cpus_polarity(int polarity) +{ + CPUState *cs; + + CPU_FOREACH(cs) { + if (polarity == POLARITY_HORIZONTAL) { + S390_CPU(cs)->env.entitlement = 0; + } else if (S390_CPU(cs)->env.dedicated) { + S390_CPU(cs)->env.entitlement = POLARITY_VERTICAL_HIGH; + } else { + S390_CPU(cs)->env.entitlement = POLARITY_VERTICAL_MEDIUM; + } + } +} + +/* + * s390_handle_ptf: + * + * @register 1: contains the function code + * + * Function codes 0 (horizontal) and 1 (vertical) define the CPU + * polarization requested by the guest. + * + * Verify that the polarization really need to change and call + * s390_topology_set_cpus_polarity() specifying the requested polarity + * to set for all CPUs. + * + * Function code 2 is handling topology changes and is interpreted + * by the SIE. + */ +void s390_handle_ptf(S390CPU *cpu, uint8_t r1, uintptr_t ra) +{ + CPUS390XState *env = &cpu->env; + uint64_t reg = env->regs[r1]; + int fc = reg & S390_TOPO_FC_MASK; + + if (!s390_has_feat(S390_FEAT_CONFIGURATION_TOPOLOGY)) { + s390_program_interrupt(env, PGM_OPERATION, ra); + return; + } + + if (env->psw.mask & PSW_MASK_PSTATE) { + s390_program_interrupt(env, PGM_PRIVILEGED, ra); + return; + } + + if (reg & ~S390_TOPO_FC_MASK) { + s390_program_interrupt(env, PGM_SPECIFICATION, ra); + return; + } + + switch (fc) { + case POLARITY_VERTICAL: + case POLARITY_HORIZONTAL: + if (s390_topology.polarity == fc) { + env->regs[r1] |= S390_PTF_REASON_DONE; + setcc(cpu, 2); + } else { + s390_topology.polarity = fc; + s390_cpu_topology_set_modified(); + s390_topology_set_cpus_polarity(fc); + setcc(cpu, 0); + } + break; + default: + /* Note that fc == 2 is interpreted by the SIE */ + s390_program_interrupt(env, PGM_SPECIFICATION, ra); + } +} + /** * s390_topology_reset: * * Generic reset for CPU topology, calls s390_topology_reset() * s390_topology_reset() to reset the kernel Modified Topology * change record. + * Then set global and all CPUs polarity to POLARITY_HORIZONTAL. */ void s390_topology_reset(void) { s390_cpu_topology_reset(); + /* Set global polarity to POLARITY_HORIZONTAL */ + s390_topology.polarity = POLARITY_HORIZONTAL; + /* Set all CPU polarity to POLARITY_HORIZONTAL */ + s390_topology_set_cpus_polarity(POLARITY_HORIZONTAL); } /** @@ -137,6 +225,21 @@ static void s390_topology_cpu_default(S390CPU *cpu, Error **errp) (smp->books * smp->sockets * smp->cores)) % smp->drawers; } + + /* + * Machine polarity is set inside the global s390_topology structure. + * In the case the polarity is set as horizontal set the entitlement + * to POLARITY_VERTICAL_MEDIUM which is the better equivalent when + * machine polarity is set to vertical or POLARITY_VERTICAL_HIGH if + * the vCPU is dedicated. + */ + if (s390_topology.polarity && !env->entitlement) { + if (env->dedicated) { + env->entitlement = POLARITY_VERTICAL_HIGH; + } else { + env->entitlement = POLARITY_VERTICAL_MEDIUM; + } + } } /** diff --git a/target/s390x/cpu-sysemu.c b/target/s390x/cpu-sysemu.c index e27864c5f5..82e3f3891e 100644 --- a/target/s390x/cpu-sysemu.c +++ b/target/s390x/cpu-sysemu.c @@ -37,6 +37,7 @@ #include "sysemu/sysemu.h" #include "sysemu/tcg.h" #include "hw/core/sysemu-cpu-ops.h" +#include "hw/s390x/cpu-topology.h" /* S390CPUClass::load_normal() */ static void s390_cpu_load_normal(CPUState *s) @@ -319,3 +320,16 @@ void s390_cpu_topology_reset(void) } } } + +void s390_cpu_topology_set_modified(void) +{ + int ret; + + if (kvm_enabled()) { + ret = kvm_s390_topology_set_mtcr(1); + if (ret) { + error_report("Failed to set Modified Topology Change Report: %s", + strerror(-ret)); + } + } +} diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c index bc953151ce..fb63be41b7 100644 --- a/target/s390x/kvm/kvm.c +++ b/target/s390x/kvm/kvm.c @@ -96,6 +96,7 @@ #define PRIV_B9_EQBS 0x9c #define PRIV_B9_CLP 0xa0 +#define PRIV_B9_PTF 0xa2 #define PRIV_B9_PCISTG 0xd0 #define PRIV_B9_PCILG 0xd2 #define PRIV_B9_RPCIT 0xd3 @@ -1464,6 +1465,13 @@ static int kvm_mpcifc_service_call(S390CPU *cpu, struct kvm_run *run) } } +static void kvm_handle_ptf(S390CPU *cpu, struct kvm_run *run) +{ + uint8_t r1 = (run->s390_sieic.ipb >> 20) & 0x0f; + + s390_handle_ptf(cpu, r1, RA_IGNORED); +} + static int handle_b9(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1) { int r = 0; @@ -1481,6 +1489,9 @@ static int handle_b9(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1) case PRIV_B9_RPCIT: r = kvm_rpcit_service_call(cpu, run); break; + case PRIV_B9_PTF: + kvm_handle_ptf(cpu, run); + break; case PRIV_B9_EQBS: /* just inject exception */ r = -1;
When the host supports the CPU topology facility, the PTF instruction with function code 2 is interpreted by the SIE, provided that the userland hypervizor activates the interpretation by using the KVM_CAP_S390_CPU_TOPOLOGY KVM extension. The PTF instructions with function code 0 and 1 are intercepted and must be emulated by the userland hypervizor. During RESET all CPU of the configuration are placed in horizontal polarity. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- include/hw/s390x/s390-virtio-ccw.h | 6 ++ target/s390x/cpu.h | 1 + hw/s390x/cpu-topology.c | 103 +++++++++++++++++++++++++++++ target/s390x/cpu-sysemu.c | 14 ++++ target/s390x/kvm/kvm.c | 11 +++ 5 files changed, 135 insertions(+)