ACK: [SRU] [H/I/Unstable/OEM-5.10/OEM-5.13] [PATCH 0/1] Fix kernel panic caused by legacy devices on AMD platforms

[Andrea Righi]

On Thu, Aug 19, 2021 at 03:56:15PM +0800, Kai-Heng Feng wrote:
> On Thu, Aug 19, 2021 at 3:32 PM Andrea Righi <andrea.righi at canonical.com> wrote:
> >
> > On Sat, Jul 17, 2021 at 12:58:02AM +0800, Kai-Heng Feng wrote:
> > > BugLink: https://bugs.launchpad.net/bugs/1936682
> > >
> > > [Impact]
> > > When a legacy device is only 32bit DMA capable and it's in the same
> > > IOMMU group with iommu_v2 capable devices, the device in question will
> > > be forced to use identity mapping and triggers kernel panic on DMA
> > > operation because it can't do 64bit DMA.
> > >
> > > [Fix]
> > > Keep swiotlb enabled so legacy devices can do 64bit DMA. This is also
> > > how Intel and ARM64 platforms deal with legacy devices.
> > >
> > > [Test]
> > > Boot an affected system. Kernel panic in Realtek WiFi driver's probe
> > > routine.
> > > After the patch is applied, the system can work normally.
> > >
> > > [Where problems could occur]
> > > The default swiotlb uses 64MB memory, so if the system doesn't have any
> > > legacy device, there are 64MB ram less for the system to use.
> >
> > Isn't this going to affect other drivers/systems potentially? Keeping
> > the swiotlb enabled is going to add only the extra memory consumption
> > (64MB) or is it going to potentially affect also the performance of some
> > devices?
> 
> It's only going to affect devices that can do 32bit DMA, it's either
> use swiotlb or kernel panic.

I agree that a potential kernel panic is definitely worse than wasting
some memory.

> 
> >
> > Moreover, even if 64MB doesn't seem that much, it could be a significant
> > amount of memory in small systems (embedded, or potentially even VMs
> > with a limited amount of memory maybe).
> 
> Well, Intel and ARM64 always eat 64MB right now...
> For the other case, does guest VM need IOMMU DMA translation? I am not
> sure but I thought that's done in the host.

I'm quite sure that guest VMs can also use IOMMU DMA, but it's probably
a very specific case with device passthrough (maybe) and in such case
even if there's some extra memory usage it won't be a problem (I guess
device passthrough won't be used in really tiny VMs).

> 
> >
> > Would it be possible to force swiotlb to be enabled only when some of
> > these legacy 32-bit only DMA devices are present?
> 
> Yes, that's what maintainer suggested.
> But since iommu type can be changed at runtime, the fix isn't that trivial.

I see. OK thanks for the clarification. It'd be nice to keep track if
there are progresses on this upstream and eventually replace this SAUCE
patch with a better upstream solution, but for now it makes sense to
apply this patch.

So, not sure if it's still needed, but:

Acked-by: Andrea Righi <andrea.righi at canonical.com>