| Message ID | 562f2bfae1661e6ff6abdb280faa0dd49df9fbdc.1728491453.git.nicolinc@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cover-letter: iommufd: Add vIOMMU infrastructure (Part-1) | expand |
On Wed, Oct 09, 2024 at 09:38:11AM -0700, Nicolin Chen wrote: > Add a new driver-type for ARM SMMUv3 to enum iommu_viommu_type. Implement > the viommu_alloc op with an arm_vsmmu_alloc function. As an initial step, > copy the VMID from s2_parent. A later cleanup series is required to move > the VMID allocation out of the stage-2 domain allocation routine to this. > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 18 ++++++++++++++ > include/uapi/linux/iommufd.h | 2 ++ > .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 24 +++++++++++++++++++ > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 + > 4 files changed, 45 insertions(+) I squashed the following changes to this commit (will be in v4). It replaces nested_domain->s2_parent with nested_domain->vsmmu: diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c index 5e235fca8f13..3cd8ad0a8980 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c @@ -36,14 +36,15 @@ arm_smmu_get_msi_mapping_domain(struct iommu_domain *domain) struct arm_smmu_nested_domain *nested_domain = container_of(domain, struct arm_smmu_nested_domain, domain); - return &nested_domain->s2_parent->domain; + return &nested_domain->vsmmu->s2_parent->domain; } static void arm_smmu_make_nested_cd_table_ste( struct arm_smmu_ste *target, struct arm_smmu_master *master, struct arm_smmu_nested_domain *nested_domain, bool ats_enabled) { - arm_smmu_make_s2_domain_ste(target, master, nested_domain->s2_parent, + arm_smmu_make_s2_domain_ste(target, master, + nested_domain->vsmmu->s2_parent, ats_enabled); target->data[0] = cpu_to_le64(STRTAB_STE_0_V | @@ -84,7 +85,8 @@ static void arm_smmu_make_nested_domain_ste( break; case STRTAB_STE_0_CFG_BYPASS: arm_smmu_make_s2_domain_ste( - target, master, nested_domain->s2_parent, ats_enabled); + target, master, nested_domain->vsmmu->s2_parent, + ats_enabled); break; case STRTAB_STE_0_CFG_ABORT: default: @@ -109,7 +111,7 @@ static int arm_smmu_attach_dev_nested(struct iommu_domain *domain, struct arm_smmu_ste ste; int ret; - if (nested_domain->s2_parent->smmu != master->smmu) + if (nested_domain->vsmmu->smmu != master->smmu) return -EINVAL; if (arm_smmu_ssids_in_use(&master->cd_table)) return -EBUSY; @@ -164,8 +166,10 @@ static int arm_smmu_validate_vste(struct iommu_hwpt_arm_smmuv3 *arg) struct iommu_domain * arm_smmu_domain_alloc_nesting(struct device *dev, u32 flags, struct iommu_domain *parent, + struct iommufd_viommu *viommu, const struct iommu_user_data *user_data) { + struct arm_vsmmu *vsmmu = container_of(viommu, struct arm_vsmmu, core); struct arm_smmu_master *master = dev_iommu_priv_get(dev); struct arm_smmu_nested_domain *nested_domain; struct arm_smmu_domain *smmu_parent; @@ -183,6 +187,10 @@ arm_smmu_domain_alloc_nesting(struct device *dev, u32 flags, !(master->smmu->features & ARM_SMMU_FEAT_S2FWB)) return ERR_PTR(-EOPNOTSUPP); + /* Driver only supports nesting with the vIOMMU infrastructure */ + if (!viommu) + return ERR_PTR(-EOPNOTSUPP); + /* * The core code checks that parent was created with * IOMMU_HWPT_ALLOC_NEST_PARENT @@ -206,7 +214,7 @@ arm_smmu_domain_alloc_nesting(struct device *dev, u32 flags, nested_domain->domain.type = IOMMU_DOMAIN_NESTED; nested_domain->domain.ops = &arm_smmu_nested_ops; - nested_domain->s2_parent = smmu_parent; + nested_domain->vsmmu = vsmmu; nested_domain->ste[0] = arg.ste[0]; nested_domain->ste[1] = arg.ste[1] & ~cpu_to_le64(STRTAB_STE_1_EATS); diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 6a23e6dcd5cf..460d2fe5dd49 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -2660,7 +2661,7 @@ to_smmu_domain_devices(struct iommu_domain *domain) domain->type == IOMMU_DOMAIN_SVA) return to_smmu_domain(domain); if (domain->type == IOMMU_DOMAIN_NESTED) - return to_smmu_nested_domain(domain)->s2_parent; + return to_smmu_nested_domain(domain)->vsmmu->s2_parent; return NULL; } @@ -3128,7 +3129,7 @@ arm_smmu_domain_alloc_user(struct device *dev, u32 flags, if (parent) return arm_smmu_domain_alloc_nesting(dev, flags, parent, - user_data); + viommu, user_data); if (flags & ~PAGING_FLAGS) return ERR_PTR(-EOPNOTSUPP); diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h index 844d1dfdea55..0ea1afe1bb7a 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h @@ -836,7 +836,7 @@ struct arm_smmu_domain { struct arm_smmu_nested_domain { struct iommu_domain domain; - struct arm_smmu_domain *s2_parent; + struct arm_vsmmu *vsmmu; __le64 ste[2]; }; @@ -1018,6 +1018,7 @@ void *arm_smmu_hw_info(struct device *dev, u32 *length, u32 *type); struct iommu_domain * arm_smmu_domain_alloc_nesting(struct device *dev, u32 flags, struct iommu_domain *parent, + struct iommufd_viommu *viommu, const struct iommu_user_data *user_data); struct iommufd_viommu * arm_vsmmu_alloc(struct iommu_device *iommu_dev, struct iommu_domain *parent, @@ -1027,6 +1028,7 @@ arm_vsmmu_alloc(struct iommu_device *iommu_dev, struct iommu_domain *parent, static inline struct iommu_domain * arm_smmu_domain_alloc_nesting(struct device *dev, u32 flags, struct iommu_domain *parent, + struct iommufd_viommu *viommu, const struct iommu_user_data *user_data) { return ERR_PTR(-EOPNOTSUPP);
On Thu, Oct 17, 2024 at 09:28:16AM -0700, Nicolin Chen wrote: > On Wed, Oct 09, 2024 at 09:38:11AM -0700, Nicolin Chen wrote: > > Add a new driver-type for ARM SMMUv3 to enum iommu_viommu_type. Implement > > the viommu_alloc op with an arm_vsmmu_alloc function. As an initial step, > > copy the VMID from s2_parent. A later cleanup series is required to move > > the VMID allocation out of the stage-2 domain allocation routine to this. > > > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> > > --- > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 18 ++++++++++++++ > > include/uapi/linux/iommufd.h | 2 ++ > > .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 24 +++++++++++++++++++ > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 + > > 4 files changed, 45 insertions(+) > > I squashed the following changes to this commit (will be in v4). > It replaces nested_domain->s2_parent with nested_domain->vsmmu Err, do we want to make a viommu a hard requirement to use nesting? Is that what is happening here? Jason
On Thu, Oct 17, 2024 at 01:41:23PM -0300, Jason Gunthorpe wrote: > On Thu, Oct 17, 2024 at 09:28:16AM -0700, Nicolin Chen wrote: > > On Wed, Oct 09, 2024 at 09:38:11AM -0700, Nicolin Chen wrote: > > > Add a new driver-type for ARM SMMUv3 to enum iommu_viommu_type. Implement > > > the viommu_alloc op with an arm_vsmmu_alloc function. As an initial step, > > > copy the VMID from s2_parent. A later cleanup series is required to move > > > the VMID allocation out of the stage-2 domain allocation routine to this. > > > > > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> > > > --- > > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 18 ++++++++++++++ > > > include/uapi/linux/iommufd.h | 2 ++ > > > .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 24 +++++++++++++++++++ > > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 + > > > 4 files changed, 45 insertions(+) > > > > I squashed the following changes to this commit (will be in v4). > > It replaces nested_domain->s2_parent with nested_domain->vsmmu > > Err, do we want to make a viommu a hard requirement to use nesting? Is > that what is happening here? For SMMUv3 driver, we have to make it a hard requirement since the invalidation can be only done with a vIOMMU, right? Nicolin
On Thu, Oct 17, 2024 at 09:43:22AM -0700, Nicolin Chen wrote: > On Thu, Oct 17, 2024 at 01:41:23PM -0300, Jason Gunthorpe wrote: > > On Thu, Oct 17, 2024 at 09:28:16AM -0700, Nicolin Chen wrote: > > > On Wed, Oct 09, 2024 at 09:38:11AM -0700, Nicolin Chen wrote: > > > > Add a new driver-type for ARM SMMUv3 to enum iommu_viommu_type. Implement > > > > the viommu_alloc op with an arm_vsmmu_alloc function. As an initial step, > > > > copy the VMID from s2_parent. A later cleanup series is required to move > > > > the VMID allocation out of the stage-2 domain allocation routine to this. > > > > > > > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> > > > > --- > > > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 18 ++++++++++++++ > > > > include/uapi/linux/iommufd.h | 2 ++ > > > > .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 24 +++++++++++++++++++ > > > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 + > > > > 4 files changed, 45 insertions(+) > > > > > > I squashed the following changes to this commit (will be in v4). > > > It replaces nested_domain->s2_parent with nested_domain->vsmmu > > > > Err, do we want to make a viommu a hard requirement to use nesting? Is > > that what is happening here? > > For SMMUv3 driver, we have to make it a hard requirement since the > invalidation can be only done with a vIOMMU, right? Oh, right yes, OK Jason
On Wed, Oct 09, 2024 at 09:38:11AM -0700, Nicolin Chen wrote: > Add a new driver-type for ARM SMMUv3 to enum iommu_viommu_type. Implement > +static inline struct iommufd_viommu * > +arm_vsmmu_alloc(struct iommu_device *iommu_dev, struct iommu_domain *parent, > + struct iommufd_ctx *ictx, unsigned int viommu_type) > +{ > + return ERR_PTR(-EOPNOTSUPP); > +} Let's do #define NULL here instead so we don't get an op at all. > +struct iommufd_viommu * > +arm_vsmmu_alloc(struct iommu_device *iommu_dev, struct iommu_domain *parent, > + struct iommufd_ctx *ictx, unsigned int viommu_type) > +{ > + struct arm_smmu_device *smmu = > + container_of(iommu_dev, struct arm_smmu_device, iommu); > + struct arm_smmu_domain *s2_parent = to_smmu_domain(parent); > + struct arm_vsmmu *vsmmu; > + > + if (viommu_type != IOMMU_VIOMMU_TYPE_ARM_SMMUV3) > + return ERR_PTR(-EOPNOTSUPP); So what happens if the user tries to create a default domain? It skips this and just creates an normal viommu object But then what? The driver needs to make sure it never casts that to a arm_vsmmu ? How? Jason
On Thu, Oct 17, 2024 at 03:40:15PM -0300, Jason Gunthorpe wrote: > On Wed, Oct 09, 2024 at 09:38:11AM -0700, Nicolin Chen wrote: > > Add a new driver-type for ARM SMMUv3 to enum iommu_viommu_type. Implement > > +static inline struct iommufd_viommu * > > +arm_vsmmu_alloc(struct iommu_device *iommu_dev, struct iommu_domain *parent, > > + struct iommufd_ctx *ictx, unsigned int viommu_type) > > +{ > > + return ERR_PTR(-EOPNOTSUPP); > > +} > > Let's do #define NULL here instead so we don't get an op at all. Ack. > > +struct iommufd_viommu * > > +arm_vsmmu_alloc(struct iommu_device *iommu_dev, struct iommu_domain *parent, > > + struct iommufd_ctx *ictx, unsigned int viommu_type) > > +{ > > + struct arm_smmu_device *smmu = > > + container_of(iommu_dev, struct arm_smmu_device, iommu); > > + struct arm_smmu_domain *s2_parent = to_smmu_domain(parent); > > + struct arm_vsmmu *vsmmu; > > + > > + if (viommu_type != IOMMU_VIOMMU_TYPE_ARM_SMMUV3) > > + return ERR_PTR(-EOPNOTSUPP); > > So what happens if the user tries to create a default domain? > > It skips this and just creates an normal viommu object > > But then what? The driver needs to make sure it never casts that to a > arm_vsmmu ? How? So long as a driver doesn't provide iommu_ops->default_viommu_ops, it should be fine. We may also block DEFAULT viommu allocations in the core if the driver doesn't provide that default_viommu_ops. Nicolin
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h index e394943c0b4b..844d1dfdea55 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h @@ -10,6 +10,7 @@ #include <linux/bitfield.h> #include <linux/iommu.h> +#include <linux/iommufd.h> #include <linux/kernel.h> #include <linux/mmzone.h> #include <linux/sizes.h> @@ -1005,12 +1006,22 @@ tegra241_cmdqv_probe(struct arm_smmu_device *smmu) } #endif /* CONFIG_TEGRA241_CMDQV */ +struct arm_vsmmu { + struct iommufd_viommu core; + struct arm_smmu_device *smmu; + struct arm_smmu_domain *s2_parent; + u16 vmid; +}; + #if IS_ENABLED(CONFIG_ARM_SMMU_V3_IOMMUFD) void *arm_smmu_hw_info(struct device *dev, u32 *length, u32 *type); struct iommu_domain * arm_smmu_domain_alloc_nesting(struct device *dev, u32 flags, struct iommu_domain *parent, const struct iommu_user_data *user_data); +struct iommufd_viommu * +arm_vsmmu_alloc(struct iommu_device *iommu_dev, struct iommu_domain *parent, + struct iommufd_ctx *ictx, unsigned int viommu_type); #else #define arm_smmu_hw_info NULL static inline struct iommu_domain * @@ -1020,6 +1031,13 @@ arm_smmu_domain_alloc_nesting(struct device *dev, u32 flags, { return ERR_PTR(-EOPNOTSUPP); } + +static inline struct iommufd_viommu * +arm_vsmmu_alloc(struct iommu_device *iommu_dev, struct iommu_domain *parent, + struct iommufd_ctx *ictx, unsigned int viommu_type) +{ + return ERR_PTR(-EOPNOTSUPP); +} #endif /* CONFIG_ARM_SMMU_V3_IOMMUFD */ #endif /* _ARM_SMMU_V3_H */ diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h index ff8aece8212f..6ee841a8c79b 100644 --- a/include/uapi/linux/iommufd.h +++ b/include/uapi/linux/iommufd.h @@ -857,9 +857,11 @@ struct iommu_fault_alloc { /** * enum iommu_viommu_type - Virtual IOMMU Type * @IOMMU_VIOMMU_TYPE_DEFAULT: Core-managed virtual IOMMU type + * @IOMMU_VIOMMU_TYPE_ARM_SMMUV3: ARM SMMUv3 driver specific type */ enum iommu_viommu_type { IOMMU_VIOMMU_TYPE_DEFAULT = 0, + IOMMU_VIOMMU_TYPE_ARM_SMMUV3 = 1, }; /** diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c index 51260f63be94..5e235fca8f13 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c @@ -212,3 +212,27 @@ arm_smmu_domain_alloc_nesting(struct device *dev, u32 flags, return &nested_domain->domain; } + +struct iommufd_viommu * +arm_vsmmu_alloc(struct iommu_device *iommu_dev, struct iommu_domain *parent, + struct iommufd_ctx *ictx, unsigned int viommu_type) +{ + struct arm_smmu_device *smmu = + container_of(iommu_dev, struct arm_smmu_device, iommu); + struct arm_smmu_domain *s2_parent = to_smmu_domain(parent); + struct arm_vsmmu *vsmmu; + + if (viommu_type != IOMMU_VIOMMU_TYPE_ARM_SMMUV3) + return ERR_PTR(-EOPNOTSUPP); + + vsmmu = iommufd_viommu_alloc(ictx, arm_vsmmu, core, NULL); + if (IS_ERR(vsmmu)) + return ERR_CAST(vsmmu); + + vsmmu->smmu = smmu; + vsmmu->s2_parent = s2_parent; + /* FIXME Move VMID allocation from the S2 domain allocation to here */ + vsmmu->vmid = s2_parent->s2_cfg.vmid; + + return &vsmmu->core; +} diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 4b836a5e9fde..6a23e6dcd5cf 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -3541,6 +3541,7 @@ static struct iommu_ops arm_smmu_ops = { .dev_disable_feat = arm_smmu_dev_disable_feature, .page_response = arm_smmu_page_response, .def_domain_type = arm_smmu_def_domain_type, + .viommu_alloc = arm_vsmmu_alloc, .pgsize_bitmap = -1UL, /* Restricted during device attach */ .owner = THIS_MODULE, .default_domain_ops = &(const struct iommu_domain_ops) {
Add a new driver-type for ARM SMMUv3 to enum iommu_viommu_type. Implement the viommu_alloc op with an arm_vsmmu_alloc function. As an initial step, copy the VMID from s2_parent. A later cleanup series is required to move the VMID allocation out of the stage-2 domain allocation routine to this. Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 18 ++++++++++++++ include/uapi/linux/iommufd.h | 2 ++ .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 24 +++++++++++++++++++ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 + 4 files changed, 45 insertions(+)