Hi Zenghui,
On 9/23/20 1:27 PM, Zenghui Yu wrote:
> Hi Eric,
>
> On 2020/3/21 0:19, Eric Auger wrote:
>> From: "Liu, Yi L" <yi.l.liu@xxxxxxxxxxxxxxx>
>>
>> This patch adds an VFIO_IOMMU_SET_PASID_TABLE ioctl
>> which aims to pass the virtual iommu guest configuration
>> to the host. This latter takes the form of the so-called
>> PASID table.
>>
>> Signed-off-by: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx>
>> Signed-off-by: Liu, Yi L <yi.l.liu@xxxxxxxxxxxxxxx>
>> Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx>
>
> [...]
>
>> diff --git a/drivers/vfio/vfio_iommu_type1.c
>> b/drivers/vfio/vfio_iommu_type1.c
>> index a177bf2c6683..bfacbd876ee1 100644
>> --- a/drivers/vfio/vfio_iommu_type1.c
>> +++ b/drivers/vfio/vfio_iommu_type1.c
>> @@ -2172,6 +2172,43 @@ static int vfio_iommu_iova_build_caps(struct
>> vfio_iommu *iommu,
>> return ret;
>> }
>> +static void
>> +vfio_detach_pasid_table(struct vfio_iommu *iommu)
>> +{
>> + struct vfio_domain *d;
>> +
>> + mutex_lock(&iommu->lock);
>> +
>> + list_for_each_entry(d, &iommu->domain_list, next) {
>> + iommu_detach_pasid_table(d->domain);
>> + }
>> + mutex_unlock(&iommu->lock);
>> +}
>> +
>> +static int
>> +vfio_attach_pasid_table(struct vfio_iommu *iommu,
>> + struct vfio_iommu_type1_set_pasid_table *ustruct)
>> +{
>> + struct vfio_domain *d;
>> + int ret = 0;
>> +
>> + mutex_lock(&iommu->lock);
>> +
>> + list_for_each_entry(d, &iommu->domain_list, next) {
>> + ret = iommu_attach_pasid_table(d->domain, &ustruct->config);
>> + if (ret)
>> + goto unwind;
>> + }
>> + goto unlock;
>> +unwind:
>> + list_for_each_entry_continue_reverse(d, &iommu->domain_list, next) {
>> + iommu_detach_pasid_table(d->domain);
>> + }
>> +unlock:
>> + mutex_unlock(&iommu->lock);
>> + return ret;
>> +}
>> +
>> static long vfio_iommu_type1_ioctl(void *iommu_data,
>> unsigned int cmd, unsigned long arg)
>> {
>> @@ -2276,6 +2313,25 @@ static long vfio_iommu_type1_ioctl(void
>> *iommu_data,
>> return copy_to_user((void __user *)arg, &unmap, minsz) ?
>> -EFAULT : 0;
>> + } else if (cmd == VFIO_IOMMU_SET_PASID_TABLE) {
>> + struct vfio_iommu_type1_set_pasid_table ustruct;
>> +
>> + minsz = offsetofend(struct vfio_iommu_type1_set_pasid_table,
>> + config);
>> +
>> + if (copy_from_user(&ustruct, (void __user *)arg, minsz))
>> + return -EFAULT;
>> +
>> + if (ustruct.argsz < minsz)
>> + return -EINVAL;
>> +
>> + if (ustruct.flags & VFIO_PASID_TABLE_FLAG_SET)
>> + return vfio_attach_pasid_table(iommu, &ustruct);
>> + else if (ustruct.flags & VFIO_PASID_TABLE_FLAG_UNSET) {
>> + vfio_detach_pasid_table(iommu);
>> + return 0;
>> + } else
>> + return -EINVAL;
>
> Nit:
>
> What if user-space blindly set both flags? Should we check that only one
> flag is allowed to be set at this stage, and return error otherwise?
Indeed I can check that.
>
> Besides, before going through the whole series [1][2], I'd like to know
> if this is the latest version of your Nested-Stage-Setup work in case I
> had missed something.
>
> [1] https://lore.kernel.org/r/20200320161911.27494-1-eric.auger@xxxxxxxxxx
> [2] https://lore.kernel.org/r/20200414150607.28488-1-eric.auger@xxxxxxxxxx
yes those 2 series are the last ones. Thank you for reviewing.
FYI, I intend to respin within a week or 2 on top of Jacob's [PATCH v10
0/7] IOMMU user API enhancement. But functionally there won't a lot of
changes.
Thanks
Eric
>
>
> Thanks,
> Zenghui
>
