iommu/arm-smmu: bugfixs and add support for non-pci devices
cover iommu/arm-smmu: bugfixs and add support for non-pci devices
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
Add a new local variable to store the value of log2size, so that it will not be overridden by L1 table size.
--- drivers/iommu/arm-smmu-v3.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/iommu/arm-smmu-v3.c +7 -7
@@ -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
Hi Zhen Lei,
Quoted 18 lines · 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 <[email protected]>--- drivers/iommu/arm-smmu-v3.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)drivers/iommu/arm-smmu-v3.c +1
-1@@ -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;
drivers/iommu/arm-smmu-v3.c static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu) · line 2031
Quoted 10 lines
drivers/iommu/arm-smmu-v3.c +3
-3int 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
drivers/iommu/arm-smmu-v3.c +9 -8
@@ -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
Quoted 24 lines · Will Deacon
Hi Zhen Lei,
Quoted 21 lines · 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 <[email protected]>--- drivers/iommu/arm-smmu-v3.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)drivers/iommu/arm-smmu-v3.c +1
-1@@ -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;
Quoted 11 lines
drivers/iommu/arm-smmu-v3.c static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu) · line 2031
Quoted 7 lines
drivers/iommu/arm-smmu-v3.c +3
-3/* 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.
Quoted 52 lines
Will
--->8
drivers/iommu/arm-smmu-v3.c +9
-8@@ -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
In reply to Zhen LeiThe element size of cfg->strtab is just one DWORD, should use multiply operation.
--- drivers/iommu/arm-smmu-v3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iommu/arm-smmu-v3.c +1 -1
@@ -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
Quoted 2 lines · Zhen Lei
The element size of
cfg->strtabis 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
Quoted 22 lines
Signed-off-by Zhen Lei <[email protected]>--- drivers/iommu/arm-smmu-v3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/iommu/arm-smmu-v3.c +1
-2@@ -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
In reply to Zhen LeiIn context descriptor, the offset of IR0 is 8, the offset of OR0 is 10.
--- drivers/iommu/arm-smmu-v3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iommu/arm-smmu-v3.c +2 -2
@@ -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
Quoted 14 lines · Zhen Lei
In context descriptor, the offset of IR0 is 8, the offset of OR0 is 10.
Signed-off-by Zhen Lei <[email protected]>--- drivers/iommu/arm-smmu-v3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/iommu/arm-smmu-v3.c +0
-0@@ -269,10 +269,10 @@#define ARM64_TCR_TG0_SHIFT 14#define ARM64_TCR_TG0_MASK 0x3UL
drivers/iommu/arm-smmu-v3.c line 278
Quoted 10 lines
drivers/iommu/arm-smmu-v3.c +2
-2#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
Quoted 28 lines · Will Deacon
Quoted 14 lines · Zhen Lei
In context descriptor, the offset of IR0 is 8, the offset of OR0 is 10.
Signed-off-by Zhen Lei <[email protected]>--- drivers/iommu/arm-smmu-v3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/iommu/arm-smmu-v3.c +0
-0@@ -269,10 +269,10 @@#define ARM64_TCR_TG0_SHIFT 14#define ARM64_TCR_TG0_MASK 0x3ULdrivers/iommu/arm-smmu-v3.c line 278
Quoted 10 lines
drivers/iommu/arm-smmu-v3.c +2
-2#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 12I don't understand this patch.
Quoted 4 lines
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.
Quoted 5 lines
Will
.
PATCH 3/8 iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT
Quoted 36 lines · leizhen
Quoted 28 lines · Will Deacon
Quoted 14 lines · Zhen Lei
In context descriptor, the offset of IR0 is 8, the offset of OR0 is 10.
Signed-off-by Zhen Lei <[email protected]>--- drivers/iommu/arm-smmu-v3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/iommu/arm-smmu-v3.c +0
-0@@ -269,10 +269,10 @@#define ARM64_TCR_TG0_SHIFT 14#define ARM64_TCR_TG0_MASK 0x3ULdrivers/iommu/arm-smmu-v3.c line 278
Quoted 10 lines
drivers/iommu/arm-smmu-v3.c +2
-2#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 12I don't understand this patch.
Quoted 4 lines
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.
Quoted 7 lines
In
io-pgtable-arm.c:
#defineARM_LPAE_TCR_ORGN0_SHIFT10
#defineARM_LPAE_TCR_IRGN0_SHIFT8So, the description should be modified as below:
InSMMU_CBn_TCRwhen 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
In reply to Zhen LeiNow we only use TT0 translation, disable TT1 translation will safer.
--- drivers/iommu/arm-smmu-v3.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/iommu/arm-smmu-v3.c +2 -1
@@ -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
Quoted 20 lines · Zhen Lei
Now we only use TT0 translation, disable TT1 translation will safer.
Signed-off-by Zhen Lei <[email protected]>--- drivers/iommu/arm-smmu-v3.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/iommu/arm-smmu-v3.c +1
-0@@ -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
drivers/iommu/arm-smmu-v3.c static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu, · line 897
Quoted 5 lines
drivers/iommu/arm-smmu-v3.c +1
-1@@ -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
Quoted 19 lines · Will Deacon
Quoted 18 lines · Zhen Lei
Now we only use TT0 translation, disable TT1 translation will safer.
Signed-off-by Zhen Lei <[email protected]>--- drivers/iommu/arm-smmu-v3.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/iommu/arm-smmu-v3.c +1
-0@@ -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)
Quoted 10 lines
drivers/iommu/arm-smmu-v3.c static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu, · line 897
Quoted 7 lines
drivers/iommu/arm-smmu-v3.c +1
-1#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.
Quoted 5 lines
Will
.
PATCH 5/8 iommu/arm-smmu: rename __arm_smmu_get_pci_sid
In reply to Zhen LeiRemove the words "pci", to make this function can also be used by non-pci devices.
--- drivers/iommu/arm-smmu-v3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iommu/arm-smmu-v3.c +2 -2
@@ -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
In reply to Zhen LeiNow, 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.
--- arch/arm64/include/asm/device.h | 2 + drivers/iommu/arm-smmu-v3.c | 88 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 87 insertions(+), 3 deletions(-)
arch/arm64/include/asm/device.h +2 -0
@@ -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; };
drivers/iommu/arm-smmu-v3.c +85 -3
134 lines not on this page · raw message
--
1.8.0
PATCH 6/8 iommu/arm-smmu: add support for non-pci devices
Quoted 5 lines · 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.
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
Quoted 10 lines · Will Deacon
Quoted 5 lines · 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.
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?
drivers/iommu/arm-smmu-v3.c +1 -2
@@ -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,
Quoted 5 lines
Will
.
Robin Murphy
reviewerPATCH 6/8 iommu/arm-smmu: add support for non-pci devices
In reply to Zhen LeiQuoted 17 lines · 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 <[email protected]>--- arch/arm64/include/asm/device.h | 2 + drivers/iommu/arm-smmu-v3.c | 88 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 87 insertions(+), 3 deletions(-)arch/arm64/include/asm/device.h +0
-0
arch/arm64/include/asm/device.h struct dev_archdata { · line 22
Quoted 3 lines
arch/arm64/include/asm/device.h +0
-0@@ -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.
arch/arm64/include/asm/device.h struct dev_archdata { · line 24
Quoted 2 lines
arch/arm64/include/asm/device.h +2
-0+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.
Quoted 32 lines
arch/arm64/include/asm/device.h +0
-0#endif bool dma_coherent; };drivers/iommu/arm-smmu-v3.c +11
-0@@ -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) {
drivers/iommu/arm-smmu-v3.c static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom) · line 631
Quoted 10 lines
drivers/iommu/arm-smmu-v3.c +10
-0+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...
Quoted 44 lines
drivers/iommu/arm-smmu-v3.c +30
-3/* 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);
drivers/iommu/arm-smmu-v3.c static int arm_smmu_add_device(struct device *dev) · line 1845
Quoted 5 lines
drivers/iommu/arm-smmu-v3.c +2
-0@@ -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?
Quoted 11 lines
drivers/iommu/arm-smmu-v3.c +4
-0for (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,
drivers/iommu/arm-smmu-v3.c out_unlock: · line 1947
Quoted 10 lines
drivers/iommu/arm-smmu-v3.c +10
-0+"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
Quoted 45 lines
drivers/iommu/arm-smmu-v3.c +18
-1+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 <[email protected]>" );MODULE_LICENSE ("GPL v2" ); --1.8.0
_______________________________________________
iommu mailing list
iommu at lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
PATCH 6/8 iommu/arm-smmu: add support for non-pci devices
Quoted 24 lines · Robin Murphy
Quoted 17 lines · 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 <[email protected]>--- arch/arm64/include/asm/device.h | 2 + drivers/iommu/arm-smmu-v3.c | 88 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 87 insertions(+), 3 deletions(-)arch/arm64/include/asm/device.h +0
-0arch/arm64/include/asm/device.h struct dev_archdata { · line 22
Quoted 3 lines
arch/arm64/include/asm/device.h +0
-0@@ -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.
Quoted 7 lines
arch/arm64/include/asm/device.h struct dev_archdata { · line 24
Quoted 2 lines
arch/arm64/include/asm/device.h +2
-0+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.
Quoted 122 lines
Quoted 33 lines
arch/arm64/include/asm/device.h +0
-0#endif bool dma_coherent; };drivers/iommu/arm-smmu-v3.c +11
-0@@ -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) {drivers/iommu/arm-smmu-v3.c static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom) · line 631
Quoted 10 lines
drivers/iommu/arm-smmu-v3.c +10
-0+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...
Quoted 44 lines
drivers/iommu/arm-smmu-v3.c +30
-3/* 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);drivers/iommu/arm-smmu-v3.c static int arm_smmu_add_device(struct device *dev) · line 1845
Quoted 5 lines
drivers/iommu/arm-smmu-v3.c +2
-0@@ -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?
Quoted 11 lines
drivers/iommu/arm-smmu-v3.c +4
-0for (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,drivers/iommu/arm-smmu-v3.c out_unlock: · line 1947
Quoted 10 lines
drivers/iommu/arm-smmu-v3.c +10
-0+"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:
Quoted 7 lines
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_devicedirectly here and obviate the indirect lookup. That would depend on having correct probe ordering, but you need to enforce that anyway, since anyadd_devicecallbacks 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.
Quoted 54 lines
Robin.
[1]:http://thread.gmane.org/gmane.linux.kernel.samsung-soc/45416
Quoted 45 lines
drivers/iommu/arm-smmu-v3.c +18
-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 <[email protected]>" );MODULE_LICENSE ("GPL v2" );Signature · 7 lines
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
In reply to Zhen LeiBecause 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.
--- drivers/iommu/arm-smmu-v3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iommu/arm-smmu-v3.c +1 -1
@@ -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
Quoted 14 lines · Zhen Lei
Because we will choose the minimum value between
STRTAB_L1_SZ_SHIFTand IDR1.SIDSIZE, so enlargeSTRTAB_L1_SZ_SHIFTwill not impact the platforms whose IDR1.SIDSIZE is smaller than oldSTRTAB_L1_SZ_SHIFTvalue.Signed-off-by Zhen Lei <[email protected]>--- drivers/iommu/arm-smmu-v3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/iommu/arm-smmu-v3.c +0
-0
drivers/iommu/arm-smmu-v3.c line 212
Quoted 8 lines
drivers/iommu/arm-smmu-v3.c +1
-1@@ -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
Quoted 22 lines · Will Deacon
Quoted 21 lines · Zhen Lei
Because we will choose the minimum value between
STRTAB_L1_SZ_SHIFTand IDR1.SIDSIZE, so enlargeSTRTAB_L1_SZ_SHIFTwill not impact the platforms whose IDR1.SIDSIZE is smaller than oldSTRTAB_L1_SZ_SHIFTvalue.Signed-off-by Zhen Lei <[email protected]>--- drivers/iommu/arm-smmu-v3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/iommu/arm-smmu-v3.c +1
-1@@ -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
Quoted 4 lines
drivers/iommu/arm-smmu-v3.c line 212
Quoted 1 line
drivers/iommu/arm-smmu-v3.c +0
-0#define STRTAB_L1_DESC_DWORDS 1Can you update the comment too, please, to say that we now have 128k entries at level 1?
OK, no problem.
Quoted 5 lines
Will
.
PATCH 8/8 iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG execution
In reply to Zhen LeiSome 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.
--- drivers/iommu/arm-smmu-v3.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
drivers/iommu/arm-smmu-v3.c +14 -4
@@ -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
Quoted 28 lines · Zhen Lei
Some broken SMMUv3 devices treat
CMD_PREFETCH_CONFIGas illegal command, it's ugly to print error information.CMD_PREFETCH_CONFIGis just used to prefetch the configuration for a specified StreamID, without this command will not impact SMMUv3 function.Signed-off-by Zhen Lei <[email protected]>--- drivers/iommu/arm-smmu-v3.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)drivers/iommu/arm-smmu-v3.c +1
-2@@ -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]);
drivers/iommu/arm-smmu-v3.c static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu) · line 832
Quoted 10 lines
drivers/iommu/arm-smmu-v3.c +3
-2-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?
Quoted 6 lines
drivers/iommu/arm-smmu-v3.c +0
-0return ; }@@ -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. */
drivers/iommu/arm-smmu-v3.c static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu) · line 849
Quoted 10 lines
drivers/iommu/arm-smmu-v3.c +9
-0queue_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
Quoted 42 lines · Will Deacon
Quoted 28 lines · Zhen Lei
Some broken SMMUv3 devices treat
CMD_PREFETCH_CONFIGas illegal command, it's ugly to print error information.CMD_PREFETCH_CONFIGis just used to prefetch the configuration for a specified StreamID, without this command will not impact SMMUv3 function.Signed-off-by Zhen Lei <[email protected]>--- drivers/iommu/arm-smmu-v3.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)drivers/iommu/arm-smmu-v3.c +1
-2@@ -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]);drivers/iommu/arm-smmu-v3.c static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu) · line 832
Quoted 10 lines
drivers/iommu/arm-smmu-v3.c +3
-2-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?
Quoted 2 lines
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.
Quoted 22 lines
Quoted 7 lines
drivers/iommu/arm-smmu-v3.c +0
-0return ; }@@ -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. */ drivers/iommu/arm-smmu-v3.c static void arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu) · line 849
Quoted 10 lines
drivers/iommu/arm-smmu-v3.c +9
-0queue_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.
Quoted 2 lines
Something along the lines of the
arm_smmu_optionsinarm-smmu.cwould be a good basis for quirks.
It's a good idea. I known the arm_smmu_options, I will rewrite this patch in V2.
Quoted 5 lines
Will
.