Re: [PATCH v2 25/30] vfio-pci/zdev: wire up zPCI interpretive execution support

[Matthew Rosato]

On 1/25/22 8:01 AM, Pierre Morel wrote:
> On 1/14/22 21:31, Matthew Rosato wrote:
> > Introduce support for VFIO_DEVICE_FEATURE_ZPCI_INTERP, which is a new
> > VFIO_DEVICE_FEATURE ioctl.  This interface is used to indicate that an
> > s390x vfio-pci device wishes to enable/disable zPCI interpretive
> > execution, which allows zPCI instructions to be executed directly by
> > underlying firmware without KVM involvement.
> >
> > Signed-off-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
> > ---
> >   arch/s390/include/asm/kvm_pci.h  |  1 +
> >   drivers/vfio/pci/vfio_pci_core.c |  2 +
> >   drivers/vfio/pci/vfio_pci_zdev.c | 78 ++++++++++++++++++++++++++++++++
> >   include/linux/vfio_pci_core.h    | 10 ++++
> >   include/uapi/linux/vfio.h        |  7 +++
> >   include/uapi/linux/vfio_zdev.h   | 15 ++++++
> >   6 files changed, 113 insertions(+)
> >
> > diff --git a/arch/s390/include/asm/kvm_pci.h 
> > b/arch/s390/include/asm/kvm_pci.h
> > index 97a90b37c87d..dc00c3f27a00 100644
> > --- a/arch/s390/include/asm/kvm_pci.h
> > +++ b/arch/s390/include/asm/kvm_pci.h
> > @@ -35,6 +35,7 @@ struct kvm_zdev {
> >       struct kvm_zdev_ioat ioat;
> >       struct zpci_fib fib;
> >       struct notifier_block nb;
> > +    bool interp;
>
> NIT: s/interp/interpretation/ ?

OK

> >   };
> >   int kvm_s390_pci_dev_open(struct zpci_dev *zdev);
> > diff --git a/drivers/vfio/pci/vfio_pci_core.c 
> > b/drivers/vfio/pci/vfio_pci_core.c
> > index fc57d4d0abbe..2b2d64a2190c 100644
> > --- a/drivers/vfio/pci/vfio_pci_core.c
> > +++ b/drivers/vfio/pci/vfio_pci_core.c
> > @@ -1172,6 +1172,8 @@ long vfio_pci_core_ioctl(struct vfio_device 
> > *core_vdev, unsigned int cmd,
> >               mutex_unlock(&vdev->vf_token->lock);
> >               return 0;
> > +        case VFIO_DEVICE_FEATURE_ZPCI_INTERP:
> > +            return vfio_pci_zdev_feat_interp(vdev, feature, arg);
> >           default:
> >               return -ENOTTY;
> >           }
> > diff --git a/drivers/vfio/pci/vfio_pci_zdev.c 
> > b/drivers/vfio/pci/vfio_pci_zdev.c
> > index 5c2bddc57b39..4339f48b98bc 100644
> > --- a/drivers/vfio/pci/vfio_pci_zdev.c
> > +++ b/drivers/vfio/pci/vfio_pci_zdev.c
> > @@ -54,6 +54,10 @@ static int zpci_group_cap(struct zpci_dev *zdev, 
> > struct vfio_info_cap *caps)
> >           .version = zdev->version
> >       };
> > +    /* Some values are different for interpreted devices */
> > +    if (zdev->kzdev && zdev->kzdev->interp)
> > +        cap.maxstbl = zdev->maxstbl;
> > +
> >       return vfio_info_add_capability(caps, &cap.header, sizeof(cap));
> >   }
> > @@ -138,6 +142,72 @@ int vfio_pci_info_zdev_add_caps(struct 
> > vfio_pci_core_device *vdev,
> >       return ret;
> >   }
> > +int vfio_pci_zdev_feat_interp(struct vfio_pci_core_device *vdev,
> > +                  struct vfio_device_feature feature,
> > +                  unsigned long arg)
> > +{
> > +    struct zpci_dev *zdev = to_zpci(vdev->pdev);
> > +    struct vfio_device_zpci_interp *data;
> > +    struct vfio_device_feature *feat;
> > +    unsigned long minsz;
> > +    int size, rc;
> > +
> > +    if (!zdev || !zdev->kzdev)
> > +        return -EINVAL;
> > +
> > +    /* If PROBE specified, return probe results immediately */
> > +    if (feature.flags & VFIO_DEVICE_FEATURE_PROBE)
> > +        return kvm_s390_pci_interp_probe(zdev);
> > +
> > +    /* GET and SET are mutually exclusive */
> > +    if ((feature.flags & VFIO_DEVICE_FEATURE_GET) &&
> > +        (feature.flags & VFIO_DEVICE_FEATURE_SET))
> > +        return -EINVAL;
>
> Isn't the check already done in VFIO core?

Oh, yes you are correct.  Then this can be removed for this patch as 
well as the next 2 patches.