* [PATCH 0/8] iommu/arm-smmu: bugfixs and add support for non-pci devices
@ 2015-06-26 8:32 ` Zhen Lei
0 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:32 UTC (permalink / raw)
To: Will Deacon, Joerg Roedel, linux-arm-kernel, iommu
Cc: Xinwei Hu, Zhen Lei, Zefan Li, Tianhong Ding
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 0/8] iommu/arm-smmu: bugfixs and add support for non-pci devices
@ 2015-06-26 8:32 ` Zhen Lei
0 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:32 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 1/8] iommu/arm-smmu: fix the assignment of log2size field
2015-06-26 8:32 ` Zhen Lei
@ 2015-06-26 8:32 ` Zhen Lei
-1 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:32 UTC (permalink / raw)
To: Will Deacon, Joerg Roedel, linux-arm-kernel, iommu
Cc: Xinwei Hu, Zhen Lei, Zefan Li, Tianhong Ding
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 1/8] iommu/arm-smmu: fix the assignment of log2size field
@ 2015-06-26 8:32 ` Zhen Lei
0 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:32 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 2/8] iommu/arm-smmu: fix the index calculation of strtab
2015-06-26 8:32 ` Zhen Lei
@ 2015-06-26 8:32 ` Zhen Lei
-1 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:32 UTC (permalink / raw)
To: Will Deacon, Joerg Roedel, linux-arm-kernel, iommu
Cc: Xinwei Hu, Zhen Lei, Zefan Li, Tianhong Ding
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 2/8] iommu/arm-smmu: fix the index calculation of strtab
@ 2015-06-26 8:32 ` Zhen Lei
0 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:32 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT
2015-06-26 8:32 ` Zhen Lei
@ 2015-06-26 8:32 ` Zhen Lei
-1 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:32 UTC (permalink / raw)
To: Will Deacon, Joerg Roedel, linux-arm-kernel, iommu
Cc: Xinwei Hu, Zhen Lei, Zefan Li, Tianhong Ding
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT
@ 2015-06-26 8:32 ` Zhen Lei
0 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:32 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 4/8] iommu/arm-smmu: set EPD1 to disable TT1 translation table walk
2015-06-26 8:32 ` Zhen Lei
@ 2015-06-26 8:33 ` Zhen Lei
-1 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:33 UTC (permalink / raw)
To: Will Deacon, Joerg Roedel, linux-arm-kernel, iommu
Cc: Xinwei Hu, Zhen Lei, Zefan Li, Tianhong Ding
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 4/8] iommu/arm-smmu: set EPD1 to disable TT1 translation table walk
@ 2015-06-26 8:33 ` Zhen Lei
0 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:33 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 5/8] iommu/arm-smmu: rename __arm_smmu_get_pci_sid
2015-06-26 8:32 ` Zhen Lei
@ 2015-06-26 8:33 ` Zhen Lei
-1 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:33 UTC (permalink / raw)
To: Will Deacon, Joerg Roedel, linux-arm-kernel, iommu
Cc: Xinwei Hu, Zhen Lei, Zefan Li, Tianhong Ding
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 5/8] iommu/arm-smmu: rename __arm_smmu_get_pci_sid
@ 2015-06-26 8:33 ` Zhen Lei
0 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:33 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 6/8] iommu/arm-smmu: add support for non-pci devices
2015-06-26 8:32 ` Zhen Lei
@ 2015-06-26 8:33 ` Zhen Lei
-1 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:33 UTC (permalink / raw)
To: Will Deacon, Joerg Roedel, linux-arm-kernel, iommu
Cc: Xinwei Hu, Zhen Lei, Zefan Li, Tianhong Ding
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 6/8] iommu/arm-smmu: add support for non-pci devices
@ 2015-06-26 8:33 ` Zhen Lei
0 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:33 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 7/8] iommu/arm-smmu: enlarge STRTAB_L1_SZ_SHIFT to support larger sidsize
2015-06-26 8:32 ` Zhen Lei
@ 2015-06-26 8:33 ` Zhen Lei
-1 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:33 UTC (permalink / raw)
To: Will Deacon, Joerg Roedel, linux-arm-kernel, iommu
Cc: Xinwei Hu, Zhen Lei, Zefan Li, Tianhong Ding
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 7/8] iommu/arm-smmu: enlarge STRTAB_L1_SZ_SHIFT to support larger sidsize
@ 2015-06-26 8:33 ` Zhen Lei
0 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:33 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 8/8] iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG execution
2015-06-26 8:32 ` Zhen Lei
@ 2015-06-26 8:33 ` Zhen Lei
-1 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:33 UTC (permalink / raw)
To: Will Deacon, Joerg Roedel, linux-arm-kernel, iommu
Cc: Xinwei Hu, Zhen Lei, Zefan Li, Tianhong Ding
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 8/8] iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG execution
@ 2015-06-26 8:33 ` Zhen Lei
0 siblings, 0 replies; 50+ messages in thread
From: Zhen Lei @ 2015-06-26 8:33 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH 1/8] iommu/arm-smmu: fix the assignment of log2size field
2015-06-26 8:32 ` Zhen Lei
@ 2015-06-29 17:05 ` Will Deacon
-1 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-29 17:05 UTC (permalink / raw)
To: Zhen Lei
Cc: huxinwei-hv44wF8Li93QT0dZR+AlfA, iommu, Zefan Li, Tianhong Ding,
linux-arm-kernel
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;
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 1/8] iommu/arm-smmu: fix the assignment of log2size field
@ 2015-06-29 17:05 ` Will Deacon
0 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-29 17:05 UTC (permalink / raw)
To: linux-arm-kernel
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;
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH 2/8] iommu/arm-smmu: fix the index calculation of strtab
2015-06-26 8:32 ` Zhen Lei
@ 2015-06-29 17:17 ` Will Deacon
-1 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-29 17:17 UTC (permalink / raw)
To: Zhen Lei
Cc: huxinwei-hv44wF8Li93QT0dZR+AlfA, iommu, Zefan Li, Tianhong Ding,
linux-arm-kernel
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
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 2/8] iommu/arm-smmu: fix the index calculation of strtab
@ 2015-06-29 17:17 ` Will Deacon
0 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-29 17:17 UTC (permalink / raw)
To: linux-arm-kernel
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
>
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT
2015-06-26 8:32 ` Zhen Lei
@ 2015-06-29 17:25 ` Will Deacon
-1 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-29 17:25 UTC (permalink / raw)
To: Zhen Lei
Cc: huxinwei-hv44wF8Li93QT0dZR+AlfA, iommu, Zefan Li, Tianhong Ding,
linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT
@ 2015-06-29 17:25 ` Will Deacon
0 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-29 17:25 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 4/8] iommu/arm-smmu: set EPD1 to disable TT1 translation table walk
2015-06-26 8:33 ` Zhen Lei
@ 2015-06-29 17:26 ` Will Deacon
-1 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-29 17:26 UTC (permalink / raw)
To: Zhen Lei
Cc: huxinwei-hv44wF8Li93QT0dZR+AlfA, iommu, Zefan Li, Tianhong Ding,
linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 4/8] iommu/arm-smmu: set EPD1 to disable TT1 translation table walk
@ 2015-06-29 17:26 ` Will Deacon
0 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-29 17:26 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 6/8] iommu/arm-smmu: add support for non-pci devices
2015-06-26 8:33 ` Zhen Lei
@ 2015-06-29 17:28 ` Will Deacon
-1 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-29 17:28 UTC (permalink / raw)
To: Zhen Lei
Cc: laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw,
huxinwei-hv44wF8Li93QT0dZR+AlfA, iommu, Zefan Li, Tianhong Ding,
linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 6/8] iommu/arm-smmu: add support for non-pci devices
@ 2015-06-29 17:28 ` Will Deacon
0 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-29 17:28 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 7/8] iommu/arm-smmu: enlarge STRTAB_L1_SZ_SHIFT to support larger sidsize
2015-06-26 8:33 ` Zhen Lei
@ 2015-06-29 17:35 ` Will Deacon
-1 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-29 17:35 UTC (permalink / raw)
To: Zhen Lei
Cc: huxinwei-hv44wF8Li93QT0dZR+AlfA, iommu, Zefan Li, Tianhong Ding,
linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 7/8] iommu/arm-smmu: enlarge STRTAB_L1_SZ_SHIFT to support larger sidsize
@ 2015-06-29 17:35 ` Will Deacon
0 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-29 17:35 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 8/8] iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG execution
2015-06-26 8:33 ` Zhen Lei
@ 2015-06-29 17:49 ` Will Deacon
-1 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-29 17:49 UTC (permalink / raw)
To: Zhen Lei
Cc: huxinwei-hv44wF8Li93QT0dZR+AlfA, iommu, Zefan Li, Tianhong Ding,
linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 8/8] iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG execution
@ 2015-06-29 17:49 ` Will Deacon
0 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-29 17:49 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 1/8] iommu/arm-smmu: fix the assignment of log2size field
2015-06-29 17:05 ` Will Deacon
@ 2015-06-30 3:47 ` leizhen
-1 siblings, 0 replies; 50+ messages in thread
From: leizhen @ 2015-06-30 3:47 UTC (permalink / raw)
To: Will Deacon
Cc: huxinwei-hv44wF8Li93QT0dZR+AlfA, iommu, Zefan Li, Tianhong Ding,
linux-arm-kernel
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;
>
> .
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 1/8] iommu/arm-smmu: fix the assignment of log2size field
@ 2015-06-30 3:47 ` leizhen
0 siblings, 0 replies; 50+ messages in thread
From: leizhen @ 2015-06-30 3:47 UTC (permalink / raw)
To: linux-arm-kernel
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;
>
> .
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT
2015-06-29 17:25 ` Will Deacon
@ 2015-06-30 3:57 ` leizhen
-1 siblings, 0 replies; 50+ messages in thread
From: leizhen @ 2015-06-30 3:57 UTC (permalink / raw)
To: Will Deacon
Cc: huxinwei-hv44wF8Li93QT0dZR+AlfA, iommu, Zefan Li, Tianhong Ding,
linux-arm-kernel
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
>
> .
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT
@ 2015-06-30 3:57 ` leizhen
0 siblings, 0 replies; 50+ messages in thread
From: leizhen @ 2015-06-30 3:57 UTC (permalink / raw)
To: linux-arm-kernel
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
>
> .
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 4/8] iommu/arm-smmu: set EPD1 to disable TT1 translation table walk
2015-06-29 17:26 ` Will Deacon
@ 2015-06-30 4:40 ` leizhen
-1 siblings, 0 replies; 50+ messages in thread
From: leizhen @ 2015-06-30 4:40 UTC (permalink / raw)
To: Will Deacon
Cc: huxinwei-hv44wF8Li93QT0dZR+AlfA, iommu, Zefan Li, Tianhong Ding,
linux-arm-kernel
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
>
> .
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 4/8] iommu/arm-smmu: set EPD1 to disable TT1 translation table walk
@ 2015-06-30 4:40 ` leizhen
0 siblings, 0 replies; 50+ messages in thread
From: leizhen @ 2015-06-30 4:40 UTC (permalink / raw)
To: linux-arm-kernel
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
>
> .
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 6/8] iommu/arm-smmu: add support for non-pci devices
2015-06-29 17:28 ` Will Deacon
@ 2015-06-30 8:51 ` leizhen
-1 siblings, 0 replies; 50+ messages in thread
From: leizhen @ 2015-06-30 8:51 UTC (permalink / raw)
To: Will Deacon
Cc: laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw,
huxinwei-hv44wF8Li93QT0dZR+AlfA, iommu, Zefan Li, Tianhong Ding,
linux-arm-kernel
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
>
> .
>
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH 6/8] iommu/arm-smmu: add support for non-pci devices
@ 2015-06-30 8:51 ` leizhen
0 siblings, 0 replies; 50+ messages in thread
From: leizhen @ 2015-06-30 8:51 UTC (permalink / raw)
To: linux-arm-kernel
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
>
> .
>
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH 7/8] iommu/arm-smmu: enlarge STRTAB_L1_SZ_SHIFT to support larger sidsize
2015-06-29 17:35 ` Will Deacon
@ 2015-06-30 8:57 ` leizhen
-1 siblings, 0 replies; 50+ messages in thread
From: leizhen @ 2015-06-30 8:57 UTC (permalink / raw)
To: Will Deacon
Cc: huxinwei-hv44wF8Li93QT0dZR+AlfA, iommu, Zefan Li, Tianhong Ding,
linux-arm-kernel
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
>
> .
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 7/8] iommu/arm-smmu: enlarge STRTAB_L1_SZ_SHIFT to support larger sidsize
@ 2015-06-30 8:57 ` leizhen
0 siblings, 0 replies; 50+ messages in thread
From: leizhen @ 2015-06-30 8:57 UTC (permalink / raw)
To: linux-arm-kernel
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
>
> .
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 8/8] iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG execution
2015-06-29 17:49 ` Will Deacon
@ 2015-06-30 9:18 ` leizhen
-1 siblings, 0 replies; 50+ messages in thread
From: leizhen @ 2015-06-30 9:18 UTC (permalink / raw)
To: Will Deacon
Cc: huxinwei-hv44wF8Li93QT0dZR+AlfA, iommu, Zefan Li, Tianhong Ding,
linux-arm-kernel
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
>
> .
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 8/8] iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG execution
@ 2015-06-30 9:18 ` leizhen
0 siblings, 0 replies; 50+ messages in thread
From: leizhen @ 2015-06-30 9:18 UTC (permalink / raw)
To: linux-arm-kernel
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
>
> .
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 6/8] iommu/arm-smmu: add support for non-pci devices
2015-06-26 8:33 ` Zhen Lei
@ 2015-06-30 11:26 ` Robin Murphy
-1 siblings, 0 replies; 50+ messages in thread
From: Robin Murphy @ 2015-06-30 11:26 UTC (permalink / raw)
To: Zhen Lei, Will Deacon, Joerg Roedel, linux-arm-kernel, iommu
Cc: huxinwei-hv44wF8Li93QT0dZR+AlfA, Zefan Li, Tianhong Ding
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
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 6/8] iommu/arm-smmu: add support for non-pci devices
@ 2015-06-30 11:26 ` Robin Murphy
0 siblings, 0 replies; 50+ messages in thread
From: Robin Murphy @ 2015-06-30 11:26 UTC (permalink / raw)
To: linux-arm-kernel
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
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT
2015-06-30 3:57 ` leizhen
@ 2015-06-30 14:11 ` Will Deacon
-1 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-30 14:11 UTC (permalink / raw)
To: leizhen
Cc: huxinwei-hv44wF8Li93QT0dZR+AlfA, iommu, Zefan Li, Tianhong Ding,
linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT
@ 2015-06-30 14:11 ` Will Deacon
0 siblings, 0 replies; 50+ messages in thread
From: Will Deacon @ 2015-06-30 14:11 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 6/8] iommu/arm-smmu: add support for non-pci devices
2015-06-30 11:26 ` Robin Murphy
@ 2015-07-01 2:16 ` leizhen
-1 siblings, 0 replies; 50+ messages in thread
From: leizhen @ 2015-07-01 2:16 UTC (permalink / raw)
To: Robin Murphy
Cc: Will Deacon, huxinwei-hv44wF8Li93QT0dZR+AlfA, iommu, Zefan Li,
Tianhong Ding, linux-arm-kernel
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
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 6/8] iommu/arm-smmu: add support for non-pci devices
@ 2015-07-01 2:16 ` leizhen
0 siblings, 0 replies; 50+ messages in thread
From: leizhen @ 2015-07-01 2:16 UTC (permalink / raw)
To: linux-arm-kernel
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
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 50+ messages in thread
end of thread, other threads:[~2015-07-01 2:16 UTC | newest]
Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-26 8:32 [PATCH 0/8] iommu/arm-smmu: bugfixs and add support for non-pci devices Zhen Lei
2015-06-26 8:32 ` Zhen Lei
[not found] ` <1435307584-9812-1-git-send-email-thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2015-06-26 8:32 ` [PATCH 1/8] iommu/arm-smmu: fix the assignment of log2size field Zhen Lei
2015-06-26 8:32 ` Zhen Lei
[not found] ` <1435307584-9812-2-git-send-email-thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2015-06-29 17:05 ` Will Deacon
2015-06-29 17:05 ` Will Deacon
[not found] ` <20150629170517.GH17474-5wv7dgnIgG8@public.gmane.org>
2015-06-30 3:47 ` leizhen
2015-06-30 3:47 ` leizhen
2015-06-26 8:32 ` [PATCH 2/8] iommu/arm-smmu: fix the index calculation of strtab Zhen Lei
2015-06-26 8:32 ` Zhen Lei
[not found] ` <1435307584-9812-3-git-send-email-thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2015-06-29 17:17 ` Will Deacon
2015-06-29 17:17 ` Will Deacon
2015-06-26 8:32 ` [PATCH 3/8] iommu/arm-smmu: fix the values of ARM64_TCR_IRGN0_SHIFT and ARM64_TCR_ORGN0_SHIFT Zhen Lei
2015-06-26 8:32 ` Zhen Lei
[not found] ` <1435307584-9812-4-git-send-email-thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2015-06-29 17:25 ` Will Deacon
2015-06-29 17:25 ` Will Deacon
[not found] ` <20150629172531.GJ17474-5wv7dgnIgG8@public.gmane.org>
2015-06-30 3:57 ` leizhen
2015-06-30 3:57 ` leizhen
[not found] ` <559213AE.6060206-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2015-06-30 14:11 ` Will Deacon
2015-06-30 14:11 ` Will Deacon
2015-06-26 8:33 ` [PATCH 4/8] iommu/arm-smmu: set EPD1 to disable TT1 translation table walk Zhen Lei
2015-06-26 8:33 ` Zhen Lei
[not found] ` <1435307584-9812-5-git-send-email-thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2015-06-29 17:26 ` Will Deacon
2015-06-29 17:26 ` Will Deacon
[not found] ` <20150629172622.GK17474-5wv7dgnIgG8@public.gmane.org>
2015-06-30 4:40 ` leizhen
2015-06-30 4:40 ` leizhen
2015-06-26 8:33 ` [PATCH 5/8] iommu/arm-smmu: rename __arm_smmu_get_pci_sid Zhen Lei
2015-06-26 8:33 ` Zhen Lei
2015-06-26 8:33 ` [PATCH 6/8] iommu/arm-smmu: add support for non-pci devices Zhen Lei
2015-06-26 8:33 ` Zhen Lei
[not found] ` <1435307584-9812-7-git-send-email-thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2015-06-29 17:28 ` Will Deacon
2015-06-29 17:28 ` Will Deacon
[not found] ` <20150629172831.GL17474-5wv7dgnIgG8@public.gmane.org>
2015-06-30 8:51 ` leizhen
2015-06-30 8:51 ` leizhen
2015-06-30 11:26 ` Robin Murphy
2015-06-30 11:26 ` Robin Murphy
[not found] ` <55927CEC.4090900-5wv7dgnIgG8@public.gmane.org>
2015-07-01 2:16 ` leizhen
2015-07-01 2:16 ` leizhen
2015-06-26 8:33 ` [PATCH 7/8] iommu/arm-smmu: enlarge STRTAB_L1_SZ_SHIFT to support larger sidsize Zhen Lei
2015-06-26 8:33 ` Zhen Lei
[not found] ` <1435307584-9812-8-git-send-email-thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2015-06-29 17:35 ` Will Deacon
2015-06-29 17:35 ` Will Deacon
[not found] ` <20150629173539.GM17474-5wv7dgnIgG8@public.gmane.org>
2015-06-30 8:57 ` leizhen
2015-06-30 8:57 ` leizhen
2015-06-26 8:33 ` [PATCH 8/8] iommu/arm-smmu: suppress fault information about CMD_PREFETCH_CONFIG execution Zhen Lei
2015-06-26 8:33 ` Zhen Lei
[not found] ` <1435307584-9812-9-git-send-email-thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2015-06-29 17:49 ` Will Deacon
2015-06-29 17:49 ` Will Deacon
[not found] ` <20150629174909.GN17474-5wv7dgnIgG8@public.gmane.org>
2015-06-30 9:18 ` leizhen
2015-06-30 9:18 ` leizhen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.