| Message ID | 20240912131729.14951-5-yi.l.liu@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | vfio-pci support pasid attach/detach | expand |
On Thu, Sep 12, 2024 at 06:17:29AM -0700, Yi Liu wrote: > @@ -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); Don't we do this twice now? Let's just keep it in the pci core? It looks Ok otherwise Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Jason
On 2024/9/27 03:35, Jason Gunthorpe wrote: > On Thu, Sep 12, 2024 at 06:17:29AM -0700, Yi Liu wrote: >> @@ -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); > > Don't we do this twice now? > > Let's just keep it in the pci core? yes. let me drop it. > It looks Ok otherwise > > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> thanks.
> From: Liu, Yi L <yi.l.liu@intel.com> > Sent: Thursday, September 12, 2024 9:17 PM > > 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> Reviewed-by: Kevin Tian <kevin.tian@intel.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
On Thu, 12 Sept 2024 at 21:18, Yi Liu <yi.l.liu@intel.com> 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> Thanks Yi Have verified on aarch64 platform. Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
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. + * 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; + int pasid; + + if (pdev->is_virtfn) + pdev = pci_physfn(pdev); + + pasid = pdev->pasid_cap; + if (!pasid) + return -EINVAL; + + pci_read_config_word(pdev, pasid + PCI_PASID_CTRL, &ctrl); + + ctrl &= PCI_PASID_CTRL_ENABLE | PCI_PASID_CTRL_EXEC | + PCI_PASID_CTRL_PRIV; + + return ctrl; +} #endif /* CONFIG_PCI_PASID */ diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h index df54cd5b15db..5cee388752a0 100644 --- a/include/linux/pci-ats.h +++ b/include/linux/pci-ats.h @@ -39,6 +39,7 @@ int pci_enable_pasid(struct pci_dev *pdev, int features); void pci_disable_pasid(struct pci_dev *pdev); int pci_pasid_features(struct pci_dev *pdev); int pci_max_pasids(struct pci_dev *pdev); +int pci_pasid_ctrl_status(struct pci_dev *pdev); #else /* CONFIG_PCI_PASID */ static inline int pci_enable_pasid(struct pci_dev *pdev, int features) { return -EINVAL; } @@ -47,6 +48,8 @@ static inline int pci_pasid_features(struct pci_dev *pdev) { return -EINVAL; } static inline int pci_max_pasids(struct pci_dev *pdev) { return -EINVAL; } +static inline int pci_pasid_ctrl_status(struct pci_dev *pdev) +{ return -EINVAL; } #endif /* CONFIG_PCI_PASID */ #endif /* LINUX_PCI_ATS_H */ diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h index 4dde745cfb7e..60eca4c73b25 100644 --- a/include/uapi/linux/iommufd.h +++ b/include/uapi/linux/iommufd.h @@ -504,9 +504,17 @@ enum iommu_hw_info_type { * IOMMU_HWPT_GET_DIRTY_BITMAP * IOMMU_HWPT_SET_DIRTY_TRACKING * + * @IOMMU_HW_CAP_PASID_EXEC: Execute Permission Supported, user ignores it + * when the struct iommu_hw_info::out_max_pasid_log2 + * is zero. + * @IOMMU_HW_CAP_PASID_PRIV: Privileged Mode Supported, user ignores it + * when the struct iommu_hw_info::out_max_pasid_log2 + * is zero. */ enum iommufd_hw_capabilities { IOMMU_HW_CAP_DIRTY_TRACKING = 1 << 0, + IOMMU_HW_CAP_PCI_PASID_EXEC = 1 << 1, + IOMMU_HW_CAP_PCI_PASID_PRIV = 1 << 2, }; /** @@ -522,6 +530,9 @@ enum iommufd_hw_capabilities { * iommu_hw_info_type. * @out_capabilities: Output the generic iommu capability info type as defined * in the enum iommu_hw_capabilities. + * @out_max_pasid_log2: Output the width of PASIDs. 0 means no PASID support. + * PCI devices turn to out_capabilities to check if the + * specific capabilities is supported or not. * @__reserved: Must be 0 * * Query an iommu type specific hardware information data from an iommu behind @@ -545,7 +556,8 @@ struct iommu_hw_info { __u32 data_len; __aligned_u64 data_uptr; __u32 out_data_type; - __u32 __reserved; + __u8 out_max_pasid_log2; + __u8 __reserved[3]; __aligned_u64 out_capabilities; }; #define IOMMU_GET_HW_INFO _IO(IOMMUFD_TYPE, IOMMUFD_CMD_GET_HW_INFO)
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(-)