Re: [PATCH 4.19 070/118] iommu/arm-smmu-v3: Dont disable SMMU in kdump kernel

[Will Deacon]

[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