Re: [PATCH v3 4/4] iommufd: Extend IOMMU_GET_HW_INFO to report PASID capability

[Vasant Hegde <vasant.hegde@amd.com>]

Hi Yi,


On 9/12/2024 6:47 PM, Yi Liu wrote:
> PASID usage requires PASID support in both device and IOMMU. Since the
> iommu drivers always enable the PASID capability for the device if it
> is supported, so it is reasonable to extend the IOMMU_GET_HW_INFO to
> report the PASID capability to userspace.
> 
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
>  drivers/iommu/iommufd/device.c | 27 ++++++++++++++++++++++++++-
>  drivers/pci/ats.c              | 32 ++++++++++++++++++++++++++++++++
>  include/linux/pci-ats.h        |  3 +++
>  include/uapi/linux/iommufd.h   | 14 +++++++++++++-
>  4 files changed, 74 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> index 18f94aa462ea..6b7e3e5f4598 100644
> --- a/drivers/iommu/iommufd/device.c
> +++ b/drivers/iommu/iommufd/device.c
> @@ -4,6 +4,8 @@
>  #include <linux/iommufd.h>
>  #include <linux/slab.h>
>  #include <linux/iommu.h>
> +#include <linux/pci.h>
> +#include <linux/pci-ats.h>
>  #include <uapi/linux/iommufd.h>
>  #include "../iommu-priv.h"
>  
> @@ -1185,7 +1187,8 @@ int iommufd_get_hw_info(struct iommufd_ucmd *ucmd)
>  	void *data;
>  	int rc;
>  
> -	if (cmd->flags || cmd->__reserved)
> +	if (cmd->flags || cmd->__reserved[0] || cmd->__reserved[1] ||
> +	    cmd->__reserved[2])
>  		return -EOPNOTSUPP;
>  
>  	idev = iommufd_get_device(ucmd, cmd->dev_id);
> @@ -1242,6 +1245,28 @@ int iommufd_get_hw_info(struct iommufd_ucmd *ucmd)
>  	if (device_iommu_capable(idev->dev, IOMMU_CAP_DIRTY_TRACKING))
>  		cmd->out_capabilities |= IOMMU_HW_CAP_DIRTY_TRACKING;
>  
> +	cmd->out_max_pasid_log2 = 0;
> +
> +	if (dev_is_pci(idev->dev)) {
> +		struct pci_dev *pdev = to_pci_dev(idev->dev);
> +		int ctrl;
> +
> +		if (pdev->is_virtfn)
> +			pdev = pci_physfn(pdev);
> +
> +		ctrl = pci_pasid_ctrl_status(pdev);
> +		if (ctrl >= 0 && (ctrl & PCI_PASID_CTRL_ENABLE)) {
> +			cmd->out_max_pasid_log2 =
> +					ilog2(idev->dev->iommu->max_pasids);
> +			if (ctrl & PCI_PASID_CTRL_EXEC)
> +				cmd->out_capabilities |=
> +						IOMMU_HW_CAP_PCI_PASID_EXEC;
> +			if (ctrl & PCI_PASID_CTRL_PRIV)
> +				cmd->out_capabilities |=
> +						IOMMU_HW_CAP_PCI_PASID_PRIV;
> +		}
> +	}
> +
>  	rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
>  out_free:
>  	kfree(data);
> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> index c570892b2090..886f24e3999f 100644
> --- a/drivers/pci/ats.c
> +++ b/drivers/pci/ats.c
> @@ -505,4 +505,36 @@ int pci_max_pasids(struct pci_dev *pdev)
>  	return (1 << FIELD_GET(PCI_PASID_CAP_WIDTH, supported));
>  }
>  EXPORT_SYMBOL_GPL(pci_max_pasids);
> +
> +/**
> + * pci_pasid_ctrl_status - Check the PASID status
> + * @pdev: PCI device structure
> + *
> + * Returns a negative value when no PASI capability is present.

s/PASI/PASID/


> + * Otherwise the value of the control register is returned.
> + * Status reported are:
> + *
> + * PCI_PASID_CTRL_ENABLE - PASID enabled
> + * PCI_PASID_CTRL_EXEC - Execute permission enabled
> + * PCI_PASID_CTRL_PRIV - Privileged mode enabled
> + */
> +int pci_pasid_ctrl_status(struct pci_dev *pdev)
> +{
> +	u16 ctrl = 0;

No need to initialize ctrl.

-Vasant