Re: [PATCH iommufd 2/9] vfio/type1: Check that every device supports IOMMU_CAP_INTR_REMAP

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu,  8 Dec 2022 16:26:29 -0400
Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:

> iommu_group_for_each_dev() exits the loop at the first callback that
> returns 1 - thus returning 1 fails to check the rest of the devices in the
> group.
> 
> msi_remap (aka secure MSI) requires that all the devices in the group
> support it, not just any one. This is only a theoretical problem as no
> current drivers will have different secure MSI properties within a group.

Which is exactly how Robin justified the behavior in the referenced
commit:

  As with domains, any capability must in practice be consistent for
  devices in a given group - and after all it's still the same
  capability which was expected to be consistent across an entire bus!
  - so there's no need for any complicated validation.

That suggests to me that it's intentional that we break if any device
supports the capability and therefore this isn't so much a "Fixes:", as
it is a refactoring expressly to support msi_device_has_secure_msi(),
which cannot make these sort of assumptions as a non-group API.  Thanks,

Alex

> Make vfio_iommu_device_secure_msi() reduce AND across all the devices.
> 
> Fixes: eed20c782aea ("vfio/type1: Simplify bus_type determination")
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> ---
>  drivers/vfio/vfio_iommu_type1.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 23c24fe98c00d4..3025b4e643c135 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -2160,10 +2160,12 @@ static void vfio_iommu_iova_insert_copy(struct vfio_iommu *iommu,
>  	list_splice_tail(iova_copy, iova);
>  }
>  
> -/* Redundantly walks non-present capabilities to simplify caller */
> -static int vfio_iommu_device_capable(struct device *dev, void *data)
> +static int vfio_iommu_device_secure_msi(struct device *dev, void *data)
>  {
> -	return device_iommu_capable(dev, (enum iommu_cap)data);
> +	bool *secure_msi = data;
> +
> +	*secure_msi &= device_iommu_capable(dev, IOMMU_CAP_INTR_REMAP);
> +	return 0;
>  }
>  
>  static int vfio_iommu_domain_alloc(struct device *dev, void *data)
> @@ -2278,9 +2280,12 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
>  	INIT_LIST_HEAD(&domain->group_list);
>  	list_add(&group->next, &domain->group_list);
>  
> -	msi_remap = irq_domain_check_msi_remap() ||
> -		    iommu_group_for_each_dev(iommu_group, (void *)IOMMU_CAP_INTR_REMAP,
> -					     vfio_iommu_device_capable);
> +	msi_remap = irq_domain_check_msi_remap();
> +	if (!msi_remap) {
> +		msi_remap = true;
> +		iommu_group_for_each_dev(iommu_group, &msi_remap,
> +					 vfio_iommu_device_secure_msi);
> +	}
>  
>  	if (!allow_unsafe_interrupts && !msi_remap) {
>  		pr_warn("%s: No interrupt remapping support.  Use the module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this platform\n",




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux