Re: [Xen-devel] [PATCH v2] arm, smmu: backport "Disable stalling faults for all endpoints"

[Julien Grall <julien.grall@xxxxxxx>]

On 10/29/18 9:14 PM, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefanos@xxxxxxxxxx>
>
> Backport commit 3714ce1d6655098ee69ede632883e5874d67e4ab
> "iommu/arm-smmu: Disable stalling faults for all endpoints" from the
> Linux kernel. This works-around Erratum #842869.
>
> Original commit message:
>
>    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.
>
>    Cc: <stable@xxxxxxxxxxxxxxx>
>    Fixes: 48ec83bcbcf5 ("iommu/arm-smmu: Add initial driver support for ARM SMMUv3 
> devices")
>    Reported-by: Matt Evans <matt.evans@xxxxxxx>
>    Signed-off-by: Will Deacon <will.deacon@xxxxxxx>
>
> Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
Acked-by: Julien Grall <julien.grall@xxxxxxx>

Cheers,

> ---
> Changes in v2:
> - add Erratum to docs/misc/arm/silicon-errata.txt
> - add in-code comment about Erratum
> ---
>   docs/misc/arm/silicon-errata.txt   |  1 +
>   xen/drivers/passthrough/arm/smmu.c | 40 ++++++++++++--------------------------
>   2 files changed, 13 insertions(+), 28 deletions(-)
>
> diff --git a/docs/misc/arm/silicon-errata.txt b/docs/misc/arm/silicon-errata.txt
> index c9854c3..906bf5f 100644
> --- a/docs/misc/arm/silicon-errata.txt
> +++ b/docs/misc/arm/silicon-errata.txt
> @@ -48,3 +48,4 @@ stable hypervisors.
>   | ARM            | Cortex-A57      | #852523         | N/A                    
>  |
>   | ARM            | Cortex-A57      | #832075         | ARM64_ERRATUM_832075   
>  |
>   | ARM            | Cortex-A57      | #834220         | ARM64_ERRATUM_834220   
>  |
> +| ARM            | MMU-500         | #842869         | N/A                     
> |
> diff --git a/xen/drivers/passthrough/arm/smmu.c 
> b/xen/drivers/passthrough/arm/smmu.c
> index b510399..9612c0f 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -898,8 +898,7 @@ static void arm_smmu_tlb_inv_context(struct arm_smmu_domain 
> *smmu_domain)
>   
>   static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
>   {
> -       int flags, ret;
> -       u32 fsr, far, fsynr, resume;
> +       u32 fsr, far, fsynr;
>         unsigned long iova;
>         struct iommu_domain *domain = dev;
>         struct arm_smmu_domain *smmu_domain = domain->priv;
> @@ -913,13 +912,7 @@ static irqreturn_t arm_smmu_context_fault(int irq, void 
> *dev)
>         if (!(fsr & FSR_FAULT))
>                 return IRQ_NONE;
>   
> -	if (fsr & FSR_IGN)
> -               dev_err_ratelimited(smmu->dev,
> -                                   "Unexpected context fault (fsr 0x%x)\n",
> -                                   fsr);
> -
>         fsynr = readl_relaxed(cb_base + ARM_SMMU_CB_FSYNR0);
> -       flags = fsynr & FSYNR0_WNR ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ;
>   
>   	far = readl_relaxed(cb_base + ARM_SMMU_CB_FAR_LO);
>         iova = far;
> @@ -928,25 +921,12 @@ static irqreturn_t arm_smmu_context_fault(int irq, void 
> *dev)
>         iova |= ((unsigned long)far << 32);
>   #endif
>   
> -	if (!report_iommu_fault(domain, smmu->dev, iova, flags)) {
> -               ret = IRQ_HANDLED;
> -               resume = RESUME_RETRY;
> -       } else {
> -               dev_err_ratelimited(smmu->dev,
> -                   "Unhandled context fault: iova=0x%08lx, fsynr=0x%x, 
> cb=%d\n",
> -                   iova, fsynr, cfg->cbndx);
> -               ret = IRQ_NONE;
> -               resume = RESUME_TERMINATE;
> -       }
> -
> -       /* Clear the faulting FSR */
> +       dev_err_ratelimited(smmu->dev,
> +       "Unhandled context fault: fsr=0x%x, iova=0x%08lx, fsynr=0x%x, cb=%d\n",
> +                           fsr, iova, fsynr, cfg->cbndx);
> +
>         writel(fsr, cb_base + ARM_SMMU_CB_FSR);
> -
> -       /* Retry or terminate any stalled transactions */
> -       if (fsr & FSR_SS)
> -               writel_relaxed(resume, cb_base + ARM_SMMU_CB_RESUME);
> -
> -       return ret;
> +       return IRQ_HANDLED;
>   }
>   
>   static irqreturn_t arm_smmu_global_fault(int irq, void *dev)
> @@ -1180,8 +1160,12 @@ static void arm_smmu_init_context_bank(struct 
> arm_smmu_domain *smmu_domain)
>                 writel_relaxed(reg, cb_base + ARM_SMMU_CB_S1_MAIR0);
>         }
>   
> -	/* SCTLR */
> -       reg = SCTLR_CFCFG | SCTLR_CFIE | SCTLR_CFRE | SCTLR_M | SCTLR_EAE_SBOP;
> +       /*
> +        * SCTLR
> +        *
> +        * Do not set SCTLR_CFCFG, because of Erratum #842869
> +        */
> +       reg = SCTLR_CFIE | SCTLR_CFRE | SCTLR_M | SCTLR_EAE_SBOP;
>         if (stage1)
>                 reg |= SCTLR_S1_ASIDPNE;
>   #ifdef __BIG_ENDIAN
--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel