irqchip, gicv3-its:Workaround for HiSilicon erratum 161010801

RFC In review Shameerali Kolothum Thodi 4 participants 14 messages Jan 24, 2017 – Jan 25, 2017 1 message not in the archive

Subsystems (3)

arm64 irqchip drivers Arm generic interrupt controller drivers

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Shameerali Kolothum Thodi

author

2017-01-24 13:47 UTC #1 lore raw

RFC 2/4 irqchip, gicv3-its:Workaround for HiSilicon erratum 161010801

The HiSilicon erratum 161010801 describes the limitation of certain HiSilicon platforms to support the SMMU mappings for MSI transactions.

On these platforms GICv3 ITS translator is presented with the deviceID by extending the MSI payload data to 64 bits to include the deviceID. Hence, the PCIe controller on this platforms has to differentiate the MSI payload against other DMA payload and has to modify the MSI payload. This basically makes it difficult for this platforms to have a SMMU translation for MSI. Also these platforms doesn't have a proper IIDR register to use the existing IIDR based quirk mechanism.

This workaround based on the devicetree binding property, supports bypassing the SMMU for the MSI transactions on this platforms.

Signed-off-by shameer <[email protected]>

---
 arch/arm64/Kconfig               | 15 ++++++++++++
 drivers/irqchip/irq-gic-common.h |  1 +
 drivers/irqchip/irq-gic-v3-its.c | 52 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 67 insertions(+), 1 deletion(-)

arch/arm64/Kconfig +15 -0

index 0ae0427..8d600b0 100644
@@ -485,6 +485,21 @@ config CAVIUM_ERRATUM_27456
 
 	  If unsure, say Y.
 
+config HISILICON_ERRATUM_161010801
+	bool "HiSilicon erratum 161010801"
+	default y
+	help
+	  Enable workaround for erratum 161010801.
+
+	  This implements a gicv3-its errata workaround for HiSilicon
+	  platforms Hip05/Hip07. These platforms cannot support the MSI
+	  interrupt remapping and MSI transaction has to be bypassed by SMMU.
+
+	  The fix is to avoid calling the remapping hook into the SMMU
+	  driver from the its_irq_compose_msi_msg().
+
+	  If unsure, say Y.
+
 endmenu

drivers/irqchip/irq-gic-common.h +1 -0

index 205e5fd..de0385a 100644
@@ -26,6 +26,7 @@ struct gic_quirk {
 	void (*init)(void *data);
 	u32 iidr;
 	u32 mask;
+	const char *erratum;
 };
 
 int gic_configure_irq(unsigned int irq, unsigned int type,

drivers/irqchip/irq-gic-v3-its.c +51 -1

91 lines not on this page · raw message

Signature · 4 lines

1.9.1


.

Marc Zyngier

2017-01-24 14:11 UTC #2 lore raw

+ Robin,

Quoted 72 lines · Shameerali Kolothum Thodi

The HiSilicon erratum 161010801 describes the limitation of certain HiSilicon platforms to support the SMMU mappings for MSI transactions.
On these platforms GICv3 ITS translator is presented with the deviceID by extending the MSI payload data to 64 bits to include the deviceID. Hence, the PCIe controller on this platforms has to differentiate the MSI payload against other DMA payload and has to modify the MSI payload. This basically makes it difficult for this platforms to have a SMMU translation for MSI. Also these platforms doesn't have a proper IIDR register to use the existing IIDR based quirk mechanism.
This workaround based on the devicetree binding property, supports bypassing the SMMU for the MSI transactions on this platforms.
Signed-off-by shameer <[email protected]>
---
 arch/arm64/Kconfig               | 15 ++++++++++++
 drivers/irqchip/irq-gic-common.h |  1 +
 drivers/irqchip/irq-gic-v3-its.c | 52 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 67 insertions(+), 1 deletion(-)
arch/arm64/Kconfig +15 -0
index 0ae0427..8d600b0 100644
@@ -485,6 +485,21 @@ config CAVIUM_ERRATUM_27456
 
 	  If unsure, say Y.
 
+config HISILICON_ERRATUM_161010801
+	bool "HiSilicon erratum 161010801"
+	default y
+	help
+	  Enable workaround for erratum 161010801.
+
+	  This implements a gicv3-its errata workaround for HiSilicon
+	  platforms Hip05/Hip07. These platforms cannot support the MSI
+	  interrupt remapping and MSI transaction has to be bypassed by SMMU.
+
+	  The fix is to avoid calling the remapping hook into the SMMU
+	  driver from the its_irq_compose_msi_msg().
+
+	  If unsure, say Y.
+
 endmenu
 
 
drivers/irqchip/irq-gic-common.h +1 -0
index 205e5fd..de0385a 100644
@@ -26,6 +26,7 @@ struct gic_quirk {
 	void (*init)(void *data);
 	u32 iidr;
 	u32 mask;
+	const char *erratum;
 };
 
 int gic_configure_irq(unsigned int irq, unsigned int type,
drivers/irqchip/irq-gic-v3-its.c +1 -0
index f471939..0a326f6 100644
@@ -44,6 +44,7 @@
 #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING		(1ULL << 0)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_22375	(1ULL << 1)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_23144	(1ULL << 2)
+#define ITS_FLAGS_WORKAROUND_HISILICON_161010801	(1ULL << 3)
 
 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING	(1 << 0)
 

drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg) · line 664

Quoted 6 lines

drivers/irqchip/irq-gic-v3-its.c +2 -1

@@ -659,7 +660,8 @@ static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg)
 	msg->address_hi		= upper_32_bits(addr);
 	msg->data		= its_get_event_id(d);
 
-	iommu_dma_map_msi_msg(d->irq, msg);
+	if (!(its->flags & ITS_FLAGS_WORKAROUND_HISILICON_161010801))
+		iommu_dma_map_msi_msg(d->irq, msg);

Let's contemplate this for a moment. If we're on the affected ITS, we're using the physical address of the GITS_TRANSLATER register. What guarantees that this is not going to conflict with an IOVA that DMA is going to use? From looking at these patches, my feeling is "not much".

So if I'm right, you're opening the door to some interesting memory corruption if the two regions ever intersect.

Robin, what do you think?

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Robin Murphy

2017-01-24 14:41 UTC #3 lore raw

Quoted 87 lines · Marc Zyngier

+ Robin,
Quoted 72 lines · Shameerali Kolothum Thodi
The HiSilicon erratum 161010801 describes the limitation of certain HiSilicon platforms to support the SMMU mappings for MSI transactions.
On these platforms GICv3 ITS translator is presented with the deviceID by extending the MSI payload data to 64 bits to include the deviceID. Hence, the PCIe controller on this platforms has to differentiate the MSI payload against other DMA payload and has to modify the MSI payload. This basically makes it difficult for this platforms to have a SMMU translation for MSI. Also these platforms doesn't have a proper IIDR register to use the existing IIDR based quirk mechanism.
This workaround based on the devicetree binding property, supports bypassing the SMMU for the MSI transactions on this platforms.
Signed-off-by shameer <[email protected]>
---
 arch/arm64/Kconfig               | 15 ++++++++++++
 drivers/irqchip/irq-gic-common.h |  1 +
 drivers/irqchip/irq-gic-v3-its.c | 52 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 67 insertions(+), 1 deletion(-)
arch/arm64/Kconfig +15 -0
index 0ae0427..8d600b0 100644
@@ -485,6 +485,21 @@ config CAVIUM_ERRATUM_27456
 
 	  If unsure, say Y.
 
+config HISILICON_ERRATUM_161010801
+	bool "HiSilicon erratum 161010801"
+	default y
+	help
+	  Enable workaround for erratum 161010801.
+
+	  This implements a gicv3-its errata workaround for HiSilicon
+	  platforms Hip05/Hip07. These platforms cannot support the MSI
+	  interrupt remapping and MSI transaction has to be bypassed by SMMU.
+
+	  The fix is to avoid calling the remapping hook into the SMMU
+	  driver from the its_irq_compose_msi_msg().
+
+	  If unsure, say Y.
+
 endmenu
 
 
drivers/irqchip/irq-gic-common.h +1 -0
index 205e5fd..de0385a 100644
@@ -26,6 +26,7 @@ struct gic_quirk {
 	void (*init)(void *data);
 	u32 iidr;
 	u32 mask;
+	const char *erratum;
 };
 
 int gic_configure_irq(unsigned int irq, unsigned int type,
drivers/irqchip/irq-gic-v3-its.c +1 -0
index f471939..0a326f6 100644
@@ -44,6 +44,7 @@
 #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING		(1ULL << 0)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_22375	(1ULL << 1)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_23144	(1ULL << 2)
+#define ITS_FLAGS_WORKAROUND_HISILICON_161010801	(1ULL << 3)
 
 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING	(1 << 0)
 
drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg) · line 664
Quoted 6 lines
drivers/irqchip/irq-gic-v3-its.c +2 -1
@@ -659,7 +660,8 @@ static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg)
 	msg->address_hi		= upper_32_bits(addr);
 	msg->data		= its_get_event_id(d);
 
-	iommu_dma_map_msi_msg(d->irq, msg);
+	if (!(its->flags & ITS_FLAGS_WORKAROUND_HISILICON_161010801))
+		iommu_dma_map_msi_msg(d->irq, msg);
Let's contemplate this for a moment. If we're on the affected ITS, we're using the physical address of the GITS_TRANSLATER register. What guarantees that this is not going to conflict with an IOVA that DMA is going to use? From looking at these patches, my feeling is "not much".

Quoted 4 lines

So if I'm right, you're opening the door to some interesting memory corruption if the two regions ever intersect.
Robin, what do you think?

Yup. Unless the ITS physical address is actually reserved from the IOVA domain, it's still free to be allocated for DMA mappings, and if that ever happens then you'll get odd bits of data landing in the ITS instead of RAM, and maybe even locked-up devices or worse if the doorbell gives back decode errors on read attempts. It's essentially the exact same problem as we have with memory-mapped PCI windows, and needs to be solved in the same fashion, i.e. between the SMMU and the IOMMU-DMA code.

Robin.

Quoted 3 lines

M.

Marc Zyngier

2017-01-24 14:52 UTC #4 lore raw

Quoted 93 lines · Robin Murphy

Quoted 87 lines · Marc Zyngier
+ Robin,
Quoted 72 lines · Shameerali Kolothum Thodi
The HiSilicon erratum 161010801 describes the limitation of certain HiSilicon platforms to support the SMMU mappings for MSI transactions.
On these platforms GICv3 ITS translator is presented with the deviceID by extending the MSI payload data to 64 bits to include the deviceID. Hence, the PCIe controller on this platforms has to differentiate the MSI payload against other DMA payload and has to modify the MSI payload. This basically makes it difficult for this platforms to have a SMMU translation for MSI. Also these platforms doesn't have a proper IIDR register to use the existing IIDR based quirk mechanism.
This workaround based on the devicetree binding property, supports bypassing the SMMU for the MSI transactions on this platforms.
Signed-off-by shameer <[email protected]>
---
 arch/arm64/Kconfig               | 15 ++++++++++++
 drivers/irqchip/irq-gic-common.h |  1 +
 drivers/irqchip/irq-gic-v3-its.c | 52 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 67 insertions(+), 1 deletion(-)
arch/arm64/Kconfig +15 -0
index 0ae0427..8d600b0 100644
@@ -485,6 +485,21 @@ config CAVIUM_ERRATUM_27456
 
 	  If unsure, say Y.
 
+config HISILICON_ERRATUM_161010801
+	bool "HiSilicon erratum 161010801"
+	default y
+	help
+	  Enable workaround for erratum 161010801.
+
+	  This implements a gicv3-its errata workaround for HiSilicon
+	  platforms Hip05/Hip07. These platforms cannot support the MSI
+	  interrupt remapping and MSI transaction has to be bypassed by SMMU.
+
+	  The fix is to avoid calling the remapping hook into the SMMU
+	  driver from the its_irq_compose_msi_msg().
+
+	  If unsure, say Y.
+
 endmenu
 
 
drivers/irqchip/irq-gic-common.h +1 -0
index 205e5fd..de0385a 100644
@@ -26,6 +26,7 @@ struct gic_quirk {
 	void (*init)(void *data);
 	u32 iidr;
 	u32 mask;
+	const char *erratum;
 };
 
 int gic_configure_irq(unsigned int irq, unsigned int type,
drivers/irqchip/irq-gic-v3-its.c +1 -0
index f471939..0a326f6 100644
@@ -44,6 +44,7 @@
 #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING		(1ULL << 0)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_22375	(1ULL << 1)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_23144	(1ULL << 2)
+#define ITS_FLAGS_WORKAROUND_HISILICON_161010801	(1ULL << 3)
 
 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING	(1 << 0)
 
drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg) · line 664
Quoted 6 lines
drivers/irqchip/irq-gic-v3-its.c +2 -1
@@ -659,7 +660,8 @@ static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg)
 	msg->address_hi		= upper_32_bits(addr);
 	msg->data		= its_get_event_id(d);
 
-	iommu_dma_map_msi_msg(d->irq, msg);
+	if (!(its->flags & ITS_FLAGS_WORKAROUND_HISILICON_161010801))
+		iommu_dma_map_msi_msg(d->irq, msg);
Let's contemplate this for a moment. If we're on the affected ITS, we're using the physical address of the GITS_TRANSLATER register. What guarantees that this is not going to conflict with an IOVA that DMA is going to use? From looking at these patches, my feeling is "not much".
Quoted 4 lines
So if I'm right, you're opening the door to some interesting memory corruption if the two regions ever intersect.
Robin, what do you think?

Quoted 7 lines

Yup. Unless the ITS physical address is actually reserved from the IOVA domain, it's still free to be allocated for DMA mappings, and if that ever happens then you'll get odd bits of data landing in the ITS instead of RAM, and maybe even locked-up devices or worse if the doorbell gives back decode errors on read attempts. It's essentially the exact same problem as we have with memory-mapped PCI windows, and needs to be solved in the same fashion, i.e. between the SMMU and the IOMMU-DMA code.

We're in violent agreement, and I'll leave the matter into your capable hands! ;-)

Thanks,

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Shameerali Kolothum Thodi

author

2017-01-24 15:39 UTC #5 lore raw

In reply to Robin Murphy

+Eric

Quoted 117 lines

-----Original Message-----
From: Robin Murphy [mailto:robin.murphy at arm.com]
Sent: Tuesday, January 24, 2017 2:42 PM
To: Marc Zyngier; Shameerali Kolothum Thodi; mark.rutland at arm.com; will.deacon at arm.com
Cc: linux-arm-kernel at lists.infradead.org; Linuxarm; linux- kernel at vger.kernel.org; devicetree at vger.kernel.org; John Garry; Guohanjun (Hanjun Guo)
Subject: Re: [RFC 2/4] irqchip, gicv3-its:Workaround for HiSilicon erratum 161010801
Quoted 5 lines · Marc Zyngier
+ Robin,
Quoted 2 lines · Shameerali Kolothum Thodi
The HiSilicon erratum 161010801 describes the limitation of certain HiSilicon platforms to support the SMMU mappings for MSI
transactions.
Quoted 2 lines
Quoted 2 lines
On these platforms GICv3 ITS translator is presented with the
deviceID
Quoted 1 line
Quoted 1 line
by extending the MSI payload data to 64 bits to include the
deviceID.
Quoted 1 line
Quoted 1 line
Hence, the PCIe controller on this platforms has to differentiate
the
Quoted 1 line
Quoted 1 line
MSI payload against other DMA payload and has to modify the MSI
payload.
Quoted 4 lines
Quoted 4 lines
This basically makes it difficult for this platforms to have a SMMU translation for MSI. Also these platforms doesn't have a proper IIDR register to use the existing IIDR based quirk mechanism.
Quoted 8 lines
Quoted 8 lines
This workaround based on the devicetree binding property, supports bypassing the SMMU for the MSI transactions on this platforms.
Signed-off-by shameer <[email protected]>
---
 arch/arm64/Kconfig               | 15 ++++++++++++
 drivers/irqchip/irq-gic-common.h |  1 +
 drivers/irqchip/irq-gic-v3-its.c | 52
+++++++++++++++++++++++++++++++++++++++-
Quoted 10 lines
Quoted 10 lines
 3 files changed, 67 insertions(+), 1 deletion(-)
arch/arm64/Kconfig +0 -0
index 0ae0427..8d600b0 100644
@@ -485,6 +485,21 @@ config CAVIUM_ERRATUM_27456
 
 	  If unsure, say Y.
 
Quoted 9 lines
arch/arm64/Kconfig config CAVIUM_ERRATUM_27456
Quoted 9 lines
arch/arm64/Kconfig +9 -0
+config HISILICON_ERRATUM_161010801
+	bool "HiSilicon erratum 161010801"
+	default y
+	help
+	  Enable workaround for erratum 161010801.
+
+	  This implements a gicv3-its errata workaround for HiSilicon
+	  platforms Hip05/Hip07. These platforms cannot support the MSI
+	  interrupt remapping and MSI transaction has to be bypassed by
SMMU.
Quoted 10 lines
drivers/irqchip/irq-
Quoted 10 lines
arch/arm64/Kconfig +6 -0
+
+	  The fix is to avoid calling the remapping hook into the SMMU
+	  driver from the its_irq_compose_msi_msg().
+
+	  If unsure, say Y.
+
 endmenu
 
 
drivers/irqchip/irq- renamed from drivers/irqchip/irq-gic-common.h +0 -0
gic-common.h
Quoted 10 lines
Quoted 10 lines
index 205e5fd..de0385a 100644
drivers/irqchip/irq-gic-common.h +1 -0
@@ -26,6 +26,7 @@ struct gic_quirk {
 	void (*init)(void *data);
 	u32 iidr;
 	u32 mask;
+	const char *erratum;
 };
 
Quoted 2 lines
drivers/irqchip/irq-
Quoted 2 lines
drivers/irqchip/irq-gic-common.h +0 -0
 int gic_configure_irq(unsigned int irq, unsigned int type,
drivers/irqchip/irq- renamed from drivers/irqchip/irq-gic-v3-its.c +0 -0
gic-v3-its.c
Quoted 9 lines
Quoted 9 lines
index f471939..0a326f6 100644
drivers/irqchip/irq-gic-v3-its.c +1 -0
@@ -44,6 +44,7 @@
 #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING		(1ULL << 0)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_22375	(1ULL << 1)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_23144	(1ULL << 2)
+#define ITS_FLAGS_WORKAROUND_HISILICON_161010801	(1ULL << 3)
 
Quoted 3 lines
drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct
Quoted 3 lines
drivers/irqchip/irq-gic-v3-its.c +0 -0
 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING	(1 << 0)
 
@@ -659,7 +660,8 @@ static void its_irq_compose_msi_msg(struct
irq_data *d, struct msi_msg *msg)
Quoted 8 lines
drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg) · line 664
Quoted 6 lines
drivers/irqchip/irq-gic-v3-its.c +2 -1
 	msg->address_hi		= upper_32_bits(addr);
 	msg->data		= its_get_event_id(d);
 
-	iommu_dma_map_msi_msg(d->irq, msg);
+	if (!(its->flags & ITS_FLAGS_WORKAROUND_HISILICON_161010801))
+		iommu_dma_map_msi_msg(d->irq, msg);
Let's contemplate this for a moment. If we're on the affected ITS,
we're
Quoted 2 lines
using the physical address of the GITS_TRANSLATER register. What guarantees that this is not going to conflict with an IOVA that DMA
is
Quoted 1 line
going to use? From looking at these patches, my feeling is "not
much".
Quoted 5 lines
So if I'm right, you're opening the door to some interesting memory corruption if the two regions ever intersect.
Robin, what do you think?

Quoted 9 lines

Yup. Unless the ITS physical address is actually reserved from the IOVA domain, it's still free to be allocated for DMA mappings, and if that ever happens then you'll get odd bits of data landing in the ITS instead
of RAM, and maybe even locked-up devices or worse if the doorbell gives back decode errors on read attempts. It's essentially the exact same problem as we have with memory-mapped PCI windows, and needs to be solved in the same fashion, i.e. between the SMMU and the IOMMU-DMA code.

Is this something that can incorporated in Eric's latest patch series[1]? It does mentions reserved regions can be:
- directly mapped regions
- regions that cannot be iommu mapped (PCI host bridge windows, ...)
- MSI regions (because they belong to another address space or because they are not translated by the IOMMU and need special handling)

Though I am not clear our case comes under "the MSI regions that are not translated by the IOMMU and need special handling" or not.

Thanks,
Shameer

[1] https://github.com/eauger/linux/commit/5b35ea30de0b34fe4d02f7da8fab0995514781ff

Marc Zyngier

2017-01-24 15:49 UTC #6 lore raw

Quoted 136 lines · Shameerali Kolothum Thodi

+Eric
Quoted 117 lines
-----Original Message-----
From: Robin Murphy [mailto:robin.murphy at arm.com]
Sent: Tuesday, January 24, 2017 2:42 PM
To: Marc Zyngier; Shameerali Kolothum Thodi; mark.rutland at arm.com; will.deacon at arm.com
Cc: linux-arm-kernel at lists.infradead.org; Linuxarm; linux- kernel at vger.kernel.org; devicetree at vger.kernel.org; John Garry; Guohanjun (Hanjun Guo)
Subject: Re: [RFC 2/4] irqchip, gicv3-its:Workaround for HiSilicon erratum 161010801
Quoted 5 lines · Marc Zyngier
+ Robin,
Quoted 2 lines · Shameerali Kolothum Thodi
The HiSilicon erratum 161010801 describes the limitation of certain HiSilicon platforms to support the SMMU mappings for MSI
transactions.
Quoted 2 lines
Quoted 2 lines
On these platforms GICv3 ITS translator is presented with the
deviceID
Quoted 1 line
Quoted 1 line
by extending the MSI payload data to 64 bits to include the
deviceID.
Quoted 1 line
Quoted 1 line
Hence, the PCIe controller on this platforms has to differentiate
the
Quoted 1 line
Quoted 1 line
MSI payload against other DMA payload and has to modify the MSI
payload.
Quoted 4 lines
Quoted 4 lines
This basically makes it difficult for this platforms to have a SMMU translation for MSI. Also these platforms doesn't have a proper IIDR register to use the existing IIDR based quirk mechanism.
Quoted 8 lines
Quoted 8 lines
This workaround based on the devicetree binding property, supports bypassing the SMMU for the MSI transactions on this platforms.
Signed-off-by shameer <[email protected]>
---
 arch/arm64/Kconfig               | 15 ++++++++++++
 drivers/irqchip/irq-gic-common.h |  1 +
 drivers/irqchip/irq-gic-v3-its.c | 52
+++++++++++++++++++++++++++++++++++++++-
Quoted 10 lines
Quoted 10 lines
 3 files changed, 67 insertions(+), 1 deletion(-)
arch/arm64/Kconfig +0 -0
index 0ae0427..8d600b0 100644
@@ -485,6 +485,21 @@ config CAVIUM_ERRATUM_27456
 
 	  If unsure, say Y.
 
Quoted 9 lines
arch/arm64/Kconfig config CAVIUM_ERRATUM_27456
Quoted 9 lines
arch/arm64/Kconfig +9 -0
+config HISILICON_ERRATUM_161010801
+	bool "HiSilicon erratum 161010801"
+	default y
+	help
+	  Enable workaround for erratum 161010801.
+
+	  This implements a gicv3-its errata workaround for HiSilicon
+	  platforms Hip05/Hip07. These platforms cannot support the MSI
+	  interrupt remapping and MSI transaction has to be bypassed by
SMMU.
Quoted 10 lines
drivers/irqchip/irq-
Quoted 10 lines
arch/arm64/Kconfig +6 -0
+
+	  The fix is to avoid calling the remapping hook into the SMMU
+	  driver from the its_irq_compose_msi_msg().
+
+	  If unsure, say Y.
+
 endmenu
 
 
drivers/irqchip/irq- renamed from drivers/irqchip/irq-gic-common.h +0 -0
gic-common.h
Quoted 10 lines
Quoted 10 lines
index 205e5fd..de0385a 100644
drivers/irqchip/irq-gic-common.h +1 -0
@@ -26,6 +26,7 @@ struct gic_quirk {
 	void (*init)(void *data);
 	u32 iidr;
 	u32 mask;
+	const char *erratum;
 };
 
Quoted 2 lines
drivers/irqchip/irq-
Quoted 2 lines
drivers/irqchip/irq-gic-common.h +0 -0
 int gic_configure_irq(unsigned int irq, unsigned int type,
drivers/irqchip/irq- renamed from drivers/irqchip/irq-gic-v3-its.c +0 -0
gic-v3-its.c
Quoted 9 lines
Quoted 9 lines
index f471939..0a326f6 100644
drivers/irqchip/irq-gic-v3-its.c +1 -0
@@ -44,6 +44,7 @@
 #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING		(1ULL << 0)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_22375	(1ULL << 1)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_23144	(1ULL << 2)
+#define ITS_FLAGS_WORKAROUND_HISILICON_161010801	(1ULL << 3)
 
Quoted 3 lines
drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct
Quoted 3 lines
drivers/irqchip/irq-gic-v3-its.c +0 -0
 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING	(1 << 0)
 
@@ -659,7 +660,8 @@ static void its_irq_compose_msi_msg(struct
irq_data *d, struct msi_msg *msg)
Quoted 8 lines
drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg) · line 664
Quoted 6 lines
drivers/irqchip/irq-gic-v3-its.c +2 -1
 	msg->address_hi		= upper_32_bits(addr);
 	msg->data		= its_get_event_id(d);
 
-	iommu_dma_map_msi_msg(d->irq, msg);
+	if (!(its->flags & ITS_FLAGS_WORKAROUND_HISILICON_161010801))
+		iommu_dma_map_msi_msg(d->irq, msg);
Let's contemplate this for a moment. If we're on the affected ITS,
we're
Quoted 2 lines
using the physical address of the GITS_TRANSLATER register. What guarantees that this is not going to conflict with an IOVA that DMA
is
Quoted 1 line
going to use? From looking at these patches, my feeling is "not
much".
Quoted 5 lines
So if I'm right, you're opening the door to some interesting memory corruption if the two regions ever intersect.
Robin, what do you think?
Quoted 9 lines
Yup. Unless the ITS physical address is actually reserved from the IOVA domain, it's still free to be allocated for DMA mappings, and if that ever happens then you'll get odd bits of data landing in the ITS instead
of RAM, and maybe even locked-up devices or worse if the doorbell gives back decode errors on read attempts. It's essentially the exact same problem as we have with memory-mapped PCI windows, and needs to be solved in the same fashion, i.e. between the SMMU and the IOMMU-DMA code.
Is this something that can incorporated in Eric's latest patch series[1]? It does mentions reserved regions can be:
- directly mapped regions
- regions that cannot be iommu mapped (PCI host bridge windows, ...)
- MSI regions (because they belong to another address space or because they are not translated by the IOMMU and need special handling)

Quoted 2 lines

Though I am not clear our case comes under "the MSI regions that are not translated by the IOMMU and need special handling" or not.

Well, given that in your case, the IOMMU never sees the MSI write, it definitely falls into the "not translated" category.

As for handling it on top of Eric's series, that's probably the most reasonable thing to do, which also means that none of this should appear in the ITS driver. Robin seems to have an idea on how to approach this.

Thanks,

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Shameerali Kolothum Thodi

author

2017-01-24 16:14 UTC #7 lore raw

Quoted 166 lines

-----Original Message-----
From: Marc Zyngier [mailto:marc.zyngier at arm.com]
Sent: Tuesday, January 24, 2017 3:50 PM
To: Shameerali Kolothum Thodi; Robin Murphy; mark.rutland at arm.com; will.deacon at arm.com; eric.auger at redhat.com
Cc: linux-arm-kernel at lists.infradead.org; Linuxarm; linux- kernel at vger.kernel.org; devicetree at vger.kernel.org; John Garry; Guohanjun (Hanjun Guo)
Subject: Re: [RFC 2/4] irqchip, gicv3-its:Workaround for HiSilicon erratum 161010801
Quoted 18 lines · Shameerali Kolothum Thodi
+Eric
Quoted 16 lines
-----Original Message-----
From: Robin Murphy [mailto:robin.murphy at arm.com]
Sent: Tuesday, January 24, 2017 2:42 PM
To: Marc Zyngier; Shameerali Kolothum Thodi; mark.rutland at arm.com; will.deacon at arm.com
Cc: linux-arm-kernel at lists.infradead.org; Linuxarm; linux- kernel at vger.kernel.org; devicetree at vger.kernel.org; John Garry; Guohanjun (Hanjun Guo)
Subject: Re: [RFC 2/4] irqchip, gicv3-its:Workaround for HiSilicon erratum 161010801
Quoted 4 lines · Marc Zyngier
+ Robin,
Quoted 1 line · Shameerali Kolothum Thodi
The HiSilicon erratum 161010801 describes the limitation of
certain
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
HiSilicon platforms to support the SMMU mappings for MSI
transactions.
Quoted 2 lines
Quoted 2 lines
On these platforms GICv3 ITS translator is presented with the
Quoted 8 lines
Quoted 8 lines
deviceID
Quoted 1 line
Quoted 1 line
by extending the MSI payload data to 64 bits to include the
deviceID.
Quoted 1 line
Quoted 1 line
Hence, the PCIe controller on this platforms has to differentiate
the
Quoted 1 line
Quoted 1 line
MSI payload against other DMA payload and has to modify the MSI
payload.
Quoted 1 line
Quoted 1 line
This basically makes it difficult for this platforms to have a
SMMU
Quoted 22 lines
Quoted 22 lines
Quoted 11 lines
Quoted 11 lines
translation for MSI. Also these platforms doesn't have a proper IIDR register to use the existing IIDR based quirk mechanism.
This workaround based on the devicetree binding property, supports bypassing the SMMU for the MSI transactions on this platforms.
Signed-off-by shameer <[email protected]>
---
 arch/arm64/Kconfig               | 15 ++++++++++++
 drivers/irqchip/irq-gic-common.h |  1 +
drivers/irqchip/irq-gic-v3-its.c | 52
+++++++++++++++++++++++++++++++++++++++-
Quoted 10 lines
Quoted 10 lines
 3 files changed, 67 insertions(+), 1 deletion(-)
arch/arm64/Kconfig index renamed from arch/arm64/Kconfig +0 -0
0ae0427..8d600b0 100644
arch/arm64/Kconfig +0 -0
@@ -485,6 +485,21 @@ config CAVIUM_ERRATUM_27456
 
 	  If unsure, say Y.
 
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
arch/arm64/Kconfig config CAVIUM_ERRATUM_27456
Quoted 7 lines
arch/arm64/Kconfig +7 -0
+config HISILICON_ERRATUM_161010801
+	bool "HiSilicon erratum 161010801"
+	default y
+	help
+	  Enable workaround for erratum 161010801.
+
+	  This implements a gicv3-its errata workaround for
HiSilicon
Quoted 1 line
Quoted 1 line
Quoted 1 line
arch/arm64/Kconfig config CAVIUM_ERRATUM_27456 · line 495
Quoted 1 line
arch/arm64/Kconfig +1 -0
+	  platforms Hip05/Hip07. These platforms cannot support the
MSI
Quoted 1 line
Quoted 1 line
Quoted 1 line
arch/arm64/Kconfig config CAVIUM_ERRATUM_27456 · line 496
Quoted 1 line
arch/arm64/Kconfig +1 -0
+	  interrupt remapping and MSI transaction has to be
bypassed by
Quoted 3 lines
Quoted 3 lines
SMMU.
Quoted 2 lines
arch/arm64/Kconfig config CAVIUM_ERRATUM_27456 · line 498
Quoted 2 lines
arch/arm64/Kconfig +2 -0
+
+	  The fix is to avoid calling the remapping hook into the
SMMU
Quoted 34 lines
Quoted 34 lines
Quoted 9 lines
drivers/irqchip/irq-
Quoted 9 lines
arch/arm64/Kconfig +4 -0
+	  driver from the its_irq_compose_msi_msg().
+
+	  If unsure, say Y.
+
 endmenu
 
 
drivers/irqchip/irq- renamed from drivers/irqchip/irq-gic-common.h +0 -0
gic-common.h
Quoted 10 lines
Quoted 10 lines
index 205e5fd..de0385a 100644
drivers/irqchip/irq-gic-common.h +1 -0
@@ -26,6 +26,7 @@ struct gic_quirk {
 	void (*init)(void *data);
 	u32 iidr;
 	u32 mask;
+	const char *erratum;
 };
 
Quoted 2 lines
drivers/irqchip/irq-gic-common.h struct gic_quirk {
Quoted 2 lines
drivers/irqchip/irq-gic-common.h +0 -1
 int gic_configure_irq(unsigned int irq, unsigned int type, diff
--git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-
gic-v3-its.c
Quoted 11 lines
Quoted 11 lines
index f471939..0a326f6 100644
drivers/irqchip/irq-gic-v3-its.c +1 -0
@@ -44,6 +44,7 @@
 #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING		(1ULL << 0)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_22375	(1ULL << 1)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_23144	(1ULL << 2)
+#define ITS_FLAGS_WORKAROUND_HISILICON_161010801	(1ULL << 3)
 
 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING	(1 << 0)
 
Quoted 7 lines
Quoted 7 lines
Quoted 1 line
drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct
Quoted 1 line
drivers/irqchip/irq-gic-v3-its.c +0 -0
@@ -659,7 +660,8 @@ static void its_irq_compose_msi_msg(struct
irq_data *d, struct msi_msg *msg)
Quoted 5 lines
drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg)
Quoted 5 lines
drivers/irqchip/irq-gic-v3-its.c +1 -1
 	msg->address_hi		= upper_32_bits(addr);
 	msg->data		= its_get_event_id(d);
 
-	iommu_dma_map_msi_msg(d->irq, msg);
+	if (!(its->flags &
ITS_FLAGS_WORKAROUND_HISILICON_161010801))
Quoted 10 lines
Quoted 10 lines
Quoted 3 lines
drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg) · line 664
Quoted 1 line
drivers/irqchip/irq-gic-v3-its.c +1 -0
+		iommu_dma_map_msi_msg(d->irq, msg);
Let's contemplate this for a moment. If we're on the affected ITS,
we're
Quoted 2 lines
using the physical address of the GITS_TRANSLATER register. What guarantees that this is not going to conflict with an IOVA that DMA
is
Quoted 1 line
going to use? From looking at these patches, my feeling is "not
much".
Quoted 1 line
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
So if I'm right, you're opening the door to some interesting memory corruption if the two regions ever intersect.
Robin, what do you think?
Yup. Unless the ITS physical address is actually reserved from the IOVA domain, it's still free to be allocated for DMA mappings, and
if
Quoted 1 line
Quoted 1 line
that ever happens then you'll get odd bits of data landing in the
ITS
Quoted 3 lines
Quoted 3 lines
instead of RAM, and maybe even locked-up devices or worse if the doorbell gives back decode errors on read attempts. It's essentially the exact same problem as we have with memory-mapped PCI windows,
and
Quoted 1 line
Quoted 1 line
needs to be solved in the same fashion, i.e. between the SMMU and
the
Quoted 3 lines
Quoted 1 line
IOMMU-DMA code.
Is this something that can incorporated in Eric's latest patch
series[1]?
Quoted 4 lines
It does mentions reserved regions can be:
- directly mapped regions
- regions that cannot be iommu mapped (PCI host bridge windows, ...)
- MSI regions (because they belong to another address space or
because
Quoted 4 lines
they are not translated by the IOMMU and need special handling)
Though I am not clear our case comes under "the MSI regions that are not translated by the IOMMU and need special handling" or not.

Quoted 7 lines

Well, given that in your case, the IOMMU never sees the MSI write, it definitely falls into the "not translated" category.
As for handling it on top of Eric's series, that's probably the most reasonable thing to do, which also means that none of this should appear in the ITS driver. Robin seems to have an idea on how to approach this.

Ok. Thanks for that Marc/Robin.

But I am not sure we can get away with ITS driver. Because current vfio patch series[1] treats GICV3 ITS as irq safe and is setting IRQ_DOMAIN_FLAG_MSI_REMAP in ITS driver. But this is not the case with our ITS.

Thanks,
Shameer

[1] https://github.com/eauger/linux/commit/df2a17bd8cc73b5a381ed1570870a2aeb6f7e068

Marc Zyngier

2017-01-24 16:29 UTC #8 lore raw

Quoted 46 lines · Shameerali Kolothum Thodi

Quoted 38 lines
Quoted 8 lines
Quoted 8 lines
Quoted 1 line
Let's contemplate this for a moment. If we're on the affected ITS,
we're
Quoted 2 lines
using the physical address of the GITS_TRANSLATER register. What guarantees that this is not going to conflict with an IOVA that DMA
is
Quoted 1 line
going to use? From looking at these patches, my feeling is "not
much".
Quoted 1 line
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
So if I'm right, you're opening the door to some interesting memory corruption if the two regions ever intersect.
Robin, what do you think?
Yup. Unless the ITS physical address is actually reserved from the IOVA domain, it's still free to be allocated for DMA mappings, and
if
Quoted 1 line
Quoted 1 line
that ever happens then you'll get odd bits of data landing in the
ITS
Quoted 3 lines
Quoted 3 lines
instead of RAM, and maybe even locked-up devices or worse if the doorbell gives back decode errors on read attempts. It's essentially the exact same problem as we have with memory-mapped PCI windows,
and
Quoted 1 line
Quoted 1 line
needs to be solved in the same fashion, i.e. between the SMMU and
the
Quoted 3 lines
Quoted 1 line
IOMMU-DMA code.
Is this something that can incorporated in Eric's latest patch
series[1]?
Quoted 4 lines
It does mentions reserved regions can be:
- directly mapped regions
- regions that cannot be iommu mapped (PCI host bridge windows, ...)
- MSI regions (because they belong to another address space or
because
Quoted 4 lines
they are not translated by the IOMMU and need special handling)
Though I am not clear our case comes under "the MSI regions that are not translated by the IOMMU and need special handling" or not.
Quoted 7 lines
Well, given that in your case, the IOMMU never sees the MSI write, it definitely falls into the "not translated" category.
As for handling it on top of Eric's series, that's probably the most reasonable thing to do, which also means that none of this should appear in the ITS driver. Robin seems to have an idea on how to approach this.

Quoted 6 lines

Ok. Thanks for that Marc/Robin.
But I am not sure we can get away with ITS driver. Because current vfio patch series[1] treats GICV3 ITS as irq safe and is setting IRQ_DOMAIN_FLAG_MSI_REMAP in ITS driver. But this is not the case with our ITS.

The ITS itself is perfectly safe, as it does perform device isolation just fine (at least as far as I can tell from this bug description).

There is two things we need to take care of:
- When the device is used on the host, the hardwired MSI region must be excluded from the DMA IOVA allocator, and the iommu_dma_map_msi_msg() call becomes a NOP.
- When the device is assigned to VFIO, the MSI region must be exposed to userspace through /sys so that it knows that the guest RAM cannot alias with this region (or face the corruption we've talked about above).

None of that actually involves the ITS. Eric's stuff has some of the initial infrastructure, but there is of course more to it. I'll let Robin chime in and correct me if I've missed something (very likely).

Thanks,

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Robin Murphy

2017-01-24 16:42 UTC #9 lore raw

Quoted 65 lines · Marc Zyngier

Quoted 46 lines · Shameerali Kolothum Thodi
Quoted 38 lines
Quoted 8 lines
Quoted 8 lines
Quoted 1 line
Let's contemplate this for a moment. If we're on the affected ITS,
we're
Quoted 2 lines
using the physical address of the GITS_TRANSLATER register. What guarantees that this is not going to conflict with an IOVA that DMA
is
Quoted 1 line
going to use? From looking at these patches, my feeling is "not
much".
Quoted 1 line
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
So if I'm right, you're opening the door to some interesting memory corruption if the two regions ever intersect.
Robin, what do you think?
Yup. Unless the ITS physical address is actually reserved from the IOVA domain, it's still free to be allocated for DMA mappings, and
if
Quoted 1 line
Quoted 1 line
that ever happens then you'll get odd bits of data landing in the
ITS
Quoted 3 lines
Quoted 3 lines
instead of RAM, and maybe even locked-up devices or worse if the doorbell gives back decode errors on read attempts. It's essentially the exact same problem as we have with memory-mapped PCI windows,
and
Quoted 1 line
Quoted 1 line
needs to be solved in the same fashion, i.e. between the SMMU and
the
Quoted 3 lines
Quoted 1 line
IOMMU-DMA code.
Is this something that can incorporated in Eric's latest patch
series[1]?
Quoted 4 lines
It does mentions reserved regions can be:
- directly mapped regions
- regions that cannot be iommu mapped (PCI host bridge windows, ...)
- MSI regions (because they belong to another address space or
because
Quoted 4 lines
they are not translated by the IOMMU and need special handling)
Though I am not clear our case comes under "the MSI regions that are not translated by the IOMMU and need special handling" or not.
Quoted 7 lines
Well, given that in your case, the IOMMU never sees the MSI write, it definitely falls into the "not translated" category.
As for handling it on top of Eric's series, that's probably the most reasonable thing to do, which also means that none of this should appear in the ITS driver. Robin seems to have an idea on how to approach this.
Quoted 6 lines
Ok. Thanks for that Marc/Robin.
But I am not sure we can get away with ITS driver. Because current vfio patch series[1] treats GICV3 ITS as irq safe and is setting IRQ_DOMAIN_FLAG_MSI_REMAP in ITS driver. But this is not the case with our ITS.
The ITS itself is perfectly safe, as it does perform device isolation just fine (at least as far as I can tell from this bug description).
There is two things we need to take care of:
- When the device is used on the host, the hardwired MSI region must be excluded from the DMA IOVA allocator, and the iommu_dma_map_msi_msg() call becomes a NOP.
- When the device is assigned to VFIO, the MSI region must be exposed to userspace through /sys so that it knows that the guest RAM cannot alias with this region (or face the corruption we've talked about above).

Quoted 3 lines

None of that actually involves the ITS. Eric's stuff has some of the initial infrastructure, but there is of course more to it. I'll let Robin chime in and correct me if I've missed something (very likely).

That's pretty much it. Once Eric's patches for the iommu_resv_regions interface have been merged, I'm planning to convert IOMMU-DMA over to using those instead of the internal PCI-specific hook it currently has. Then we would simply need the SMMU driver to expose this hardwired MSI region where necessary so that IOMMU-DMA can directly insert 1:1 iommu_dma_msi_page entries to cover it up-front in iommu_dma_init_domain().

Robin.

Quoted 5 lines

Thanks,
	M.

Shameerali Kolothum Thodi

author

2017-01-24 16:51 UTC #10 lore raw

Quoted 91 lines

-----Original Message-----
From: Robin Murphy [mailto:robin.murphy at arm.com]
Sent: Tuesday, January 24, 2017 4:43 PM
To: Marc Zyngier; Shameerali Kolothum Thodi; mark.rutland at arm.com; will.deacon at arm.com; eric.auger at redhat.com
Cc: linux-arm-kernel at lists.infradead.org; Linuxarm; linux- kernel at vger.kernel.org; devicetree at vger.kernel.org; John Garry; Guohanjun (Hanjun Guo)
Subject: Re: [RFC 2/4] irqchip, gicv3-its:Workaround for HiSilicon erratum 161010801
Quoted 9 lines · Marc Zyngier
Quoted 8 lines · Shameerali Kolothum Thodi
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 2 lines
Let's contemplate this for a moment. If we're on the affected ITS,
we're
Quoted 3 lines
using the physical address of the GITS_TRANSLATER register. What guarantees that this is not going to conflict with an IOVA that DMA
is
Quoted 1 line
going to use? From looking at these patches, my feeling is "not
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
much".
Quoted 5 lines
So if I'm right, you're opening the door to some interesting memory corruption if the two regions ever intersect.
Robin, what do you think?
Yup. Unless the ITS physical address is actually reserved from
the
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
IOVA domain, it's still free to be allocated for DMA mappings,
and
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
if
Quoted 1 line
Quoted 1 line
that ever happens then you'll get odd bits of data landing in the
ITS
Quoted 4 lines
Quoted 4 lines
instead of RAM, and maybe even locked-up devices or worse if the doorbell gives back decode errors on read attempts. It's essentially the exact same problem as we have with memory-mapped PCI windows,
and
Quoted 1 line
Quoted 1 line
needs to be solved in the same fashion, i.e. between the SMMU and
the
Quoted 3 lines
Quoted 1 line
IOMMU-DMA code.
Is this something that can incorporated in Eric's latest patch
series[1]?
Quoted 5 lines
It does mentions reserved regions can be:
- directly mapped regions
- regions that cannot be iommu mapped (PCI host bridge windows, ...)
- MSI regions (because they belong to another address space or
because
Quoted 2 lines
they are not translated by the IOMMU and need special handling)
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 2 lines
Though I am not clear our case comes under "the MSI regions that are not translated by the IOMMU and need special handling" or not.
Well, given that in your case, the IOMMU never sees the MSI write, it definitely falls into the "not translated" category.
As for handling it on top of Eric's series, that's probably the
most
Quoted 14 lines
Quoted 10 lines
Quoted 3 lines
reasonable thing to do, which also means that none of this should appear in the ITS driver. Robin seems to have an idea on how to approach this.
Ok. Thanks for that Marc/Robin.
But I am not sure we can get away with ITS driver. Because current vfio patch series[1] treats GICV3 ITS as irq safe and is setting IRQ_DOMAIN_FLAG_MSI_REMAP in ITS driver. But this is not the case with our ITS.
The ITS itself is perfectly safe, as it does perform device isolation just fine (at least as far as I can tell from this bug description).
Quoted 7 lines
There is two things we need to take care of:
- When the device is used on the host, the hardwired MSI region must be excluded from the DMA IOVA allocator, and the iommu_dma_map_msi_msg() call becomes a NOP.
- When the device is assigned to VFIO, the MSI region must be exposed to userspace through /sys so that it knows that the guest RAM cannot alias with this region (or face the corruption we've talked about
above).
Quoted 4 lines
None of that actually involves the ITS. Eric's stuff has some of the initial infrastructure, but there is of course more to it. I'll let Robin chime in and correct me if I've missed something (very likely).

Quoted 8 lines

That's pretty much it. Once Eric's patches for the iommu_resv_regions interface have been merged, I'm planning to convert IOMMU-DMA over to using those instead of the internal PCI-specific hook it currently has. Then we would simply need the SMMU driver to expose this hardwired MSI region where necessary so that IOMMU-DMA can directly insert 1:1 iommu_dma_msi_page entries to cover it up-front in iommu_dma_init_domain().

Thanks for the details. Just wondering how this 1:1 map regions will be specified. I suppose this can be a DT property to SMMU driver. Is there anything available in ACPI table for this?

Thanks,
Shameer

Robin Murphy

2017-01-24 16:30 UTC #11 lore raw

In reply to Shameerali Kolothum Thodi

Quoted 174 lines · Shameerali Kolothum Thodi

Quoted 166 lines
-----Original Message-----
From: Marc Zyngier [mailto:marc.zyngier at arm.com]
Sent: Tuesday, January 24, 2017 3:50 PM
To: Shameerali Kolothum Thodi; Robin Murphy; mark.rutland at arm.com; will.deacon at arm.com; eric.auger at redhat.com
Cc: linux-arm-kernel at lists.infradead.org; Linuxarm; linux- kernel at vger.kernel.org; devicetree at vger.kernel.org; John Garry; Guohanjun (Hanjun Guo)
Subject: Re: [RFC 2/4] irqchip, gicv3-its:Workaround for HiSilicon erratum 161010801
Quoted 18 lines · Shameerali Kolothum Thodi
+Eric
Quoted 16 lines
-----Original Message-----
From: Robin Murphy [mailto:robin.murphy at arm.com]
Sent: Tuesday, January 24, 2017 2:42 PM
To: Marc Zyngier; Shameerali Kolothum Thodi; mark.rutland at arm.com; will.deacon at arm.com
Cc: linux-arm-kernel at lists.infradead.org; Linuxarm; linux- kernel at vger.kernel.org; devicetree at vger.kernel.org; John Garry; Guohanjun (Hanjun Guo)
Subject: Re: [RFC 2/4] irqchip, gicv3-its:Workaround for HiSilicon erratum 161010801
Quoted 4 lines · Marc Zyngier
+ Robin,
Quoted 1 line · Shameerali Kolothum Thodi
The HiSilicon erratum 161010801 describes the limitation of
certain
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
HiSilicon platforms to support the SMMU mappings for MSI
transactions.
Quoted 2 lines
Quoted 2 lines
On these platforms GICv3 ITS translator is presented with the
Quoted 8 lines
Quoted 8 lines
deviceID
Quoted 1 line
Quoted 1 line
by extending the MSI payload data to 64 bits to include the
deviceID.
Quoted 1 line
Quoted 1 line
Hence, the PCIe controller on this platforms has to differentiate
the
Quoted 1 line
Quoted 1 line
MSI payload against other DMA payload and has to modify the MSI
payload.
Quoted 1 line
Quoted 1 line
This basically makes it difficult for this platforms to have a
SMMU
Quoted 22 lines
Quoted 22 lines
Quoted 11 lines
Quoted 11 lines
translation for MSI. Also these platforms doesn't have a proper IIDR register to use the existing IIDR based quirk mechanism.
This workaround based on the devicetree binding property, supports bypassing the SMMU for the MSI transactions on this platforms.
Signed-off-by shameer <[email protected]>
---
 arch/arm64/Kconfig               | 15 ++++++++++++
 drivers/irqchip/irq-gic-common.h |  1 +
drivers/irqchip/irq-gic-v3-its.c | 52
+++++++++++++++++++++++++++++++++++++++-
Quoted 10 lines
Quoted 10 lines
 3 files changed, 67 insertions(+), 1 deletion(-)
arch/arm64/Kconfig index renamed from arch/arm64/Kconfig +0 -0
0ae0427..8d600b0 100644
arch/arm64/Kconfig +0 -0
@@ -485,6 +485,21 @@ config CAVIUM_ERRATUM_27456
 
 	  If unsure, say Y.
 
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
arch/arm64/Kconfig config CAVIUM_ERRATUM_27456
Quoted 7 lines
arch/arm64/Kconfig +7 -0
+config HISILICON_ERRATUM_161010801
+	bool "HiSilicon erratum 161010801"
+	default y
+	help
+	  Enable workaround for erratum 161010801.
+
+	  This implements a gicv3-its errata workaround for
HiSilicon
Quoted 1 line
Quoted 1 line
Quoted 1 line
arch/arm64/Kconfig config CAVIUM_ERRATUM_27456 · line 495
Quoted 1 line
arch/arm64/Kconfig +1 -0
+	  platforms Hip05/Hip07. These platforms cannot support the
MSI
Quoted 1 line
Quoted 1 line
Quoted 1 line
arch/arm64/Kconfig config CAVIUM_ERRATUM_27456 · line 496
Quoted 1 line
arch/arm64/Kconfig +1 -0
+	  interrupt remapping and MSI transaction has to be
bypassed by
Quoted 3 lines
Quoted 3 lines
SMMU.
Quoted 2 lines
arch/arm64/Kconfig config CAVIUM_ERRATUM_27456 · line 498
Quoted 2 lines
arch/arm64/Kconfig +2 -0
+
+	  The fix is to avoid calling the remapping hook into the
SMMU
Quoted 34 lines
Quoted 34 lines
Quoted 9 lines
drivers/irqchip/irq-
Quoted 9 lines
arch/arm64/Kconfig +4 -0
+	  driver from the its_irq_compose_msi_msg().
+
+	  If unsure, say Y.
+
 endmenu
 
 
drivers/irqchip/irq- renamed from drivers/irqchip/irq-gic-common.h +0 -0
gic-common.h
Quoted 10 lines
Quoted 10 lines
index 205e5fd..de0385a 100644
drivers/irqchip/irq-gic-common.h +1 -0
@@ -26,6 +26,7 @@ struct gic_quirk {
 	void (*init)(void *data);
 	u32 iidr;
 	u32 mask;
+	const char *erratum;
 };
 
Quoted 2 lines
drivers/irqchip/irq-gic-common.h struct gic_quirk {
Quoted 2 lines
drivers/irqchip/irq-gic-common.h +0 -1
 int gic_configure_irq(unsigned int irq, unsigned int type, diff
--git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-
gic-v3-its.c
Quoted 11 lines
Quoted 11 lines
index f471939..0a326f6 100644
drivers/irqchip/irq-gic-v3-its.c +1 -0
@@ -44,6 +44,7 @@
 #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING		(1ULL << 0)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_22375	(1ULL << 1)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_23144	(1ULL << 2)
+#define ITS_FLAGS_WORKAROUND_HISILICON_161010801	(1ULL << 3)
 
 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING	(1 << 0)
 
Quoted 7 lines
Quoted 7 lines
Quoted 1 line
drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct
Quoted 1 line
drivers/irqchip/irq-gic-v3-its.c +0 -0
@@ -659,7 +660,8 @@ static void its_irq_compose_msi_msg(struct
irq_data *d, struct msi_msg *msg)
Quoted 5 lines
drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg)
Quoted 5 lines
drivers/irqchip/irq-gic-v3-its.c +1 -1
 	msg->address_hi		= upper_32_bits(addr);
 	msg->data		= its_get_event_id(d);
 
-	iommu_dma_map_msi_msg(d->irq, msg);
+	if (!(its->flags &
ITS_FLAGS_WORKAROUND_HISILICON_161010801))
Quoted 10 lines
Quoted 10 lines
Quoted 3 lines
drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct irq_data *d, struct msi_msg *msg) · line 664
Quoted 1 line
drivers/irqchip/irq-gic-v3-its.c +1 -0
+		iommu_dma_map_msi_msg(d->irq, msg);
Let's contemplate this for a moment. If we're on the affected ITS,
we're
Quoted 2 lines
using the physical address of the GITS_TRANSLATER register. What guarantees that this is not going to conflict with an IOVA that DMA
is
Quoted 1 line
going to use? From looking at these patches, my feeling is "not
much".
Quoted 1 line
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
So if I'm right, you're opening the door to some interesting memory corruption if the two regions ever intersect.
Robin, what do you think?
Yup. Unless the ITS physical address is actually reserved from the IOVA domain, it's still free to be allocated for DMA mappings, and
if
Quoted 1 line
Quoted 1 line
that ever happens then you'll get odd bits of data landing in the
ITS
Quoted 3 lines
Quoted 3 lines
instead of RAM, and maybe even locked-up devices or worse if the doorbell gives back decode errors on read attempts. It's essentially the exact same problem as we have with memory-mapped PCI windows,
and
Quoted 1 line
Quoted 1 line
needs to be solved in the same fashion, i.e. between the SMMU and
the
Quoted 3 lines
Quoted 1 line
IOMMU-DMA code.
Is this something that can incorporated in Eric's latest patch
series[1]?
Quoted 4 lines
It does mentions reserved regions can be:
- directly mapped regions
- regions that cannot be iommu mapped (PCI host bridge windows, ...)
- MSI regions (because they belong to another address space or
because
Quoted 4 lines
they are not translated by the IOMMU and need special handling)
Though I am not clear our case comes under "the MSI regions that are not translated by the IOMMU and need special handling" or not.
Quoted 7 lines
Well, given that in your case, the IOMMU never sees the MSI write, it definitely falls into the "not translated" category.
As for handling it on top of Eric's series, that's probably the most reasonable thing to do, which also means that none of this should appear in the ITS driver. Robin seems to have an idea on how to approach this.

Quoted 6 lines

Ok. Thanks for that Marc/Robin.
But I am not sure we can get away with ITS driver. Because current vfio patch series[1] treats GICV3 ITS as irq safe and is setting IRQ_DOMAIN_FLAG_MSI_REMAP in ITS driver. But this is not the case with our ITS.

As far as I understand, it *is* the case, and largely the reason we're here in the first place. "MSI remapping" is an x86-specific term for the part of their IOMMU which intercepts writes to the architectural MSI region and can "remap" a given device's interrupts to different targets on the CPU side - i.e. more or less the same thing the ITS does with the device ID data which your implementation apparently needs this address-matching widget to append to the write. If your ITS couldn't actually distinguish between requesters (like a GICv2m) we'd have a *much* bigger problem.

Robin.

Quoted 7 lines

Thanks,
Shameer
[1] https://github.com/eauger/linux/commit/df2a17bd8cc73b5a381ed1570870a2aeb6f7e068

Shameerali Kolothum Thodi

author

2017-01-24 16:40 UTC #12 lore raw

Quoted 206 lines

-----Original Message-----
From: Robin Murphy [mailto:robin.murphy at arm.com]
Sent: Tuesday, January 24, 2017 4:30 PM
To: Shameerali Kolothum Thodi; Marc Zyngier; mark.rutland at arm.com; will.deacon at arm.com; eric.auger at redhat.com
Cc: linux-arm-kernel at lists.infradead.org; Linuxarm; linux- kernel at vger.kernel.org; devicetree at vger.kernel.org; John Garry; Guohanjun (Hanjun Guo)
Subject: Re: [RFC 2/4] irqchip, gicv3-its:Workaround for HiSilicon erratum 161010801
Quoted 33 lines · Shameerali Kolothum Thodi
Quoted 33 lines
-----Original Message-----
From: Marc Zyngier [mailto:marc.zyngier at arm.com]
Sent: Tuesday, January 24, 2017 3:50 PM
To: Shameerali Kolothum Thodi; Robin Murphy; mark.rutland at arm.com; will.deacon at arm.com; eric.auger at redhat.com
Cc: linux-arm-kernel at lists.infradead.org; Linuxarm; linux- kernel at vger.kernel.org; devicetree at vger.kernel.org; John Garry; Guohanjun (Hanjun Guo)
Subject: Re: [RFC 2/4] irqchip, gicv3-its:Workaround for HiSilicon erratum 161010801
Quoted 18 lines · Shameerali Kolothum Thodi
+Eric
Quoted 16 lines
-----Original Message-----
From: Robin Murphy [mailto:robin.murphy at arm.com]
Sent: Tuesday, January 24, 2017 2:42 PM
To: Marc Zyngier; Shameerali Kolothum Thodi; mark.rutland at arm.com; will.deacon at arm.com
Cc: linux-arm-kernel at lists.infradead.org; Linuxarm; linux- kernel at vger.kernel.org; devicetree at vger.kernel.org; John Garry; Guohanjun (Hanjun Guo)
Subject: Re: [RFC 2/4] irqchip, gicv3-its:Workaround for HiSilicon erratum 161010801
Quoted 4 lines · Marc Zyngier
+ Robin,
Quoted 1 line · Shameerali Kolothum Thodi
The HiSilicon erratum 161010801 describes the limitation of
certain
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 1 line
Quoted 1 line
HiSilicon platforms to support the SMMU mappings for MSI
transactions.
Quoted 2 lines
Quoted 2 lines
On these platforms GICv3 ITS translator is presented with the
deviceID
Quoted 1 line
Quoted 1 line
by extending the MSI payload data to 64 bits to include the
deviceID.
Quoted 1 line
Quoted 1 line
Hence, the PCIe controller on this platforms has to
differentiate
Quoted 9 lines
Quoted 9 lines
Quoted 4 lines
Quoted 4 lines
the
Quoted 1 line
Quoted 1 line
MSI payload against other DMA payload and has to modify the MSI
payload.
Quoted 1 line
Quoted 1 line
This basically makes it difficult for this platforms to have a
SMMU
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
translation for MSI. Also these platforms doesn't have a proper IIDR register to use the existing IIDR based quirk mechanism.
This workaround based on the devicetree binding property,
supports
Quoted 73 lines
Quoted 73 lines
Quoted 18 lines
Quoted 18 lines
Quoted 7 lines
Quoted 7 lines
bypassing the SMMU for the MSI transactions on this platforms.
Signed-off-by shameer <[email protected]>
---
 arch/arm64/Kconfig               | 15 ++++++++++++
 drivers/irqchip/irq-gic-common.h |  1 +
drivers/irqchip/irq-gic-v3-its.c | 52
+++++++++++++++++++++++++++++++++++++++-
Quoted 10 lines
Quoted 10 lines
 3 files changed, 67 insertions(+), 1 deletion(-)
arch/arm64/Kconfig index renamed from arch/arm64/Kconfig +0 -0
0ae0427..8d600b0 100644
arch/arm64/Kconfig +0 -0
@@ -485,6 +485,21 @@ config CAVIUM_ERRATUM_27456
 
 	  If unsure, say Y.
 
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
arch/arm64/Kconfig config CAVIUM_ERRATUM_27456 · line 494
Quoted 7 lines
arch/arm64/Kconfig +7 -0
+config HISILICON_ERRATUM_161010801
+	bool "HiSilicon erratum 161010801"
+	default y
+	help
+	  Enable workaround for erratum 161010801.
+
+	  This implements a gicv3-its errata workaround for
HiSilicon
Quoted 1 line
Quoted 1 line
Quoted 1 line
arch/arm64/Kconfig config CAVIUM_ERRATUM_27456 · line 495
Quoted 1 line
arch/arm64/Kconfig +1 -0
+	  platforms Hip05/Hip07. These platforms cannot support the
MSI
Quoted 1 line
Quoted 1 line
Quoted 1 line
arch/arm64/Kconfig config CAVIUM_ERRATUM_27456 · line 496
Quoted 1 line
arch/arm64/Kconfig +1 -0
+	  interrupt remapping and MSI transaction has to be
bypassed by
Quoted 3 lines
Quoted 3 lines
SMMU.
Quoted 2 lines
arch/arm64/Kconfig config CAVIUM_ERRATUM_27456 · line 498
Quoted 2 lines
arch/arm64/Kconfig +2 -0
+
+	  The fix is to avoid calling the remapping hook into the
SMMU
Quoted 39 lines
Quoted 39 lines
Quoted 9 lines
drivers/irqchip/irq-
Quoted 9 lines
arch/arm64/Kconfig +4 -0
+	  driver from the its_irq_compose_msi_msg().
+
+	  If unsure, say Y.
+
 endmenu
 
 
drivers/irqchip/irq- renamed from drivers/irqchip/irq-gic-common.h +0 -0
gic-common.h
Quoted 10 lines
Quoted 10 lines
index 205e5fd..de0385a 100644
drivers/irqchip/irq-gic-common.h +1 -0
@@ -26,6 +26,7 @@ struct gic_quirk {
 	void (*init)(void *data);
 	u32 iidr;
 	u32 mask;
+	const char *erratum;
 };
 
Quoted 2 lines
drivers/irqchip/irq-gic-common.h struct gic_quirk {
Quoted 2 lines
drivers/irqchip/irq-gic-common.h +0 -1
 int gic_configure_irq(unsigned int irq, unsigned int type, diff
--git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-
gic-v3-its.c
Quoted 9 lines
Quoted 9 lines
index f471939..0a326f6 100644
drivers/irqchip/irq-gic-v3-its.c +1 -0
@@ -44,6 +44,7 @@
 #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING		(1ULL << 0)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_22375	(1ULL << 1)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_23144	(1ULL << 2)
+#define ITS_FLAGS_WORKAROUND_HISILICON_161010801	(1ULL << 3)
 
Quoted 3 lines
drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct
Quoted 3 lines
drivers/irqchip/irq-gic-v3-its.c +0 -0
 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING	(1 << 0)
 
@@ -659,7 +660,8 @@ static void its_irq_compose_msi_msg(struct
irq_data *d, struct msi_msg *msg)
Quoted 3 lines
Quoted 3 lines
drivers/irqchip/irq-gic-v3-its.c +0 -0
 	msg->address_hi		= upper_32_bits(addr);
 	msg->data		= its_get_event_id(d);
 
Quoted 6 lines
Quoted 6 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct · line 663
Quoted 2 lines
drivers/irqchip/irq-gic-v3-its.c +1 -1
-	iommu_dma_map_msi_msg(d->irq, msg);
+	if (!(its->flags &
ITS_FLAGS_WORKAROUND_HISILICON_161010801))
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
drivers/irqchip/irq-gic-v3-its.c static void its_irq_compose_msi_msg(struct · line 664
Quoted 1 line
drivers/irqchip/irq-gic-v3-its.c +1 -0
+		iommu_dma_map_msi_msg(d->irq, msg);
Let's contemplate this for a moment. If we're on the affected
ITS,
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
we're
Quoted 2 lines
using the physical address of the GITS_TRANSLATER register. What guarantees that this is not going to conflict with an IOVA that
DMA
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
is
Quoted 1 line
going to use? From looking at these patches, my feeling is "not
much".
Quoted 2 lines
So if I'm right, you're opening the door to some interesting
memory
Quoted 11 lines
Quoted 11 lines
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
corruption if the two regions ever intersect.
Robin, what do you think?
Yup. Unless the ITS physical address is actually reserved from the IOVA domain, it's still free to be allocated for DMA mappings, and
if
Quoted 1 line
Quoted 1 line
that ever happens then you'll get odd bits of data landing in the
ITS
Quoted 2 lines
Quoted 2 lines
instead of RAM, and maybe even locked-up devices or worse if the doorbell gives back decode errors on read attempts. It's
essentially
Quoted 11 lines
Quoted 11 lines
Quoted 1 line
Quoted 1 line
the exact same problem as we have with memory-mapped PCI windows,
and
Quoted 1 line
Quoted 1 line
needs to be solved in the same fashion, i.e. between the SMMU and
the
Quoted 3 lines
Quoted 1 line
IOMMU-DMA code.
Is this something that can incorporated in Eric's latest patch
series[1]?
Quoted 3 lines
It does mentions reserved regions can be:
- directly mapped regions
- regions that cannot be iommu mapped (PCI host bridge windows,
...)
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
- MSI regions (because they belong to another address space or
because
Quoted 3 lines
they are not translated by the IOMMU and need special handling)
Though I am not clear our case comes under "the MSI regions that
are
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
not translated by the IOMMU and need special handling" or not.
Well, given that in your case, the IOMMU never sees the MSI write,
it
Quoted 10 lines
Quoted 6 lines
definitely falls into the "not translated" category.
As for handling it on top of Eric's series, that's probably the most reasonable thing to do, which also means that none of this should appear in the ITS driver. Robin seems to have an idea on how to approach this.
Ok. Thanks for that Marc/Robin.
But I am not sure we can get away with ITS driver. Because current
vfio
Quoted 2 lines
patch series[1] treats GICV3 ITS as irq safe and is setting IRQ_DOMAIN_FLAG_MSI_REMAP in ITS driver. But this is not the case
with
Quoted 1 line
our ITS.

Quoted 11 lines

As far as I understand, it *is* the case, and largely the reason we're here in the first place. "MSI remapping" is an x86-specific term for the
part of their IOMMU which intercepts writes to the architectural MSI region and can "remap" a given device's interrupts to different targets on the CPU side - i.e. more or less the same thing the ITS does with the
device ID data which your implementation apparently needs this address-matching widget to append to the write. If your ITS couldn't actually distinguish between requesters (like a GICv2m) we'd have a *much* bigger problem.

Ok. Got it. Since the PCIe RC tags the transaction with the device ID, the ITS is able to perform IRQ isolation and is safe.

Thanks,
Shameer

Mark Rutland

reviewer

2017-01-24 14:15 UTC #13 lore raw

In reply to Shameerali Kolothum Thodi

Quoted 9 lines · Shameerali Kolothum Thodi

The HiSilicon erratum 161010801 describes the limitation of certain HiSilicon platforms to support the SMMU mappings for MSI transactions.
On these platforms GICv3 ITS translator is presented with the deviceID by extending the MSI payload data to 64 bits to include the deviceID. Hence, the PCIe controller on this platforms has to differentiate the MSI payload against other DMA payload and has to modify the MSI payload. This basically makes it difficult for this platforms to have a SMMU translation for MSI.

Do you mean that the PCIe root controller looking at the (virtual) addresses of DMA and comparing these against the (physical) address of the ITS in order to determine if a write is an MSI?

I can't see anything in this patch specifically enabling bypass for MSIs. Do writes to the ITS (physical) address always bypass the SMMU, and go straight to the ITS? Regardless of translation applied to other DMA?

It sounds like this will have severe implications for virtualization.

Quoted 2 lines

Also these platforms doesn't have a proper IIDR register to use the existing IIDR based quirk mechanism.

What exactly is wrong with the IIDR on these platforms? That sounds like an erratum as of itself.

What precise value do reads of the IIDR return? Or do reads result in other erroneous behaviour?

Thanks,
Mark.

Shameerali Kolothum Thodi

author

2017-01-25 10:30 UTC #14 lore raw

Quoted 12 lines

-----Original Message-----
From: Mark Rutland [mailto:mark.rutland at arm.com]
Sent: Tuesday, January 24, 2017 2:15 PM
To: Shameerali Kolothum Thodi
Cc: marc.zyngier at arm.com; will.deacon at arm.com; linux-arm- kernel at lists.infradead.org; Linuxarm; linux-kernel at vger.kernel.org; devicetree at vger.kernel.org; John Garry; Guohanjun (Hanjun Guo); robin.murphy at arm.com
Subject: Re: [RFC 2/4] irqchip, gicv3-its:Workaround for HiSilicon erratum 161010801

sounds like this will have severe implications for virtualization.

Quoted 10 lines

Quoted 3 lines
Also these platforms doesn't have a proper IIDR register to use the existing IIDR based quirk mechanism.
What exactly is wrong with the IIDR on these platforms? That sounds like
an erratum as of itself.
What precise value do reads of the IIDR return? Or do reads result in other erroneous behaviour?

As far as I know, there is no erroneous behavior while reading. But the IIDR JEP106 identity code is 0 on these platforms, hence not much of use.

This will be corrected in next revision of hw.

Thanks,
Shameer