An stage-2 STE requires a vmid that has been so far allocated per domain,
so arm_smmu_make_s2_domain_ste() has been extracting the vmid from the S2
domain.
To share an S2 parent domain across vSMMUs in the same VM, a vmid will be
no longer allocated for nor stored in the S2 domain, but per vSMMU, which
means the arm_smmu_make_s2_domain_ste() can get a vmid either from an S2
domain (non nesting parent) or a vSMMU.
Allow to pass in vmid explicitly to arm_smmu_make_s2_domain_ste(), giving
its callers a chance to pick the vmid between a domain or a vSMMU.
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 +-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 6 ++++--
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c | 3 ++-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 6 +++---
4 files changed, 10 insertions(+), 7 deletions(-)
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 bd9d7c85576a..e08c4ede4b2d 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -887,7 +887,7 @@ struct arm_smmu_entry_writer_ops {
void arm_smmu_make_abort_ste(struct arm_smmu_ste *target);
void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target,
struct arm_smmu_master *master,
- struct arm_smmu_domain *smmu_domain,
+ struct arm_smmu_domain *smmu_domain, u16 vmid,
bool ats_enabled);
#if IS_ENABLED(CONFIG_KUNIT)
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 5aa2e7af58b4..ff8b550159f2 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
@@ -34,8 +34,9 @@ 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->vsmmu->s2_parent, ats_enabled);
+ arm_smmu_make_s2_domain_ste(target, master,
+ nested_domain->vsmmu->s2_parent,
+ nested_domain->vsmmu->vmid, ats_enabled);
target->data[0] = cpu_to_le64(STRTAB_STE_0_V |
FIELD_PREP(STRTAB_STE_0_CFG,
@@ -76,6 +77,7 @@ static void arm_smmu_make_nested_domain_ste(
case STRTAB_STE_0_CFG_BYPASS:
arm_smmu_make_s2_domain_ste(target, master,
nested_domain->vsmmu->s2_parent,
+ nested_domain->vsmmu->vmid,
ats_enabled);
break;
case STRTAB_STE_0_CFG_ABORT:
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c
index d2671bfd3798..7fac5a112c5c 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c
@@ -316,7 +316,8 @@ static void arm_smmu_test_make_s2_ste(struct arm_smmu_ste *ste,
io_pgtable.cfg.arm_lpae_s2_cfg.vtcr.sl = 3;
io_pgtable.cfg.arm_lpae_s2_cfg.vtcr.tsz = 4;
- arm_smmu_make_s2_domain_ste(ste, &master, &smmu_domain, ats_enabled);
+ arm_smmu_make_s2_domain_ste(ste, &master, &smmu_domain,
+ smmu_domain.s2_cfg.vmid, ats_enabled);
}
static void arm_smmu_v3_write_ste_test_s2_to_abort(struct kunit *test)
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 358072b4e293..310bb4109ec9 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1656,10 +1656,9 @@ EXPORT_SYMBOL_IF_KUNIT(arm_smmu_make_cdtable_ste);
void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target,
struct arm_smmu_master *master,
- struct arm_smmu_domain *smmu_domain,
+ struct arm_smmu_domain *smmu_domain, u16 vmid,
bool ats_enabled)
{
- struct arm_smmu_s2_cfg *s2_cfg = &smmu_domain->s2_cfg;
const struct io_pgtable_cfg *pgtbl_cfg =
&io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops)->cfg;
typeof(&pgtbl_cfg->arm_lpae_s2_cfg.vtcr) vtcr =
@@ -1690,7 +1689,7 @@ void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target,
FIELD_PREP(STRTAB_STE_2_VTCR_S2TG, vtcr->tg) |
FIELD_PREP(STRTAB_STE_2_VTCR_S2PS, vtcr->ps);
target->data[2] = cpu_to_le64(
- FIELD_PREP(STRTAB_STE_2_S2VMID, s2_cfg->vmid) |
+ FIELD_PREP(STRTAB_STE_2_S2VMID, vmid) |
FIELD_PREP(STRTAB_STE_2_VTCR, vtcr_val) |
STRTAB_STE_2_S2AA64 |
#ifdef __BIG_ENDIAN
@@ -2969,6 +2968,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
}
case ARM_SMMU_DOMAIN_S2:
arm_smmu_make_s2_domain_ste(&target, master, smmu_domain,
+ smmu_domain->s2_cfg.vmid,
state.ats_enabled);
arm_smmu_install_ste_for_dev(master, &target);
arm_smmu_clear_cd(master, IOMMU_NO_PASID);
--
2.43.0On Tue, Mar 04, 2025 at 09:04:00PM -0800, Nicolin Chen wrote: > An stage-2 STE requires a vmid that has been so far allocated per domain, > so arm_smmu_make_s2_domain_ste() has been extracting the vmid from the S2 > domain. > > To share an S2 parent domain across vSMMUs in the same VM, a vmid will be > no longer allocated for nor stored in the S2 domain, but per vSMMU, which > means the arm_smmu_make_s2_domain_ste() can get a vmid either from an S2 > domain (non nesting parent) or a vSMMU. > > Allow to pass in vmid explicitly to arm_smmu_make_s2_domain_ste(), giving > its callers a chance to pick the vmid between a domain or a vSMMU. > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> Reviewed-by: Pranjal Shrivastava <praan@google.com> > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 +- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 6 ++++-- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c | 3 ++- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 6 +++--- > 4 files changed, 10 insertions(+), 7 deletions(-) > Thanks
On Tue, Mar 04, 2025 at 09:04:00PM -0800, Nicolin Chen wrote: > An stage-2 STE requires a vmid that has been so far allocated per domain, > so arm_smmu_make_s2_domain_ste() has been extracting the vmid from the S2 > domain. > > To share an S2 parent domain across vSMMUs in the same VM, a vmid will be > no longer allocated for nor stored in the S2 domain, but per vSMMU, which > means the arm_smmu_make_s2_domain_ste() can get a vmid either from an S2 > domain (non nesting parent) or a vSMMU. > > Allow to pass in vmid explicitly to arm_smmu_make_s2_domain_ste(), giving > its callers a chance to pick the vmid between a domain or a vSMMU. > > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 +- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 6 ++++-- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c | 3 ++- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 6 +++--- > 4 files changed, 10 insertions(+), 7 deletions(-) Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Jason
Hi Nicolin,
Thanks for sending out this series. This might as well help me to rework my pinned KVM
VMID series here,
https://lore.kernel.org/linux-iommu/20240208151837.35068-1-shameerali.kolothum.thodi@huawei.com/
> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: Wednesday, March 5, 2025 5:04 AM
> To: will@kernel.org; robin.murphy@arm.com; jgg@nvidia.com
> Cc: joro@8bytes.org; linux-arm-kernel@lists.infradead.org;
> iommu@lists.linux.dev; linux-kernel@vger.kernel.org; Shameerali Kolothum
> Thodi <shameerali.kolothum.thodi@huawei.com>
> Subject: [PATCH v1 1/4] iommu/arm-smmu-v3: Pass in vmid to
> arm_smmu_make_s2_domain_ste()
>
> An stage-2 STE requires a vmid that has been so far allocated per domain,
> so arm_smmu_make_s2_domain_ste() has been extracting the vmid from
> the S2
> domain.
>
> To share an S2 parent domain across vSMMUs in the same VM, a vmid will
> be
> no longer allocated for nor stored in the S2 domain, but per vSMMU, which
> means the arm_smmu_make_s2_domain_ste() can get a vmid either from
> an S2
> domain (non nesting parent) or a vSMMU.
>
> Allow to pass in vmid explicitly to arm_smmu_make_s2_domain_ste(),
> giving
> its callers a chance to pick the vmid between a domain or a vSMMU.
>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 +-
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 6 ++++--
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-test.c | 3 ++-
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 6 +++---
> 4 files changed, 10 insertions(+), 7 deletions(-)
>
> 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 bd9d7c85576a..e08c4ede4b2d 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -887,7 +887,7 @@ struct arm_smmu_entry_writer_ops {
> void arm_smmu_make_abort_ste(struct arm_smmu_ste *target);
> void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target,
> struct arm_smmu_master *master,
> - struct arm_smmu_domain *smmu_domain,
> + struct arm_smmu_domain *smmu_domain,
> u16 vmid,
> bool ats_enabled);
Now that vmid is an input, do we need some kind of validation here as
at least vmid = 0 is reserved I guess for bypass STEs.
Thanks,
Shameer
On Wed, Mar 05, 2025 at 08:50:17AM +0000, Shameerali Kolothum Thodi wrote:
> > 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 bd9d7c85576a..e08c4ede4b2d 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> > @@ -887,7 +887,7 @@ struct arm_smmu_entry_writer_ops {
> > void arm_smmu_make_abort_ste(struct arm_smmu_ste *target);
> > void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target,
> > struct arm_smmu_master *master,
> > - struct arm_smmu_domain *smmu_domain,
> > + struct arm_smmu_domain *smmu_domain,
> > u16 vmid,
> > bool ats_enabled);
>
> Now that vmid is an input, do we need some kind of validation here as
> at least vmid = 0 is reserved I guess for bypass STEs.
Perhaps it should do a WARN_ON_ONCE(!vmid), as it doesn't make
sense for a caller to make an S2-bypass STE with this function.
Thanks
Nicolin
On Wed, Mar 05, 2025 at 09:44:30AM -0800, Nicolin Chen wrote:
> On Wed, Mar 05, 2025 at 08:50:17AM +0000, Shameerali Kolothum Thodi wrote:
> > > 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 bd9d7c85576a..e08c4ede4b2d 100644
> > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> > > @@ -887,7 +887,7 @@ struct arm_smmu_entry_writer_ops {
> > > void arm_smmu_make_abort_ste(struct arm_smmu_ste *target);
> > > void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target,
> > > struct arm_smmu_master *master,
> > > - struct arm_smmu_domain *smmu_domain,
> > > + struct arm_smmu_domain *smmu_domain,
> > > u16 vmid,
> > > bool ats_enabled);
> >
> > Now that vmid is an input, do we need some kind of validation here as
> > at least vmid = 0 is reserved I guess for bypass STEs.
>
> Perhaps it should do a WARN_ON_ONCE(!vmid), as it doesn't make
> sense for a caller to make an S2-bypass STE with this function.
>
+1, a warning should suffice as the caller isn't expected to call this
with vmid = 0. For the s2 bypass case, we already set vmid field to 0 in
make_cdtable_ste for clients who *really* want the vmid = 0.
> Thanks
> Nicolin
>
Thanks
Praan
© 2016 - 2026 Red Hat, Inc.