Re: [PATCH v2 01/17] iommufd: Move isolated msi enforcement to iommufd_device_bind()

[Baolu Lu <baolu.lu@linux.intel.com>]

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