[PATCH 0/8] iommu/arm-smmu: bugfixs and add support for non-pci devices

[PATCH 0/8] iommu/arm-smmu: bugfixs and add support for non-pci devices

[Zhen Lei]

As Documentation\devicetree\bindings\iommu\iommu.txt mentioned, a master may
belongs to many SMMUs or have more than one device-id(stream id). But on current
arm/arm64 platforms, a master with only one stream id. So I directly add two
members(of_smmu and device-id) in struct dev_archdata, and only support a master
with only one stream id. If some platforms that a master belongs to more than one
SMMUs or have more than one device-id, we should dynamic memory allocation to record
all information.

Zhen Lei (8):
  iommu/arm-smmu: fix the assignment of log2size field
  iommu/arm-smmu: fix the index calculation of strtab
  iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and
    ARM64_TCR_ORGN0_SHIFT
  iommu/arm-smmu: set EPD1 to disable TT1 translation table walk
  iommu/arm-smmu: rename __arm_smmu_get_pci_sid
  iommu/arm-smmu: add support for non-pci devices
  iommu/arm-smmu: enlarge STRTAB_L1_SZ_SHIFT to support larger sidsize
  iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG
    execution

 arch/arm64/include/asm/device.h |   2 +
 drivers/iommu/arm-smmu-v3.c     | 135 +++++++++++++++++++++++++++++++++-------
 2 files changed, 116 insertions(+), 21 deletions(-)

--
1.8.0

[PATCH 0/8] iommu/arm-smmu: bugfixs and add support for non-pci devices

[Zhen Lei]

As Documentation\devicetree\bindings\iommu\iommu.txt mentioned, a master may
belongs to many SMMUs or have more than one device-id(stream id). But on current
arm/arm64 platforms, a master with only one stream id. So I directly add two
members(of_smmu and device-id) in struct dev_archdata, and only support a master
with only one stream id. If some platforms that a master belongs to more than one
SMMUs or have more than one device-id, we should dynamic memory allocation to record
all information.

Zhen Lei (8):
  iommu/arm-smmu: fix the assignment of log2size field
  iommu/arm-smmu: fix the index calculation of strtab
  iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and
    ARM64_TCR_ORGN0_SHIFT
  iommu/arm-smmu: set EPD1 to disable TT1 translation table walk
  iommu/arm-smmu: rename __arm_smmu_get_pci_sid
  iommu/arm-smmu: add support for non-pci devices
  iommu/arm-smmu: enlarge STRTAB_L1_SZ_SHIFT to support larger sidsize
  iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG
    execution

 arch/arm64/include/asm/device.h |   2 +
 drivers/iommu/arm-smmu-v3.c     | 135 +++++++++++++++++++++++++++++++++-------
 2 files changed, 116 insertions(+), 21 deletions(-)

--
1.8.0

[PATCH 1/8] iommu/arm-smmu: fix the assignment of log2size field

[Zhen Lei]

Add a new local variable to store the value of log2size, so that it will
not be overridden by L1 table size.

Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index f141301..ba7fe2d 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2021,19 +2021,19 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
 {
 	void *strtab;
 	u64 reg;
-	u32 size;
+	u32 size, log2size;
 	int ret;
 	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;

 	/* Calculate the L1 size, capped to the SIDSIZE */
-	size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
-	size = min(size, smmu->sid_bits - STRTAB_SPLIT);
-	if (size + STRTAB_SPLIT < smmu->sid_bits)
+	log2size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
+	log2size = min(log2size, smmu->sid_bits - STRTAB_SPLIT);
+	if (log2size + STRTAB_SPLIT < smmu->sid_bits)
 		dev_warn(smmu->dev,
 			 "2-level strtab only covers %u/%u bits of SID\n",
-			 size + STRTAB_SPLIT, smmu->sid_bits);
+			 log2size + STRTAB_SPLIT, smmu->sid_bits);

-	cfg->num_l1_ents = 1 << size;
+	cfg->num_l1_ents = 1 << log2size;
 	size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
 	strtab = dma_zalloc_coherent(smmu->dev, size, &cfg->strtab_dma,
 				     GFP_KERNEL);
@@ -2047,7 +2047,7 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)

 	/* Configure strtab_base_cfg for 2 levels */
 	reg  = STRTAB_BASE_CFG_FMT_2LVL;
-	reg |= (size & STRTAB_BASE_CFG_LOG2SIZE_MASK)
+	reg |= (log2size & STRTAB_BASE_CFG_LOG2SIZE_MASK)
 		<< STRTAB_BASE_CFG_LOG2SIZE_SHIFT;
 	reg |= (STRTAB_SPLIT & STRTAB_BASE_CFG_SPLIT_MASK)
 		<< STRTAB_BASE_CFG_SPLIT_SHIFT;
--
1.8.0

[PATCH 1/8] iommu/arm-smmu: fix the assignment of log2size field

[Zhen Lei]

Add a new local variable to store the value of log2size, so that it will
not be overridden by L1 table size.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/iommu/arm-smmu-v3.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index f141301..ba7fe2d 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2021,19 +2021,19 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
 {
 	void *strtab;
 	u64 reg;
-	u32 size;
+	u32 size, log2size;
 	int ret;
 	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;

 	/* Calculate the L1 size, capped to the SIDSIZE */
-	size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
-	size = min(size, smmu->sid_bits - STRTAB_SPLIT);
-	if (size + STRTAB_SPLIT < smmu->sid_bits)
+	log2size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
+	log2size = min(log2size, smmu->sid_bits - STRTAB_SPLIT);
+	if (log2size + STRTAB_SPLIT < smmu->sid_bits)
 		dev_warn(smmu->dev,
 			 "2-level strtab only covers %u/%u bits of SID\n",
-			 size + STRTAB_SPLIT, smmu->sid_bits);
+			 log2size + STRTAB_SPLIT, smmu->sid_bits);

-	cfg->num_l1_ents = 1 << size;
+	cfg->num_l1_ents = 1 << log2size;
 	size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
 	strtab = dma_zalloc_coherent(smmu->dev, size, &cfg->strtab_dma,
 				     GFP_KERNEL);
@@ -2047,7 +2047,7 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)

 	/* Configure strtab_base_cfg for 2 levels */
 	reg  = STRTAB_BASE_CFG_FMT_2LVL;
-	reg |= (size & STRTAB_BASE_CFG_LOG2SIZE_MASK)
+	reg |= (log2size & STRTAB_BASE_CFG_LOG2SIZE_MASK)
 		<< STRTAB_BASE_CFG_LOG2SIZE_SHIFT;
 	reg |= (STRTAB_SPLIT & STRTAB_BASE_CFG_SPLIT_MASK)
 		<< STRTAB_BASE_CFG_SPLIT_SHIFT;
--
1.8.0

Re: [PATCH 1/8] iommu/arm-smmu: fix the assignment of log2size field

[Will Deacon]

Hi Zhen Lei,

On Fri, Jun 26, 2015 at 09:32:57AM +0100, Zhen Lei wrote:
> Add a new local variable to store the value of log2size, so that it will
> not be overridden by L1 table size.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu-v3.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index f141301..ba7fe2d 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2021,19 +2021,19 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
>  {
>  	void *strtab;
>  	u64 reg;
> -	u32 size;
> +	u32 size, log2size;
>  	int ret;
>  	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
> 
>  	/* Calculate the L1 size, capped to the SIDSIZE */
> -	size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
> -	size = min(size, smmu->sid_bits - STRTAB_SPLIT);
> -	if (size + STRTAB_SPLIT < smmu->sid_bits)
> +	log2size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
> +	log2size = min(log2size, smmu->sid_bits - STRTAB_SPLIT);
> +	if (log2size + STRTAB_SPLIT < smmu->sid_bits)

I still think this is wrong. LOG2SIZE should correspond to the whole
stream-table, not just the first level. How about the (totally untested)
patch below, instead?

Will

--->8

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 8e9ec81ce4bb..1942a45f68b2 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2020,21 +2020,23 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
 {
 	void *strtab;
 	u64 reg;
-	u32 size;
+	u32 size, l1size;
 	int ret;
 	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
 
 	/* Calculate the L1 size, capped to the SIDSIZE */
 	size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
 	size = min(size, smmu->sid_bits - STRTAB_SPLIT);
-	if (size + STRTAB_SPLIT < smmu->sid_bits)
+	cfg->num_l1_ents = 1 << size;
+
+	size += STRTAB_SPLIT;
+	if (size < smmu->sid_bits)
 		dev_warn(smmu->dev,
 			 "2-level strtab only covers %u/%u bits of SID\n",
-			 size + STRTAB_SPLIT, smmu->sid_bits);
+			 size, smmu->sid_bits);
 
-	cfg->num_l1_ents = 1 << size;
-	size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
-	strtab = dma_zalloc_coherent(smmu->dev, size, &cfg->strtab_dma,
+	l1size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
+	strtab = dma_zalloc_coherent(smmu->dev, l1size, &cfg->strtab_dma,
 				     GFP_KERNEL);
 	if (!strtab) {
 		dev_err(smmu->dev,
@@ -2055,8 +2057,7 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
 	ret = arm_smmu_init_l1_strtab(smmu);
 	if (ret)
 		dma_free_coherent(smmu->dev,
-				  cfg->num_l1_ents *
-				  (STRTAB_L1_DESC_DWORDS << 3),
+				  l1size,
 				  strtab,
 				  cfg->strtab_dma);
 	return ret;

[PATCH 1/8] iommu/arm-smmu: fix the assignment of log2size field

[Will Deacon]

Hi Zhen Lei,

On Fri, Jun 26, 2015 at 09:32:57AM +0100, Zhen Lei wrote:
> Add a new local variable to store the value of log2size, so that it will
> not be overridden by L1 table size.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index f141301..ba7fe2d 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2021,19 +2021,19 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
>  {
>  	void *strtab;
>  	u64 reg;
> -	u32 size;
> +	u32 size, log2size;
>  	int ret;
>  	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
> 
>  	/* Calculate the L1 size, capped to the SIDSIZE */
> -	size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
> -	size = min(size, smmu->sid_bits - STRTAB_SPLIT);
> -	if (size + STRTAB_SPLIT < smmu->sid_bits)
> +	log2size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
> +	log2size = min(log2size, smmu->sid_bits - STRTAB_SPLIT);
> +	if (log2size + STRTAB_SPLIT < smmu->sid_bits)

I still think this is wrong. LOG2SIZE should correspond to the whole
stream-table, not just the first level. How about the (totally untested)
patch below, instead?

Will

--->8

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 8e9ec81ce4bb..1942a45f68b2 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2020,21 +2020,23 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
 {
 	void *strtab;
 	u64 reg;
-	u32 size;
+	u32 size, l1size;
 	int ret;
 	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
 
 	/* Calculate the L1 size, capped to the SIDSIZE */
 	size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
 	size = min(size, smmu->sid_bits - STRTAB_SPLIT);
-	if (size + STRTAB_SPLIT < smmu->sid_bits)
+	cfg->num_l1_ents = 1 << size;
+
+	size += STRTAB_SPLIT;
+	if (size < smmu->sid_bits)
 		dev_warn(smmu->dev,
 			 "2-level strtab only covers %u/%u bits of SID\n",
-			 size + STRTAB_SPLIT, smmu->sid_bits);
+			 size, smmu->sid_bits);
 
-	cfg->num_l1_ents = 1 << size;
-	size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
-	strtab = dma_zalloc_coherent(smmu->dev, size, &cfg->strtab_dma,
+	l1size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
+	strtab = dma_zalloc_coherent(smmu->dev, l1size, &cfg->strtab_dma,
 				     GFP_KERNEL);
 	if (!strtab) {
 		dev_err(smmu->dev,
@@ -2055,8 +2057,7 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
 	ret = arm_smmu_init_l1_strtab(smmu);
 	if (ret)
 		dma_free_coherent(smmu->dev,
-				  cfg->num_l1_ents *
-				  (STRTAB_L1_DESC_DWORDS << 3),
+				  l1size,
 				  strtab,
 				  cfg->strtab_dma);
 	return ret;

Re: [PATCH 1/8] iommu/arm-smmu: fix the assignment of log2size field

[leizhen]

On 2015/6/30 1:05, Will Deacon wrote:
> Hi Zhen Lei,
> 
> On Fri, Jun 26, 2015 at 09:32:57AM +0100, Zhen Lei wrote:
>> Add a new local variable to store the value of log2size, so that it will
>> not be overridden by L1 table size.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index f141301..ba7fe2d 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -2021,19 +2021,19 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
>>  {
>>  	void *strtab;
>>  	u64 reg;
>> -	u32 size;
>> +	u32 size, log2size;
>>  	int ret;
>>  	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
>>
>>  	/* Calculate the L1 size, capped to the SIDSIZE */
>> -	size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
>> -	size = min(size, smmu->sid_bits - STRTAB_SPLIT);
>> -	if (size + STRTAB_SPLIT < smmu->sid_bits)
>> +	log2size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
>> +	log2size = min(log2size, smmu->sid_bits - STRTAB_SPLIT);
>> +	if (log2size + STRTAB_SPLIT < smmu->sid_bits)
> 
> I still think this is wrong. LOG2SIZE should correspond to the whole
> stream-table, not just the first level. How about the (totally untested)
> patch below, instead?

I tested the patch below, It's passed.

I use log2size, because this is the field name in SMMU_STRTAB_BASE_CFG. In fact,
it means log2(entries). Yes, I also think log2size is confused. So, I prefer yours.

> 
> Will
> 
> --->8
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 8e9ec81ce4bb..1942a45f68b2 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2020,21 +2020,23 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
>  {
>  	void *strtab;
>  	u64 reg;
> -	u32 size;
> +	u32 size, l1size;
>  	int ret;
>  	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
>  
>  	/* Calculate the L1 size, capped to the SIDSIZE */
>  	size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
>  	size = min(size, smmu->sid_bits - STRTAB_SPLIT);
> -	if (size + STRTAB_SPLIT < smmu->sid_bits)
> +	cfg->num_l1_ents = 1 << size;
> +
> +	size += STRTAB_SPLIT;
> +	if (size < smmu->sid_bits)
>  		dev_warn(smmu->dev,
>  			 "2-level strtab only covers %u/%u bits of SID\n",
> -			 size + STRTAB_SPLIT, smmu->sid_bits);
> +			 size, smmu->sid_bits);
>  
> -	cfg->num_l1_ents = 1 << size;
> -	size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
> -	strtab = dma_zalloc_coherent(smmu->dev, size, &cfg->strtab_dma,
> +	l1size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
> +	strtab = dma_zalloc_coherent(smmu->dev, l1size, &cfg->strtab_dma,
>  				     GFP_KERNEL);
>  	if (!strtab) {
>  		dev_err(smmu->dev,
> @@ -2055,8 +2057,7 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
>  	ret = arm_smmu_init_l1_strtab(smmu);
>  	if (ret)
>  		dma_free_coherent(smmu->dev,
> -				  cfg->num_l1_ents *
> -				  (STRTAB_L1_DESC_DWORDS << 3),
> +				  l1size,
>  				  strtab,
>  				  cfg->strtab_dma);
>  	return ret;
> 
> .
> 

[PATCH 1/8] iommu/arm-smmu: fix the assignment of log2size field

[leizhen]

On 2015/6/30 1:05, Will Deacon wrote:
> Hi Zhen Lei,
> 
> On Fri, Jun 26, 2015 at 09:32:57AM +0100, Zhen Lei wrote:
>> Add a new local variable to store the value of log2size, so that it will
>> not be overridden by L1 table size.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index f141301..ba7fe2d 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -2021,19 +2021,19 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
>>  {
>>  	void *strtab;
>>  	u64 reg;
>> -	u32 size;
>> +	u32 size, log2size;
>>  	int ret;
>>  	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
>>
>>  	/* Calculate the L1 size, capped to the SIDSIZE */
>> -	size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
>> -	size = min(size, smmu->sid_bits - STRTAB_SPLIT);
>> -	if (size + STRTAB_SPLIT < smmu->sid_bits)
>> +	log2size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
>> +	log2size = min(log2size, smmu->sid_bits - STRTAB_SPLIT);
>> +	if (log2size + STRTAB_SPLIT < smmu->sid_bits)
> 
> I still think this is wrong. LOG2SIZE should correspond to the whole
> stream-table, not just the first level. How about the (totally untested)
> patch below, instead?

I tested the patch below, It's passed.

I use log2size, because this is the field name in SMMU_STRTAB_BASE_CFG. In fact,
it means log2(entries). Yes, I also think log2size is confused. So, I prefer yours.

> 
> Will
> 
> --->8
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 8e9ec81ce4bb..1942a45f68b2 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2020,21 +2020,23 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
>  {
>  	void *strtab;
>  	u64 reg;
> -	u32 size;
> +	u32 size, l1size;
>  	int ret;
>  	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
>  
>  	/* Calculate the L1 size, capped to the SIDSIZE */
>  	size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
>  	size = min(size, smmu->sid_bits - STRTAB_SPLIT);
> -	if (size + STRTAB_SPLIT < smmu->sid_bits)
> +	cfg->num_l1_ents = 1 << size;
> +
> +	size += STRTAB_SPLIT;
> +	if (size < smmu->sid_bits)
>  		dev_warn(smmu->dev,
>  			 "2-level strtab only covers %u/%u bits of SID\n",
> -			 size + STRTAB_SPLIT, smmu->sid_bits);
> +			 size, smmu->sid_bits);
>  
> -	cfg->num_l1_ents = 1 << size;
> -	size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
> -	strtab = dma_zalloc_coherent(smmu->dev, size, &cfg->strtab_dma,
> +	l1size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
> +	strtab = dma_zalloc_coherent(smmu->dev, l1size, &cfg->strtab_dma,
>  				     GFP_KERNEL);
>  	if (!strtab) {
>  		dev_err(smmu->dev,
> @@ -2055,8 +2057,7 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
>  	ret = arm_smmu_init_l1_strtab(smmu);
>  	if (ret)
>  		dma_free_coherent(smmu->dev,
> -				  cfg->num_l1_ents *
> -				  (STRTAB_L1_DESC_DWORDS << 3),
> +				  l1size,
>  				  strtab,
>  				  cfg->strtab_dma);
>  	return ret;
> 
> .
> 

[PATCH 2/8] iommu/arm-smmu: fix the index calculation of strtab

[Zhen Lei]

The element size of cfg->strtab is just one DWORD, should use multiply
operation.

Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index ba7fe2d..2a5f810 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1064,7 +1064,7 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
 		return 0;

 	size = 1 << (STRTAB_SPLIT + ilog2(STRTAB_STE_DWORDS) + 3);
-	strtab = &cfg->strtab[sid >> STRTAB_SPLIT << STRTAB_L1_DESC_DWORDS];
+	strtab = &cfg->strtab[(sid >> STRTAB_SPLIT) * STRTAB_L1_DESC_DWORDS];

 	desc->span = STRTAB_SPLIT + 1;
 	desc->l2ptr = dma_zalloc_coherent(smmu->dev, size, &desc->l2ptr_dma,
--
1.8.0

[PATCH 2/8] iommu/arm-smmu: fix the index calculation of strtab

[Zhen Lei]

The element size of cfg->strtab is just one DWORD, should use multiply
operation.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/iommu/arm-smmu-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index ba7fe2d..2a5f810 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1064,7 +1064,7 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
 		return 0;

 	size = 1 << (STRTAB_SPLIT + ilog2(STRTAB_STE_DWORDS) + 3);
-	strtab = &cfg->strtab[sid >> STRTAB_SPLIT << STRTAB_L1_DESC_DWORDS];
+	strtab = &cfg->strtab[(sid >> STRTAB_SPLIT) * STRTAB_L1_DESC_DWORDS];

 	desc->span = STRTAB_SPLIT + 1;
 	desc->l2ptr = dma_zalloc_coherent(smmu->dev, size, &desc->l2ptr_dma,
--
1.8.0

Re: [PATCH 2/8] iommu/arm-smmu: fix the index calculation of strtab

[Will Deacon]

On Fri, Jun 26, 2015 at 09:32:58AM +0100, Zhen Lei wrote:
> The element size of cfg->strtab is just one DWORD, should use multiply
> operation.

Nice catch. Turns out all my emulated PCI devices sit on bus 0, so this
didn't show up in testing.

Will

> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu-v3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index ba7fe2d..2a5f810 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -1064,7 +1064,7 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
>  		return 0;
> 
>  	size = 1 << (STRTAB_SPLIT + ilog2(STRTAB_STE_DWORDS) + 3);
> -	strtab = &cfg->strtab[sid >> STRTAB_SPLIT << STRTAB_L1_DESC_DWORDS];
> +	strtab = &cfg->strtab[(sid >> STRTAB_SPLIT) * STRTAB_L1_DESC_DWORDS];
> 
>  	desc->span = STRTAB_SPLIT + 1;
>  	desc->l2ptr = dma_zalloc_coherent(smmu->dev, size, &desc->l2ptr_dma,
> --
> 1.8.0
> 
> 

[PATCH 2/8] iommu/arm-smmu: fix the index calculation of strtab

[Will Deacon]

On Fri, Jun 26, 2015 at 09:32:58AM +0100, Zhen Lei wrote:
> The element size of cfg->strtab is just one DWORD, should use multiply
> operation.

Nice catch. Turns out all my emulated PCI devices sit on bus 0, so this
didn't show up in testing.

Will

> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index ba7fe2d..2a5f810 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -1064,7 +1064,7 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
>  		return 0;
> 
>  	size = 1 << (STRTAB_SPLIT + ilog2(STRTAB_STE_DWORDS) + 3);
> -	strtab = &cfg->strtab[sid >> STRTAB_SPLIT << STRTAB_L1_DESC_DWORDS];
> +	strtab = &cfg->strtab[(sid >> STRTAB_SPLIT) * STRTAB_L1_DESC_DWORDS];
> 
>  	desc->span = STRTAB_SPLIT + 1;
>  	desc->l2ptr = dma_zalloc_coherent(smmu->dev, size, &desc->l2ptr_dma,
> --
> 1.8.0
> 
> 

[PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT

[Zhen Lei]

In context descriptor, the offset of IR0 is 8, the offset of OR0 is 10.

Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 2a5f810..43120ad 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -269,10 +269,10 @@
 #define ARM64_TCR_TG0_SHIFT		14
 #define ARM64_TCR_TG0_MASK		0x3UL
 #define CTXDESC_CD_0_TCR_IRGN0_SHIFT	8
-#define ARM64_TCR_IRGN0_SHIFT		24
+#define ARM64_TCR_IRGN0_SHIFT		8
 #define ARM64_TCR_IRGN0_MASK		0x3UL
 #define CTXDESC_CD_0_TCR_ORGN0_SHIFT	10
-#define ARM64_TCR_ORGN0_SHIFT		26
+#define ARM64_TCR_ORGN0_SHIFT		10
 #define ARM64_TCR_ORGN0_MASK		0x3UL
 #define CTXDESC_CD_0_TCR_SH0_SHIFT	12
 #define ARM64_TCR_SH0_SHIFT		12
--
1.8.0

[PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT

[Zhen Lei]

In context descriptor, the offset of IR0 is 8, the offset of OR0 is 10.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/iommu/arm-smmu-v3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 2a5f810..43120ad 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -269,10 +269,10 @@
 #define ARM64_TCR_TG0_SHIFT		14
 #define ARM64_TCR_TG0_MASK		0x3UL
 #define CTXDESC_CD_0_TCR_IRGN0_SHIFT	8
-#define ARM64_TCR_IRGN0_SHIFT		24
+#define ARM64_TCR_IRGN0_SHIFT		8
 #define ARM64_TCR_IRGN0_MASK		0x3UL
 #define CTXDESC_CD_0_TCR_ORGN0_SHIFT	10
-#define ARM64_TCR_ORGN0_SHIFT		26
+#define ARM64_TCR_ORGN0_SHIFT		10
 #define ARM64_TCR_ORGN0_MASK		0x3UL
 #define CTXDESC_CD_0_TCR_SH0_SHIFT	12
 #define ARM64_TCR_SH0_SHIFT		12
--
1.8.0

Re: [PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT

[Will Deacon]

On Fri, Jun 26, 2015 at 09:32:59AM +0100, Zhen Lei wrote:
> In context descriptor, the offset of IR0 is 8, the offset of OR0 is 10.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu-v3.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 2a5f810..43120ad 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -269,10 +269,10 @@
>  #define ARM64_TCR_TG0_SHIFT		14
>  #define ARM64_TCR_TG0_MASK		0x3UL
>  #define CTXDESC_CD_0_TCR_IRGN0_SHIFT	8
> -#define ARM64_TCR_IRGN0_SHIFT		24
> +#define ARM64_TCR_IRGN0_SHIFT		8
>  #define ARM64_TCR_IRGN0_MASK		0x3UL
>  #define CTXDESC_CD_0_TCR_ORGN0_SHIFT	10
> -#define ARM64_TCR_ORGN0_SHIFT		26
> +#define ARM64_TCR_ORGN0_SHIFT		10
>  #define ARM64_TCR_ORGN0_MASK		0x3UL
>  #define CTXDESC_CD_0_TCR_SH0_SHIFT	12
>  #define ARM64_TCR_SH0_SHIFT		12

I don't understand this patch.

The ARM64_* definitions correspond to the CPU architecture, whilst the
CTXDESC_* definitions correspond to the SMMUv3 CD description.

What problem are you seeing?

Will

[PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT

[Will Deacon]

On Fri, Jun 26, 2015 at 09:32:59AM +0100, Zhen Lei wrote:
> In context descriptor, the offset of IR0 is 8, the offset of OR0 is 10.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 2a5f810..43120ad 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -269,10 +269,10 @@
>  #define ARM64_TCR_TG0_SHIFT		14
>  #define ARM64_TCR_TG0_MASK		0x3UL
>  #define CTXDESC_CD_0_TCR_IRGN0_SHIFT	8
> -#define ARM64_TCR_IRGN0_SHIFT		24
> +#define ARM64_TCR_IRGN0_SHIFT		8
>  #define ARM64_TCR_IRGN0_MASK		0x3UL
>  #define CTXDESC_CD_0_TCR_ORGN0_SHIFT	10
> -#define ARM64_TCR_ORGN0_SHIFT		26
> +#define ARM64_TCR_ORGN0_SHIFT		10
>  #define ARM64_TCR_ORGN0_MASK		0x3UL
>  #define CTXDESC_CD_0_TCR_SH0_SHIFT	12
>  #define ARM64_TCR_SH0_SHIFT		12

I don't understand this patch.

The ARM64_* definitions correspond to the CPU architecture, whilst the
CTXDESC_* definitions correspond to the SMMUv3 CD description.

What problem are you seeing?

Will

Re: [PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT

[leizhen]

On 2015/6/30 1:25, Will Deacon wrote:
> On Fri, Jun 26, 2015 at 09:32:59AM +0100, Zhen Lei wrote:
>> In context descriptor, the offset of IR0 is 8, the offset of OR0 is 10.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 2a5f810..43120ad 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -269,10 +269,10 @@
>>  #define ARM64_TCR_TG0_SHIFT		14
>>  #define ARM64_TCR_TG0_MASK		0x3UL
>>  #define CTXDESC_CD_0_TCR_IRGN0_SHIFT	8
>> -#define ARM64_TCR_IRGN0_SHIFT		24
>> +#define ARM64_TCR_IRGN0_SHIFT		8
>>  #define ARM64_TCR_IRGN0_MASK		0x3UL
>>  #define CTXDESC_CD_0_TCR_ORGN0_SHIFT	10
>> -#define ARM64_TCR_ORGN0_SHIFT		26
>> +#define ARM64_TCR_ORGN0_SHIFT		10
>>  #define ARM64_TCR_ORGN0_MASK		0x3UL
>>  #define CTXDESC_CD_0_TCR_SH0_SHIFT	12
>>  #define ARM64_TCR_SH0_SHIFT		12
> 
> I don't understand this patch.
> 
> The ARM64_* definitions correspond to the CPU architecture, whilst the
> CTXDESC_* definitions correspond to the SMMUv3 CD description.
> 
> What problem are you seeing?

Oh, I'm sorry. My description was incorrect.

In io-pgtable-arm.c:
#define ARM_LPAE_TCR_ORGN0_SHIFT	10
#define ARM_LPAE_TCR_IRGN0_SHIFT	8

So, the description should be modified as below:
In SMMU_CBn_TCR when LPAE enabled, the offset of IRGN0 is 8, the offset of ORGN0 is 10.

> 
> Will
> 
> .
> 

[PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT

[leizhen]

On 2015/6/30 1:25, Will Deacon wrote:
> On Fri, Jun 26, 2015 at 09:32:59AM +0100, Zhen Lei wrote:
>> In context descriptor, the offset of IR0 is 8, the offset of OR0 is 10.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 2a5f810..43120ad 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -269,10 +269,10 @@
>>  #define ARM64_TCR_TG0_SHIFT		14
>>  #define ARM64_TCR_TG0_MASK		0x3UL
>>  #define CTXDESC_CD_0_TCR_IRGN0_SHIFT	8
>> -#define ARM64_TCR_IRGN0_SHIFT		24
>> +#define ARM64_TCR_IRGN0_SHIFT		8
>>  #define ARM64_TCR_IRGN0_MASK		0x3UL
>>  #define CTXDESC_CD_0_TCR_ORGN0_SHIFT	10
>> -#define ARM64_TCR_ORGN0_SHIFT		26
>> +#define ARM64_TCR_ORGN0_SHIFT		10
>>  #define ARM64_TCR_ORGN0_MASK		0x3UL
>>  #define CTXDESC_CD_0_TCR_SH0_SHIFT	12
>>  #define ARM64_TCR_SH0_SHIFT		12
> 
> I don't understand this patch.
> 
> The ARM64_* definitions correspond to the CPU architecture, whilst the
> CTXDESC_* definitions correspond to the SMMUv3 CD description.
> 
> What problem are you seeing?

Oh, I'm sorry. My description was incorrect.

In io-pgtable-arm.c:
#define ARM_LPAE_TCR_ORGN0_SHIFT	10
#define ARM_LPAE_TCR_IRGN0_SHIFT	8

So, the description should be modified as below:
In SMMU_CBn_TCR when LPAE enabled, the offset of IRGN0 is 8, the offset of ORGN0 is 10.

> 
> Will
> 
> .
> 

Re: [PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT

[Will Deacon]

On Tue, Jun 30, 2015 at 04:57:34AM +0100, leizhen wrote:
> On 2015/6/30 1:25, Will Deacon wrote:
> > On Fri, Jun 26, 2015 at 09:32:59AM +0100, Zhen Lei wrote:
> >> In context descriptor, the offset of IR0 is 8, the offset of OR0 is 10.
> >>
> >> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> >> ---
> >>  drivers/iommu/arm-smmu-v3.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> >> index 2a5f810..43120ad 100644
> >> --- a/drivers/iommu/arm-smmu-v3.c
> >> +++ b/drivers/iommu/arm-smmu-v3.c
> >> @@ -269,10 +269,10 @@
> >>  #define ARM64_TCR_TG0_SHIFT		14
> >>  #define ARM64_TCR_TG0_MASK		0x3UL
> >>  #define CTXDESC_CD_0_TCR_IRGN0_SHIFT	8
> >> -#define ARM64_TCR_IRGN0_SHIFT		24
> >> +#define ARM64_TCR_IRGN0_SHIFT		8
> >>  #define ARM64_TCR_IRGN0_MASK		0x3UL
> >>  #define CTXDESC_CD_0_TCR_ORGN0_SHIFT	10
> >> -#define ARM64_TCR_ORGN0_SHIFT		26
> >> +#define ARM64_TCR_ORGN0_SHIFT		10
> >>  #define ARM64_TCR_ORGN0_MASK		0x3UL
> >>  #define CTXDESC_CD_0_TCR_SH0_SHIFT	12
> >>  #define ARM64_TCR_SH0_SHIFT		12
> > 
> > I don't understand this patch.
> > 
> > The ARM64_* definitions correspond to the CPU architecture, whilst the
> > CTXDESC_* definitions correspond to the SMMUv3 CD description.
> > 
> > What problem are you seeing?
> 
> Oh, I'm sorry. My description was incorrect.
> 
> In io-pgtable-arm.c:
> #define ARM_LPAE_TCR_ORGN0_SHIFT	10
> #define ARM_LPAE_TCR_IRGN0_SHIFT	8
> 
> So, the description should be modified as below:
> In SMMU_CBn_TCR when LPAE enabled, the offset of IRGN0 is 8, the offset of
> ORGN0 is 10.

Gotcha, thanks.

Will

[PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT

[Will Deacon]

On Tue, Jun 30, 2015 at 04:57:34AM +0100, leizhen wrote:
> On 2015/6/30 1:25, Will Deacon wrote:
> > On Fri, Jun 26, 2015 at 09:32:59AM +0100, Zhen Lei wrote:
> >> In context descriptor, the offset of IR0 is 8, the offset of OR0 is 10.
> >>
> >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> >> ---
> >>  drivers/iommu/arm-smmu-v3.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> >> index 2a5f810..43120ad 100644
> >> --- a/drivers/iommu/arm-smmu-v3.c
> >> +++ b/drivers/iommu/arm-smmu-v3.c
> >> @@ -269,10 +269,10 @@
> >>  #define ARM64_TCR_TG0_SHIFT		14
> >>  #define ARM64_TCR_TG0_MASK		0x3UL
> >>  #define CTXDESC_CD_0_TCR_IRGN0_SHIFT	8
> >> -#define ARM64_TCR_IRGN0_SHIFT		24
> >> +#define ARM64_TCR_IRGN0_SHIFT		8
> >>  #define ARM64_TCR_IRGN0_MASK		0x3UL
> >>  #define CTXDESC_CD_0_TCR_ORGN0_SHIFT	10
> >> -#define ARM64_TCR_ORGN0_SHIFT		26
> >> +#define ARM64_TCR_ORGN0_SHIFT		10
> >>  #define ARM64_TCR_ORGN0_MASK		0x3UL
> >>  #define CTXDESC_CD_0_TCR_SH0_SHIFT	12
> >>  #define ARM64_TCR_SH0_SHIFT		12
> > 
> > I don't understand this patch.
> > 
> > The ARM64_* definitions correspond to the CPU architecture, whilst the
> > CTXDESC_* definitions correspond to the SMMUv3 CD description.
> > 
> > What problem are you seeing?
> 
> Oh, I'm sorry. My description was incorrect.
> 
> In io-pgtable-arm.c:
> #define ARM_LPAE_TCR_ORGN0_SHIFT	10
> #define ARM_LPAE_TCR_IRGN0_SHIFT	8
> 
> So, the description should be modified as below:
> In SMMU_CBn_TCR when LPAE enabled, the offset of IRGN0 is 8, the offset of
> ORGN0 is 10.

Gotcha, thanks.

Will

[PATCH 4/8] iommu/arm-smmu: set EPD1 to disable TT1 translation table walk

[Zhen Lei]

Now we only use TT0 translation, disable TT1 translation will safer.

Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 43120ad..6d6712e 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -285,6 +285,7 @@
 #define ARM64_TCR_EPD1_MASK		0x1UL

 #define CTXDESC_CD_0_ENDI		(1UL << 15)
+#define CTXDESC_CD_0_EPD1		(1UL << 30)
 #define CTXDESC_CD_0_V			(1UL << 31)

 #define CTXDESC_CD_0_TCR_IPS_SHIFT	32
@@ -893,7 +894,7 @@ static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
 #endif
 	      CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET_PRIVATE |
 	      CTXDESC_CD_0_AA64 | (u64)cfg->cd.asid << CTXDESC_CD_0_ASID_SHIFT |
-	      CTXDESC_CD_0_V;
+	      CTXDESC_CD_0_EPD1 | CTXDESC_CD_0_V;
 	cfg->cdptr[0] = cpu_to_le64(val);

 	val = cfg->cd.ttbr & CTXDESC_CD_1_TTB0_MASK << CTXDESC_CD_1_TTB0_SHIFT;
--
1.8.0

[PATCH 4/8] iommu/arm-smmu: set EPD1 to disable TT1 translation table walk

[Zhen Lei]

Now we only use TT0 translation, disable TT1 translation will safer.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/iommu/arm-smmu-v3.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 43120ad..6d6712e 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -285,6 +285,7 @@
 #define ARM64_TCR_EPD1_MASK		0x1UL

 #define CTXDESC_CD_0_ENDI		(1UL << 15)
+#define CTXDESC_CD_0_EPD1		(1UL << 30)
 #define CTXDESC_CD_0_V			(1UL << 31)

 #define CTXDESC_CD_0_TCR_IPS_SHIFT	32
@@ -893,7 +894,7 @@ static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
 #endif
 	      CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET_PRIVATE |
 	      CTXDESC_CD_0_AA64 | (u64)cfg->cd.asid << CTXDESC_CD_0_ASID_SHIFT |
-	      CTXDESC_CD_0_V;
+	      CTXDESC_CD_0_EPD1 | CTXDESC_CD_0_V;
 	cfg->cdptr[0] = cpu_to_le64(val);

 	val = cfg->cd.ttbr & CTXDESC_CD_1_TTB0_MASK << CTXDESC_CD_1_TTB0_SHIFT;
--
1.8.0

Re: [PATCH 4/8] iommu/arm-smmu: set EPD1 to disable TT1 translation table walk

[Will Deacon]

On Fri, Jun 26, 2015 at 09:33:00AM +0100, Zhen Lei wrote:
> Now we only use TT0 translation, disable TT1 translation will safer.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu-v3.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 43120ad..6d6712e 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -285,6 +285,7 @@
>  #define ARM64_TCR_EPD1_MASK		0x1UL
> 
>  #define CTXDESC_CD_0_ENDI		(1UL << 15)
> +#define CTXDESC_CD_0_EPD1		(1UL << 30)
>  #define CTXDESC_CD_0_V			(1UL << 31)
> 
>  #define CTXDESC_CD_0_TCR_IPS_SHIFT	32
> @@ -893,7 +894,7 @@ static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
>  #endif
>  	      CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET_PRIVATE |
>  	      CTXDESC_CD_0_AA64 | (u64)cfg->cd.asid << CTXDESC_CD_0_ASID_SHIFT |
> -	      CTXDESC_CD_0_V;
> +	      CTXDESC_CD_0_EPD1 | CTXDESC_CD_0_V;


This is redundant. EPD1 is already set by the io-pgtable code.

Will

[PATCH 4/8] iommu/arm-smmu: set EPD1 to disable TT1 translation table walk

[Will Deacon]

On Fri, Jun 26, 2015 at 09:33:00AM +0100, Zhen Lei wrote:
> Now we only use TT0 translation, disable TT1 translation will safer.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 43120ad..6d6712e 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -285,6 +285,7 @@
>  #define ARM64_TCR_EPD1_MASK		0x1UL
> 
>  #define CTXDESC_CD_0_ENDI		(1UL << 15)
> +#define CTXDESC_CD_0_EPD1		(1UL << 30)
>  #define CTXDESC_CD_0_V			(1UL << 31)
> 
>  #define CTXDESC_CD_0_TCR_IPS_SHIFT	32
> @@ -893,7 +894,7 @@ static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
>  #endif
>  	      CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET_PRIVATE |
>  	      CTXDESC_CD_0_AA64 | (u64)cfg->cd.asid << CTXDESC_CD_0_ASID_SHIFT |
> -	      CTXDESC_CD_0_V;
> +	      CTXDESC_CD_0_EPD1 | CTXDESC_CD_0_V;


This is redundant. EPD1 is already set by the io-pgtable code.

Will

Re: [PATCH 4/8] iommu/arm-smmu: set EPD1 to disable TT1 translation table walk

[leizhen]

On 2015/6/30 1:26, Will Deacon wrote:
> On Fri, Jun 26, 2015 at 09:33:00AM +0100, Zhen Lei wrote:
>> Now we only use TT0 translation, disable TT1 translation will safer.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 43120ad..6d6712e 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -285,6 +285,7 @@
>>  #define ARM64_TCR_EPD1_MASK		0x1UL
>>
>>  #define CTXDESC_CD_0_ENDI		(1UL << 15)
>> +#define CTXDESC_CD_0_EPD1		(1UL << 30)
>>  #define CTXDESC_CD_0_V			(1UL << 31)
>>
>>  #define CTXDESC_CD_0_TCR_IPS_SHIFT	32
>> @@ -893,7 +894,7 @@ static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
>>  #endif
>>  	      CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET_PRIVATE |
>>  	      CTXDESC_CD_0_AA64 | (u64)cfg->cd.asid << CTXDESC_CD_0_ASID_SHIFT |
>> -	      CTXDESC_CD_0_V;
>> +	      CTXDESC_CD_0_EPD1 | CTXDESC_CD_0_V;
> 
> 
> This is redundant. EPD1 is already set by the io-pgtable code.

OK, I will drop this patch. I made a mistake.

> 
> Will
> 
> .
> 

[PATCH 4/8] iommu/arm-smmu: set EPD1 to disable TT1 translation table walk

[leizhen]

On 2015/6/30 1:26, Will Deacon wrote:
> On Fri, Jun 26, 2015 at 09:33:00AM +0100, Zhen Lei wrote:
>> Now we only use TT0 translation, disable TT1 translation will safer.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 43120ad..6d6712e 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -285,6 +285,7 @@
>>  #define ARM64_TCR_EPD1_MASK		0x1UL
>>
>>  #define CTXDESC_CD_0_ENDI		(1UL << 15)
>> +#define CTXDESC_CD_0_EPD1		(1UL << 30)
>>  #define CTXDESC_CD_0_V			(1UL << 31)
>>
>>  #define CTXDESC_CD_0_TCR_IPS_SHIFT	32
>> @@ -893,7 +894,7 @@ static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
>>  #endif
>>  	      CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET_PRIVATE |
>>  	      CTXDESC_CD_0_AA64 | (u64)cfg->cd.asid << CTXDESC_CD_0_ASID_SHIFT |
>> -	      CTXDESC_CD_0_V;
>> +	      CTXDESC_CD_0_EPD1 | CTXDESC_CD_0_V;
> 
> 
> This is redundant. EPD1 is already set by the io-pgtable code.

OK, I will drop this patch. I made a mistake.

> 
> Will
> 
> .
> 

[PATCH 5/8] iommu/arm-smmu: rename __arm_smmu_get_pci_sid

[Zhen Lei]

Remove the words "pci", to make this function can also be used by
non-pci devices.

Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 6d6712e..483c918 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1708,7 +1708,7 @@ static int __arm_smmu_get_pci_sid(struct pci_dev *pdev, u16 alias, void *sidp)
 	return 0; /* Continue walking */
 }

-static void __arm_smmu_release_pci_iommudata(void *data)
+static void __arm_smmu_release_iommudata(void *data)
 {
 	kfree(data);
 }
@@ -1786,7 +1786,7 @@ static int arm_smmu_add_device(struct device *dev)
 		smmu_group->ste.valid	= true;
 		smmu_group->smmu	= smmu;
 		iommu_group_set_iommudata(group, smmu_group,
-					  __arm_smmu_release_pci_iommudata);
+					  __arm_smmu_release_iommudata);
 	} else {
 		smmu = smmu_group->smmu;
 	}
--
1.8.0

[PATCH 5/8] iommu/arm-smmu: rename __arm_smmu_get_pci_sid

[Zhen Lei]

Remove the words "pci", to make this function can also be used by
non-pci devices.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/iommu/arm-smmu-v3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 6d6712e..483c918 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1708,7 +1708,7 @@ static int __arm_smmu_get_pci_sid(struct pci_dev *pdev, u16 alias, void *sidp)
 	return 0; /* Continue walking */
 }

-static void __arm_smmu_release_pci_iommudata(void *data)
+static void __arm_smmu_release_iommudata(void *data)
 {
 	kfree(data);
 }
@@ -1786,7 +1786,7 @@ static int arm_smmu_add_device(struct device *dev)
 		smmu_group->ste.valid	= true;
 		smmu_group->smmu	= smmu;
 		iommu_group_set_iommudata(group, smmu_group,
-					  __arm_smmu_release_pci_iommudata);
+					  __arm_smmu_release_iommudata);
 	} else {
 		smmu = smmu_group->smmu;
 	}
--
1.8.0

[PATCH 6/8] iommu/arm-smmu: add support for non-pci devices

[Zhen Lei]

Now, we only support a master with only one stream id. It will cover most
hardware platforms and coding so easy.

Please refer Documentation\devicetree\bindings\iommu\iommu.txt on how to
bind device tree.

Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 arch/arm64/include/asm/device.h |  2 +
 drivers/iommu/arm-smmu-v3.c     | 88 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h
index 243ef25..225e4f9 100644
--- a/arch/arm64/include/asm/device.h
+++ b/arch/arm64/include/asm/device.h
@@ -20,6 +20,8 @@ struct dev_archdata {
 	struct dma_map_ops *dma_ops;
 #ifdef CONFIG_IOMMU_API
 	void *iommu;			/* private IOMMU data */
+	struct device_node *of_smmu;
+	u32 sid;
 #endif
 	bool dma_coherent;
 };
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 483c918..87c3d9b 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -30,9 +30,14 @@
 #include <linux/of_address.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
+#include <linux/amba/bus.h>
+#include <linux/of_iommu.h>

 #include "io-pgtable.h"

+/* Maximum number of stream IDs assigned to a single device */
+#define MAX_MASTER_STREAMIDS		1
+
 /* MMIO registers */
 #define ARM_SMMU_IDR0			0x0
 #define IDR0_ST_LVL_SHIFT		27
@@ -608,6 +613,22 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
 	return container_of(dom, struct arm_smmu_domain, domain);
 }

+static struct arm_smmu_device *find_smmu_for_device(struct device *dev)
+{
+	struct arm_smmu_device *smmu;
+
+	spin_lock(&arm_smmu_devices_lock);
+	list_for_each_entry(smmu, &arm_smmu_devices, list) {
+		if (smmu->dev->of_node == dev->archdata.of_smmu) {
+			spin_unlock(&arm_smmu_devices_lock);
+			return smmu;
+		}
+	}
+	spin_unlock(&arm_smmu_devices_lock);
+
+	return NULL;
+}
+
 /* Low-level queue manipulation functions */
 static bool queue_full(struct arm_smmu_queue *q)
 {
@@ -1760,9 +1781,36 @@ static int arm_smmu_add_device(struct device *dev)
 	struct arm_smmu_group *smmu_group;
 	struct arm_smmu_device *smmu;

-	/* We only support PCI, for now */
-	if (!dev_is_pci(dev))
-		return -ENODEV;
+	if (!dev_is_pci(dev)) {
+		smmu = find_smmu_for_device(dev);
+		if (!smmu)
+			return -ENODEV;
+
+		group = iommu_group_alloc();
+		if (IS_ERR(group)) {
+			dev_err(dev, "Failed to allocate IOMMU group\n");
+			return PTR_ERR(group);
+		}
+
+		ret = iommu_group_add_device(group, dev);
+		if (ret)
+			goto out_put_group;
+
+		smmu_group = kzalloc(sizeof(*smmu_group), GFP_KERNEL);
+		if (!smmu_group) {
+			ret = -ENOMEM;
+			goto out_put_group;
+		}
+
+		smmu_group->ste.valid = true;
+		smmu_group->smmu = smmu;
+		iommu_group_set_iommudata(group, smmu_group,
+						__arm_smmu_release_iommudata);
+
+		sid = dev->archdata.sid;
+
+		goto handle_stream_id;
+	}

 	pdev = to_pci_dev(dev);
 	group = iommu_group_get_for_dev(dev);
@@ -1793,6 +1841,8 @@ static int arm_smmu_add_device(struct device *dev)

 	/* Assume SID == RID until firmware tells us otherwise */
 	pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid, &sid);
+
+handle_stream_id:
 	for (i = 0; i < smmu_group->num_sids; ++i) {
 		/* If we already know about this SID, then we're done */
 		if (smmu_group->sids[i] == sid)
@@ -1881,7 +1931,23 @@ out_unlock:
 	return ret;
 }

+static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
+{
+	if (args->args_count > MAX_MASTER_STREAMIDS) {
+		dev_err(dev,
+			"reached maximum number (%d) of stream IDs for master device %s\n",
+			MAX_MASTER_STREAMIDS, dev->of_node->name);
+		return -ENOSPC;
+	}
+
+	dev->archdata.of_smmu = args->np;
+	dev->archdata.sid = args->args[0];
+
+	return 0;
+}
+
 static struct iommu_ops arm_smmu_ops = {
+	.of_xlate		= arm_smmu_of_xlate,
 	.capable		= arm_smmu_capable,
 	.domain_alloc		= arm_smmu_domain_alloc,
 	.domain_free		= arm_smmu_domain_free,
@@ -2655,6 +2721,14 @@ static int __init arm_smmu_init(void)
 	if (ret)
 		return ret;

+	if (!iommu_present(&platform_bus_type))
+		bus_set_iommu(&platform_bus_type, &arm_smmu_ops);
+
+#ifdef CONFIG_ARM_AMBA
+	if (!iommu_present(&amba_bustype))
+		bus_set_iommu(&amba_bustype, &arm_smmu_ops);
+#endif
+
 	return bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
 }

@@ -2666,6 +2740,14 @@ static void __exit arm_smmu_exit(void)
 subsys_initcall(arm_smmu_init);
 module_exit(arm_smmu_exit);

+static int arm_smmu_of_iommu_init(struct device_node *np)
+{
+	of_iommu_set_ops(np, &arm_smmu_ops);
+
+	return 0;
+}
+IOMMU_OF_DECLARE(arm_smmu_v3, "arm,smmu-v3", arm_smmu_of_iommu_init);
+
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
 MODULE_AUTHOR("Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>");
 MODULE_LICENSE("GPL v2");
--
1.8.0

[PATCH 6/8] iommu/arm-smmu: add support for non-pci devices

[Zhen Lei]

Now, we only support a master with only one stream id. It will cover most
hardware platforms and coding so easy.

Please refer Documentation\devicetree\bindings\iommu\iommu.txt on how to
bind device tree.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 arch/arm64/include/asm/device.h |  2 +
 drivers/iommu/arm-smmu-v3.c     | 88 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h
index 243ef25..225e4f9 100644
--- a/arch/arm64/include/asm/device.h
+++ b/arch/arm64/include/asm/device.h
@@ -20,6 +20,8 @@ struct dev_archdata {
 	struct dma_map_ops *dma_ops;
 #ifdef CONFIG_IOMMU_API
 	void *iommu;			/* private IOMMU data */
+	struct device_node *of_smmu;
+	u32 sid;
 #endif
 	bool dma_coherent;
 };
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 483c918..87c3d9b 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -30,9 +30,14 @@
 #include <linux/of_address.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
+#include <linux/amba/bus.h>
+#include <linux/of_iommu.h>

 #include "io-pgtable.h"

+/* Maximum number of stream IDs assigned to a single device */
+#define MAX_MASTER_STREAMIDS		1
+
 /* MMIO registers */
 #define ARM_SMMU_IDR0			0x0
 #define IDR0_ST_LVL_SHIFT		27
@@ -608,6 +613,22 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
 	return container_of(dom, struct arm_smmu_domain, domain);
 }

+static struct arm_smmu_device *find_smmu_for_device(struct device *dev)
+{
+	struct arm_smmu_device *smmu;
+
+	spin_lock(&arm_smmu_devices_lock);
+	list_for_each_entry(smmu, &arm_smmu_devices, list) {
+		if (smmu->dev->of_node == dev->archdata.of_smmu) {
+			spin_unlock(&arm_smmu_devices_lock);
+			return smmu;
+		}
+	}
+	spin_unlock(&arm_smmu_devices_lock);
+
+	return NULL;
+}
+
 /* Low-level queue manipulation functions */
 static bool queue_full(struct arm_smmu_queue *q)
 {
@@ -1760,9 +1781,36 @@ static int arm_smmu_add_device(struct device *dev)
 	struct arm_smmu_group *smmu_group;
 	struct arm_smmu_device *smmu;

-	/* We only support PCI, for now */
-	if (!dev_is_pci(dev))
-		return -ENODEV;
+	if (!dev_is_pci(dev)) {
+		smmu = find_smmu_for_device(dev);
+		if (!smmu)
+			return -ENODEV;
+
+		group = iommu_group_alloc();
+		if (IS_ERR(group)) {
+			dev_err(dev, "Failed to allocate IOMMU group\n");
+			return PTR_ERR(group);
+		}
+
+		ret = iommu_group_add_device(group, dev);
+		if (ret)
+			goto out_put_group;
+
+		smmu_group = kzalloc(sizeof(*smmu_group), GFP_KERNEL);
+		if (!smmu_group) {
+			ret = -ENOMEM;
+			goto out_put_group;
+		}
+
+		smmu_group->ste.valid = true;
+		smmu_group->smmu = smmu;
+		iommu_group_set_iommudata(group, smmu_group,
+						__arm_smmu_release_iommudata);
+
+		sid = dev->archdata.sid;
+
+		goto handle_stream_id;
+	}

 	pdev = to_pci_dev(dev);
 	group = iommu_group_get_for_dev(dev);
@@ -1793,6 +1841,8 @@ static int arm_smmu_add_device(struct device *dev)

 	/* Assume SID == RID until firmware tells us otherwise */
 	pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid, &sid);
+
+handle_stream_id:
 	for (i = 0; i < smmu_group->num_sids; ++i) {
 		/* If we already know about this SID, then we're done */
 		if (smmu_group->sids[i] == sid)
@@ -1881,7 +1931,23 @@ out_unlock:
 	return ret;
 }

+static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
+{
+	if (args->args_count > MAX_MASTER_STREAMIDS) {
+		dev_err(dev,
+			"reached maximum number (%d) of stream IDs for master device %s\n",
+			MAX_MASTER_STREAMIDS, dev->of_node->name);
+		return -ENOSPC;
+	}
+
+	dev->archdata.of_smmu = args->np;
+	dev->archdata.sid = args->args[0];
+
+	return 0;
+}
+
 static struct iommu_ops arm_smmu_ops = {
+	.of_xlate		= arm_smmu_of_xlate,
 	.capable		= arm_smmu_capable,
 	.domain_alloc		= arm_smmu_domain_alloc,
 	.domain_free		= arm_smmu_domain_free,
@@ -2655,6 +2721,14 @@ static int __init arm_smmu_init(void)
 	if (ret)
 		return ret;

+	if (!iommu_present(&platform_bus_type))
+		bus_set_iommu(&platform_bus_type, &arm_smmu_ops);
+
+#ifdef CONFIG_ARM_AMBA
+	if (!iommu_present(&amba_bustype))
+		bus_set_iommu(&amba_bustype, &arm_smmu_ops);
+#endif
+
 	return bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
 }

@@ -2666,6 +2740,14 @@ static void __exit arm_smmu_exit(void)
 subsys_initcall(arm_smmu_init);
 module_exit(arm_smmu_exit);

+static int arm_smmu_of_iommu_init(struct device_node *np)
+{
+	of_iommu_set_ops(np, &arm_smmu_ops);
+
+	return 0;
+}
+IOMMU_OF_DECLARE(arm_smmu_v3, "arm,smmu-v3", arm_smmu_of_iommu_init);
+
 MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
 MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
 MODULE_LICENSE("GPL v2");
--
1.8.0

Re: [PATCH 6/8] iommu/arm-smmu: add support for non-pci devices

[Will Deacon]

On Fri, Jun 26, 2015 at 09:33:02AM +0100, Zhen Lei wrote:
> Now, we only support a master with only one stream id. It will cover most
> hardware platforms and coding so easy.
> 
> Please refer Documentation\devicetree\bindings\iommu\iommu.txt on how to
> bind device tree.

Can you build this on top of Laurent's series, please?

  http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/343686.html

Will

[PATCH 6/8] iommu/arm-smmu: add support for non-pci devices

[Will Deacon]

On Fri, Jun 26, 2015 at 09:33:02AM +0100, Zhen Lei wrote:
> Now, we only support a master with only one stream id. It will cover most
> hardware platforms and coding so easy.
> 
> Please refer Documentation\devicetree\bindings\iommu\iommu.txt on how to
> bind device tree.

Can you build this on top of Laurent's series, please?

  http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/343686.html

Will

Re: [PATCH 6/8] iommu/arm-smmu: add support for non-pci devices

[leizhen]

On 2015/6/30 1:28, Will Deacon wrote:
> On Fri, Jun 26, 2015 at 09:33:02AM +0100, Zhen Lei wrote:
>> Now, we only support a master with only one stream id. It will cover most
>> hardware platforms and coding so easy.
>>
>> Please refer Documentation\devicetree\bindings\iommu\iommu.txt on how to
>> bind device tree.
> 
> Can you build this on top of Laurent's series, please?
> 
>   http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/343686.html

Sorry, I have a meeting for a long time. So my email writing was interrupted.

Because Laurent's series has not been upstreamed. So this patch can not work correctly
after patched Laurent's series. It should modified slightly as below:
Should I merge these two patches as one after Laurent's series, or send two times
(one before Laurent's series, and one after). What's your opinion?

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 2e09f10..c940522 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1953,7 +1953,7 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
        dev->archdata.of_smmu = args->np;
        dev->archdata.sid = args->args[0];

-   return 0;
+ return arm_smmu_add_device(dev);
 }

 static struct iommu_ops arm_smmu_ops = {
@@ -1966,7 +1966,6 @@ static struct iommu_ops arm_smmu_ops = {
        .map                    = arm_smmu_map,
        .unmap                  = arm_smmu_unmap,
        .iova_to_phys           = arm_smmu_iova_to_phys,
-   .add_device             = arm_smmu_add_device,
        .remove_device          = arm_smmu_remove_device,
        .domain_get_attr        = arm_smmu_domain_get_attr,
        .domain_set_attr        = arm_smmu_domain_set_attr,


> 
> Will
> 
> .
> 

[PATCH 6/8] iommu/arm-smmu: add support for non-pci devices

[leizhen]

On 2015/6/30 1:28, Will Deacon wrote:
> On Fri, Jun 26, 2015 at 09:33:02AM +0100, Zhen Lei wrote:
>> Now, we only support a master with only one stream id. It will cover most
>> hardware platforms and coding so easy.
>>
>> Please refer Documentation\devicetree\bindings\iommu\iommu.txt on how to
>> bind device tree.
> 
> Can you build this on top of Laurent's series, please?
> 
>   http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/343686.html

Sorry, I have a meeting for a long time. So my email writing was interrupted.

Because Laurent's series has not been upstreamed. So this patch can not work correctly
after patched Laurent's series. It should modified slightly as below:
Should I merge these two patches as one after Laurent's series, or send two times
(one before Laurent's series, and one after). What's your opinion?

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 2e09f10..c940522 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1953,7 +1953,7 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
        dev->archdata.of_smmu = args->np;
        dev->archdata.sid = args->args[0];

-   return 0;
+ return arm_smmu_add_device(dev);
 }

 static struct iommu_ops arm_smmu_ops = {
@@ -1966,7 +1966,6 @@ static struct iommu_ops arm_smmu_ops = {
        .map                    = arm_smmu_map,
        .unmap                  = arm_smmu_unmap,
        .iova_to_phys           = arm_smmu_iova_to_phys,
-   .add_device             = arm_smmu_add_device,
        .remove_device          = arm_smmu_remove_device,
        .domain_get_attr        = arm_smmu_domain_get_attr,
        .domain_set_attr        = arm_smmu_domain_set_attr,


> 
> Will
> 
> .
> 

Re: [PATCH 6/8] iommu/arm-smmu: add support for non-pci devices

[Robin Murphy]

On 26/06/15 09:33, Zhen Lei wrote:
> Now, we only support a master with only one stream id. It will cover most
> hardware platforms and coding so easy.
>
> Please refer Documentation\devicetree\bindings\iommu\iommu.txt on how to
> bind device tree.
>
> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>   arch/arm64/include/asm/device.h |  2 +
>   drivers/iommu/arm-smmu-v3.c     | 88 +++++++++++++++++++++++++++++++++++++++--
>   2 files changed, 87 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h
> index 243ef25..225e4f9 100644
> --- a/arch/arm64/include/asm/device.h
> +++ b/arch/arm64/include/asm/device.h
> @@ -20,6 +20,8 @@ struct dev_archdata {
>   	struct dma_map_ops *dma_ops;
>   #ifdef CONFIG_IOMMU_API
>   	void *iommu;			/* private IOMMU data */

There's already a perfectly good place to store driver-private data 
right here.

> +	struct device_node *of_smmu;
> +	u32 sid;

This looks far too specific to be in core code. It doesn't seem 
extensible for e.g. ACPI platform devices; it doesn't seem extensible 
for platform devices with multiple stream IDs e.g. PL330; it also 
doesn't seem (sensibly) extensible for IOMMUs with #iommu-cells > 1.

Just allocate an SMMU-private struct for this and stash it in 
archdata.iommu - that way we can change things as much as we like in the 
driver with zero churn in core code.

>   #endif
>   	bool dma_coherent;
>   };
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 483c918..87c3d9b 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -30,9 +30,14 @@
>   #include <linux/of_address.h>
>   #include <linux/pci.h>
>   #include <linux/platform_device.h>
> +#include <linux/amba/bus.h>
> +#include <linux/of_iommu.h>
>
>   #include "io-pgtable.h"
>
> +/* Maximum number of stream IDs assigned to a single device */
> +#define MAX_MASTER_STREAMIDS		1
> +
>   /* MMIO registers */
>   #define ARM_SMMU_IDR0			0x0
>   #define IDR0_ST_LVL_SHIFT		27
> @@ -608,6 +613,22 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>   	return container_of(dom, struct arm_smmu_domain, domain);
>   }
>
> +static struct arm_smmu_device *find_smmu_for_device(struct device *dev)
> +{
> +	struct arm_smmu_device *smmu;
> +
> +	spin_lock(&arm_smmu_devices_lock);
> +	list_for_each_entry(smmu, &arm_smmu_devices, list) {
> +		if (smmu->dev->of_node == dev->archdata.of_smmu) {
> +			spin_unlock(&arm_smmu_devices_lock);
> +			return smmu;
> +		}
> +	}
> +	spin_unlock(&arm_smmu_devices_lock);
> +
> +	return NULL;
> +}
> +

This should be unnecessary with the right probe order, see below...

>   /* Low-level queue manipulation functions */
>   static bool queue_full(struct arm_smmu_queue *q)
>   {
> @@ -1760,9 +1781,36 @@ static int arm_smmu_add_device(struct device *dev)
>   	struct arm_smmu_group *smmu_group;
>   	struct arm_smmu_device *smmu;
>
> -	/* We only support PCI, for now */
> -	if (!dev_is_pci(dev))
> -		return -ENODEV;
> +	if (!dev_is_pci(dev)) {
> +		smmu = find_smmu_for_device(dev);
> +		if (!smmu)
> +			return -ENODEV;
> +
> +		group = iommu_group_alloc();
> +		if (IS_ERR(group)) {
> +			dev_err(dev, "Failed to allocate IOMMU group\n");
> +			return PTR_ERR(group);
> +		}
> +
> +		ret = iommu_group_add_device(group, dev);
> +		if (ret)
> +			goto out_put_group;
> +
> +		smmu_group = kzalloc(sizeof(*smmu_group), GFP_KERNEL);
> +		if (!smmu_group) {
> +			ret = -ENOMEM;
> +			goto out_put_group;
> +		}
> +
> +		smmu_group->ste.valid = true;
> +		smmu_group->smmu = smmu;
> +		iommu_group_set_iommudata(group, smmu_group,
> +						__arm_smmu_release_iommudata);
> +
> +		sid = dev->archdata.sid;
> +
> +		goto handle_stream_id;
> +	}
>
>   	pdev = to_pci_dev(dev);
>   	group = iommu_group_get_for_dev(dev);
> @@ -1793,6 +1841,8 @@ static int arm_smmu_add_device(struct device *dev)
>
>   	/* Assume SID == RID until firmware tells us otherwise */
>   	pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid, &sid);
> +
> +handle_stream_id:

This is going to get messy quickly - how about breaking out the 
"platform device" and "PCI device" specifics above into their own 
functions that return the group and sid data to the common code here?

>   	for (i = 0; i < smmu_group->num_sids; ++i) {
>   		/* If we already know about this SID, then we're done */
>   		if (smmu_group->sids[i] == sid)
> @@ -1881,7 +1931,23 @@ out_unlock:
>   	return ret;
>   }
>
> +static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
> +{
> +	if (args->args_count > MAX_MASTER_STREAMIDS) {
> +		dev_err(dev,
> +			"reached maximum number (%d) of stream IDs for master device %s\n",
> +			MAX_MASTER_STREAMIDS, dev->of_node->name);
> +		return -ENOSPC;
> +	}
> +
> +	dev->archdata.of_smmu = args->np;
> +	dev->archdata.sid = args->args[0];
> +
> +	return 0;
> +}

This isn't going to work the way you expect: the way the binding is 
defined, a master with multiple stream IDs should look like so:

	iommus = <&smmu 0>, <&smmu 1>,...

so you'd get multiple calls, never hit the warning, and just end up with 
whichever ID came last.

Secondly, as mentioned above, it would be nicer to just associate the 
arm_smmu_device directly here and obviate the indirect lookup. That 
would depend on having correct probe ordering, but you need to enforce 
that anyway, since any add_device callbacks before the SMMU itself has 
been probed will break. Laurent's probe deferral series that Will 
pointed to is the ultimate goal, but for a stop-gap solution which works 
with the current code I'd suggest taking a look at patches 16 and 17 of 
Marek's Exynos SysMMU series[1]

Robin.

[1]:http://thread.gmane.org/gmane.linux.kernel.samsung-soc/45416

> +
>   static struct iommu_ops arm_smmu_ops = {
> +	.of_xlate		= arm_smmu_of_xlate,
>   	.capable		= arm_smmu_capable,
>   	.domain_alloc		= arm_smmu_domain_alloc,
>   	.domain_free		= arm_smmu_domain_free,
> @@ -2655,6 +2721,14 @@ static int __init arm_smmu_init(void)
>   	if (ret)
>   		return ret;
>
> +	if (!iommu_present(&platform_bus_type))
> +		bus_set_iommu(&platform_bus_type, &arm_smmu_ops);
> +
> +#ifdef CONFIG_ARM_AMBA
> +	if (!iommu_present(&amba_bustype))
> +		bus_set_iommu(&amba_bustype, &arm_smmu_ops);
> +#endif
> +
>   	return bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
>   }
>
> @@ -2666,6 +2740,14 @@ static void __exit arm_smmu_exit(void)
>   subsys_initcall(arm_smmu_init);
>   module_exit(arm_smmu_exit);
>
> +static int arm_smmu_of_iommu_init(struct device_node *np)
> +{
> +	of_iommu_set_ops(np, &arm_smmu_ops);
> +
> +	return 0;
> +}
> +IOMMU_OF_DECLARE(arm_smmu_v3, "arm,smmu-v3", arm_smmu_of_iommu_init);
> +
>   MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
>   MODULE_AUTHOR("Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>");
>   MODULE_LICENSE("GPL v2");
> --
> 1.8.0
>
>
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>

[PATCH 6/8] iommu/arm-smmu: add support for non-pci devices

[Robin Murphy]

On 26/06/15 09:33, Zhen Lei wrote:
> Now, we only support a master with only one stream id. It will cover most
> hardware platforms and coding so easy.
>
> Please refer Documentation\devicetree\bindings\iommu\iommu.txt on how to
> bind device tree.
>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>   arch/arm64/include/asm/device.h |  2 +
>   drivers/iommu/arm-smmu-v3.c     | 88 +++++++++++++++++++++++++++++++++++++++--
>   2 files changed, 87 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h
> index 243ef25..225e4f9 100644
> --- a/arch/arm64/include/asm/device.h
> +++ b/arch/arm64/include/asm/device.h
> @@ -20,6 +20,8 @@ struct dev_archdata {
>   	struct dma_map_ops *dma_ops;
>   #ifdef CONFIG_IOMMU_API
>   	void *iommu;			/* private IOMMU data */

There's already a perfectly good place to store driver-private data 
right here.

> +	struct device_node *of_smmu;
> +	u32 sid;

This looks far too specific to be in core code. It doesn't seem 
extensible for e.g. ACPI platform devices; it doesn't seem extensible 
for platform devices with multiple stream IDs e.g. PL330; it also 
doesn't seem (sensibly) extensible for IOMMUs with #iommu-cells > 1.

Just allocate an SMMU-private struct for this and stash it in 
archdata.iommu - that way we can change things as much as we like in the 
driver with zero churn in core code.

>   #endif
>   	bool dma_coherent;
>   };
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 483c918..87c3d9b 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -30,9 +30,14 @@
>   #include <linux/of_address.h>
>   #include <linux/pci.h>
>   #include <linux/platform_device.h>
> +#include <linux/amba/bus.h>
> +#include <linux/of_iommu.h>
>
>   #include "io-pgtable.h"
>
> +/* Maximum number of stream IDs assigned to a single device */
> +#define MAX_MASTER_STREAMIDS		1
> +
>   /* MMIO registers */
>   #define ARM_SMMU_IDR0			0x0
>   #define IDR0_ST_LVL_SHIFT		27
> @@ -608,6 +613,22 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>   	return container_of(dom, struct arm_smmu_domain, domain);
>   }
>
> +static struct arm_smmu_device *find_smmu_for_device(struct device *dev)
> +{
> +	struct arm_smmu_device *smmu;
> +
> +	spin_lock(&arm_smmu_devices_lock);
> +	list_for_each_entry(smmu, &arm_smmu_devices, list) {
> +		if (smmu->dev->of_node == dev->archdata.of_smmu) {
> +			spin_unlock(&arm_smmu_devices_lock);
> +			return smmu;
> +		}
> +	}
> +	spin_unlock(&arm_smmu_devices_lock);
> +
> +	return NULL;
> +}
> +

This should be unnecessary with the right probe order, see below...

>   /* Low-level queue manipulation functions */
>   static bool queue_full(struct arm_smmu_queue *q)
>   {
> @@ -1760,9 +1781,36 @@ static int arm_smmu_add_device(struct device *dev)
>   	struct arm_smmu_group *smmu_group;
>   	struct arm_smmu_device *smmu;
>
> -	/* We only support PCI, for now */
> -	if (!dev_is_pci(dev))
> -		return -ENODEV;
> +	if (!dev_is_pci(dev)) {
> +		smmu = find_smmu_for_device(dev);
> +		if (!smmu)
> +			return -ENODEV;
> +
> +		group = iommu_group_alloc();
> +		if (IS_ERR(group)) {
> +			dev_err(dev, "Failed to allocate IOMMU group\n");
> +			return PTR_ERR(group);
> +		}
> +
> +		ret = iommu_group_add_device(group, dev);
> +		if (ret)
> +			goto out_put_group;
> +
> +		smmu_group = kzalloc(sizeof(*smmu_group), GFP_KERNEL);
> +		if (!smmu_group) {
> +			ret = -ENOMEM;
> +			goto out_put_group;
> +		}
> +
> +		smmu_group->ste.valid = true;
> +		smmu_group->smmu = smmu;
> +		iommu_group_set_iommudata(group, smmu_group,
> +						__arm_smmu_release_iommudata);
> +
> +		sid = dev->archdata.sid;
> +
> +		goto handle_stream_id;
> +	}
>
>   	pdev = to_pci_dev(dev);
>   	group = iommu_group_get_for_dev(dev);
> @@ -1793,6 +1841,8 @@ static int arm_smmu_add_device(struct device *dev)
>
>   	/* Assume SID == RID until firmware tells us otherwise */
>   	pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid, &sid);
> +
> +handle_stream_id:

This is going to get messy quickly - how about breaking out the 
"platform device" and "PCI device" specifics above into their own 
functions that return the group and sid data to the common code here?

>   	for (i = 0; i < smmu_group->num_sids; ++i) {
>   		/* If we already know about this SID, then we're done */
>   		if (smmu_group->sids[i] == sid)
> @@ -1881,7 +1931,23 @@ out_unlock:
>   	return ret;
>   }
>
> +static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
> +{
> +	if (args->args_count > MAX_MASTER_STREAMIDS) {
> +		dev_err(dev,
> +			"reached maximum number (%d) of stream IDs for master device %s\n",
> +			MAX_MASTER_STREAMIDS, dev->of_node->name);
> +		return -ENOSPC;
> +	}
> +
> +	dev->archdata.of_smmu = args->np;
> +	dev->archdata.sid = args->args[0];
> +
> +	return 0;
> +}

This isn't going to work the way you expect: the way the binding is 
defined, a master with multiple stream IDs should look like so:

	iommus = <&smmu 0>, <&smmu 1>,...

so you'd get multiple calls, never hit the warning, and just end up with 
whichever ID came last.

Secondly, as mentioned above, it would be nicer to just associate the 
arm_smmu_device directly here and obviate the indirect lookup. That 
would depend on having correct probe ordering, but you need to enforce 
that anyway, since any add_device callbacks before the SMMU itself has 
been probed will break. Laurent's probe deferral series that Will 
pointed to is the ultimate goal, but for a stop-gap solution which works 
with the current code I'd suggest taking a look at patches 16 and 17 of 
Marek's Exynos SysMMU series[1]

Robin.

[1]:http://thread.gmane.org/gmane.linux.kernel.samsung-soc/45416

> +
>   static struct iommu_ops arm_smmu_ops = {
> +	.of_xlate		= arm_smmu_of_xlate,
>   	.capable		= arm_smmu_capable,
>   	.domain_alloc		= arm_smmu_domain_alloc,
>   	.domain_free		= arm_smmu_domain_free,
> @@ -2655,6 +2721,14 @@ static int __init arm_smmu_init(void)
>   	if (ret)
>   		return ret;
>
> +	if (!iommu_present(&platform_bus_type))
> +		bus_set_iommu(&platform_bus_type, &arm_smmu_ops);
> +
> +#ifdef CONFIG_ARM_AMBA
> +	if (!iommu_present(&amba_bustype))
> +		bus_set_iommu(&amba_bustype, &arm_smmu_ops);
> +#endif
> +
>   	return bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
>   }
>
> @@ -2666,6 +2740,14 @@ static void __exit arm_smmu_exit(void)
>   subsys_initcall(arm_smmu_init);
>   module_exit(arm_smmu_exit);
>
> +static int arm_smmu_of_iommu_init(struct device_node *np)
> +{
> +	of_iommu_set_ops(np, &arm_smmu_ops);
> +
> +	return 0;
> +}
> +IOMMU_OF_DECLARE(arm_smmu_v3, "arm,smmu-v3", arm_smmu_of_iommu_init);
> +
>   MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
>   MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
>   MODULE_LICENSE("GPL v2");
> --
> 1.8.0
>
>
> _______________________________________________
> iommu mailing list
> iommu at lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>

Re: [PATCH 6/8] iommu/arm-smmu: add support for non-pci devices

[leizhen]

On 2015/6/30 19:26, Robin Murphy wrote:
> On 26/06/15 09:33, Zhen Lei wrote:
>> Now, we only support a master with only one stream id. It will cover most
>> hardware platforms and coding so easy.
>>
>> Please refer Documentation\devicetree\bindings\iommu\iommu.txt on how to
>> bind device tree.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>>   arch/arm64/include/asm/device.h |  2 +
>>   drivers/iommu/arm-smmu-v3.c     | 88 +++++++++++++++++++++++++++++++++++++++--
>>   2 files changed, 87 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h
>> index 243ef25..225e4f9 100644
>> --- a/arch/arm64/include/asm/device.h
>> +++ b/arch/arm64/include/asm/device.h
>> @@ -20,6 +20,8 @@ struct dev_archdata {
>>       struct dma_map_ops *dma_ops;
>>   #ifdef CONFIG_IOMMU_API
>>       void *iommu;            /* private IOMMU data */
> 
> There's already a perfectly good place to store driver-private data right here.
> 
>> +    struct device_node *of_smmu;
>> +    u32 sid;
> 
> This looks far too specific to be in core code. It doesn't seem extensible for e.g. ACPI platform devices; it doesn't seem extensible for platform devices with multiple stream IDs e.g. PL330; it also
> doesn't seem (sensibly) extensible for IOMMUs with #iommu-cells > 1.
> 
> Just allocate an SMMU-private struct for this and stash it in archdata.iommu - that way we can change things as much as we like in the driver with zero churn in core code.

OK. If platform devices with multiple stream IDs exist, I will support it in patch V2.

> 
>>   #endif
>>       bool dma_coherent;
>>   };
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 483c918..87c3d9b 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -30,9 +30,14 @@
>>   #include <linux/of_address.h>
>>   #include <linux/pci.h>
>>   #include <linux/platform_device.h>
>> +#include <linux/amba/bus.h>
>> +#include <linux/of_iommu.h>
>>
>>   #include "io-pgtable.h"
>>
>> +/* Maximum number of stream IDs assigned to a single device */
>> +#define MAX_MASTER_STREAMIDS        1
>> +
>>   /* MMIO registers */
>>   #define ARM_SMMU_IDR0            0x0
>>   #define IDR0_ST_LVL_SHIFT        27
>> @@ -608,6 +613,22 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>>       return container_of(dom, struct arm_smmu_domain, domain);
>>   }
>>
>> +static struct arm_smmu_device *find_smmu_for_device(struct device *dev)
>> +{
>> +    struct arm_smmu_device *smmu;
>> +
>> +    spin_lock(&arm_smmu_devices_lock);
>> +    list_for_each_entry(smmu, &arm_smmu_devices, list) {
>> +        if (smmu->dev->of_node == dev->archdata.of_smmu) {
>> +            spin_unlock(&arm_smmu_devices_lock);
>> +            return smmu;
>> +        }
>> +    }
>> +    spin_unlock(&arm_smmu_devices_lock);
>> +
>> +    return NULL;
>> +}
>> +
> 
> This should be unnecessary with the right probe order, see below...
> 
>>   /* Low-level queue manipulation functions */
>>   static bool queue_full(struct arm_smmu_queue *q)
>>   {
>> @@ -1760,9 +1781,36 @@ static int arm_smmu_add_device(struct device *dev)
>>       struct arm_smmu_group *smmu_group;
>>       struct arm_smmu_device *smmu;
>>
>> -    /* We only support PCI, for now */
>> -    if (!dev_is_pci(dev))
>> -        return -ENODEV;
>> +    if (!dev_is_pci(dev)) {
>> +        smmu = find_smmu_for_device(dev);
>> +        if (!smmu)
>> +            return -ENODEV;
>> +
>> +        group = iommu_group_alloc();
>> +        if (IS_ERR(group)) {
>> +            dev_err(dev, "Failed to allocate IOMMU group\n");
>> +            return PTR_ERR(group);
>> +        }
>> +
>> +        ret = iommu_group_add_device(group, dev);
>> +        if (ret)
>> +            goto out_put_group;
>> +
>> +        smmu_group = kzalloc(sizeof(*smmu_group), GFP_KERNEL);
>> +        if (!smmu_group) {
>> +            ret = -ENOMEM;
>> +            goto out_put_group;
>> +        }
>> +
>> +        smmu_group->ste.valid = true;
>> +        smmu_group->smmu = smmu;
>> +        iommu_group_set_iommudata(group, smmu_group,
>> +                        __arm_smmu_release_iommudata);
>> +
>> +        sid = dev->archdata.sid;
>> +
>> +        goto handle_stream_id;
>> +    }
>>
>>       pdev = to_pci_dev(dev);
>>       group = iommu_group_get_for_dev(dev);
>> @@ -1793,6 +1841,8 @@ static int arm_smmu_add_device(struct device *dev)
>>
>>       /* Assume SID == RID until firmware tells us otherwise */
>>       pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid, &sid);
>> +
>> +handle_stream_id:
> 
> This is going to get messy quickly - how about breaking out the "platform device" and "PCI device" specifics above into their own functions that return the group and sid data to the common code here?
> 
>>       for (i = 0; i < smmu_group->num_sids; ++i) {
>>           /* If we already know about this SID, then we're done */
>>           if (smmu_group->sids[i] == sid)
>> @@ -1881,7 +1931,23 @@ out_unlock:
>>       return ret;
>>   }
>>
>> +static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
>> +{
>> +    if (args->args_count > MAX_MASTER_STREAMIDS) {
>> +        dev_err(dev,
>> +            "reached maximum number (%d) of stream IDs for master device %s\n",
>> +            MAX_MASTER_STREAMIDS, dev->of_node->name);
>> +        return -ENOSPC;
>> +    }
>> +
>> +    dev->archdata.of_smmu = args->np;
>> +    dev->archdata.sid = args->args[0];
>> +
>> +    return 0;
>> +}
> 
> This isn't going to work the way you expect: the way the binding is defined, a master with multiple stream IDs should look like so:
> 
>     iommus = <&smmu 0>, <&smmu 1>,...
> 
> so you'd get multiple calls, never hit the warning, and just end up with whichever ID came last.
> 
> Secondly, as mentioned above, it would be nicer to just associate the arm_smmu_device directly here and obviate the indirect lookup. That would depend on having correct probe ordering, but you need to
> enforce that anyway, since any add_device callbacks before the SMMU itself has been probed will break. Laurent's probe deferral series that Will pointed to is the ultimate goal, but for a stop-gap
> solution which works with the current code I'd suggest taking a look at patches 16 and 17 of Marek's Exynos SysMMU series[1]

OK, thanks. I will consider these in patch v2.

> 
> Robin.
> 
> [1]:http://thread.gmane.org/gmane.linux.kernel.samsung-soc/45416
> 
>> +
>>   static struct iommu_ops arm_smmu_ops = {
>> +    .of_xlate        = arm_smmu_of_xlate,
>>       .capable        = arm_smmu_capable,
>>       .domain_alloc        = arm_smmu_domain_alloc,
>>       .domain_free        = arm_smmu_domain_free,
>> @@ -2655,6 +2721,14 @@ static int __init arm_smmu_init(void)
>>       if (ret)
>>           return ret;
>>
>> +    if (!iommu_present(&platform_bus_type))
>> +        bus_set_iommu(&platform_bus_type, &arm_smmu_ops);
>> +
>> +#ifdef CONFIG_ARM_AMBA
>> +    if (!iommu_present(&amba_bustype))
>> +        bus_set_iommu(&amba_bustype, &arm_smmu_ops);
>> +#endif
>> +
>>       return bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
>>   }
>>
>> @@ -2666,6 +2740,14 @@ static void __exit arm_smmu_exit(void)
>>   subsys_initcall(arm_smmu_init);
>>   module_exit(arm_smmu_exit);
>>
>> +static int arm_smmu_of_iommu_init(struct device_node *np)
>> +{
>> +    of_iommu_set_ops(np, &arm_smmu_ops);
>> +
>> +    return 0;
>> +}
>> +IOMMU_OF_DECLARE(arm_smmu_v3, "arm,smmu-v3", arm_smmu_of_iommu_init);
>> +
>>   MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
>>   MODULE_AUTHOR("Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>");
>>   MODULE_LICENSE("GPL v2");
>> -- 
>> 1.8.0
>>
>>
>> _______________________________________________
>> iommu mailing list
>> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>>
> 
> 
> .
> 

[PATCH 6/8] iommu/arm-smmu: add support for non-pci devices

[leizhen]

On 2015/6/30 19:26, Robin Murphy wrote:
> On 26/06/15 09:33, Zhen Lei wrote:
>> Now, we only support a master with only one stream id. It will cover most
>> hardware platforms and coding so easy.
>>
>> Please refer Documentation\devicetree\bindings\iommu\iommu.txt on how to
>> bind device tree.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>   arch/arm64/include/asm/device.h |  2 +
>>   drivers/iommu/arm-smmu-v3.c     | 88 +++++++++++++++++++++++++++++++++++++++--
>>   2 files changed, 87 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h
>> index 243ef25..225e4f9 100644
>> --- a/arch/arm64/include/asm/device.h
>> +++ b/arch/arm64/include/asm/device.h
>> @@ -20,6 +20,8 @@ struct dev_archdata {
>>       struct dma_map_ops *dma_ops;
>>   #ifdef CONFIG_IOMMU_API
>>       void *iommu;            /* private IOMMU data */
> 
> There's already a perfectly good place to store driver-private data right here.
> 
>> +    struct device_node *of_smmu;
>> +    u32 sid;
> 
> This looks far too specific to be in core code. It doesn't seem extensible for e.g. ACPI platform devices; it doesn't seem extensible for platform devices with multiple stream IDs e.g. PL330; it also
> doesn't seem (sensibly) extensible for IOMMUs with #iommu-cells > 1.
> 
> Just allocate an SMMU-private struct for this and stash it in archdata.iommu - that way we can change things as much as we like in the driver with zero churn in core code.

OK. If platform devices with multiple stream IDs exist, I will support it in patch V2.

> 
>>   #endif
>>       bool dma_coherent;
>>   };
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 483c918..87c3d9b 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -30,9 +30,14 @@
>>   #include <linux/of_address.h>
>>   #include <linux/pci.h>
>>   #include <linux/platform_device.h>
>> +#include <linux/amba/bus.h>
>> +#include <linux/of_iommu.h>
>>
>>   #include "io-pgtable.h"
>>
>> +/* Maximum number of stream IDs assigned to a single device */
>> +#define MAX_MASTER_STREAMIDS        1
>> +
>>   /* MMIO registers */
>>   #define ARM_SMMU_IDR0            0x0
>>   #define IDR0_ST_LVL_SHIFT        27
>> @@ -608,6 +613,22 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>>       return container_of(dom, struct arm_smmu_domain, domain);
>>   }
>>
>> +static struct arm_smmu_device *find_smmu_for_device(struct device *dev)
>> +{
>> +    struct arm_smmu_device *smmu;
>> +
>> +    spin_lock(&arm_smmu_devices_lock);
>> +    list_for_each_entry(smmu, &arm_smmu_devices, list) {
>> +        if (smmu->dev->of_node == dev->archdata.of_smmu) {
>> +            spin_unlock(&arm_smmu_devices_lock);
>> +            return smmu;
>> +        }
>> +    }
>> +    spin_unlock(&arm_smmu_devices_lock);
>> +
>> +    return NULL;
>> +}
>> +
> 
> This should be unnecessary with the right probe order, see below...
> 
>>   /* Low-level queue manipulation functions */
>>   static bool queue_full(struct arm_smmu_queue *q)
>>   {
>> @@ -1760,9 +1781,36 @@ static int arm_smmu_add_device(struct device *dev)
>>       struct arm_smmu_group *smmu_group;
>>       struct arm_smmu_device *smmu;
>>
>> -    /* We only support PCI, for now */
>> -    if (!dev_is_pci(dev))
>> -        return -ENODEV;
>> +    if (!dev_is_pci(dev)) {
>> +        smmu = find_smmu_for_device(dev);
>> +        if (!smmu)
>> +            return -ENODEV;
>> +
>> +        group = iommu_group_alloc();
>> +        if (IS_ERR(group)) {
>> +            dev_err(dev, "Failed to allocate IOMMU group\n");
>> +            return PTR_ERR(group);
>> +        }
>> +
>> +        ret = iommu_group_add_device(group, dev);
>> +        if (ret)
>> +            goto out_put_group;
>> +
>> +        smmu_group = kzalloc(sizeof(*smmu_group), GFP_KERNEL);
>> +        if (!smmu_group) {
>> +            ret = -ENOMEM;
>> +            goto out_put_group;
>> +        }
>> +
>> +        smmu_group->ste.valid = true;
>> +        smmu_group->smmu = smmu;
>> +        iommu_group_set_iommudata(group, smmu_group,
>> +                        __arm_smmu_release_iommudata);
>> +
>> +        sid = dev->archdata.sid;
>> +
>> +        goto handle_stream_id;
>> +    }
>>
>>       pdev = to_pci_dev(dev);
>>       group = iommu_group_get_for_dev(dev);
>> @@ -1793,6 +1841,8 @@ static int arm_smmu_add_device(struct device *dev)
>>
>>       /* Assume SID == RID until firmware tells us otherwise */
>>       pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid, &sid);
>> +
>> +handle_stream_id:
> 
> This is going to get messy quickly - how about breaking out the "platform device" and "PCI device" specifics above into their own functions that return the group and sid data to the common code here?
> 
>>       for (i = 0; i < smmu_group->num_sids; ++i) {
>>           /* If we already know about this SID, then we're done */
>>           if (smmu_group->sids[i] == sid)
>> @@ -1881,7 +1931,23 @@ out_unlock:
>>       return ret;
>>   }
>>
>> +static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
>> +{
>> +    if (args->args_count > MAX_MASTER_STREAMIDS) {
>> +        dev_err(dev,
>> +            "reached maximum number (%d) of stream IDs for master device %s\n",
>> +            MAX_MASTER_STREAMIDS, dev->of_node->name);
>> +        return -ENOSPC;
>> +    }
>> +
>> +    dev->archdata.of_smmu = args->np;
>> +    dev->archdata.sid = args->args[0];
>> +
>> +    return 0;
>> +}
> 
> This isn't going to work the way you expect: the way the binding is defined, a master with multiple stream IDs should look like so:
> 
>     iommus = <&smmu 0>, <&smmu 1>,...
> 
> so you'd get multiple calls, never hit the warning, and just end up with whichever ID came last.
> 
> Secondly, as mentioned above, it would be nicer to just associate the arm_smmu_device directly here and obviate the indirect lookup. That would depend on having correct probe ordering, but you need to
> enforce that anyway, since any add_device callbacks before the SMMU itself has been probed will break. Laurent's probe deferral series that Will pointed to is the ultimate goal, but for a stop-gap
> solution which works with the current code I'd suggest taking a look at patches 16 and 17 of Marek's Exynos SysMMU series[1]

OK, thanks. I will consider these in patch v2.

> 
> Robin.
> 
> [1]:http://thread.gmane.org/gmane.linux.kernel.samsung-soc/45416
> 
>> +
>>   static struct iommu_ops arm_smmu_ops = {
>> +    .of_xlate        = arm_smmu_of_xlate,
>>       .capable        = arm_smmu_capable,
>>       .domain_alloc        = arm_smmu_domain_alloc,
>>       .domain_free        = arm_smmu_domain_free,
>> @@ -2655,6 +2721,14 @@ static int __init arm_smmu_init(void)
>>       if (ret)
>>           return ret;
>>
>> +    if (!iommu_present(&platform_bus_type))
>> +        bus_set_iommu(&platform_bus_type, &arm_smmu_ops);
>> +
>> +#ifdef CONFIG_ARM_AMBA
>> +    if (!iommu_present(&amba_bustype))
>> +        bus_set_iommu(&amba_bustype, &arm_smmu_ops);
>> +#endif
>> +
>>       return bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
>>   }
>>
>> @@ -2666,6 +2740,14 @@ static void __exit arm_smmu_exit(void)
>>   subsys_initcall(arm_smmu_init);
>>   module_exit(arm_smmu_exit);
>>
>> +static int arm_smmu_of_iommu_init(struct device_node *np)
>> +{
>> +    of_iommu_set_ops(np, &arm_smmu_ops);
>> +
>> +    return 0;
>> +}
>> +IOMMU_OF_DECLARE(arm_smmu_v3, "arm,smmu-v3", arm_smmu_of_iommu_init);
>> +
>>   MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
>>   MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
>>   MODULE_LICENSE("GPL v2");
>> -- 
>> 1.8.0
>>
>>
>> _______________________________________________
>> iommu mailing list
>> iommu at lists.linux-foundation.org
>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>>
> 
> 
> .
> 

[PATCH 7/8] iommu/arm-smmu: enlarge STRTAB_L1_SZ_SHIFT to support larger sidsize

[Zhen Lei]

Because we will choose the minimum value between STRTAB_L1_SZ_SHIFT and
IDR1.SIDSIZE, so enlarge STRTAB_L1_SZ_SHIFT will not impact the platforms
whose IDR1.SIDSIZE is smaller than old STRTAB_L1_SZ_SHIFT value.

Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 87c3d9b..d799feb 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -206,7 +206,7 @@
  * Linear: Enough to cover 1 << IDR1.SIDSIZE entries
  * 2lvl: 8k L1 entries, 256 lazy entries per table (each table covers a PCI bus)
  */
-#define STRTAB_L1_SZ_SHIFT		16
+#define STRTAB_L1_SZ_SHIFT		20
 #define STRTAB_SPLIT			8

 #define STRTAB_L1_DESC_DWORDS		1
--
1.8.0

[PATCH 7/8] iommu/arm-smmu: enlarge STRTAB_L1_SZ_SHIFT to support larger sidsize

[Zhen Lei]

Because we will choose the minimum value between STRTAB_L1_SZ_SHIFT and
IDR1.SIDSIZE, so enlarge STRTAB_L1_SZ_SHIFT will not impact the platforms
whose IDR1.SIDSIZE is smaller than old STRTAB_L1_SZ_SHIFT value.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/iommu/arm-smmu-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 87c3d9b..d799feb 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -206,7 +206,7 @@
  * Linear: Enough to cover 1 << IDR1.SIDSIZE entries
  * 2lvl: 8k L1 entries, 256 lazy entries per table (each table covers a PCI bus)
  */
-#define STRTAB_L1_SZ_SHIFT		16
+#define STRTAB_L1_SZ_SHIFT		20
 #define STRTAB_SPLIT			8

 #define STRTAB_L1_DESC_DWORDS		1
--
1.8.0

Re: [PATCH 7/8] iommu/arm-smmu: enlarge STRTAB_L1_SZ_SHIFT to support larger sidsize

[Will Deacon]

On Fri, Jun 26, 2015 at 09:33:03AM +0100, Zhen Lei wrote:
> Because we will choose the minimum value between STRTAB_L1_SZ_SHIFT and
> IDR1.SIDSIZE, so enlarge STRTAB_L1_SZ_SHIFT will not impact the platforms
> whose IDR1.SIDSIZE is smaller than old STRTAB_L1_SZ_SHIFT value.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu-v3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 87c3d9b..d799feb 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -206,7 +206,7 @@
>   * Linear: Enough to cover 1 << IDR1.SIDSIZE entries
>   * 2lvl: 8k L1 entries, 256 lazy entries per table (each table covers a PCI bus)
>   */
> -#define STRTAB_L1_SZ_SHIFT		16
> +#define STRTAB_L1_SZ_SHIFT		20
>  #define STRTAB_SPLIT			8
> 
>  #define STRTAB_L1_DESC_DWORDS		1

Can you update the comment too, please, to say that we now have 128k entries
at level 1?

Will

[PATCH 7/8] iommu/arm-smmu: enlarge STRTAB_L1_SZ_SHIFT to support larger sidsize

[Will Deacon]

On Fri, Jun 26, 2015 at 09:33:03AM +0100, Zhen Lei wrote:
> Because we will choose the minimum value between STRTAB_L1_SZ_SHIFT and
> IDR1.SIDSIZE, so enlarge STRTAB_L1_SZ_SHIFT will not impact the platforms
> whose IDR1.SIDSIZE is smaller than old STRTAB_L1_SZ_SHIFT value.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 87c3d9b..d799feb 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -206,7 +206,7 @@
>   * Linear: Enough to cover 1 << IDR1.SIDSIZE entries
>   * 2lvl: 8k L1 entries, 256 lazy entries per table (each table covers a PCI bus)
>   */
> -#define STRTAB_L1_SZ_SHIFT		16
> +#define STRTAB_L1_SZ_SHIFT		20
>  #define STRTAB_SPLIT			8
> 
>  #define STRTAB_L1_DESC_DWORDS		1

Can you update the comment too, please, to say that we now have 128k entries
at level 1?

Will

Re: [PATCH 7/8] iommu/arm-smmu: enlarge STRTAB_L1_SZ_SHIFT to support larger sidsize

[leizhen]

On 2015/6/30 1:35, Will Deacon wrote:
> On Fri, Jun 26, 2015 at 09:33:03AM +0100, Zhen Lei wrote:
>> Because we will choose the minimum value between STRTAB_L1_SZ_SHIFT and
>> IDR1.SIDSIZE, so enlarge STRTAB_L1_SZ_SHIFT will not impact the platforms
>> whose IDR1.SIDSIZE is smaller than old STRTAB_L1_SZ_SHIFT value.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 87c3d9b..d799feb 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -206,7 +206,7 @@
>>   * Linear: Enough to cover 1 << IDR1.SIDSIZE entries
>>   * 2lvl: 8k L1 entries, 256 lazy entries per table (each table covers a PCI bus)
>>   */
>> -#define STRTAB_L1_SZ_SHIFT		16
>> +#define STRTAB_L1_SZ_SHIFT		20
>>  #define STRTAB_SPLIT			8
>>
>>  #define STRTAB_L1_DESC_DWORDS		1
> 
> Can you update the comment too, please, to say that we now have 128k entries
> at level 1?

OK, no problem.

> 
> Will
> 
> .
> 

[PATCH 7/8] iommu/arm-smmu: enlarge STRTAB_L1_SZ_SHIFT to support larger sidsize

[leizhen]

On 2015/6/30 1:35, Will Deacon wrote:
> On Fri, Jun 26, 2015 at 09:33:03AM +0100, Zhen Lei wrote:
>> Because we will choose the minimum value between STRTAB_L1_SZ_SHIFT and
>> IDR1.SIDSIZE, so enlarge STRTAB_L1_SZ_SHIFT will not impact the platforms
>> whose IDR1.SIDSIZE is smaller than old STRTAB_L1_SZ_SHIFT value.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 87c3d9b..d799feb 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -206,7 +206,7 @@
>>   * Linear: Enough to cover 1 << IDR1.SIDSIZE entries
>>   * 2lvl: 8k L1 entries, 256 lazy entries per table (each table covers a PCI bus)
>>   */
>> -#define STRTAB_L1_SZ_SHIFT		16
>> +#define STRTAB_L1_SZ_SHIFT		20
>>  #define STRTAB_SPLIT			8
>>
>>  #define STRTAB_L1_DESC_DWORDS		1
> 
> Can you update the comment too, please, to say that we now have 128k entries
> at level 1?

OK, no problem.

> 
> Will
> 
> .
> 

[PATCH 8/8] iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG execution

[Zhen Lei]

Some broken SMMUv3 devices treat CMD_PREFETCH_CONFIG as illegal command,
it's ugly to print error information. CMD_PREFETCH_CONFIG is just used to
prefetch the configuration for a specified StreamID, without this command
will not impact SMMUv3 function.

Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index d799feb..3971aef 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -813,6 +813,7 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
 	};

 	int i;
+	u32 opcode;
 	u64 cmd[CMDQ_ENT_DWORDS];
 	struct arm_smmu_queue *q = &smmu->cmdq.q;
 	u32 cons = readl_relaxed(q->cons_reg);
@@ -821,15 +822,14 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
 		.opcode = CMDQ_OP_CMD_SYNC,
 	};

-	dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
-		cerror_str[idx]);
-
 	switch (idx) {
 	case CMDQ_ERR_CERROR_ILL_IDX:
 		break;
 	case CMDQ_ERR_CERROR_ABT_IDX:
-		dev_err(smmu->dev, "retrying command fetch\n");
 	case CMDQ_ERR_CERROR_NONE_IDX:
+		dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
+			cerror_str[idx]);
+		dev_err(smmu->dev, "retrying command execution\n");
 		return;
 	}

@@ -838,10 +838,20 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
 	 * not to touch any of the shadow cmdq state.
 	 */
 	queue_read(cmd, Q_ENT(q, idx), q->ent_dwords);
+
+	/*
+	 * Some broken SMMUv3 devices treat CMD_PREFETCH_CONFIG as illegal
+	 * command, it's ugly to print error information.
+	 */
+	opcode = (cmd[0] >> CMDQ_0_OP_SHIFT) & CMDQ_0_OP_MASK;
+	if (opcode == CMDQ_OP_PREFETCH_CFG)
+		goto skip_error_command;
+
 	dev_err(smmu->dev, "skipping command in error state:\n");
 	for (i = 0; i < ARRAY_SIZE(cmd); ++i)
 		dev_err(smmu->dev, "\t0x%016llx\n", (unsigned long long)cmd[i]);

+skip_error_command:
 	/* Convert the erroneous command into a CMD_SYNC */
 	if (arm_smmu_cmdq_build_cmd(cmd, &cmd_sync)) {
 		dev_err(smmu->dev, "failed to convert to CMD_SYNC\n");
--
1.8.0

[PATCH 8/8] iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG execution

[Zhen Lei]

Some broken SMMUv3 devices treat CMD_PREFETCH_CONFIG as illegal command,
it's ugly to print error information. CMD_PREFETCH_CONFIG is just used to
prefetch the configuration for a specified StreamID, without this command
will not impact SMMUv3 function.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/iommu/arm-smmu-v3.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index d799feb..3971aef 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -813,6 +813,7 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
 	};

 	int i;
+	u32 opcode;
 	u64 cmd[CMDQ_ENT_DWORDS];
 	struct arm_smmu_queue *q = &smmu->cmdq.q;
 	u32 cons = readl_relaxed(q->cons_reg);
@@ -821,15 +822,14 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
 		.opcode = CMDQ_OP_CMD_SYNC,
 	};

-	dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
-		cerror_str[idx]);
-
 	switch (idx) {
 	case CMDQ_ERR_CERROR_ILL_IDX:
 		break;
 	case CMDQ_ERR_CERROR_ABT_IDX:
-		dev_err(smmu->dev, "retrying command fetch\n");
 	case CMDQ_ERR_CERROR_NONE_IDX:
+		dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
+			cerror_str[idx]);
+		dev_err(smmu->dev, "retrying command execution\n");
 		return;
 	}

@@ -838,10 +838,20 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
 	 * not to touch any of the shadow cmdq state.
 	 */
 	queue_read(cmd, Q_ENT(q, idx), q->ent_dwords);
+
+	/*
+	 * Some broken SMMUv3 devices treat CMD_PREFETCH_CONFIG as illegal
+	 * command, it's ugly to print error information.
+	 */
+	opcode = (cmd[0] >> CMDQ_0_OP_SHIFT) & CMDQ_0_OP_MASK;
+	if (opcode == CMDQ_OP_PREFETCH_CFG)
+		goto skip_error_command;
+
 	dev_err(smmu->dev, "skipping command in error state:\n");
 	for (i = 0; i < ARRAY_SIZE(cmd); ++i)
 		dev_err(smmu->dev, "\t0x%016llx\n", (unsigned long long)cmd[i]);

+skip_error_command:
 	/* Convert the erroneous command into a CMD_SYNC */
 	if (arm_smmu_cmdq_build_cmd(cmd, &cmd_sync)) {
 		dev_err(smmu->dev, "failed to convert to CMD_SYNC\n");
--
1.8.0

Re: [PATCH 8/8] iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG execution

[Will Deacon]

On Fri, Jun 26, 2015 at 09:33:04AM +0100, Zhen Lei wrote:
> Some broken SMMUv3 devices treat CMD_PREFETCH_CONFIG as illegal command,
> it's ugly to print error information. CMD_PREFETCH_CONFIG is just used to
> prefetch the configuration for a specified StreamID, without this command
> will not impact SMMUv3 function.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu-v3.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index d799feb..3971aef 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -813,6 +813,7 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
>  	};
> 
>  	int i;
> +	u32 opcode;
>  	u64 cmd[CMDQ_ENT_DWORDS];
>  	struct arm_smmu_queue *q = &smmu->cmdq.q;
>  	u32 cons = readl_relaxed(q->cons_reg);
> @@ -821,15 +822,14 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
>  		.opcode = CMDQ_OP_CMD_SYNC,
>  	};
> 
> -	dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
> -		cerror_str[idx]);
> -
>  	switch (idx) {
>  	case CMDQ_ERR_CERROR_ILL_IDX:
>  		break;
>  	case CMDQ_ERR_CERROR_ABT_IDX:
> -		dev_err(smmu->dev, "retrying command fetch\n");
>  	case CMDQ_ERR_CERROR_NONE_IDX:
> +		dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
> +			cerror_str[idx]);
> +		dev_err(smmu->dev, "retrying command execution\n");

Why are you making this (unrelated) change?

Out of interest, did the gerror code manage to turn the prefetch into
a SYNC, as intended?

>  		return;
>  	}
> 
> @@ -838,10 +838,20 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
>  	 * not to touch any of the shadow cmdq state.
>  	 */
>  	queue_read(cmd, Q_ENT(q, idx), q->ent_dwords);
> +
> +	/*
> +	 * Some broken SMMUv3 devices treat CMD_PREFETCH_CONFIG as illegal
> +	 * command, it's ugly to print error information.
> +	 */
> +	opcode = (cmd[0] >> CMDQ_0_OP_SHIFT) & CMDQ_0_OP_MASK;
> +	if (opcode == CMDQ_OP_PREFETCH_CFG)
> +		goto skip_error_command;
> +

I think a better way to fix this would be to add a quick to the device-tree
along the lines of "hisilicon,broken-prefetch-cmd" which we could check
before issuing the prefetch command in arm_smmu_write_strtab_ent.

Something along the lines of the arm_smmu_options in arm-smmu.c would be
a good basis for quirks.

Will

[PATCH 8/8] iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG execution

[Will Deacon]

On Fri, Jun 26, 2015 at 09:33:04AM +0100, Zhen Lei wrote:
> Some broken SMMUv3 devices treat CMD_PREFETCH_CONFIG as illegal command,
> it's ugly to print error information. CMD_PREFETCH_CONFIG is just used to
> prefetch the configuration for a specified StreamID, without this command
> will not impact SMMUv3 function.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index d799feb..3971aef 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -813,6 +813,7 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
>  	};
> 
>  	int i;
> +	u32 opcode;
>  	u64 cmd[CMDQ_ENT_DWORDS];
>  	struct arm_smmu_queue *q = &smmu->cmdq.q;
>  	u32 cons = readl_relaxed(q->cons_reg);
> @@ -821,15 +822,14 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
>  		.opcode = CMDQ_OP_CMD_SYNC,
>  	};
> 
> -	dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
> -		cerror_str[idx]);
> -
>  	switch (idx) {
>  	case CMDQ_ERR_CERROR_ILL_IDX:
>  		break;
>  	case CMDQ_ERR_CERROR_ABT_IDX:
> -		dev_err(smmu->dev, "retrying command fetch\n");
>  	case CMDQ_ERR_CERROR_NONE_IDX:
> +		dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
> +			cerror_str[idx]);
> +		dev_err(smmu->dev, "retrying command execution\n");

Why are you making this (unrelated) change?

Out of interest, did the gerror code manage to turn the prefetch into
a SYNC, as intended?

>  		return;
>  	}
> 
> @@ -838,10 +838,20 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
>  	 * not to touch any of the shadow cmdq state.
>  	 */
>  	queue_read(cmd, Q_ENT(q, idx), q->ent_dwords);
> +
> +	/*
> +	 * Some broken SMMUv3 devices treat CMD_PREFETCH_CONFIG as illegal
> +	 * command, it's ugly to print error information.
> +	 */
> +	opcode = (cmd[0] >> CMDQ_0_OP_SHIFT) & CMDQ_0_OP_MASK;
> +	if (opcode == CMDQ_OP_PREFETCH_CFG)
> +		goto skip_error_command;
> +

I think a better way to fix this would be to add a quick to the device-tree
along the lines of "hisilicon,broken-prefetch-cmd" which we could check
before issuing the prefetch command in arm_smmu_write_strtab_ent.

Something along the lines of the arm_smmu_options in arm-smmu.c would be
a good basis for quirks.

Will

Re: [PATCH 8/8] iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG execution

[leizhen]

On 2015/6/30 1:49, Will Deacon wrote:
> On Fri, Jun 26, 2015 at 09:33:04AM +0100, Zhen Lei wrote:
>> Some broken SMMUv3 devices treat CMD_PREFETCH_CONFIG as illegal command,
>> it's ugly to print error information. CMD_PREFETCH_CONFIG is just used to
>> prefetch the configuration for a specified StreamID, without this command
>> will not impact SMMUv3 function.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index d799feb..3971aef 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -813,6 +813,7 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
>>  	};
>>
>>  	int i;
>> +	u32 opcode;
>>  	u64 cmd[CMDQ_ENT_DWORDS];
>>  	struct arm_smmu_queue *q = &smmu->cmdq.q;
>>  	u32 cons = readl_relaxed(q->cons_reg);
>> @@ -821,15 +822,14 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
>>  		.opcode = CMDQ_OP_CMD_SYNC,
>>  	};
>>
>> -	dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
>> -		cerror_str[idx]);
>> -
>>  	switch (idx) {
>>  	case CMDQ_ERR_CERROR_ILL_IDX:
>>  		break;
>>  	case CMDQ_ERR_CERROR_ABT_IDX:
>> -		dev_err(smmu->dev, "retrying command fetch\n");
>>  	case CMDQ_ERR_CERROR_NONE_IDX:
>> +		dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
>> +			cerror_str[idx]);
>> +		dev_err(smmu->dev, "retrying command execution\n");
> 
> Why are you making this (unrelated) change?
> 
> Out of interest, did the gerror code manage to turn the prefetch into
> a SYNC, as intended?

Yes, It worked fine. The program can continue to run normally, that's why I don't want to
print these error information. Because this problem is known on my platform, and can sliently
ignored.

> 
>>  		return;
>>  	}
>>
>> @@ -838,10 +838,20 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
>>  	 * not to touch any of the shadow cmdq state.
>>  	 */
>>  	queue_read(cmd, Q_ENT(q, idx), q->ent_dwords);
>> +
>> +	/*
>> +	 * Some broken SMMUv3 devices treat CMD_PREFETCH_CONFIG as illegal
>> +	 * command, it's ugly to print error information.
>> +	 */
>> +	opcode = (cmd[0] >> CMDQ_0_OP_SHIFT) & CMDQ_0_OP_MASK;
>> +	if (opcode == CMDQ_OP_PREFETCH_CFG)
>> +		goto skip_error_command;
>> +
> 
> I think a better way to fix this would be to add a quick to the device-tree
> along the lines of "hisilicon,broken-prefetch-cmd" which we could check
> before issuing the prefetch command in arm_smmu_write_strtab_ent.
> 
> Something along the lines of the arm_smmu_options in arm-smmu.c would be
> a good basis for quirks.

It's a good idea. I known the arm_smmu_options, I will rewrite this patch in V2.

> 
> Will
> 
> .
> 

[PATCH 8/8] iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG execution

[leizhen]

On 2015/6/30 1:49, Will Deacon wrote:
> On Fri, Jun 26, 2015 at 09:33:04AM +0100, Zhen Lei wrote:
>> Some broken SMMUv3 devices treat CMD_PREFETCH_CONFIG as illegal command,
>> it's ugly to print error information. CMD_PREFETCH_CONFIG is just used to
>> prefetch the configuration for a specified StreamID, without this command
>> will not impact SMMUv3 function.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index d799feb..3971aef 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -813,6 +813,7 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
>>  	};
>>
>>  	int i;
>> +	u32 opcode;
>>  	u64 cmd[CMDQ_ENT_DWORDS];
>>  	struct arm_smmu_queue *q = &smmu->cmdq.q;
>>  	u32 cons = readl_relaxed(q->cons_reg);
>> @@ -821,15 +822,14 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
>>  		.opcode = CMDQ_OP_CMD_SYNC,
>>  	};
>>
>> -	dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
>> -		cerror_str[idx]);
>> -
>>  	switch (idx) {
>>  	case CMDQ_ERR_CERROR_ILL_IDX:
>>  		break;
>>  	case CMDQ_ERR_CERROR_ABT_IDX:
>> -		dev_err(smmu->dev, "retrying command fetch\n");
>>  	case CMDQ_ERR_CERROR_NONE_IDX:
>> +		dev_err(smmu->dev, "CMDQ error (cons 0x%08x): %s\n", cons,
>> +			cerror_str[idx]);
>> +		dev_err(smmu->dev, "retrying command execution\n");
> 
> Why are you making this (unrelated) change?
> 
> Out of interest, did the gerror code manage to turn the prefetch into
> a SYNC, as intended?

Yes, It worked fine. The program can continue to run normally, that's why I don't want to
print these error information. Because this problem is known on my platform, and can sliently
ignored.

> 
>>  		return;
>>  	}
>>
>> @@ -838,10 +838,20 @@ static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu)
>>  	 * not to touch any of the shadow cmdq state.
>>  	 */
>>  	queue_read(cmd, Q_ENT(q, idx), q->ent_dwords);
>> +
>> +	/*
>> +	 * Some broken SMMUv3 devices treat CMD_PREFETCH_CONFIG as illegal
>> +	 * command, it's ugly to print error information.
>> +	 */
>> +	opcode = (cmd[0] >> CMDQ_0_OP_SHIFT) & CMDQ_0_OP_MASK;
>> +	if (opcode == CMDQ_OP_PREFETCH_CFG)
>> +		goto skip_error_command;
>> +
> 
> I think a better way to fix this would be to add a quick to the device-tree
> along the lines of "hisilicon,broken-prefetch-cmd" which we could check
> before issuing the prefetch command in arm_smmu_write_strtab_ent.
> 
> Something along the lines of the arm_smmu_options in arm-smmu.c would be
> a good basis for quirks.

It's a good idea. I known the arm_smmu_options, I will rewrite this patch in V2.

> 
> Will
> 
> .
>