Re: [PATCH 2/2] iommu: arm-smmu-v3: Report domain nesting info reuqired for stage1
Hi Vivek,
On 2/12/21 11:58 AM, Vivek Gautam wrote:
> Update nested domain information required for stage1 page table.
>
> Signed-off-by: Vivek Gautam
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> 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 c11dd3940583..728018921fae 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2555,6 +2555,7 @@ static int arm_smmu_domain_nesting_info(struct
> arm_smmu_domain *smmu_domain,
> void *data)
> {
> struct iommu_nesting_info *info = (struct iommu_nesting_info *)data;
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> unsigned int size;
>
> if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
> @@ -2571,9 +2572,20 @@ static int arm_smmu_domain_nesting_info(struct
> arm_smmu_domain *smmu_domain,
> return 0;
> }
>
> - /* report an empty iommu_nesting_info for now */
> - memset(info, 0x0, size);
> + /* Update the nesting info as required for stage1 page tables */
> + info->addr_width = smmu->ias;
> + info->format = IOMMU_PASID_FORMAT_ARM_SMMU_V3;
> + info->features = IOMMU_NESTING_FEAT_BIND_PGTBL |
> + IOMMU_NESTING_FEAT_PAGE_RESP |
IOMMU_NESTING_FEAT_PAGE_RESP definition is missing too
Eric
> + IOMMU_NESTING_FEAT_CACHE_INVLD;
> + info->pasid_bits = smmu->ssid_bits;
> + info->vendor.smmuv3.asid_bits = smmu->asid_bits;
> + info->vendor.smmuv3.pgtbl_fmt = ARM_64_LPAE_S1;
> + memset(&info->padding, 0x0, 12);
> + memset(&info->vendor.smmuv3.padding, 0x0, 9);
> +
> info->argsz = size;
> +
> return 0;
> }
>
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] iommu: arm-smmu-v3: Report domain nesting info reuqired for stage1
Hi Vivek,
On 2/12/21 11:58 AM, Vivek Gautam wrote:
> Update nested domain information required for stage1 page table.
s/reuqired/required in the commit title
>
> Signed-off-by: Vivek Gautam
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> 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 c11dd3940583..728018921fae 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2555,6 +2555,7 @@ static int arm_smmu_domain_nesting_info(struct
> arm_smmu_domain *smmu_domain,
> void *data)
> {
> struct iommu_nesting_info *info = (struct iommu_nesting_info *)data;
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> unsigned int size;
>
> if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
> @@ -2571,9 +2572,20 @@ static int arm_smmu_domain_nesting_info(struct
> arm_smmu_domain *smmu_domain,
> return 0;
> }
>
> - /* report an empty iommu_nesting_info for now */
> - memset(info, 0x0, size);
> + /* Update the nesting info as required for stage1 page tables */
> + info->addr_width = smmu->ias;
> + info->format = IOMMU_PASID_FORMAT_ARM_SMMU_V3;
> + info->features = IOMMU_NESTING_FEAT_BIND_PGTBL |
I understood IOMMU_NESTING_FEAT_BIND_PGTBL advertises the requirement to
bind tables per PASID, ie. passing iommu_gpasid_bind_data.
In ARM case I guess you plan to use attach/detach_pasid_table API with
iommu_pasid_table_config struct. So I understood we should add a new
feature here.
> + IOMMU_NESTING_FEAT_PAGE_RESP |
> + IOMMU_NESTING_FEAT_CACHE_INVLD;
> + info->pasid_bits = smmu->ssid_bits;
> + info->vendor.smmuv3.asid_bits = smmu->asid_bits;
> + info->vendor.smmuv3.pgtbl_fmt = ARM_64_LPAE_S1;
> + memset(&info->padding, 0x0, 12);
> + memset(&info->vendor.smmuv3.padding, 0x0, 9);
> +
> info->argsz = size;
> +
spurious new line
> return 0;
> }
>
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] iommu: Report domain nesting info for arm-smmu-v3
Hi Vivek,
On 2/12/21 11:58 AM, Vivek Gautam wrote:
> Add a vendor specific structure for domain nesting info for
> arm smmu-v3, and necessary info fields required to populate
> stage1 page tables.
>
> Signed-off-by: Vivek Gautam
> ---
> include/uapi/linux/iommu.h | 31 +--
> 1 file changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
> index 4d3d988fa353..5f059bcf7720 100644
> --- a/include/uapi/linux/iommu.h
> +++ b/include/uapi/linux/iommu.h
> @@ -323,7 +323,8 @@ struct iommu_gpasid_bind_data {
> #define IOMMU_GPASID_BIND_VERSION_1 1
> __u32 version;
> #define IOMMU_PASID_FORMAT_INTEL_VTD 1
> -#define IOMMU_PASID_FORMAT_LAST 2
> +#define IOMMU_PASID_FORMAT_ARM_SMMU_V3 2
> +#define IOMMU_PASID_FORMAT_LAST 3
> __u32 format;
> __u32 addr_width;
> #define IOMMU_SVA_GPASID_VAL (1 << 0) /* guest PASID valid */
> @@ -409,6 +410,21 @@ struct iommu_nesting_info_vtd {
> __u64 ecap_reg;
> };
>
> +/*
> + * struct iommu_nesting_info_arm_smmuv3 - Arm SMMU-v3 nesting info.
> + */
> +struct iommu_nesting_info_arm_smmuv3 {
> + __u32 flags;
> + __u16 asid_bits;
> +
> + /* Arm LPAE page table format as per kernel */
> +#define ARM_PGTBL_32_LPAE_S1 (0x0)
> +#define ARM_PGTBL_64_LPAE_S1 (0x2)
Shouldn't it be a bitfield instead as both can be supported (the actual
driver only supports 64b table format though). Does it match matches
IDR0.TTF?
> + __u8pgtbl_fmt;
So I understand this API is supposed to allow VFIO to expose those info
early enough to the userspace to help configuring the viommu and avoid
errors later on. I wonder how far we want to go on this path. What about
those other caps that impact the STE/CD validity. There may be others...
SMMU_IDR0.CD2L (support of 2 stage CD)
SMMU_IDR0.TTENDIAN (endianness)
SMMU_IDR0.HTTU (if 0 forbids HA/HD setting in the CD)
SMMU_IDR3.STT (impacts T0SZ)
Thanks
Eric
> +
> + __u8padding[9];
> +};
> +
> /*
> * struct iommu_nesting_info - Information for nesting-capable IOMMU.
> * userspace should check it before using
> @@ -445,11 +461,13 @@ struct iommu_nesting_info_vtd {
> * +---+--+
> *
> * data struct types defined for @format:
> - * ++=+
> - * | @format| data struct |
> - * ++=+
> - * | IOMMU_PASID_FORMAT_INTEL_VTD | struct iommu_nesting_info_vtd |
> - * ++-+
> + * ++==+
> + * | @format| data struct |
> + * ++==+
> + * | IOMMU_PASID_FORMAT_INTEL_VTD | struct iommu_nesting_info_vtd|
> + * +---+---+
> + * | IOMMU_PASID_FORMAT_ARM_SMMU_V3 | struct iommu_nesting_info_arm_smmuv3 |
> + * ++--+
> *
> */
> struct iommu_nesting_info {
> @@ -466,6 +484,7 @@ struct iommu_nesting_info {
> /* Vendor specific data */
> union {
> struct iommu_nesting_info_vtd vtd;
> + struct iommu_nesting_info_arm_smmuv3 smmuv3;
> } vendor;
> };
>
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v2] iommu: Check dev->iommu in iommu_dev_xxx functions
> -Original Message-
> From: Shameerali Kolothum Thodi
> Sent: 12 February 2021 16:45
> To: 'Robin Murphy' ; linux-ker...@vger.kernel.org;
> iommu@lists.linux-foundation.org
> Cc: j...@8bytes.org; jean-phili...@linaro.org; w...@kernel.org; Zengtao (B)
> ; linux...@openeuler.org
> Subject: RE: [PATCH v2] iommu: Check dev->iommu in iommu_dev_xxx functions
>
>
>
> > -Original Message-
> > From: Robin Murphy [mailto:robin.mur...@arm.com]
> > Sent: 12 February 2021 16:39
> > To: Shameerali Kolothum Thodi ;
> > linux-ker...@vger.kernel.org; iommu@lists.linux-foundation.org
> > Cc: j...@8bytes.org; jean-phili...@linaro.org; w...@kernel.org; Zengtao (B)
> > ; linux...@openeuler.org
> > Subject: Re: [PATCH v2] iommu: Check dev->iommu in iommu_dev_xxx
> functions
> >
> > On 2021-02-12 14:54, Shameerali Kolothum Thodi wrote:
> > > Hi Robin/Joerg,
> > >
> > >> -Original Message-
> > >> From: Shameer Kolothum
> [mailto:shameerali.kolothum.th...@huawei.com]
> > >> Sent: 01 February 2021 12:41
> > >> To: linux-ker...@vger.kernel.org; iommu@lists.linux-foundation.org
> > >> Cc: j...@8bytes.org; robin.mur...@arm.com; jean-phili...@linaro.org;
> > >> w...@kernel.org; Zengtao (B) ;
> > >> linux...@openeuler.org
> > >> Subject: [Linuxarm] [PATCH v2] iommu: Check dev->iommu in
> > iommu_dev_xxx
> > >> functions
> > >>
> > >> The device iommu probe/attach might have failed leaving dev->iommu
> > >> to NULL and device drivers may still invoke these functions resulting
> > >> in a crash in iommu vendor driver code. Hence make sure we check that.
> > >>
> > >> Also added iommu_ops to the "struct dev_iommu" and set it if the dev
> > >> is successfully associated with an iommu.
> > >>
> > >> Fixes: a3a195929d40 ("iommu: Add APIs for multiple domains per
> device")
> > >> Signed-off-by: Shameer Kolothum
> >
> > >> ---
> > >> v1 --> v2:
> > >> -Added iommu_ops to struct dev_iommu based on the discussion with
> > Robin.
> > >> -Rebased against iommu-tree core branch.
> > >
> > > A gentle ping on this...
> >
> > Is there a convincing justification for maintaining yet another copy of
> > the ops pointer rather than simply dereferencing iommu_dev->ops at point
> > of use?
> >
>
> TBH, nothing I can think of now. That was mainly the way I interpreted your
> suggestion
> from the v1. Now it looks like you didn’t mean it :). I am Ok to rework it to
> dereference
> it from iommu_dev. Please let me know.
So we can do something like this,
index fd76e2f579fe..5fd31a3cec18 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2865,10 +2865,12 @@ EXPORT_SYMBOL_GPL(iommu_fwspec_add_ids);
*/
int iommu_dev_enable_feature(struct device *dev, enum iommu_dev_features feat)
{
- const struct iommu_ops *ops = dev->bus->iommu_ops;
+ if (dev->iommu && dev->iommu->iommu_dev && dev->iommu->iommu_dev->ops)
+ struct iommu_ops *ops = dev->iommu->iommu_dev->ops;
- if (ops && ops->dev_enable_feat)
- return ops->dev_enable_feat(dev, feat);
+ if (ops->dev_enable_feat)
+ return ops->dev_enable_feat(dev, feat);
+ }
return -ENODEV;
}
Again, not sure we need to do the checking for iommu->dev and ops here. If the
dev->iommu is set, is it safe to assume that we have a valid iommu->iommu_dev
and ops always? (May be it is safer to do the checking in case something
else breaks this assumption in future). Please let me know your thoughts.
Thanks,
Shameer
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v2] iommu: Check dev->iommu in iommu_dev_xxx functions
> -Original Message-
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: 12 February 2021 16:39
> To: Shameerali Kolothum Thodi ;
> linux-ker...@vger.kernel.org; iommu@lists.linux-foundation.org
> Cc: j...@8bytes.org; jean-phili...@linaro.org; w...@kernel.org; Zengtao (B)
> ; linux...@openeuler.org
> Subject: Re: [PATCH v2] iommu: Check dev->iommu in iommu_dev_xxx functions
>
> On 2021-02-12 14:54, Shameerali Kolothum Thodi wrote:
> > Hi Robin/Joerg,
> >
> >> -Original Message-
> >> From: Shameer Kolothum [mailto:shameerali.kolothum.th...@huawei.com]
> >> Sent: 01 February 2021 12:41
> >> To: linux-ker...@vger.kernel.org; iommu@lists.linux-foundation.org
> >> Cc: j...@8bytes.org; robin.mur...@arm.com; jean-phili...@linaro.org;
> >> w...@kernel.org; Zengtao (B) ;
> >> linux...@openeuler.org
> >> Subject: [Linuxarm] [PATCH v2] iommu: Check dev->iommu in
> iommu_dev_xxx
> >> functions
> >>
> >> The device iommu probe/attach might have failed leaving dev->iommu
> >> to NULL and device drivers may still invoke these functions resulting
> >> in a crash in iommu vendor driver code. Hence make sure we check that.
> >>
> >> Also added iommu_ops to the "struct dev_iommu" and set it if the dev
> >> is successfully associated with an iommu.
> >>
> >> Fixes: a3a195929d40 ("iommu: Add APIs for multiple domains per device")
> >> Signed-off-by: Shameer Kolothum
>
> >> ---
> >> v1 --> v2:
> >> -Added iommu_ops to struct dev_iommu based on the discussion with
> Robin.
> >> -Rebased against iommu-tree core branch.
> >
> > A gentle ping on this...
>
> Is there a convincing justification for maintaining yet another copy of
> the ops pointer rather than simply dereferencing iommu_dev->ops at point
> of use?
>
TBH, nothing I can think of now. That was mainly the way I interpreted your
suggestion
from the v1. Now it looks like you didn’t mean it :). I am Ok to rework it to
dereference
it from iommu_dev. Please let me know.
Thanks,
Shameer
> Robin.
>
> > Thanks,
> > Shameer
> >
> >> ---
> >> drivers/iommu/iommu.c | 19 +++
> >> include/linux/iommu.h | 2 ++
> >> 2 files changed, 9 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> >> index fd76e2f579fe..6023d0b7c542 100644
> >> --- a/drivers/iommu/iommu.c
> >> +++ b/drivers/iommu/iommu.c
> >> @@ -217,6 +217,7 @@ static int __iommu_probe_device(struct device
> *dev,
> >> struct list_head *group_list
> >>}
> >>
> >>dev->iommu->iommu_dev = iommu_dev;
> >> + dev->iommu->ops = iommu_dev->ops;
> >>
> >>group = iommu_group_get_for_dev(dev);
> >>if (IS_ERR(group)) {
> >> @@ -2865,10 +2866,8 @@
> EXPORT_SYMBOL_GPL(iommu_fwspec_add_ids);
> >>*/
> >> int iommu_dev_enable_feature(struct device *dev, enum
> >> iommu_dev_features feat)
> >> {
> >> - const struct iommu_ops *ops = dev->bus->iommu_ops;
> >> -
> >> - if (ops && ops->dev_enable_feat)
> >> - return ops->dev_enable_feat(dev, feat);
> >> + if (dev->iommu && dev->iommu->ops->dev_enable_feat)
> >> + return dev->iommu->ops->dev_enable_feat(dev, feat);
> >>
> >>return -ENODEV;
> >> }
> >> @@ -2881,10 +2880,8 @@
> >> EXPORT_SYMBOL_GPL(iommu_dev_enable_feature);
> >>*/
> >> int iommu_dev_disable_feature(struct device *dev, enum
> >> iommu_dev_features feat)
> >> {
> >> - const struct iommu_ops *ops = dev->bus->iommu_ops;
> >> -
> >> - if (ops && ops->dev_disable_feat)
> >> - return ops->dev_disable_feat(dev, feat);
> >> + if (dev->iommu && dev->iommu->ops->dev_disable_feat)
> >> + return dev->iommu->ops->dev_disable_feat(dev, feat);
> >>
> >>return -EBUSY;
> >> }
> >> @@ -2892,10 +2889,8 @@
> >> EXPORT_SYMBOL_GPL(iommu_dev_disable_feature);
> >>
> >> bool iommu_dev_feature_enabled(struct device *dev, enum
> >> iommu_dev_features feat)
> >> {
> >> - const struct iommu_ops *ops = dev->bus->iommu_ops;
> >> -
> >> - if (ops && ops->dev_feat_enabled)
> >> - return ops->dev_feat_enabled(dev, feat);
> >> + if (dev->iommu && dev->iommu->ops->dev_feat_enabled)
> >> + return dev->iommu->ops->dev_feat_enabled(dev, feat);
> >>
> >>return false;
> >> }
> >> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> >> index 524ffc2ff64f..ff0c76bdfb67 100644
> >> --- a/include/linux/iommu.h
> >> +++ b/include/linux/iommu.h
> >> @@ -354,6 +354,7 @@ struct iommu_fault_param {
> >>* @fault_param: IOMMU detected device fault reporting data
> >>* @fwspec: IOMMU fwspec data
> >>* @iommu_dev:IOMMU device this device is linked to
> >> + * @ops: iommu-ops for talking to the iommu_dev
> >>* @priv: IOMMU Driver private data
> >>*
> >>* TODO: migrate other per device data pointers under
> iommu_dev_data,
> >> e.g.
> >> @@ -364,6 +365,7 @@ struct dev_iommu {
> >>struct iommu_fault_param*fault_param;
> >>struct iommu_fwspec *fwspec;
> >>struct iommu_device
Re: [PATCH v2] iommu: Check dev->iommu in iommu_dev_xxx functions
On 2021-02-12 14:54, Shameerali Kolothum Thodi wrote:
Hi Robin/Joerg,
-Original Message-
From: Shameer Kolothum [mailto:shameerali.kolothum.th...@huawei.com]
Sent: 01 February 2021 12:41
To: linux-ker...@vger.kernel.org; iommu@lists.linux-foundation.org
Cc: j...@8bytes.org; robin.mur...@arm.com; jean-phili...@linaro.org;
w...@kernel.org; Zengtao (B) ;
linux...@openeuler.org
Subject: [Linuxarm] [PATCH v2] iommu: Check dev->iommu in iommu_dev_xxx
functions
The device iommu probe/attach might have failed leaving dev->iommu
to NULL and device drivers may still invoke these functions resulting
in a crash in iommu vendor driver code. Hence make sure we check that.
Also added iommu_ops to the "struct dev_iommu" and set it if the dev
is successfully associated with an iommu.
Fixes: a3a195929d40 ("iommu: Add APIs for multiple domains per device")
Signed-off-by: Shameer Kolothum
---
v1 --> v2:
-Added iommu_ops to struct dev_iommu based on the discussion with Robin.
-Rebased against iommu-tree core branch.
A gentle ping on this...
Is there a convincing justification for maintaining yet another copy of
the ops pointer rather than simply dereferencing iommu_dev->ops at point
of use?
Robin.
Thanks,
Shameer
---
drivers/iommu/iommu.c | 19 +++
include/linux/iommu.h | 2 ++
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index fd76e2f579fe..6023d0b7c542 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -217,6 +217,7 @@ static int __iommu_probe_device(struct device *dev,
struct list_head *group_list
}
dev->iommu->iommu_dev = iommu_dev;
+ dev->iommu->ops = iommu_dev->ops;
group = iommu_group_get_for_dev(dev);
if (IS_ERR(group)) {
@@ -2865,10 +2866,8 @@ EXPORT_SYMBOL_GPL(iommu_fwspec_add_ids);
*/
int iommu_dev_enable_feature(struct device *dev, enum
iommu_dev_features feat)
{
- const struct iommu_ops *ops = dev->bus->iommu_ops;
-
- if (ops && ops->dev_enable_feat)
- return ops->dev_enable_feat(dev, feat);
+ if (dev->iommu && dev->iommu->ops->dev_enable_feat)
+ return dev->iommu->ops->dev_enable_feat(dev, feat);
return -ENODEV;
}
@@ -2881,10 +2880,8 @@
EXPORT_SYMBOL_GPL(iommu_dev_enable_feature);
*/
int iommu_dev_disable_feature(struct device *dev, enum
iommu_dev_features feat)
{
- const struct iommu_ops *ops = dev->bus->iommu_ops;
-
- if (ops && ops->dev_disable_feat)
- return ops->dev_disable_feat(dev, feat);
+ if (dev->iommu && dev->iommu->ops->dev_disable_feat)
+ return dev->iommu->ops->dev_disable_feat(dev, feat);
return -EBUSY;
}
@@ -2892,10 +2889,8 @@
EXPORT_SYMBOL_GPL(iommu_dev_disable_feature);
bool iommu_dev_feature_enabled(struct device *dev, enum
iommu_dev_features feat)
{
- const struct iommu_ops *ops = dev->bus->iommu_ops;
-
- if (ops && ops->dev_feat_enabled)
- return ops->dev_feat_enabled(dev, feat);
+ if (dev->iommu && dev->iommu->ops->dev_feat_enabled)
+ return dev->iommu->ops->dev_feat_enabled(dev, feat);
return false;
}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 524ffc2ff64f..ff0c76bdfb67 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -354,6 +354,7 @@ struct iommu_fault_param {
* @fault_param: IOMMU detected device fault reporting data
* @fwspec:IOMMU fwspec data
* @iommu_dev: IOMMU device this device is linked to
+ * @ops:iommu-ops for talking to the iommu_dev
* @priv: IOMMU Driver private data
*
* TODO: migrate other per device data pointers under iommu_dev_data,
e.g.
@@ -364,6 +365,7 @@ struct dev_iommu {
struct iommu_fault_param*fault_param;
struct iommu_fwspec *fwspec;
struct iommu_device *iommu_dev;
+ const struct iommu_ops *ops;
void*priv;
};
--
2.17.1
___
Linuxarm mailing list -- linux...@openeuler.org
To unsubscribe send an email to linuxarm-le...@openeuler.org
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v2] iommu: Check dev->iommu in iommu_dev_xxx functions
Hi Robin/Joerg,
> -Original Message-
> From: Shameer Kolothum [mailto:shameerali.kolothum.th...@huawei.com]
> Sent: 01 February 2021 12:41
> To: linux-ker...@vger.kernel.org; iommu@lists.linux-foundation.org
> Cc: j...@8bytes.org; robin.mur...@arm.com; jean-phili...@linaro.org;
> w...@kernel.org; Zengtao (B) ;
> linux...@openeuler.org
> Subject: [Linuxarm] [PATCH v2] iommu: Check dev->iommu in iommu_dev_xxx
> functions
>
> The device iommu probe/attach might have failed leaving dev->iommu
> to NULL and device drivers may still invoke these functions resulting
> in a crash in iommu vendor driver code. Hence make sure we check that.
>
> Also added iommu_ops to the "struct dev_iommu" and set it if the dev
> is successfully associated with an iommu.
>
> Fixes: a3a195929d40 ("iommu: Add APIs for multiple domains per device")
> Signed-off-by: Shameer Kolothum
> ---
> v1 --> v2:
> -Added iommu_ops to struct dev_iommu based on the discussion with Robin.
> -Rebased against iommu-tree core branch.
A gentle ping on this...
Thanks,
Shameer
> ---
> drivers/iommu/iommu.c | 19 +++
> include/linux/iommu.h | 2 ++
> 2 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index fd76e2f579fe..6023d0b7c542 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -217,6 +217,7 @@ static int __iommu_probe_device(struct device *dev,
> struct list_head *group_list
> }
>
> dev->iommu->iommu_dev = iommu_dev;
> + dev->iommu->ops = iommu_dev->ops;
>
> group = iommu_group_get_for_dev(dev);
> if (IS_ERR(group)) {
> @@ -2865,10 +2866,8 @@ EXPORT_SYMBOL_GPL(iommu_fwspec_add_ids);
> */
> int iommu_dev_enable_feature(struct device *dev, enum
> iommu_dev_features feat)
> {
> - const struct iommu_ops *ops = dev->bus->iommu_ops;
> -
> - if (ops && ops->dev_enable_feat)
> - return ops->dev_enable_feat(dev, feat);
> + if (dev->iommu && dev->iommu->ops->dev_enable_feat)
> + return dev->iommu->ops->dev_enable_feat(dev, feat);
>
> return -ENODEV;
> }
> @@ -2881,10 +2880,8 @@
> EXPORT_SYMBOL_GPL(iommu_dev_enable_feature);
> */
> int iommu_dev_disable_feature(struct device *dev, enum
> iommu_dev_features feat)
> {
> - const struct iommu_ops *ops = dev->bus->iommu_ops;
> -
> - if (ops && ops->dev_disable_feat)
> - return ops->dev_disable_feat(dev, feat);
> + if (dev->iommu && dev->iommu->ops->dev_disable_feat)
> + return dev->iommu->ops->dev_disable_feat(dev, feat);
>
> return -EBUSY;
> }
> @@ -2892,10 +2889,8 @@
> EXPORT_SYMBOL_GPL(iommu_dev_disable_feature);
>
> bool iommu_dev_feature_enabled(struct device *dev, enum
> iommu_dev_features feat)
> {
> - const struct iommu_ops *ops = dev->bus->iommu_ops;
> -
> - if (ops && ops->dev_feat_enabled)
> - return ops->dev_feat_enabled(dev, feat);
> + if (dev->iommu && dev->iommu->ops->dev_feat_enabled)
> + return dev->iommu->ops->dev_feat_enabled(dev, feat);
>
> return false;
> }
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 524ffc2ff64f..ff0c76bdfb67 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -354,6 +354,7 @@ struct iommu_fault_param {
> * @fault_param: IOMMU detected device fault reporting data
> * @fwspec: IOMMU fwspec data
> * @iommu_dev:IOMMU device this device is linked to
> + * @ops: iommu-ops for talking to the iommu_dev
> * @priv: IOMMU Driver private data
> *
> * TODO: migrate other per device data pointers under iommu_dev_data,
> e.g.
> @@ -364,6 +365,7 @@ struct dev_iommu {
> struct iommu_fault_param*fault_param;
> struct iommu_fwspec *fwspec;
> struct iommu_device *iommu_dev;
> + const struct iommu_ops *ops;
> void*priv;
> };
>
> --
> 2.17.1
> ___
> Linuxarm mailing list -- linux...@openeuler.org
> To unsubscribe send an email to linuxarm-le...@openeuler.org
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 02/16] iommu/smmu: Report empty domain nesting info
Hi Eric,
On 2/12/21 3:48 PM, Vivek Kumar Gautam wrote:
Hi Eric,
On 2/12/21 3:27 PM, Auger Eric wrote:
Hi Vivek, Yi,
On 2/12/21 8:14 AM, Vivek Gautam wrote:
Hi Yi,
On Sat, Jan 23, 2021 at 2:29 PM Liu, Yi L wrote:
Hi Eric,
From: Auger Eric
Sent: Tuesday, January 19, 2021 6:03 PM
Hi Yi, Vivek,
[...]
I see. I think there needs a change in the code there. Should also
expect
a nesting_info returned instead of an int anymore. @Eric, how
about your
opinion?
domain = iommu_get_domain_for_dev(&vdev->pdev->dev);
ret = iommu_domain_get_attr(domain, DOMAIN_ATTR_NESTING,
&info);
if (ret || !(info.features & IOMMU_NESTING_FEAT_PAGE_RESP)) {
/*
* No need go futher as no page request service support.
*/
return 0;
}
Sure I think it is "just" a matter of synchro between the 2 series.
Yi,
exactly.
do you have plans to respin part of
[PATCH v7 00/16] vfio: expose virtual Shared Virtual Addressing to VMs
or would you allow me to embed this patch in my series.
My v7 hasn’t touch the prq change yet. So I think it's better for
you to
embed it to your series. ^_^>>
Can you please let me know if you have an updated series of these
patches? It will help me to work with virtio-iommu/arm side changes.
As per the previous discussion, I plan to take those 2 patches in my
SMMUv3 nested stage series:
[PATCH v7 01/16] iommu: Report domain nesting info
[PATCH v7 02/16] iommu/smmu: Report empty domain nesting info
we need to upgrade both since we do not want to report an empty nesting
info anymore, for arm.
Absolutely. Let me send the couple of patches that I have been using,
that add arm configuration.
Posted the couple of patches that I have been using -
https://lore.kernel.org/linux-iommu/20210212105859.8445-1-vivek.gau...@arm.com/T/#t
Thanks & regards
Vivek
Best regards
Vivek
Thanks
Eric
Thanks & regards
Vivek
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/2] iommu: arm-smmu-v3: Report domain nesting info reuqired for stage1
Update nested domain information required for stage1 page table.
Signed-off-by: Vivek Gautam
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
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 c11dd3940583..728018921fae 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2555,6 +2555,7 @@ static int arm_smmu_domain_nesting_info(struct
arm_smmu_domain *smmu_domain,
void *data)
{
struct iommu_nesting_info *info = (struct iommu_nesting_info *)data;
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
unsigned int size;
if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
@@ -2571,9 +2572,20 @@ static int arm_smmu_domain_nesting_info(struct
arm_smmu_domain *smmu_domain,
return 0;
}
- /* report an empty iommu_nesting_info for now */
- memset(info, 0x0, size);
+ /* Update the nesting info as required for stage1 page tables */
+ info->addr_width = smmu->ias;
+ info->format = IOMMU_PASID_FORMAT_ARM_SMMU_V3;
+ info->features = IOMMU_NESTING_FEAT_BIND_PGTBL |
+IOMMU_NESTING_FEAT_PAGE_RESP |
+IOMMU_NESTING_FEAT_CACHE_INVLD;
+ info->pasid_bits = smmu->ssid_bits;
+ info->vendor.smmuv3.asid_bits = smmu->asid_bits;
+ info->vendor.smmuv3.pgtbl_fmt = ARM_64_LPAE_S1;
+ memset(&info->padding, 0x0, 12);
+ memset(&info->vendor.smmuv3.padding, 0x0, 9);
+
info->argsz = size;
+
return 0;
}
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/2] iommu: Report domain nesting info for arm-smmu-v3
Add a vendor specific structure for domain nesting info for
arm smmu-v3, and necessary info fields required to populate
stage1 page tables.
Signed-off-by: Vivek Gautam
---
include/uapi/linux/iommu.h | 31 +--
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index 4d3d988fa353..5f059bcf7720 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -323,7 +323,8 @@ struct iommu_gpasid_bind_data {
#define IOMMU_GPASID_BIND_VERSION_11
__u32 version;
#define IOMMU_PASID_FORMAT_INTEL_VTD 1
-#define IOMMU_PASID_FORMAT_LAST2
+#define IOMMU_PASID_FORMAT_ARM_SMMU_V3 2
+#define IOMMU_PASID_FORMAT_LAST3
__u32 format;
__u32 addr_width;
#define IOMMU_SVA_GPASID_VAL (1 << 0) /* guest PASID valid */
@@ -409,6 +410,21 @@ struct iommu_nesting_info_vtd {
__u64 ecap_reg;
};
+/*
+ * struct iommu_nesting_info_arm_smmuv3 - Arm SMMU-v3 nesting info.
+ */
+struct iommu_nesting_info_arm_smmuv3 {
+ __u32 flags;
+ __u16 asid_bits;
+
+ /* Arm LPAE page table format as per kernel */
+#define ARM_PGTBL_32_LPAE_S1 (0x0)
+#define ARM_PGTBL_64_LPAE_S1 (0x2)
+ __u8pgtbl_fmt;
+
+ __u8padding[9];
+};
+
/*
* struct iommu_nesting_info - Information for nesting-capable IOMMU.
*userspace should check it before using
@@ -445,11 +461,13 @@ struct iommu_nesting_info_vtd {
* +---+--+
*
* data struct types defined for @format:
- * ++=+
- * | @format| data struct |
- * ++=+
- * | IOMMU_PASID_FORMAT_INTEL_VTD | struct iommu_nesting_info_vtd |
- * ++-+
+ * ++==+
+ * | @format| data struct |
+ * ++==+
+ * | IOMMU_PASID_FORMAT_INTEL_VTD | struct iommu_nesting_info_vtd|
+ * +---+---+
+ * | IOMMU_PASID_FORMAT_ARM_SMMU_V3 | struct iommu_nesting_info_arm_smmuv3 |
+ * ++--+
*
*/
struct iommu_nesting_info {
@@ -466,6 +484,7 @@ struct iommu_nesting_info {
/* Vendor specific data */
union {
struct iommu_nesting_info_vtd vtd;
+ struct iommu_nesting_info_arm_smmuv3 smmuv3;
} vendor;
};
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/2] Domain nesting info for arm-smmu
These couple of patches are adding nesting information for arm and are based on the domain nesting info patches by Yi [1,2,3]. Based on the discussion in the thread [4], sending these out as I have been using in my tree [5] for nested translation based on virtio-iommu on Arm reference platforms. Thanks & regards Vivek [1] https://lore.kernel.org/kvm/1599734733-6431-2-git-send-email-yi.l@intel.com/ [2] https://lore.kernel.org/kvm/1599734733-6431-3-git-send-email-yi.l@intel.com/ [3] https://lore.kernel.org/kvm/1599734733-6431-4-git-send-email-yi.l@intel.com/ [4] https://lore.kernel.org/kvm/306e7dd2-9eb2-0ca3-6a93-7c9aa0821...@arm.com/ [5] https://github.com/vivek-arm/linux/tree/5.11-rc3-nested-pgtbl-arm-smmuv3-virtio-iommu Vivek Gautam (2): iommu: Report domain nesting info for arm-smmu-v3 iommu: arm-smmu-v3: Report domain nesting info reuqired for stage1 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 16 +-- include/uapi/linux/iommu.h | 31 + 2 files changed, 39 insertions(+), 8 deletions(-) -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: Add device name to iommu map/unmap trace events
On Tue, Feb 09, 2021 at 06:06:20PM +0530, Sai Prakash Ranjan wrote:
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 5e7fe519430a..6064187d9bb6 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -87,6 +87,7 @@ struct iommu_domain {
> void *handler_token;
> struct iommu_domain_geometry geometry;
> void *iova_cookie;
> + char dev_name[32];
> };
No, definitly not. A domain is a device DMA address space which can be
used by more than one device. Just look at IOMMU groups with more than
one member device, in this case just one device name would be very
misleading.
Regards,
Joerg
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Fix performance counter initialization
On Mon, Feb 08, 2021 at 06:27:12AM -0600, Suravee Suthikulpanit wrote: > drivers/iommu/amd/init.c | 45 ++-- > 1 file changed, 34 insertions(+), 11 deletions(-) Applied, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 02/16] iommu/smmu: Report empty domain nesting info
Hi Eric,
On 2/12/21 3:27 PM, Auger Eric wrote:
Hi Vivek, Yi,
On 2/12/21 8:14 AM, Vivek Gautam wrote:
Hi Yi,
On Sat, Jan 23, 2021 at 2:29 PM Liu, Yi L wrote:
Hi Eric,
From: Auger Eric
Sent: Tuesday, January 19, 2021 6:03 PM
Hi Yi, Vivek,
[...]
I see. I think there needs a change in the code there. Should also expect
a nesting_info returned instead of an int anymore. @Eric, how about your
opinion?
domain = iommu_get_domain_for_dev(&vdev->pdev->dev);
ret = iommu_domain_get_attr(domain, DOMAIN_ATTR_NESTING,
&info);
if (ret || !(info.features & IOMMU_NESTING_FEAT_PAGE_RESP)) {
/*
* No need go futher as no page request service support.
*/
return 0;
}
Sure I think it is "just" a matter of synchro between the 2 series. Yi,
exactly.
do you have plans to respin part of
[PATCH v7 00/16] vfio: expose virtual Shared Virtual Addressing to VMs
or would you allow me to embed this patch in my series.
My v7 hasn’t touch the prq change yet. So I think it's better for you to
embed it to your series. ^_^>>
Can you please let me know if you have an updated series of these
patches? It will help me to work with virtio-iommu/arm side changes.
As per the previous discussion, I plan to take those 2 patches in my
SMMUv3 nested stage series:
[PATCH v7 01/16] iommu: Report domain nesting info
[PATCH v7 02/16] iommu/smmu: Report empty domain nesting info
we need to upgrade both since we do not want to report an empty nesting
info anymore, for arm.
Absolutely. Let me send the couple of patches that I have been using,
that add arm configuration.
Best regards
Vivek
Thanks
Eric
Thanks & regards
Vivek
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 02/16] iommu/smmu: Report empty domain nesting info
Hi Vivek, Yi,
On 2/12/21 8:14 AM, Vivek Gautam wrote:
> Hi Yi,
>
>
> On Sat, Jan 23, 2021 at 2:29 PM Liu, Yi L wrote:
>>
>> Hi Eric,
>>
>>> From: Auger Eric
>>> Sent: Tuesday, January 19, 2021 6:03 PM
>>>
>>> Hi Yi, Vivek,
>>>
>> [...]
I see. I think there needs a change in the code there. Should also expect
a nesting_info returned instead of an int anymore. @Eric, how about your
opinion?
domain = iommu_get_domain_for_dev(&vdev->pdev->dev);
ret = iommu_domain_get_attr(domain, DOMAIN_ATTR_NESTING,
>>> &info);
if (ret || !(info.features & IOMMU_NESTING_FEAT_PAGE_RESP)) {
/*
* No need go futher as no page request service support.
*/
return 0;
}
>>> Sure I think it is "just" a matter of synchro between the 2 series. Yi,
>>
>> exactly.
>>
>>> do you have plans to respin part of
>>> [PATCH v7 00/16] vfio: expose virtual Shared Virtual Addressing to VMs
>>> or would you allow me to embed this patch in my series.
>>
>> My v7 hasn’t touch the prq change yet. So I think it's better for you to
>> embed it to your series. ^_^>>
>
> Can you please let me know if you have an updated series of these
> patches? It will help me to work with virtio-iommu/arm side changes.
As per the previous discussion, I plan to take those 2 patches in my
SMMUv3 nested stage series:
[PATCH v7 01/16] iommu: Report domain nesting info
[PATCH v7 02/16] iommu/smmu: Report empty domain nesting info
we need to upgrade both since we do not want to report an empty nesting
info anymore, for arm.
Thanks
Eric
>
> Thanks & regards
> Vivek
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE
On 12.02.21 08:02, Anshuman Khandual wrote:
On 2/11/21 2:07 PM, David Hildenbrand wrote:
On 11.02.21 07:22, Anshuman Khandual wrote:
The following warning gets triggered while trying to boot a 64K page size
without THP config kernel on arm64 platform.
WARNING: CPU: 5 PID: 124 at mm/vmstat.c:1080 __fragmentation_index+0xa4/0xc0
Modules linked in:
CPU: 5 PID: 124 Comm: kswapd0 Not tainted 5.11.0-rc6-4-ga0ea7d62002 #159
Hardware name: linux,dummy-virt (DT)
[ 8.810673] pstate: 2045 (nzCv daif +PAN -UAO -TCO BTYPE=--)
[ 8.811732] pc : __fragmentation_index+0xa4/0xc0
[ 8.812555] lr : fragmentation_index+0xf8/0x138
[ 8.813360] sp : 864079b0
[ 8.813958] x29: 864079b0 x28: 0372
[ 8.814901] x27: 7682 x26: 8000135b3948
[ 8.815847] x25: 1fffe00010c80f48 x24:
[ 8.816805] x23: x22: 000d
[ 8.817764] x21: 0030 x20: 0005ffcb4d58
[ 8.818712] x19: 000b x18:
[ 8.819656] x17: x16:
[ 8.820613] x15: x14: 8000114c6258
[ 8.821560] x13: 6000bff969ba x12: 1fffe000bff969b9
[ 8.822514] x11: 1fffe000bff969b9 x10: 6000bff969b9
[ 8.823461] x9 : dfff8000 x8 : 0005ffcb4dcf
[ 8.824415] x7 : 0001 x6 : 41b58ab3
[ 8.825359] x5 : 600010c80f48 x4 : dfff8000
[ 8.826313] x3 : 8000102be670 x2 : 0007
[ 8.827259] x1 : 86407a60 x0 : 000d
[ 8.828218] Call trace:
[ 8.828667] __fragmentation_index+0xa4/0xc0
[ 8.829436] fragmentation_index+0xf8/0x138
[ 8.830194] compaction_suitable+0x98/0xb8
[ 8.830934] wakeup_kcompactd+0xdc/0x128
[ 8.831640] balance_pgdat+0x71c/0x7a0
[ 8.832327] kswapd+0x31c/0x520
[ 8.832902] kthread+0x224/0x230
[ 8.833491] ret_from_fork+0x10/0x30
[ 8.834150] ---[ end trace 472836f79c15516b ]---
This warning comes from __fragmentation_index() when the requested order
is greater than MAX_ORDER.
static int __fragmentation_index(unsigned int order,
struct contig_page_info *info)
{
unsigned long requested = 1UL << order;
if (WARN_ON_ONCE(order >= MAX_ORDER)) <= Triggered here
return 0;
Digging it further reveals that pageblock_order has been assigned a value
which is greater than MAX_ORDER failing the above check. But why this
happened ? Because HUGETLB_PAGE_ORDER for the given config on arm64 is
greater than MAX_ORDER.
The solution involves enabling HUGETLB_PAGE_SIZE_VARIABLE which would make
pageblock_order a variable instead of constant HUGETLB_PAGE_ORDER. But that
change alone also did not really work as pageblock_order still got assigned
as HUGETLB_PAGE_ORDER in set_pageblock_order(). HUGETLB_PAGE_ORDER needs to
be less than MAX_ORDER for its appropriateness as pageblock_order otherwise
just fallback to MAX_ORDER - 1 as before. While here it also fixes a build
problem via type casting MAX_ORDER in rmem_cma_setup().
I'm wondering, is there any real value in allowing FORCE_MAX_ZONEORDER to be
"11" with ARM64_64K_PAGES/ARM64_16K_PAGES?
MAX_ORDER should be as high as would be required for the current config.
Unless THP is enabled, there is no need for it to be any higher than 11.
But I might be missing historical reasons around this as well. Probably
others from arm64 could help here.
Theoretically yes, practically no. If nobody cares about a
configuration, no need to make the code more complicated for that
configuration.
Meaning: are there any real use cases that actually build a kernel without
TRANSPARENT_HUGEPAGE and with ARM64_64K_PAGES/ARM64_16K_PAGES?
THP is always optional. Besides kernel builds without THP should always
be supported. Assuming that all builds will have THP enabled, might not
be accurate.
As builds are essentially broken, I assume this is not that relevant? Or how
long has it been broken?
Git blame shows that it's been there for some time now. But how does
that make this irrelevant ? A problem should be fixed nonetheless.
When exactly did I say not to fix it? I'm saying if nobody uses it, we
might be able to simplify.
It might be easier to just drop the "TRANSPARENT_HUGEPAGE" part from the
FORCE_MAX_ZONEORDER config.
Not sure if it would be a good idea to unnecessarily have larger MAX_ORDER
value for a given config. But I might be missing other contexts here.
My point is: keep it simple if there is no need to make it complicated.
If these arm64 variants are the only cases where we run into that issue
and nobody uses them ("hat it's been there for some time now"), why make
stuff complicated?
The current code seems to assume that HUGETLB_PAGE_ORDER <= MAX_ORDER.
Instead of changing that for optimizing an unused use case (it is
broken), just simplify the arm64 conditions. I'd even say add a
/*
* Some code assumes that HUGETLB_PAG
Re: [PATCH v13 02/15] iommu: Introduce bind/unbind_guest_msi
Hi Keqian,
On 2/1/21 12:52 PM, Keqian Zhu wrote:
> Hi Eric,
>
> On 2020/11/18 19:21, Eric Auger wrote:
>> On ARM, MSI are translated by the SMMU. An IOVA is allocated
>> for each MSI doorbell. If both the host and the guest are exposed
>> with SMMUs, we end up with 2 different IOVAs allocated by each.
>> guest allocates an IOVA (gIOVA) to map onto the guest MSI
>> doorbell (gDB). The Host allocates another IOVA (hIOVA) to map
>> onto the physical doorbell (hDB).
>>
>> So we end up with 2 untied mappings:
>> S1S2
>> gIOVA->gDB
>> hIOVA->hDB
>>
>> Currently the PCI device is programmed by the host with hIOVA
>> as MSI doorbell. So this does not work.
>>
>> This patch introduces an API to pass gIOVA/gDB to the host so
>> that gIOVA can be reused by the host instead of re-allocating
>> a new IOVA. So the goal is to create the following nested mapping:
> Does the gDB can be reused under non-nested mode?
Under non nested mode the hIOVA is allocated within the MSI reserved
region exposed by the SMMU driver, [0x800, 80f]. see
iommu_dma_prepare_msi/iommu_dma_get_msi_page in dma_iommu.c. this hIOVA
is programmed in the physical device so that the physical SMMU
translates it into the physical doorbell (hDB = host physical ITS
doorbell). The gDB is not used at pIOMMU programming level. It is only
used when setting up the KVM irq route.
Hope this answers your question.
>
>>
>> S1S2
>> gIOVA->gDB ->hDB
>>
>> and program the PCI device with gIOVA MSI doorbell.
>>
>> In case we have several devices attached to this nested domain
>> (devices belonging to the same group), they cannot be isolated
>> on guest side either. So they should also end up in the same domain
>> on guest side. We will enforce that all the devices attached to
>> the host iommu domain use the same physical doorbell and similarly
>> a single virtual doorbell mapping gets registered (1 single
>> virtual doorbell is used on guest as well).
>>
> [...]
>
>> + *
>> + * The associated IOVA can be reused by the host to create a nested
>> + * stage2 binding mapping translating into the physical doorbell used
>> + * by the devices attached to the domain.
>> + *
>> + * All devices within the domain must share the same physical doorbell.
>> + * A single MSI GIOVA/GPA mapping can be attached to an iommu_domain.
>> + */
>> +
>> +int iommu_bind_guest_msi(struct iommu_domain *domain,
>> + dma_addr_t giova, phys_addr_t gpa, size_t size)
>> +{
>> +if (unlikely(!domain->ops->bind_guest_msi))
>> +return -ENODEV;
>> +
>> +return domain->ops->bind_guest_msi(domain, giova, gpa, size);
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_bind_guest_msi);
>> +
>> +void iommu_unbind_guest_msi(struct iommu_domain *domain,
>> +dma_addr_t iova)
> nit: s/iova/giova
sure
>
>> +{
>> +if (unlikely(!domain->ops->unbind_guest_msi))
>> +return;
>> +
>> +domain->ops->unbind_guest_msi(domain, iova);
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_unbind_guest_msi);
>> +
> [...]
>
> Thanks,
> Keqian
>
Thanks
Eric
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
