Re: [RFC PATCH v3 8/8] vfio: Add nested IOPF support

2021-05-31 Thread Shenming Lu
On 2021/5/27 19:18, Lu Baolu wrote:
> Hi Shenming and Alex,
> 
> On 5/27/21 7:03 PM, Shenming Lu wrote:
>>> I haven't fully read all the references, but who imposes the fact that
>>> there's only one fault handler per device?  If type1 could register one
>>> handler and the vfio-pci bus driver another for the other level, would
>>> we need this path through vfio-core?
>> If we could register more than one handler per device, things would become
>> much more simple, and the path through vfio-core would not be needed.
>>
>> Hi Baolu,
>> Is there any restriction for having more than one handler per device?
>>
> 
> Currently, each device could only have one fault handler. But one device
> might consume multiple page tables. From this point of view, it's more
> reasonable to have one handler per page table.

Sounds good to me. I have pointed it out in the IOASID uAPI proposal. :-)

Thanks,
Shenming

> 
> Best regards,
> baolu
> .
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC PATCH v3 8/8] vfio: Add nested IOPF support

2021-05-27 Thread Lu Baolu

Hi Shenming and Alex,

On 5/27/21 7:03 PM, Shenming Lu wrote:

I haven't fully read all the references, but who imposes the fact that
there's only one fault handler per device?  If type1 could register one
handler and the vfio-pci bus driver another for the other level, would
we need this path through vfio-core?

If we could register more than one handler per device, things would become
much more simple, and the path through vfio-core would not be needed.

Hi Baolu,
Is there any restriction for having more than one handler per device?



Currently, each device could only have one fault handler. But one device
might consume multiple page tables. From this point of view, it's more
reasonable to have one handler per page table.

Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC PATCH v3 8/8] vfio: Add nested IOPF support

2021-05-27 Thread Shenming Lu
On 2021/5/25 6:11, Alex Williamson wrote:
> On Mon, 24 May 2021 21:11:11 +0800
> Shenming Lu  wrote:
> 
>> On 2021/5/21 15:59, Shenming Lu wrote:
>>> On 2021/5/19 2:58, Alex Williamson wrote:  
 On Fri, 9 Apr 2021 11:44:20 +0800
 Shenming Lu  wrote:
  
> To set up nested mode, drivers such as vfio_pci need to register a
> handler to receive stage/level 1 faults from the IOMMU, but since
> currently each device can only have one iommu dev fault handler,
> and if stage 2 IOPF is already enabled (VFIO_IOMMU_ENABLE_IOPF),
> we choose to update the registered handler (a consolidated one) via
> flags (set FAULT_REPORT_NESTED_L1), and further deliver the received
> stage 1 faults in the handler to the guest through a newly added
> vfio_device_ops callback.  

 Are there proposed in-kernel drivers that would use any of these
 symbols?  
>>>
>>> I hope that such as Eric's SMMUv3 Nested Stage Setup series [1] can
>>> use these symbols to consolidate the two page fault handlers into one.
>>>
>>> [1] 
>>> https://patchwork.kernel.org/project/kvm/cover/2021044659.15051-1-eric.au...@redhat.com/
>>>   
  
> Signed-off-by: Shenming Lu 
> ---
>  drivers/vfio/vfio.c | 81 +
>  drivers/vfio/vfio_iommu_type1.c | 49 +++-
>  include/linux/vfio.h| 12 +
>  3 files changed, 141 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 44c8dfabf7de..4245f15914bf 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -2356,6 +2356,87 @@ struct iommu_domain 
> *vfio_group_iommu_domain(struct vfio_group *group)
>  }
>  EXPORT_SYMBOL_GPL(vfio_group_iommu_domain);
>  
> +/*
> + * Register/Update the VFIO IOPF handler to receive
> + * nested stage/level 1 faults.
> + */
> +int vfio_iommu_dev_fault_handler_register_nested(struct device *dev)
> +{
> + struct vfio_container *container;
> + struct vfio_group *group;
> + struct vfio_iommu_driver *driver;
> + int ret;
> +
> + if (!dev)
> + return -EINVAL;
> +
> + group = vfio_group_get_from_dev(dev);
> + if (!group)
> + return -ENODEV;
> +
> + ret = vfio_group_add_container_user(group);
> + if (ret)
> + goto out;
> +
> + container = group->container;
> + driver = container->iommu_driver;
> + if (likely(driver && driver->ops->register_handler))
> + ret = driver->ops->register_handler(container->iommu_data, dev);
> + else
> + ret = -ENOTTY;
> +
> + vfio_group_try_dissolve_container(group);
> +
> +out:
> + vfio_group_put(group);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(vfio_iommu_dev_fault_handler_register_nested);
> +
> +int vfio_iommu_dev_fault_handler_unregister_nested(struct device *dev)
> +{
> + struct vfio_container *container;
> + struct vfio_group *group;
> + struct vfio_iommu_driver *driver;
> + int ret;
> +
> + if (!dev)
> + return -EINVAL;
> +
> + group = vfio_group_get_from_dev(dev);
> + if (!group)
> + return -ENODEV;
> +
> + ret = vfio_group_add_container_user(group);
> + if (ret)
> + goto out;
> +
> + container = group->container;
> + driver = container->iommu_driver;
> + if (likely(driver && driver->ops->unregister_handler))
> + ret = driver->ops->unregister_handler(container->iommu_data, 
> dev);
> + else
> + ret = -ENOTTY;
> +
> + vfio_group_try_dissolve_container(group);
> +
> +out:
> + vfio_group_put(group);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(vfio_iommu_dev_fault_handler_unregister_nested);
> +
> +int vfio_transfer_iommu_fault(struct device *dev, struct iommu_fault 
> *fault)
> +{
> + struct vfio_device *device = dev_get_drvdata(dev);
> +
> + if (unlikely(!device->ops->transfer))
> + return -EOPNOTSUPP;
> +
> + return device->ops->transfer(device->device_data, fault);
> +}
> +EXPORT_SYMBOL_GPL(vfio_transfer_iommu_fault);
> +
>  /**
>   * Module/class support
>   */
> diff --git a/drivers/vfio/vfio_iommu_type1.c 
> b/drivers/vfio/vfio_iommu_type1.c
> index ba2b5a1cf6e9..9d1adeddb303 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -3821,13 +3821,32 @@ static int vfio_iommu_type1_dma_map_iopf(struct 
> iommu_fault *fault, void *data)
>   struct vfio_batch batch;
>   struct vfio_range *range;
>   dma_addr_t iova = ALIGN_DOWN(fault->prm.addr, PAGE_SIZE);
> - int access_flags = 0;
> + int access_flags = 0, nested;
>   size_t premap_len, map_len, mapped_len = 0;
>   unsigned long bit_offset, vaddr, pfn, i

Re: [RFC PATCH v3 8/8] vfio: Add nested IOPF support

2021-05-24 Thread Alex Williamson
On Mon, 24 May 2021 21:11:11 +0800
Shenming Lu  wrote:

> On 2021/5/21 15:59, Shenming Lu wrote:
> > On 2021/5/19 2:58, Alex Williamson wrote:  
> >> On Fri, 9 Apr 2021 11:44:20 +0800
> >> Shenming Lu  wrote:
> >>  
> >>> To set up nested mode, drivers such as vfio_pci need to register a
> >>> handler to receive stage/level 1 faults from the IOMMU, but since
> >>> currently each device can only have one iommu dev fault handler,
> >>> and if stage 2 IOPF is already enabled (VFIO_IOMMU_ENABLE_IOPF),
> >>> we choose to update the registered handler (a consolidated one) via
> >>> flags (set FAULT_REPORT_NESTED_L1), and further deliver the received
> >>> stage 1 faults in the handler to the guest through a newly added
> >>> vfio_device_ops callback.  
> >>
> >> Are there proposed in-kernel drivers that would use any of these
> >> symbols?  
> > 
> > I hope that such as Eric's SMMUv3 Nested Stage Setup series [1] can
> > use these symbols to consolidate the two page fault handlers into one.
> > 
> > [1] 
> > https://patchwork.kernel.org/project/kvm/cover/2021044659.15051-1-eric.au...@redhat.com/
> >   
> >>  
> >>> Signed-off-by: Shenming Lu 
> >>> ---
> >>>  drivers/vfio/vfio.c | 81 +
> >>>  drivers/vfio/vfio_iommu_type1.c | 49 +++-
> >>>  include/linux/vfio.h| 12 +
> >>>  3 files changed, 141 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> >>> index 44c8dfabf7de..4245f15914bf 100644
> >>> --- a/drivers/vfio/vfio.c
> >>> +++ b/drivers/vfio/vfio.c
> >>> @@ -2356,6 +2356,87 @@ struct iommu_domain 
> >>> *vfio_group_iommu_domain(struct vfio_group *group)
> >>>  }
> >>>  EXPORT_SYMBOL_GPL(vfio_group_iommu_domain);
> >>>  
> >>> +/*
> >>> + * Register/Update the VFIO IOPF handler to receive
> >>> + * nested stage/level 1 faults.
> >>> + */
> >>> +int vfio_iommu_dev_fault_handler_register_nested(struct device *dev)
> >>> +{
> >>> + struct vfio_container *container;
> >>> + struct vfio_group *group;
> >>> + struct vfio_iommu_driver *driver;
> >>> + int ret;
> >>> +
> >>> + if (!dev)
> >>> + return -EINVAL;
> >>> +
> >>> + group = vfio_group_get_from_dev(dev);
> >>> + if (!group)
> >>> + return -ENODEV;
> >>> +
> >>> + ret = vfio_group_add_container_user(group);
> >>> + if (ret)
> >>> + goto out;
> >>> +
> >>> + container = group->container;
> >>> + driver = container->iommu_driver;
> >>> + if (likely(driver && driver->ops->register_handler))
> >>> + ret = driver->ops->register_handler(container->iommu_data, dev);
> >>> + else
> >>> + ret = -ENOTTY;
> >>> +
> >>> + vfio_group_try_dissolve_container(group);
> >>> +
> >>> +out:
> >>> + vfio_group_put(group);
> >>> + return ret;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(vfio_iommu_dev_fault_handler_register_nested);
> >>> +
> >>> +int vfio_iommu_dev_fault_handler_unregister_nested(struct device *dev)
> >>> +{
> >>> + struct vfio_container *container;
> >>> + struct vfio_group *group;
> >>> + struct vfio_iommu_driver *driver;
> >>> + int ret;
> >>> +
> >>> + if (!dev)
> >>> + return -EINVAL;
> >>> +
> >>> + group = vfio_group_get_from_dev(dev);
> >>> + if (!group)
> >>> + return -ENODEV;
> >>> +
> >>> + ret = vfio_group_add_container_user(group);
> >>> + if (ret)
> >>> + goto out;
> >>> +
> >>> + container = group->container;
> >>> + driver = container->iommu_driver;
> >>> + if (likely(driver && driver->ops->unregister_handler))
> >>> + ret = driver->ops->unregister_handler(container->iommu_data, 
> >>> dev);
> >>> + else
> >>> + ret = -ENOTTY;
> >>> +
> >>> + vfio_group_try_dissolve_container(group);
> >>> +
> >>> +out:
> >>> + vfio_group_put(group);
> >>> + return ret;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(vfio_iommu_dev_fault_handler_unregister_nested);
> >>> +
> >>> +int vfio_transfer_iommu_fault(struct device *dev, struct iommu_fault 
> >>> *fault)
> >>> +{
> >>> + struct vfio_device *device = dev_get_drvdata(dev);
> >>> +
> >>> + if (unlikely(!device->ops->transfer))
> >>> + return -EOPNOTSUPP;
> >>> +
> >>> + return device->ops->transfer(device->device_data, fault);
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(vfio_transfer_iommu_fault);
> >>> +
> >>>  /**
> >>>   * Module/class support
> >>>   */
> >>> diff --git a/drivers/vfio/vfio_iommu_type1.c 
> >>> b/drivers/vfio/vfio_iommu_type1.c
> >>> index ba2b5a1cf6e9..9d1adeddb303 100644
> >>> --- a/drivers/vfio/vfio_iommu_type1.c
> >>> +++ b/drivers/vfio/vfio_iommu_type1.c
> >>> @@ -3821,13 +3821,32 @@ static int vfio_iommu_type1_dma_map_iopf(struct 
> >>> iommu_fault *fault, void *data)
> >>>   struct vfio_batch batch;
> >>>   struct vfio_range *range;
> >>>   dma_addr_t iova = ALIGN_DOWN(fault->prm.addr, PAGE_SIZE);
> >>> - int access_flags = 0;
> >>> + int access_flags = 0, nested;
> >>>   size_t premap_len, map_len, mapped_len = 0;
> >>>   unsigned long bit_offset, vaddr, pfn, i, npages;
> >>>   int ret;
> >>>   enum iommu_p

Re: [RFC PATCH v3 8/8] vfio: Add nested IOPF support

2021-05-24 Thread Shenming Lu
On 2021/5/21 15:59, Shenming Lu wrote:
> On 2021/5/19 2:58, Alex Williamson wrote:
>> On Fri, 9 Apr 2021 11:44:20 +0800
>> Shenming Lu  wrote:
>>
>>> To set up nested mode, drivers such as vfio_pci need to register a
>>> handler to receive stage/level 1 faults from the IOMMU, but since
>>> currently each device can only have one iommu dev fault handler,
>>> and if stage 2 IOPF is already enabled (VFIO_IOMMU_ENABLE_IOPF),
>>> we choose to update the registered handler (a consolidated one) via
>>> flags (set FAULT_REPORT_NESTED_L1), and further deliver the received
>>> stage 1 faults in the handler to the guest through a newly added
>>> vfio_device_ops callback.
>>
>> Are there proposed in-kernel drivers that would use any of these
>> symbols?
> 
> I hope that such as Eric's SMMUv3 Nested Stage Setup series [1] can
> use these symbols to consolidate the two page fault handlers into one.
> 
> [1] 
> https://patchwork.kernel.org/project/kvm/cover/2021044659.15051-1-eric.au...@redhat.com/
> 
>>
>>> Signed-off-by: Shenming Lu 
>>> ---
>>>  drivers/vfio/vfio.c | 81 +
>>>  drivers/vfio/vfio_iommu_type1.c | 49 +++-
>>>  include/linux/vfio.h| 12 +
>>>  3 files changed, 141 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
>>> index 44c8dfabf7de..4245f15914bf 100644
>>> --- a/drivers/vfio/vfio.c
>>> +++ b/drivers/vfio/vfio.c
>>> @@ -2356,6 +2356,87 @@ struct iommu_domain *vfio_group_iommu_domain(struct 
>>> vfio_group *group)
>>>  }
>>>  EXPORT_SYMBOL_GPL(vfio_group_iommu_domain);
>>>  
>>> +/*
>>> + * Register/Update the VFIO IOPF handler to receive
>>> + * nested stage/level 1 faults.
>>> + */
>>> +int vfio_iommu_dev_fault_handler_register_nested(struct device *dev)
>>> +{
>>> +   struct vfio_container *container;
>>> +   struct vfio_group *group;
>>> +   struct vfio_iommu_driver *driver;
>>> +   int ret;
>>> +
>>> +   if (!dev)
>>> +   return -EINVAL;
>>> +
>>> +   group = vfio_group_get_from_dev(dev);
>>> +   if (!group)
>>> +   return -ENODEV;
>>> +
>>> +   ret = vfio_group_add_container_user(group);
>>> +   if (ret)
>>> +   goto out;
>>> +
>>> +   container = group->container;
>>> +   driver = container->iommu_driver;
>>> +   if (likely(driver && driver->ops->register_handler))
>>> +   ret = driver->ops->register_handler(container->iommu_data, dev);
>>> +   else
>>> +   ret = -ENOTTY;
>>> +
>>> +   vfio_group_try_dissolve_container(group);
>>> +
>>> +out:
>>> +   vfio_group_put(group);
>>> +   return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(vfio_iommu_dev_fault_handler_register_nested);
>>> +
>>> +int vfio_iommu_dev_fault_handler_unregister_nested(struct device *dev)
>>> +{
>>> +   struct vfio_container *container;
>>> +   struct vfio_group *group;
>>> +   struct vfio_iommu_driver *driver;
>>> +   int ret;
>>> +
>>> +   if (!dev)
>>> +   return -EINVAL;
>>> +
>>> +   group = vfio_group_get_from_dev(dev);
>>> +   if (!group)
>>> +   return -ENODEV;
>>> +
>>> +   ret = vfio_group_add_container_user(group);
>>> +   if (ret)
>>> +   goto out;
>>> +
>>> +   container = group->container;
>>> +   driver = container->iommu_driver;
>>> +   if (likely(driver && driver->ops->unregister_handler))
>>> +   ret = driver->ops->unregister_handler(container->iommu_data, 
>>> dev);
>>> +   else
>>> +   ret = -ENOTTY;
>>> +
>>> +   vfio_group_try_dissolve_container(group);
>>> +
>>> +out:
>>> +   vfio_group_put(group);
>>> +   return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(vfio_iommu_dev_fault_handler_unregister_nested);
>>> +
>>> +int vfio_transfer_iommu_fault(struct device *dev, struct iommu_fault 
>>> *fault)
>>> +{
>>> +   struct vfio_device *device = dev_get_drvdata(dev);
>>> +
>>> +   if (unlikely(!device->ops->transfer))
>>> +   return -EOPNOTSUPP;
>>> +
>>> +   return device->ops->transfer(device->device_data, fault);
>>> +}
>>> +EXPORT_SYMBOL_GPL(vfio_transfer_iommu_fault);
>>> +
>>>  /**
>>>   * Module/class support
>>>   */
>>> diff --git a/drivers/vfio/vfio_iommu_type1.c 
>>> b/drivers/vfio/vfio_iommu_type1.c
>>> index ba2b5a1cf6e9..9d1adeddb303 100644
>>> --- a/drivers/vfio/vfio_iommu_type1.c
>>> +++ b/drivers/vfio/vfio_iommu_type1.c
>>> @@ -3821,13 +3821,32 @@ static int vfio_iommu_type1_dma_map_iopf(struct 
>>> iommu_fault *fault, void *data)
>>> struct vfio_batch batch;
>>> struct vfio_range *range;
>>> dma_addr_t iova = ALIGN_DOWN(fault->prm.addr, PAGE_SIZE);
>>> -   int access_flags = 0;
>>> +   int access_flags = 0, nested;
>>> size_t premap_len, map_len, mapped_len = 0;
>>> unsigned long bit_offset, vaddr, pfn, i, npages;
>>> int ret;
>>> enum iommu_page_response_code status = IOMMU_PAGE_RESP_INVALID;
>>> struct iommu_page_response resp = {0};
>>>  
>>> +   if (vfio_dev_domian_nested(dev, &nested))
>>> +   return -ENODEV;
>>> +
>>> +   /*
>>> +* When configured in nested m

Re: [RFC PATCH v3 8/8] vfio: Add nested IOPF support

2021-05-21 Thread Shenming Lu
On 2021/5/19 2:58, Alex Williamson wrote:
> On Fri, 9 Apr 2021 11:44:20 +0800
> Shenming Lu  wrote:
> 
>> To set up nested mode, drivers such as vfio_pci need to register a
>> handler to receive stage/level 1 faults from the IOMMU, but since
>> currently each device can only have one iommu dev fault handler,
>> and if stage 2 IOPF is already enabled (VFIO_IOMMU_ENABLE_IOPF),
>> we choose to update the registered handler (a consolidated one) via
>> flags (set FAULT_REPORT_NESTED_L1), and further deliver the received
>> stage 1 faults in the handler to the guest through a newly added
>> vfio_device_ops callback.
> 
> Are there proposed in-kernel drivers that would use any of these
> symbols?

I hope that such as Eric's SMMUv3 Nested Stage Setup series [1] can
use these symbols to consolidate the two page fault handlers into one.

[1] 
https://patchwork.kernel.org/project/kvm/cover/2021044659.15051-1-eric.au...@redhat.com/

> 
>> Signed-off-by: Shenming Lu 
>> ---
>>  drivers/vfio/vfio.c | 81 +
>>  drivers/vfio/vfio_iommu_type1.c | 49 +++-
>>  include/linux/vfio.h| 12 +
>>  3 files changed, 141 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
>> index 44c8dfabf7de..4245f15914bf 100644
>> --- a/drivers/vfio/vfio.c
>> +++ b/drivers/vfio/vfio.c
>> @@ -2356,6 +2356,87 @@ struct iommu_domain *vfio_group_iommu_domain(struct 
>> vfio_group *group)
>>  }
>>  EXPORT_SYMBOL_GPL(vfio_group_iommu_domain);
>>  
>> +/*
>> + * Register/Update the VFIO IOPF handler to receive
>> + * nested stage/level 1 faults.
>> + */
>> +int vfio_iommu_dev_fault_handler_register_nested(struct device *dev)
>> +{
>> +struct vfio_container *container;
>> +struct vfio_group *group;
>> +struct vfio_iommu_driver *driver;
>> +int ret;
>> +
>> +if (!dev)
>> +return -EINVAL;
>> +
>> +group = vfio_group_get_from_dev(dev);
>> +if (!group)
>> +return -ENODEV;
>> +
>> +ret = vfio_group_add_container_user(group);
>> +if (ret)
>> +goto out;
>> +
>> +container = group->container;
>> +driver = container->iommu_driver;
>> +if (likely(driver && driver->ops->register_handler))
>> +ret = driver->ops->register_handler(container->iommu_data, dev);
>> +else
>> +ret = -ENOTTY;
>> +
>> +vfio_group_try_dissolve_container(group);
>> +
>> +out:
>> +vfio_group_put(group);
>> +return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(vfio_iommu_dev_fault_handler_register_nested);
>> +
>> +int vfio_iommu_dev_fault_handler_unregister_nested(struct device *dev)
>> +{
>> +struct vfio_container *container;
>> +struct vfio_group *group;
>> +struct vfio_iommu_driver *driver;
>> +int ret;
>> +
>> +if (!dev)
>> +return -EINVAL;
>> +
>> +group = vfio_group_get_from_dev(dev);
>> +if (!group)
>> +return -ENODEV;
>> +
>> +ret = vfio_group_add_container_user(group);
>> +if (ret)
>> +goto out;
>> +
>> +container = group->container;
>> +driver = container->iommu_driver;
>> +if (likely(driver && driver->ops->unregister_handler))
>> +ret = driver->ops->unregister_handler(container->iommu_data, 
>> dev);
>> +else
>> +ret = -ENOTTY;
>> +
>> +vfio_group_try_dissolve_container(group);
>> +
>> +out:
>> +vfio_group_put(group);
>> +return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(vfio_iommu_dev_fault_handler_unregister_nested);
>> +
>> +int vfio_transfer_iommu_fault(struct device *dev, struct iommu_fault *fault)
>> +{
>> +struct vfio_device *device = dev_get_drvdata(dev);
>> +
>> +if (unlikely(!device->ops->transfer))
>> +return -EOPNOTSUPP;
>> +
>> +return device->ops->transfer(device->device_data, fault);
>> +}
>> +EXPORT_SYMBOL_GPL(vfio_transfer_iommu_fault);
>> +
>>  /**
>>   * Module/class support
>>   */
>> diff --git a/drivers/vfio/vfio_iommu_type1.c 
>> b/drivers/vfio/vfio_iommu_type1.c
>> index ba2b5a1cf6e9..9d1adeddb303 100644
>> --- a/drivers/vfio/vfio_iommu_type1.c
>> +++ b/drivers/vfio/vfio_iommu_type1.c
>> @@ -3821,13 +3821,32 @@ static int vfio_iommu_type1_dma_map_iopf(struct 
>> iommu_fault *fault, void *data)
>>  struct vfio_batch batch;
>>  struct vfio_range *range;
>>  dma_addr_t iova = ALIGN_DOWN(fault->prm.addr, PAGE_SIZE);
>> -int access_flags = 0;
>> +int access_flags = 0, nested;
>>  size_t premap_len, map_len, mapped_len = 0;
>>  unsigned long bit_offset, vaddr, pfn, i, npages;
>>  int ret;
>>  enum iommu_page_response_code status = IOMMU_PAGE_RESP_INVALID;
>>  struct iommu_page_response resp = {0};
>>  
>> +if (vfio_dev_domian_nested(dev, &nested))
>> +return -ENODEV;
>> +
>> +/*
>> + * When configured in nested mode, further deliver the
>> + * stage/level 1 faults to the guest.
>> + */
>> +if (nested) {
>> +bool l2;
>> +
>

Re: [RFC PATCH v3 8/8] vfio: Add nested IOPF support

2021-05-18 Thread Alex Williamson
On Fri, 9 Apr 2021 11:44:20 +0800
Shenming Lu  wrote:

> To set up nested mode, drivers such as vfio_pci need to register a
> handler to receive stage/level 1 faults from the IOMMU, but since
> currently each device can only have one iommu dev fault handler,
> and if stage 2 IOPF is already enabled (VFIO_IOMMU_ENABLE_IOPF),
> we choose to update the registered handler (a consolidated one) via
> flags (set FAULT_REPORT_NESTED_L1), and further deliver the received
> stage 1 faults in the handler to the guest through a newly added
> vfio_device_ops callback.

Are there proposed in-kernel drivers that would use any of these
symbols?

> Signed-off-by: Shenming Lu 
> ---
>  drivers/vfio/vfio.c | 81 +
>  drivers/vfio/vfio_iommu_type1.c | 49 +++-
>  include/linux/vfio.h| 12 +
>  3 files changed, 141 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 44c8dfabf7de..4245f15914bf 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -2356,6 +2356,87 @@ struct iommu_domain *vfio_group_iommu_domain(struct 
> vfio_group *group)
>  }
>  EXPORT_SYMBOL_GPL(vfio_group_iommu_domain);
>  
> +/*
> + * Register/Update the VFIO IOPF handler to receive
> + * nested stage/level 1 faults.
> + */
> +int vfio_iommu_dev_fault_handler_register_nested(struct device *dev)
> +{
> + struct vfio_container *container;
> + struct vfio_group *group;
> + struct vfio_iommu_driver *driver;
> + int ret;
> +
> + if (!dev)
> + return -EINVAL;
> +
> + group = vfio_group_get_from_dev(dev);
> + if (!group)
> + return -ENODEV;
> +
> + ret = vfio_group_add_container_user(group);
> + if (ret)
> + goto out;
> +
> + container = group->container;
> + driver = container->iommu_driver;
> + if (likely(driver && driver->ops->register_handler))
> + ret = driver->ops->register_handler(container->iommu_data, dev);
> + else
> + ret = -ENOTTY;
> +
> + vfio_group_try_dissolve_container(group);
> +
> +out:
> + vfio_group_put(group);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(vfio_iommu_dev_fault_handler_register_nested);
> +
> +int vfio_iommu_dev_fault_handler_unregister_nested(struct device *dev)
> +{
> + struct vfio_container *container;
> + struct vfio_group *group;
> + struct vfio_iommu_driver *driver;
> + int ret;
> +
> + if (!dev)
> + return -EINVAL;
> +
> + group = vfio_group_get_from_dev(dev);
> + if (!group)
> + return -ENODEV;
> +
> + ret = vfio_group_add_container_user(group);
> + if (ret)
> + goto out;
> +
> + container = group->container;
> + driver = container->iommu_driver;
> + if (likely(driver && driver->ops->unregister_handler))
> + ret = driver->ops->unregister_handler(container->iommu_data, 
> dev);
> + else
> + ret = -ENOTTY;
> +
> + vfio_group_try_dissolve_container(group);
> +
> +out:
> + vfio_group_put(group);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(vfio_iommu_dev_fault_handler_unregister_nested);
> +
> +int vfio_transfer_iommu_fault(struct device *dev, struct iommu_fault *fault)
> +{
> + struct vfio_device *device = dev_get_drvdata(dev);
> +
> + if (unlikely(!device->ops->transfer))
> + return -EOPNOTSUPP;
> +
> + return device->ops->transfer(device->device_data, fault);
> +}
> +EXPORT_SYMBOL_GPL(vfio_transfer_iommu_fault);
> +
>  /**
>   * Module/class support
>   */
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index ba2b5a1cf6e9..9d1adeddb303 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -3821,13 +3821,32 @@ static int vfio_iommu_type1_dma_map_iopf(struct 
> iommu_fault *fault, void *data)
>   struct vfio_batch batch;
>   struct vfio_range *range;
>   dma_addr_t iova = ALIGN_DOWN(fault->prm.addr, PAGE_SIZE);
> - int access_flags = 0;
> + int access_flags = 0, nested;
>   size_t premap_len, map_len, mapped_len = 0;
>   unsigned long bit_offset, vaddr, pfn, i, npages;
>   int ret;
>   enum iommu_page_response_code status = IOMMU_PAGE_RESP_INVALID;
>   struct iommu_page_response resp = {0};
>  
> + if (vfio_dev_domian_nested(dev, &nested))
> + return -ENODEV;
> +
> + /*
> +  * When configured in nested mode, further deliver the
> +  * stage/level 1 faults to the guest.
> +  */
> + if (nested) {
> + bool l2;
> +
> + if (fault->type == IOMMU_FAULT_PAGE_REQ)
> + l2 = fault->prm.flags & IOMMU_FAULT_PAGE_REQUEST_L2;
> + if (fault->type == IOMMU_FAULT_DMA_UNRECOV)
> + l2 = fault->event.flags & IOMMU_FAULT_UNRECOV_L2;
> +
> + if (!l2)
> + return vfio_transfer_iommu_fault(dev, fault);
> +  

[RFC PATCH v3 8/8] vfio: Add nested IOPF support

2021-04-08 Thread Shenming Lu
To set up nested mode, drivers such as vfio_pci need to register a
handler to receive stage/level 1 faults from the IOMMU, but since
currently each device can only have one iommu dev fault handler,
and if stage 2 IOPF is already enabled (VFIO_IOMMU_ENABLE_IOPF),
we choose to update the registered handler (a consolidated one) via
flags (set FAULT_REPORT_NESTED_L1), and further deliver the received
stage 1 faults in the handler to the guest through a newly added
vfio_device_ops callback.

Signed-off-by: Shenming Lu 
---
 drivers/vfio/vfio.c | 81 +
 drivers/vfio/vfio_iommu_type1.c | 49 +++-
 include/linux/vfio.h| 12 +
 3 files changed, 141 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 44c8dfabf7de..4245f15914bf 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -2356,6 +2356,87 @@ struct iommu_domain *vfio_group_iommu_domain(struct 
vfio_group *group)
 }
 EXPORT_SYMBOL_GPL(vfio_group_iommu_domain);
 
+/*
+ * Register/Update the VFIO IOPF handler to receive
+ * nested stage/level 1 faults.
+ */
+int vfio_iommu_dev_fault_handler_register_nested(struct device *dev)
+{
+   struct vfio_container *container;
+   struct vfio_group *group;
+   struct vfio_iommu_driver *driver;
+   int ret;
+
+   if (!dev)
+   return -EINVAL;
+
+   group = vfio_group_get_from_dev(dev);
+   if (!group)
+   return -ENODEV;
+
+   ret = vfio_group_add_container_user(group);
+   if (ret)
+   goto out;
+
+   container = group->container;
+   driver = container->iommu_driver;
+   if (likely(driver && driver->ops->register_handler))
+   ret = driver->ops->register_handler(container->iommu_data, dev);
+   else
+   ret = -ENOTTY;
+
+   vfio_group_try_dissolve_container(group);
+
+out:
+   vfio_group_put(group);
+   return ret;
+}
+EXPORT_SYMBOL_GPL(vfio_iommu_dev_fault_handler_register_nested);
+
+int vfio_iommu_dev_fault_handler_unregister_nested(struct device *dev)
+{
+   struct vfio_container *container;
+   struct vfio_group *group;
+   struct vfio_iommu_driver *driver;
+   int ret;
+
+   if (!dev)
+   return -EINVAL;
+
+   group = vfio_group_get_from_dev(dev);
+   if (!group)
+   return -ENODEV;
+
+   ret = vfio_group_add_container_user(group);
+   if (ret)
+   goto out;
+
+   container = group->container;
+   driver = container->iommu_driver;
+   if (likely(driver && driver->ops->unregister_handler))
+   ret = driver->ops->unregister_handler(container->iommu_data, 
dev);
+   else
+   ret = -ENOTTY;
+
+   vfio_group_try_dissolve_container(group);
+
+out:
+   vfio_group_put(group);
+   return ret;
+}
+EXPORT_SYMBOL_GPL(vfio_iommu_dev_fault_handler_unregister_nested);
+
+int vfio_transfer_iommu_fault(struct device *dev, struct iommu_fault *fault)
+{
+   struct vfio_device *device = dev_get_drvdata(dev);
+
+   if (unlikely(!device->ops->transfer))
+   return -EOPNOTSUPP;
+
+   return device->ops->transfer(device->device_data, fault);
+}
+EXPORT_SYMBOL_GPL(vfio_transfer_iommu_fault);
+
 /**
  * Module/class support
  */
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index ba2b5a1cf6e9..9d1adeddb303 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -3821,13 +3821,32 @@ static int vfio_iommu_type1_dma_map_iopf(struct 
iommu_fault *fault, void *data)
struct vfio_batch batch;
struct vfio_range *range;
dma_addr_t iova = ALIGN_DOWN(fault->prm.addr, PAGE_SIZE);
-   int access_flags = 0;
+   int access_flags = 0, nested;
size_t premap_len, map_len, mapped_len = 0;
unsigned long bit_offset, vaddr, pfn, i, npages;
int ret;
enum iommu_page_response_code status = IOMMU_PAGE_RESP_INVALID;
struct iommu_page_response resp = {0};
 
+   if (vfio_dev_domian_nested(dev, &nested))
+   return -ENODEV;
+
+   /*
+* When configured in nested mode, further deliver the
+* stage/level 1 faults to the guest.
+*/
+   if (nested) {
+   bool l2;
+
+   if (fault->type == IOMMU_FAULT_PAGE_REQ)
+   l2 = fault->prm.flags & IOMMU_FAULT_PAGE_REQUEST_L2;
+   if (fault->type == IOMMU_FAULT_DMA_UNRECOV)
+   l2 = fault->event.flags & IOMMU_FAULT_UNRECOV_L2;
+
+   if (!l2)
+   return vfio_transfer_iommu_fault(dev, fault);
+   }
+
if (fault->type != IOMMU_FAULT_PAGE_REQ)
return -EOPNOTSUPP;
 
@@ -4201,6 +4220,32 @@ static void vfio_iommu_type1_notify(void *iommu_data,
wake_up_all(&iommu->vaddr_wait);
 }
 
+static int vfio_iommu_type1_register_handler(void *iommu_d