Re: [PATCH v14 06/11] s390x/cpu topology: interception of PTF instruction
On 1/16/23 19:24, Nina Schoetterl-Glausch wrote: On Thu, 2023-01-05 at 15:53 +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 --- include/hw/s390x/cpu-topology.h| 3 + include/hw/s390x/s390-virtio-ccw.h | 6 ++ target/s390x/cpu.h | 1 + hw/s390x/cpu-topology.c| 92 ++ target/s390x/cpu-sysemu.c | 16 ++ target/s390x/kvm/kvm.c | 11 6 files changed, 129 insertions(+) diff --git a/include/hw/s390x/cpu-topology.h b/include/hw/s390x/cpu-topology.h index 9571aa70e5..33e23d78b9 100644 --- a/include/hw/s390x/cpu-topology.h +++ b/include/hw/s390x/cpu-topology.h @@ -55,11 +55,13 @@ typedef struct S390Topology { QTAILQ_HEAD(, S390TopologyEntry) list; uint8_t *sockets; CpuTopology *smp; +int polarity; } S390Topology; #ifdef CONFIG_KVM bool s390_has_topology(void); void s390_topology_set_cpu(MachineState *ms, S390CPU *cpu, Error **errp); +void s390_topology_set_polarity(int polarity); #else static inline bool s390_has_topology(void) { @@ -68,6 +70,7 @@ static inline bool s390_has_topology(void) static inline void s390_topology_set_cpu(MachineState *ms, S390CPU *cpu, Error **errp) {} +static inline void s390_topology_set_polarity(int polarity) {} #endif extern S390Topology s390_topology; 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 01ade07009..5da4041576 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -864,6 +864,7 @@ 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_reset(void); +void s390_cpu_topology_set(void); I don't like this name much, it's nondescript. s390_cpu_topology_set_modified ? #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 438055c612..e6b4692581 100644 --- a/hw/s390x/cpu-topology.c +++ b/hw/s390x/cpu-topology.c @@ -97,6 +97,98 @@ static s390_topology_id s390_topology_from_cpu(S390CPU *cpu) } /** + * s390_topology_set_polarity + * @polarity: horizontal or vertical + * + * Changes the polarity of all the CPU in the configuration. + * + * If the dedicated CPU modifier attribute is set a vertical + * polarization is always high (Architecture). + * Otherwise we decide to set it as medium. + * + * Once done, advertise a topology change. + */ +void s390_topology_set_polarity(int polarity) I don't like that this function ignores what kind of vertical polarization is passed, it's confusing. That seems like a further reason to split horizontal/vertical from the entitlement. Yes. I will also make the ordering of the TLE at the last moment instead of doing it during hotplug or QAPI command. This is something Cedric proposed some time ago and I think it is right, it will be easier to do it once in STSI than on every change of topology, hotplug, QAPI and polarization change with PTF. Regards, Pierre -- Pierre Morel IBM Lab Boeblingen
Re: [PATCH v14 06/11] s390x/cpu topology: interception of PTF instruction
On 1/16/23 19:24, Nina Schoetterl-Glausch wrote: On Thu, 2023-01-05 at 15:53 +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 --- include/hw/s390x/cpu-topology.h| 3 + include/hw/s390x/s390-virtio-ccw.h | 6 ++ target/s390x/cpu.h | 1 + hw/s390x/cpu-topology.c| 92 ++ target/s390x/cpu-sysemu.c | 16 ++ target/s390x/kvm/kvm.c | 11 6 files changed, 129 insertions(+) diff --git a/include/hw/s390x/cpu-topology.h b/include/hw/s390x/cpu-topology.h index 9571aa70e5..33e23d78b9 100644 --- a/include/hw/s390x/cpu-topology.h +++ b/include/hw/s390x/cpu-topology.h @@ -55,11 +55,13 @@ typedef struct S390Topology { QTAILQ_HEAD(, S390TopologyEntry) list; uint8_t *sockets; CpuTopology *smp; +int polarity; } S390Topology; #ifdef CONFIG_KVM bool s390_has_topology(void); void s390_topology_set_cpu(MachineState *ms, S390CPU *cpu, Error **errp); +void s390_topology_set_polarity(int polarity); #else static inline bool s390_has_topology(void) { @@ -68,6 +70,7 @@ static inline bool s390_has_topology(void) static inline void s390_topology_set_cpu(MachineState *ms, S390CPU *cpu, Error **errp) {} +static inline void s390_topology_set_polarity(int polarity) {} #endif extern S390Topology s390_topology; 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 01ade07009..5da4041576 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -864,6 +864,7 @@ 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_reset(void); +void s390_cpu_topology_set(void); I don't like this name much, it's nondescript. s390_cpu_topology_set_modified ? yes, better. #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 438055c612..e6b4692581 100644 --- a/hw/s390x/cpu-topology.c +++ b/hw/s390x/cpu-topology.c @@ -97,6 +97,98 @@ static s390_topology_id s390_topology_from_cpu(S390CPU *cpu) } /** + * s390_topology_set_polarity + * @polarity: horizontal or vertical + * + * Changes the polarity of all the CPU in the configuration. + * + * If the dedicated CPU modifier attribute is set a vertical + * polarization is always high (Architecture). + * Otherwise we decide to set it as medium. + * + * Once done, advertise a topology change. + */ +void s390_topology_set_polarity(int polarity) I don't like that this function ignores what kind of vertical polarization is passed, it's confusing. That seems like a further reason to split horizontal/vertical from the entitlement. OK, you are right. I remove this function and put the s390_cpu_topology_set() inside the handle_ptf() +{ +S390TopologyEntry *entry; I also expected this function to set s390_topology.polarization, but it doesn't. + +QTAILQ_FOREACH(entry, _topology.list, next) { +if (polarity == S390_TOPOLOGY_POLARITY_HORIZONTAL) { +entry->id.p = polarity; +} else { +if (entry->id.d) { +entry->id.p = S390_TOPOLOGY_POLARITY_VERTICAL_HIGH; +} else { +entry->id.p = S390_TOPOLOGY_POLARITY_VERTICAL_MEDIUM; +} +} +} +s390_cpu_topology_set(); +} + +/* + * s390_handle_ptf: + * + * @register 1: contains the function code + * + * Function codes 0 and 1 handle the CPU polarization. + * We assume an horizontal topology, the only one supported currently + * by Linux, consequently we answer to function code 0, requesting + * horizontal polarization that it is already the current polarization + * and reject vertical polarization request
Re: [PATCH v14 06/11] s390x/cpu topology: interception of PTF instruction
On Thu, 2023-01-05 at 15:53 +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 > --- > include/hw/s390x/cpu-topology.h| 3 + > include/hw/s390x/s390-virtio-ccw.h | 6 ++ > target/s390x/cpu.h | 1 + > hw/s390x/cpu-topology.c| 92 ++ > target/s390x/cpu-sysemu.c | 16 ++ > target/s390x/kvm/kvm.c | 11 > 6 files changed, 129 insertions(+) > > diff --git a/include/hw/s390x/cpu-topology.h b/include/hw/s390x/cpu-topology.h > index 9571aa70e5..33e23d78b9 100644 > --- a/include/hw/s390x/cpu-topology.h > +++ b/include/hw/s390x/cpu-topology.h > @@ -55,11 +55,13 @@ typedef struct S390Topology { > QTAILQ_HEAD(, S390TopologyEntry) list; > uint8_t *sockets; > CpuTopology *smp; > +int polarity; > } S390Topology; > > #ifdef CONFIG_KVM > bool s390_has_topology(void); > void s390_topology_set_cpu(MachineState *ms, S390CPU *cpu, Error **errp); > +void s390_topology_set_polarity(int polarity); > #else > static inline bool s390_has_topology(void) > { > @@ -68,6 +70,7 @@ static inline bool s390_has_topology(void) > static inline void s390_topology_set_cpu(MachineState *ms, > S390CPU *cpu, > Error **errp) {} > +static inline void s390_topology_set_polarity(int polarity) {} > #endif > extern S390Topology s390_topology; > > 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 01ade07009..5da4041576 100644 > --- a/target/s390x/cpu.h > +++ b/target/s390x/cpu.h > @@ -864,6 +864,7 @@ 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_reset(void); > +void s390_cpu_topology_set(void); I don't like this name much, it's nondescript. s390_cpu_topology_set_modified ? > #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 438055c612..e6b4692581 100644 > --- a/hw/s390x/cpu-topology.c > +++ b/hw/s390x/cpu-topology.c > @@ -97,6 +97,98 @@ static s390_topology_id s390_topology_from_cpu(S390CPU > *cpu) > } > > /** > + * s390_topology_set_polarity > + * @polarity: horizontal or vertical > + * > + * Changes the polarity of all the CPU in the configuration. > + * > + * If the dedicated CPU modifier attribute is set a vertical > + * polarization is always high (Architecture). > + * Otherwise we decide to set it as medium. > + * > + * Once done, advertise a topology change. > + */ > +void s390_topology_set_polarity(int polarity) I don't like that this function ignores what kind of vertical polarization is passed, it's confusing. That seems like a further reason to split horizontal/vertical from the entitlement. > +{ > +S390TopologyEntry *entry; I also expected this function to set s390_topology.polarization, but it doesn't. > + > +QTAILQ_FOREACH(entry, _topology.list, next) { > +if (polarity == S390_TOPOLOGY_POLARITY_HORIZONTAL) { > +entry->id.p = polarity; > +} else { > +if (entry->id.d) { > +entry->id.p = S390_TOPOLOGY_POLARITY_VERTICAL_HIGH; > +} else { > +entry->id.p = S390_TOPOLOGY_POLARITY_VERTICAL_MEDIUM; > +} > +} > +} > +s390_cpu_topology_set(); > +} > + > +/* > + * s390_handle_ptf: > + * > + * @register 1: contains the function code > + * > + * Function codes 0 and 1 handle the CPU polarization. > + * We assume an horizontal topology, the only one supported currently > + * by Linux, consequently we answer to function code 0, requesting > + * horizontal polarization that it is already the current polarization > + * and reject vertical