[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190616200654.GA13018@brain-police>
Date: Sun, 16 Jun 2019 21:06:55 +0100
From: Will Deacon <will@...nel.org>
To: Pavel Machek <pavel@...x.de>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-kernel@...r.kernel.org, stable@...r.kernel.org,
"Leizhen (ThunderTown)" <thunder.leizhen@...wei.com>,
Bhupesh Sharma <bhsharma@...hat.com>,
Will Deacon <will.deacon@....com>,
Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH 4.19 070/118] iommu/arm-smmu-v3: Dont disable SMMU in
kdump kernel
[FYI: This was in my spam folder. I'll reserve judgement on whether that's
the right decision.]
On Sun, Jun 16, 2019 at 09:42:36PM +0200, Pavel Machek wrote:
> > [ Upstream commit 3f54c447df34ff9efac7809a4a80fd3208efc619 ]
> >
> > Disabling the SMMU when probing from within a kdump kernel so that all
> > incoming transactions are terminated can prevent the core of the crashed
> > kernel from being transferred off the machine if all I/O devices are
> > behind the SMMU.
> >
> > Instead, continue to probe the SMMU after it is disabled so that we can
> > reinitialise it entirely and re-attach the DMA masters as they are reset.
> > Since the kdump kernel may not have drivers for all of the active DMA
> > masters, we suppress fault reporting to avoid spamming the console and
> > swamping the IRQ threads.
>
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -2414,13 +2414,9 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
> > /* Clear CR0 and sync (disables SMMU and queue processing) */
> > reg = readl_relaxed(smmu->base + ARM_SMMU_CR0);
> > if (reg & CR0_SMMUEN) {
> > - if (is_kdump_kernel()) {
> > - arm_smmu_update_gbpa(smmu, GBPA_ABORT, 0);
> > - arm_smmu_device_disable(smmu);
> > - return -EBUSY;
> > - }
> > -
> > dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n");
> > + WARN_ON(is_kdump_kernel() && !disable_bypass);
> > + arm_smmu_update_gbpa(smmu, GBPA_ABORT, 0);
> > }
> >
>
> This changes behaviour in !is_kdump_kernel() case. Is that
> ok/intended?
Yes, that's intentional. If we find the SMMU in an enabled state, it's
probably a good idea to configure it to abort all transactions before
disabling it, otherwise virtual addresses suddenly become physical addresses
and we could corrupt random memory. However, I don't think I've ever seen
this happen outside of kdump so it's admittedly a bit of a theoretical
scenario.
Regardless, patches to -stable should probably match their upstream
counterparts so even if this was an issue, I don't think this is the right
place to discuss it.
Will
Powered by blists - more mailing lists