| Message ID | 1-v2-51b9896e7862+8a8c-iommufd_alloc_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add iommufd physical device operations for replace and alloc hwpt | expand |
On 3/8/23 8:35 AM, Jason Gunthorpe wrote: > With the recent rework this no longer needs to be done at domain > attachment time, we know if the device is usable by iommufd when we bind > it. > > The value of msi_device_has_isolated_msi() is not allowed to change while > a driver is bound. > > Reviewed-by: Kevin Tian<kevin.tian@intel.com> > Signed-off-by: Jason Gunthorpe<jgg@nvidia.com> > --- > drivers/iommu/iommufd/device.c | 38 ++++++++++++++++++---------------- > 1 file changed, 20 insertions(+), 18 deletions(-) > > diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c > index c6f4852a8a0c08..63b65cdfe97f29 100644 > --- a/drivers/iommu/iommufd/device.c > +++ b/drivers/iommu/iommufd/device.c > @@ -60,6 +60,26 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx, > if (!group) > return ERR_PTR(-ENODEV); > > + /* > + * For historical compat with VFIO the insecure interrupt path is > + * allowed if the module parameter is set. Insecure means that a MemWr > + * operation from the device (eg a simple DMA) cannot trigger an Nit: "... cannot trigger an ..." or "... can trigger an ..."? > + * interrupt outside this iommufd context. > + */ > + if (!iommufd_selftest_is_mock_dev(dev) && > + !iommu_group_has_isolated_msi(group)) { > + if (!allow_unsafe_interrupts) { > + rc = -EPERM; > + goto out_group_put; > + } > + > + dev_warn( > + dev, > + "MSI interrupts are not secure, they cannot be isolated by the platform. " > + "Check that platform features like interrupt remapping are enabled. " > + "Use the \"allow_unsafe_interrupts\" module parameter to override\n"); > + } Anyway, Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com> Best regards, baolu
On Wed, Mar 08, 2023 at 10:55:31AM +0800, Baolu Lu wrote: > On 3/8/23 8:35 AM, Jason Gunthorpe wrote: > > With the recent rework this no longer needs to be done at domain > > attachment time, we know if the device is usable by iommufd when we bind > > it. > > > > The value of msi_device_has_isolated_msi() is not allowed to change while > > a driver is bound. > > > > Reviewed-by: Kevin Tian<kevin.tian@intel.com> > > Signed-off-by: Jason Gunthorpe<jgg@nvidia.com> > > --- > > drivers/iommu/iommufd/device.c | 38 ++++++++++++++++++---------------- > > 1 file changed, 20 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c > > index c6f4852a8a0c08..63b65cdfe97f29 100644 > > --- a/drivers/iommu/iommufd/device.c > > +++ b/drivers/iommu/iommufd/device.c > > @@ -60,6 +60,26 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx, > > if (!group) > > return ERR_PTR(-ENODEV); > > + /* > > + * For historical compat with VFIO the insecure interrupt path is > > + * allowed if the module parameter is set. Insecure means that a MemWr > > + * operation from the device (eg a simple DMA) cannot trigger an > > Nit: > > "... cannot trigger an ..." or "... can trigger an ..."? Oh, yes that got flipped at some point Thanks, Jason
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c index c6f4852a8a0c08..63b65cdfe97f29 100644 --- a/drivers/iommu/iommufd/device.c +++ b/drivers/iommu/iommufd/device.c @@ -60,6 +60,26 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx, if (!group) return ERR_PTR(-ENODEV); + /* + * For historical compat with VFIO the insecure interrupt path is + * allowed if the module parameter is set. Insecure means that a MemWr + * operation from the device (eg a simple DMA) cannot trigger an + * interrupt outside this iommufd context. + */ + if (!iommufd_selftest_is_mock_dev(dev) && + !iommu_group_has_isolated_msi(group)) { + if (!allow_unsafe_interrupts) { + rc = -EPERM; + goto out_group_put; + } + + dev_warn( + dev, + "MSI interrupts are not secure, they cannot be isolated by the platform. " + "Check that platform features like interrupt remapping are enabled. " + "Use the \"allow_unsafe_interrupts\" module parameter to override\n"); + } + rc = iommu_device_claim_dma_owner(dev, ictx); if (rc) goto out_group_put; @@ -146,24 +166,6 @@ static int iommufd_device_setup_msi(struct iommufd_device *idev, */ hwpt->msi_cookie = true; } - - /* - * For historical compat with VFIO the insecure interrupt path is - * allowed if the module parameter is set. Insecure means that a MemWr - * operation from the device (eg a simple DMA) cannot trigger an - * interrupt outside this iommufd context. - */ - if (!iommufd_selftest_is_mock_dev(idev->dev) && - !iommu_group_has_isolated_msi(idev->group)) { - if (!allow_unsafe_interrupts) - return -EPERM; - - dev_warn( - idev->dev, - "MSI interrupts are not secure, they cannot be isolated by the platform. " - "Check that platform features like interrupt remapping are enabled. " - "Use the \"allow_unsafe_interrupts\" module parameter to override\n"); - } return 0; }