Re: [PATCH v16 06/11] s390x/cpu topology: interception of PTF instruction

[Pierre Morel]

On 2/27/23 13:39, Thomas Huth wrote:
> On 22/02/2023 15.21, 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 hypervisor 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 hypervisor.
> >
> > 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 +++
> >   hw/s390x/cpu-topology.c            | 85 ++++++++++++++++++++++++++++++
> >   target/s390x/kvm/kvm.c             | 11 ++++
> >   3 files changed, 102 insertions(+)
> >
> > 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/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c
> > index 08642e0e04..40253a2444 100644
> > --- a/hw/s390x/cpu-topology.c
> > +++ b/hw/s390x/cpu-topology.c
> > @@ -87,6 +87,89 @@ static void s390_topology_init(MachineState *ms)
> >       QTAILQ_INSERT_HEAD(&s390_topology.list, entry, next);
> >   }
> >   +/**
> > + * s390_topology_set_cpus_entitlement:
> > + * @polarization: polarization requested by the caller
> > + *
> > + * Set all CPU entitlement according to polarization and
> > + * dedication.
> > + * Default vertical entitlement is S390_CPU_ENTITLEMENT_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 S390_CPU_ENTITLEMENT_HIGH
> > + * entitlement.
> > + */
> > +static void s390_topology_set_cpus_entitlement(int polarization)
> > +{
> > +    CPUState *cs;
> > +
> > +    CPU_FOREACH(cs) {
> > +        if (polarization == S390_CPU_POLARIZATION_HORIZONTAL) {
> > +            S390_CPU(cs)->env.entitlement = 0;
>
> Maybe use S390_CPU_ENTITLEMENT_HORIZONTAL instead of "0" ?


OK


> > +        } else if (S390_CPU(cs)->env.dedicated) {
> > +            S390_CPU(cs)->env.entitlement = S390_CPU_ENTITLEMENT_HIGH;
> > +        } else {
> > +            S390_CPU(cs)->env.entitlement = 
> > S390_CPU_ENTITLEMENT_MEDIUM;
> > +        }
> > +    }
> > +}
>
> With the nit above fixed:
> Reviewed-by: Thomas Huth <thuth@redhat.com>
Thanks,

Pierre