* [PATCH] arm, smmu: backport "Disable stalling faults for all endpoints"
@ 2018-10-14 23:03 Stefano Stabellini
2018-10-26 18:47 ` Julien Grall
0 siblings, 1 reply; 3+ messages in thread
From: Stefano Stabellini @ 2018-10-14 23:03 UTC (permalink / raw)
To: julien.grall; +Cc: xen-devel, sstabellini
Backport commit 3714ce1d6655098ee69ede632883e5874d67e4ab
"iommu/arm-smmu: Disable stalling faults for all endpoints" from the
Linux kernel.
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@vger.kernel.org>
Fixes: 48ec83bcbcf5 ("iommu/arm-smmu: Add initial driver support for ARM SMMUv3 devices")
Reported-by: Matt Evans <matt.evans@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index b510399..8de5d16 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)
@@ -1181,7 +1161,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
}
/* SCTLR */
- reg = SCTLR_CFCFG | SCTLR_CFIE | SCTLR_CFRE | SCTLR_M | SCTLR_EAE_SBOP;
+ reg = SCTLR_CFIE | SCTLR_CFRE | SCTLR_M | SCTLR_EAE_SBOP;
if (stage1)
reg |= SCTLR_S1_ASIDPNE;
#ifdef __BIG_ENDIAN
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] arm, smmu: backport "Disable stalling faults for all endpoints"
2018-10-14 23:03 [PATCH] arm, smmu: backport "Disable stalling faults for all endpoints" Stefano Stabellini
@ 2018-10-26 18:47 ` Julien Grall
2018-10-29 21:08 ` Stefano Stabellini
0 siblings, 1 reply; 3+ messages in thread
From: Julien Grall @ 2018-10-26 18:47 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel
Hi Stefano,
On 10/15/18 12:03 AM, Stefano Stabellini wrote:
> Backport commit 3714ce1d6655098ee69ede632883e5874d67e4ab
> "iommu/arm-smmu: Disable stalling faults for all endpoints" from the
> Linux kernel.
>
> 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@vger.kernel.org>
> Fixes: 48ec83bcbcf5 ("iommu/arm-smmu: Add initial driver support for ARM SMMUv3 devices")
> Reported-by: Matt Evans <matt.evans@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
>
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
This is actually an errata on some SMMUv2 implementation. Can you update
docs/misc/arm/silicon-errata.txt accordingly?
>
> diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
> index b510399..8de5d16 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)
> @@ -1181,7 +1161,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
> }
>
> /* SCTLR */
> - reg = SCTLR_CFCFG | SCTLR_CFIE | SCTLR_CFRE | SCTLR_M | SCTLR_EAE_SBOP;
> + reg = SCTLR_CFIE | SCTLR_CFRE | SCTLR_M | SCTLR_EAE_SBOP;
It would also document here SCTLR_CFIE is not set because of erratum
#... This would avoid us to turn it on again by mistake.
Cheers,
> if (stage1)
> reg |= SCTLR_S1_ASIDPNE;
> #ifdef __BIG_ENDIAN
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] arm, smmu: backport "Disable stalling faults for all endpoints"
2018-10-26 18:47 ` Julien Grall
@ 2018-10-29 21:08 ` Stefano Stabellini
0 siblings, 0 replies; 3+ messages in thread
From: Stefano Stabellini @ 2018-10-29 21:08 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, Stefano Stabellini
On Fri, 26 Oct 2018, Julien Grall wrote:
> Hi Stefano,
>
> On 10/15/18 12:03 AM, Stefano Stabellini wrote:
> > Backport commit 3714ce1d6655098ee69ede632883e5874d67e4ab
> > "iommu/arm-smmu: Disable stalling faults for all endpoints" from the
> > Linux kernel.
> >
> > 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@vger.kernel.org>
> > Fixes: 48ec83bcbcf5 ("iommu/arm-smmu: Add initial driver support for ARM
> > SMMUv3 devices")
> > Reported-by: Matt Evans <matt.evans@arm.com>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> >
> > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
>
> This is actually an errata on some SMMUv2 implementation. Can you update
> docs/misc/arm/silicon-errata.txt accordingly?
OK
> >
> > diff --git a/xen/drivers/passthrough/arm/smmu.c
> > b/xen/drivers/passthrough/arm/smmu.c
> > index b510399..8de5d16 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)
> > @@ -1181,7 +1161,7 @@ static void arm_smmu_init_context_bank(struct
> > arm_smmu_domain *smmu_domain)
> > }
> > /* SCTLR */
> > - reg = SCTLR_CFCFG | SCTLR_CFIE | SCTLR_CFRE | SCTLR_M |
> > SCTLR_EAE_SBOP;
> > + reg = SCTLR_CFIE | SCTLR_CFRE | SCTLR_M | SCTLR_EAE_SBOP;
>
> It would also document here SCTLR_CFIE is not set because of erratum #... This
> would avoid us to turn it on again by mistake.
OK
> > if (stage1)
> > reg |= SCTLR_S1_ASIDPNE;
> > #ifdef __BIG_ENDIAN
> >
>
> --
> Julien Grall
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-10-29 21:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-14 23:03 [PATCH] arm, smmu: backport "Disable stalling faults for all endpoints" Stefano Stabellini
2018-10-26 18:47 ` Julien Grall
2018-10-29 21:08 ` Stefano Stabellini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.