linux-kernel - Re: [PATCH] sound: sof: ioc4-topology: avoid extra dai

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: <a91373cef3d55ba7f13f347e2b10b2398c347c35.camel@linux.intel.com>
Date: Wed, 07 Aug 2024 09:21:46 -0700
From: Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>
To: Arnd Bergmann <arnd@...db.de>, Mark Brown <broonie@...nel.org>, Arnd
 Bergmann <arnd@...nel.org>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>, Liam
 Girdwood <lgirdwood@...il.com>, Peter Ujfalusi
 <peter.ujfalusi@...ux.intel.com>, Bard Liao
 <yung-chuan.liao@...ux.intel.com>, Daniel Baluta <daniel.baluta@....com>, 
 Seppo Ingalsuo <seppo.ingalsuo@...ux.intel.com>, Kai Vehmanen
 <kai.vehmanen@...ux.intel.com>, Jaroslav Kysela <perex@...ex.cz>, Takashi
 Iwai <tiwai@...e.com>, Nathan Chancellor <nathan@...nel.org>, Nick
 Desaulniers <ndesaulniers@...gle.com>, Bill Wendling <morbo@...gle.com>,
 Justin Stitt <justinstitt@...gle.com>, Brent Lu <brent.lu@...el.com>,
 sound-open-firmware@...a-project.org,  linux-sound@...r.kernel.org,
 linux-kernel@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH] sound: sof: ioc4-topology: avoid extra dai_params copy

On Wed, 2024-08-07 at 17:18 +0200, Arnd Bergmann wrote:
> On Wed, Aug 7, 2024, at 17:09, Mark Brown wrote:
> > On Wed, Aug 07, 2024 at 10:02:27AM +0200, Arnd Bergmann wrote:
> > 
> > > From what I can tell, this was unintentional, as both
> > > sof_ipc4_prepare_dai_copier() and
> > > sof_ipc4_prepare_copier_module() make a
> > > copy for the same purpose, but copying it once has the exact same
> > > effect.
> > 
> > > Remove the extra copy and change the direct struct assignment to
> > > an explicit memcpy() call to make it clearer to the reader that
> > > this
> > > is what happens. Note that gcc treats struct assignment as a
> > > memcpy()
> > > that may be inlined anyway, so the resulting object code is the
> > > same.
> > 
> > The effect of the copy is to ensure that if the function fails the
> > argument is unmodified - did you do the analysis to check that it's
> > OK
> > to modify on error?  Your commit log says "the same purpose" but
> > never
> > specifies what that purpose is.
> 
> There is always a chance that I misunderstood the code, but
> yes, I did understand that the idea is to not modify the
> parameters inside of sof_ipc4_prepare_dai_copier.
Hi Arnd,

The idea behind the local copy is that the DAI widget needs to handle
its audio formats in topology differently from the other widgets in the
pipeline. So, locally the sof_ipc4_prepare_dai_copier() modifies the
params to make sure the right NHLT blobs are chosen based on what's
available in topology and the information passed in the params. But
when the params variable is passed on to the next widget in the
pipeline, any local modifications done by the DAI widget should be
carried forward.

For your reference, this is code that does the propagation of the
prepare callback for each widget in the playback/capture path in the
pipeline.
https://github.com/thesofproject/linux/blob/bc47b82db6e03d540061964d4540a371e7d344c8/sound/soc/sof/sof-audio.c#L442

Thanks,
Ranjani