Re: [PATCH 1/1] s390x:clp: implementing CLP immediate commands

[Pierre Morel]

On 10/12/21 09:27, Thomas Huth wrote:
> On 17/09/2021 14.06, Pierre Morel wrote:
> > CLP immediate commands allow to query the Logical Processor
> > available on the machine and to check for a specific one.
> >
> > Let's add these commands.
> >
> > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> > ---
> >   hw/s390x/s390-pci-inst.c         | 33 ++++++++++++++++++++++++++++++++
> >   include/hw/s390x/s390-pci-inst.h |  5 +++++
> >   target/s390x/kvm/kvm.c           |  6 ++++++
> >   3 files changed, 44 insertions(+)
> >
> > diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> > index 1c8ad91175..9fd0669591 100644
> > --- a/hw/s390x/s390-pci-inst.c
> > +++ b/hw/s390x/s390-pci-inst.c
> > @@ -156,6 +156,39 @@ out:
> >       return rc;
> >   }
> > +int clp_immediate_cmd(S390CPU *cpu, uint8_t r1, uint8_t r2, uint8_t i3,
> > +                      uintptr_t ra)
> > +{
> > +    CPUS390XState *env = &cpu->env;
> > +
> > +    switch (r2) {
> > +    case 0: /* Command Check */
> > +        switch (i3 & 0x07) {
> > +        case CLP_LPS_PCI: /* PCI */
> > +            if (!s390_has_feat(S390_FEAT_ZPCI)) {
> > +                setcc(cpu, 3);
> > +                return 0;
> > +            }
> > +            /* fallthrough */
> > +        case CLP_LPS_BASE: /* Base LP */
> > +            setcc(cpu, 0);
> > +            return 0;
> > +        }
> > +        setcc(cpu, 3);
> > +        return 0;
> > +    case 1: /* Command Query */
> > +        env->regs[r1] = CLP_QUERY_LP_BASE;
> > +        if (s390_has_feat(S390_FEAT_ZPCI)) {
> > +            env->regs[r1] |= CLP_QUERY_LP_BASE >> CLP_LPS_PCI;
> > +        }
> > +        setcc(cpu, 0);
> > +        return 0;
> > +    }
> > +
> > +    s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>
> Just a matter of taste, but I'd rather put that into a "default:" case, 
> so that the other cases could "break" instead of returning immediately.

OK, is better.

> > +    return 0;
> > +}
>
> All return statements return 0. Thus the return value does not really 
> matter, you could also turn this into a "void" function.

Yes.

> >   int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
> >   {
> >       ClpReqHdr *reqh;
> > diff --git a/include/hw/s390x/s390-pci-inst.h 
> > b/include/hw/s390x/s390-pci-inst.h
> > index a55c448aad..07721b08da 100644
> > --- a/include/hw/s390x/s390-pci-inst.h
> > +++ b/include/hw/s390x/s390-pci-inst.h
> > @@ -101,6 +101,11 @@ typedef struct ZpciFib {
> >   int pci_dereg_irqs(S390PCIBusDevice *pbdev);
> >   void pci_dereg_ioat(S390PCIIOMMU *iommu);
> >   int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra);
> > +#define CLP_LPS_BASE 0
> > +#define CLP_LPS_PCI  2
> > +#define CLP_QUERY_LP_BASE (1UL << 63)
> > +int clp_immediate_cmd(S390CPU *cpu, uint8_t r1, uint8_t r2, uint8_t i3,
> > +                      uintptr_t ra);
> >   int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, 
> > uintptr_t ra);
> >   int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, 
> > uintptr_t ra);
> >   int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, 
> > uintptr_t ra);
> > diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> > index 0a5f2aced2..af1316372d 100644
> > --- a/target/s390x/kvm/kvm.c
> > +++ b/target/s390x/kvm/kvm.c
> > @@ -1345,7 +1345,13 @@ static uint64_t get_base_disp_rsy(S390CPU *cpu, 
> > struct kvm_run *run,
> >   static int kvm_clp_service_call(S390CPU *cpu, struct kvm_run *run)
> >   {
> > +    uint8_t r1 = (run->s390_sieic.ipb & 0x00f00000) >> 20;
> >       uint8_t r2 = (run->s390_sieic.ipb & 0x000f0000) >> 16;
> > +    uint8_t i3 = (run->s390_sieic.ipb & 0xff000000) >> 24;
> > +
> > +    if (i3 & 0x80) {
> > +        return clp_immediate_cmd(cpu, r1, r2, i3, RA_IGNORED);
> > +    }
> >       if (s390_has_feat(S390_FEAT_ZPCI)) {
> >           return clp_service_call(cpu, r2, RA_IGNORED);
>
> Could you please adjust the TCG part accordingly, too?
>
>   Thomas

Thanks for the comments,
I adjust TCG too.

Regards,
Pierre


--
Pierre Morel
IBM Lab Boeblingen