[v5,5/8] iommu/arm-smmu-v3: Add bypass flag to arm_smmu_write_strtab_ent()

Message ID 20210524110222.2212-6-shameerali.kolothum.thodi@huawei.com
State New
Headers show
Series
  • ACPI/IORT: Support for IORT RMR node
Related show

Commit Message

Shameerali Kolothum Thodi May 24, 2021, 11:02 a.m.
By default, disable_bypass is set and any dev without an iommu domain
installs STE with CFG_ABORT during arm_smmu_init_bypass_stes(). Introduce
a "bypass" flag to arm_smmu_write_strtab_ent() so that we can force it to
install CFG_BYPASS STE for specific SIDs. This will be useful in follow
up patch to install bypass for IORT RMR SIDs.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>

---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.17.1

Comments

Robin Murphy June 14, 2021, 10:23 a.m. | #1
On 2021-05-24 12:02, Shameer Kolothum wrote:
> By default, disable_bypass is set and any dev without an iommu domain

> installs STE with CFG_ABORT during arm_smmu_init_bypass_stes(). Introduce

> a "bypass" flag to arm_smmu_write_strtab_ent() so that we can force it to

> install CFG_BYPASS STE for specific SIDs. This will be useful in follow

> up patch to install bypass for IORT RMR SIDs.

> 

> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>

> ---

>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 ++++----

>   1 file changed, 4 insertions(+), 4 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 754bad6092c1..f9195b740f48 100644

> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

> @@ -1174,7 +1174,7 @@ static void arm_smmu_sync_ste_for_sid(struct arm_smmu_device *smmu, u32 sid)

>   }

>   

>   static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,

> -				      __le64 *dst)

> +				      __le64 *dst, bool bypass)

>   {

>   	/*

>   	 * This is hideously complicated, but we only really care about

> @@ -1245,7 +1245,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,

>   

>   	/* Bypass/fault */

>   	if (!smmu_domain || !(s1_cfg || s2_cfg)) {

> -		if (!smmu_domain && disable_bypass)

> +		if (!smmu_domain && disable_bypass && !bypass)


Umm...

>   			val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_ABORT);

>   		else

>   			val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_BYPASS);

> @@ -1320,7 +1320,7 @@ static void arm_smmu_init_bypass_stes(__le64 *strtab, unsigned int nent)

>   	unsigned int i;

>   

>   	for (i = 0; i < nent; ++i) {

> -		arm_smmu_write_strtab_ent(NULL, -1, strtab);

> +		arm_smmu_write_strtab_ent(NULL, -1, strtab, false);


...and in particular, an operation named "init_bypass_stes" passing 
bypass=false is just breaking my brain. Can we pull the logic for 
default bypass/fault out to here as part of the refactoring so that it 
actually makes sense?

Robin.

>   		strtab += STRTAB_STE_DWORDS;

>   	}

>   }

> @@ -2097,7 +2097,7 @@ static void arm_smmu_install_ste_for_dev(struct arm_smmu_master *master)

>   		if (j < i)

>   			continue;

>   

> -		arm_smmu_write_strtab_ent(master, sid, step);

> +		arm_smmu_write_strtab_ent(master, sid, step, false);

>   	}

>   }

>   

>
Shameerali Kolothum Thodi June 14, 2021, 12:51 p.m. | #2
> -----Original Message-----

> From: Robin Murphy [mailto:robin.murphy@arm.com]

> Sent: 14 June 2021 11:23

> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;

> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;

> iommu@lists.linux-foundation.org

> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com;

> joro@8bytes.org; wanghuiqiang <wanghuiqiang@huawei.com>; Guohanjun

> (Hanjun Guo) <guohanjun@huawei.com>; steven.price@arm.com;

> Sami.Mujawar@arm.com; jon@solid-run.com; eric.auger@redhat.com;

> yangyicong <yangyicong@huawei.com>

> Subject: Re: [PATCH v5 5/8] iommu/arm-smmu-v3: Add bypass flag

> to arm_smmu_write_strtab_ent()

> 

> On 2021-05-24 12:02, Shameer Kolothum wrote:

> > By default, disable_bypass is set and any dev without an iommu domain

> > installs STE with CFG_ABORT during arm_smmu_init_bypass_stes().

> Introduce

> > a "bypass" flag to arm_smmu_write_strtab_ent() so that we can force it to

> > install CFG_BYPASS STE for specific SIDs. This will be useful in follow

> > up patch to install bypass for IORT RMR SIDs.

> >

> > Signed-off-by: Shameer Kolothum

> <shameerali.kolothum.thodi@huawei.com>

> > ---

> >   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 8 ++++----

> >   1 file changed, 4 insertions(+), 4 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 754bad6092c1..f9195b740f48 100644

> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

> > @@ -1174,7 +1174,7 @@ static void arm_smmu_sync_ste_for_sid(struct

> arm_smmu_device *smmu, u32 sid)

> >   }

> >

> >   static void arm_smmu_write_strtab_ent(struct arm_smmu_master

> *master, u32 sid,

> > -				      __le64 *dst)

> > +				      __le64 *dst, bool bypass)

> >   {

> >   	/*

> >   	 * This is hideously complicated, but we only really care about

> > @@ -1245,7 +1245,7 @@ static void arm_smmu_write_strtab_ent(struct

> arm_smmu_master *master, u32 sid,

> >

> >   	/* Bypass/fault */

> >   	if (!smmu_domain || !(s1_cfg || s2_cfg)) {

> > -		if (!smmu_domain && disable_bypass)

> > +		if (!smmu_domain && disable_bypass && !bypass)

> 

> Umm...

> 

> >   			val |= FIELD_PREP(STRTAB_STE_0_CFG,

> STRTAB_STE_0_CFG_ABORT);

> >   		else

> >   			val |= FIELD_PREP(STRTAB_STE_0_CFG,

> STRTAB_STE_0_CFG_BYPASS);

> > @@ -1320,7 +1320,7 @@ static void arm_smmu_init_bypass_stes(__le64

> *strtab, unsigned int nent)

> >   	unsigned int i;

> >

> >   	for (i = 0; i < nent; ++i) {

> > -		arm_smmu_write_strtab_ent(NULL, -1, strtab);

> > +		arm_smmu_write_strtab_ent(NULL, -1, strtab, false);

> 

> ...and in particular, an operation named "init_bypass_stes" passing

> bypass=false is just breaking my brain. Can we pull the logic for

> default bypass/fault out to here as part of the refactoring so that it

> actually makes sense?


Agree, it is confusing. Will add the default bypass/fault logic here.

Thanks,
Shameer

> 

> Robin.

> 

> >   		strtab += STRTAB_STE_DWORDS;

> >   	}

> >   }

> > @@ -2097,7 +2097,7 @@ static void arm_smmu_install_ste_for_dev(struct

> arm_smmu_master *master)

> >   		if (j < i)

> >   			continue;

> >

> > -		arm_smmu_write_strtab_ent(master, sid, step);

> > +		arm_smmu_write_strtab_ent(master, sid, step, false);

> >   	}

> >   }

> >

> >

Patch

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 754bad6092c1..f9195b740f48 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1174,7 +1174,7 @@  static void arm_smmu_sync_ste_for_sid(struct arm_smmu_device *smmu, u32 sid)
 }
 
 static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
-				      __le64 *dst)
+				      __le64 *dst, bool bypass)
 {
 	/*
 	 * This is hideously complicated, but we only really care about
@@ -1245,7 +1245,7 @@  static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
 
 	/* Bypass/fault */
 	if (!smmu_domain || !(s1_cfg || s2_cfg)) {
-		if (!smmu_domain && disable_bypass)
+		if (!smmu_domain && disable_bypass && !bypass)
 			val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_ABORT);
 		else
 			val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_BYPASS);
@@ -1320,7 +1320,7 @@  static void arm_smmu_init_bypass_stes(__le64 *strtab, unsigned int nent)
 	unsigned int i;
 
 	for (i = 0; i < nent; ++i) {
-		arm_smmu_write_strtab_ent(NULL, -1, strtab);
+		arm_smmu_write_strtab_ent(NULL, -1, strtab, false);
 		strtab += STRTAB_STE_DWORDS;
 	}
 }
@@ -2097,7 +2097,7 @@  static void arm_smmu_install_ste_for_dev(struct arm_smmu_master *master)
 		if (j < i)
 			continue;
 
-		arm_smmu_write_strtab_ent(master, sid, step);
+		arm_smmu_write_strtab_ent(master, sid, step, false);
 	}
 }