| Message ID | 214b10db02f1046efdc70e2c4803111357f60070.1741150594.git.nicolinc@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | iommu/arm-smmu-v3: Allocate vmid per vsmmu instead of s2_parent | expand |
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 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
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
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);
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(-)