Re: [PATCH v3] arm64: SMMU-v2: Workaround for Cavium ThunderX erratum 28168

[Akula, Geethasowjanya]

On 12/20/2016 03:56 AM, Will Deacon wrote:
> On Tue, Dec 20, 2016 at 11:52:58AM +, Marc Zyngier wrote:
>> On 20/12/16 11:06, Geetha sowjanya wrote:
 From: Tirumalesh Chalamarla 
 +#ifdef CONFIG_CAVIUM_ERRATUM_28168
 +/*
 + * Cavium ThunderX erratum 28168
 + *
 + * Due to erratum #28168 PCI-inbound MSI-X store to the interrupt
 + * controller are delivered to the interrupt controller before older
 + * PCI-inbound memory stores are committed. Doing a sync on SMMU
 + * will make sure all prior data transfers are completed before
 + * invoking ISR.
 + *
 + */
 +void dev_smmu_tlb_sync(struct device *dev)
 +{
 +   struct iommu_fwspec *fwspec = dev->iommu_fwspec;
 +   struct arm_smmu_device *smmu = fwspec_smmu(fwspec);
 +
 +   if (smmu)
 +   __arm_smmu_tlb_sync(smmu);
 +}
 +#endif
>>>
>>> I'll let Robin and Will comment on this, but it strikes be as rather odd
>>> that nothing will happen if the SMMU is in bypass or simply compiled
>>> out. So this workaround is at best incomplete.
>>
>> Agreed. Unless the SMMU is the cause of the issue, relying on it to
>> implement the workaround is fragile and unnecessarily introduces a linkage
>> between SMMU driver internals and the interrupt handling code.

We checked all possible ways. This is the only workaround we found.

>The SMMU is not the cause of the problem, it is the solution, and as
>such is required.

>Perhaps we should have a Kconfig "select" for the SMMU driver if
>CAVIUM_ERRATUM_28168 is selected.


>
> Not a fan.

>In general, I don't think anybody is a big fan of errata and their
>workarounds.

Thank you,
Geetha.

>
> Will
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH] iommu/arm-smmu-v3: Clear prior settings when updating STEs

[Nate Watterson]

To prevent corruption of the stage-1 context pointer field when
updating STEs, rebuild the entire containing dword instead of
clearing individual fields.

Signed-off-by: Nate Watterson 
---
 drivers/iommu/arm-smmu-v3.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 4d6ec44..94f305d 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1042,13 +1042,8 @@ static void arm_smmu_write_strtab_ent(struct 
arm_smmu_device *smmu, u32 sid,
}
}
 
-   /* Nuke the existing Config, as we're going to rewrite it */
-   val &= ~(STRTAB_STE_0_CFG_MASK << STRTAB_STE_0_CFG_SHIFT);
-
-   if (ste->valid)
-   val |= STRTAB_STE_0_V;
-   else
-   val &= ~STRTAB_STE_0_V;
+   /* Nuke the existing STE_0 value, as we're going to rewrite it */
+   val = ste->valid ? STRTAB_STE_0_V : 0;
 
if (ste->bypass) {
val |= disable_bypass ? STRTAB_STE_0_CFG_ABORT
@@ -1083,7 +1078,6 @@ static void arm_smmu_write_strtab_ent(struct 
arm_smmu_device *smmu, u32 sid,
val |= (ste->s1_cfg->cdptr_dma & STRTAB_STE_0_S1CTXPTR_MASK
<< STRTAB_STE_0_S1CTXPTR_SHIFT) |
STRTAB_STE_0_CFG_S1_TRANS;
-
}
 
if (ste->s2_cfg) {
-- 
Qualcomm Datacenter Technologies, Inc. on behalf of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux
Foundation Collaborative Project.

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC PATCH] iommu/arm-smmu: Add global SMR masking property

[Rob Herring]

On Fri, Dec 16, 2016 at 01:19:29PM +, Robin Murphy wrote:
> The current SMR masking support using a 2-cell iommu-specifier is
> primarily intended to handle individual masters with large and/or
> complex Stream ID assignments; it quickly gets a bit clunky in other SMR
> use-cases where we just want to consistently mask out the same part of
> every Stream ID (e.g. for MMU-500 configurations where the appended TBU
> number gets in the way unnecessarily). Let's add a new property to allow
> a single global mask value to better fit the latter situation.
> 
> CC: Stuart Yoder 
> Signed-off-by: Robin Murphy 
> ---
> 
> Compile-tested only...
> 
>  Documentation/devicetree/bindings/iommu/arm,smmu.txt | 8 
>  drivers/iommu/arm-smmu.c | 4 +++-
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt 
> b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index e862d1485205..98f5cbe5fdb4 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -60,6 +60,14 @@ conditions.
>aliases of secure registers have to be used during
>SMMU configuration.
>  
> +- stream-match-mask : Specifies a fixed SMR mask value to combine with

Needs a vendor prefix.

Otherwise looks fine.

> +  the Stream ID value from every iommu-specifier. This
> +  may be used instead of an "#iommu-cells" value of 2
> +  when there is no need for per-master SMR masks, but
> +  it is still desired to mask some portion of every
> +  Stream ID (e.g. for certain MMU-500 configurations
> +  given globally unique external IDs).
> +
>  ** Deprecated properties:
>  
>  - mmu-masters (deprecated in favour of the generic "iommus" binding) :
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3] arm64: SMMU-v2: Workaround for Cavium ThunderX erratum 28168

[David Daney]

On 12/20/2016 03:56 AM, Will Deacon wrote:

On Tue, Dec 20, 2016 at 11:52:58AM +, Marc Zyngier wrote:

On 20/12/16 11:06, Geetha sowjanya wrote:

From: Tirumalesh Chalamarla 
+#ifdef CONFIG_CAVIUM_ERRATUM_28168
+/*
+ * Cavium ThunderX erratum 28168
+ *
+ * Due to erratum #28168 PCI-inbound MSI-X store to the interrupt
+ * controller are delivered to the interrupt controller before older
+ * PCI-inbound memory stores are committed. Doing a sync on SMMU
+ * will make sure all prior data transfers are completed before
+ * invoking ISR.
+ *
+ */
+void dev_smmu_tlb_sync(struct device *dev)
+{
+   struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+   struct arm_smmu_device *smmu = fwspec_smmu(fwspec);
+
+   if (smmu)
+   __arm_smmu_tlb_sync(smmu);
+}
+#endif


I'll let Robin and Will comment on this, but it strikes be as rather odd
that nothing will happen if the SMMU is in bypass or simply compiled
out. So this workaround is at best incomplete.


Agreed. Unless the SMMU is the cause of the issue, relying on it to
implement the workaround is fragile and unnecessarily introduces a linkage
between SMMU driver internals and the interrupt handling code.



The SMMU is not the cause of the problem, it is the solution, and as 
such is required.


Perhaps we should have a Kconfig "select" for the SMMU driver if 
CAVIUM_ERRATUM_28168 is selected.





Not a fan.


In general, I don't think anybody is a big fan of errata and their 
workarounds.





Will

___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 3/4] iommu/arm-smmu: Disable stalling faults for all endpoints

[Will Deacon]

On Mon, Dec 19, 2016 at 02:33:36PM +0530, Sricharan wrote:
> >On Tue, Dec 06, 2016 at 06:30:21PM -0500, Rob Clark wrote:
> >> On Thu, Aug 18, 2016 at 9:05 AM, Will Deacon  wrote:
> >> > Enabling stalling faults can result in hardware deadlock on poorly
> >> > designed systems, particularly those with a PCI root complex upstream of
> >> > the SMMU.
> >> >
> >> > Although it's not really Linux's job to save hardware integrators from
> >> > their own misfortune, it *is* our job to stop userspace (e.g. VFIO
> >> > clients) from hosing the system for everybody else, even if they might
> >> > already be required to have elevated privileges.
> >> >
> >> > Given that the fault handling code currently executes entirely in IRQ
> >> > context, there is nothing that can sensibly be done to recover from
> >> > things like page faults anyway, so let's rip this code out for now and
> >> > avoid the potential for deadlock.
> >>
> >> so, I'd like to re-introduce this feature, I *guess* as some sort of
> >> opt-in quirk (ie. disabled by default unless something in DT tells you
> >> otherwise??  But I'm open to suggestions.  I'm not entirely sure what
> >> hw was having problems due to this feature.)
> >>
> >> On newer snapdragon devices we are using arm-smmu for the GPU, and
> >> halting the GPU so the driver's fault handler can dump some GPU state
> >> on faults is enormously helpful for debugging and tracking down where
> >> in the gpu cmdstream the fault was triggered.  In addition, we will
> >> eventually want the ability to update pagetables from fault handler
> >> and resuming the faulting transition.
> >
> >I'm not against reintroducing this, but it would certainly need to be
> >opt-in, as you suggest. If we want to try to use stall faults to enable
> >demand paging for DMA, then that means running core mm code to resolve
> >the fault and we can't do that in irq context. Consequently, we have to
> >hand this off to a thread, which means the hardware must allow the SS
> >bit to remain set without immediately reasserting the interrupt line.
> >Furthermore, we can't handle multiple faults on a context-bank, so we'd
> >need to restrict ourselves to one device (i.e. faulting stream) per
> >domain (CB).
> >
> >I think that means we want both specific compatible strings to identify
> >the SS bit behaviour, but also a way to opt-in for the stall model as a
> >separate property to indicate that the SoC integration can support this
> >without e.g. deadlocking.
> >
> 
> To understand the reason on the need for the quirk based on SS bit behavior,
> if the platform supports stall model and enabled, then SS bit should be 
> implemented
> and remain set until the RESUME register is written back, means same behavior
> always ?

The behaviour of the SS bit is IMPLEMENTATION DEFINED per the architecture,
so we need to know which way the given implementation chose to go. If we
want to support paging, then we absolutely need a way to return from the
interrupt handler without having handled the stall (i.e. without having
written to the RESUME register). That means that we mustn't take the same
interrupt immediately, otherwise we'll end up getting stuck in an infinite
fault. One hacky option would be to mask the interrupt at the GIC, but
that adds an additional requirement of one interrupt per context bank,
which isn't typically implemented in my experience.

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3] arm64: SMMU-v2: Workaround for Cavium ThunderX erratum 28168

[Will Deacon]

On Tue, Dec 20, 2016 at 11:52:58AM +, Marc Zyngier wrote:
> On 20/12/16 11:06, Geetha sowjanya wrote:
> > From: Tirumalesh Chalamarla 
> > +#ifdef CONFIG_CAVIUM_ERRATUM_28168
> > +/*
> > + * Cavium ThunderX erratum 28168
> > + *
> > + * Due to erratum #28168 PCI-inbound MSI-X store to the interrupt
> > + * controller are delivered to the interrupt controller before older
> > + * PCI-inbound memory stores are committed. Doing a sync on SMMU
> > + * will make sure all prior data transfers are completed before
> > + * invoking ISR.
> > + *
> > + */
> > +void dev_smmu_tlb_sync(struct device *dev)
> > +{
> > +   struct iommu_fwspec *fwspec = dev->iommu_fwspec;
> > +   struct arm_smmu_device *smmu = fwspec_smmu(fwspec);
> > +
> > +   if (smmu)
> > +   __arm_smmu_tlb_sync(smmu);
> > +}
> > +#endif
> 
> I'll let Robin and Will comment on this, but it strikes be as rather odd
> that nothing will happen if the SMMU is in bypass or simply compiled
> out. So this workaround is at best incomplete.

Agreed. Unless the SMMU is the cause of the issue, relying on it to
implement the workaround is fragile and unnecessarily introduces a linkage
between SMMU driver internals and the interrupt handling code.

Not a fan.

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3] arm64: SMMU-v2: Workaround for Cavium ThunderX erratum 28168

[Marc Zyngier]

Geetha,

On 20/12/16 11:06, Geetha sowjanya wrote:
> From: Tirumalesh Chalamarla 
> 
>   This patch implements Cavium ThunderX erratum 28168.
> 
>   PCI requires stores complete in order. Due to erratum #28168
>   PCI-inbound MSI-X store to the interrupt controller are delivered
>   to the interrupt controller before older PCI-inbound memory stores
>   are committed.
>   Doing a sync on SMMU will make sure all prior data transfers are
>   completed before invoking ISR.
> 
>  changes from v2:
> - added entry in Documentation/arm64/silicon-errata.txt
> - moved registration of the preflow_handler into
>msi_domain_ops.msi_finish() handler.
> - create linux/arm.smmu.h to expose smmu API.
> 
> Signed-off-by: Tirumalesh Chalamarla 

Where is your SoB? This is a requirement if you're picking a patch from
someone else.

> ---
>  Documentation/arm64/silicon-errata.txt   |  1 +
>  arch/arm64/Kconfig   | 12 
>  arch/arm64/include/asm/cpucaps.h |  3 +-
>  arch/arm64/kernel/cpu_errata.c   | 16 +++
>  drivers/iommu/arm-smmu.c | 22 +++
>  drivers/irqchip/irq-gic-v3-its-pci-msi.c | 48 
> +++-
>  include/linux/arm-smmu.h |  8 ++
>  7 files changed, 108 insertions(+), 2 deletions(-)
>  create mode 100644 include/linux/arm-smmu.h
> 
> diff --git a/Documentation/arm64/silicon-errata.txt 
> b/Documentation/arm64/silicon-errata.txt
> index 405da11..1311f90 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -61,5 +61,6 @@ stable kernels.
>  | Cavium | ThunderX GICv3  | #23154  | CAVIUM_ERRATUM_23154  
>   |
>  | Cavium | ThunderX Core   | #27456  | CAVIUM_ERRATUM_27456  
>   |
>  | Cavium | ThunderX SMMUv2 | #27704  | N/A  |
> +| Cavium | ThunderX PCI| #28168  | CAVIUM_ERRATUM_28168  
>   |
>  || | |   
>   |
>  | Freescale/NXP  | LS2080A/LS1043A | A-008585| FSL_ERRATUM_A008585   
>   |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 969ef88..cb647f4 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -474,6 +474,18 @@ config CAVIUM_ERRATUM_27456
>  
> If unsure, say Y.
>  
> +config CAVIUM_ERRATUM_28168
> +   bool "Cavium erratum 28168: Make sure DMA data transfer is done 
> before MSIX"
> +   depends on ARM64 && ARM_SMMU
> +   default y
> +   select IRQ_PREFLOW_FASTEOI
> +   help
> +Due to erratum #28168 PCI-inbound MSI-X store to the interrupt
> +controller are delivered to the interrupt controller before older
> +PCI-inbound memory stores are committed. Doing a sync on SMMU
> +will make sure all prior data transfers are done before invoking ISR.
> +
> +If unsure, say Y.
>  endmenu
>  
>  
> diff --git a/arch/arm64/include/asm/cpucaps.h 
> b/arch/arm64/include/asm/cpucaps.h
> index 87b4465..6975a01 100644
> --- a/arch/arm64/include/asm/cpucaps.h
> +++ b/arch/arm64/include/asm/cpucaps.h
> @@ -34,7 +34,8 @@
>  #define ARM64_HAS_32BIT_EL0  13
>  #define ARM64_HYP_OFFSET_LOW 14
>  #define ARM64_MISMATCHED_CACHE_LINE_SIZE 15
> +#define ARM64_WORKAROUND_CAVIUM_2816816
>  
> -#define ARM64_NCAPS  16
> +#define ARM64_NCAPS  17
>  
>  #endif /* __ASM_CPUCAPS_H */
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index b75e917..fac6d74 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -123,6 +123,22 @@ static int cpu_enable_trap_ctr_access(void *__unused)
>   MIDR_RANGE(MIDR_THUNDERX_81XX, 0x00, 0x00),
>   },
>  #endif
> +#ifdef CONFIG_CAVIUM_ERRATUM_28168
> + {
> + /* Cavium ThunderX, T88 pass 1.x - 2.1 */
> + .desc = "Cavium erratum 28168",
> + .capability = ARM64_WORKAROUND_CAVIUM_28168,
> + MIDR_RANGE(MIDR_THUNDERX, 0x00,
> +(1 << MIDR_VARIANT_SHIFT) | 1),
> + },
> + {
> + /* Cavium ThunderX, T81 pass 1.0 */
> + .desc = "Cavium erratum 28168",
> + .capability = ARM64_WORKAROUND_CAVIUM_28168,
> + MIDR_RANGE(MIDR_THUNDERX_81XX, 0x00, 0x00),
> + },
> +#endif
> +
>   {
>   .desc = "Mismatched cache line size",
>   .capability = ARM64_MISMATCHED_CACHE_LINE_SIZE,
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 06167da..451b393 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -49,6 +49,8 @@
>  #include 
>  
>  #include 
> +#include 
> +
>  
>  #include "io-pgtable.h"
>  
> @@ -577,6 +579,26 @@ static void 

[PATCH v3] arm64: SMMU-v2: Workaround for Cavium ThunderX erratum 28168

[Geetha sowjanya]

From: Tirumalesh Chalamarla 

  This patch implements Cavium ThunderX erratum 28168.

  PCI requires stores complete in order. Due to erratum #28168
  PCI-inbound MSI-X store to the interrupt controller are delivered
  to the interrupt controller before older PCI-inbound memory stores
  are committed.
  Doing a sync on SMMU will make sure all prior data transfers are
  completed before invoking ISR.

 changes from v2:
- added entry in Documentation/arm64/silicon-errata.txt
- moved registration of the preflow_handler into
   msi_domain_ops.msi_finish() handler.
- create linux/arm.smmu.h to expose smmu API.

Signed-off-by: Tirumalesh Chalamarla 
---
 Documentation/arm64/silicon-errata.txt   |  1 +
 arch/arm64/Kconfig   | 12 
 arch/arm64/include/asm/cpucaps.h |  3 +-
 arch/arm64/kernel/cpu_errata.c   | 16 +++
 drivers/iommu/arm-smmu.c | 22 +++
 drivers/irqchip/irq-gic-v3-its-pci-msi.c | 48 +++-
 include/linux/arm-smmu.h |  8 ++
 7 files changed, 108 insertions(+), 2 deletions(-)
 create mode 100644 include/linux/arm-smmu.h

diff --git a/Documentation/arm64/silicon-errata.txt 
b/Documentation/arm64/silicon-errata.txt
index 405da11..1311f90 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -61,5 +61,6 @@ stable kernels.
 | Cavium | ThunderX GICv3  | #23154  | CAVIUM_ERRATUM_23154
|
 | Cavium | ThunderX Core   | #27456  | CAVIUM_ERRATUM_27456
|
 | Cavium | ThunderX SMMUv2 | #27704  | N/A|
+| Cavium | ThunderX PCI| #28168  | CAVIUM_ERRATUM_28168
|
 || | | 
|
 | Freescale/NXP  | LS2080A/LS1043A | A-008585| FSL_ERRATUM_A008585 
|
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 969ef88..cb647f4 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -474,6 +474,18 @@ config CAVIUM_ERRATUM_27456
 
  If unsure, say Y.
 
+config CAVIUM_ERRATUM_28168
+   bool "Cavium erratum 28168: Make sure DMA data transfer is done before 
MSIX"
+   depends on ARM64 && ARM_SMMU
+   default y
+   select IRQ_PREFLOW_FASTEOI
+   help
+Due to erratum #28168 PCI-inbound MSI-X store to the interrupt
+controller are delivered to the interrupt controller before older
+PCI-inbound memory stores are committed. Doing a sync on SMMU
+will make sure all prior data transfers are done before invoking ISR.
+
+If unsure, say Y.
 endmenu
 
 
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index 87b4465..6975a01 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -34,7 +34,8 @@
 #define ARM64_HAS_32BIT_EL013
 #define ARM64_HYP_OFFSET_LOW   14
 #define ARM64_MISMATCHED_CACHE_LINE_SIZE   15
+#define ARM64_WORKAROUND_CAVIUM_28168  16
 
-#define ARM64_NCAPS16
+#define ARM64_NCAPS17
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index b75e917..fac6d74 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -123,6 +123,22 @@ static int cpu_enable_trap_ctr_access(void *__unused)
MIDR_RANGE(MIDR_THUNDERX_81XX, 0x00, 0x00),
},
 #endif
+#ifdef CONFIG_CAVIUM_ERRATUM_28168
+   {
+   /* Cavium ThunderX, T88 pass 1.x - 2.1 */
+   .desc = "Cavium erratum 28168",
+   .capability = ARM64_WORKAROUND_CAVIUM_28168,
+   MIDR_RANGE(MIDR_THUNDERX, 0x00,
+  (1 << MIDR_VARIANT_SHIFT) | 1),
+   },
+   {
+   /* Cavium ThunderX, T81 pass 1.0 */
+   .desc = "Cavium erratum 28168",
+   .capability = ARM64_WORKAROUND_CAVIUM_28168,
+   MIDR_RANGE(MIDR_THUNDERX_81XX, 0x00, 0x00),
+   },
+#endif
+
{
.desc = "Mismatched cache line size",
.capability = ARM64_MISMATCHED_CACHE_LINE_SIZE,
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 06167da..451b393 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -49,6 +49,8 @@
 #include 
 
 #include 
+#include 
+
 
 #include "io-pgtable.h"
 
@@ -577,6 +579,26 @@ static void __arm_smmu_tlb_sync(struct arm_smmu_device 
*smmu)
}
 }
 
+#ifdef CONFIG_CAVIUM_ERRATUM_28168
+/*
+ * Cavium ThunderX erratum 28168
+ *
+ * Due to erratum #28168 PCI-inbound MSI-X store to the interrupt
+ * controller are delivered to the interrupt controller before older
+ * PCI-inbound memory stores are committed. Doing a sync on SMMU
+ * will make sure all 

Re: [PATCH] iommu/arm-smmu-v3: avoid over allocating for l2 stream tables

[Will Deacon]

Hi Nate,

On Mon, Dec 19, 2016 at 03:26:40PM -0500, Nate Watterson wrote:
> Currently, all l2 stream tables are being allocated with space for
> (1< physically supports. To avoid allocating memory for inaccessible
> stes, this patch limits the span of an l2 table to be no larger
> than the sid size of the smmu to which it belongs.
> 
> Signed-off-by: Nate Watterson 
> ---
>  drivers/iommu/arm-smmu-v3.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)

I can't help but think you'd be better off using a linear stream table
in this scenario. If we hack the feature check for
ARM_SMMU_FEAT_2_LVL_STRTAB so that it doesn't report support for 2 level
tables if the number of sids is less than that covered by a single l2
entry, would that solve your problem?

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/arm-smmu-v3: prevent corruption of ste stage-1 context ptr

[Will Deacon]

Hi Nate,

Thanks for the patch.

On Mon, Dec 19, 2016 at 03:38:38PM -0500, Nate Watterson wrote:
> To ensure that the stage-1 context ptr for an ste points to the
> intended context descriptor, this patch adds code to clear away
> the stale context ptr value prior to or'ing in the new one.
> 
> Signed-off-by: Nate Watterson 
> ---
>  drivers/iommu/arm-smmu-v3.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 4d6ec44..093f9f1 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -1080,6 +1080,8 @@ static void arm_smmu_write_strtab_ent(struct 
> arm_smmu_device *smmu, u32 sid,
>   if (smmu->features & ARM_SMMU_FEAT_STALLS)
>   dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
>  
> + val &= ~(STRTAB_STE_0_S1CTXPTR_MASK <<
> +  STRTAB_STE_0_S1CTXPTR_SHIFT);
>   val |= (ste->s1_cfg->cdptr_dma & STRTAB_STE_0_S1CTXPTR_MASK
>   << STRTAB_STE_0_S1CTXPTR_SHIFT) |
>   STRTAB_STE_0_CFG_S1_TRANS;

Good catch. We only clear the Config field at present, although I think
it would be better if we just did val = 0 instead of clearing the Config
field, and then just recreate all of the S1-related fields (ctxptr, fmt,
cdmax) if we're installing a stage-1 STE. The other STE fields aren't
treated as read-modify-write, so it's more consistent not to treat the
initial dword specially other than for determining ste_live.

What do you think?

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu