On Fri, Sep 13, 2019 at 12:48:37PM +0100, Robin Murphy wrote:
> Although CONFIG_ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT is a welcome tool
> for smoking out inadequate firmware, the failure mode is non-obvious
> and can be confusing for end users. Add some special-case reporting of
> Unidentified Stream Faults to help clarify this particular symptom.
Having encountered this on a board that turned up this week, it may
be better to use the hex representation of the stream ID, especially
as it seems normal for the stream ID to be made up of implementation
defined bitfields.
If we want to stick with decimal, maybe masking the stream ID with
the number of allowable bits would be a good idea, so that the
decimal value remains meaningful should other bits be non-zero?
> CC: Douglas Anderson <diand...@chromium.org>
> Signed-off-by: Robin Murphy <robin.mur...@arm.com>
> ---
> drivers/iommu/arm-smmu.c | 5 +++++
> drivers/iommu/arm-smmu.h | 2 ++
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index b7cf24402a94..76ac8c180695 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -499,6 +499,11 @@ static irqreturn_t arm_smmu_global_fault(int irq, void
> *dev)
> dev_err_ratelimited(smmu->dev,
> "\tGFSR 0x%08x, GFSYNR0 0x%08x, GFSYNR1 0x%08x, GFSYNR2
> 0x%08x\n",
> gfsr, gfsynr0, gfsynr1, gfsynr2);
> + if (IS_ENABLED(CONFIG_ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT) &&
> + (gfsr & sGFSR_USF))
> + dev_err_ratelimited(smmu->dev,
> + "Stream ID %hu may not be described by firmware, try
> booting with \"arm-smmu.disable_bypass=0\"\n",
> + (u16)gfsynr1);
>
> arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_sGFSR, gfsr);
> return IRQ_HANDLED;
> diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
> index c9c13b5785f2..46f7e161e83e 100644
> --- a/drivers/iommu/arm-smmu.h
> +++ b/drivers/iommu/arm-smmu.h
> @@ -79,6 +79,8 @@
> #define ID7_MINOR GENMASK(3, 0)
>
> #define ARM_SMMU_GR0_sGFSR 0x48
> +#define sGFSR_USF BIT(2)
I do wonder if this is another instance where writing "(1 << 1)"
would have resulted in less chance of a mistake being made...
wrapping stuff up into macros is not always better!
9.6.15 SMMU_sGFSR, Global Fault Status Register
The SMMU_sGFSR bit assignments are:
USF, bit[1] Unidentified stream fault. The possible values of this
bit are:
0 No Unidentified stream fault.
1 Unidentified stream fault.
So this wants to be:
#define sGFSR_USF BIT(1)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
