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: <20250901130348.wqwdmovih7fq7rz4@skbuf>
Date: Mon, 1 Sep 2025 16:03:48 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: James Clark <james.clark@...aro.org>
Cc: Vladimir Oltean <olteanv@...il.com>, Mark Brown <broonie@...nel.org>,
	Arnd Bergmann <arnd@...db.de>,
	Larisa Grigore <larisa.grigore@....com>,
	Frank Li <Frank.li@....com>, Christoph Hellwig <hch@....de>,
	linux-spi@...r.kernel.org, imx@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 3/7] spi: spi-fsl-dspi: Stub out DMA functions

On Mon, Sep 01, 2025 at 10:09:10AM +0100, James Clark wrote:
> On 29/08/2025 3:26 pm, Vladimir Oltean wrote:
> > Hi James,
> > 
> > On Fri, Aug 29, 2025 at 12:46:45PM +0100, James Clark wrote:
> > > This will allow the build to succeed with !CONFIG_DMA_ENGINE, either due
> > > to a randconfig build test or when the target only uses one of the
> > > non-DMA transfer modes which this driver supports.
> > > 
> > > Signed-off-by: James Clark <james.clark@...aro.org>
> > > ---
> > 
> > Sorry for not paying attention to this previously. I didn't pick up from
> > previous conversations what problem this is addressing. I built an arm64
> > kernel with CONFIG_DMA_ENGINE disabled (by unsetting CONFIG_DMADEVICES)
> > and it got built successfully. The .config is attached.
> 
> Did you test with or without these patches applied? It's only needed to make
> the commit "spi: spi-fsl-dspi: Use non-coherent memory for DMA" build
> because that's a newer function that isn't stubbed out like the old ones.

Without the patches applied. Nowhere in the commit message is it written
that this change is preparatory for upcoming work.

> I originally tried to fix it in the DMA code so it got spun out into a new
> thread, which is probably why you missed it. And the commit message is
> missing the reported-by and closes tags which I'll add.

I see now.

> > Please post the specific build error if you haven't done so already, and
> > give a reasonable hint at the technical motivation in the commit message.
> 
> The reason we saw the issue was a randconfig build. But it's possible that
> someone is building the driver without DMA as well, hence: "or when the
> target only uses one of the non-DMA transfer modes which this driver
> supports".
> 
> The new commit message can be:
> 
>   spi: spi-fsl-dspi: Stub out DMA functions
> 
>   In a later commit we'll use dma_alloc_noncoherent() which isn't
>   stubbed out for builds without CONFIG_DMA_ENGINE and results in the
>   following build error:
> 
>     spi-fsl-dspi.c:(.text+0x644): undefined reference to
>        `dma_free_pages'
>     m68k-linux-ld: spi-fsl-dspi.c:(.text+0x67a): undefined reference to
>        `dma_free_pages'
> 
>   To continue to support devices that only need XSPI mode and so that
>   randconfig builds work, stub out DMA functionality in the DSPI driver.
> 
>   Reported-by: kernel test robot <lkp@...el.com>
>   Closes: https://lore.kernel.org/oe-kbuild-all/202506160036.t9VDxF6p-lkp@intel.com/
>   Signed-off-by: James Clark <james.clark@...aro.org>

There's no reason to mention the kernel test robot when addressing a
report that wasn't on merged code. Also, Closes: is for bug fixes, which
further confuses things because you are preemptively avoiding the bug
before it exists in merged code (as one should). Just provide a
two-sentence summary of why API-level stubbing wasn't preferred, and
a Link: to relevant discussions on the topic.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ