| Message ID | 3-v4-c93b774edcc4+42d2b-smmuv3_newapi_p1_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Update SMMUv3 to the modern iommu API (part 1/3) | expand |
> -----Original Message----- > From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Thursday, January 25, 2024 11:57 PM > To: iommu@lists.linux.dev; Joerg Roedel <joro@8bytes.org>; linux-arm- > kernel@lists.infradead.org; Robin Murphy <robin.murphy@arm.com>; Will > Deacon <will@kernel.org> > Cc: Moritz Fischer <mdf@kernel.org>; Moritz Fischer <moritzf@google.com>; > Michael Shavit <mshavit@google.com>; Nicolin Chen <nicolinc@nvidia.com>; > patches@lists.linux.dev; Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@huawei.com> > Subject: [PATCH v4 03/16] iommu/arm-smmu-v3: Move > arm_smmu_rmr_install_bypass_ste() > > Logically arm_smmu_init_strtab_linear() is the function that allocates and > populates the stream table with the initial value of the STEs. After this > function returns the stream table should be fully ready. > > arm_smmu_rmr_install_bypass_ste() adjusts the initial stream table to force > any SIDs that the FW says have IOMMU_RESV_DIRECT to use bypass. This > ensures there is no disruption to the identity mapping during boot. > > Put arm_smmu_rmr_install_bypass_ste() into arm_smmu_init_strtab_linear(), > it already executes immediately after arm_smmu_init_strtab_linear(). > > No functional change intended. I think this actually changes the behavior and will cause regression as we now install rmr sids only for linear stream table not for SMMUv3 with 2-level stream table supported. Please check. Thanks, Shameer > > Reviewed-by: Michael Shavit <mshavit@google.com> > Reviewed-by: Nicolin Chen <nicolinc@nvidia.com> > Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > Tested-by: Nicolin Chen <nicolinc@nvidia.com> > Tested-by: Moritz Fischer <moritzf@google.com> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 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 38bcb4ed1fccc1..df8fc7b87a7907 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > @@ -102,6 +102,8 @@ static struct arm_smmu_option_prop > arm_smmu_options[] = { > { 0, NULL}, > }; > > +static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device > *smmu); > + > static void parse_driver_options(struct arm_smmu_device *smmu) > { > int i = 0; > @@ -3250,6 +3252,9 @@ static int arm_smmu_init_strtab_linear(struct > arm_smmu_device *smmu) > cfg->strtab_base_cfg = reg; > > arm_smmu_init_bypass_stes(strtab, cfg->num_l1_ents); > + > + /* Check for RMRs and install bypass STEs if any */ > + arm_smmu_rmr_install_bypass_ste(smmu); > return 0; > } > > @@ -4063,9 +4068,6 @@ static int arm_smmu_device_probe(struct > platform_device *pdev) > /* Record our private device structure */ > platform_set_drvdata(pdev, smmu); > > - /* Check for RMRs and install bypass STEs if any */ > - arm_smmu_rmr_install_bypass_ste(smmu); > - > /* Reset the device */ > ret = arm_smmu_device_reset(smmu, bypass); > if (ret) > -- > 2.43.0
On Mon, Jan 29, 2024 at 03:07:21PM +0000, Shameerali Kolothum Thodi wrote: > > Logically arm_smmu_init_strtab_linear() is the function that allocates and > > populates the stream table with the initial value of the STEs. After this > > function returns the stream table should be fully ready. > > > > arm_smmu_rmr_install_bypass_ste() adjusts the initial stream table to force > > any SIDs that the FW says have IOMMU_RESV_DIRECT to use bypass. This > > ensures there is no disruption to the identity mapping during boot. > > > > Put arm_smmu_rmr_install_bypass_ste() into arm_smmu_init_strtab_linear(), > > it already executes immediately after arm_smmu_init_strtab_linear(). > > > > No functional change intended. > > I think this actually changes the behavior and will cause regression as we > now install rmr sids only for linear stream table not for SMMUv3 with > 2-level stream table supported. Oh you are right, it should be in arm_smmu_init_strtab() Thanks! Jason
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 38bcb4ed1fccc1..df8fc7b87a7907 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -102,6 +102,8 @@ static struct arm_smmu_option_prop arm_smmu_options[] = { { 0, NULL}, }; +static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu); + static void parse_driver_options(struct arm_smmu_device *smmu) { int i = 0; @@ -3250,6 +3252,9 @@ static int arm_smmu_init_strtab_linear(struct arm_smmu_device *smmu) cfg->strtab_base_cfg = reg; arm_smmu_init_bypass_stes(strtab, cfg->num_l1_ents); + + /* Check for RMRs and install bypass STEs if any */ + arm_smmu_rmr_install_bypass_ste(smmu); return 0; } @@ -4063,9 +4068,6 @@ static int arm_smmu_device_probe(struct platform_device *pdev) /* Record our private device structure */ platform_set_drvdata(pdev, smmu); - /* Check for RMRs and install bypass STEs if any */ - arm_smmu_rmr_install_bypass_ste(smmu); - /* Reset the device */ ret = arm_smmu_device_reset(smmu, bypass); if (ret)