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

[Matthew Rosato <mjrosato@linux.ibm.com>]

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@linux.ibm.com>
>> ---
>>   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.