linux-kernel - Re: [PATCH v3 1/1] dmaengine: dw: Select only supported masters for ACPI devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZvElEesYTX-89u_g@smile.fi.intel.com>
Date: Mon, 23 Sep 2024 11:21:37 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Serge Semin <fancer.lancer@...il.com>
Cc: dmaengine@...r.kernel.org, linux-kernel@...r.kernel.org,
	Viresh Kumar <vireshk@...nel.org>, Vinod Koul <vkoul@...nel.org>,
	stable@...r.kernel.org, Ferry Toth <fntoth@...il.com>
Subject: Re: [PATCH v3 1/1] dmaengine: dw: Select only supported masters for
 ACPI devices

On Mon, Sep 23, 2024 at 01:01:08AM +0300, Serge Semin wrote:
> On Fri, Sep 20, 2024 at 06:56:17PM +0300, Andy Shevchenko wrote:

...

> > Fix the problem by specifying the default master ID for both memory
> > and peripheral devices in the driver data. Thus the issue noticed for
> > the iDMA 32-bit controllers will be eliminated and the ACPI-probed
> > DW DMA controllers will be configured with the correct master ID by
> > default.

> > ---
> > v3: rewrote to use driver_data
> > v2: https://lore.kernel.org/r/20240919185151.7331-1-fancer.lancer@gmail.com
> 
> IMO v2 looked better for me.

I disagree, obviously, since I sent a v3.
(I can drop your authorship and tags in v4)

> I am sure you know, but Master IDs is a
> platform-specific thing specific for each slave/peripheral device
> connected to the DMA controller. Depending on the chip design one
> peripheral device can be accessed over the one master IDs, another
> device/memory may have another master connected (can be up to four
> master IDs in general). That's why the master IDs have been declared
> in the dw_dma_slave structure.

Correct.

> So adding them to struct
> dw_dma_chip_pdata doesn't seem like a good idea seeing it contains the
> generic DW DMA controller info.

So far there is no evidence that the channels are integrated differently on
the same DMA controller over all hardware that uses this IP.

> On the contrary my implementation
> seems a bit more coherent since it just changes the default slave IDs
> defined in the dw_dma_acpi_filter() method and initialized in the
> dw_dma_slave instance without adding slave-specific fields to the
> generic controller data.

The default enumeration for PCI + ACPI or pure ACPI devices is not
changed with my patch, but actually makes it better (increases granularity).
The defaults are platform specific and that's what driver_data is for.

While you like your solution, the problem with it that it doesn't cover
different orders, so it's half-baked solution, I think. Mine doesn't
change the status quo about integration (see above) and has better approach
regarding different ordering. Both implementations have a flaw regarding per-channel master configuration.

> What seems like a much better alternative to the both approaches, is
> to use the dw_dma_slave instance defined in the mrfld_spi_setup()
> method for the Intel Merrifield SPI PXA2xx DMA-interface in
> drivers/spi/spi-pxa2xx-pci.c. But AFAICT that data is left unused
> since the DMA-engine handle and connection parameters are determined
> by the channel name. Right? Is it possible to make use of the
> filter-function specified to the dma_request_slave_channel_compat()
> method?

Unfortunately no, in ACPI case the only data we use is the name (index) of
the channel in the respective resources. Everything else is done automatically.

-- 
With Best Regards,
Andy Shevchenko