[alsa-devel] [PATCH 0/2 v4] ALSA: pcm: anonymous dup implementation
This patchset contains the anonymous dup implementation with permissions checking for the ALSA's PCM interface in kernel to enable the restricted DMA sound buffer sharing for the restricted tasks.
The code was tested through qemu and it seems to be pretty stable.
The initial tinyalsa implementation can be found here:
https://github.com/perexg/tinyalsa/commits/anondup
v2 of the patches:
- change clone parameter to subdevice number for the pcm attach - change SNDRV_PCM_PERM_MAX to SNDRV_PCM_PERM_MASK - the tinyalsa implementation was a little updated (restructured)
v3 of the patches:
- group integer declarations in snd_pcm_anonymous_dup() - replaced substream->pcm with pcm in snd_pcm_anonymous_dup() - added SNDRV_PCM_PERM_RW check for read/write/readv/writev syscalls
v4 of the patches:
- more simple restriction control (only two modes - full/buffer) - the tinyalsa implementation follows this change
Cc: Phil Burk philburk@google.com Cc: Zach Riggle riggle@google.com Cc: Mark Brown broonie@kernel.org Cc: Leo Yan leo.yan@linaro.org Cc: Baolin Wang baolin.wang@linaro.org
Jaroslav Kysela (2): ALSA: pcm: implement the anonymous dup (inode file descriptor) ALSA: pcm: implement the mmap buffer mode for the anonymous dup
include/sound/pcm.h | 10 +++-- include/uapi/sound/asound.h | 6 ++- sound/core/oss/pcm_oss.c | 2 +- sound/core/pcm.c | 13 +++--- sound/core/pcm_compat.c | 1 + sound/core/pcm_native.c | 97 ++++++++++++++++++++++++++++++++++++++++++--- 6 files changed, 110 insertions(+), 19 deletions(-)
This patch implements new SNDRV_PCM_IOCTL_ANONYMOUS_DUP ioctl which returns the new duplicated anonymous inode file descriptor (anon_inode:snd-pcm) which can be passed to the restricted clients.
This patch is meant to be the alternative for the dma-buf interface. Both implementation have some pros and cons:
anon_inode:dmabuf
- a bit standard export API for the DMA buffers - fencing for the concurrent access [1] - driver/kernel interface for the DMA buffer [1] - multiple attach/detach scheme [1]
[1] the real usage for the sound PCM is unknown at the moment for this feature
anon_inode:snd-pcm
- simple (no problem with ref-counting, non-standard mmap implementation etc.) - allow to use more sound interfaces for the file descriptor like status ioctls - more fine grained security policies (another anon_inode name unshared with other drivers)
Signed-off-by: Jaroslav Kysela perex@perex.cz Reviewed-by: Takashi Iwai tiwai@suse.de --- include/sound/pcm.h | 8 +++--- include/uapi/sound/asound.h | 3 +- sound/core/oss/pcm_oss.c | 2 +- sound/core/pcm.c | 13 ++++----- sound/core/pcm_compat.c | 1 + sound/core/pcm_native.c | 67 +++++++++++++++++++++++++++++++++++++++++---- 6 files changed, 75 insertions(+), 19 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index ca20f80f8976..b79ffaa0241d 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -579,11 +579,11 @@ static inline int snd_pcm_suspend_all(struct snd_pcm *pcm) } #endif int snd_pcm_kernel_ioctl(struct snd_pcm_substream *substream, unsigned int cmd, void *arg); -int snd_pcm_open_substream(struct snd_pcm *pcm, int stream, struct file *file, - struct snd_pcm_substream **rsubstream); +int snd_pcm_open_substream(struct snd_pcm *pcm, int stream, int subdevice, + struct file *file, struct snd_pcm_substream **rsubstream); void snd_pcm_release_substream(struct snd_pcm_substream *substream); -int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream, struct file *file, - struct snd_pcm_substream **rsubstream); +int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream, int subdevice, + struct file *file, struct snd_pcm_substream **rsubstream); void snd_pcm_detach_substream(struct snd_pcm_substream *substream); int snd_pcm_mmap_data(struct snd_pcm_substream *substream, struct file *file, struct vm_area_struct *area);
diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index 404d4b9ffe76..ebc17d5a3490 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -153,7 +153,7 @@ struct snd_hwdep_dsp_image { * * *****************************************************************************/
-#define SNDRV_PCM_VERSION SNDRV_PROTOCOL_VERSION(2, 0, 14) +#define SNDRV_PCM_VERSION SNDRV_PROTOCOL_VERSION(2, 0, 15)
typedef unsigned long snd_pcm_uframes_t; typedef signed long snd_pcm_sframes_t; @@ -576,6 +576,7 @@ enum { #define SNDRV_PCM_IOCTL_TSTAMP _IOW('A', 0x02, int) #define SNDRV_PCM_IOCTL_TTSTAMP _IOW('A', 0x03, int) #define SNDRV_PCM_IOCTL_USER_PVERSION _IOW('A', 0x04, int) +#define SNDRV_PCM_IOCTL_ANONYMOUS_DUP _IOWR('A', 0x05, int) #define SNDRV_PCM_IOCTL_HW_REFINE _IOWR('A', 0x10, struct snd_pcm_hw_params) #define SNDRV_PCM_IOCTL_HW_PARAMS _IOWR('A', 0x11, struct snd_pcm_hw_params) #define SNDRV_PCM_IOCTL_HW_FREE _IO('A', 0x12) diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c index d5b0d7ba83c4..2ed609b65c45 100644 --- a/sound/core/oss/pcm_oss.c +++ b/sound/core/oss/pcm_oss.c @@ -2420,7 +2420,7 @@ static int snd_pcm_oss_open_file(struct file *file, if (! (f_mode & FMODE_READ)) continue; } - err = snd_pcm_open_substream(pcm, idx, file, &substream); + err = snd_pcm_open_substream(pcm, idx, -1, file, &substream); if (err < 0) { snd_pcm_oss_release_file(pcm_oss_file); return err; diff --git a/sound/core/pcm.c b/sound/core/pcm.c index 4f45b3000347..af6f7fc3687b 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -918,15 +918,14 @@ static int snd_pcm_dev_free(struct snd_device *device) return snd_pcm_free(pcm); }
-int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream, - struct file *file, +int snd_pcm_attach_substream(struct snd_pcm *pcm, + int stream, int subdevice, struct file *file, struct snd_pcm_substream **rsubstream) { struct snd_pcm_str * pstr; struct snd_pcm_substream *substream; struct snd_pcm_runtime *runtime; struct snd_card *card; - int prefer_subdevice; size_t size;
if (snd_BUG_ON(!pcm || !rsubstream)) @@ -940,7 +939,6 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream, return -ENODEV;
card = pcm->card; - prefer_subdevice = snd_ctl_get_preferred_subdevice(card, SND_CTL_SUBDEV_PCM);
if (pcm->info_flags & SNDRV_PCM_INFO_HALF_DUPLEX) { int opposite = !stream; @@ -953,14 +951,14 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream, }
if (file->f_flags & O_APPEND) { - if (prefer_subdevice < 0) { + if (subdevice < 0) { if (pstr->substream_count > 1) return -EINVAL; /* must be unique */ substream = pstr->substream; } else { for (substream = pstr->substream; substream; substream = substream->next) - if (substream->number == prefer_subdevice) + if (substream->number == subdevice) break; } if (! substream) @@ -974,8 +972,7 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
for (substream = pstr->substream; substream; substream = substream->next) { if (!SUBSTREAM_BUSY(substream) && - (prefer_subdevice == -1 || - substream->number == prefer_subdevice)) + (subdevice == -1 || substream->number == subdevice)) break; } if (substream == NULL) diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c index 946ab080ac00..22446cd574ee 100644 --- a/sound/core/pcm_compat.c +++ b/sound/core/pcm_compat.c @@ -675,6 +675,7 @@ static long snd_pcm_ioctl_compat(struct file *file, unsigned int cmd, unsigned l case SNDRV_PCM_IOCTL_TSTAMP: case SNDRV_PCM_IOCTL_TTSTAMP: case SNDRV_PCM_IOCTL_USER_PVERSION: + case SNDRV_PCM_IOCTL_ANONYMOUS_DUP: case SNDRV_PCM_IOCTL_HWSYNC: case SNDRV_PCM_IOCTL_PREPARE: case SNDRV_PCM_IOCTL_RESET: diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 0bc4aa0ac9cf..bb14658e4482 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -37,6 +37,8 @@ #include <sound/minors.h> #include <linux/uio.h> #include <linux/delay.h> +#include <linux/anon_inodes.h> +#include <linux/syscalls.h>
#include "pcm_local.h"
@@ -2437,14 +2439,17 @@ void snd_pcm_release_substream(struct snd_pcm_substream *substream) } EXPORT_SYMBOL(snd_pcm_release_substream);
-int snd_pcm_open_substream(struct snd_pcm *pcm, int stream, +int snd_pcm_open_substream(struct snd_pcm *pcm, int stream, int subdevice, struct file *file, struct snd_pcm_substream **rsubstream) { struct snd_pcm_substream *substream; int err;
- err = snd_pcm_attach_substream(pcm, stream, file, &substream); + if (subdevice < 0 && pcm) + subdevice = snd_ctl_get_preferred_subdevice(pcm->card, SND_CTL_SUBDEV_PCM); + + err = snd_pcm_attach_substream(pcm, stream, subdevice, file, &substream); if (err < 0) return err; if (substream->ref_count > 1) { @@ -2480,13 +2485,14 @@ EXPORT_SYMBOL(snd_pcm_open_substream);
static int snd_pcm_open_file(struct file *file, struct snd_pcm *pcm, - int stream) + int stream, + int subdevice) { struct snd_pcm_file *pcm_file; struct snd_pcm_substream *substream; int err;
- err = snd_pcm_open_substream(pcm, stream, file, &substream); + err = snd_pcm_open_substream(pcm, stream, subdevice, file, &substream); if (err < 0) return err;
@@ -2551,7 +2557,7 @@ static int snd_pcm_open(struct file *file, struct snd_pcm *pcm, int stream) add_wait_queue(&pcm->open_wait, &wait); mutex_lock(&pcm->open_mutex); while (1) { - err = snd_pcm_open_file(file, pcm, stream); + err = snd_pcm_open_file(file, pcm, stream, -1); if (err >= 0) break; if (err == -EAGAIN) { @@ -2595,6 +2601,9 @@ static int snd_pcm_release(struct inode *inode, struct file *file) struct snd_pcm_file *pcm_file;
pcm_file = file->private_data; + /* a problem in the anonymous dup can hit the NULL pcm_file */ + if (pcm_file == NULL) + return 0; substream = pcm_file->substream; if (snd_BUG_ON(!substream)) return -ENXIO; @@ -2878,6 +2887,52 @@ static int snd_pcm_forward_ioctl(struct snd_pcm_substream *substream, return result < 0 ? result : 0; }
+static int snd_pcm_anonymous_dup(struct file *file, + struct snd_pcm_substream *substream, + int __user *arg) +{ + int fd, err, perm, flags; + struct file *nfile; + struct snd_pcm *pcm = substream->pcm; + + if (get_user(perm, arg)) + return -EFAULT; + if (perm < 0) + return -EPERM; + flags = file->f_flags & (O_ACCMODE | O_NONBLOCK); + flags |= O_APPEND | O_CLOEXEC; + fd = get_unused_fd_flags(flags); + if (fd < 0) + return fd; + nfile = anon_inode_getfile("snd-pcm", file->f_op, NULL, flags); + if (IS_ERR(nfile)) { + put_unused_fd(fd); + return PTR_ERR(nfile); + } + /* anon_inode_getfile() filters the O_APPEND flag out */ + nfile->f_flags |= O_APPEND; + fd_install(fd, nfile); + if (!try_module_get(pcm->card->module)) { + err = -EFAULT; + goto __error1; + } + err = snd_card_file_add(pcm->card, nfile); + if (err < 0) + goto __error2; + err = snd_pcm_open_file(nfile, substream->pcm, + substream->stream, substream->number); + if (err >= 0) { + put_user(fd, arg); + return 0; + } + snd_card_file_remove(pcm->card, nfile); + __error2: + module_put(pcm->card->module); + __error1: + ksys_close(fd); + return err; +} + static int snd_pcm_common_ioctl(struct file *file, struct snd_pcm_substream *substream, unsigned int cmd, void __user *arg) @@ -2906,6 +2961,8 @@ static int snd_pcm_common_ioctl(struct file *file, (unsigned int __user *)arg)) return -EFAULT; return 0; + case SNDRV_PCM_IOCTL_ANONYMOUS_DUP: + return snd_pcm_anonymous_dup(file, substream, (int __user *)arg); case SNDRV_PCM_IOCTL_HW_REFINE: return snd_pcm_hw_refine_user(substream, arg); case SNDRV_PCM_IOCTL_HW_PARAMS:
Android requires to allow the passing an anonymous inode file descriptor with the restricted functionality - only the mmap operation for the DMA sound buffer (anon_inode:snd-pcm-buffer). Android uses this access mode for the direct EXCLUSIVE sound device access by applications.
This requirement is for the proper SELinux audit.
Signed-off-by: Jaroslav Kysela perex@perex.cz --- include/sound/pcm.h | 2 ++ include/uapi/sound/asound.h | 3 +++ sound/core/pcm_native.c | 36 +++++++++++++++++++++++++++++++++--- 3 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index b79ffaa0241d..55b95476d15e 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -227,6 +227,7 @@ struct snd_pcm_ops { struct snd_pcm_file { struct snd_pcm_substream *substream; int no_compat_mmap; + unsigned int perm; /* file descriptor permissions */ unsigned int user_pversion; /* supported protocol version */ };
@@ -537,6 +538,7 @@ struct snd_pcm { * Registering */
+extern const struct file_operations snd_pcm_f_op_buffer; extern const struct file_operations snd_pcm_f_ops[2];
int snd_pcm_new(struct snd_card *card, const char *id, int device, diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index ebc17d5a3490..b0270d07cf4e 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -571,6 +571,9 @@ enum { #define SNDRV_CHMAP_PHASE_INVERSE (0x01 << 16) #define SNDRV_CHMAP_DRIVER_SPEC (0x02 << 16)
+#define SNDRV_PCM_PERM_MODE_FULL 0 /* full access - no restrictions */ +#define SNDRV_PCM_PERM_MODE_BUFFER 1 /* allow to export only sound buffer through mmap */ + #define SNDRV_PCM_IOCTL_PVERSION _IOR('A', 0x00, int) #define SNDRV_PCM_IOCTL_INFO _IOR('A', 0x01, struct snd_pcm_info) #define SNDRV_PCM_IOCTL_TSTAMP _IOW('A', 0x02, int) diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index bb14658e4482..7a21a2335a8d 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2502,6 +2502,7 @@ static int snd_pcm_open_file(struct file *file, return -ENOMEM; } pcm_file->substream = substream; + pcm_file->perm = 0; if (substream->ref_count == 1) substream->pcm_release = pcm_release_private; file->private_data = pcm_file; @@ -2894,17 +2895,30 @@ static int snd_pcm_anonymous_dup(struct file *file, int fd, err, perm, flags; struct file *nfile; struct snd_pcm *pcm = substream->pcm; + struct snd_pcm_file *pcm_file; + const char *aname; + const struct file_operations *f_op;
if (get_user(perm, arg)) return -EFAULT; - if (perm < 0) - return -EPERM; + switch (perm) { + case SNDRV_PCM_PERM_MODE_FULL: + aname = "snd-pcm"; + f_op = file->f_op; + break; + case SNDRV_PCM_PERM_MODE_BUFFER: + aname = "snd-pcm-buf"; + f_op = &snd_pcm_f_op_buffer; + break; + default: + return -EINVAL; + } flags = file->f_flags & (O_ACCMODE | O_NONBLOCK); flags |= O_APPEND | O_CLOEXEC; fd = get_unused_fd_flags(flags); if (fd < 0) return fd; - nfile = anon_inode_getfile("snd-pcm", file->f_op, NULL, flags); + nfile = anon_inode_getfile(aname, f_op, NULL, flags); if (IS_ERR(nfile)) { put_unused_fd(fd); return PTR_ERR(nfile); @@ -2922,6 +2936,8 @@ static int snd_pcm_anonymous_dup(struct file *file, err = snd_pcm_open_file(nfile, substream->pcm, substream->stream, substream->number); if (err >= 0) { + pcm_file = nfile->private_data; + pcm_file->perm = perm; put_user(fd, arg); return 0; } @@ -3295,6 +3311,9 @@ static int snd_pcm_mmap_status(struct snd_pcm_substream *substream, struct file struct vm_area_struct *area) { long size; + struct snd_pcm_file *pcm_file = file->private_data; + if (pcm_file->perm != SNDRV_PCM_PERM_MODE_FULL) + return -EPERM; if (!(area->vm_flags & VM_READ)) return -EINVAL; size = area->vm_end - area->vm_start; @@ -3331,6 +3350,9 @@ static int snd_pcm_mmap_control(struct snd_pcm_substream *substream, struct file struct vm_area_struct *area) { long size; + struct snd_pcm_file *pcm_file = file->private_data; + if (pcm_file->perm != SNDRV_PCM_PERM_MODE_FULL) + return -EPERM; if (!(area->vm_flags & VM_READ)) return -EINVAL; size = area->vm_end - area->vm_start; @@ -3738,6 +3760,14 @@ static unsigned long snd_pcm_get_unmapped_area(struct file *file, * Register section */
+const struct file_operations snd_pcm_f_op_buffer = { + .owner = THIS_MODULE, + .release = snd_pcm_release, + .llseek = no_llseek, + .mmap = snd_pcm_mmap, + .get_unmapped_area = snd_pcm_get_unmapped_area +}; + const struct file_operations snd_pcm_f_ops[2] = { { .owner = THIS_MODULE,
On Mon, 04 Feb 2019 10:39:10 +0100, Jaroslav Kysela wrote:
@@ -3295,6 +3311,9 @@ static int snd_pcm_mmap_status(struct snd_pcm_substream *substream, struct file struct vm_area_struct *area) { long size;
- struct snd_pcm_file *pcm_file = file->private_data;
- if (pcm_file->perm != SNDRV_PCM_PERM_MODE_FULL)
return -EPERM;
This check can be better put to pcm_status_mmap_allowed().
@@ -3331,6 +3350,9 @@ static int snd_pcm_mmap_control(struct snd_pcm_substream *substream, struct file struct vm_area_struct *area) { long size;
- struct snd_pcm_file *pcm_file = file->private_data;
- if (pcm_file->perm != SNDRV_PCM_PERM_MODE_FULL)
return -EPERM;
Ditto, put to pcm_control_mmap_allowed().
Other than that, looks good to me: Reviewed-by: Takashi Iwai tiwai@suse.de
thanks,
Takashi
Thanks for these patches for tinyalsa. Looks great!
I made a few minor comments on the code layout. https://github.com/perexg/tinyalsa/commit/f62b953f37693e8426ee4c20e53baae757... Can you see them?
Phil Burk
On Mon, Feb 4, 2019 at 1:39 AM Jaroslav Kysela perex@perex.cz wrote:
This patchset contains the anonymous dup implementation with permissions checking for the ALSA's PCM interface in kernel to enable the restricted DMA sound buffer sharing for the restricted tasks.
The code was tested through qemu and it seems to be pretty stable.
The initial tinyalsa implementation can be found here:
https://github.com/perexg/tinyalsa/commits/anondup
v2 of the patches:
- change clone parameter to subdevice number for the pcm attach
- change SNDRV_PCM_PERM_MAX to SNDRV_PCM_PERM_MASK
- the tinyalsa implementation was a little updated (restructured)
v3 of the patches:
- group integer declarations in snd_pcm_anonymous_dup()
- replaced substream->pcm with pcm in snd_pcm_anonymous_dup()
- added SNDRV_PCM_PERM_RW check for read/write/readv/writev syscalls
v4 of the patches:
- more simple restriction control (only two modes - full/buffer)
- the tinyalsa implementation follows this change
Cc: Phil Burk philburk@google.com Cc: Zach Riggle riggle@google.com Cc: Mark Brown broonie@kernel.org Cc: Leo Yan leo.yan@linaro.org Cc: Baolin Wang baolin.wang@linaro.org
Jaroslav Kysela (2): ALSA: pcm: implement the anonymous dup (inode file descriptor) ALSA: pcm: implement the mmap buffer mode for the anonymous dup
include/sound/pcm.h | 10 +++-- include/uapi/sound/asound.h | 6 ++- sound/core/oss/pcm_oss.c | 2 +- sound/core/pcm.c | 13 +++--- sound/core/pcm_compat.c | 1 + sound/core/pcm_native.c | 97 ++++++++++++++++++++++++++++++++++++++++++--- 6 files changed, 110 insertions(+), 19 deletions(-)
-- 2.13.6
Dne 7.2.2019 v 18:41 Phil Burk napsal(a):
Thanks for these patches for tinyalsa. Looks great!
I made a few minor comments on the code layout. https://github.com/perexg/tinyalsa/commit/f62b953f37693e8426ee4c20e53baae757... Can you see them?
Yes, I updated the anondup branch with the suggested changes (except the else comment which I don't understand - it's just copy-n-paste of the origin).
Thanks, Jaroslav
(except the else comment which I don't understand - it's just
copy-n-paste of the origin).
OK. No problem. I see that was already there.
Thanks for fixing the indentation change.
Phil Burk
On Thu, Feb 7, 2019 at 9:53 AM Jaroslav Kysela perex@perex.cz wrote:
Dne 7.2.2019 v 18:41 Phil Burk napsal(a):
Thanks for these patches for tinyalsa. Looks great!
I made a few minor comments on the code layout.
https://github.com/perexg/tinyalsa/commit/f62b953f37693e8426ee4c20e53baae757...
Can you see them?
Yes, I updated the anondup branch with the suggested changes (except the else comment which I don't understand - it's just copy-n-paste of the origin).
Thanks, Jaroslav-- Jaroslav Kysela perex@perex.cz Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
On Mon, Feb 04, 2019 at 10:39:08AM +0100, Jaroslav Kysela wrote:
This patchset contains the anonymous dup implementation with permissions checking for the ALSA's PCM interface in kernel to enable the restricted DMA sound buffer sharing for the restricted tasks.
Is there any news on merging this during the current development cycle, or anything else needed to move this forwards?
On Tue, 26 Mar 2019 15:09:28 +0100, Mark Brown wrote:
On Mon, Feb 04, 2019 at 10:39:08AM +0100, Jaroslav Kysela wrote:
This patchset contains the anonymous dup implementation with permissions checking for the ALSA's PCM interface in kernel to enable the restricted DMA sound buffer sharing for the restricted tasks.
Is there any news on merging this during the current development cycle, or anything else needed to move this forwards?
I've seen no reaction whether the patch really tested, worked or helped in the real scenario...
Takashi
Hello,
Thanks for keeping this moving forward.
Any suggestions for testing this? We can try to test it here in Android. It may be tricky because your patches may conflict with some proprietary Qualcomm changes in the kernel we are currently using.
Can I get a summary list of the patches required for kernel and tinyalsa?
Thanks, Phil Burk
On Tue, Mar 26, 2019 at 7:13 AM Takashi Iwai tiwai@suse.de wrote:
On Tue, 26 Mar 2019 15:09:28 +0100, Mark Brown wrote:
On Mon, Feb 04, 2019 at 10:39:08AM +0100, Jaroslav Kysela wrote:
This patchset contains the anonymous dup implementation with
permissions
checking for the ALSA's PCM interface in kernel to enable the
restricted
DMA sound buffer sharing for the restricted tasks.
Is there any news on merging this during the current development cycle, or anything else needed to move this forwards?
I've seen no reaction whether the patch really tested, worked or helped in the real scenario...
Takashi
Dne 26. 03. 19 v 15:27 Phil Burk napsal(a):
Hello,
Thanks for keeping this moving forward.
Any suggestions for testing this? We can try to test it here in Android. It may be tricky because your patches may conflict with some proprietary Qualcomm changes in the kernel we are currently using.
Can I get a summary list of the patches required for kernel and tinyalsa?
The v6 is the last version, the full information is in the cover letter:
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145127.h... https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145128.h... https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145129.h... https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145130.h...
Jaroslav
Thanks, Phil Burk
On Tue, Mar 26, 2019 at 7:13 AM Takashi Iwai <tiwai@suse.de mailto:tiwai@suse.de> wrote:
On Tue, 26 Mar 2019 15:09:28 +0100, Mark Brown wrote: > > On Mon, Feb 04, 2019 at 10:39:08AM +0100, Jaroslav Kysela wrote: > > This patchset contains the anonymous dup implementation with permissions > > checking for the ALSA's PCM interface in kernel to enable the restricted > > DMA sound buffer sharing for the restricted tasks. > > Is there any news on merging this during the current development cycle, > or anything else needed to move this forwards? I've seen no reaction whether the patch really tested, worked or helped in the real scenario... Takashi
On Tue, 26 Mar 2019 15:41:11 +0100, Jaroslav Kysela wrote:
Dne 26. 03. 19 v 15:27 Phil Burk napsal(a):
Hello,
Thanks for keeping this moving forward.
Any suggestions for testing this? We can try to test it here in Android. It may be tricky because your patches may conflict with some proprietary Qualcomm changes in the kernel we are currently using.
Can I get a summary list of the patches required for kernel and tinyalsa?
The v6 is the last version, the full information is in the cover letter:
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145127.h... https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145128.h... https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145129.h... https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145130.h...
Is there any test result from the practical use cases?
Basically now is the last chance for merging such an intrusive change for 5.2 kernel. If we miss it, it'll be postponed at least for 5.3, i.e. more three months.
thanks,
Takashi
JaroslavThanks, Phil Burk
On Tue, Mar 26, 2019 at 7:13 AM Takashi Iwai <tiwai@suse.de mailto:tiwai@suse.de> wrote:
On Tue, 26 Mar 2019 15:09:28 +0100, Mark Brown wrote: > > On Mon, Feb 04, 2019 at 10:39:08AM +0100, Jaroslav Kysela wrote: > > This patchset contains the anonymous dup implementation with permissions > > checking for the ALSA's PCM interface in kernel to enable the restricted > > DMA sound buffer sharing for the restricted tasks. > > Is there any news on merging this during the current development cycle, > or anything else needed to move this forwards? I've seen no reaction whether the patch really tested, worked or helped in the real scenario... Takashi-- Jaroslav Kysela perex@perex.cz Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
Hello Takashi,
Sorry for the late reply. I got pulled off on some other projects.
We will try to test this in-house but we will need Qualcomm's help. I will also try to get some of our SOC partners to help with testing.
If we miss it, it'll be postponed at least for 5.3, i.e. more three months.
I think it will be difficult to complete the testing before that 5.2 deadline. We should probably shoot for 5.3. If someone needs it sooner that they will have the patches.
Thanks again for all this work.
I assume the four emails below have all the latest patches needed for rev 6.
Thanks, Phil Burk
On Tue, Apr 23, 2019 at 11:08 AM Takashi Iwai tiwai@suse.de wrote:
On Tue, 26 Mar 2019 15:41:11 +0100, Jaroslav Kysela wrote:
Dne 26. 03. 19 v 15:27 Phil Burk napsal(a):
Hello,
Thanks for keeping this moving forward.
Any suggestions for testing this? We can try to test it here in
Android. It may be tricky because your patches may conflict with some proprietary Qualcomm changes in the kernel we are currently using.
Can I get a summary list of the patches required for kernel and
tinyalsa?
The v6 is the last version, the full information is in the cover letter:
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145127.h...
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145128.h...
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145129.h...
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145130.h...
Is there any test result from the practical use cases?
Basically now is the last chance for merging such an intrusive change for 5.2 kernel. If we miss it, it'll be postponed at least for 5.3, i.e. more three months.
thanks,
Takashi
JaroslavThanks, Phil Burk
On Tue, Mar 26, 2019 at 7:13 AM Takashi Iwai <tiwai@suse.de <mailto:
tiwai@suse.de>> wrote:
On Tue, 26 Mar 2019 15:09:28 +0100, Mark Brown wrote: > > On Mon, Feb 04, 2019 at 10:39:08AM +0100, Jaroslav Kysela wrote: > > This patchset contains the anonymous dup implementation withpermissions
> > checking for the ALSA's PCM interface in kernel to enable therestricted
> > DMA sound buffer sharing for the restricted tasks. > > Is there any news on merging this during the current developmentcycle,
> or anything else needed to move this forwards? I've seen no reaction whether the patch really tested, worked or helped in the real scenario... Takashi-- Jaroslav Kysela perex@perex.cz Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
On Tue, 23 Apr 2019 22:11:50 +0200, Phil Burk wrote:
Hello Takashi,
Sorry for the late reply. I got pulled off on some other projects.
We will try to test this in-house but we will need Qualcomm's help. I will also try to get some of our SOC partners to help with testing.
OK.
If we miss it, it'll be postponed at least for 5.3, i.e. more three months.
I think it will be difficult to complete the testing before that 5.2 deadline. We should probably shoot for 5.3. If someone needs it sooner that they will have the patches.
Fair enough, let's postpone the merge, then.
Thanks again for all this work.
I assume the four emails below have all the latest patches needed for rev 6.
I believe yes.
Thanks for the quick update!
Takashi
Thanks, Phil Burk
On Tue, Apr 23, 2019 at 11:08 AM Takashi Iwai tiwai@suse.de wrote:
On Tue, 26 Mar 2019 15:41:11 +0100, Jaroslav Kysela wrote: > > Dne 26. 03. 19 v 15:27 Phil Burk napsal(a): > > Hello, > > > > Thanks for keeping this moving forward. > > > > Any suggestions for testing this? We can try to test it here in Android. It may be tricky because your patches may conflict with some proprietary Qualcomm changes in the kernel we are currently using. > > > > Can I get a summary list of the patches required for kernel and tinyalsa? > > The v6 is the last version, the full information is in the cover letter: > > https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145127.html > https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145128.html > https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145129.html > https://mailman.alsa-project.org/pipermail/alsa-devel/2019-February/145130.html Is there any test result from the practical use cases? Basically now is the last chance for merging such an intrusive change for 5.2 kernel. If we miss it, it'll be postponed at least for 5.3, i.e. more three months. thanks, Takashi > > Jaroslav > > > > > Thanks, > > Phil Burk > > > > > > On Tue, Mar 26, 2019 at 7:13 AM Takashi Iwai <tiwai@suse.de <mailto: tiwai@suse.de>> wrote: > > > > On Tue, 26 Mar 2019 15:09:28 +0100, > > Mark Brown wrote: > > > > > > On Mon, Feb 04, 2019 at 10:39:08AM +0100, Jaroslav Kysela wrote: > > > > This patchset contains the anonymous dup implementation with permissions > > > > checking for the ALSA's PCM interface in kernel to enable the restricted > > > > DMA sound buffer sharing for the restricted tasks. > > > > > > Is there any news on merging this during the current development cycle, > > > or anything else needed to move this forwards? > > > > I've seen no reaction whether the patch really tested, worked or > > helped in the real scenario... > > > > > > Takashi > > > > > -- > Jaroslav Kysela <perex@perex.cz> > Linux Sound Maintainer; ALSA Project; Red Hat, Inc. >
On Tue, Apr 23, 2019 at 01:11:50PM -0700, Phil Burk wrote:
Hello Takashi,
Sorry for the late reply. I got pulled off on some other projects.
We will try to test this in-house but we will need Qualcomm's help. I will also try to get some of our SOC partners to help with testing.
Did anything ever happen with this testing? These anonymous mmap patches never got merged.
Hello Mark,
Thank you for keeping this moving forward.
We sent the patches out to several SOC vendors for Android last year. They thanked us and said they would send feedback but never did. I pinged them again.
If we cannot get the changes tested by partners then I will try to get them tested internally.
For reference, this is being tracked internally at b/119712034
Thanks, Phil Burk
On Mon, Jun 8, 2020 at 6:45 AM Mark Brown broonie@kernel.org wrote:
On Tue, Apr 23, 2019 at 01:11:50PM -0700, Phil Burk wrote:
Hello Takashi,
Sorry for the late reply. I got pulled off on some other projects.
We will try to test this in-house but we will need Qualcomm's help. I will also try to get some of our SOC partners to help with testing.
Did anything ever happen with this testing? These anonymous mmap patches never got merged.
Hello Mark,
I wrote to Peter Huang at UniSOC
Did you ever get a chance to try the Linaro patches? Did they work for
you?
Peter wrote:
we have tried these patches, and they works. [snip]
And Baolin also suggests that his patch is too complicated and difficult
to maintain, and a better solution is list as below, this new patch I have not tried.
[1]:
https://lore.kernel.org/patchwork/patch/1033206/
[2]:
https://www.alsa-project.org/pipermail/alsa-devel/2019-January/144925.html
https://www.alsa-project.org/pipermail/alsa-devel/2019-January/144924.html
It is hard for me to extract the relevant patches from those emails.
I would love to finish this project but I am not sure how. It seems we need to:
1) Evaluate the patches that Baolin suggests as a simpler alternative.
2) Test them in an Android kernel with AAudio MMAP.
If you can provide a clear description of the latest set of patches then maybe I can work with someone in-house to test this.
I am open to suggestions.
Phil Burk
On Mon, Jun 8, 2020 at 10:49 AM Phil Burk philburk@google.com wrote:
Hello Mark,
Thank you for keeping this moving forward.
We sent the patches out to several SOC vendors for Android last year. They thanked us and said they would send feedback but never did. I pinged them again.
If we cannot get the changes tested by partners then I will try to get them tested internally.
For reference, this is being tracked internally at b/119712034 https://buganizer.corp.google.com/119712034
Thanks, Phil Burk
On Mon, Jun 8, 2020 at 6:45 AM Mark Brown broonie@kernel.org wrote:
On Tue, Apr 23, 2019 at 01:11:50PM -0700, Phil Burk wrote:
Hello Takashi,
Sorry for the late reply. I got pulled off on some other projects.
We will try to test this in-house but we will need Qualcomm's help. I will also try to get some of our SOC partners to help with testing.
Did anything ever happen with this testing? These anonymous mmap patches never got merged.
On Wed, Jun 10, 2020 at 04:10:15PM -0700, Phil Burk wrote:
https://www.alsa-project.org/pipermail/alsa-devel/2019-January/144925.html https://www.alsa-project.org/pipermail/alsa-devel/2019-January/144924.html
It is hard for me to extract the relevant patches from those emails.
I've attached them in mbox format (b4 is a great tool for extracting stuff from the archives on lore.kernel.org BTW:
). They seem to apply cleanly with git am -3 for me so should be easy enough to apply to your tree hopefully.
I would love to finish this project but I am not sure how. It seems we need to:
- Evaluate the patches that Baolin suggests as a simpler alternative.
- Test them in an Android kernel with AAudio MMAP.
If you can provide a clear description of the latest set of patches then maybe I can work with someone in-house to test this.
I agree that that's the best approach - hopefully the mbox helps with getting the patches onto a relevant tree, let me know if you still have trouble.
For reference it's the patches from Jaroslav in this thread we're on here, which can also be found here:
https://lore.kernel.org/alsa-devel/20190204093910.23878-1-perex@perex.cz/
participants (4)
-
Jaroslav Kysela -
Mark Brown -
Phil Burk -
Takashi Iwai