diff mbox series

[v14,06/11] s390x/cpu topology: interception of PTF instruction

Message ID 20230105145313.168489-7-pmorel@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: CPU Topology | expand

Commit Message

Pierre Morel Jan. 5, 2023, 2:53 p.m. UTC
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/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(+)

Comments

Nina Schoetterl-Glausch Jan. 16, 2023, 6:24 p.m. UTC | #1
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 <pmorel@linux.ibm.com>
> ---
>  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, &s390_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 without further explanation.

This comment is outdated, right? Same for those in the function body.

> + *
> + * 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];
> +    uint8_t 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 0:    /* Horizontal polarization is already set */
> +        if (s390_topology.polarity == S390_TOPOLOGY_POLARITY_HORIZONTAL) {
> +            env->regs[r1] |= S390_PTF_REASON_DONE;
> +            setcc(cpu, 2);
> +        } else {
> +            s390_topology_set_polarity(S390_TOPOLOGY_POLARITY_HORIZONTAL);
> +            s390_topology.polarity = S390_TOPOLOGY_POLARITY_HORIZONTAL;
> +            setcc(cpu, 0);
> +        }
> +        break;
> +    case 1:    /* Vertical polarization is not supported */
> +        if (s390_topology.polarity != S390_TOPOLOGY_POLARITY_HORIZONTAL) {
> +            env->regs[r1] |= S390_PTF_REASON_DONE;
> +            setcc(cpu, 2);
> +        } else {
> +            s390_topology_set_polarity(S390_TOPOLOGY_POLARITY_VERTICAL_LOW);

This is why I said it's confusing, nothing gets set to LOW.

> +            s390_topology.polarity = S390_TOPOLOGY_POLARITY_VERTICAL_LOW;

Why LOW here?

> +            setcc(cpu, 0);
> +        }
> +        break;
> +    default:
> +        /* Note that fc == 2 is interpreted by the SIE */
> +        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
> +    }

You can simplify this by doing:

int new_polarity;
switch (fc) {
case 0:
	new_polarity = S390_TOPOLOGY_POLARITY_HORIZONTAL;
	break;
case 1:
	new_polarity = S390_TOPOLOGY_POLARITY_VERTICAL_?;
	break;
default:
	/* Note that fc == 2 is interpreted by the SIE */
	s390_program_interrupt(env, PGM_SPECIFICATION, ra);
	return;
}

if same polarity:
	rc done, rejected
else
	set polarity, initiated

Might be a good idea to turn the polarity values into an enum.

> +}
[...]
Pierre Morel Jan. 18, 2023, 9:54 a.m. UTC | #2
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 <pmorel@linux.ibm.com>
>> ---
>>   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, &s390_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 without further explanation.
> 
> This comment is outdated, right? Same for those in the function body.
> 
>> + *
>> + * 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];
>> +    uint8_t 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 0:    /* Horizontal polarization is already set */
>> +        if (s390_topology.polarity == S390_TOPOLOGY_POLARITY_HORIZONTAL) {
>> +            env->regs[r1] |= S390_PTF_REASON_DONE;
>> +            setcc(cpu, 2);
>> +        } else {
>> +            s390_topology_set_polarity(S390_TOPOLOGY_POLARITY_HORIZONTAL);
>> +            s390_topology.polarity = S390_TOPOLOGY_POLARITY_HORIZONTAL;
>> +            setcc(cpu, 0);
>> +        }
>> +        break;
>> +    case 1:    /* Vertical polarization is not supported */
>> +        if (s390_topology.polarity != S390_TOPOLOGY_POLARITY_HORIZONTAL) {
>> +            env->regs[r1] |= S390_PTF_REASON_DONE;
>> +            setcc(cpu, 2);
>> +        } else {
>> +            s390_topology_set_polarity(S390_TOPOLOGY_POLARITY_VERTICAL_LOW);
> 
> This is why I said it's confusing, nothing gets set to LOW.
> 
>> +            s390_topology.polarity = S390_TOPOLOGY_POLARITY_VERTICAL_LOW;
> 
> Why LOW here?
I wanted something not being S390_TOPOLOGY_POLARITY_HORIZONTAL and did 
not want to define a S390_TOPOLOGY_POLARITY_VERTICAL.

OK I define S390_TOPOLOGY_POLARITY_HORIZONTAL=0 and ..._VERTICAL=1



> 
>> +            setcc(cpu, 0);
>> +        }
>> +        break;
>> +    default:
>> +        /* Note that fc == 2 is interpreted by the SIE */
>> +        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>> +    }
> 
> You can simplify this by doing:
> 
> int new_polarity;
> switch (fc) {
> case 0:
> 	new_polarity = S390_TOPOLOGY_POLARITY_HORIZONTAL;
> 	break;
> case 1:
> 	new_polarity = S390_TOPOLOGY_POLARITY_VERTICAL_?;
> 	break;
> default:
> 	/* Note that fc == 2 is interpreted by the SIE */
> 	s390_program_interrupt(env, PGM_SPECIFICATION, ra);
> 	return;
> }
> 
> if same polarity:
> 	rc done, rejected
> else
> 	set polarity, initiated
> 
> Might be a good idea to turn the polarity values into an enum.
> 
>> +}
> [...]
> 

Even I never really understood the added value of an enum I can do this.

Thanks,

regards,
Pierre
Pierre Morel Jan. 20, 2023, 2:32 p.m. UTC | #3
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 <pmorel@linux.ibm.com>
>> ---
>>   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
diff mbox series

Patch

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);
 #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)
+{
+    S390TopologyEntry *entry;
+
+    QTAILQ_FOREACH(entry, &s390_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 without further explanation.
+ *
+ * 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];
+    uint8_t 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 0:    /* Horizontal polarization is already set */
+        if (s390_topology.polarity == S390_TOPOLOGY_POLARITY_HORIZONTAL) {
+            env->regs[r1] |= S390_PTF_REASON_DONE;
+            setcc(cpu, 2);
+        } else {
+            s390_topology_set_polarity(S390_TOPOLOGY_POLARITY_HORIZONTAL);
+            s390_topology.polarity = S390_TOPOLOGY_POLARITY_HORIZONTAL;
+            setcc(cpu, 0);
+        }
+        break;
+    case 1:    /* Vertical polarization is not supported */
+        if (s390_topology.polarity != S390_TOPOLOGY_POLARITY_HORIZONTAL) {
+            env->regs[r1] |= S390_PTF_REASON_DONE;
+            setcc(cpu, 2);
+        } else {
+            s390_topology_set_polarity(S390_TOPOLOGY_POLARITY_VERTICAL_LOW);
+            s390_topology.polarity = S390_TOPOLOGY_POLARITY_VERTICAL_LOW;
+            setcc(cpu, 0);
+        }
+        break;
+    default:
+        /* Note that fc == 2 is interpreted by the SIE */
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+    }
+}
+
+ /**
  * s390_topology_set_entry:
  * @entry: Topology entry to setup
  * @id: topology id to use for the setup
diff --git a/target/s390x/cpu-sysemu.c b/target/s390x/cpu-sysemu.c
index e27864c5f5..8643320947 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)
@@ -311,6 +312,8 @@  void s390_cpu_topology_reset(void)
 {
     int ret;
 
+    s390_topology_set_polarity(S390_TOPOLOGY_POLARITY_HORIZONTAL);
+
     if (kvm_enabled()) {
         ret = kvm_s390_topology_set_mtcr(0);
         if (ret) {
@@ -319,3 +322,16 @@  void s390_cpu_topology_reset(void)
         }
     }
 }
+
+void s390_cpu_topology_set(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;