Re: [PATCH 1/6] fake-dma: add fake dma engine driver

[Luis Chamberlain <mcgrof@kernel.org>]

On Thu, May 22, 2025 at 01:18:45PM +0200, Marek Szyprowski wrote:
> On 21.05.2025 19:07, Luis Chamberlain wrote:
> > On Wed, May 21, 2025 at 03:20:11PM +0100, Robin Murphy wrote:
> >> On 2025-05-20 11:39 pm, Luis Chamberlain wrote:
> >>> Today on x86_64 q35 guests we can't easily test some of the DMA API
> >>> with the dmatest out of the box because we lack a DMA engine as the
> >>> current qemu intel IOT patches are out of tree. This implements a basic
> >>> dma engine to let us use the dmatest API to expand on it and leverage
> >>> it on q35 guests.
> >> What does doing so ultimately achieve though?
> > What do you do to test for regressions automatically today for the DMA API?
> >
> > This patch series didn't just add a fake-dma engine though but let's
> > first address that as its what you raised a question for:
> >
> > Although I didn't add them, with this we can easily enable kernel
> > selftests to now allow any q35 guest to easily run basic API tests for the
> > DMA API. It's actually how I found the dma benchmark code, as its the only
> > selftest we have for DMA. However that benchmark test is not easy to
> > configure or enable. With kernel selftests you can test for things
> > outside of the scope of performance.
> 
> IMHO adding a fake driver just to use some of its side-effects that are 
> related with dma-mapping without the obvious information what would 
> actually be tested, is not the right approach. Maybe the dma benchmark 
> code can be extended with similar functionality as the selftests for 
> dma-engine, I didn't check yet.

I like the idea, however I will can save you time. The dmatest was
added with the requirement for a DMA controller exposing DMA channels
through the DMA engine. The dma benchmark does not, it test an existing device.

At a cursory glance, if we want to do something like this I think this
would be evaluating merging both. Merging both is possible if we're willing
to then make the DMA channel requests optional, as I don't think every
device which we'd bind to the DMA benchmark would / could use the DMA
channels.

The point to the fake-dma driver was to write a DMA controller which
simulates fake DMA channels and registers to the DMA engine, it exposes
these channels so we can leverage the dmatest. The DMA controller capabitilies
are a full feature set to ensure we can test all APIs used for them through
DMA channels. At first I was under the impression DMA controllers always
need to register DMA channels, however I'm now under the impression DMA
controllers don't need to expose DMA channels. Is that right?

If DMA channels are optional and a specialized feature only leveraged
by certain devices, then bundling this into dma-benchmark just
architecturally doesn't make sense.

> It would be better to have such  self-test in the proper layer.

Agreed. Perhaps the dmatest reflects the age of the evolution of the
DMA engine with some specialized DMA controllers with DMA channels,
either private or public for verfy specialized offload operations. And
that's optional?

Regardless, its a feature of the DMA engine, and if we want to keep
test coverage for it, my point that its not feasible today to test
dmatest on q35 guests stands, still validating the need for something
like the fake-dma driver.

Then we'd need stick with two selftests:

 - dmatest - perhaps should be renamed for the emphasis on channels
 - dma-benchmark - for regular DMA API

If this is correct, this patchset still seems to be going in the
right direction.

> If adding the needed functionality to dma 
> benchmark is not possible, then maybe create another self-test, which 
> will do similar calls to the dma-mapping api as those dma-engine 
> self-tests do, but without the whole dma-engine related part.

That's what this patchset does already. What I wish was easier was
to not have to *require* doing to unbind/bind of a real device. That
would allow us to also enable CI testing through virtualization. I'll
try the few knobs suggested by Leon to see if that enables it.

  Luis