[PATCH v2 02/25] ALSA: pcm: Add copy ops with iov_iter

From: Takashi Iwai
Date: Tue Aug 15 2023 - 15:03:25 EST


iov_iter is a universal interface to copy the data chunk from/to
user-space and kernel in a unified manner. This API can fit for ALSA
PCM copy ops, too; we had to split to copy_user and copy_kernel in the
past, and those can be unified to a single ops with iov_iter.

This patch adds a new PCM copy ops that passes iov_iter for copying
both kernel and user-space in the same way. This patch touches only
the ALSA PCM core part, and the actual users will be replaced in the
following patches.

The expansion of iov_iter is done in the PCM core right before calling
each copy callback. It's a bit suboptimal, but I took this now as
it's the most straightforward replacement. The more conversion to
iov_iter in the caller side is a TODO for future.

As of now, the old copy_user and copy_kernel ops are still kept.
Once after all users are converted, we'll drop the old copy_user and
copy_kernel ops, too.

Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
---
include/sound/pcm.h | 3 ++
sound/core/pcm_lib.c | 111 ++++++++++++++++++++++++----------------
sound/core/pcm_native.c | 2 +-
3 files changed, 71 insertions(+), 45 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 19f564606ac4..ff4a0c1c93a2 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -16,6 +16,7 @@
#include <linux/bitops.h>
#include <linux/pm_qos.h>
#include <linux/refcount.h>
+#include <linux/uio.h>

#define snd_pcm_substream_chip(substream) ((substream)->private_data)
#define snd_pcm_chip(pcm) ((pcm)->private_data)
@@ -68,6 +69,8 @@ struct snd_pcm_ops {
struct snd_pcm_audio_tstamp_report *audio_tstamp_report);
int (*fill_silence)(struct snd_pcm_substream *substream, int channel,
unsigned long pos, unsigned long bytes);
+ int (*copy)(struct snd_pcm_substream *substream, int channel,
+ unsigned long pos, struct iov_iter *iter, unsigned long bytes);
int (*copy_user)(struct snd_pcm_substream *substream, int channel,
unsigned long pos, void __user *buf,
unsigned long bytes);
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index 9c121a921b04..3303914c58ea 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -1973,10 +1973,11 @@ static int wait_for_avail(struct snd_pcm_substream *substream,

typedef int (*pcm_transfer_f)(struct snd_pcm_substream *substream,
int channel, unsigned long hwoff,
- void *buf, unsigned long bytes);
+ struct iov_iter *iter, unsigned long bytes);

typedef int (*pcm_copy_f)(struct snd_pcm_substream *, snd_pcm_uframes_t, void *,
- snd_pcm_uframes_t, snd_pcm_uframes_t, pcm_transfer_f);
+ snd_pcm_uframes_t, snd_pcm_uframes_t, pcm_transfer_f,
+ bool);

/* calculate the target DMA-buffer position to be written/read */
static void *get_dma_ptr(struct snd_pcm_runtime *runtime,
@@ -1986,32 +1987,24 @@ static void *get_dma_ptr(struct snd_pcm_runtime *runtime,
channel * (runtime->dma_bytes / runtime->channels);
}

-/* default copy_user ops for write; used for both interleaved and non- modes */
+/* default copy ops for write; used for both interleaved and non- modes */
static int default_write_copy(struct snd_pcm_substream *substream,
int channel, unsigned long hwoff,
- void *buf, unsigned long bytes)
+ struct iov_iter *iter, unsigned long bytes)
{
- if (copy_from_user(get_dma_ptr(substream->runtime, channel, hwoff),
- (void __user *)buf, bytes))
+ if (!copy_from_iter(get_dma_ptr(substream->runtime, channel, hwoff),
+ bytes, iter))
return -EFAULT;
return 0;
}

-/* default copy_kernel ops for write */
-static int default_write_copy_kernel(struct snd_pcm_substream *substream,
- int channel, unsigned long hwoff,
- void *buf, unsigned long bytes)
-{
- memcpy(get_dma_ptr(substream->runtime, channel, hwoff), buf, bytes);
- return 0;
-}
-
/* fill silence instead of copy data; called as a transfer helper
* from __snd_pcm_lib_write() or directly from noninterleaved_copy() when
* a NULL buffer is passed
*/
static int fill_silence(struct snd_pcm_substream *substream, int channel,
- unsigned long hwoff, void *buf, unsigned long bytes)
+ unsigned long hwoff, struct iov_iter *iter,
+ unsigned long bytes)
{
struct snd_pcm_runtime *runtime = substream->runtime;

@@ -2027,25 +2020,54 @@ static int fill_silence(struct snd_pcm_substream *substream, int channel,
return 0;
}

-/* default copy_user ops for read; used for both interleaved and non- modes */
+/* default copy ops for read; used for both interleaved and non- modes */
static int default_read_copy(struct snd_pcm_substream *substream,
int channel, unsigned long hwoff,
- void *buf, unsigned long bytes)
+ struct iov_iter *iter, unsigned long bytes)
{
- if (copy_to_user((void __user *)buf,
- get_dma_ptr(substream->runtime, channel, hwoff),
- bytes))
+ if (!copy_to_iter(get_dma_ptr(substream->runtime, channel, hwoff),
+ bytes, iter))
return -EFAULT;
return 0;
}

-/* default copy_kernel ops for read */
-static int default_read_copy_kernel(struct snd_pcm_substream *substream,
- int channel, unsigned long hwoff,
- void *buf, unsigned long bytes)
+/* a wrapper for calling old copy_kernel or copy_user ops */
+static int call_old_copy(struct snd_pcm_substream *substream,
+ int channel, unsigned long hwoff,
+ struct iov_iter *iter, unsigned long bytes)
{
- memcpy(buf, get_dma_ptr(substream->runtime, channel, hwoff), bytes);
- return 0;
+ if (iov_iter_is_kvec(iter))
+ return substream->ops->copy_kernel(substream, channel, hwoff,
+ iter_iov_addr(iter), bytes);
+ else
+ return substream->ops->copy_user(substream, channel, hwoff,
+ iter_iov_addr(iter), bytes);
+}
+
+/* call transfer with the filled iov_iter */
+static int do_transfer(struct snd_pcm_substream *substream, int c,
+ unsigned long hwoff, void *data, unsigned long bytes,
+ pcm_transfer_f transfer, bool in_kernel)
+{
+ struct iov_iter iter;
+ int err, type;
+
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+ type = ITER_SOURCE;
+ else
+ type = ITER_DEST;
+
+ if (in_kernel) {
+ struct kvec kvec = { data, bytes };
+
+ iov_iter_kvec(&iter, type, &kvec, 1, bytes);
+ return transfer(substream, c, hwoff, &iter, bytes);
+ }
+
+ err = import_ubuf(type, (__force void __user *)data, bytes, &iter);
+ if (err)
+ return err;
+ return transfer(substream, c, hwoff, &iter, bytes);
}

/* call transfer function with the converted pointers and sizes;
@@ -2055,7 +2077,8 @@ static int interleaved_copy(struct snd_pcm_substream *substream,
snd_pcm_uframes_t hwoff, void *data,
snd_pcm_uframes_t off,
snd_pcm_uframes_t frames,
- pcm_transfer_f transfer)
+ pcm_transfer_f transfer,
+ bool in_kernel)
{
struct snd_pcm_runtime *runtime = substream->runtime;

@@ -2063,7 +2086,9 @@ static int interleaved_copy(struct snd_pcm_substream *substream,
hwoff = frames_to_bytes(runtime, hwoff);
off = frames_to_bytes(runtime, off);
frames = frames_to_bytes(runtime, frames);
- return transfer(substream, 0, hwoff, data + off, frames);
+
+ return do_transfer(substream, 0, hwoff, data + off, frames, transfer,
+ in_kernel);
}

/* call transfer function with the converted pointers and sizes for each
@@ -2073,7 +2098,8 @@ static int noninterleaved_copy(struct snd_pcm_substream *substream,
snd_pcm_uframes_t hwoff, void *data,
snd_pcm_uframes_t off,
snd_pcm_uframes_t frames,
- pcm_transfer_f transfer)
+ pcm_transfer_f transfer,
+ bool in_kernel)
{
struct snd_pcm_runtime *runtime = substream->runtime;
int channels = runtime->channels;
@@ -2091,8 +2117,8 @@ static int noninterleaved_copy(struct snd_pcm_substream *substream,
if (!data || !*bufs)
err = fill_silence(substream, c, hwoff, NULL, frames);
else
- err = transfer(substream, c, hwoff, *bufs + off,
- frames);
+ err = do_transfer(substream, c, hwoff, *bufs + off,
+ frames, transfer, in_kernel);
if (err < 0)
return err;
}
@@ -2108,10 +2134,10 @@ static int fill_silence_frames(struct snd_pcm_substream *substream,
if (substream->runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED ||
substream->runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED)
return interleaved_copy(substream, off, NULL, 0, frames,
- fill_silence);
+ fill_silence, true);
else
return noninterleaved_copy(substream, off, NULL, 0, frames,
- fill_silence);
+ fill_silence, true);
}

/* sanity-check for read/write methods */
@@ -2121,7 +2147,7 @@ static int pcm_sanity_check(struct snd_pcm_substream *substream)
if (PCM_RUNTIME_CHECK(substream))
return -ENXIO;
runtime = substream->runtime;
- if (snd_BUG_ON(!substream->ops->copy_user && !runtime->dma_area))
+ if (snd_BUG_ON(!substream->ops->copy && !substream->ops->copy_user && !runtime->dma_area))
return -EINVAL;
if (runtime->state == SNDRV_PCM_STATE_OPEN)
return -EBADFD;
@@ -2226,15 +2252,12 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
transfer = fill_silence;
else
return -EINVAL;
- } else if (in_kernel) {
- if (substream->ops->copy_kernel)
- transfer = substream->ops->copy_kernel;
- else
- transfer = is_playback ?
- default_write_copy_kernel : default_read_copy_kernel;
} else {
- if (substream->ops->copy_user)
- transfer = (pcm_transfer_f)substream->ops->copy_user;
+ if (substream->ops->copy)
+ transfer = substream->ops->copy;
+ else if ((in_kernel && substream->ops->copy_kernel) ||
+ (!in_kernel && substream->ops->copy_user))
+ transfer = call_old_copy;
else
transfer = is_playback ?
default_write_copy : default_read_copy;
@@ -2307,7 +2330,7 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
if (!is_playback)
snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_CPU);
err = writer(substream, appl_ofs, data, offset, frames,
- transfer);
+ transfer, in_kernel);
if (is_playback)
snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_DEVICE);
snd_pcm_stream_lock_irq(substream);
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 95fc56e403b1..34efd4d198d6 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -809,7 +809,7 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
runtime->boundary *= 2;

/* clear the buffer for avoiding possible kernel info leaks */
- if (runtime->dma_area && !substream->ops->copy_user) {
+ if (runtime->dma_area && !substream->ops->copy && !substream->ops->copy_user) {
size_t size = runtime->dma_bytes;

if (runtime->info & SNDRV_PCM_INFO_MMAP)
--
2.35.3