[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5FC3163CFD30C246ABAA99954A238FA8174AC552@lhreml504-mbs>
Date: Tue, 24 Jan 2017 15:39:04 +0000
From: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@...wei.com>
To: Robin Murphy <robin.murphy@....com>,
Marc Zyngier <marc.zyngier@....com>,
"mark.rutland@....com" <mark.rutland@....com>,
"will.deacon@....com" <will.deacon@....com>,
"eric.auger@...hat.com" <eric.auger@...hat.com>
CC: "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
Linuxarm <linuxarm@...wei.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
John Garry <john.garry@...wei.com>,
"Guohanjun (Hanjun Guo)" <guohanjun@...wei.com>
Subject: RE: [RFC 2/4] irqchip, gicv3-its:Workaround for HiSilicon erratum
161010801
+Eric
> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@....com]
> Sent: Tuesday, January 24, 2017 2:42 PM
> To: Marc Zyngier; Shameerali Kolothum Thodi; mark.rutland@....com;
> will.deacon@....com
> Cc: linux-arm-kernel@...ts.infradead.org; Linuxarm; linux-
> kernel@...r.kernel.org; devicetree@...r.kernel.org; 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@...wei.com>
> >> ---
> >> 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
Powered by blists - more mailing lists