2023-08-14 13:57:34

by Takashi Iwai

[permalink] [raw]
Subject: [PATCH 00/25] ALSA: Generic PCM copy ops using iov_iter

Hi,

this is a revised patch set for cleaning up the PCM copy ops using
iov_iter to deal with kernel / user-space pointers consistently.

The previous patch set was based on sockptr_t:
https://lore.kernel.org/r/[email protected]
But this approach was NAK'ed as iov_iter is a preferred way for
achieving the purpose:
https://lore.kernel.org/r/[email protected]

The patch set starts with the missing export of import_ubuf()
function, followed by the new copy_ops using iov_iter, extensions of
some helpers and replacements of the existing code. It resulted in a
good amount of code reduction.


Takashi

===

Cc: Alexander Viro <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Andrey Utkin <[email protected]>
Cc: Anton Sviridenko <[email protected]>
Cc: Arnaud Pouliquen <[email protected]>
Cc: Banajit Goswami <[email protected]>
Cc: Bluecherry Maintainers <[email protected]>
Cc: Claudiu Beznea <[email protected]>
Cc: Ismael Luceno <[email protected]>
Cc: Lars-Peter Clausen <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Mauro Carvalho Chehab <[email protected]>
Cc: Oleksandr Andrushchenko <[email protected]>
Cc: Olivier Moysan <[email protected]>
Cc: Srinivas Kandagatla <[email protected]>
Cc: [email protected]
Cc: [email protected]

===

Takashi Iwai (25):
iov_iter: Export import_ubuf()
ALSA: pcm: Add copy ops with iov_iter
ALSA: core: Add memory copy helpers between iov_iter and iomem
ALSA: dummy: Convert to generic PCM copy ops
ALSA: gus: Convert to generic PCM copy ops
ALSA: emu8000: Convert to generic PCM copy ops
ALSA: es1938: Convert to generic PCM copy ops
ALSA: korg1212: Convert to generic PCM copy ops
ALSA: nm256: Convert to generic PCM copy ops
ALSA: rme32: Convert to generic PCM copy ops
ALSA: rme96: Convert to generic PCM copy ops
ALSA: hdsp: Convert to generic PCM copy ops
ALSA: rme9652: Convert to generic PCM copy ops
ALSA: sh: Convert to generic PCM copy ops
ALSA: xen: Convert to generic PCM copy ops
ALSA: pcmtest: Update comment about PCM copy ops
media: solo6x10: Convert to generic PCM copy ops
ASoC: component: Add generic PCM copy ops
ASoC: mediatek: Convert to generic PCM copy ops
ASoC: qcom: Convert to generic PCM copy ops
ASoC: dmaengine: Convert to generic PCM copy ops
ASoC: dmaengine: Use iov_iter for process callback, too
ALSA: doc: Update description for the new PCM copy ops
ASoC: pcm: Drop obsoleted PCM copy_user ops
ALSA: pcm: Drop obsoleted PCM copy_user and copy_kernel ops

.../kernel-api/writing-an-alsa-driver.rst | 58 ++++-------
drivers/media/pci/solo6x10/solo6x10-g723.c | 38 +-------
include/sound/dmaengine_pcm.h | 2 +-
include/sound/pcm.h | 13 ++-
include/sound/soc-component.h | 14 +--
lib/iov_iter.c | 1 +
sound/core/memory.c | 56 +++++++++--
sound/core/pcm_lib.c | 95 ++++++++++---------
sound/core/pcm_native.c | 2 +-
sound/drivers/dummy.c | 12 +--
sound/drivers/pcmtest.c | 2 +-
sound/isa/gus/gus_pcm.c | 23 +----
sound/isa/sb/emu8000_pcm.c | 74 ++++-----------
sound/pci/es1938.c | 30 +-----
sound/pci/korg1212/korg1212.c | 50 +++-------
sound/pci/nm256/nm256.c | 42 ++------
sound/pci/rme32.c | 50 +++-------
sound/pci/rme96.c | 42 ++------
sound/pci/rme9652/hdsp.c | 42 ++------
sound/pci/rme9652/rme9652.c | 46 ++-------
sound/sh/sh_dac_audio.c | 25 +----
sound/soc/atmel/mchp-pdmc.c | 2 +-
sound/soc/mediatek/common/mtk-btcvsd.c | 23 ++---
sound/soc/qcom/lpass-platform.c | 13 +--
sound/soc/soc-component.c | 10 +-
sound/soc/soc-generic-dmaengine-pcm.c | 18 ++--
sound/soc/soc-pcm.c | 4 +-
sound/soc/stm/stm32_sai_sub.c | 2 +-
sound/xen/xen_snd_front_alsa.c | 55 ++---------
29 files changed, 263 insertions(+), 581 deletions(-)

--
2.35.3



2023-08-14 14:00:34

by Takashi Iwai

[permalink] [raw]
Subject: [PATCH 14/25] ALSA: sh: Convert to generic PCM copy ops

This patch converts the sh_dac_audio driver code to use the new
unified PCM copy callback. It's a straightforward conversion from
*_user() to *_iter() variants.

Signed-off-by: Takashi Iwai <[email protected]>
---
sound/sh/sh_dac_audio.c | 25 +++----------------------
1 file changed, 3 insertions(+), 22 deletions(-)

diff --git a/sound/sh/sh_dac_audio.c b/sound/sh/sh_dac_audio.c
index 8cf571955c9d..95ba3abd4e47 100644
--- a/sound/sh/sh_dac_audio.c
+++ b/sound/sh/sh_dac_audio.c
@@ -158,12 +158,12 @@ static int snd_sh_dac_pcm_trigger(struct snd_pcm_substream *substream, int cmd)

static int snd_sh_dac_pcm_copy(struct snd_pcm_substream *substream,
int channel, unsigned long pos,
- void __user *src, unsigned long count)
+ struct iov_iter *src, unsigned long count)
{
/* channel is not used (interleaved data) */
struct snd_sh_dac *chip = snd_pcm_substream_chip(substream);

- if (copy_from_user_toio(chip->data_buffer + pos, src, count))
+ if (copy_from_iter_toio(chip->data_buffer + pos, src, count))
return -EFAULT;
chip->buffer_end = chip->data_buffer + pos + count;

@@ -175,24 +175,6 @@ static int snd_sh_dac_pcm_copy(struct snd_pcm_substream *substream,
return 0;
}

-static int snd_sh_dac_pcm_copy_kernel(struct snd_pcm_substream *substream,
- int channel, unsigned long pos,
- void *src, unsigned long count)
-{
- /* channel is not used (interleaved data) */
- struct snd_sh_dac *chip = snd_pcm_substream_chip(substream);
-
- memcpy_toio(chip->data_buffer + pos, src, count);
- chip->buffer_end = chip->data_buffer + pos + count;
-
- if (chip->empty) {
- chip->empty = 0;
- dac_audio_start_timer(chip);
- }
-
- return 0;
-}
-
static int snd_sh_dac_pcm_silence(struct snd_pcm_substream *substream,
int channel, unsigned long pos,
unsigned long count)
@@ -227,8 +209,7 @@ static const struct snd_pcm_ops snd_sh_dac_pcm_ops = {
.prepare = snd_sh_dac_pcm_prepare,
.trigger = snd_sh_dac_pcm_trigger,
.pointer = snd_sh_dac_pcm_pointer,
- .copy_user = snd_sh_dac_pcm_copy,
- .copy_kernel = snd_sh_dac_pcm_copy_kernel,
+ .copy = snd_sh_dac_pcm_copy,
.fill_silence = snd_sh_dac_pcm_silence,
.mmap = snd_pcm_lib_mmap_iomem,
};
--
2.35.3


2023-08-14 14:05:10

by Takashi Iwai

[permalink] [raw]
Subject: [PATCH 11/25] ALSA: rme96: Convert to generic PCM copy ops

This patch converts the rme96 driver code to use the new unified PCM
copy callback. It's a straightforward conversion from *_user() to
*_iter() variants.

Signed-off-by: Takashi Iwai <[email protected]>
---
sound/pci/rme96.c | 42 ++++++++----------------------------------
1 file changed, 8 insertions(+), 34 deletions(-)

diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c
index bccb7e0d3d11..6b5ffb18197b 100644
--- a/sound/pci/rme96.c
+++ b/sound/pci/rme96.c
@@ -320,48 +320,26 @@ snd_rme96_playback_silence(struct snd_pcm_substream *substream,
static int
snd_rme96_playback_copy(struct snd_pcm_substream *substream,
int channel, unsigned long pos,
- void __user *src, unsigned long count)
+ struct iov_iter *src, unsigned long count)
{
struct rme96 *rme96 = snd_pcm_substream_chip(substream);

- return copy_from_user_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos,
+ return copy_from_iter_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos,
src, count);
}

-static int
-snd_rme96_playback_copy_kernel(struct snd_pcm_substream *substream,
- int channel, unsigned long pos,
- void *src, unsigned long count)
-{
- struct rme96 *rme96 = snd_pcm_substream_chip(substream);
-
- memcpy_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, src, count);
- return 0;
-}
-
static int
snd_rme96_capture_copy(struct snd_pcm_substream *substream,
int channel, unsigned long pos,
- void __user *dst, unsigned long count)
+ struct iov_iter *dst, unsigned long count)
{
struct rme96 *rme96 = snd_pcm_substream_chip(substream);

- return copy_to_user_fromio(dst,
+ return copy_to_iter_fromio(dst,
rme96->iobase + RME96_IO_REC_BUFFER + pos,
count);
}

-static int
-snd_rme96_capture_copy_kernel(struct snd_pcm_substream *substream,
- int channel, unsigned long pos,
- void *dst, unsigned long count)
-{
- struct rme96 *rme96 = snd_pcm_substream_chip(substream);
-
- memcpy_fromio(dst, rme96->iobase + RME96_IO_REC_BUFFER + pos, count);
- return 0;
-}
-
/*
* Digital output capabilities (S/PDIF)
*/
@@ -1518,8 +1496,7 @@ static const struct snd_pcm_ops snd_rme96_playback_spdif_ops = {
.prepare = snd_rme96_playback_prepare,
.trigger = snd_rme96_playback_trigger,
.pointer = snd_rme96_playback_pointer,
- .copy_user = snd_rme96_playback_copy,
- .copy_kernel = snd_rme96_playback_copy_kernel,
+ .copy = snd_rme96_playback_copy,
.fill_silence = snd_rme96_playback_silence,
.mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1531,8 +1508,7 @@ static const struct snd_pcm_ops snd_rme96_capture_spdif_ops = {
.prepare = snd_rme96_capture_prepare,
.trigger = snd_rme96_capture_trigger,
.pointer = snd_rme96_capture_pointer,
- .copy_user = snd_rme96_capture_copy,
- .copy_kernel = snd_rme96_capture_copy_kernel,
+ .copy = snd_rme96_capture_copy,
.mmap = snd_pcm_lib_mmap_iomem,
};

@@ -1543,8 +1519,7 @@ static const struct snd_pcm_ops snd_rme96_playback_adat_ops = {
.prepare = snd_rme96_playback_prepare,
.trigger = snd_rme96_playback_trigger,
.pointer = snd_rme96_playback_pointer,
- .copy_user = snd_rme96_playback_copy,
- .copy_kernel = snd_rme96_playback_copy_kernel,
+ .copy = snd_rme96_playback_copy,
.fill_silence = snd_rme96_playback_silence,
.mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1556,8 +1531,7 @@ static const struct snd_pcm_ops snd_rme96_capture_adat_ops = {
.prepare = snd_rme96_capture_prepare,
.trigger = snd_rme96_capture_trigger,
.pointer = snd_rme96_capture_pointer,
- .copy_user = snd_rme96_capture_copy,
- .copy_kernel = snd_rme96_capture_copy_kernel,
+ .copy = snd_rme96_capture_copy,
.mmap = snd_pcm_lib_mmap_iomem,
};

--
2.35.3


2023-08-14 14:07:53

by Takashi Iwai

[permalink] [raw]
Subject: [PATCH 07/25] ALSA: es1938: Convert to generic PCM copy ops

This patch converts the es1938 driver code to use the new unified PCM
copy callback. It's a straightforward conversion from *_user() to
*_iter() variants in most parts.

Note that copy_from/to_iter() returns the copied bytes, hence the
error condition is inverted from copy_from/to_user().

Signed-off-by: Takashi Iwai <[email protected]>
---
sound/pci/es1938.c | 30 +++++-------------------------
1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/sound/pci/es1938.c b/sound/pci/es1938.c
index e34ec6f89e7e..9e28e9fecd2a 100644
--- a/sound/pci/es1938.c
+++ b/sound/pci/es1938.c
@@ -824,7 +824,7 @@ static snd_pcm_uframes_t snd_es1938_playback_pointer(struct snd_pcm_substream *s

static int snd_es1938_capture_copy(struct snd_pcm_substream *substream,
int channel, unsigned long pos,
- void __user *dst, unsigned long count)
+ struct iov_iter *dst, unsigned long count)
{
struct snd_pcm_runtime *runtime = substream->runtime;
struct es1938 *chip = snd_pcm_substream_chip(substream);
@@ -832,36 +832,17 @@ static int snd_es1938_capture_copy(struct snd_pcm_substream *substream,
if (snd_BUG_ON(pos + count > chip->dma1_size))
return -EINVAL;
if (pos + count < chip->dma1_size) {
- if (copy_to_user(dst, runtime->dma_area + pos + 1, count))
+ if (!copy_to_iter(runtime->dma_area + pos + 1, count, dst))
return -EFAULT;
} else {
- if (copy_to_user(dst, runtime->dma_area + pos + 1, count - 1))
+ if (!copy_to_iter(runtime->dma_area + pos + 1, count - 1, dst))
return -EFAULT;
- if (put_user(runtime->dma_area[0],
- ((unsigned char __user *)dst) + count - 1))
+ if (!copy_to_iter(runtime->dma_area, 1, dst))
return -EFAULT;
}
return 0;
}

-static int snd_es1938_capture_copy_kernel(struct snd_pcm_substream *substream,
- int channel, unsigned long pos,
- void *dst, unsigned long count)
-{
- struct snd_pcm_runtime *runtime = substream->runtime;
- struct es1938 *chip = snd_pcm_substream_chip(substream);
-
- if (snd_BUG_ON(pos + count > chip->dma1_size))
- return -EINVAL;
- if (pos + count < chip->dma1_size) {
- memcpy(dst, runtime->dma_area + pos + 1, count);
- } else {
- memcpy(dst, runtime->dma_area + pos + 1, count - 1);
- runtime->dma_area[0] = *((unsigned char *)dst + count - 1);
- }
- return 0;
-}
-
/* ----------------------------------------------------------------------
* Audio1 Capture (ADC)
* ----------------------------------------------------------------------*/
@@ -987,8 +968,7 @@ static const struct snd_pcm_ops snd_es1938_capture_ops = {
.prepare = snd_es1938_capture_prepare,
.trigger = snd_es1938_capture_trigger,
.pointer = snd_es1938_capture_pointer,
- .copy_user = snd_es1938_capture_copy,
- .copy_kernel = snd_es1938_capture_copy_kernel,
+ .copy = snd_es1938_capture_copy,
};

static int snd_es1938_new_pcm(struct es1938 *chip, int device)
--
2.35.3


2023-08-14 14:38:18

by Takashi Iwai

[permalink] [raw]
Subject: [PATCH 21/25] ASoC: dmaengine: Convert to generic PCM copy ops

This patch converts the ASoC dmaenging driver code to use the new
unified PCM copy callback. It's a straightforward conversion from
*_user() to *_iter() variants.

The process callback is still using the direct pointer as of now, but
it'll be converted in the next patch.

Note that copy_from/to_iter() returns the copied bytes, hence the
error condition is inverted from copy_from/to_user().

Cc: Lars-Peter Clausen <[email protected]>
Cc: Mark Brown <[email protected]>
Signed-off-by: Takashi Iwai <[email protected]>
---
sound/soc/soc-generic-dmaengine-pcm.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
index 3b99f619e37e..1a4f000fddb9 100644
--- a/sound/soc/soc-generic-dmaengine-pcm.c
+++ b/sound/soc/soc-generic-dmaengine-pcm.c
@@ -287,10 +287,10 @@ static snd_pcm_uframes_t dmaengine_pcm_pointer(
return snd_dmaengine_pcm_pointer(substream);
}

-static int dmaengine_copy_user(struct snd_soc_component *component,
- struct snd_pcm_substream *substream,
- int channel, unsigned long hwoff,
- void __user *buf, unsigned long bytes)
+static int dmaengine_copy(struct snd_soc_component *component,
+ struct snd_pcm_substream *substream,
+ int channel, unsigned long hwoff,
+ struct iov_iter *buf, unsigned long bytes)
{
struct snd_pcm_runtime *runtime = substream->runtime;
struct dmaengine_pcm *pcm = soc_component_to_pcm(component);
@@ -300,19 +300,20 @@ static int dmaengine_copy_user(struct snd_soc_component *component,
bool is_playback = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
void *dma_ptr = runtime->dma_area + hwoff +
channel * (runtime->dma_bytes / runtime->channels);
+ void *ptr = (void __force *)iter_iov_addr(buf);

if (is_playback)
- if (copy_from_user(dma_ptr, buf, bytes))
+ if (!copy_from_iter(dma_ptr, bytes, buf))
return -EFAULT;

if (process) {
- int ret = process(substream, channel, hwoff, (__force void *)buf, bytes);
+ int ret = process(substream, channel, hwoff, ptr, bytes);
if (ret < 0)
return ret;
}

if (!is_playback)
- if (copy_to_user(buf, dma_ptr, bytes))
+ if (!copy_to_iter(dma_ptr, bytes, buf))
return -EFAULT;

return 0;
@@ -337,7 +338,7 @@ static const struct snd_soc_component_driver dmaengine_pcm_component_process = {
.hw_params = dmaengine_pcm_hw_params,
.trigger = dmaengine_pcm_trigger,
.pointer = dmaengine_pcm_pointer,
- .copy_user = dmaengine_copy_user,
+ .copy = dmaengine_copy,
.pcm_construct = dmaengine_pcm_new,
};

--
2.35.3


2023-08-20 02:27:40

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 21/25] ASoC: dmaengine: Convert to generic PCM copy ops

On Mon, Aug 14, 2023 at 01:55:19PM +0200, Takashi Iwai wrote:
> This patch converts the ASoC dmaenging driver code to use the new
> unified PCM copy callback. It's a straightforward conversion from
> *_user() to *_iter() variants.
>
> The process callback is still using the direct pointer as of now, but
> it'll be converted in the next patch.
>
> Note that copy_from/to_iter() returns the copied bytes, hence the
> error condition is inverted from copy_from/to_user().

...

> if (is_playback)
> - if (copy_from_user(dma_ptr, buf, bytes))
> + if (!copy_from_iter(dma_ptr, bytes, buf))

!= bytes ?

> return -EFAULT;

Can be compressed to a single conditional:

if (is_playback && copy_from_iter(dma_ptr, bytes, buf) != bytes)

...

> if (!is_playback)
> - if (copy_to_user(buf, dma_ptr, bytes))
> + if (!copy_to_iter(dma_ptr, bytes, buf))
> return -EFAULT;

As per above.

--
With Best Regards,
Andy Shevchenko