+Eric > -----Original Message----- > From: Robin Murphy [mailto:robin.murphy@xxxxxxx] > Sent: Tuesday, January 24, 2017 2:42 PM > To: Marc Zyngier; Shameerali Kolothum Thodi; mark.rutland@xxxxxxx; > will.deacon@xxxxxxx > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; Linuxarm; linux- > kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; John Garry; > Guohanjun (Hanjun Guo) > Subject: Re: [RFC 2/4] irqchip, gicv3-its:Workaround for HiSilicon > erratum 161010801 > > On 24/01/17 14:11, Marc Zyngier wrote: > > + Robin, > > > > On 24/01/17 13:47, Shameerali Kolothum Thodi wrote: > >> 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 <shameerali.kolothum.thodi@xxxxxxxxxx> > >> --- > >> 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(-) > >> > >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > >> index 0ae0427..8d600b0 100644 > >> --- a/arch/arm64/Kconfig > >> +++ b/arch/arm64/Kconfig > >> @@ -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 > >> > >> > >> diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq- > gic-common.h > >> index 205e5fd..de0385a 100644 > >> --- a/drivers/irqchip/irq-gic-common.h > >> +++ b/drivers/irqchip/irq-gic-common.h > >> @@ -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, > >> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq- > gic-v3-its.c > >> index f471939..0a326f6 100644 > >> --- a/drivers/irqchip/irq-gic-v3-its.c > >> +++ b/drivers/irqchip/irq-gic-v3-its.c > >> @@ -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) > >> > >> @@ -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? > > 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
