Re: [PATCH v13 01/15] iommu: Introduce attach/detach_pasid_table API
Hi Keqian, On 2/1/21 12:27 PM, Keqian Zhu wrote: > Hi Eric, > > On 2020/11/18 19:21, Eric Auger wrote: >> In virtualization use case, when a guest is assigned >> a PCI host device, protected by a virtual IOMMU on the guest, >> the physical IOMMU must be programmed to be consistent with >> the guest mappings. If the physical IOMMU supports two >> translation stages it makes sense to program guest mappings >> onto the first stage/level (ARM/Intel terminology) while the host >> owns the stage/level 2. >> >> In that case, it is mandated to trap on guest configuration >> settings and pass those to the physical iommu driver. >> >> This patch adds a new API to the iommu subsystem that allows >> to set/unset the pasid table information. >> >> A generic iommu_pasid_table_config struct is introduced in >> a new iommu.h uapi header. This is going to be used by the VFIO >> user API. >> >> Signed-off-by: Jean-Philippe Brucker >> Signed-off-by: Liu, Yi L >> Signed-off-by: Ashok Raj >> Signed-off-by: Jacob Pan >> Signed-off-by: Eric Auger >> >> --- >> >> v12 -> v13: >> - Fix config check >> >> v11 -> v12: >> - add argsz, name the union >> --- >> drivers/iommu/iommu.c | 68 ++ >> include/linux/iommu.h | 21 >> include/uapi/linux/iommu.h | 54 ++ >> 3 files changed, 143 insertions(+) >> >> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >> index b53446bb8c6b..978fe34378fb 100644 >> --- a/drivers/iommu/iommu.c >> +++ b/drivers/iommu/iommu.c >> @@ -2171,6 +2171,74 @@ int iommu_uapi_sva_unbind_gpasid(struct iommu_domain >> *domain, struct device *dev >> } >> EXPORT_SYMBOL_GPL(iommu_uapi_sva_unbind_gpasid); >> >> +int iommu_attach_pasid_table(struct iommu_domain *domain, >> + struct iommu_pasid_table_config *cfg) >> +{ >> +if (unlikely(!domain->ops->attach_pasid_table)) >> +return -ENODEV; >> + >> +return domain->ops->attach_pasid_table(domain, cfg); >> +} > miss export symbol? yes we do > >> + >> +int iommu_uapi_attach_pasid_table(struct iommu_domain *domain, >> + void __user *uinfo) >> +{ >> +struct iommu_pasid_table_config pasid_table_data = { 0 }; >> +u32 minsz; >> + >> +if (unlikely(!domain->ops->attach_pasid_table)) >> +return -ENODEV; >> + >> +/* >> + * No new spaces can be added before the variable sized union, the >> + * minimum size is the offset to the union. >> + */ >> +minsz = offsetof(struct iommu_pasid_table_config, vendor_data); >> + >> +/* Copy minsz from user to get flags and argsz */ >> +if (copy_from_user(_table_data, uinfo, minsz)) >> +return -EFAULT; >> + >> +/* Fields before the variable size union are mandatory */ >> +if (pasid_table_data.argsz < minsz) >> +return -EINVAL; >> + >> +/* PASID and address granu require additional info beyond minsz */ >> +if (pasid_table_data.version != PASID_TABLE_CFG_VERSION_1) >> +return -EINVAL; >> +if (pasid_table_data.format == IOMMU_PASID_FORMAT_SMMUV3 && >> +pasid_table_data.argsz < >> +offsetofend(struct iommu_pasid_table_config, >> vendor_data.smmuv3)) >> +return -EINVAL; >> + >> +/* >> + * User might be using a newer UAPI header which has a larger data >> + * size, we shall support the existing flags within the current >> + * size. Copy the remaining user data _after_ minsz but not more >> + * than the current kernel supported size. >> + */ >> +if (copy_from_user((void *)_table_data + minsz, uinfo + minsz, >> + min_t(u32, pasid_table_data.argsz, >> sizeof(pasid_table_data)) - minsz)) >> +return -EFAULT; >> + >> +/* Now the argsz is validated, check the content */ >> +if (pasid_table_data.config < IOMMU_PASID_CONFIG_TRANSLATE || >> +pasid_table_data.config > IOMMU_PASID_CONFIG_ABORT) >> +return -EINVAL; >> + >> +return domain->ops->attach_pasid_table(domain, _table_data); >> +} >> +EXPORT_SYMBOL_GPL(iommu_uapi_attach_pasid_table); >> + >> +void iommu_detach_pasid_table(struct iommu_domain *domain) >> +{ >> +if (unlikely(!domain->ops->detach_pasid_table)) >> +return; >> + >> +domain->ops->detach_pasid_table(domain); >> +} >> +EXPORT_SYMBOL_GPL(iommu_detach_pasid_table); >> + >> static void __iommu_detach_device(struct iommu_domain *domain, >>struct device *dev) >> { >> diff --git a/include/linux/iommu.h b/include/linux/iommu.h >> index b95a6f8db6ff..464fcbecf841 100644 >> --- a/include/linux/iommu.h >> +++ b/include/linux/iommu.h >> @@ -223,6 +223,8 @@ struct iommu_iotlb_gather { >> * @cache_invalidate: invalidate translation caches >> * @sva_bind_gpasid: bind guest pasid and mm >> * @sva_unbind_gpasid: unbind guest pasid and mm >> + * @attach_pasid_table: attach a pasid table >> + *
Re: [PATCH v13 01/15] iommu: Introduce attach/detach_pasid_table API
Hi Eric, On 2020/11/18 19:21, Eric Auger wrote: > In virtualization use case, when a guest is assigned > a PCI host device, protected by a virtual IOMMU on the guest, > the physical IOMMU must be programmed to be consistent with > the guest mappings. If the physical IOMMU supports two > translation stages it makes sense to program guest mappings > onto the first stage/level (ARM/Intel terminology) while the host > owns the stage/level 2. > > In that case, it is mandated to trap on guest configuration > settings and pass those to the physical iommu driver. > > This patch adds a new API to the iommu subsystem that allows > to set/unset the pasid table information. > > A generic iommu_pasid_table_config struct is introduced in > a new iommu.h uapi header. This is going to be used by the VFIO > user API. > > Signed-off-by: Jean-Philippe Brucker > Signed-off-by: Liu, Yi L > Signed-off-by: Ashok Raj > Signed-off-by: Jacob Pan > Signed-off-by: Eric Auger > > --- > > v12 -> v13: > - Fix config check > > v11 -> v12: > - add argsz, name the union > --- > drivers/iommu/iommu.c | 68 ++ > include/linux/iommu.h | 21 > include/uapi/linux/iommu.h | 54 ++ > 3 files changed, 143 insertions(+) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index b53446bb8c6b..978fe34378fb 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -2171,6 +2171,74 @@ int iommu_uapi_sva_unbind_gpasid(struct iommu_domain > *domain, struct device *dev > } > EXPORT_SYMBOL_GPL(iommu_uapi_sva_unbind_gpasid); > > +int iommu_attach_pasid_table(struct iommu_domain *domain, > + struct iommu_pasid_table_config *cfg) > +{ > + if (unlikely(!domain->ops->attach_pasid_table)) > + return -ENODEV; > + > + return domain->ops->attach_pasid_table(domain, cfg); > +} miss export symbol? > + > +int iommu_uapi_attach_pasid_table(struct iommu_domain *domain, > + void __user *uinfo) > +{ > + struct iommu_pasid_table_config pasid_table_data = { 0 }; > + u32 minsz; > + > + if (unlikely(!domain->ops->attach_pasid_table)) > + return -ENODEV; > + > + /* > + * No new spaces can be added before the variable sized union, the > + * minimum size is the offset to the union. > + */ > + minsz = offsetof(struct iommu_pasid_table_config, vendor_data); > + > + /* Copy minsz from user to get flags and argsz */ > + if (copy_from_user(_table_data, uinfo, minsz)) > + return -EFAULT; > + > + /* Fields before the variable size union are mandatory */ > + if (pasid_table_data.argsz < minsz) > + return -EINVAL; > + > + /* PASID and address granu require additional info beyond minsz */ > + if (pasid_table_data.version != PASID_TABLE_CFG_VERSION_1) > + return -EINVAL; > + if (pasid_table_data.format == IOMMU_PASID_FORMAT_SMMUV3 && > + pasid_table_data.argsz < > + offsetofend(struct iommu_pasid_table_config, > vendor_data.smmuv3)) > + return -EINVAL; > + > + /* > + * User might be using a newer UAPI header which has a larger data > + * size, we shall support the existing flags within the current > + * size. Copy the remaining user data _after_ minsz but not more > + * than the current kernel supported size. > + */ > + if (copy_from_user((void *)_table_data + minsz, uinfo + minsz, > +min_t(u32, pasid_table_data.argsz, > sizeof(pasid_table_data)) - minsz)) > + return -EFAULT; > + > + /* Now the argsz is validated, check the content */ > + if (pasid_table_data.config < IOMMU_PASID_CONFIG_TRANSLATE || > + pasid_table_data.config > IOMMU_PASID_CONFIG_ABORT) > + return -EINVAL; > + > + return domain->ops->attach_pasid_table(domain, _table_data); > +} > +EXPORT_SYMBOL_GPL(iommu_uapi_attach_pasid_table); > + > +void iommu_detach_pasid_table(struct iommu_domain *domain) > +{ > + if (unlikely(!domain->ops->detach_pasid_table)) > + return; > + > + domain->ops->detach_pasid_table(domain); > +} > +EXPORT_SYMBOL_GPL(iommu_detach_pasid_table); > + > static void __iommu_detach_device(struct iommu_domain *domain, > struct device *dev) > { > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index b95a6f8db6ff..464fcbecf841 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -223,6 +223,8 @@ struct iommu_iotlb_gather { > * @cache_invalidate: invalidate translation caches > * @sva_bind_gpasid: bind guest pasid and mm > * @sva_unbind_gpasid: unbind guest pasid and mm > + * @attach_pasid_table: attach a pasid table > + * @detach_pasid_table: detach the pasid table > * @def_domain_type: device default domain type, return value: > * - IOMMU_DOMAIN_IDENTITY:
Re: [PATCH v13 01/15] iommu: Introduce attach/detach_pasid_table API
Hi Jacob, On 11/18/20 5:19 PM, Jacob Pan wrote: > Hi Eric, > > On Wed, 18 Nov 2020 12:21:37 +0100, Eric Auger > wrote: > >> In virtualization use case, when a guest is assigned >> a PCI host device, protected by a virtual IOMMU on the guest, >> the physical IOMMU must be programmed to be consistent with >> the guest mappings. If the physical IOMMU supports two >> translation stages it makes sense to program guest mappings >> onto the first stage/level (ARM/Intel terminology) while the host >> owns the stage/level 2. >> >> In that case, it is mandated to trap on guest configuration >> settings and pass those to the physical iommu driver. >> >> This patch adds a new API to the iommu subsystem that allows >> to set/unset the pasid table information. >> >> A generic iommu_pasid_table_config struct is introduced in >> a new iommu.h uapi header. This is going to be used by the VFIO >> user API. >> >> Signed-off-by: Jean-Philippe Brucker >> Signed-off-by: Liu, Yi L >> Signed-off-by: Ashok Raj >> Signed-off-by: Jacob Pan >> Signed-off-by: Eric Auger >> >> --- >> >> v12 -> v13: >> - Fix config check >> >> v11 -> v12: >> - add argsz, name the union >> --- >> drivers/iommu/iommu.c | 68 ++ >> include/linux/iommu.h | 21 >> include/uapi/linux/iommu.h | 54 ++ >> 3 files changed, 143 insertions(+) >> >> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >> index b53446bb8c6b..978fe34378fb 100644 >> --- a/drivers/iommu/iommu.c >> +++ b/drivers/iommu/iommu.c >> @@ -2171,6 +2171,74 @@ int iommu_uapi_sva_unbind_gpasid(struct >> iommu_domain *domain, struct device *dev } >> EXPORT_SYMBOL_GPL(iommu_uapi_sva_unbind_gpasid); >> >> +int iommu_attach_pasid_table(struct iommu_domain *domain, >> + struct iommu_pasid_table_config *cfg) >> +{ >> +if (unlikely(!domain->ops->attach_pasid_table)) >> +return -ENODEV; >> + >> +return domain->ops->attach_pasid_table(domain, cfg); >> +} >> + >> +int iommu_uapi_attach_pasid_table(struct iommu_domain *domain, >> + void __user *uinfo) >> +{ >> +struct iommu_pasid_table_config pasid_table_data = { 0 }; >> +u32 minsz; >> + >> +if (unlikely(!domain->ops->attach_pasid_table)) >> +return -ENODEV; >> + >> +/* >> + * No new spaces can be added before the variable sized union, >> the >> + * minimum size is the offset to the union. >> + */ >> +minsz = offsetof(struct iommu_pasid_table_config, vendor_data); >> + >> +/* Copy minsz from user to get flags and argsz */ >> +if (copy_from_user(_table_data, uinfo, minsz)) >> +return -EFAULT; >> + >> +/* Fields before the variable size union are mandatory */ >> +if (pasid_table_data.argsz < minsz) >> +return -EINVAL; >> + >> +/* PASID and address granu require additional info beyond minsz >> */ >> +if (pasid_table_data.version != PASID_TABLE_CFG_VERSION_1) >> +return -EINVAL; >> +if (pasid_table_data.format == IOMMU_PASID_FORMAT_SMMUV3 && >> +pasid_table_data.argsz < >> +offsetofend(struct iommu_pasid_table_config, >> vendor_data.smmuv3)) >> +return -EINVAL; >> + >> +/* >> + * User might be using a newer UAPI header which has a larger >> data >> + * size, we shall support the existing flags within the current >> + * size. Copy the remaining user data _after_ minsz but not more >> + * than the current kernel supported size. >> + */ >> +if (copy_from_user((void *)_table_data + minsz, uinfo + >> minsz, >> + min_t(u32, pasid_table_data.argsz, >> sizeof(pasid_table_data)) - minsz)) >> +return -EFAULT; >> + >> +/* Now the argsz is validated, check the content */ >> +if (pasid_table_data.config < IOMMU_PASID_CONFIG_TRANSLATE || >> +pasid_table_data.config > IOMMU_PASID_CONFIG_ABORT) >> +return -EINVAL; >> + >> +return domain->ops->attach_pasid_table(domain, >> _table_data); +} >> +EXPORT_SYMBOL_GPL(iommu_uapi_attach_pasid_table); >> + >> +void iommu_detach_pasid_table(struct iommu_domain *domain) >> +{ >> +if (unlikely(!domain->ops->detach_pasid_table)) >> +return; >> + >> +domain->ops->detach_pasid_table(domain); >> +} >> +EXPORT_SYMBOL_GPL(iommu_detach_pasid_table); >> + >> static void __iommu_detach_device(struct iommu_domain *domain, >>struct device *dev) >> { >> diff --git a/include/linux/iommu.h b/include/linux/iommu.h >> index b95a6f8db6ff..464fcbecf841 100644 >> --- a/include/linux/iommu.h >> +++ b/include/linux/iommu.h >> @@ -223,6 +223,8 @@ struct iommu_iotlb_gather { >> * @cache_invalidate: invalidate translation caches >> * @sva_bind_gpasid: bind guest pasid and mm >> * @sva_unbind_gpasid: unbind guest pasid and mm >> + * @attach_pasid_table: attach a pasid table >> + *
Re: [PATCH v13 01/15] iommu: Introduce attach/detach_pasid_table API
Hi Eric, On Wed, 18 Nov 2020 12:21:37 +0100, Eric Auger wrote: > In virtualization use case, when a guest is assigned > a PCI host device, protected by a virtual IOMMU on the guest, > the physical IOMMU must be programmed to be consistent with > the guest mappings. If the physical IOMMU supports two > translation stages it makes sense to program guest mappings > onto the first stage/level (ARM/Intel terminology) while the host > owns the stage/level 2. > > In that case, it is mandated to trap on guest configuration > settings and pass those to the physical iommu driver. > > This patch adds a new API to the iommu subsystem that allows > to set/unset the pasid table information. > > A generic iommu_pasid_table_config struct is introduced in > a new iommu.h uapi header. This is going to be used by the VFIO > user API. > > Signed-off-by: Jean-Philippe Brucker > Signed-off-by: Liu, Yi L > Signed-off-by: Ashok Raj > Signed-off-by: Jacob Pan > Signed-off-by: Eric Auger > > --- > > v12 -> v13: > - Fix config check > > v11 -> v12: > - add argsz, name the union > --- > drivers/iommu/iommu.c | 68 ++ > include/linux/iommu.h | 21 > include/uapi/linux/iommu.h | 54 ++ > 3 files changed, 143 insertions(+) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index b53446bb8c6b..978fe34378fb 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -2171,6 +2171,74 @@ int iommu_uapi_sva_unbind_gpasid(struct > iommu_domain *domain, struct device *dev } > EXPORT_SYMBOL_GPL(iommu_uapi_sva_unbind_gpasid); > > +int iommu_attach_pasid_table(struct iommu_domain *domain, > + struct iommu_pasid_table_config *cfg) > +{ > + if (unlikely(!domain->ops->attach_pasid_table)) > + return -ENODEV; > + > + return domain->ops->attach_pasid_table(domain, cfg); > +} > + > +int iommu_uapi_attach_pasid_table(struct iommu_domain *domain, > + void __user *uinfo) > +{ > + struct iommu_pasid_table_config pasid_table_data = { 0 }; > + u32 minsz; > + > + if (unlikely(!domain->ops->attach_pasid_table)) > + return -ENODEV; > + > + /* > + * No new spaces can be added before the variable sized union, > the > + * minimum size is the offset to the union. > + */ > + minsz = offsetof(struct iommu_pasid_table_config, vendor_data); > + > + /* Copy minsz from user to get flags and argsz */ > + if (copy_from_user(_table_data, uinfo, minsz)) > + return -EFAULT; > + > + /* Fields before the variable size union are mandatory */ > + if (pasid_table_data.argsz < minsz) > + return -EINVAL; > + > + /* PASID and address granu require additional info beyond minsz > */ > + if (pasid_table_data.version != PASID_TABLE_CFG_VERSION_1) > + return -EINVAL; > + if (pasid_table_data.format == IOMMU_PASID_FORMAT_SMMUV3 && > + pasid_table_data.argsz < > + offsetofend(struct iommu_pasid_table_config, > vendor_data.smmuv3)) > + return -EINVAL; > + > + /* > + * User might be using a newer UAPI header which has a larger > data > + * size, we shall support the existing flags within the current > + * size. Copy the remaining user data _after_ minsz but not more > + * than the current kernel supported size. > + */ > + if (copy_from_user((void *)_table_data + minsz, uinfo + > minsz, > +min_t(u32, pasid_table_data.argsz, > sizeof(pasid_table_data)) - minsz)) > + return -EFAULT; > + > + /* Now the argsz is validated, check the content */ > + if (pasid_table_data.config < IOMMU_PASID_CONFIG_TRANSLATE || > + pasid_table_data.config > IOMMU_PASID_CONFIG_ABORT) > + return -EINVAL; > + > + return domain->ops->attach_pasid_table(domain, > _table_data); +} > +EXPORT_SYMBOL_GPL(iommu_uapi_attach_pasid_table); > + > +void iommu_detach_pasid_table(struct iommu_domain *domain) > +{ > + if (unlikely(!domain->ops->detach_pasid_table)) > + return; > + > + domain->ops->detach_pasid_table(domain); > +} > +EXPORT_SYMBOL_GPL(iommu_detach_pasid_table); > + > static void __iommu_detach_device(struct iommu_domain *domain, > struct device *dev) > { > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index b95a6f8db6ff..464fcbecf841 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -223,6 +223,8 @@ struct iommu_iotlb_gather { > * @cache_invalidate: invalidate translation caches > * @sva_bind_gpasid: bind guest pasid and mm > * @sva_unbind_gpasid: unbind guest pasid and mm > + * @attach_pasid_table: attach a pasid table > + * @detach_pasid_table: detach the pasid table > * @def_domain_type: device default domain type, return value: > * - IOMMU_DOMAIN_IDENTITY: must