linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
@ 2017-08-07 12:22 Oleksandr Andrushchenko
  2017-08-07 12:22 ` [PATCH RESEND1 01/12] ALSA: vsnd: Introduce Xen para-virtualized sound " Oleksandr Andrushchenko
                   ` (12 more replies)
  0 siblings, 13 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-07 12:22 UTC (permalink / raw)
  To: alsa-devel, xen-devel, linux-kernel
  Cc: perex, tiwai, andr2000, Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

This patch series adds support for Xen [1] para-virtualized
sound frontend driver. It implements the protocol from
include/xen/interface/io/sndif.h with the following limitations:
- mute/unmute is not supported
- get/set volume is not supported
Volume control is not supported for the reason that most of the
use-cases (at the moment) are based on scenarious where
unprivileged OS (e.g. Android, AGL etc) use software mixers.

Both capture and playback are supported.

Thank you,
Oleksandr

Resending because of rebase onto [2] + added missing patch

[1] https://xenproject.org/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/log/?h=for-next

Oleksandr Andrushchenko (12):
  ALSA: vsnd: Introduce Xen para-virtualized sound frontend driver
  ALSA: vsnd: Implement driver's probe/remove
  ALSA: vsnd: Implement Xen bus state handling
  ALSA: vsnd: Read sound driver configuration from Xen store
  ALSA: vsnd: Implement Xen event channel handling
  ALSA: vsnd: Implement handling of shared buffers
  ALSA: vsnd: Introduce ALSA virtual sound driver
  ALSA: vsnd: Initialize virtul sound card
  ALSA: vsnd: Add timer for period interrupt emulation
  ALSA: vsnd: Implement ALSA PCM operations
  ALSA: vsnd: Implement communication with backend
  ALSA: vsnd: Introduce Kconfig option to enable Xen PV sound

 sound/drivers/Kconfig     |   12 +
 sound/drivers/Makefile    |    2 +
 sound/drivers/xen-front.c | 2107 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 2121 insertions(+)
 create mode 100644 sound/drivers/xen-front.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH RESEND1 01/12] ALSA: vsnd: Introduce Xen para-virtualized sound frontend driver
  2017-08-07 12:22 [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Oleksandr Andrushchenko
@ 2017-08-07 12:22 ` Oleksandr Andrushchenko
  2017-08-07 12:22 ` [PATCH RESEND1 02/12] ALSA: vsnd: Implement driver's probe/remove Oleksandr Andrushchenko
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-07 12:22 UTC (permalink / raw)
  To: alsa-devel, xen-devel, linux-kernel
  Cc: perex, tiwai, andr2000, Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Introduce skeleton of the para-virtualized Xen sound
frontend driver. This patch only adds required
essential stubs.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
 sound/drivers/xen-front.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)
 create mode 100644 sound/drivers/xen-front.c

diff --git a/sound/drivers/xen-front.c b/sound/drivers/xen-front.c
new file mode 100644
index 000000000000..61779c36cae3
--- /dev/null
+++ b/sound/drivers/xen-front.c
@@ -0,0 +1,78 @@
+/*
+ * Xen para-virtual sound device
+ *
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation; either version 2 of the License, or
+ *   (at your option) any later version.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ *
+ * Based on: sound/drivers/dummy.c
+ *
+ * Copyright (C) 2016-2017 EPAM Systems Inc.
+ *
+ * Author: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
+ */
+
+#include <linux/module.h>
+
+#include <xen/platform_pci.h>
+#include <xen/xen.h>
+#include <xen/xenbus.h>
+
+#include <xen/interface/io/sndif.h>
+
+static void xdrv_be_on_changed(struct xenbus_device *xb_dev,
+	enum xenbus_state backend_state)
+{
+}
+
+static int xdrv_probe(struct xenbus_device *xb_dev,
+	const struct xenbus_device_id *id)
+{
+	return 0;
+}
+
+static int xdrv_remove(struct xenbus_device *dev)
+{
+	return 0;
+}
+
+static const struct xenbus_device_id xdrv_ids[] = {
+	{ XENSND_DRIVER_NAME },
+	{ "" }
+};
+
+static struct xenbus_driver xen_driver = {
+	.ids = xdrv_ids,
+	.probe = xdrv_probe,
+	.remove = xdrv_remove,
+	.otherend_changed = xdrv_be_on_changed,
+};
+
+static int __init xdrv_init(void)
+{
+	if (!xen_domain())
+		return -ENODEV;
+
+	pr_info("Initialising Xen " XENSND_DRIVER_NAME " frontend driver\n");
+	return xenbus_register_frontend(&xen_driver);
+}
+
+static void __exit xdrv_cleanup(void)
+{
+	pr_info("Unregistering Xen " XENSND_DRIVER_NAME " frontend driver\n");
+	xenbus_unregister_driver(&xen_driver);
+}
+
+module_init(xdrv_init);
+module_exit(xdrv_cleanup);
+
+MODULE_DESCRIPTION("Xen virtual sound device frontend");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("xen:"XENSND_DRIVER_NAME);
+MODULE_SUPPORTED_DEVICE("{{ALSA,Virtual soundcard}}");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH RESEND1 02/12] ALSA: vsnd: Implement driver's probe/remove
  2017-08-07 12:22 [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Oleksandr Andrushchenko
  2017-08-07 12:22 ` [PATCH RESEND1 01/12] ALSA: vsnd: Introduce Xen para-virtualized sound " Oleksandr Andrushchenko
@ 2017-08-07 12:22 ` Oleksandr Andrushchenko
  2017-08-07 12:22 ` [PATCH RESEND1 03/12] ALSA: vsnd: Implement Xen bus state handling Oleksandr Andrushchenko
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-07 12:22 UTC (permalink / raw)
  To: alsa-devel, xen-devel, linux-kernel
  Cc: perex, tiwai, andr2000, Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Add essential driver private info structure, initialize
locks and implement probe/remove of the Xen frontend
driver.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
 sound/drivers/xen-front.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/sound/drivers/xen-front.c b/sound/drivers/xen-front.c
index 61779c36cae3..8c5de7b0e7b5 100644
--- a/sound/drivers/xen-front.c
+++ b/sound/drivers/xen-front.c
@@ -26,6 +26,16 @@
 
 #include <xen/interface/io/sndif.h>
 
+struct xdrv_info {
+	struct xenbus_device *xb_dev;
+	spinlock_t io_lock;
+	struct mutex mutex;
+};
+
+static void xdrv_remove_internal(struct xdrv_info *drv_info)
+{
+}
+
 static void xdrv_be_on_changed(struct xenbus_device *xb_dev,
 	enum xenbus_state backend_state)
 {
@@ -34,11 +44,28 @@ static void xdrv_be_on_changed(struct xenbus_device *xb_dev,
 static int xdrv_probe(struct xenbus_device *xb_dev,
 	const struct xenbus_device_id *id)
 {
+	struct xdrv_info *drv_info;
+
+	drv_info = devm_kzalloc(&xb_dev->dev, sizeof(*drv_info), GFP_KERNEL);
+	if (!drv_info) {
+		xenbus_dev_fatal(xb_dev, -ENOMEM, "allocating device memory");
+		return -ENOMEM;
+	}
+
+	drv_info->xb_dev = xb_dev;
+	spin_lock_init(&drv_info->io_lock);
+	mutex_init(&drv_info->mutex);
+	dev_set_drvdata(&xb_dev->dev, drv_info);
 	return 0;
 }
 
 static int xdrv_remove(struct xenbus_device *dev)
 {
+	struct xdrv_info *drv_info = dev_get_drvdata(&dev->dev);
+
+	mutex_lock(&drv_info->mutex);
+	xdrv_remove_internal(drv_info);
+	mutex_unlock(&drv_info->mutex);
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH RESEND1 03/12] ALSA: vsnd: Implement Xen bus state handling
  2017-08-07 12:22 [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Oleksandr Andrushchenko
  2017-08-07 12:22 ` [PATCH RESEND1 01/12] ALSA: vsnd: Introduce Xen para-virtualized sound " Oleksandr Andrushchenko
  2017-08-07 12:22 ` [PATCH RESEND1 02/12] ALSA: vsnd: Implement driver's probe/remove Oleksandr Andrushchenko
@ 2017-08-07 12:22 ` Oleksandr Andrushchenko
  2017-08-07 12:22 ` [PATCH RESEND1 04/12] ALSA: vsnd: Read sound driver configuration from Xen store Oleksandr Andrushchenko
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-07 12:22 UTC (permalink / raw)
  To: alsa-devel, xen-devel, linux-kernel
  Cc: perex, tiwai, andr2000, Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Initial handling for Xen bus states: implement
Xen bus state machine for the front driver according to
the state diagram and recovery flow from sound para-virtualized
protocol: xen/interface/io/sndif.h.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
 sound/drivers/xen-front.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/sound/drivers/xen-front.c b/sound/drivers/xen-front.c
index 8c5de7b0e7b5..c4fd21cac3a7 100644
--- a/sound/drivers/xen-front.c
+++ b/sound/drivers/xen-front.c
@@ -36,9 +36,99 @@ static void xdrv_remove_internal(struct xdrv_info *drv_info)
 {
 }
 
+static int xdrv_be_on_initwait(struct xdrv_info *drv_info)
+{
+	return 0;
+}
+
+static inline int xdrv_be_on_connected(struct xdrv_info *drv_info)
+{
+	return 0;
+}
+
+static inline void xdrv_be_on_disconnected(struct xdrv_info *drv_info)
+{
+	xdrv_remove_internal(drv_info);
+}
+
 static void xdrv_be_on_changed(struct xenbus_device *xb_dev,
 	enum xenbus_state backend_state)
 {
+	struct xdrv_info *drv_info = dev_get_drvdata(&xb_dev->dev);
+	int ret;
+
+	switch (backend_state) {
+	case XenbusStateReconfiguring:
+		/* fall through */
+	case XenbusStateReconfigured:
+		/* fall through */
+	case XenbusStateInitialised:
+		/* fall through */
+		break;
+
+	case XenbusStateInitialising:
+		if (xb_dev->state == XenbusStateInitialising)
+			break;
+
+		/* recovering after backend unexpected closure */
+		mutex_lock(&drv_info->mutex);
+		xdrv_be_on_disconnected(drv_info);
+		mutex_unlock(&drv_info->mutex);
+		xenbus_switch_state(xb_dev, XenbusStateInitialising);
+		break;
+
+	case XenbusStateInitWait:
+		if (xb_dev->state != XenbusStateInitialising)
+			break;
+
+		mutex_lock(&drv_info->mutex);
+		ret = xdrv_be_on_initwait(drv_info);
+		mutex_unlock(&drv_info->mutex);
+		if (ret < 0) {
+			xenbus_dev_fatal(xb_dev, ret,
+				"initializing " XENSND_DRIVER_NAME);
+			break;
+		}
+
+		xenbus_switch_state(xb_dev, XenbusStateInitialised);
+		break;
+
+	case XenbusStateConnected:
+		if (xb_dev->state != XenbusStateInitialised)
+			break;
+
+		mutex_lock(&drv_info->mutex);
+		ret = xdrv_be_on_connected(drv_info);
+		mutex_unlock(&drv_info->mutex);
+		if (ret < 0) {
+			xenbus_dev_fatal(xb_dev, ret,
+				"connecting " XENSND_DRIVER_NAME);
+			break;
+		}
+
+		xenbus_switch_state(xb_dev, XenbusStateConnected);
+		break;
+
+	case XenbusStateClosing:
+		/*
+		 * in this state backend starts freeing resources,
+		 * so let it go into closed state first, so we can also
+		 * remove ours
+		 */
+		break;
+
+	case XenbusStateUnknown:
+		/* fall through */
+	case XenbusStateClosed:
+		if (xb_dev->state == XenbusStateClosed)
+			break;
+
+		mutex_lock(&drv_info->mutex);
+		xdrv_be_on_disconnected(drv_info);
+		mutex_unlock(&drv_info->mutex);
+		xenbus_switch_state(xb_dev, XenbusStateInitialising);
+		break;
+	}
 }
 
 static int xdrv_probe(struct xenbus_device *xb_dev,
@@ -56,6 +146,7 @@ static int xdrv_probe(struct xenbus_device *xb_dev,
 	spin_lock_init(&drv_info->io_lock);
 	mutex_init(&drv_info->mutex);
 	dev_set_drvdata(&xb_dev->dev, drv_info);
+	xenbus_switch_state(xb_dev, XenbusStateInitialising);
 	return 0;
 }
 
@@ -63,6 +154,7 @@ static int xdrv_remove(struct xenbus_device *dev)
 {
 	struct xdrv_info *drv_info = dev_get_drvdata(&dev->dev);
 
+	xenbus_switch_state(dev, XenbusStateClosed);
 	mutex_lock(&drv_info->mutex);
 	xdrv_remove_internal(drv_info);
 	mutex_unlock(&drv_info->mutex);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH RESEND1 04/12] ALSA: vsnd: Read sound driver configuration from Xen store
  2017-08-07 12:22 [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Oleksandr Andrushchenko
                   ` (2 preceding siblings ...)
  2017-08-07 12:22 ` [PATCH RESEND1 03/12] ALSA: vsnd: Implement Xen bus state handling Oleksandr Andrushchenko
@ 2017-08-07 12:22 ` Oleksandr Andrushchenko
  2017-08-07 12:22 ` [PATCH RESEND1 05/12] ALSA: vsnd: Implement Xen event channel handling Oleksandr Andrushchenko
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-07 12:22 UTC (permalink / raw)
  To: alsa-devel, xen-devel, linux-kernel
  Cc: perex, tiwai, andr2000, Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Read configuration values from Xen store according
to xen/interface/io/sndif.h protocol:
- introduce configuration structures for different
  components, e.g. sound card, device, stream
- read PCM HW parameters, e.g rate, format etc.
- detect stream type (capture/playback)
- read device and card parameters

Fill in platform data with the configuration read, so
it can be passed to sound driver later.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
 sound/drivers/xen-front.c | 530 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 530 insertions(+)

diff --git a/sound/drivers/xen-front.c b/sound/drivers/xen-front.c
index c4fd21cac3a7..ef48cbf44cf2 100644
--- a/sound/drivers/xen-front.c
+++ b/sound/drivers/xen-front.c
@@ -20,24 +20,554 @@
 
 #include <linux/module.h>
 
+#include <sound/core.h>
+#include <sound/pcm.h>
+
 #include <xen/platform_pci.h>
 #include <xen/xen.h>
 #include <xen/xenbus.h>
 
 #include <xen/interface/io/sndif.h>
 
+/* maximum number of supported streams */
+#define VSND_MAX_STREAM		8
+
+struct cfg_stream {
+	int unique_id;
+	char *xenstore_path;
+	struct snd_pcm_hardware pcm_hw;
+};
+
+struct cfg_pcm_instance {
+	char name[80];
+	int device_id;
+	struct snd_pcm_hardware pcm_hw;
+	int  num_streams_pb;
+	struct cfg_stream *streams_pb;
+	int  num_streams_cap;
+	struct cfg_stream *streams_cap;
+};
+
+struct cfg_card {
+	char name_short[32];
+	char name_long[80];
+	struct snd_pcm_hardware pcm_hw;
+	int num_pcm_instances;
+	struct cfg_pcm_instance *pcm_instances;
+};
+
+struct sdev_card_plat_data {
+	struct xdrv_info *xdrv_info;
+	struct cfg_card cfg_card;
+};
+
 struct xdrv_info {
 	struct xenbus_device *xb_dev;
 	spinlock_t io_lock;
 	struct mutex mutex;
+	struct sdev_card_plat_data cfg_plat_data;
 };
 
+#define MAX_XEN_BUFFER_SIZE	(64 * 1024)
+#define MAX_BUFFER_SIZE		MAX_XEN_BUFFER_SIZE
+#define MIN_PERIOD_SIZE		64
+#define MAX_PERIOD_SIZE		(MAX_BUFFER_SIZE / 8)
+#define USE_FORMATS		(SNDRV_PCM_FMTBIT_U8 | \
+				 SNDRV_PCM_FMTBIT_S16_LE)
+#define USE_RATE		(SNDRV_PCM_RATE_CONTINUOUS | \
+				 SNDRV_PCM_RATE_8000_48000)
+#define USE_RATE_MIN		5512
+#define USE_RATE_MAX		48000
+#define USE_CHANNELS_MIN	1
+#define USE_CHANNELS_MAX	2
+#define USE_PERIODS_MIN		2
+#define USE_PERIODS_MAX		8
+
+static struct snd_pcm_hardware sdrv_pcm_hw_default = {
+	.info = (SNDRV_PCM_INFO_MMAP |
+		 SNDRV_PCM_INFO_INTERLEAVED |
+		 SNDRV_PCM_INFO_RESUME |
+		 SNDRV_PCM_INFO_MMAP_VALID),
+	.formats = USE_FORMATS,
+	.rates = USE_RATE,
+	.rate_min = USE_RATE_MIN,
+	.rate_max = USE_RATE_MAX,
+	.channels_min = USE_CHANNELS_MIN,
+	.channels_max = USE_CHANNELS_MAX,
+	.buffer_bytes_max = MAX_BUFFER_SIZE,
+	.period_bytes_min = MIN_PERIOD_SIZE,
+	.period_bytes_max = MAX_PERIOD_SIZE,
+	.periods_min = USE_PERIODS_MIN,
+	.periods_max = USE_PERIODS_MAX,
+	.fifo_size = 0,
+};
+
+struct CFG_HW_SAMPLE_RATE {
+	const char *name;
+	unsigned int mask;
+	unsigned int value;
+};
+
+static struct CFG_HW_SAMPLE_RATE cfg_hw_supported_rates[] = {
+	{ .name = "5512",   .mask = SNDRV_PCM_RATE_5512,   .value = 5512 },
+	{ .name = "8000",   .mask = SNDRV_PCM_RATE_8000,   .value = 8000 },
+	{ .name = "11025",  .mask = SNDRV_PCM_RATE_11025,  .value = 11025 },
+	{ .name = "16000",  .mask = SNDRV_PCM_RATE_16000,  .value = 16000 },
+	{ .name = "22050",  .mask = SNDRV_PCM_RATE_22050,  .value = 22050 },
+	{ .name = "32000",  .mask = SNDRV_PCM_RATE_32000,  .value = 32000 },
+	{ .name = "44100",  .mask = SNDRV_PCM_RATE_44100,  .value = 44100 },
+	{ .name = "48000",  .mask = SNDRV_PCM_RATE_48000,  .value = 48000 },
+	{ .name = "64000",  .mask = SNDRV_PCM_RATE_64000,  .value = 64000 },
+	{ .name = "96000",  .mask = SNDRV_PCM_RATE_96000,  .value = 96000 },
+	{ .name = "176400", .mask = SNDRV_PCM_RATE_176400, .value = 176400 },
+	{ .name = "192000", .mask = SNDRV_PCM_RATE_192000, .value = 192000 },
+};
+
+struct CFG_HW_SAMPLE_FORMAT {
+	const char *name;
+	u64 mask;
+};
+
+static struct CFG_HW_SAMPLE_FORMAT cfg_hw_supported_formats[] = {
+	{
+		.name = XENSND_PCM_FORMAT_U8_STR,
+		.mask = SNDRV_PCM_FMTBIT_U8
+	},
+	{
+		.name = XENSND_PCM_FORMAT_S8_STR,
+		.mask = SNDRV_PCM_FMTBIT_S8
+	},
+	{
+		.name = XENSND_PCM_FORMAT_U16_LE_STR,
+		.mask = SNDRV_PCM_FMTBIT_U16_LE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_U16_BE_STR,
+		.mask = SNDRV_PCM_FMTBIT_U16_BE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_S16_LE_STR,
+		.mask = SNDRV_PCM_FMTBIT_S16_LE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_S16_BE_STR,
+		.mask = SNDRV_PCM_FMTBIT_S16_BE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_U24_LE_STR,
+		.mask = SNDRV_PCM_FMTBIT_U24_LE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_U24_BE_STR,
+		.mask = SNDRV_PCM_FMTBIT_U24_BE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_S24_LE_STR,
+		.mask = SNDRV_PCM_FMTBIT_S24_LE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_S24_BE_STR,
+		.mask = SNDRV_PCM_FMTBIT_S24_BE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_U32_LE_STR,
+		.mask = SNDRV_PCM_FMTBIT_U32_LE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_U32_BE_STR,
+		.mask = SNDRV_PCM_FMTBIT_U32_BE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_S32_LE_STR,
+		.mask = SNDRV_PCM_FMTBIT_S32_LE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_S32_BE_STR,
+		.mask = SNDRV_PCM_FMTBIT_S32_BE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_A_LAW_STR,
+		.mask = SNDRV_PCM_FMTBIT_A_LAW
+	},
+	{
+		.name = XENSND_PCM_FORMAT_MU_LAW_STR,
+		.mask = SNDRV_PCM_FMTBIT_MU_LAW
+	},
+	{
+		.name = XENSND_PCM_FORMAT_F32_LE_STR,
+		.mask = SNDRV_PCM_FMTBIT_FLOAT_LE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_F32_BE_STR,
+		.mask = SNDRV_PCM_FMTBIT_FLOAT_BE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_F64_LE_STR,
+		.mask = SNDRV_PCM_FMTBIT_FLOAT64_LE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_F64_BE_STR,
+		.mask = SNDRV_PCM_FMTBIT_FLOAT64_BE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_IEC958_SUBFRAME_LE_STR,
+		.mask = SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_IEC958_SUBFRAME_BE_STR,
+		.mask = SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_BE
+	},
+	{
+		.name = XENSND_PCM_FORMAT_IMA_ADPCM_STR,
+		.mask = SNDRV_PCM_FMTBIT_IMA_ADPCM
+	},
+	{
+		.name = XENSND_PCM_FORMAT_MPEG_STR,
+		.mask = SNDRV_PCM_FMTBIT_MPEG
+	},
+	{
+		.name = XENSND_PCM_FORMAT_GSM_STR,
+		.mask = SNDRV_PCM_FMTBIT_GSM
+	},
+};
+
+static void cfg_hw_rates(char *list, unsigned int len,
+	const char *path, struct snd_pcm_hardware *pcm_hw)
+{
+	char *cur_rate;
+	unsigned int cur_mask;
+	unsigned int cur_value;
+	unsigned int rates;
+	unsigned int rate_min;
+	unsigned int rate_max;
+	int i;
+
+	rates = 0;
+	rate_min = -1;
+	rate_max = 0;
+	while ((cur_rate = strsep(&list, XENSND_LIST_SEPARATOR))) {
+		for (i = 0; i < ARRAY_SIZE(cfg_hw_supported_rates); i++)
+			if (!strncasecmp(cur_rate,
+					cfg_hw_supported_rates[i].name,
+					XENSND_SAMPLE_RATE_MAX_LEN)) {
+				cur_mask = cfg_hw_supported_rates[i].mask;
+				cur_value = cfg_hw_supported_rates[i].value;
+				rates |= cur_mask;
+				if (rate_min > cur_value)
+					rate_min = cur_value;
+				if (rate_max < cur_value)
+					rate_max = cur_value;
+			}
+	}
+
+	if (rates) {
+		pcm_hw->rates = rates;
+		pcm_hw->rate_min = rate_min;
+		pcm_hw->rate_max = rate_max;
+	}
+}
+
+static void cfg_formats(char *list, unsigned int len,
+	const char *path, struct snd_pcm_hardware *pcm_hw)
+{
+	u64 formats;
+	char *cur_format;
+	int i;
+
+	formats = 0;
+	while ((cur_format = strsep(&list, XENSND_LIST_SEPARATOR))) {
+		for (i = 0; i < ARRAY_SIZE(cfg_hw_supported_formats); i++)
+			if (!strncasecmp(cur_format,
+					cfg_hw_supported_formats[i].name,
+					XENSND_SAMPLE_FORMAT_MAX_LEN))
+				formats |= cfg_hw_supported_formats[i].mask;
+	}
+
+	if (formats)
+		pcm_hw->formats = formats;
+}
+
+static void cfg_pcm_hw(const char *path,
+	struct snd_pcm_hardware *parent_pcm_hw,
+	struct snd_pcm_hardware *pcm_hw)
+{
+	char *list;
+	int val;
+	size_t buf_sz;
+	unsigned int len;
+
+	/* inherit parent's PCM HW and read overrides if any */
+	*pcm_hw = *parent_pcm_hw;
+
+	val = xenbus_read_unsigned(path, XENSND_FIELD_CHANNELS_MIN, 0);
+	if (val)
+		pcm_hw->channels_min = val;
+
+	val = xenbus_read_unsigned(path, XENSND_FIELD_CHANNELS_MAX, 0);
+	if (val)
+		pcm_hw->channels_max = val;
+
+	list = xenbus_read(XBT_NIL, path, XENSND_FIELD_SAMPLE_RATES, &len);
+	if (!IS_ERR(list)) {
+		cfg_hw_rates(list, len, path, pcm_hw);
+		kfree(list);
+	}
+
+	list = xenbus_read(XBT_NIL, path, XENSND_FIELD_SAMPLE_FORMATS, &len);
+	if (!IS_ERR(list)) {
+		cfg_formats(list, len, path, pcm_hw);
+		kfree(list);
+	}
+
+	buf_sz = xenbus_read_unsigned(path, XENSND_FIELD_BUFFER_SIZE, 0);
+	if (buf_sz)
+		pcm_hw->buffer_bytes_max = buf_sz;
+}
+
+static int cfg_get_stream_type(const char *path, int index,
+	int *num_pb, int *num_cap)
+{
+	char *str = NULL;
+	char *stream_path;
+	int ret;
+
+	*num_pb = 0;
+	*num_cap = 0;
+	stream_path = kasprintf(GFP_KERNEL, "%s/%d", path, index);
+	if (!stream_path) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	str = xenbus_read(XBT_NIL, stream_path, XENSND_FIELD_TYPE, NULL);
+	if (IS_ERR(str)) {
+		ret = -EINVAL;
+		goto fail;
+	}
+
+	if (!strncasecmp(str, XENSND_STREAM_TYPE_PLAYBACK,
+		sizeof(XENSND_STREAM_TYPE_PLAYBACK)))
+		(*num_pb)++;
+	else if (!strncasecmp(str, XENSND_STREAM_TYPE_CAPTURE,
+		sizeof(XENSND_STREAM_TYPE_CAPTURE)))
+		(*num_cap)++;
+	else {
+		ret = -EINVAL;
+		goto fail;
+	}
+	ret = 0;
+
+fail:
+	kfree(stream_path);
+	kfree(str);
+	return ret;
+}
+
+static int cfg_stream(struct xdrv_info *drv_info,
+	struct cfg_pcm_instance *pcm_instance,
+	const char *path, int index, int *cur_pb, int *cur_cap,
+	int *stream_idx)
+{
+	char *str = NULL;
+	char *stream_path;
+	struct cfg_stream *stream;
+	int ret;
+
+	stream_path = devm_kasprintf(&drv_info->xb_dev->dev,
+		GFP_KERNEL, "%s/%d", path, index);
+	if (!stream_path) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	str = xenbus_read(XBT_NIL, stream_path, XENSND_FIELD_TYPE, NULL);
+	if (IS_ERR(str)) {
+		ret = -EINVAL;
+		goto fail;
+	}
+
+	if (!strncasecmp(str, XENSND_STREAM_TYPE_PLAYBACK,
+		sizeof(XENSND_STREAM_TYPE_PLAYBACK))) {
+		stream = &pcm_instance->streams_pb[(*cur_pb)++];
+	} else if (!strncasecmp(str, XENSND_STREAM_TYPE_CAPTURE,
+		sizeof(XENSND_STREAM_TYPE_CAPTURE))) {
+		stream = &pcm_instance->streams_cap[(*cur_cap)++];
+	} else {
+		ret = -EINVAL;
+		goto fail;
+	}
+
+	/* get next stream index */
+	stream->unique_id = (*stream_idx)++;
+	stream->xenstore_path = stream_path;
+	/*
+	 * check in Xen store if PCM HW configuration exists for this stream
+	 * and update if so, e.g. we inherit all values from device's PCM HW,
+	 * but can still override some of the values for the stream
+	 */
+	cfg_pcm_hw(stream->xenstore_path,
+		&pcm_instance->pcm_hw, &stream->pcm_hw);
+	ret = 0;
+
+fail:
+	kfree(str);
+	return ret;
+}
+
+static int cfg_device(struct xdrv_info *drv_info,
+	struct cfg_pcm_instance *pcm_instance,
+	struct snd_pcm_hardware *parent_pcm_hw,
+	const char *path, int node_index, int *stream_idx)
+{
+	char *str;
+	char *device_path;
+	int ret, i, num_streams;
+	int num_pb, num_cap;
+	int cur_pb, cur_cap;
+	char node[3];
+
+	device_path = kasprintf(GFP_KERNEL, "%s/%d", path, node_index);
+	if (!device_path)
+		return -ENOMEM;
+
+	str = xenbus_read(XBT_NIL, device_path, XENSND_FIELD_DEVICE_NAME, NULL);
+	if (!IS_ERR(str)) {
+		strncpy(pcm_instance->name, str, sizeof(pcm_instance->name));
+		kfree(str);
+	}
+
+	pcm_instance->device_id = node_index;
+
+	/*
+	 * check in Xen store if PCM HW configuration exists for this device
+	 * and update if so, e.g. we inherit all values from card's PCM HW,
+	 * but can still override some of the values for the device
+	 */
+	cfg_pcm_hw(device_path, parent_pcm_hw, &pcm_instance->pcm_hw);
+
+	/*
+	 * find out how many streams were configured in Xen store:
+	 * streams must have sequential unique IDs, so stop when one
+	 * does not exist
+	 */
+	num_streams = 0;
+	do {
+		snprintf(node, sizeof(node), "%d", num_streams);
+		if (!xenbus_exists(XBT_NIL, device_path, node))
+			break;
+
+		num_streams++;
+	} while (num_streams < VSND_MAX_STREAM);
+
+	pcm_instance->num_streams_pb = 0;
+	pcm_instance->num_streams_cap = 0;
+	/* get number of playback and capture streams */
+	for (i = 0; i < num_streams; i++) {
+		ret = cfg_get_stream_type(device_path, i, &num_pb, &num_cap);
+		if (ret < 0)
+			goto fail;
+
+		pcm_instance->num_streams_pb += num_pb;
+		pcm_instance->num_streams_cap += num_cap;
+	}
+
+	if (pcm_instance->num_streams_pb) {
+		pcm_instance->streams_pb = devm_kcalloc(
+			&drv_info->xb_dev->dev,
+			pcm_instance->num_streams_pb,
+			sizeof(struct cfg_stream), GFP_KERNEL);
+		if (!pcm_instance->streams_pb) {
+			ret = -ENOMEM;
+			goto fail;
+		}
+	}
+
+	if (pcm_instance->num_streams_cap) {
+		pcm_instance->streams_cap = devm_kcalloc(
+			&drv_info->xb_dev->dev,
+			pcm_instance->num_streams_cap,
+			sizeof(struct cfg_stream), GFP_KERNEL);
+		if (!pcm_instance->streams_cap) {
+			ret = -ENOMEM;
+			goto fail;
+		}
+	}
+
+	cur_pb = 0;
+	cur_cap = 0;
+	for (i = 0; i < num_streams; i++) {
+		ret = cfg_stream(drv_info,
+			pcm_instance, device_path, i, &cur_pb, &cur_cap,
+			stream_idx);
+		if (ret < 0)
+			goto fail;
+	}
+	ret = 0;
+
+fail:
+	kfree(device_path);
+	return ret;
+}
+
+static int cfg_card(struct xdrv_info *drv_info,
+	struct sdev_card_plat_data *plat_data, int *stream_idx)
+{
+	struct xenbus_device *xb_dev = drv_info->xb_dev;
+	int ret, num_devices, i;
+	char node[3];
+
+	num_devices = 0;
+	do {
+		snprintf(node, sizeof(node), "%d", num_devices);
+		if (!xenbus_exists(XBT_NIL, xb_dev->nodename, node))
+			break;
+
+		num_devices++;
+	} while (num_devices < SNDRV_PCM_DEVICES);
+
+	if (!num_devices) {
+		dev_warn(&xb_dev->dev,
+			"No devices configured for sound card at %s\n",
+			xb_dev->nodename);
+		return -ENODEV;
+	}
+
+	/* start from default PCM HW configuration for the card */
+	cfg_pcm_hw(xb_dev->nodename, &sdrv_pcm_hw_default,
+		&plat_data->cfg_card.pcm_hw);
+
+	plat_data->cfg_card.pcm_instances = devm_kcalloc(
+		&drv_info->xb_dev->dev, num_devices,
+		sizeof(struct cfg_pcm_instance), GFP_KERNEL);
+	if (!plat_data->cfg_card.pcm_instances)
+		return -ENOMEM;
+
+	for (i = 0; i < num_devices; i++) {
+		ret = cfg_device(drv_info,
+			&plat_data->cfg_card.pcm_instances[i],
+			&plat_data->cfg_card.pcm_hw,
+			xb_dev->nodename, i, stream_idx);
+		if (ret < 0)
+			return ret;
+	}
+	plat_data->cfg_card.num_pcm_instances = num_devices;
+	return 0;
+}
+
 static void xdrv_remove_internal(struct xdrv_info *drv_info)
 {
 }
 
 static int xdrv_be_on_initwait(struct xdrv_info *drv_info)
 {
+	int stream_idx;
+	int ret;
+
+	drv_info->cfg_plat_data.xdrv_info = drv_info;
+	stream_idx = 0;
+	ret = cfg_card(drv_info, &drv_info->cfg_plat_data, &stream_idx);
+	if (ret < 0)
+		return ret;
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH RESEND1 05/12] ALSA: vsnd: Implement Xen event channel handling
  2017-08-07 12:22 [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Oleksandr Andrushchenko
                   ` (3 preceding siblings ...)
  2017-08-07 12:22 ` [PATCH RESEND1 04/12] ALSA: vsnd: Read sound driver configuration from Xen store Oleksandr Andrushchenko
@ 2017-08-07 12:22 ` Oleksandr Andrushchenko
  2017-08-07 12:22 ` [PATCH RESEND1 06/12] ALSA: vsnd: Implement handling of shared buffers Oleksandr Andrushchenko
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-07 12:22 UTC (permalink / raw)
  To: alsa-devel, xen-devel, linux-kernel
  Cc: perex, tiwai, andr2000, Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

1. Create event channels for all configured streams and publish
corresponding ring references and event channels in Xen store,
so backend can connect.
2. Implement event channel interrupt handler.
3. Create and destroy event channels with respect to Xen bus state.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
 sound/drivers/xen-front.c | 269 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 268 insertions(+), 1 deletion(-)

diff --git a/sound/drivers/xen-front.c b/sound/drivers/xen-front.c
index ef48cbf44cf2..a92459b2737e 100644
--- a/sound/drivers/xen-front.c
+++ b/sound/drivers/xen-front.c
@@ -24,14 +24,40 @@
 #include <sound/pcm.h>
 
 #include <xen/platform_pci.h>
+#include <xen/events.h>
+#include <xen/grant_table.h>
 #include <xen/xen.h>
 #include <xen/xenbus.h>
 
 #include <xen/interface/io/sndif.h>
 
+/*
+ * FIXME: usage of grant reference 0 as invalid grant reference:
+ * grant reference 0 is valid, but never exposed to a PV driver,
+ * because of the fact it is already in use/reserved by the PV console.
+ */
+#define GRANT_INVALID_REF	0
 /* maximum number of supported streams */
 #define VSND_MAX_STREAM		8
 
+enum xdrv_evtchnl_state {
+	EVTCHNL_STATE_DISCONNECTED,
+	EVTCHNL_STATE_CONNECTED,
+};
+
+struct xdrv_evtchnl_info {
+	struct xdrv_info *drv_info;
+	struct xen_sndif_front_ring ring;
+	int ring_ref;
+	int port;
+	int irq;
+	struct completion completion;
+	enum xdrv_evtchnl_state state;
+	/* latest response status and its corresponding id */
+	int resp_status;
+	uint16_t resp_id;
+};
+
 struct cfg_stream {
 	int unique_id;
 	char *xenstore_path;
@@ -65,6 +91,8 @@ struct xdrv_info {
 	struct xenbus_device *xb_dev;
 	spinlock_t io_lock;
 	struct mutex mutex;
+	int num_evt_channels;
+	struct xdrv_evtchnl_info *evt_chnls;
 	struct sdev_card_plat_data cfg_plat_data;
 };
 
@@ -102,6 +130,244 @@ static struct snd_pcm_hardware sdrv_pcm_hw_default = {
 	.fifo_size = 0,
 };
 
+static irqreturn_t xdrv_evtchnl_interrupt(int irq, void *dev_id)
+{
+	struct xdrv_evtchnl_info *channel = dev_id;
+	struct xdrv_info *drv_info = channel->drv_info;
+	struct xensnd_resp *resp;
+	RING_IDX i, rp;
+	unsigned long flags;
+
+	spin_lock_irqsave(&drv_info->io_lock, flags);
+	if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
+		goto out;
+
+again:
+	rp = channel->ring.sring->rsp_prod;
+	/* ensure we see queued responses up to rp */
+	rmb();
+
+	for (i = channel->ring.rsp_cons; i != rp; i++) {
+		resp = RING_GET_RESPONSE(&channel->ring, i);
+		if (resp->id != channel->resp_id)
+			continue;
+		switch (resp->operation) {
+		case XENSND_OP_OPEN:
+			/* fall through */
+		case XENSND_OP_CLOSE:
+			/* fall through */
+		case XENSND_OP_READ:
+			/* fall through */
+		case XENSND_OP_WRITE:
+			channel->resp_status = resp->status;
+			complete(&channel->completion);
+			break;
+
+		default:
+			dev_err(&drv_info->xb_dev->dev,
+				"Operation %d is not supported\n",
+				resp->operation);
+			break;
+		}
+	}
+
+	channel->ring.rsp_cons = i;
+	if (i != channel->ring.req_prod_pvt) {
+		int more_to_do;
+
+		RING_FINAL_CHECK_FOR_RESPONSES(&channel->ring, more_to_do);
+		if (more_to_do)
+			goto again;
+	} else
+		channel->ring.sring->rsp_event = i + 1;
+
+out:
+	spin_unlock_irqrestore(&drv_info->io_lock, flags);
+	return IRQ_HANDLED;
+}
+
+static inline void xdrv_evtchnl_flush(
+		struct xdrv_evtchnl_info *channel)
+{
+	int notify;
+
+	channel->ring.req_prod_pvt++;
+	RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&channel->ring, notify);
+	if (notify)
+		notify_remote_via_irq(channel->irq);
+}
+
+static void xdrv_evtchnl_free(struct xdrv_info *drv_info,
+		struct xdrv_evtchnl_info *channel)
+{
+	if (!channel->ring.sring)
+		return;
+
+	channel->state = EVTCHNL_STATE_DISCONNECTED;
+	channel->resp_status = -EIO;
+	complete_all(&channel->completion);
+
+	if (channel->irq)
+		unbind_from_irqhandler(channel->irq, channel);
+	channel->irq = 0;
+
+	if (channel->port)
+		xenbus_free_evtchn(drv_info->xb_dev, channel->port);
+	channel->port = 0;
+
+	if (channel->ring_ref != GRANT_INVALID_REF)
+		gnttab_end_foreign_access(channel->ring_ref, 0,
+			(unsigned long)channel->ring.sring);
+	channel->ring_ref = GRANT_INVALID_REF;
+	channel->ring.sring = NULL;
+}
+
+static void xdrv_evtchnl_free_all(struct xdrv_info *drv_info)
+{
+	int i;
+
+	if (!drv_info->evt_chnls)
+		return;
+
+	for (i = 0; i < drv_info->num_evt_channels; i++)
+		xdrv_evtchnl_free(drv_info, &drv_info->evt_chnls[i]);
+
+	devm_kfree(&drv_info->xb_dev->dev, drv_info->evt_chnls);
+	drv_info->evt_chnls = NULL;
+}
+
+static int xdrv_evtchnl_alloc(struct xdrv_info *drv_info,
+		struct xdrv_evtchnl_info *evt_channel)
+{
+	struct xenbus_device *xb_dev = drv_info->xb_dev;
+	struct xen_sndif_sring *sring;
+	grant_ref_t gref;
+	int ret;
+
+	evt_channel->drv_info = drv_info;
+	init_completion(&evt_channel->completion);
+	evt_channel->state = EVTCHNL_STATE_DISCONNECTED;
+	evt_channel->ring_ref = GRANT_INVALID_REF;
+	evt_channel->ring.sring = NULL;
+	evt_channel->port = 0;
+	evt_channel->irq = 0;
+
+	sring = (struct xen_sndif_sring *)get_zeroed_page(
+		GFP_NOIO | __GFP_HIGH);
+	if (!sring) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	SHARED_RING_INIT(sring);
+	FRONT_RING_INIT(&evt_channel->ring, sring, XEN_PAGE_SIZE);
+	ret = xenbus_grant_ring(xb_dev, sring, 1, &gref);
+	if (ret < 0)
+		goto fail;
+	evt_channel->ring_ref = gref;
+
+	ret = xenbus_alloc_evtchn(xb_dev, &evt_channel->port);
+	if (ret < 0)
+		goto fail;
+
+	ret = bind_evtchn_to_irqhandler(evt_channel->port,
+		xdrv_evtchnl_interrupt, 0, xb_dev->devicetype, evt_channel);
+	if (ret < 0)
+		goto fail;
+
+	evt_channel->irq = ret;
+	return 0;
+
+fail:
+	dev_err(&xb_dev->dev, "Failed to allocate ring: %d\n", ret);
+	return ret;
+}
+
+static int xdrv_evtchnl_create(struct xdrv_info *drv_info,
+		struct xdrv_evtchnl_info *evt_channel,
+		const char *path)
+{
+	int ret;
+
+	ret = xdrv_evtchnl_alloc(drv_info, evt_channel);
+	if (ret < 0) {
+		dev_err(&drv_info->xb_dev->dev,
+			"allocating event channel: %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * write values to Xen store, so backend can find ring reference
+	 * and event channel
+	 */
+	ret = xenbus_printf(XBT_NIL, path, XENSND_FIELD_RING_REF, "%u",
+			evt_channel->ring_ref);
+	if (ret < 0) {
+		dev_err(&drv_info->xb_dev->dev,
+			"writing " XENSND_FIELD_RING_REF": %d\n", ret);
+		return ret;
+	}
+
+	ret = xenbus_printf(XBT_NIL, path, XENSND_FIELD_EVT_CHNL, "%u",
+		evt_channel->port);
+	if (ret < 0) {
+		dev_err(&drv_info->xb_dev->dev,
+			"writing " XENSND_FIELD_EVT_CHNL": %d\n", ret);
+		return ret;
+	}
+	return 0;
+}
+
+static int xdrv_evtchnl_create_all(struct xdrv_info *drv_info,
+		int num_streams)
+{
+	struct cfg_card *cfg_card;
+	int d, ret = 0;
+
+	drv_info->evt_chnls = devm_kcalloc(&drv_info->xb_dev->dev,
+		num_streams, sizeof(struct xdrv_evtchnl_info), GFP_KERNEL);
+	if (!drv_info->evt_chnls) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	cfg_card = &drv_info->cfg_plat_data.cfg_card;
+	/* iterate over devices and their streams and create event channels */
+	for (d = 0; d < cfg_card->num_pcm_instances; d++) {
+		struct cfg_pcm_instance *pcm_instance;
+		int s, stream_idx;
+
+		pcm_instance = &cfg_card->pcm_instances[d];
+
+		for (s = 0; s < pcm_instance->num_streams_pb; s++) {
+			stream_idx = pcm_instance->streams_pb[s].unique_id;
+			ret = xdrv_evtchnl_create(drv_info,
+				&drv_info->evt_chnls[stream_idx],
+				pcm_instance->streams_pb[s].xenstore_path);
+			if (ret < 0)
+				goto fail;
+		}
+
+		for (s = 0; s < pcm_instance->num_streams_cap; s++) {
+			stream_idx = pcm_instance->streams_cap[s].unique_id;
+			ret = xdrv_evtchnl_create(drv_info,
+				&drv_info->evt_chnls[stream_idx],
+				pcm_instance->streams_cap[s].xenstore_path);
+			if (ret < 0)
+				goto fail;
+		}
+	}
+	if (ret < 0)
+		goto fail;
+
+	drv_info->num_evt_channels = num_streams;
+	return 0;
+
+fail:
+	xdrv_evtchnl_free_all(drv_info);
+	return ret;
+}
+
 struct CFG_HW_SAMPLE_RATE {
 	const char *name;
 	unsigned int mask;
@@ -556,6 +822,7 @@ static int cfg_card(struct xdrv_info *drv_info,
 
 static void xdrv_remove_internal(struct xdrv_info *drv_info)
 {
+	xdrv_evtchnl_free_all(drv_info);
 }
 
 static int xdrv_be_on_initwait(struct xdrv_info *drv_info)
@@ -568,7 +835,7 @@ static int xdrv_be_on_initwait(struct xdrv_info *drv_info)
 	ret = cfg_card(drv_info, &drv_info->cfg_plat_data, &stream_idx);
 	if (ret < 0)
 		return ret;
-	return 0;
+	return xdrv_evtchnl_create_all(drv_info, stream_idx);
 }
 
 static inline int xdrv_be_on_connected(struct xdrv_info *drv_info)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH RESEND1 06/12] ALSA: vsnd: Implement handling of shared buffers
  2017-08-07 12:22 [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Oleksandr Andrushchenko
                   ` (4 preceding siblings ...)
  2017-08-07 12:22 ` [PATCH RESEND1 05/12] ALSA: vsnd: Implement Xen event channel handling Oleksandr Andrushchenko
@ 2017-08-07 12:22 ` Oleksandr Andrushchenko
  2017-08-07 12:22 ` [PATCH RESEND1 07/12] ALSA: vsnd: Introduce ALSA virtual sound driver Oleksandr Andrushchenko
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-07 12:22 UTC (permalink / raw)
  To: alsa-devel, xen-devel, linux-kernel
  Cc: perex, tiwai, andr2000, Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Implement shared buffer handling according to the
para-virtualized sound device protocol at xen/interface/io/sndif.h:
- manage buffer memory
- handle granted references
- handle page directories

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
 sound/drivers/xen-front.c | 178 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 178 insertions(+)

diff --git a/sound/drivers/xen-front.c b/sound/drivers/xen-front.c
index a92459b2737e..04ebc15757f4 100644
--- a/sound/drivers/xen-front.c
+++ b/sound/drivers/xen-front.c
@@ -58,6 +58,14 @@ struct xdrv_evtchnl_info {
 	uint16_t resp_id;
 };
 
+struct sh_buf_info {
+	int num_grefs;
+	grant_ref_t *grefs;
+	uint8_t *vdirectory;
+	uint8_t *vbuffer;
+	size_t vbuffer_sz;
+};
+
 struct cfg_stream {
 	int unique_id;
 	char *xenstore_path;
@@ -825,6 +833,176 @@ static void xdrv_remove_internal(struct xdrv_info *drv_info)
 	xdrv_evtchnl_free_all(drv_info);
 }
 
+static inline grant_ref_t sh_buf_get_dir_start(struct sh_buf_info *buf)
+{
+	if (!buf->grefs)
+		return GRANT_INVALID_REF;
+	return buf->grefs[0];
+}
+
+static inline void sh_buf_clear(struct sh_buf_info *buf)
+{
+	memset(buf, 0, sizeof(*buf));
+}
+
+static void sh_buf_free(struct sh_buf_info *buf)
+{
+	int i;
+
+	if (buf->grefs) {
+		for (i = 0; i < buf->num_grefs; i++)
+			if (buf->grefs[i] != GRANT_INVALID_REF)
+				gnttab_end_foreign_access(buf->grefs[i],
+						0, 0UL);
+		kfree(buf->grefs);
+	}
+	kfree(buf->vdirectory);
+	free_pages_exact(buf->vbuffer, buf->vbuffer_sz);
+	sh_buf_clear(buf);
+}
+
+/*
+ * number of grant references a page can hold with respect to the
+ * xendispl_page_directory header
+ */
+#define XENSND_NUM_GREFS_PER_PAGE ((XEN_PAGE_SIZE - \
+	offsetof(struct xensnd_page_directory, gref)) / \
+	sizeof(grant_ref_t))
+
+static void sh_buf_fill_page_dir(struct sh_buf_info *buf, int num_pages_dir)
+{
+	struct xensnd_page_directory *page_dir;
+	unsigned char *ptr;
+	int i, cur_gref, grefs_left, to_copy;
+
+	ptr = buf->vdirectory;
+	grefs_left = buf->num_grefs - num_pages_dir;
+	/*
+	 * skip grant references at the beginning, they are for pages granted
+	 * for the page directory itself
+	 */
+	cur_gref = num_pages_dir;
+	for (i = 0; i < num_pages_dir; i++) {
+		page_dir = (struct xensnd_page_directory *)ptr;
+		if (grefs_left <= XENSND_NUM_GREFS_PER_PAGE) {
+			to_copy = grefs_left;
+			page_dir->gref_dir_next_page = GRANT_INVALID_REF;
+		} else {
+			to_copy = XENSND_NUM_GREFS_PER_PAGE;
+			page_dir->gref_dir_next_page = buf->grefs[i + 1];
+		}
+		memcpy(&page_dir->gref, &buf->grefs[cur_gref],
+			to_copy * sizeof(grant_ref_t));
+		ptr += XEN_PAGE_SIZE;
+		grefs_left -= to_copy;
+		cur_gref += to_copy;
+	}
+}
+
+static int sh_buf_grant_refs(struct xenbus_device *xb_dev,
+	struct sh_buf_info *buf,
+	int num_pages_dir, int num_pages_vbuffer, int num_grefs)
+{
+	grant_ref_t priv_gref_head;
+	int ret, i, j, cur_ref;
+	int otherend_id;
+
+	ret = gnttab_alloc_grant_references(num_grefs, &priv_gref_head);
+	if (ret)
+		return ret;
+
+	buf->num_grefs = num_grefs;
+	otherend_id = xb_dev->otherend_id;
+	j = 0;
+
+	for (i = 0; i < num_pages_dir; i++) {
+		cur_ref = gnttab_claim_grant_reference(&priv_gref_head);
+		if (cur_ref < 0) {
+			ret = cur_ref;
+			goto fail;
+		}
+
+		gnttab_grant_foreign_access_ref(cur_ref, otherend_id,
+			xen_page_to_gfn(virt_to_page(buf->vdirectory +
+				XEN_PAGE_SIZE * i)), 0);
+		buf->grefs[j++] = cur_ref;
+	}
+
+	for (i = 0; i < num_pages_vbuffer; i++) {
+		cur_ref = gnttab_claim_grant_reference(&priv_gref_head);
+		if (cur_ref < 0) {
+			ret = cur_ref;
+			goto fail;
+		}
+
+		gnttab_grant_foreign_access_ref(cur_ref, otherend_id,
+			xen_page_to_gfn(virt_to_page(buf->vbuffer +
+				XEN_PAGE_SIZE * i)), 0);
+		buf->grefs[j++] = cur_ref;
+	}
+
+	gnttab_free_grant_references(priv_gref_head);
+	sh_buf_fill_page_dir(buf, num_pages_dir);
+	return 0;
+
+fail:
+	gnttab_free_grant_references(priv_gref_head);
+	return ret;
+}
+
+static int sh_buf_alloc_int_buffers(struct sh_buf_info *buf,
+		int num_pages_dir, int num_pages_vbuffer, int num_grefs)
+{
+	buf->grefs = kcalloc(num_grefs, sizeof(*buf->grefs), GFP_KERNEL);
+	if (!buf->grefs)
+		return -ENOMEM;
+
+	buf->vdirectory = kcalloc(num_pages_dir, XEN_PAGE_SIZE, GFP_KERNEL);
+	if (!buf->vdirectory)
+		goto fail;
+
+	buf->vbuffer_sz = num_pages_vbuffer * XEN_PAGE_SIZE;
+	buf->vbuffer = alloc_pages_exact(buf->vbuffer_sz, GFP_KERNEL);
+	if (!buf->vbuffer)
+		goto fail;
+	return 0;
+
+fail:
+	kfree(buf->grefs);
+	buf->grefs = NULL;
+	kfree(buf->vdirectory);
+	buf->vdirectory = NULL;
+	return -ENOMEM;
+}
+
+static int sh_buf_alloc(struct xenbus_device *xb_dev,
+	struct sh_buf_info *buf, unsigned int buffer_size)
+{
+	int num_pages_vbuffer, num_pages_dir, num_grefs;
+	int ret;
+
+	sh_buf_clear(buf);
+
+	num_pages_vbuffer = DIV_ROUND_UP(buffer_size, XEN_PAGE_SIZE);
+	/* number of pages the page directory consumes itself */
+	num_pages_dir = DIV_ROUND_UP(num_pages_vbuffer,
+		XENSND_NUM_GREFS_PER_PAGE);
+	num_grefs = num_pages_vbuffer + num_pages_dir;
+
+	ret = sh_buf_alloc_int_buffers(buf, num_pages_dir,
+		num_pages_vbuffer, num_grefs);
+	if (ret < 0)
+		return ret;
+
+	ret = sh_buf_grant_refs(xb_dev, buf,
+		num_pages_dir, num_pages_vbuffer, num_grefs);
+	if (ret < 0)
+		return ret;
+
+	sh_buf_fill_page_dir(buf, num_pages_dir);
+	return 0;
+}
+
 static int xdrv_be_on_initwait(struct xdrv_info *drv_info)
 {
 	int stream_idx;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH RESEND1 07/12] ALSA: vsnd: Introduce ALSA virtual sound driver
  2017-08-07 12:22 [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Oleksandr Andrushchenko
                   ` (5 preceding siblings ...)
  2017-08-07 12:22 ` [PATCH RESEND1 06/12] ALSA: vsnd: Implement handling of shared buffers Oleksandr Andrushchenko
@ 2017-08-07 12:22 ` Oleksandr Andrushchenko
  2017-08-07 12:22 ` [PATCH RESEND1 08/12] ALSA: vsnd: Initialize virtul sound card Oleksandr Andrushchenko
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-07 12:22 UTC (permalink / raw)
  To: alsa-devel, xen-devel, linux-kernel
  Cc: perex, tiwai, andr2000, Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Implement essential initialization of the sound driver:
- introduce required data structures
- handle driver registration
- handle sound card registration
- register sound driver on backend connection
- remove sound driver on backend disconnect

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
 sound/drivers/xen-front.c | 161 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 159 insertions(+), 2 deletions(-)

diff --git a/sound/drivers/xen-front.c b/sound/drivers/xen-front.c
index 04ebc15757f4..f3e3f64f0aa6 100644
--- a/sound/drivers/xen-front.c
+++ b/sound/drivers/xen-front.c
@@ -19,13 +19,14 @@
  */
 
 #include <linux/module.h>
+#include <linux/platform_device.h>
 
 #include <sound/core.h>
 #include <sound/pcm.h>
 
-#include <xen/platform_pci.h>
 #include <xen/events.h>
 #include <xen/grant_table.h>
+#include <xen/platform_pci.h>
 #include <xen/xen.h>
 #include <xen/xenbus.h>
 
@@ -66,6 +67,33 @@ struct sh_buf_info {
 	size_t vbuffer_sz;
 };
 
+struct sdev_pcm_stream_info {
+	int unique_id;
+	struct snd_pcm_hardware pcm_hw;
+	struct xdrv_evtchnl_info *evt_chnl;
+	bool is_open;
+	uint8_t req_next_id;
+	struct sh_buf_info sh_buf;
+};
+
+struct sdev_pcm_instance_info {
+	struct sdev_card_info *card_info;
+	struct snd_pcm *pcm;
+	struct snd_pcm_hardware pcm_hw;
+	int num_pcm_streams_pb;
+	struct sdev_pcm_stream_info *streams_pb;
+	int num_pcm_streams_cap;
+	struct sdev_pcm_stream_info *streams_cap;
+};
+
+struct sdev_card_info {
+	struct xdrv_info *xdrv_info;
+	struct snd_card *card;
+	struct snd_pcm_hardware pcm_hw;
+	int num_pcm_instances;
+	struct sdev_pcm_instance_info *pcm_instances;
+};
+
 struct cfg_stream {
 	int unique_id;
 	char *xenstore_path;
@@ -99,6 +127,8 @@ struct xdrv_info {
 	struct xenbus_device *xb_dev;
 	spinlock_t io_lock;
 	struct mutex mutex;
+	bool sdrv_registered;
+	struct platform_device *sdrv_pdev;
 	int num_evt_channels;
 	struct xdrv_evtchnl_info *evt_chnls;
 	struct sdev_card_plat_data cfg_plat_data;
@@ -138,6 +168,132 @@ static struct snd_pcm_hardware sdrv_pcm_hw_default = {
 	.fifo_size = 0,
 };
 
+static int sdrv_new_pcm(struct sdev_card_info *card_info,
+	struct cfg_pcm_instance *instance_config,
+	struct sdev_pcm_instance_info *pcm_instance_info)
+{
+	return 0;
+}
+
+static int sdrv_probe(struct platform_device *pdev)
+{
+	struct sdev_card_info *card_info;
+	struct sdev_card_plat_data *platdata;
+	struct snd_card *card;
+	int ret, i;
+
+	platdata = dev_get_platdata(&pdev->dev);
+
+	dev_dbg(&pdev->dev, "Creating virtual sound card\n");
+
+	ret = snd_card_new(&pdev->dev, 0, XENSND_DRIVER_NAME, THIS_MODULE,
+		sizeof(struct sdev_card_info), &card);
+	if (ret < 0)
+		return ret;
+
+	card_info = card->private_data;
+	card_info->xdrv_info = platdata->xdrv_info;
+	card_info->card = card;
+	card_info->pcm_instances = devm_kcalloc(&pdev->dev,
+			platdata->cfg_card.num_pcm_instances,
+			sizeof(struct sdev_pcm_instance_info), GFP_KERNEL);
+	if (!card_info->pcm_instances) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	card_info->num_pcm_instances = platdata->cfg_card.num_pcm_instances;
+	card_info->pcm_hw = platdata->cfg_card.pcm_hw;
+
+	for (i = 0; i < platdata->cfg_card.num_pcm_instances; i++) {
+		ret = sdrv_new_pcm(card_info,
+			&platdata->cfg_card.pcm_instances[i],
+			&card_info->pcm_instances[i]);
+		if (ret < 0)
+			goto fail;
+	}
+
+	strncpy(card->driver, XENSND_DRIVER_NAME, sizeof(card->driver));
+	strncpy(card->shortname, platdata->cfg_card.name_short,
+		sizeof(card->shortname));
+	strncpy(card->longname, platdata->cfg_card.name_long,
+		sizeof(card->longname));
+
+	ret = snd_card_register(card);
+	if (ret < 0)
+		goto fail;
+
+	platform_set_drvdata(pdev, card);
+	return 0;
+
+fail:
+	snd_card_free(card);
+	return ret;
+}
+
+static int sdrv_remove(struct platform_device *pdev)
+{
+	struct sdev_card_info *info;
+	struct snd_card *card = platform_get_drvdata(pdev);
+
+	info = card->private_data;
+	dev_dbg(&pdev->dev, "Removing virtual sound card %d\n",
+		info->card->number);
+	snd_card_free(card);
+	return 0;
+}
+
+static struct platform_driver sdrv_info = {
+	.probe	= sdrv_probe,
+	.remove	= sdrv_remove,
+	.driver	= {
+		.name	= XENSND_DRIVER_NAME,
+	},
+};
+
+static void sdrv_cleanup(struct xdrv_info *drv_info)
+{
+	if (!drv_info->sdrv_registered)
+		return;
+
+	if (drv_info->sdrv_pdev) {
+		struct platform_device *sdrv_pdev;
+
+		sdrv_pdev = drv_info->sdrv_pdev;
+		if (sdrv_pdev)
+			platform_device_unregister(sdrv_pdev);
+	}
+	platform_driver_unregister(&sdrv_info);
+	drv_info->sdrv_registered = false;
+}
+
+static int sdrv_init(struct xdrv_info *drv_info)
+{
+	struct platform_device *sdrv_pdev;
+	int ret;
+
+	ret = platform_driver_register(&sdrv_info);
+	if (ret < 0)
+		return ret;
+
+	drv_info->sdrv_registered = true;
+	/* pass card configuration via platform data */
+	sdrv_pdev = platform_device_register_data(NULL,
+		XENSND_DRIVER_NAME, 0, &drv_info->cfg_plat_data,
+		sizeof(drv_info->cfg_plat_data));
+	if (IS_ERR(sdrv_pdev))
+		goto fail;
+
+	drv_info->sdrv_pdev = sdrv_pdev;
+	return 0;
+
+fail:
+	dev_err(&drv_info->xb_dev->dev,
+		"failed to register virtual sound driver\n");
+	sdrv_cleanup(drv_info);
+	return -ENODEV;
+}
+
 static irqreturn_t xdrv_evtchnl_interrupt(int irq, void *dev_id)
 {
 	struct xdrv_evtchnl_info *channel = dev_id;
@@ -830,6 +986,7 @@ static int cfg_card(struct xdrv_info *drv_info,
 
 static void xdrv_remove_internal(struct xdrv_info *drv_info)
 {
+	sdrv_cleanup(drv_info);
 	xdrv_evtchnl_free_all(drv_info);
 }
 
@@ -1018,7 +1175,7 @@ static int xdrv_be_on_initwait(struct xdrv_info *drv_info)
 
 static inline int xdrv_be_on_connected(struct xdrv_info *drv_info)
 {
-	return 0;
+	return sdrv_init(drv_info);
 }
 
 static inline void xdrv_be_on_disconnected(struct xdrv_info *drv_info)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH RESEND1 08/12] ALSA: vsnd: Initialize virtul sound card
  2017-08-07 12:22 [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Oleksandr Andrushchenko
                   ` (6 preceding siblings ...)
  2017-08-07 12:22 ` [PATCH RESEND1 07/12] ALSA: vsnd: Introduce ALSA virtual sound driver Oleksandr Andrushchenko
@ 2017-08-07 12:22 ` Oleksandr Andrushchenko
  2017-08-07 12:22 ` [PATCH RESEND1 09/12] ALSA: vsnd: Add timer for period interrupt emulation Oleksandr Andrushchenko
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-07 12:22 UTC (permalink / raw)
  To: alsa-devel, xen-devel, linux-kernel
  Cc: perex, tiwai, andr2000, Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Initialize virtual sound card with streams according to the
Xen store configuration.
Add stubs for stream PCM operations.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
 sound/drivers/xen-front.c | 232 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 232 insertions(+)

diff --git a/sound/drivers/xen-front.c b/sound/drivers/xen-front.c
index f3e3f64f0aa6..9f31e6832086 100644
--- a/sound/drivers/xen-front.c
+++ b/sound/drivers/xen-front.c
@@ -134,6 +134,129 @@ struct xdrv_info {
 	struct sdev_card_plat_data cfg_plat_data;
 };
 
+static struct sdev_pcm_stream_info *sdrv_stream_get(
+	struct snd_pcm_substream *substream)
+{
+	struct sdev_pcm_instance_info *pcm_instance =
+		snd_pcm_substream_chip(substream);
+	struct sdev_pcm_stream_info *stream;
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		stream = &pcm_instance->streams_pb[substream->number];
+	else
+		stream = &pcm_instance->streams_cap[substream->number];
+	return stream;
+}
+
+static void sdrv_copy_pcm_hw(struct snd_pcm_hardware *dst,
+	struct snd_pcm_hardware *src,
+	struct snd_pcm_hardware *ref_pcm_hw)
+{
+	*dst = *ref_pcm_hw;
+
+	if (src->formats)
+		dst->formats = src->formats;
+	if (src->buffer_bytes_max)
+		dst->buffer_bytes_max =
+			src->buffer_bytes_max;
+	if (src->period_bytes_min)
+		dst->period_bytes_min =
+			src->period_bytes_min;
+	if (src->period_bytes_max)
+		dst->period_bytes_max =
+			src->period_bytes_max;
+	if (src->periods_min)
+		dst->periods_min = src->periods_min;
+	if (src->periods_max)
+		dst->periods_max = src->periods_max;
+	if (src->rates)
+		dst->rates = src->rates;
+	if (src->rate_min)
+		dst->rate_min = src->rate_min;
+	if (src->rate_max)
+		dst->rate_max = src->rate_max;
+	if (src->channels_min)
+		dst->channels_min = src->channels_min;
+	if (src->channels_max)
+		dst->channels_max = src->channels_max;
+	if (src->buffer_bytes_max) {
+		dst->buffer_bytes_max = src->buffer_bytes_max;
+		dst->period_bytes_max = src->buffer_bytes_max /
+			src->periods_max;
+		dst->periods_max = dst->buffer_bytes_max /
+			dst->period_bytes_max;
+	}
+}
+
+static int sdrv_alsa_open(struct snd_pcm_substream *substream)
+{
+	return 0;
+}
+
+static int sdrv_alsa_close(struct snd_pcm_substream *substream)
+{
+	return 0;
+}
+
+static int sdrv_alsa_hw_params(struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params)
+{
+	return 0;
+}
+
+static int sdrv_alsa_hw_free(struct snd_pcm_substream *substream)
+{
+	return 0;
+}
+
+static int sdrv_alsa_prepare(struct snd_pcm_substream *substream)
+{
+	return 0;
+}
+
+static int sdrv_alsa_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	return 0;
+}
+
+static inline snd_pcm_uframes_t sdrv_alsa_pointer(
+	struct snd_pcm_substream *substream)
+{
+	return 0;
+}
+
+static int sdrv_alsa_playback_copy_user(struct snd_pcm_substream *substream,
+		int channel, unsigned long pos, void __user *buf,
+		unsigned long bytes)
+{
+	return 0;
+}
+
+static int sdrv_alsa_playback_copy_kernel(struct snd_pcm_substream *substream,
+		int channel, unsigned long pos, void *buf, unsigned long bytes)
+{
+	return 0;
+}
+
+static int sdrv_alsa_capture_copy_user(struct snd_pcm_substream *substream,
+		int channel, unsigned long pos, void __user *buf,
+		unsigned long bytes)
+{
+	return 0;
+}
+
+static int sdrv_alsa_capture_copy_kernel(struct snd_pcm_substream *substream,
+		int channel, unsigned long pos, void *buf, unsigned long bytes)
+{
+	return 0;
+}
+
+static int sdrv_alsa_playback_fill_silence(struct snd_pcm_substream *substream,
+	int channel, unsigned long pos, unsigned long bytes)
+{
+	return 0;
+}
+
 #define MAX_XEN_BUFFER_SIZE	(64 * 1024)
 #define MAX_BUFFER_SIZE		MAX_XEN_BUFFER_SIZE
 #define MIN_PERIOD_SIZE		64
@@ -168,10 +291,119 @@ static struct snd_pcm_hardware sdrv_pcm_hw_default = {
 	.fifo_size = 0,
 };
 
+/*
+ * FIXME: The mmaped data transfer is asynchronous and there is no
+ * ack signal from user-space when it is done. This is the
+ * reason it is not implemented in the PV driver as we do need
+ * to know when the buffer can be transferred to the backend.
+ */
+
+static struct snd_pcm_ops sdrv_alsa_playback_ops = {
+	.open = sdrv_alsa_open,
+	.close = sdrv_alsa_close,
+	.ioctl = snd_pcm_lib_ioctl,
+	.hw_params = sdrv_alsa_hw_params,
+	.hw_free = sdrv_alsa_hw_free,
+	.prepare = sdrv_alsa_prepare,
+	.trigger = sdrv_alsa_trigger,
+	.pointer = sdrv_alsa_pointer,
+	.copy_user = sdrv_alsa_playback_copy_user,
+	.copy_kernel = sdrv_alsa_playback_copy_kernel,
+	.fill_silence = sdrv_alsa_playback_fill_silence,
+};
+
+static struct snd_pcm_ops sdrv_alsa_capture_ops = {
+	.open = sdrv_alsa_open,
+	.close = sdrv_alsa_close,
+	.ioctl = snd_pcm_lib_ioctl,
+	.hw_params = sdrv_alsa_hw_params,
+	.hw_free = sdrv_alsa_hw_free,
+	.prepare = sdrv_alsa_prepare,
+	.trigger = sdrv_alsa_trigger,
+	.pointer = sdrv_alsa_pointer,
+	.copy_user = sdrv_alsa_capture_copy_user,
+	.copy_kernel = sdrv_alsa_capture_copy_kernel,
+};
+
 static int sdrv_new_pcm(struct sdev_card_info *card_info,
 	struct cfg_pcm_instance *instance_config,
 	struct sdev_pcm_instance_info *pcm_instance_info)
 {
+	struct snd_pcm *pcm;
+	int ret, i;
+
+	dev_dbg(&card_info->xdrv_info->xb_dev->dev,
+		"New PCM device \"%s\" with id %d playback %d capture %d",
+		instance_config->name,
+		instance_config->device_id,
+		instance_config->num_streams_pb,
+		instance_config->num_streams_cap);
+
+	pcm_instance_info->card_info = card_info;
+
+	sdrv_copy_pcm_hw(&pcm_instance_info->pcm_hw,
+		&instance_config->pcm_hw, &card_info->pcm_hw);
+
+	if (instance_config->num_streams_pb) {
+		pcm_instance_info->streams_pb = devm_kcalloc(
+			&card_info->card->card_dev,
+			instance_config->num_streams_pb,
+			sizeof(struct sdev_pcm_stream_info),
+			GFP_KERNEL);
+		if (!pcm_instance_info->streams_pb)
+			return -ENOMEM;
+	}
+
+	if (instance_config->num_streams_cap) {
+		pcm_instance_info->streams_cap = devm_kcalloc(
+			&card_info->card->card_dev,
+			instance_config->num_streams_cap,
+			sizeof(struct sdev_pcm_stream_info),
+			GFP_KERNEL);
+		if (!pcm_instance_info->streams_cap)
+			return -ENOMEM;
+	}
+
+	pcm_instance_info->num_pcm_streams_pb =
+			instance_config->num_streams_pb;
+	pcm_instance_info->num_pcm_streams_cap =
+			instance_config->num_streams_cap;
+
+	for (i = 0; i < pcm_instance_info->num_pcm_streams_pb; i++) {
+		pcm_instance_info->streams_pb[i].pcm_hw =
+			instance_config->streams_pb[i].pcm_hw;
+		pcm_instance_info->streams_pb[i].unique_id =
+			instance_config->streams_pb[i].unique_id;
+	}
+
+	for (i = 0; i < pcm_instance_info->num_pcm_streams_cap; i++) {
+		pcm_instance_info->streams_cap[i].pcm_hw =
+			instance_config->streams_cap[i].pcm_hw;
+		pcm_instance_info->streams_cap[i].unique_id =
+			instance_config->streams_cap[i].unique_id;
+	}
+
+	ret = snd_pcm_new(card_info->card, instance_config->name,
+			instance_config->device_id,
+			instance_config->num_streams_pb,
+			instance_config->num_streams_cap,
+			&pcm);
+	if (ret < 0)
+		return ret;
+
+	pcm->private_data = pcm_instance_info;
+	pcm->info_flags = 0;
+	strncpy(pcm->name, "Virtual card PCM", sizeof(pcm->name));
+
+	if (instance_config->num_streams_pb)
+		snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK,
+				&sdrv_alsa_playback_ops);
+
+	if (instance_config->num_streams_cap)
+		snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE,
+				&sdrv_alsa_capture_ops);
+
+	pcm_instance_info->pcm = pcm;
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH RESEND1 09/12] ALSA: vsnd: Add timer for period interrupt emulation
  2017-08-07 12:22 [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Oleksandr Andrushchenko
                   ` (7 preceding siblings ...)
  2017-08-07 12:22 ` [PATCH RESEND1 08/12] ALSA: vsnd: Initialize virtul sound card Oleksandr Andrushchenko
@ 2017-08-07 12:22 ` Oleksandr Andrushchenko
  2017-08-07 12:22 ` [PATCH RESEND1 10/12] ALSA: vsnd: Implement ALSA PCM operations Oleksandr Andrushchenko
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-07 12:22 UTC (permalink / raw)
  To: alsa-devel, xen-devel, linux-kernel
  Cc: perex, tiwai, andr2000, Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Front sound driver has no real interrupts, so
playback/capture period passed interrupt needs to be emulated:
this is done via timer. Add required timer operations,
this is based on sound/drivers/dummy.c.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
 sound/drivers/xen-front.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 121 insertions(+)

diff --git a/sound/drivers/xen-front.c b/sound/drivers/xen-front.c
index 9f31e6832086..507c5eb343c8 100644
--- a/sound/drivers/xen-front.c
+++ b/sound/drivers/xen-front.c
@@ -67,12 +67,29 @@ struct sh_buf_info {
 	size_t vbuffer_sz;
 };
 
+struct sdev_alsa_timer_info {
+	spinlock_t lock;
+	struct timer_list timer;
+	unsigned long base_time;
+	/* fractional sample position (based HZ) */
+	unsigned int frac_pos;
+	unsigned int frac_period_rest;
+	/* buffer_size * HZ */
+	unsigned int frac_buffer_size;
+	/* period_size * HZ */
+	unsigned int frac_period_size;
+	unsigned int rate;
+	int elapsed;
+	struct snd_pcm_substream *substream;
+};
+
 struct sdev_pcm_stream_info {
 	int unique_id;
 	struct snd_pcm_hardware pcm_hw;
 	struct xdrv_evtchnl_info *evt_chnl;
 	bool is_open;
 	uint8_t req_next_id;
+	struct sdev_alsa_timer_info timer;
 	struct sh_buf_info sh_buf;
 };
 
@@ -148,6 +165,110 @@ static struct sdev_pcm_stream_info *sdrv_stream_get(
 	return stream;
 }
 
+static inline void sdrv_alsa_timer_rearm(struct sdev_alsa_timer_info *dpcm)
+{
+	mod_timer(&dpcm->timer, jiffies +
+		(dpcm->frac_period_rest + dpcm->rate - 1) / dpcm->rate);
+}
+
+static void sdrv_alsa_timer_update(struct sdev_alsa_timer_info *dpcm)
+{
+	unsigned long delta;
+
+	delta = jiffies - dpcm->base_time;
+	if (!delta)
+		return;
+	dpcm->base_time += delta;
+	delta *= dpcm->rate;
+	dpcm->frac_pos += delta;
+	while (dpcm->frac_pos >= dpcm->frac_buffer_size)
+		dpcm->frac_pos -= dpcm->frac_buffer_size;
+	while (dpcm->frac_period_rest <= delta) {
+		dpcm->elapsed++;
+		dpcm->frac_period_rest += dpcm->frac_period_size;
+	}
+	dpcm->frac_period_rest -= delta;
+}
+
+static int sdrv_alsa_timer_start(struct snd_pcm_substream *substream)
+{
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+	struct sdev_alsa_timer_info *dpcm = &stream->timer;
+
+	spin_lock(&dpcm->lock);
+	dpcm->base_time = jiffies;
+	sdrv_alsa_timer_rearm(dpcm);
+	spin_unlock(&dpcm->lock);
+	return 0;
+}
+
+static int sdrv_alsa_timer_stop(struct snd_pcm_substream *substream)
+{
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+	struct sdev_alsa_timer_info *dpcm = &stream->timer;
+
+	spin_lock(&dpcm->lock);
+	del_timer(&dpcm->timer);
+	spin_unlock(&dpcm->lock);
+	return 0;
+}
+
+static int sdrv_alsa_timer_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+	struct sdev_alsa_timer_info *dpcm = &stream->timer;
+
+	dpcm->frac_pos = 0;
+	dpcm->rate = runtime->rate;
+	dpcm->frac_buffer_size = runtime->buffer_size * HZ;
+	dpcm->frac_period_size = runtime->period_size * HZ;
+	dpcm->frac_period_rest = dpcm->frac_period_size;
+	dpcm->elapsed = 0;
+	return 0;
+}
+
+static void sdrv_alsa_timer_callback(unsigned long data)
+{
+	struct sdev_alsa_timer_info *dpcm = (struct sdev_alsa_timer_info *)data;
+	int elapsed;
+
+	spin_lock(&dpcm->lock);
+	sdrv_alsa_timer_update(dpcm);
+	sdrv_alsa_timer_rearm(dpcm);
+	elapsed = dpcm->elapsed;
+	dpcm->elapsed = 0;
+	spin_unlock(&dpcm->lock);
+	if (elapsed)
+		snd_pcm_period_elapsed(dpcm->substream);
+}
+
+static snd_pcm_uframes_t sdrv_alsa_timer_pointer(
+	struct snd_pcm_substream *substream)
+{
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+	struct sdev_alsa_timer_info *dpcm = &stream->timer;
+	snd_pcm_uframes_t pos;
+
+	spin_lock(&dpcm->lock);
+	sdrv_alsa_timer_update(dpcm);
+	pos = dpcm->frac_pos / HZ;
+	spin_unlock(&dpcm->lock);
+	return pos;
+}
+
+static int sdrv_alsa_timer_create(struct snd_pcm_substream *substream)
+{
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+	struct sdev_alsa_timer_info *dpcm = &stream->timer;
+
+	spin_lock_init(&dpcm->lock);
+	dpcm->substream = substream;
+	setup_timer(&dpcm->timer, sdrv_alsa_timer_callback,
+		(unsigned long) dpcm);
+	return 0;
+}
+
 static void sdrv_copy_pcm_hw(struct snd_pcm_hardware *dst,
 	struct snd_pcm_hardware *src,
 	struct snd_pcm_hardware *ref_pcm_hw)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH RESEND1 10/12] ALSA: vsnd: Implement ALSA PCM operations
  2017-08-07 12:22 [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Oleksandr Andrushchenko
                   ` (8 preceding siblings ...)
  2017-08-07 12:22 ` [PATCH RESEND1 09/12] ALSA: vsnd: Add timer for period interrupt emulation Oleksandr Andrushchenko
@ 2017-08-07 12:22 ` Oleksandr Andrushchenko
  2017-08-07 12:22 ` [PATCH RESEND1 11/12] ALSA: vsnd: Implement communication with backend Oleksandr Andrushchenko
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-07 12:22 UTC (permalink / raw)
  To: alsa-devel, xen-devel, linux-kernel
  Cc: perex, tiwai, andr2000, Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Implement ALSA driver operations including:
- start/stop period interrupt emulation
- manage frontend/backend shraed buffers
- manage Xen bus event channel state

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
 sound/drivers/xen-front.c | 175 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 163 insertions(+), 12 deletions(-)

diff --git a/sound/drivers/xen-front.c b/sound/drivers/xen-front.c
index 507c5eb343c8..7275e9cb38c0 100644
--- a/sound/drivers/xen-front.c
+++ b/sound/drivers/xen-front.c
@@ -151,6 +151,11 @@ struct xdrv_info {
 	struct sdev_card_plat_data cfg_plat_data;
 };
 
+static inline void sh_buf_clear(struct sh_buf_info *buf);
+static void sh_buf_free(struct sh_buf_info *buf);
+static int sh_buf_alloc(struct xenbus_device *xb_dev, struct sh_buf_info *buf,
+	unsigned int buffer_size);
+
 static struct sdev_pcm_stream_info *sdrv_stream_get(
 	struct snd_pcm_substream *substream)
 {
@@ -311,71 +316,217 @@ static void sdrv_copy_pcm_hw(struct snd_pcm_hardware *dst,
 
 static int sdrv_alsa_open(struct snd_pcm_substream *substream)
 {
-	return 0;
+	struct sdev_pcm_instance_info *pcm_instance =
+		snd_pcm_substream_chip(substream);
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct xdrv_info *xdrv_info;
+	unsigned long flags;
+	int ret;
+
+	sdrv_copy_pcm_hw(&runtime->hw, &stream->pcm_hw, &pcm_instance->pcm_hw);
+	runtime->hw.info &= ~(SNDRV_PCM_INFO_MMAP |
+		SNDRV_PCM_INFO_MMAP_VALID |
+		SNDRV_PCM_INFO_DOUBLE |
+		SNDRV_PCM_INFO_BATCH |
+		SNDRV_PCM_INFO_NONINTERLEAVED |
+		SNDRV_PCM_INFO_RESUME |
+		SNDRV_PCM_INFO_PAUSE);
+	runtime->hw.info |= SNDRV_PCM_INFO_INTERLEAVED;
+
+	xdrv_info = pcm_instance->card_info->xdrv_info;
+
+	ret = sdrv_alsa_timer_create(substream);
+
+	spin_lock_irqsave(&xdrv_info->io_lock, flags);
+	sh_buf_clear(&stream->sh_buf);
+	stream->evt_chnl = &xdrv_info->evt_chnls[stream->unique_id];
+	if (ret < 0)
+		stream->evt_chnl->state = EVTCHNL_STATE_DISCONNECTED;
+	else
+		stream->evt_chnl->state = EVTCHNL_STATE_CONNECTED;
+	spin_unlock_irqrestore(&xdrv_info->io_lock, flags);
+	return ret;
 }
 
 static int sdrv_alsa_close(struct snd_pcm_substream *substream)
 {
+	struct sdev_pcm_instance_info *pcm_instance =
+		snd_pcm_substream_chip(substream);
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+	struct xdrv_info *xdrv_info;
+	unsigned long flags;
+
+	xdrv_info = pcm_instance->card_info->xdrv_info;
+
+	sdrv_alsa_timer_stop(substream);
+
+	spin_lock_irqsave(&xdrv_info->io_lock, flags);
+	stream->evt_chnl->state = EVTCHNL_STATE_DISCONNECTED;
+	spin_unlock_irqrestore(&xdrv_info->io_lock, flags);
 	return 0;
 }
 
 static int sdrv_alsa_hw_params(struct snd_pcm_substream *substream,
 	struct snd_pcm_hw_params *params)
 {
+	struct sdev_pcm_instance_info *pcm_instance =
+		snd_pcm_substream_chip(substream);
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+	struct xdrv_info *xdrv_info;
+	unsigned int buffer_size;
+	int ret;
+
+	buffer_size = params_buffer_bytes(params);
+	sh_buf_clear(&stream->sh_buf);
+	xdrv_info = pcm_instance->card_info->xdrv_info;
+	ret = sh_buf_alloc(xdrv_info->xb_dev,
+		&stream->sh_buf, buffer_size);
+	if (ret < 0)
+		goto fail;
 	return 0;
+
+fail:
+	dev_err(&xdrv_info->xb_dev->dev,
+		"Failed to allocate buffers for stream idx %d\n",
+		stream->unique_id);
+	return ret;
 }
 
 static int sdrv_alsa_hw_free(struct snd_pcm_substream *substream)
 {
+	struct sdev_pcm_instance_info *pcm_instance =
+		snd_pcm_substream_chip(substream);
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+	struct xdrv_info *xdrv_info;
+	unsigned long flags;
+
+	xdrv_info = pcm_instance->card_info->xdrv_info;
+	spin_lock_irqsave(&xdrv_info->io_lock, flags);
+	sh_buf_free(&stream->sh_buf);
+	spin_unlock_irqrestore(&xdrv_info->io_lock, flags);
 	return 0;
 }
 
 static int sdrv_alsa_prepare(struct snd_pcm_substream *substream)
 {
-	return 0;
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+	int ret = 0;
+
+	if (!stream->is_open)
+		ret = sdrv_alsa_timer_prepare(substream);
+	return ret;
 }
 
 static int sdrv_alsa_trigger(struct snd_pcm_substream *substream, int cmd)
 {
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		/* fall through */
+	case SNDRV_PCM_TRIGGER_RESUME:
+		return sdrv_alsa_timer_start(substream);
+
+	case SNDRV_PCM_TRIGGER_STOP:
+		/* fall through */
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+		return sdrv_alsa_timer_stop(substream);
+
+	default:
+		break;
+	}
 	return 0;
 }
 
 static inline snd_pcm_uframes_t sdrv_alsa_pointer(
 	struct snd_pcm_substream *substream)
 {
+	return sdrv_alsa_timer_pointer(substream);
+}
+
+static int sdrv_alsa_playback_do_write(struct snd_pcm_substream *substream,
+	unsigned long pos, unsigned long count)
+{
 	return 0;
 }
 
 static int sdrv_alsa_playback_copy_user(struct snd_pcm_substream *substream,
-		int channel, unsigned long pos, void __user *buf,
-		unsigned long bytes)
+		int channel, unsigned long pos, void __user *src,
+		unsigned long count)
 {
-	return 0;
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+
+	if (unlikely(pos + count > stream->sh_buf.vbuffer_sz))
+		return -EINVAL;
+
+	if (copy_from_user(stream->sh_buf.vbuffer + pos, src, count))
+		return -EFAULT;
+
+	return sdrv_alsa_playback_do_write(substream, pos, count);
 }
 
 static int sdrv_alsa_playback_copy_kernel(struct snd_pcm_substream *substream,
-		int channel, unsigned long pos, void *buf, unsigned long bytes)
+		int channel, unsigned long pos, void *src, unsigned long count)
+{
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+
+	if (unlikely(pos + count > stream->sh_buf.vbuffer_sz))
+		return -EINVAL;
+
+	memcpy(stream->sh_buf.vbuffer + pos, src, count);
+	return sdrv_alsa_playback_do_write(substream, pos, count);
+}
+
+static int sdrv_alsa_playback_do_read(struct snd_pcm_substream *substream,
+	unsigned long pos, unsigned long count)
 {
 	return 0;
 }
 
 static int sdrv_alsa_capture_copy_user(struct snd_pcm_substream *substream,
-		int channel, unsigned long pos, void __user *buf,
-		unsigned long bytes)
+		int channel, unsigned long pos, void __user *dst,
+		unsigned long count)
 {
-	return 0;
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+	int ret;
+
+	if (unlikely(pos + count > stream->sh_buf.vbuffer_sz))
+		return -EINVAL;
+
+	ret = sdrv_alsa_playback_do_read(substream, pos, count);
+	if (ret < 0)
+		return ret;
+
+	return copy_to_user(dst, stream->sh_buf.vbuffer + pos, count) ?
+		-EFAULT : 0;
 }
 
 static int sdrv_alsa_capture_copy_kernel(struct snd_pcm_substream *substream,
-		int channel, unsigned long pos, void *buf, unsigned long bytes)
+		int channel, unsigned long pos, void *dst, unsigned long count)
 {
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+	int ret;
+
+	if (unlikely(pos + count > stream->sh_buf.vbuffer_sz))
+		return -EINVAL;
+
+	ret = sdrv_alsa_playback_do_read(substream, pos, count);
+	if (ret < 0)
+		return ret;
+
+	memcpy(dst, stream->sh_buf.vbuffer + pos, count);
 	return 0;
 }
 
 static int sdrv_alsa_playback_fill_silence(struct snd_pcm_substream *substream,
-	int channel, unsigned long pos, unsigned long bytes)
+	int channel, unsigned long pos, unsigned long count)
 {
-	return 0;
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+
+	if (unlikely(pos + count > stream->sh_buf.vbuffer_sz))
+		return -EINVAL;
+
+	memset(stream->sh_buf.vbuffer + pos, 0, count);
+	return sdrv_alsa_playback_do_write(substream, pos, count);
 }
 
 #define MAX_XEN_BUFFER_SIZE	(64 * 1024)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH RESEND1 11/12] ALSA: vsnd: Implement communication with backend
  2017-08-07 12:22 [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Oleksandr Andrushchenko
                   ` (9 preceding siblings ...)
  2017-08-07 12:22 ` [PATCH RESEND1 10/12] ALSA: vsnd: Implement ALSA PCM operations Oleksandr Andrushchenko
@ 2017-08-07 12:22 ` Oleksandr Andrushchenko
  2017-08-07 12:22 ` [PATCH RESEND1 12/12] ALSA: vsnd: Introduce Kconfig option to enable Xen PV sound Oleksandr Andrushchenko
  2017-08-10  3:14 ` [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Takashi Sakamoto
  12 siblings, 0 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-07 12:22 UTC (permalink / raw)
  To: alsa-devel, xen-devel, linux-kernel
  Cc: perex, tiwai, andr2000, Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Implement frontend to backend communication according to
the para-virtualized sound protocol: xen/interface/io/sndif.h.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
 sound/drivers/xen-front.c | 302 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 288 insertions(+), 14 deletions(-)

diff --git a/sound/drivers/xen-front.c b/sound/drivers/xen-front.c
index 7275e9cb38c0..8bfec43ef03a 100644
--- a/sound/drivers/xen-front.c
+++ b/sound/drivers/xen-front.c
@@ -38,6 +38,8 @@
  * because of the fact it is already in use/reserved by the PV console.
  */
 #define GRANT_INVALID_REF	0
+/* timeout in ms to wait for backend to respond */
+#define VSND_WAIT_BACK_MS	3000
 /* maximum number of supported streams */
 #define VSND_MAX_STREAM		8
 
@@ -151,10 +153,12 @@ struct xdrv_info {
 	struct sdev_card_plat_data cfg_plat_data;
 };
 
+static inline void xdrv_evtchnl_flush(struct xdrv_evtchnl_info *channel);
 static inline void sh_buf_clear(struct sh_buf_info *buf);
 static void sh_buf_free(struct sh_buf_info *buf);
 static int sh_buf_alloc(struct xenbus_device *xb_dev, struct sh_buf_info *buf,
 	unsigned int buffer_size);
+static grant_ref_t sh_buf_get_dir_start(struct sh_buf_info *buf);
 
 static struct sdev_pcm_stream_info *sdrv_stream_get(
 	struct snd_pcm_substream *substream)
@@ -314,6 +318,234 @@ static void sdrv_copy_pcm_hw(struct snd_pcm_hardware *dst,
 	}
 }
 
+struct ALSA_SNDIF_SAMPLE_FORMAT {
+	uint8_t sndif;
+	snd_pcm_format_t alsa;
+};
+
+static struct ALSA_SNDIF_SAMPLE_FORMAT alsa_sndif_formats[] = {
+	{
+		.sndif = XENSND_PCM_FORMAT_U8,
+		.alsa = SNDRV_PCM_FORMAT_U8
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_S8,
+		.alsa = SNDRV_PCM_FORMAT_S8
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_U16_LE,
+		.alsa = SNDRV_PCM_FORMAT_U16_LE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_U16_BE,
+		.alsa = SNDRV_PCM_FORMAT_U16_BE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_S16_LE,
+		.alsa = SNDRV_PCM_FORMAT_S16_LE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_S16_BE,
+		.alsa = SNDRV_PCM_FORMAT_S16_BE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_U24_LE,
+		.alsa = SNDRV_PCM_FORMAT_U24_LE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_U24_BE,
+		.alsa = SNDRV_PCM_FORMAT_U24_BE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_S24_LE,
+		.alsa = SNDRV_PCM_FORMAT_S24_LE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_S24_BE,
+		.alsa = SNDRV_PCM_FORMAT_S24_BE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_U32_LE,
+		.alsa = SNDRV_PCM_FORMAT_U32_LE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_U32_BE,
+		.alsa = SNDRV_PCM_FORMAT_U32_BE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_S32_LE,
+		.alsa = SNDRV_PCM_FORMAT_S32_LE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_S32_BE,
+		.alsa = SNDRV_PCM_FORMAT_S32_BE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_A_LAW,
+		.alsa = SNDRV_PCM_FORMAT_A_LAW
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_MU_LAW,
+		.alsa = SNDRV_PCM_FORMAT_MU_LAW
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_F32_LE,
+		.alsa = SNDRV_PCM_FORMAT_FLOAT_LE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_F32_BE,
+		.alsa = SNDRV_PCM_FORMAT_FLOAT_BE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_F64_LE,
+		.alsa = SNDRV_PCM_FORMAT_FLOAT64_LE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_F64_BE,
+		.alsa = SNDRV_PCM_FORMAT_FLOAT64_BE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_IEC958_SUBFRAME_LE,
+		.alsa = SNDRV_PCM_FORMAT_IEC958_SUBFRAME_LE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_IEC958_SUBFRAME_BE,
+		.alsa = SNDRV_PCM_FORMAT_IEC958_SUBFRAME_BE
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_IMA_ADPCM,
+		.alsa = SNDRV_PCM_FORMAT_IMA_ADPCM
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_MPEG,
+		.alsa = SNDRV_PCM_FORMAT_MPEG
+	},
+	{
+		.sndif = XENSND_PCM_FORMAT_GSM,
+		.alsa = SNDRV_PCM_FORMAT_GSM
+	},
+};
+
+static int alsa_to_sndif_format(snd_pcm_format_t format)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(alsa_sndif_formats); i++)
+		if (alsa_sndif_formats[i].alsa == format)
+			return alsa_sndif_formats[i].sndif;
+	return -EINVAL;
+}
+
+static void sdrv_stream_clear(struct sdev_pcm_stream_info *stream)
+{
+	stream->is_open = false;
+	stream->req_next_id = 0;
+	sh_buf_clear(&stream->sh_buf);
+}
+
+static struct xensnd_req *sdrv_be_stream_prepare_req(
+	struct sdev_pcm_stream_info *stream, uint8_t operation)
+{
+	struct xensnd_req *req;
+
+	req = RING_GET_REQUEST(&stream->evt_chnl->ring,
+		stream->evt_chnl->ring.req_prod_pvt);
+	req->operation = operation;
+	req->id = stream->req_next_id++;
+	stream->evt_chnl->resp_id = req->id;
+	return req;
+}
+
+static void sdrv_be_stream_free(struct sdev_pcm_stream_info *stream)
+{
+	sh_buf_free(&stream->sh_buf);
+	sdrv_stream_clear(stream);
+}
+
+static int sdrv_be_stream_do_io(struct xdrv_evtchnl_info *evtchnl)
+{
+	if (unlikely(evtchnl->state != EVTCHNL_STATE_CONNECTED))
+		return -EIO;
+
+	reinit_completion(&evtchnl->completion);
+	xdrv_evtchnl_flush(evtchnl);
+	return 0;
+}
+
+static inline int sdrv_be_stream_wait_io(struct xdrv_evtchnl_info *evtchnl)
+{
+	if (wait_for_completion_timeout(
+			&evtchnl->completion,
+			msecs_to_jiffies(VSND_WAIT_BACK_MS)) <= 0)
+		return -ETIMEDOUT;
+	return 0;
+}
+
+static int sdrv_be_stream_open(struct snd_pcm_substream *substream,
+	struct sdev_pcm_stream_info *stream)
+{
+	struct sdev_pcm_instance_info *pcm_instance =
+		snd_pcm_substream_chip(substream);
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct xdrv_info *xdrv_info;
+	struct xensnd_req *req;
+	unsigned long flags;
+	int ret;
+
+	xdrv_info = pcm_instance->card_info->xdrv_info;
+
+	ret = alsa_to_sndif_format(runtime->format);
+	if (ret < 0) {
+		dev_err(&xdrv_info->xb_dev->dev,
+			"Unsupported sample format: %d\n", runtime->format);
+		return ret;
+	}
+
+	spin_lock_irqsave(&xdrv_info->io_lock, flags);
+	stream->is_open = false;
+	req = sdrv_be_stream_prepare_req(stream, XENSND_OP_OPEN);
+	req->op.open.pcm_format = (uint8_t)ret;
+	req->op.open.pcm_channels = runtime->channels;
+	req->op.open.pcm_rate = runtime->rate;
+	req->op.open.buffer_sz = stream->sh_buf.vbuffer_sz;
+	req->op.open.gref_directory = sh_buf_get_dir_start(&stream->sh_buf);
+
+	ret = sdrv_be_stream_do_io(stream->evt_chnl);
+	spin_unlock_irqrestore(&xdrv_info->io_lock, flags);
+
+	if (ret < 0)
+		return ret;
+
+	ret = sdrv_be_stream_wait_io(stream->evt_chnl);
+	stream->is_open = ret < 0 ? false : true;
+	return ret;
+}
+
+static int sdrv_be_stream_close(struct snd_pcm_substream *substream,
+	struct sdev_pcm_stream_info *stream)
+{
+	struct sdev_pcm_instance_info *pcm_instance =
+		snd_pcm_substream_chip(substream);
+	struct xdrv_info *xdrv_info;
+	struct xensnd_req *req;
+	unsigned long flags;
+	int ret;
+
+	xdrv_info = pcm_instance->card_info->xdrv_info;
+
+	spin_lock_irqsave(&xdrv_info->io_lock, flags);
+	stream->is_open = false;
+	req = sdrv_be_stream_prepare_req(stream, XENSND_OP_CLOSE);
+
+	ret = sdrv_be_stream_do_io(stream->evt_chnl);
+	spin_unlock_irqrestore(&xdrv_info->io_lock, flags);
+
+	if (ret < 0)
+		return ret;
+
+	return sdrv_be_stream_wait_io(stream->evt_chnl);
+}
+
 static int sdrv_alsa_open(struct snd_pcm_substream *substream)
 {
 	struct sdev_pcm_instance_info *pcm_instance =
@@ -339,7 +571,7 @@ static int sdrv_alsa_open(struct snd_pcm_substream *substream)
 	ret = sdrv_alsa_timer_create(substream);
 
 	spin_lock_irqsave(&xdrv_info->io_lock, flags);
-	sh_buf_clear(&stream->sh_buf);
+	sdrv_stream_clear(stream);
 	stream->evt_chnl = &xdrv_info->evt_chnls[stream->unique_id];
 	if (ret < 0)
 		stream->evt_chnl->state = EVTCHNL_STATE_DISCONNECTED;
@@ -378,7 +610,7 @@ static int sdrv_alsa_hw_params(struct snd_pcm_substream *substream,
 	int ret;
 
 	buffer_size = params_buffer_bytes(params);
-	sh_buf_clear(&stream->sh_buf);
+	sdrv_stream_clear(stream);
 	xdrv_info = pcm_instance->card_info->xdrv_info;
 	ret = sh_buf_alloc(xdrv_info->xb_dev,
 		&stream->sh_buf, buffer_size);
@@ -390,22 +622,18 @@ static int sdrv_alsa_hw_params(struct snd_pcm_substream *substream,
 	dev_err(&xdrv_info->xb_dev->dev,
 		"Failed to allocate buffers for stream idx %d\n",
 		stream->unique_id);
+	sdrv_be_stream_free(stream);
 	return ret;
 }
 
 static int sdrv_alsa_hw_free(struct snd_pcm_substream *substream)
 {
-	struct sdev_pcm_instance_info *pcm_instance =
-		snd_pcm_substream_chip(substream);
 	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
-	struct xdrv_info *xdrv_info;
-	unsigned long flags;
+	int ret;
 
-	xdrv_info = pcm_instance->card_info->xdrv_info;
-	spin_lock_irqsave(&xdrv_info->io_lock, flags);
-	sh_buf_free(&stream->sh_buf);
-	spin_unlock_irqrestore(&xdrv_info->io_lock, flags);
-	return 0;
+	ret = sdrv_be_stream_close(substream, stream);
+	sdrv_be_stream_free(stream);
+	return ret;
 }
 
 static int sdrv_alsa_prepare(struct snd_pcm_substream *substream)
@@ -413,8 +641,12 @@ static int sdrv_alsa_prepare(struct snd_pcm_substream *substream)
 	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
 	int ret = 0;
 
-	if (!stream->is_open)
+	if (!stream->is_open) {
+		ret = sdrv_be_stream_open(substream, stream);
+		if (ret < 0)
+			return ret;
 		ret = sdrv_alsa_timer_prepare(substream);
+	}
 	return ret;
 }
 
@@ -446,7 +678,28 @@ static inline snd_pcm_uframes_t sdrv_alsa_pointer(
 static int sdrv_alsa_playback_do_write(struct snd_pcm_substream *substream,
 	unsigned long pos, unsigned long count)
 {
-	return 0;
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+	struct sdev_pcm_instance_info *pcm_instance =
+		snd_pcm_substream_chip(substream);
+	struct xdrv_info *xdrv_info;
+	struct xensnd_req *req;
+	unsigned long flags;
+	int ret;
+
+	xdrv_info = pcm_instance->card_info->xdrv_info;
+
+	spin_lock_irqsave(&xdrv_info->io_lock, flags);
+	req = sdrv_be_stream_prepare_req(stream, XENSND_OP_WRITE);
+	req->op.rw.length = count;
+	req->op.rw.offset = pos;
+
+	ret = sdrv_be_stream_do_io(stream->evt_chnl);
+	spin_unlock_irqrestore(&xdrv_info->io_lock, flags);
+
+	if (ret < 0)
+		return ret;
+
+	return sdrv_be_stream_wait_io(stream->evt_chnl);
 }
 
 static int sdrv_alsa_playback_copy_user(struct snd_pcm_substream *substream,
@@ -479,7 +732,28 @@ static int sdrv_alsa_playback_copy_kernel(struct snd_pcm_substream *substream,
 static int sdrv_alsa_playback_do_read(struct snd_pcm_substream *substream,
 	unsigned long pos, unsigned long count)
 {
-	return 0;
+	struct sdev_pcm_stream_info *stream = sdrv_stream_get(substream);
+	struct sdev_pcm_instance_info *pcm_instance =
+		snd_pcm_substream_chip(substream);
+	struct xdrv_info *xdrv_info;
+	struct xensnd_req *req;
+	unsigned long flags;
+	int ret;
+
+	xdrv_info = pcm_instance->card_info->xdrv_info;
+
+	spin_lock_irqsave(&xdrv_info->io_lock, flags);
+	req = sdrv_be_stream_prepare_req(stream, XENSND_OP_READ);
+	req->op.rw.length = count;
+	req->op.rw.offset = pos;
+
+	ret = sdrv_be_stream_do_io(stream->evt_chnl);
+	spin_unlock_irqrestore(&xdrv_info->io_lock, flags);
+
+	if (ret < 0)
+		return ret;
+
+	return sdrv_be_stream_wait_io(stream->evt_chnl);
 }
 
 static int sdrv_alsa_capture_copy_user(struct snd_pcm_substream *substream,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH RESEND1 12/12] ALSA: vsnd: Introduce Kconfig option to enable Xen PV sound
  2017-08-07 12:22 [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Oleksandr Andrushchenko
                   ` (10 preceding siblings ...)
  2017-08-07 12:22 ` [PATCH RESEND1 11/12] ALSA: vsnd: Implement communication with backend Oleksandr Andrushchenko
@ 2017-08-07 12:22 ` Oleksandr Andrushchenko
  2017-08-10  3:14 ` [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Takashi Sakamoto
  12 siblings, 0 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-07 12:22 UTC (permalink / raw)
  To: alsa-devel, xen-devel, linux-kernel
  Cc: perex, tiwai, andr2000, Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Introduce Kconfig option to enable Xen para-virtualized sound
frontend driver. Also add sound frontend to the Makefile.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
 sound/drivers/Kconfig  | 12 ++++++++++++
 sound/drivers/Makefile |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/sound/drivers/Kconfig b/sound/drivers/Kconfig
index 7144cc36e8ae..6b8fa6110ca3 100644
--- a/sound/drivers/Kconfig
+++ b/sound/drivers/Kconfig
@@ -235,4 +235,16 @@ config SND_AC97_POWER_SAVE_DEFAULT
 
 	  See SND_AC97_POWER_SAVE for more details.
 
+config SND_XEN_FRONTEND
+	tristate "Xen virtual sound front-end driver"
+	depends on SND_PCM && XEN
+	select XEN_XENBUS_FRONTEND
+	default n
+	help
+	  This driver implements a front-end for the Xen
+	  para-virtualized sound.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called snd-xen-front.
+
 endif	# SND_DRIVERS
diff --git a/sound/drivers/Makefile b/sound/drivers/Makefile
index 1a8440c8b138..70ed920a030f 100644
--- a/sound/drivers/Makefile
+++ b/sound/drivers/Makefile
@@ -11,6 +11,7 @@ snd-portman2x4-objs := portman2x4.o
 snd-serial-u16550-objs := serial-u16550.o
 snd-virmidi-objs := virmidi.o
 snd-ml403-ac97cr-objs := ml403-ac97cr.o pcm-indirect2.o
+snd-xen-front-objs := xen-front.o
 
 # Toplevel Module Dependency
 obj-$(CONFIG_SND_DUMMY) += snd-dummy.o
@@ -21,5 +22,6 @@ obj-$(CONFIG_SND_MTPAV) += snd-mtpav.o
 obj-$(CONFIG_SND_MTS64) += snd-mts64.o
 obj-$(CONFIG_SND_PORTMAN2X4) += snd-portman2x4.o
 obj-$(CONFIG_SND_ML403_AC97CR) += snd-ml403-ac97cr.o
+obj-$(CONFIG_SND_XEN_FRONTEND) += snd-xen-front.o
 
 obj-$(CONFIG_SND) += opl3/ opl4/ mpu401/ vx/ pcsp/
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-08-07 12:22 [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Oleksandr Andrushchenko
                   ` (11 preceding siblings ...)
  2017-08-07 12:22 ` [PATCH RESEND1 12/12] ALSA: vsnd: Introduce Kconfig option to enable Xen PV sound Oleksandr Andrushchenko
@ 2017-08-10  3:14 ` Takashi Sakamoto
  2017-08-10  8:10   ` Oleksandr Andrushchenko
  12 siblings, 1 reply; 34+ messages in thread
From: Takashi Sakamoto @ 2017-08-10  3:14 UTC (permalink / raw)
  To: Oleksandr Andrushchenko, alsa-devel, xen-devel, linux-kernel
  Cc: Oleksandr Andrushchenko, tiwai

Hi,

On Aug 7 2017 21:22, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> 
> This patch series adds support for Xen [1] para-virtualized
> sound frontend driver. It implements the protocol from
> include/xen/interface/io/sndif.h with the following limitations:
> - mute/unmute is not supported
> - get/set volume is not supported
> Volume control is not supported for the reason that most of the
> use-cases (at the moment) are based on scenarious where
> unprivileged OS (e.g. Android, AGL etc) use software mixers.
> 
> Both capture and playback are supported.
> 
> Thank you,
> Oleksandr
> 
> Resending because of rebase onto [2] + added missing patch
> 
> [1] https://xenproject.org/
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/log/?h=for-next
> 
> Oleksandr Andrushchenko (12):
>    ALSA: vsnd: Introduce Xen para-virtualized sound frontend driver
>    ALSA: vsnd: Implement driver's probe/remove
>    ALSA: vsnd: Implement Xen bus state handling
>    ALSA: vsnd: Read sound driver configuration from Xen store
>    ALSA: vsnd: Implement Xen event channel handling
>    ALSA: vsnd: Implement handling of shared buffers
>    ALSA: vsnd: Introduce ALSA virtual sound driver
>    ALSA: vsnd: Initialize virtul sound card
>    ALSA: vsnd: Add timer for period interrupt emulation
>    ALSA: vsnd: Implement ALSA PCM operations
>    ALSA: vsnd: Implement communication with backend
>    ALSA: vsnd: Introduce Kconfig option to enable Xen PV sound
> 
>   sound/drivers/Kconfig     |   12 +
>   sound/drivers/Makefile    |    2 +
>   sound/drivers/xen-front.c | 2107 +++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 2121 insertions(+)
>   create mode 100644 sound/drivers/xen-front.c

For this patchset, I have the same concern which Clemens Ladisch
denoted[1]. If I can understand your explanation about queueing between
Dom0/DomU stuffs, the concern can be described in short words; this
driver works without any synchronization to data transmission by actual
sound hardwares.

In design of ALSA PCM core, drivers are expected to synchronize to
actual hardwares for semi-realtime data transmission. The
synchronization is done by two points:
1) Interrupts to respond events from actual hardwares.
2) Positions of actual data transmission in any serial sound interfaces
    of actual hardwares.

These two points comes from typical designs of actual hardwares, thus
they doesn't come from unfair, unreasonable, intrusive demands from
software side.

In design of typical stuffs on para-virtualization, Dom0 stuffs are hard
to give enough abstraction of sound hardwares in these two points for
DomU stuffs. Especially, it cannot abstract point 2) at all because the
value of position should be accurate against actual time frame, while
there's an overhead for DomU stuffs to read it. When DomU stuffs handles
the value, the value is enough past due to context switches between
Dom0/DomU. Therefore, this driver must rely on point 1) to synchronize
to actual sound hardwares. Typically, drivers configure hardwares to
generate interrupts per period of PCM buffer. This means that this
driver should notify to Dom0 about the value of period size requested
by applications.

In 'include/xen/interface/io/sndif.h', there's no functionalities I
described the above:
1. notifications from DomU to Dom0 about the size of period for
    interrupts from actual hardwares. Or no way from Dom0 to DomU about
    the configured size of the period.
2. notifications of the interrupts from actual hardwares to DomU.

For the reasons, your driver used kernel's timer interface to generate
'pseudo' interrupts for the purpose. However, it depends on Dom0's
abstraction different from sound hardwares and Linux kernel's
abstraction for timer functionality. In this case, gap between 'actual'
interrupts from hardware and the 'pseudo' interrupts from a combination
of several components brings unexpected result on several situations.

I think this is defects of 'sndif' interface in Xen side. I think it
better for you to work in Xen community to improve the above interface
at first, then work for Linux stuffs.


Additionally, in next time, please remind of several points below:
  * When a first patch adds an initial code for drivers, it should
    include entries for Makefile and Kconfig, so that the driver can be
    built even if it's still in an initial shape. Each patch should be
    self-contained and should be in a shape so that developers easily run
    bisecting. In other words, your first patch[2] includes modification
   for Makefile and Kconfig in your last patch[3].
  * When any read-only symbols is added,  it should have 'const'
    qualifier so that the symbol places to .rodata section of ELF
    binaries. For example, in your code, 'alsa_sndif_formats' is such an
    symbol. In recent Linux development, some developers work for
    constifying such symbols. Please remind of their continuous works in
    upstream[4].
  * You can split your driver to several files. In
    'include/xen/interface/io/sndif.h', Dom0 produces functionalities for
    DomU to control gain/volume/mute and in future your driver may get
    more codes. If split to several files make it readable, it should be
    done.
  * In my taste, a prefix of the subject line should be 'xen-front',
   instead of 'vsnd'. It comes from name of your driver.

[1] [alsa-devel] [PATCH 08/11] ALSA: vsnd: Add timer for period 
interrupt emulation
http://mailman.alsa-project.org/pipermail/alsa-devel/2017-August/123617.html
[2] [PATCH RESEND1 01/12] ALSA: vsnd: Introduce Xen para-virtualized 
sound frontend driver
http://mailman.alsa-project.org/pipermail/alsa-devel/2017-August/123654.html
[3] [alsa-devel] [PATCH RESEND1 12/12] ALSA: vsnd: Introduce Kconfig 
option to enable Xen PV sound
http://mailman.alsa-project.org/pipermail/alsa-devel/2017-August/123662.html
[4] You can see many posts for this; e.g. [alsa-devel] [PATCH 0/7] 
constify ALSA usb_device_id.
http://mailman.alsa-project.org/pipermail/alsa-devel/2017-August/123564.html

Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-08-10  3:14 ` [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Takashi Sakamoto
@ 2017-08-10  8:10   ` Oleksandr Andrushchenko
  2017-08-17 10:05     ` [Xen-devel] " Oleksandr Grytsov
  0 siblings, 1 reply; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-10  8:10 UTC (permalink / raw)
  To: Takashi Sakamoto, alsa-devel, xen-devel, linux-kernel
  Cc: Oleksandr Andrushchenko, tiwai

Hi,

thank you very much for valuable comments and your time!

On 08/10/2017 06:14 AM, Takashi Sakamoto wrote:
> Hi,
>
> On Aug 7 2017 21:22, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>
>> This patch series adds support for Xen [1] para-virtualized
>> sound frontend driver. It implements the protocol from
>> include/xen/interface/io/sndif.h with the following limitations:
>> - mute/unmute is not supported
>> - get/set volume is not supported
>> Volume control is not supported for the reason that most of the
>> use-cases (at the moment) are based on scenarious where
>> unprivileged OS (e.g. Android, AGL etc) use software mixers.
>>
>> Both capture and playback are supported.
>>
>> Thank you,
>> Oleksandr
>>
>> Resending because of rebase onto [2] + added missing patch
>>
>> [1] https://xenproject.org/
>> [2] 
>> https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/log/?h=for-next
>>
>> Oleksandr Andrushchenko (12):
>>    ALSA: vsnd: Introduce Xen para-virtualized sound frontend driver
>>    ALSA: vsnd: Implement driver's probe/remove
>>    ALSA: vsnd: Implement Xen bus state handling
>>    ALSA: vsnd: Read sound driver configuration from Xen store
>>    ALSA: vsnd: Implement Xen event channel handling
>>    ALSA: vsnd: Implement handling of shared buffers
>>    ALSA: vsnd: Introduce ALSA virtual sound driver
>>    ALSA: vsnd: Initialize virtul sound card
>>    ALSA: vsnd: Add timer for period interrupt emulation
>>    ALSA: vsnd: Implement ALSA PCM operations
>>    ALSA: vsnd: Implement communication with backend
>>    ALSA: vsnd: Introduce Kconfig option to enable Xen PV sound
>>
>>   sound/drivers/Kconfig     |   12 +
>>   sound/drivers/Makefile    |    2 +
>>   sound/drivers/xen-front.c | 2107 
>> +++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 2121 insertions(+)
>>   create mode 100644 sound/drivers/xen-front.c
>
> For this patchset, I have the same concern which Clemens Ladisch
> denoted[1]. If I can understand your explanation about queueing between
> Dom0/DomU stuffs, the concern can be described in short words; this
> driver works without any synchronization to data transmission by actual
> sound hardwares.
>
Yes, both your concerns and understanding are correct
> In design of ALSA PCM core, drivers are expected to synchronize to
> actual hardwares for semi-realtime data transmission. The
> synchronization is done by two points:
> 1) Interrupts to respond events from actual hardwares.
> 2) Positions of actual data transmission in any serial sound interfaces
>    of actual hardwares.
>
> These two points comes from typical designs of actual hardwares, thus
> they doesn't come from unfair, unreasonable, intrusive demands from
> software side.
>
This clear, thank you
> In design of typical stuffs on para-virtualization, Dom0 stuffs are hard
> to give enough abstraction of sound hardwares in these two points for
> DomU stuffs. Especially, it cannot abstract point 2) at all because the
> value of position should be accurate against actual time frame, while
> there's an overhead for DomU stuffs to read it. When DomU stuffs handles
> the value, the value is enough past due to context switches between
> Dom0/DomU. Therefore, this driver must rely on point 1) to synchronize
> to actual sound hardwares.
Which will also introduce some latency too: time needed to deliver and
handle interrupt from Dom0 to DomU
> Typically, drivers configure hardwares to
> generate interrupts per period of PCM buffer. This means that this
> driver should notify to Dom0 about the value of period size requested
> by applications.
>
> In 'include/xen/interface/io/sndif.h', there's no functionalities I
> described the above:
> 1. notifications from DomU to Dom0 about the size of period for
>    interrupts from actual hardwares. Or no way from Dom0 to DomU about
>    the configured size of the period.
Ok, then on "open" command I will pass from DomU to Dom0 an additional
parameter, period size. Then Dom0 will respond with actual period size
for DomU to use. So, this way period size will be negotiated.
Does the above look ok to you?
> 2. notifications of the interrupts from actual hardwares to DomU.
>
Ok, I will introduce an event from Dom0 to DomU to signal period elapsed.

Taking into account the fact that period size may be as small as,
say, 1ms, do you think we can/need to mangle period size in 1) on Dom0 side
to be reasonable, so we do not flood with interrupts/events from Dom0 to 
DomU?
Do you see any "formula" to determine that reasonable/acceptable
period limit, so both Dom0 and DomU are happy?

> For the reasons, your driver used kernel's timer interface to generate
> 'pseudo' interrupts for the purpose. However, it depends on Dom0's
> abstraction different from sound hardwares and Linux kernel's
> abstraction for timer functionality. In this case, gap between 'actual'
> interrupts from hardware and the 'pseudo' interrupts from a combination
> of several components brings unexpected result on several situations.
>
You are right
> I think this is defects of 'sndif' interface in Xen side. I think it
> better for you to work in Xen community to improve the above interface
> at first, then work for Linux stuffs.
>
Please see above for planned changes to the protocol
>
> Additionally, in next time, please remind of several points below:
>  * When a first patch adds an initial code for drivers, it should
>    include entries for Makefile and Kconfig, so that the driver can be
>    built even if it's still in an initial shape.
Will do
> Each patch should be
>    self-contained and should be in a shape so that developers easily run
>    bisecting. In other words, your first patch[2] includes modification
>   for Makefile and Kconfig in your last patch[3].
Will do
>  * When any read-only symbols is added,  it should have 'const'
>    qualifier so that the symbol places to .rodata section of ELF
>    binaries. For example, in your code, 'alsa_sndif_formats' is such an
>    symbol. In recent Linux development, some developers work for
>    constifying such symbols. Please remind of their continuous works in
>    upstream[4].
Will do
>  * You can split your driver to several files. In
>    'include/xen/interface/io/sndif.h', Dom0 produces functionalities for
>    DomU to control gain/volume/mute and in future your driver may get
>    more codes. If split to several files make it readable, it should be
>    done.
Will do. If I split, do you think it would be better to move the driver
from sound/drivers to sound/xen folder, so all those files do not mix
with the rest?
>  * In my taste, a prefix of the subject line should be 'xen-front',
>   instead of 'vsnd'. It comes from name of your driver.
>
Will do
> [1] [alsa-devel] [PATCH 08/11] ALSA: vsnd: Add timer for period 
> interrupt emulation
> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-August/123617.html 
>
> [2] [PATCH RESEND1 01/12] ALSA: vsnd: Introduce Xen para-virtualized 
> sound frontend driver
> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-August/123654.html 
>
> [3] [alsa-devel] [PATCH RESEND1 12/12] ALSA: vsnd: Introduce Kconfig 
> option to enable Xen PV sound
> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-August/123662.html 
>
> [4] You can see many posts for this; e.g. [alsa-devel] [PATCH 0/7] 
> constify ALSA usb_device_id.
> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-August/123564.html 
>
>
> Regards
>
> Takashi Sakamoto
Thank you,
Oleksandr

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [Xen-devel] [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-08-10  8:10   ` Oleksandr Andrushchenko
@ 2017-08-17 10:05     ` Oleksandr Grytsov
  2017-08-18  5:43       ` Takashi Sakamoto
  0 siblings, 1 reply; 34+ messages in thread
From: Oleksandr Grytsov @ 2017-08-17 10:05 UTC (permalink / raw)
  To: Oleksandr Andrushchenko, clemens, o-takashi
  Cc: alsa-devel, xen-devel, linux-kernel, tiwai, Oleksandr Andrushchenko

On Thu, Aug 10, 2017 at 11:10 AM, Oleksandr Andrushchenko
<andr2000@gmail.com> wrote:
> Hi,
>
> thank you very much for valuable comments and your time!
>
>
> On 08/10/2017 06:14 AM, Takashi Sakamoto wrote:
>>
>> Hi,
>>
>> On Aug 7 2017 21:22, Oleksandr Andrushchenko wrote:
>>>
>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>
>>> This patch series adds support for Xen [1] para-virtualized
>>> sound frontend driver. It implements the protocol from
>>> include/xen/interface/io/sndif.h with the following limitations:
>>> - mute/unmute is not supported
>>> - get/set volume is not supported
>>> Volume control is not supported for the reason that most of the
>>> use-cases (at the moment) are based on scenarious where
>>> unprivileged OS (e.g. Android, AGL etc) use software mixers.
>>>
>>> Both capture and playback are supported.
>>>
>>> Thank you,
>>> Oleksandr
>>>
>>> Resending because of rebase onto [2] + added missing patch
>>>
>>> [1] https://xenproject.org/
>>> [2]
>>> https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/log/?h=for-next
>>>
>>> Oleksandr Andrushchenko (12):
>>>    ALSA: vsnd: Introduce Xen para-virtualized sound frontend driver
>>>    ALSA: vsnd: Implement driver's probe/remove
>>>    ALSA: vsnd: Implement Xen bus state handling
>>>    ALSA: vsnd: Read sound driver configuration from Xen store
>>>    ALSA: vsnd: Implement Xen event channel handling
>>>    ALSA: vsnd: Implement handling of shared buffers
>>>    ALSA: vsnd: Introduce ALSA virtual sound driver
>>>    ALSA: vsnd: Initialize virtul sound card
>>>    ALSA: vsnd: Add timer for period interrupt emulation
>>>    ALSA: vsnd: Implement ALSA PCM operations
>>>    ALSA: vsnd: Implement communication with backend
>>>    ALSA: vsnd: Introduce Kconfig option to enable Xen PV sound
>>>
>>>   sound/drivers/Kconfig     |   12 +
>>>   sound/drivers/Makefile    |    2 +
>>>   sound/drivers/xen-front.c | 2107
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 2121 insertions(+)
>>>   create mode 100644 sound/drivers/xen-front.c
>>
>>
>> For this patchset, I have the same concern which Clemens Ladisch
>> denoted[1]. If I can understand your explanation about queueing between
>> Dom0/DomU stuffs, the concern can be described in short words; this
>> driver works without any synchronization to data transmission by actual
>> sound hardwares.
>>
> Yes, both your concerns and understanding are correct
>>
>> In design of ALSA PCM core, drivers are expected to synchronize to
>> actual hardwares for semi-realtime data transmission. The
>> synchronization is done by two points:
>> 1) Interrupts to respond events from actual hardwares.
>> 2) Positions of actual data transmission in any serial sound interfaces
>>    of actual hardwares.
>>
>> These two points comes from typical designs of actual hardwares, thus
>> they doesn't come from unfair, unreasonable, intrusive demands from
>> software side.
>>
> This clear, thank you
>>
>> In design of typical stuffs on para-virtualization, Dom0 stuffs are hard
>> to give enough abstraction of sound hardwares in these two points for
>> DomU stuffs. Especially, it cannot abstract point 2) at all because the
>> value of position should be accurate against actual time frame, while
>> there's an overhead for DomU stuffs to read it. When DomU stuffs handles
>> the value, the value is enough past due to context switches between
>> Dom0/DomU. Therefore, this driver must rely on point 1) to synchronize
>> to actual sound hardwares.

In order to implement option 1) discussed (Interrupts to respond events from
actual hardwares) we did number of experiments to find out if it can be
implemented in the way it satisfies the requirements with respect to latency,
interrupt number and use-cases.

First of all the sound backend is a user-space application which uses either
ALSA or PulseAudio to play/capture audio depending on configuration.
Most of the use-cases we have are using PulseAudio as it allows to
implement more complex use cases then just plain ALSA.

We started to look at how can we get such an event so it can be used as
a period elapsed notification to the backend.

In case of ALSA we used poll mechanism to wait for events from ALSA:
we configured SW params to have period event, but the problem here is that
it is notified not only when period elapses, but also when ALSA is ready to
consume more data. There is no mechanism to distinguish between these
two events (please correct us if there is one). Anyways, even if ALSA provides
period event to user-space (again, backend is a user-space application)
latency will consist of: time from kernel to user-space, user-space Dom0 to
frontend driver DomU. Both are variable and depend on many factors,
so the latency is not deterministic.

(We were also thinking that we can implement a helper driver in Dom0 to have
a dedicated channel from ALSA to the backend to deliver period elapsed event,
so for instance, it can have some kind of a hook on snd_pcm_period_elapsed,
but it will not solve the use-case with PulseAudio discussed below.
Also it is unclear how to handle scenario when multiple DomU plays through
mixer with different frame rates, channels etc.).

In case of PulseAudio it is even worse. PulseAudio can’t provide any period
related information (again, please let us know if this is not true). From our
understanding, event if get such an event from PulseAudio it will be software
emulated in user-space: for example, when PulseAudio uses a single ALSA
sink and mixes two streams with different frame rates, it will generate at least
for one of the streams a software period elapsed event.

So, from the above we think that period elapsed event derived in the described
ways may not improve latency and will complicate the system. So, for that
reason we are thinking of the option 2) (Positions of actual data transmission
in any serial sound interfaces of actual hardwares.)

In both ALSA and PulseAudio cases we can get timestamp information
(current sample timestamp). It can be converted to frames or bytes or
whatever that has been processed by the HW. The backend can get
timestamp periodically (with polling period configured with respect
to framerates being played) and pass it to the frontend. Due to context
switch and other factors this information will be outdated as well, but it seems
to be the best sync approach the backend can provide.

So, from the above, both options for synchronization cannot guarantee that
we will indeed be synchronous or latency is deterministic, but may improve
things comparing to the kernel timer on frontend’s side.

All, could you please tell us your opinion on the above and suggest what
could be the right way to go?

> Which will also introduce some latency too: time needed to deliver and
> handle interrupt from Dom0 to DomU
>>
>> Typically, drivers configure hardwares to
>> generate interrupts per period of PCM buffer. This means that this
>> driver should notify to Dom0 about the value of period size requested
>> by applications.
>>
>> In 'include/xen/interface/io/sndif.h', there's no functionalities I
>> described the above:
>> 1. notifications from DomU to Dom0 about the size of period for
>>    interrupts from actual hardwares. Or no way from Dom0 to DomU about
>>    the configured size of the period.
>
> Ok, then on "open" command I will pass from DomU to Dom0 an additional
> parameter, period size. Then Dom0 will respond with actual period size
> for DomU to use. So, this way period size will be negotiated.
> Does the above look ok to you?
>>
>> 2. notifications of the interrupts from actual hardwares to DomU.
>>
> Ok, I will introduce an event from Dom0 to DomU to signal period elapsed.
>
> Taking into account the fact that period size may be as small as,
> say, 1ms, do you think we can/need to mangle period size in 1) on Dom0 side
> to be reasonable, so we do not flood with interrupts/events from Dom0 to
> DomU?
> Do you see any "formula" to determine that reasonable/acceptable
> period limit, so both Dom0 and DomU are happy?
>
>> For the reasons, your driver used kernel's timer interface to generate
>> 'pseudo' interrupts for the purpose. However, it depends on Dom0's
>> abstraction different from sound hardwares and Linux kernel's
>> abstraction for timer functionality. In this case, gap between 'actual'
>> interrupts from hardware and the 'pseudo' interrupts from a combination
>> of several components brings unexpected result on several situations.
>>
> You are right
>>
>> I think this is defects of 'sndif' interface in Xen side. I think it
>> better for you to work in Xen community to improve the above interface
>> at first, then work for Linux stuffs.
>>
> Please see above for planned changes to the protocol
>>
>>
>> Additionally, in next time, please remind of several points below:
>>  * When a first patch adds an initial code for drivers, it should
>>    include entries for Makefile and Kconfig, so that the driver can be
>>    built even if it's still in an initial shape.
>
> Will do
>>
>> Each patch should be
>>    self-contained and should be in a shape so that developers easily run
>>    bisecting. In other words, your first patch[2] includes modification
>>   for Makefile and Kconfig in your last patch[3].
>
> Will do
>>
>>  * When any read-only symbols is added,  it should have 'const'
>>    qualifier so that the symbol places to .rodata section of ELF
>>    binaries. For example, in your code, 'alsa_sndif_formats' is such an
>>    symbol. In recent Linux development, some developers work for
>>    constifying such symbols. Please remind of their continuous works in
>>    upstream[4].
>
> Will do
>>
>>  * You can split your driver to several files. In
>>    'include/xen/interface/io/sndif.h', Dom0 produces functionalities for
>>    DomU to control gain/volume/mute and in future your driver may get
>>    more codes. If split to several files make it readable, it should be
>>    done.
>
> Will do. If I split, do you think it would be better to move the driver
> from sound/drivers to sound/xen folder, so all those files do not mix
> with the rest?
>>
>>  * In my taste, a prefix of the subject line should be 'xen-front',
>>   instead of 'vsnd'. It comes from name of your driver.
>>
> Will do
>>
>> [1] [alsa-devel] [PATCH 08/11] ALSA: vsnd: Add timer for period interrupt
>> emulation
>>
>> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-August/123617.html
>> [2] [PATCH RESEND1 01/12] ALSA: vsnd: Introduce Xen para-virtualized sound
>> frontend driver
>>
>> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-August/123654.html
>> [3] [alsa-devel] [PATCH RESEND1 12/12] ALSA: vsnd: Introduce Kconfig
>> option to enable Xen PV sound
>>
>> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-August/123662.html
>> [4] You can see many posts for this; e.g. [alsa-devel] [PATCH 0/7]
>> constify ALSA usb_device_id.
>>
>> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-August/123564.html
>>
>> Regards
>>
>> Takashi Sakamoto
>
> Thank you,
> Oleksandr
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel



-- 
Best Regards,
Oleksandr Grytsov.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-08-17 10:05     ` [Xen-devel] " Oleksandr Grytsov
@ 2017-08-18  5:43       ` Takashi Sakamoto
  2017-08-18  5:56         ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 34+ messages in thread
From: Takashi Sakamoto @ 2017-08-18  5:43 UTC (permalink / raw)
  To: Oleksandr Grytsov, Oleksandr Andrushchenko, clemens
  Cc: alsa-devel, xen-devel, linux-kernel, tiwai, Oleksandr Andrushchenko

On Aug 17 2017 19:05, Oleksandr Grytsov wrote:
> So, from the above we think that period elapsed event derived in the described
> ways may not improve latency and will complicate the system. So, for that
> reason we are thinking of the option 2) (Positions of actual data transmission
> in any serial sound interfaces of actual hardwares.)

Please declare the way to enable stuffs on DomU to get the positions 
from actual hardware.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-08-18  5:43       ` Takashi Sakamoto
@ 2017-08-18  5:56         ` Oleksandr Andrushchenko
  2017-08-18  7:17           ` Takashi Sakamoto
  0 siblings, 1 reply; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-18  5:56 UTC (permalink / raw)
  To: Takashi Sakamoto, Oleksandr Grytsov, clemens
  Cc: alsa-devel, xen-devel, linux-kernel, tiwai, Oleksandr Andrushchenko

Hello,

On 08/18/2017 08:43 AM, Takashi Sakamoto wrote:
> On Aug 17 2017 19:05, Oleksandr Grytsov wrote:
>> So, from the above we think that period elapsed event derived in the 
>> described
>> ways may not improve latency and will complicate the system. So, for 
>> that
>> reason we are thinking of the option 2) (Positions of actual data 
>> transmission
>> in any serial sound interfaces of actual hardwares.)
>
> Please declare the way to enable stuffs on DomU to get the positions 
> from actual hardware.
>
This is what we haven't investigated yet in detail as we were mostly 
focusing on
period elapsed approach, so we can report our findings to the community.

Taking into account the fact that the backend we have is a user-space 
application
running on top of ALSA/PulseAudio we cannot get HW pointers from actual
hardware by any means (PulseAudio use-case is the most tough thing in 
equation)

At the moment we are seeking for any advise from more experienced developers
in the field on possible ways to solve the problem of Dom0/DomU 
synchronization.
Hope, together we can identify such a way that would be accepted by the 
community.

If you have something on your mind could you please share your thoughts?
>
> Regards
>
> Takashi Sakamoto
Thank you,
Oleksandr Andrushchenko

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-08-18  5:56         ` Oleksandr Andrushchenko
@ 2017-08-18  7:17           ` Takashi Sakamoto
  2017-08-18  7:23             ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 34+ messages in thread
From: Takashi Sakamoto @ 2017-08-18  7:17 UTC (permalink / raw)
  To: Oleksandr Andrushchenko, Oleksandr Grytsov, clemens
  Cc: alsa-devel, xen-devel, linux-kernel, tiwai, Oleksandr Andrushchenko

On Aug 18 2017 14:56, Oleksandr Andrushchenko wrote:
> Taking into account the fact that the backend we have is a user-space 
> application
> running on top of ALSA/PulseAudio we cannot get HW pointers from actual
> hardware by any means (PulseAudio use-case is the most tough thing in 
> equation)

You mean that any alsa-lib or libpulse applications run on Dom0 as a
backend driver for the frontend driver on DomU?


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-08-18  7:17           ` Takashi Sakamoto
@ 2017-08-18  7:23             ` Oleksandr Andrushchenko
  2017-08-22  2:43               ` Takashi Sakamoto
  0 siblings, 1 reply; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-18  7:23 UTC (permalink / raw)
  To: Takashi Sakamoto, Oleksandr Grytsov, clemens
  Cc: alsa-devel, xen-devel, linux-kernel, tiwai, Oleksandr Andrushchenko


On 08/18/2017 10:17 AM, Takashi Sakamoto wrote:
> On Aug 18 2017 14:56, Oleksandr Andrushchenko wrote:
>> Taking into account the fact that the backend we have is a user-space 
>> application
>> running on top of ALSA/PulseAudio we cannot get HW pointers from actual
>> hardware by any means (PulseAudio use-case is the most tough thing in 
>> equation)
>
> You mean that any alsa-lib or libpulse applications run on Dom0 as a
> backend driver for the frontend driver on DomU?
>
No, the sound backend [1] is a user-space application (ALSA/PulseAudio 
client)
which runs as a Xen para-virtual backend in Dom0 and serves all the
frontends running in DomU(s).
Other ALSA/PulseAudio clients in Dom0 are also allowed to run at the
same time.
>
> Regards
>
> Takashi Sakamoto
Thank you,
Oleksandr

[1] https://github.com/xen-troops/snd_be

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-08-18  7:23             ` Oleksandr Andrushchenko
@ 2017-08-22  2:43               ` Takashi Sakamoto
  2017-08-23 14:51                 ` Oleksandr Grytsov
  0 siblings, 1 reply; 34+ messages in thread
From: Takashi Sakamoto @ 2017-08-22  2:43 UTC (permalink / raw)
  To: Oleksandr Andrushchenko, Oleksandr Grytsov, clemens
  Cc: alsa-devel, xen-devel, linux-kernel, tiwai, Oleksandr Andrushchenko

Hi,

On Aug 18 2017 16:23, Oleksandr Andrushchenko wrote:
 >> You mean that any alsa-lib or libpulse applications run on Dom0 as a
 >> backend driver for the frontend driver on DomU?
 >>
 > No, the sound backend [1] is a user-space application (ALSA/PulseAudio
 > client)
 > which runs as a Xen para-virtual backend in Dom0 and serves all the
 > frontends running in DomU(s).
 > Other ALSA/PulseAudio clients in Dom0 are also allowed to run at the
 > same time.
 >
 > [1] https://github.com/xen-troops/snd_be

Actually, you did what I meant.

Playback                                                     Capture
  delay       DomU-A    DomU-B    DomU-C                      delay
             --------- --------- ---------
             |       | |       | |       |
(queueing)  | App-A | | App-B | | App-C |                   (handling)
     |       |       | |       | |       |                       ^
     |       | (TSS) | | (TSS) | | (TSS) |                       |
     |       |       | |       | |       |                       |
     |       ---^----- ----^---- ----^----                       |
     |       ===|==========|=========|==== XenBus and            |
     |       ---|----------|-------- |----   mapped page frame   |
     |  Dom0 |  v          v         v   |                       |
     |       |App-0      App-1     App-2 |                       |
     |       |  ^          ^         ^   |                       |
     |       |  |-> App-3<-|         |   |                       |
     |       |(IPC)  ^   (IPC)       |   |                       |
     |       |       v               v   |                       |
     |       |==HW abstraction for TSS ==|                       |
     |       |          ^     ^          |                       |
     |       -----------|-----|-----------                       |
     |                  |     |    (TSS = Time Sharing System)   |
     |                  v     v                                  |
     |                  Hardwares                                |
     v                     v                                     |
(presenting)          physical part                         (sampling)

I can easily imagine that several applications (App[0|1|2]) run in Dom0
as backend drivers of this context, to add several 'virtual' sound
device for DomU, via Xenbus. The backend drivers can handle different
hardware for the 'virtual' sound devices; e.g. it can be BSD socket
applications.  Of course, this is a sample based on my imagination.
Actually, you assume that your application exclusively produces the
'virtual' sound cards, I guess.  Anyway, it's not a point of this
discussion.

 > In order to implement option 1) discussed (Interrupts to respond 
events from
 > actual hardware) we did number of experiments to find out if it can be
 > implemented in the way it satisfies the requirements with respect to 
latency,
 > interrupt number and use-cases.
 >
 > First of all the sound backend is a user-space application which uses 
either
 > ALSA or PulseAudio to play/capture audio depending on configuration.
 > Most of the use-cases we have are using PulseAudio as it allows to
 > implement more complex use cases then just plain ALSA.

When assuming App-3 in the above diagram as PulseAudio, a combination
of App-0/App-1/App-3 may correspond to the backend driver in your
use-case.

 > We started to look at how can we get such an event so it can be used as
 > a period elapsed notification to the backend.
 >
 > In case of ALSA we used poll mechanism to wait for events from ALSA:
 > we configured SW params to have period event, but the problem here is 
that
 > it is notified not only when period elapses, but also when ALSA is 
ready to
 > consume more data. There is no mechanism to distinguish between these
 > two events (please correct us if there is one). Anyways, even if ALSA 
provides
 > period event to user-space (again, backend is a user-space application)
 > latency will consist of: time from kernel to user-space, user-space 
Dom0 to
 > frontend driver DomU. Both are variable and depend on many factors,
 > so the latency is not deterministic.
 >
 > (We were also thinking that we can implement a helper driver in Dom0 
to have
 > a dedicated channel from ALSA to the backend to deliver period 
elapsed event,
 > so for instance, it can have some kind of a hook on 
snd_pcm_period_elapsed,
 > but it will not solve the use-case with PulseAudio discussed below.
 > Also it is unclear how to handle scenario when multiple DomU plays 
through
 > mixer with different frame rates, channels etc.).

In design of ALSA PCM core, processes are awakened from poll wait by
the other running tasks, which calculate available space on PCM buffer.
This is done by a call of 'snd_pcm_hw_prw0()' in 'sound/core/pcm_lib.c'
in kernel land. In this function, ALSA PCM core calls implementation of
'struct snd_pcm_ops.pointer()' in each driver and get current position
of data transmission within buffer size, then 'hw_ptr' of PCM buffer
is updated, then calculates the avail space.

Typical ALSA PCM drivers call the function in any hw IRQ context for
interrupts generated by hardware, or sw IRQ context for interrupts
generated by packet-oriented drivers for general-purpose buses such as
USB. This is a reason that the drivers configure hardware to generate
interrupts.

Actually, the value of 'avail_min' can be configured by user threads
as 'struct snd_pcm_sw_params'. As a default, this equals to the size of
period of PCM buffer.

On the other hand, any user thread can also call the function in a
call graph of ioctl(2) with some commands; e.g. SNDRV_PCM_IOCTL_HWSYNC.
Even if a user thread is on poll wait, the other user thread can awake
the thread by calling ioctl(2) with such commands. But usual program
processes I/O in one user thread and this scenario is rare.

The above is a typical scenario to use ALSA stuffs for semi-realtime
data transmission for sound hardware. Programs rely on the IRQ
generated by hardware. Drivers are programmed to configure the
hardware generating the IRQ. ALSA PCM applications are awakened by IRQ
handlers and queue/handle PCM frames in avail space on PCM buffer.

For efficiency, the interval of IRQ is configured as the same size
as a period of PCM buffer in frame unit. This is a concept of the
'period'. But there's a rest not to configure the interval per period;
e.g. IEC 61883-1/6 engine in ALSA firewire stack configures 1394 OHCI
isochronous context for callback per 2msec in its sw IRQ context while
the size of period is restricted to get one interrupt at least.
Therefore, the interval of interrupt is not necessarily as the same as
the size of period as long as IRQ handler enables applications to handle
avail space.


In a recent decade, ALSA PCM core supports the other scenario, which
rely on system timer with enough accuracy. In this scenario,
applications get an additional descriptor for system timer and
configure the timer to wake up as applications' convenience, or use
precise system call for multiplexed I/O such as ppoll(2). Applications
wake up as they prefer, the applications call ioctl(2) with
SNDRV_PCM_IOCTL_HWSYNC and calculate the avail space, then process PCM
frames. When all of handled PCM frames are queued, they schedule to
wake up far enough. Else, they schedule to wake up soon to reduce delay
for handled PCM frames.

In this scenario, any hw/sw interrupt is not necessarily required as
long as system timer is enough accurate and data transmission
automatically runs regardless of IRQ handlers. For this scenario, a few
drivers have conditional code to suppress hw/sw intervals; e.g. drivers
for 'Intel HDA' and 'C-Media 87xx' because this scenario requires
actual hardware to transfer data frames automatically but make it
available for drivers to get precise position of the transmission.
Furthermore, there's a application which supports this scenario. As
long as I know, excluding PulseAudio, nothing.

As a supplement, I note that audio timestamp is also calculated in the
function, 'snd_pcm_hw_prw0()'.


Well, as I indicated, the frontend driver works without any
synchronization to data transmission by actual sound hardware. It
relies on system timer on each of DomU and Dom0. I note my concern
against this design at last.

Linux is a kind of Time Sharing System. CPU time is divided for each
tasks. Thus there's delay of scheduling. ALSA is designed to rely on
hw/sw interrupts, because IRQ context can run regardless of the task
scheduling. (actually many exceptions I know.). This design dedicates
data transmission for actual time frame.

In a diagram of top of this message, the frontend driver runs on each
of DomU. Timer functionality of the DomU is based on scheduling on Dom0
somehow, thus there's a delay due to scheduling. At least, it has a
restriction for its preciseness. Additionally, applications on DomU are
schedulable tasks, thus they're dominated by task scheduler on DomU.
There's no reliance for actual time frame. Furthermore, libpulse
applications on Dom0 perform IPC to pulseaudio daemon. This brings
an additional overhead to synchronize to the other processes.

This is not an issue for usual applications. But for applications to
transfer data against actual time frame, it's a problem. Totally,
there's no guarantee of the data transmission for semi-realtime
capability. Any applications on DomU must run with large delay for safe
against timing gap.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-08-22  2:43               ` Takashi Sakamoto
@ 2017-08-23 14:51                 ` Oleksandr Grytsov
  2017-08-24  4:38                   ` Takashi Sakamoto
  0 siblings, 1 reply; 34+ messages in thread
From: Oleksandr Grytsov @ 2017-08-23 14:51 UTC (permalink / raw)
  To: Takashi Sakamoto
  Cc: Oleksandr Andrushchenko, clemens, alsa-devel, xen-devel,
	linux-kernel, tiwai, Oleksandr Andrushchenko

Hi,

Thank you for detailed explanation.

We understand that emulated interrupt on the frontend side is completely not
acceptable and definitely we need to provide some feedback mechanism from
Dom0 to DomU.

In our case it is technically impossible to provide precise period interrupt
(mostly because our backend is a user space application).
The best we can implement it is provide number of frames (time, bytes etc.)
consumed by real HW. This info will be outdated due to different delays but
we can provide precise timestamps when this info was acquired.

Would this info be useful to update the frontend driver state?

If you have in mind any other solution we would appreciate.

-- 
Best Regards,
Oleksandr Grytsov.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-08-23 14:51                 ` Oleksandr Grytsov
@ 2017-08-24  4:38                   ` Takashi Sakamoto
  2017-08-24  7:04                     ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 34+ messages in thread
From: Takashi Sakamoto @ 2017-08-24  4:38 UTC (permalink / raw)
  To: Oleksandr Grytsov
  Cc: Oleksandr Andrushchenko, clemens, alsa-devel, xen-devel,
	linux-kernel, tiwai, Oleksandr Andrushchenko

On Aug 23 2017 23:51, Oleksandr Grytsov wrote:
> Hi,
> 
> Thank you for detailed explanation.
> 
> We understand that emulated interrupt on the frontend side is completely not
> acceptable and definitely we need to provide some feedback mechanism from
> Dom0 to DomU.
> 
> In our case it is technically impossible to provide precise period interrupt
> (mostly because our backend is a user space application).
> The best we can implement it is provide number of frames (time, bytes etc.)
> consumed by real HW. This info will be outdated due to different delays but
> we can provide precise timestamps when this info was acquired.

Stuffs of ALSA PCM in kernel land is an abstraction layer for actual
hardware for data transmission. The stuffs get affects from a design of
actual hardware. Furthermore, sound subsystems on the other operating
systems such as Microsoft Windows are also designed with a consideration
about actual hardware. When you design any interfaces as an abstraction
for such software layer, it's better to understand actual hardware and
design of low-level software layer somehow.

Actually the 'sndif' has no good abstraction for actual hardware,
therefore an idea to implement frontend driver as an ALSA driver is not
reasonable at all. It's better to implement it as an application in
the other software layer, e.g. sinks/sources of PulseAudio in DomU via
Xenbus. This idea is nearer an original concept of Xen framework, I
guess. But I don't know we can write any applications of Xenbus in user
land of DomU or not.

Anyway, it's not a good idea to have an ALSA driver for the present 
'sndif', in my opinion.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-08-24  4:38                   ` Takashi Sakamoto
@ 2017-08-24  7:04                     ` Oleksandr Andrushchenko
  2017-09-04  7:21                       ` Oleksandr Andrushchenko
  2017-09-05  7:24                       ` [alsa-devel] " Clemens Ladisch
  0 siblings, 2 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-08-24  7:04 UTC (permalink / raw)
  To: Takashi Sakamoto, Oleksandr Grytsov
  Cc: clemens, alsa-devel, xen-devel, linux-kernel, tiwai,
	Oleksandr Andrushchenko

Hello,

On 08/24/2017 07:38 AM, Takashi Sakamoto wrote:
> On Aug 23 2017 23:51, Oleksandr Grytsov wrote:
>> Hi,
>>
>> Thank you for detailed explanation.
>>
>> We understand that emulated interrupt on the frontend side is 
>> completely not
>> acceptable and definitely we need to provide some feedback mechanism 
>> from
>> Dom0 to DomU.
>>
>> In our case it is technically impossible to provide precise period 
>> interrupt
>> (mostly because our backend is a user space application).
>> The best we can implement it is provide number of frames (time, bytes 
>> etc.)
>> consumed by real HW. This info will be outdated due to different 
>> delays but
>> we can provide precise timestamps when this info was acquired.
>
> Stuffs of ALSA PCM in kernel land is an abstraction layer for actual
> hardware for data transmission. The stuffs get affects from a design of
> actual hardware. Furthermore, sound subsystems on the other operating
> systems such as Microsoft Windows are also designed with a consideration
> about actual hardware. When you design any interfaces as an abstraction
> for such software layer, it's better to understand actual hardware and
> design of low-level software layer somehow.
>
> Actually the 'sndif' has no good abstraction for actual hardware,
> therefore an idea to implement frontend driver as an ALSA driver is not
> reasonable at all. 
the reason for that is that you can use the same frontend driver for various
DomUs without the need to write yet another HAL/application, e.g. if one 
of the
DomUs has no PulseAudio (uses ALSA) and yet another DomU has PulseAudio,
then using the same driver allows you to enable both out of the box with the
same codebase.
If we can imagine something else running on top of ALSA (say some other
mixing software other than PulseAudio) then we will have to support that 
as well

> It's better to implement it as an application in
> the other software layer, e.g. sinks/sources of PulseAudio in DomU
please see our reasoning above
> via
> Xenbus. This idea is nearer an original concept of Xen framework, I
> guess. But I don't know we can write any applications of Xenbus in user
> land of DomU or not.
>
> Anyway, it's not a good idea to have an ALSA driver for the present 
> 'sndif', in my opinion.
>
ok, so the main concern here is that we cannot properly synchronize 
Dom0-DomU.
If we put this apart for a second are there any other concerns on having 
ALSA
frontend driver? If not, can we have the driver with timer 
implementation upstreamed
as experimental until we have some acceptable synchronization solution?
This will allow broader audience to try and feel the solution and 
probably contribute?
>
> Regards
>
> Takashi Sakamoto
Thank you very much for your time,
Oleksandr Andrushchenko

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-08-24  7:04                     ` Oleksandr Andrushchenko
@ 2017-09-04  7:21                       ` Oleksandr Andrushchenko
  2017-09-05  7:24                       ` [alsa-devel] " Clemens Ladisch
  1 sibling, 0 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-09-04  7:21 UTC (permalink / raw)
  To: Takashi Sakamoto, Oleksandr Grytsov
  Cc: clemens, alsa-devel, xen-devel, linux-kernel, tiwai,
	Oleksandr Andrushchenko


On 08/24/2017 10:04 AM, Oleksandr Andrushchenko wrote:
> Hello,
>
> On 08/24/2017 07:38 AM, Takashi Sakamoto wrote:
>> On Aug 23 2017 23:51, Oleksandr Grytsov wrote:
>>> Hi,
>>>
>>> Thank you for detailed explanation.
>>>
>>> We understand that emulated interrupt on the frontend side is 
>>> completely not
>>> acceptable and definitely we need to provide some feedback mechanism 
>>> from
>>> Dom0 to DomU.
>>>
>>> In our case it is technically impossible to provide precise period 
>>> interrupt
>>> (mostly because our backend is a user space application).
>>> The best we can implement it is provide number of frames (time, 
>>> bytes etc.)
>>> consumed by real HW. This info will be outdated due to different 
>>> delays but
>>> we can provide precise timestamps when this info was acquired.
>>
>> Stuffs of ALSA PCM in kernel land is an abstraction layer for actual
>> hardware for data transmission. The stuffs get affects from a design of
>> actual hardware. Furthermore, sound subsystems on the other operating
>> systems such as Microsoft Windows are also designed with a consideration
>> about actual hardware. When you design any interfaces as an abstraction
>> for such software layer, it's better to understand actual hardware and
>> design of low-level software layer somehow.
>>
>> Actually the 'sndif' has no good abstraction for actual hardware,
>> therefore an idea to implement frontend driver as an ALSA driver is not
>> reasonable at all. 
> the reason for that is that you can use the same frontend driver for 
> various
> DomUs without the need to write yet another HAL/application, e.g. if 
> one of the
> DomUs has no PulseAudio (uses ALSA) and yet another DomU has PulseAudio,
> then using the same driver allows you to enable both out of the box 
> with the
> same codebase.
> If we can imagine something else running on top of ALSA (say some other
> mixing software other than PulseAudio) then we will have to support 
> that as well
>
>> It's better to implement it as an application in
>> the other software layer, e.g. sinks/sources of PulseAudio in DomU
> please see our reasoning above
>> via
>> Xenbus. This idea is nearer an original concept of Xen framework, I
>> guess. But I don't know we can write any applications of Xenbus in user
>> land of DomU or not.
>>
>> Anyway, it's not a good idea to have an ALSA driver for the present 
>> 'sndif', in my opinion.
>>
> ok, so the main concern here is that we cannot properly synchronize 
> Dom0-DomU.
> If we put this apart for a second are there any other concerns on 
> having ALSA
> frontend driver? If not, can we have the driver with timer 
> implementation upstreamed
> as experimental until we have some acceptable synchronization solution?
> This will allow broader audience to try and feel the solution and 
> probably contribute?
any thoughts on this?
>>
>> Regards
>>
>> Takashi Sakamoto
> Thank you very much for your time,
> Oleksandr Andrushchenko

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [alsa-devel] [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-08-24  7:04                     ` Oleksandr Andrushchenko
  2017-09-04  7:21                       ` Oleksandr Andrushchenko
@ 2017-09-05  7:24                       ` Clemens Ladisch
  2017-09-12  7:52                         ` Oleksandr Grytsov
  1 sibling, 1 reply; 34+ messages in thread
From: Clemens Ladisch @ 2017-09-05  7:24 UTC (permalink / raw)
  To: Oleksandr Andrushchenko, Takashi Sakamoto, Oleksandr Grytsov
  Cc: alsa-devel, Oleksandr Andrushchenko, tiwai, linux-kernel, xen-devel

Oleksandr Andrushchenko wrote:
>>> We understand that emulated interrupt on the frontend side is completely not
>>> acceptable

Allow me to expand on that:  Proper synchronization requires that the
exact position is communicated, not estimated.  Just because the nominal
rate of the stream is known does not imply that you know the actual rate.
Forget for the moment that there even is a nominal rate; assume that it
works like, e.g., a storage controller, and that you can know that a DMA
buffer was consumed by the device only after it has told you.

It's possible and likely that there is a latency when reporting the
stream position, but that is still better than guessing what the DMA
is doing.  (You would never just try to guess when writing data to
disk, would you?)

>>> and definitely we need to provide some feedback mechanism from
>>> Dom0 to DomU.
>>>
>>> In our case it is technically impossible to provide precise period interrupt
>>> (mostly because our backend is a user space application).

As far as I can see, all audio APIs (ALSA, PulseAudio, etc.) have poll()
or callbacks or similar mechanisms to inform you when new data can be
written, and always allow to query the current position.

> [...]
> ok, so the main concern here is that we cannot properly synchronize Dom0-DomU.
> If we put this apart for a second are there any other concerns on having ALSA
> frontend driver? If not, can we have the driver with timer implementation upstreamed
> as experimental until we have some acceptable synchronization solution?
> This will allow broader audience to try and feel the solution and probably contribute?

I doubt that the driver architecture will stay completely the same, so I
do not think that this experimental driver would demonstrate how the
solution would feel.

As the first step, I would suggest creating a driver with proper
synchronization, even if it has high latency.  Reducing the latency
would then be 'just' an optimization.


Regards,
Clemens

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [alsa-devel] [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-09-05  7:24                       ` [alsa-devel] " Clemens Ladisch
@ 2017-09-12  7:52                         ` Oleksandr Grytsov
  2017-09-19  8:57                           ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 34+ messages in thread
From: Oleksandr Grytsov @ 2017-09-12  7:52 UTC (permalink / raw)
  To: Clemens Ladisch
  Cc: Oleksandr Andrushchenko, Takashi Sakamoto, alsa-devel,
	Oleksandr Andrushchenko, tiwai, linux-kernel, xen-devel

On Tue, Sep 5, 2017 at 10:24 AM, Clemens Ladisch <clemens@ladisch.de> wrote:
> Oleksandr Andrushchenko wrote:
>>>> We understand that emulated interrupt on the frontend side is completely not
>>>> acceptable
>
> Allow me to expand on that:  Proper synchronization requires that the
> exact position is communicated, not estimated.  Just because the nominal
> rate of the stream is known does not imply that you know the actual rate.
> Forget for the moment that there even is a nominal rate; assume that it
> works like, e.g., a storage controller, and that you can know that a DMA
> buffer was consumed by the device only after it has told you.
>
> It's possible and likely that there is a latency when reporting the
> stream position, but that is still better than guessing what the DMA
> is doing.  (You would never just try to guess when writing data to
> disk, would you?)
>
>>>> and definitely we need to provide some feedback mechanism from
>>>> Dom0 to DomU.
>>>>
>>>> In our case it is technically impossible to provide precise period interrupt
>>>> (mostly because our backend is a user space application).
>
> As far as I can see, all audio APIs (ALSA, PulseAudio, etc.) have poll()
> or callbacks or similar mechanisms to inform you when new data can be
> written, and always allow to query the current position.
>
>> [...]
>> ok, so the main concern here is that we cannot properly synchronize Dom0-DomU.
>> If we put this apart for a second are there any other concerns on having ALSA
>> frontend driver? If not, can we have the driver with timer implementation upstreamed
>> as experimental until we have some acceptable synchronization solution?
>> This will allow broader audience to try and feel the solution and probably contribute?
>
> I doubt that the driver architecture will stay completely the same, so I
> do not think that this experimental driver would demonstrate how the
> solution would feel.
>
> As the first step, I would suggest creating a driver with proper
> synchronization, even if it has high latency.  Reducing the latency
> would then be 'just' an optimization.
>
>
> Regards,
> Clemens

Definitely feedback from the backend side is required. Currently
we are working on synchronized version on the backend
and frontend side. We will be back once we have the solution.

Thanks.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [alsa-devel] [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-09-12  7:52                         ` Oleksandr Grytsov
@ 2017-09-19  8:57                           ` Oleksandr Andrushchenko
  2017-09-26 11:35                             ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-09-19  8:57 UTC (permalink / raw)
  To: Oleksandr Grytsov, Clemens Ladisch, Takashi Sakamoto, tiwai
  Cc: alsa-devel, Oleksandr Andrushchenko, linux-kernel, xen-devel,
	Oleksandr Grytsov

[-- Attachment #1: Type: text/plain, Size: 3566 bytes --]

Hi, all!

We did some work on implementing the idea with

feedback events from the backend to the frontend.

Please see attached the changes to the existing sndif protocol [1]:

1. Introduced a new event channel from back to front

2. New event with number of bytes played/captured (XENSND_EVT_CUR_POS,

to be used for sending snd_pcm_period_elapsed at frontend.

Sent in bytes, not frames to make the protocol generic and consistent)

3. New request for playback/capture control (XENSND_OP_TRIGGER)

with start/pause/stop/resume sub-ops.

The implementation we have showed that this is sufficient to
successfully play/capture w/o using emulated interrupts.

Clemens, Sakamoto-san,
could you please review the changes and confirm that these are ok to
be upstreamed to the sndif protocol and are enough for the frontend
driver we want to upstream (we have it implemented, just need to make
sure the general approach is accepted by the ALSA community).

Thank you very much for your time,
Oleksandr Andrushchenko
Oleksandr Grytsov

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/xen/interface/io/sndif.h?h=v4.14-rc1

On 09/12/2017 10:52 AM, Oleksandr Grytsov wrote:
> On Tue, Sep 5, 2017 at 10:24 AM, Clemens Ladisch <clemens@ladisch.de> wrote:
>> Oleksandr Andrushchenko wrote:
>>>>> We understand that emulated interrupt on the frontend side is completely not
>>>>> acceptable
>> Allow me to expand on that:  Proper synchronization requires that the
>> exact position is communicated, not estimated.  Just because the nominal
>> rate of the stream is known does not imply that you know the actual rate.
>> Forget for the moment that there even is a nominal rate; assume that it
>> works like, e.g., a storage controller, and that you can know that a DMA
>> buffer was consumed by the device only after it has told you.
>>
>> It's possible and likely that there is a latency when reporting the
>> stream position, but that is still better than guessing what the DMA
>> is doing.  (You would never just try to guess when writing data to
>> disk, would you?)
>>
>>>>> and definitely we need to provide some feedback mechanism from
>>>>> Dom0 to DomU.
>>>>>
>>>>> In our case it is technically impossible to provide precise period interrupt
>>>>> (mostly because our backend is a user space application).
>> As far as I can see, all audio APIs (ALSA, PulseAudio, etc.) have poll()
>> or callbacks or similar mechanisms to inform you when new data can be
>> written, and always allow to query the current position.
>>
>>> [...]
>>> ok, so the main concern here is that we cannot properly synchronize Dom0-DomU.
>>> If we put this apart for a second are there any other concerns on having ALSA
>>> frontend driver? If not, can we have the driver with timer implementation upstreamed
>>> as experimental until we have some acceptable synchronization solution?
>>> This will allow broader audience to try and feel the solution and probably contribute?
>> I doubt that the driver architecture will stay completely the same, so I
>> do not think that this experimental driver would demonstrate how the
>> solution would feel.
>>
>> As the first step, I would suggest creating a driver with proper
>> synchronization, even if it has high latency.  Reducing the latency
>> would then be 'just' an optimization.
>>
>>
>> Regards,
>> Clemens
> Definitely feedback from the backend side is required. Currently
> we are working on synchronized version on the backend
> and frontend side. We will be back once we have the solution.
>
> Thanks.


[-- Attachment #2: sndif_v2.patch --]
[-- Type: text/x-patch, Size: 11688 bytes --]

diff --git a/include/xen/interface/io/sndif.h b/include/xen/interface/io/sndif.h
index 5c918276835e..763a3f0164f4 100644
--- a/include/xen/interface/io/sndif.h
+++ b/include/xen/interface/io/sndif.h
@@ -38,6 +38,13 @@
 
 /*
  ******************************************************************************
+ *                           Protocol version
+ ******************************************************************************
+ */
+#define XENDISPL_PROTOCOL_VERSION	"2"
+
+/*
+ ******************************************************************************
  *                  Feature and Parameter Negotiation
  ******************************************************************************
  *
@@ -104,8 +111,10 @@
  * /local/domain/1/device/vsnd/0/0/0/sample-formats = "s8,u8"
  * /local/domain/1/device/vsnd/0/0/0/unique-id = "0"
  *
- * /local/domain/1/device/vsnd/0/0/0/ring-ref = "386"
- * /local/domain/1/device/vsnd/0/0/0/event-channel = "15"
+ * /local/domain/1/device/vsnd/0/0/0/req-ring-ref = "386"
+ * /local/domain/1/device/vsnd/0/0/0/req-event-channel = "15"
+ * /local/domain/1/device/vsnd/0/0/0/evt-ring-ref = "1386"
+ * /local/domain/1/device/vsnd/0/0/0/evt-event-channel = "215"
  *
  *------------------------------ Stream 1, capture ----------------------------
  *
@@ -113,8 +122,10 @@
  * /local/domain/1/device/vsnd/0/0/1/channels-max = "2"
  * /local/domain/1/device/vsnd/0/0/1/unique-id = "1"
  *
- * /local/domain/1/device/vsnd/0/0/1/ring-ref = "384"
- * /local/domain/1/device/vsnd/0/0/1/event-channel = "13"
+ * /local/domain/1/device/vsnd/0/0/1/req-ring-ref = "384"
+ * /local/domain/1/device/vsnd/0/0/1/req-event-channel = "13"
+ * /local/domain/1/device/vsnd/0/0/1/evt-ring-ref = "1384"
+ * /local/domain/1/device/vsnd/0/0/1/evt-event-channel = "213"
  *
  *------------------------------- PCM device 1 --------------------------------
  *
@@ -126,8 +137,10 @@
  * /local/domain/1/device/vsnd/0/1/0/type = "c"
  * /local/domain/1/device/vsnd/0/1/0/unique-id = "2"
  *
- * /local/domain/1/device/vsnd/0/1/0/ring-ref = "387"
- * /local/domain/1/device/vsnd/0/1/0/event-channel = "151"
+ * /local/domain/1/device/vsnd/0/1/0/req-ring-ref = "387"
+ * /local/domain/1/device/vsnd/0/1/0/req-event-channel = "151"
+ * /local/domain/1/device/vsnd/0/1/0/evt-ring-ref = "1387"
+ * /local/domain/1/device/vsnd/0/1/0/evt-event-channel = "351"
  *
  *------------------------------- PCM device 2 --------------------------------
  *
@@ -138,8 +151,10 @@
  * /local/domain/1/device/vsnd/0/2/0/type = "p"
  * /local/domain/1/device/vsnd/0/2/0/unique-id = "3"
  *
- * /local/domain/1/device/vsnd/0/2/0/ring-ref = "389"
- * /local/domain/1/device/vsnd/0/2/0/event-channel = "152"
+ * /local/domain/1/device/vsnd/0/2/0/req-ring-ref = "389"
+ * /local/domain/1/device/vsnd/0/2/0/req-event-channel = "152"
+ * /local/domain/1/device/vsnd/0/2/0/evt-ring-ref = "1389"
+ * /local/domain/1/device/vsnd/0/2/0/evt-event-channel = "452"
  *
  ******************************************************************************
  *                            Backend XenBus Nodes
@@ -273,13 +288,30 @@
  *
  *-------------------- Stream Request Transport Parameters --------------------
  *
- * event-channel
+ * req-event-channel
+ *      Values:         <uint32_t>
+ *
+ *      The identifier of the Xen event channel used to signal activity
+ *      in the ring buffer.
+ *
+ * req-ring-ref
+ *      Values:         <uint32_t>
+ *
+ *      The Xen grant reference granting permission for the backend to map
+ *      a sole page in a single page sized ring buffer.
+ *
+ *--------------------- Stream Event Transport Parameters ---------------------
+ *
+ * This communication path is used to deliver asynchronous events from backend
+ * to frontend, set up per stream.
+ *
+ * evt-event-channel
  *      Values:         <uint32_t>
  *
  *      The identifier of the Xen event channel used to signal activity
  *      in the ring buffer.
  *
- * ring-ref
+ * evt-ring-ref
  *      Values:         <uint32_t>
  *
  *      The Xen grant reference granting permission for the backend to map
@@ -432,6 +464,19 @@
 #define XENSND_OP_GET_VOLUME		5
 #define XENSND_OP_MUTE			6
 #define XENSND_OP_UNMUTE		7
+#define XENSND_OP_TRIGGER		8
+
+#define XENSND_OP_TRIGGER_START		0
+#define XENSND_OP_TRIGGER_PAUSE		1
+#define XENSND_OP_TRIGGER_STOP		2
+#define XENSND_OP_TRIGGER_RESUME	3
+
+/*
+ ******************************************************************************
+ *                                 EVENT CODES
+ ******************************************************************************
+ */
+#define XENSND_EVT_CUR_POS		0
 
 /*
  ******************************************************************************
@@ -446,8 +491,10 @@
 #define XENSND_FIELD_FE_VERSION		"version"
 #define XENSND_FIELD_VCARD_SHORT_NAME	"short-name"
 #define XENSND_FIELD_VCARD_LONG_NAME	"long-name"
-#define XENSND_FIELD_RING_REF		"ring-ref"
-#define XENSND_FIELD_EVT_CHNL		"event-channel"
+#define XENSND_FIELD_REQ_RING_REF	"req-ring-ref"
+#define XENSND_FIELD_REQ_EVT_CHNL	"req-event-channel"
+#define XENSND_FIELD_EVT_RING_REF	"evt-ring-ref"
+#define XENSND_FIELD_EVT_EVT_CHNL	"evt-event-channel"
 #define XENSND_FIELD_DEVICE_NAME	"name"
 #define XENSND_FIELD_TYPE		"type"
 #define XENSND_FIELD_STREAM_UNIQUE_ID	"unique-id"
@@ -559,9 +606,7 @@
  * +----------------+----------------+----------------+----------------+
  * |                           gref_directory                          | 24
  * +----------------+----------------+----------------+----------------+
- * |                             reserved                              | 28
- * +----------------+----------------+----------------+----------------+
- * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * |                             period_sz                             | 28
  * +----------------+----------------+----------------+----------------+
  * |                             reserved                              | 32
  * +----------------+----------------+----------------+----------------+
@@ -571,6 +616,14 @@
  * pcm_channels - uint8_t, number of channels of this stream,
  *   [channels-min; channels-max]
  * buffer_sz - uint32_t, buffer size to be allocated, octets
+ * period_sz - uint32_t, recommended event period size, octets
+ *   This is the recommended (hint) value of the period at which frontend would
+ *   like to receive XENSND_EVT_CUR_POS notifications from the backend when
+ *   stream position advances during playback/capture.
+ *   It shows how many octets are expected to be played/captured before
+ *   sending such an event.
+ *   If set to 0 no XENSND_EVT_CUR_POS events are sent by the backend.
+ *
  * gref_directory - grant_ref_t, a reference to the first shared page
  *   describing shared buffer references. At least one page exists. If shared
  *   buffer size  (buffer_sz) exceeds what can be addressed by this single page,
@@ -585,6 +638,11 @@ struct xensnd_open_req {
 	uint16_t reserved;
 	uint32_t buffer_sz;
 	grant_ref_t gref_directory;
+	uint32_t period_sz;
+};
+
+struct xensnd_trigger_req {
+	uint8_t type;
 };
 
 /*
@@ -767,8 +825,51 @@ struct xensnd_rw_req {
  * id - uint16_t, copied from the request
  * operation - uint8_t, XENSND_OP_* - copied from request
  * status - int32_t, response status, zero on success and -XEN_EXX on failure
+ *
+ *----------------------------------- Events ----------------------------------
+ *
+ * Events are sent via a shared page allocated by the front and propagated by
+ *   evt-event-channel/evt-ring-ref XenStore entries
+ * All event packets have the same length (32 octets)
+ * All event packets have common header:
+ *         0                1                 2               3        octet
+ * +----------------+----------------+----------------+----------------+
+ * |               id                |      type      |   reserved     | 4
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 8
+ * +----------------+----------------+----------------+----------------+
+ *
+ * id - uint16_t, event id, may be used by front
+ * type - uint8_t, type of the event
+ *
+ *
+ * Current stream position - event from back to front when stream's
+ *   playback/capture position has advanced:
+ *         0                1                 2               3        octet
+ * +----------------+----------------+----------------+----------------+
+ * |               id                |   _EVT_CUR_POS |   reserved     | 4
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 8
+ * +----------------+----------------+----------------+----------------+
+ * |                         position low 32-bit                       | 12
+ * +----------------+----------------+----------------+----------------+
+ * |                         position high 32-bit                      | 16
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 20
+ * +----------------+----------------+----------------+----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 32
+ * +----------------+----------------+----------------+----------------+
+ *
+ * position - current value of stream's playback/capture position, octets
+ *
  */
 
+struct xensnd_cur_pos_evt {
+	uint64_t position;
+};
+
 struct xensnd_req {
 	uint16_t id;
 	uint8_t operation;
@@ -776,6 +877,7 @@ struct xensnd_req {
 	union {
 		struct xensnd_open_req open;
 		struct xensnd_rw_req rw;
+		struct xensnd_trigger_req trigger;
 		uint8_t reserved[24];
 	} op;
 };
@@ -788,6 +890,47 @@ struct xensnd_resp {
 	uint8_t reserved1[24];
 };
 
+struct xensnd_evt {
+	uint16_t id;
+	uint8_t type;
+	uint8_t reserved[5];
+	union {
+		struct xensnd_cur_pos_evt cur_pos;
+		uint8_t reserved[24];
+	} op;
+};
+
 DEFINE_RING_TYPES(xen_sndif, struct xensnd_req, struct xensnd_resp);
 
+/*
+ ******************************************************************************
+ *                        Back to front events delivery
+ ******************************************************************************
+ * In order to deliver asynchronous events from back to front a shared page is
+ * allocated by front and its granted reference propagated to back via
+ * XenStore entries (evt-ring-ref/evt-event-channel).
+ * This page has a common header used by both front and back to synchronize
+ * access and control event's ring buffer, while back being a producer of the
+ * events and front being a consumer. The rest of the page after the header
+ * is used for event packets.
+ *
+ * Upon reception of an event(s) front may confirm its reception
+ * for either each event, group of events or none.
+ */
+
+struct xensnd_event_page {
+	uint32_t in_cons;
+	uint32_t in_prod;
+	uint8_t reserved[24];
+};
+
+#define XENSND_EVENT_PAGE_SIZE XEN_PAGE_SIZE
+#define XENSND_IN_RING_OFFS (sizeof(struct xensnd_event_page))
+#define XENSND_IN_RING_SIZE (XENSND_EVENT_PAGE_SIZE - XENSND_IN_RING_OFFS)
+#define XENSND_IN_RING_LEN (XENSND_IN_RING_SIZE / sizeof(struct xensnd_evt))
+#define XENSND_IN_RING(page) \
+	((struct xensnd_evt *)((char *)(page) + XENSND_IN_RING_OFFS))
+#define XENSND_IN_RING_REF(page, idx) \
+	(XENSND_IN_RING((page))[(idx) % XENSND_IN_RING_LEN])
+
 #endif /* __XEN_PUBLIC_IO_SNDIF_H__ */

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [alsa-devel] [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-09-19  8:57                           ` Oleksandr Andrushchenko
@ 2017-09-26 11:35                             ` Oleksandr Andrushchenko
  2017-10-04  6:50                               ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-09-26 11:35 UTC (permalink / raw)
  To: Oleksandr Grytsov, Clemens Ladisch, Takashi Sakamoto, tiwai
  Cc: alsa-devel, Oleksandr Andrushchenko, linux-kernel, xen-devel,
	Oleksandr Grytsov

Clemens, Sakamoto-san,

could you please review the below if you by chance have a minute?

Thank you,
Oleksandr

On 09/19/2017 11:57 AM, Oleksandr Andrushchenko wrote:
> Hi, all!
>
> We did some work on implementing the idea with
>
> feedback events from the backend to the frontend.
>
> Please see attached the changes to the existing sndif protocol [1]:
>
> 1. Introduced a new event channel from back to front
>
> 2. New event with number of bytes played/captured (XENSND_EVT_CUR_POS,
>
> to be used for sending snd_pcm_period_elapsed at frontend.
>
> Sent in bytes, not frames to make the protocol generic and consistent)
>
> 3. New request for playback/capture control (XENSND_OP_TRIGGER)
>
> with start/pause/stop/resume sub-ops.
>
> The implementation we have showed that this is sufficient to
> successfully play/capture w/o using emulated interrupts.
>
> Clemens, Sakamoto-san,
> could you please review the changes and confirm that these are ok to
> be upstreamed to the sndif protocol and are enough for the frontend
> driver we want to upstream (we have it implemented, just need to make
> sure the general approach is accepted by the ALSA community).
>
> Thank you very much for your time,
> Oleksandr Andrushchenko
> Oleksandr Grytsov
>
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/xen/interface/io/sndif.h?h=v4.14-rc1
>
> On 09/12/2017 10:52 AM, Oleksandr Grytsov wrote:
>> On Tue, Sep 5, 2017 at 10:24 AM, Clemens Ladisch <clemens@ladisch.de> 
>> wrote:
>>> Oleksandr Andrushchenko wrote:
>>>>>> We understand that emulated interrupt on the frontend side is 
>>>>>> completely not
>>>>>> acceptable
>>> Allow me to expand on that:  Proper synchronization requires that the
>>> exact position is communicated, not estimated.  Just because the 
>>> nominal
>>> rate of the stream is known does not imply that you know the actual 
>>> rate.
>>> Forget for the moment that there even is a nominal rate; assume that it
>>> works like, e.g., a storage controller, and that you can know that a 
>>> DMA
>>> buffer was consumed by the device only after it has told you.
>>>
>>> It's possible and likely that there is a latency when reporting the
>>> stream position, but that is still better than guessing what the DMA
>>> is doing.  (You would never just try to guess when writing data to
>>> disk, would you?)
>>>
>>>>>> and definitely we need to provide some feedback mechanism from
>>>>>> Dom0 to DomU.
>>>>>>
>>>>>> In our case it is technically impossible to provide precise 
>>>>>> period interrupt
>>>>>> (mostly because our backend is a user space application).
>>> As far as I can see, all audio APIs (ALSA, PulseAudio, etc.) have 
>>> poll()
>>> or callbacks or similar mechanisms to inform you when new data can be
>>> written, and always allow to query the current position.
>>>
>>>> [...]
>>>> ok, so the main concern here is that we cannot properly synchronize 
>>>> Dom0-DomU.
>>>> If we put this apart for a second are there any other concerns on 
>>>> having ALSA
>>>> frontend driver? If not, can we have the driver with timer 
>>>> implementation upstreamed
>>>> as experimental until we have some acceptable synchronization 
>>>> solution?
>>>> This will allow broader audience to try and feel the solution and 
>>>> probably contribute?
>>> I doubt that the driver architecture will stay completely the same, 
>>> so I
>>> do not think that this experimental driver would demonstrate how the
>>> solution would feel.
>>>
>>> As the first step, I would suggest creating a driver with proper
>>> synchronization, even if it has high latency.  Reducing the latency
>>> would then be 'just' an optimization.
>>>
>>>
>>> Regards,
>>> Clemens
>> Definitely feedback from the backend side is required. Currently
>> we are working on synchronized version on the backend
>> and frontend side. We will be back once we have the solution.
>>
>> Thanks.
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [alsa-devel] [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-09-26 11:35                             ` Oleksandr Andrushchenko
@ 2017-10-04  6:50                               ` Oleksandr Andrushchenko
  2017-10-13  6:15                                 ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-10-04  6:50 UTC (permalink / raw)
  To: Oleksandr Grytsov, Clemens Ladisch, Takashi Sakamoto, tiwai
  Cc: alsa-devel, Oleksandr Andrushchenko, linux-kernel, xen-devel,
	Oleksandr Grytsov

gentle reminder

On 09/26/2017 02:35 PM, Oleksandr Andrushchenko wrote:
> Clemens, Sakamoto-san,
>
> could you please review the below if you by chance have a minute?
>
> Thank you,
> Oleksandr
>
> On 09/19/2017 11:57 AM, Oleksandr Andrushchenko wrote:
>> Hi, all!
>>
>> We did some work on implementing the idea with
>>
>> feedback events from the backend to the frontend.
>>
>> Please see attached the changes to the existing sndif protocol [1]:
>>
>> 1. Introduced a new event channel from back to front
>>
>> 2. New event with number of bytes played/captured (XENSND_EVT_CUR_POS,
>>
>> to be used for sending snd_pcm_period_elapsed at frontend.
>>
>> Sent in bytes, not frames to make the protocol generic and consistent)
>>
>> 3. New request for playback/capture control (XENSND_OP_TRIGGER)
>>
>> with start/pause/stop/resume sub-ops.
>>
>> The implementation we have showed that this is sufficient to
>> successfully play/capture w/o using emulated interrupts.
>>
>> Clemens, Sakamoto-san,
>> could you please review the changes and confirm that these are ok to
>> be upstreamed to the sndif protocol and are enough for the frontend
>> driver we want to upstream (we have it implemented, just need to make
>> sure the general approach is accepted by the ALSA community).
>>
>> Thank you very much for your time,
>> Oleksandr Andrushchenko
>> Oleksandr Grytsov
>>
>> [1] 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/xen/interface/io/sndif.h?h=v4.14-rc1
>>
>> On 09/12/2017 10:52 AM, Oleksandr Grytsov wrote:
>>> On Tue, Sep 5, 2017 at 10:24 AM, Clemens Ladisch 
>>> <clemens@ladisch.de> wrote:
>>>> Oleksandr Andrushchenko wrote:
>>>>>>> We understand that emulated interrupt on the frontend side is 
>>>>>>> completely not
>>>>>>> acceptable
>>>> Allow me to expand on that:  Proper synchronization requires that the
>>>> exact position is communicated, not estimated.  Just because the 
>>>> nominal
>>>> rate of the stream is known does not imply that you know the actual 
>>>> rate.
>>>> Forget for the moment that there even is a nominal rate; assume 
>>>> that it
>>>> works like, e.g., a storage controller, and that you can know that 
>>>> a DMA
>>>> buffer was consumed by the device only after it has told you.
>>>>
>>>> It's possible and likely that there is a latency when reporting the
>>>> stream position, but that is still better than guessing what the DMA
>>>> is doing.  (You would never just try to guess when writing data to
>>>> disk, would you?)
>>>>
>>>>>>> and definitely we need to provide some feedback mechanism from
>>>>>>> Dom0 to DomU.
>>>>>>>
>>>>>>> In our case it is technically impossible to provide precise 
>>>>>>> period interrupt
>>>>>>> (mostly because our backend is a user space application).
>>>> As far as I can see, all audio APIs (ALSA, PulseAudio, etc.) have 
>>>> poll()
>>>> or callbacks or similar mechanisms to inform you when new data can be
>>>> written, and always allow to query the current position.
>>>>
>>>>> [...]
>>>>> ok, so the main concern here is that we cannot properly 
>>>>> synchronize Dom0-DomU.
>>>>> If we put this apart for a second are there any other concerns on 
>>>>> having ALSA
>>>>> frontend driver? If not, can we have the driver with timer 
>>>>> implementation upstreamed
>>>>> as experimental until we have some acceptable synchronization 
>>>>> solution?
>>>>> This will allow broader audience to try and feel the solution and 
>>>>> probably contribute?
>>>> I doubt that the driver architecture will stay completely the same, 
>>>> so I
>>>> do not think that this experimental driver would demonstrate how the
>>>> solution would feel.
>>>>
>>>> As the first step, I would suggest creating a driver with proper
>>>> synchronization, even if it has high latency.  Reducing the latency
>>>> would then be 'just' an optimization.
>>>>
>>>>
>>>> Regards,
>>>> Clemens
>>> Definitely feedback from the backend side is required. Currently
>>> we are working on synchronized version on the backend
>>> and frontend side. We will be back once we have the solution.
>>>
>>> Thanks.
>>
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [alsa-devel] [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-10-04  6:50                               ` Oleksandr Andrushchenko
@ 2017-10-13  6:15                                 ` Oleksandr Andrushchenko
  2017-10-30  6:33                                   ` [RFC] " Oleksandr Andrushchenko
  0 siblings, 1 reply; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-10-13  6:15 UTC (permalink / raw)
  To: Oleksandr Grytsov, Clemens Ladisch, Takashi Sakamoto, tiwai
  Cc: alsa-devel, Oleksandr Andrushchenko, linux-kernel, xen-devel,
	Oleksandr Grytsov

ping

On 10/04/2017 09:50 AM, Oleksandr Andrushchenko wrote:
> gentle reminder
>
> On 09/26/2017 02:35 PM, Oleksandr Andrushchenko wrote:
>> Clemens, Sakamoto-san,
>>
>> could you please review the below if you by chance have a minute?
>>
>> Thank you,
>> Oleksandr
>>
>> On 09/19/2017 11:57 AM, Oleksandr Andrushchenko wrote:
>>> Hi, all!
>>>
>>> We did some work on implementing the idea with
>>>
>>> feedback events from the backend to the frontend.
>>>
>>> Please see attached the changes to the existing sndif protocol [1]:
>>>
>>> 1. Introduced a new event channel from back to front
>>>
>>> 2. New event with number of bytes played/captured (XENSND_EVT_CUR_POS,
>>>
>>> to be used for sending snd_pcm_period_elapsed at frontend.
>>>
>>> Sent in bytes, not frames to make the protocol generic and consistent)
>>>
>>> 3. New request for playback/capture control (XENSND_OP_TRIGGER)
>>>
>>> with start/pause/stop/resume sub-ops.
>>>
>>> The implementation we have showed that this is sufficient to
>>> successfully play/capture w/o using emulated interrupts.
>>>
>>> Clemens, Sakamoto-san,
>>> could you please review the changes and confirm that these are ok to
>>> be upstreamed to the sndif protocol and are enough for the frontend
>>> driver we want to upstream (we have it implemented, just need to make
>>> sure the general approach is accepted by the ALSA community).
>>>
>>> Thank you very much for your time,
>>> Oleksandr Andrushchenko
>>> Oleksandr Grytsov
>>>
>>> [1] 
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/xen/interface/io/sndif.h?h=v4.14-rc1
>>>
>>> On 09/12/2017 10:52 AM, Oleksandr Grytsov wrote:
>>>> On Tue, Sep 5, 2017 at 10:24 AM, Clemens Ladisch 
>>>> <clemens@ladisch.de> wrote:
>>>>> Oleksandr Andrushchenko wrote:
>>>>>>>> We understand that emulated interrupt on the frontend side is 
>>>>>>>> completely not
>>>>>>>> acceptable
>>>>> Allow me to expand on that:  Proper synchronization requires that the
>>>>> exact position is communicated, not estimated.  Just because the 
>>>>> nominal
>>>>> rate of the stream is known does not imply that you know the 
>>>>> actual rate.
>>>>> Forget for the moment that there even is a nominal rate; assume 
>>>>> that it
>>>>> works like, e.g., a storage controller, and that you can know that 
>>>>> a DMA
>>>>> buffer was consumed by the device only after it has told you.
>>>>>
>>>>> It's possible and likely that there is a latency when reporting the
>>>>> stream position, but that is still better than guessing what the DMA
>>>>> is doing.  (You would never just try to guess when writing data to
>>>>> disk, would you?)
>>>>>
>>>>>>>> and definitely we need to provide some feedback mechanism from
>>>>>>>> Dom0 to DomU.
>>>>>>>>
>>>>>>>> In our case it is technically impossible to provide precise 
>>>>>>>> period interrupt
>>>>>>>> (mostly because our backend is a user space application).
>>>>> As far as I can see, all audio APIs (ALSA, PulseAudio, etc.) have 
>>>>> poll()
>>>>> or callbacks or similar mechanisms to inform you when new data can be
>>>>> written, and always allow to query the current position.
>>>>>
>>>>>> [...]
>>>>>> ok, so the main concern here is that we cannot properly 
>>>>>> synchronize Dom0-DomU.
>>>>>> If we put this apart for a second are there any other concerns on 
>>>>>> having ALSA
>>>>>> frontend driver? If not, can we have the driver with timer 
>>>>>> implementation upstreamed
>>>>>> as experimental until we have some acceptable synchronization 
>>>>>> solution?
>>>>>> This will allow broader audience to try and feel the solution and 
>>>>>> probably contribute?
>>>>> I doubt that the driver architecture will stay completely the 
>>>>> same, so I
>>>>> do not think that this experimental driver would demonstrate how the
>>>>> solution would feel.
>>>>>
>>>>> As the first step, I would suggest creating a driver with proper
>>>>> synchronization, even if it has high latency.  Reducing the latency
>>>>> would then be 'just' an optimization.
>>>>>
>>>>>
>>>>> Regards,
>>>>> Clemens
>>>> Definitely feedback from the backend side is required. Currently
>>>> we are working on synchronized version on the backend
>>>> and frontend side. We will be back once we have the solution.
>>>>
>>>> Thanks.
>>>
>>
>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [RFC] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-10-13  6:15                                 ` Oleksandr Andrushchenko
@ 2017-10-30  6:33                                   ` Oleksandr Andrushchenko
  2017-11-02  9:44                                     ` Takashi Sakamoto
  0 siblings, 1 reply; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-10-30  6:33 UTC (permalink / raw)
  To: tiwai, alsa-devel, linux-kernel, xen-devel, Konrad Rzeszutek Wilk
  Cc: Oleksandr Andrushchenko, Oleksandr Grytsov, Clemens Ladisch,
	Takashi Sakamoto

Hi, all!

This is an attempt to summarize previous discussions on Xen para-virtual
sound driver.

A first attempt has been made to upstream the driver [1] which brought 
number
of fundamental questions, one of the biggest ones was that the frontend 
driver
has no means to synchronize its period elapsed event with the host driver,
but uses software emulation on the guest side [2] with a timer.
In order to address this a change to the existing Xen para-virtual sound
protocol [3] was proposed to fill this gap [4] and remove emulation:
1. Introduced a new event channel from back to front
2. New event with number of bytes played/captured (XENSND_EVT_CUR_POS,
to be used for sending snd_pcm_period_elapsed at frontend
(in Linux implementation, sent in bytes, not frames to make the protocol
generic and consistent)
3. New request for playback/capture control (XENSND_OP_TRIGGER) with
start/pause/stop/resume sub-ops.

Along with these changes other comments on the driver were addressed,
e.g. split into smaller chunks, moved the driver from misc to xen etc. [5].

Hope, this helps to get the full picture of what was discussed and makes it
possible to move forward: if the approach seems ok, then I'll start
upstreaming the changes to the sndif protocol and then will send the 
updated
version of the driver for the further review.

Thank you,
Oleksandr


[1] https://lkml.org/lkml/2017/8/7/363
[2] https://lkml.org/lkml/2017/8/9/1167
[3] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/xen/interface/io/sndif.h
[4] https://lkml.org/lkml/2017/9/19/121
[5] https://github.com/andr2000/linux/tree/snd_upstream_v1/sound/xen

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-10-30  6:33                                   ` [RFC] " Oleksandr Andrushchenko
@ 2017-11-02  9:44                                     ` Takashi Sakamoto
  2017-11-02  9:55                                       ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 34+ messages in thread
From: Takashi Sakamoto @ 2017-11-02  9:44 UTC (permalink / raw)
  To: Oleksandr Andrushchenko, tiwai, alsa-devel, linux-kernel,
	xen-devel, Konrad Rzeszutek Wilk
  Cc: Oleksandr Andrushchenko, Oleksandr Grytsov, Clemens Ladisch

On Oct 30 2017 15:33, Oleksandr Andrushchenko wrote:
> This is an attempt to summarize previous discussions on Xen para-virtual
> sound driver.
> 
> A first attempt has been made to upstream the driver [1] which brought 
> number
> of fundamental questions, one of the biggest ones was that the frontend 
> driver
> has no means to synchronize its period elapsed event with the host driver,
> but uses software emulation on the guest side [2] with a timer.
> In order to address this a change to the existing Xen para-virtual sound
> protocol [3] was proposed to fill this gap [4] and remove emulation:
> 1. Introduced a new event channel from back to front
> 2. New event with number of bytes played/captured (XENSND_EVT_CUR_POS,
> to be used for sending snd_pcm_period_elapsed at frontend
> (in Linux implementation, sent in bytes, not frames to make the protocol
> generic and consistent)
> 3. New request for playback/capture control (XENSND_OP_TRIGGER) with
> start/pause/stop/resume sub-ops.
> 
> Along with these changes other comments on the driver were addressed,
> e.g. split into smaller chunks, moved the driver from misc to xen etc. [5].
> 
> Hope, this helps to get the full picture of what was discussed and makes it
> possible to move forward: if the approach seems ok, then I'll start
> upstreaming the changes to the sndif protocol and then will send the 
> updated
> version of the driver for the further review.

This message has below line in its header.

 > In-Reply-To: <e56a09e9-da66-b748-4e82-4b96a18cef32@gmail.com>

This field is defined in RFC822[1], and recent mail clients use this 
header field to associate the message to a message which the field 
indicates. This results in a series of messages, so-called 'message 
thread'. Iwai-san would like you to start a new message thread for your 
topic. Would you please post this message again without the header field?

Generally, receiving no reactions means that readers/reviewers don't get 
enough information for your idea yet. (Of course, there's a probability 
that your work attracts no one...) In this case, submitting more 
resources is better, rather than requesting comments to them. For 
instance, you can point links to backend/frontend implementation as 
para-virtualization drivers which use the new feature of interface, if 
you did work for it. Indicating procedure to use a series of your work 
is better for test, if possible.

[1] https://www.ietf.org/rfc/rfc0822.txt

Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [RFC] ALSA: vsnd: Add Xen para-virtualized frontend driver
  2017-11-02  9:44                                     ` Takashi Sakamoto
@ 2017-11-02  9:55                                       ` Oleksandr Andrushchenko
  0 siblings, 0 replies; 34+ messages in thread
From: Oleksandr Andrushchenko @ 2017-11-02  9:55 UTC (permalink / raw)
  To: Takashi Sakamoto, tiwai, alsa-devel, linux-kernel, xen-devel,
	Konrad Rzeszutek Wilk
  Cc: Oleksandr Andrushchenko, Oleksandr Grytsov, Clemens Ladisch


On 11/02/2017 11:44 AM, Takashi Sakamoto wrote:
> On Oct 30 2017 15:33, Oleksandr Andrushchenko wrote:
>> This is an attempt to summarize previous discussions on Xen para-virtual
>> sound driver.
>>
>> A first attempt has been made to upstream the driver [1] which 
>> brought number
>> of fundamental questions, one of the biggest ones was that the 
>> frontend driver
>> has no means to synchronize its period elapsed event with the host 
>> driver,
>> but uses software emulation on the guest side [2] with a timer.
>> In order to address this a change to the existing Xen para-virtual sound
>> protocol [3] was proposed to fill this gap [4] and remove emulation:
>> 1. Introduced a new event channel from back to front
>> 2. New event with number of bytes played/captured (XENSND_EVT_CUR_POS,
>> to be used for sending snd_pcm_period_elapsed at frontend
>> (in Linux implementation, sent in bytes, not frames to make the protocol
>> generic and consistent)
>> 3. New request for playback/capture control (XENSND_OP_TRIGGER) with
>> start/pause/stop/resume sub-ops.
>>
>> Along with these changes other comments on the driver were addressed,
>> e.g. split into smaller chunks, moved the driver from misc to xen 
>> etc. [5].
>>
>> Hope, this helps to get the full picture of what was discussed and 
>> makes it
>> possible to move forward: if the approach seems ok, then I'll start
>> upstreaming the changes to the sndif protocol and then will send the 
>> updated
>> version of the driver for the further review.
>
> This message has below line in its header.
>
> > In-Reply-To: <e56a09e9-da66-b748-4e82-4b96a18cef32@gmail.com>
>
> This field is defined in RFC822[1], and recent mail clients use this 
> header field to associate the message to a message which the field 
> indicates. This results in a series of messages, so-called 'message 
> thread'. Iwai-san would like you to start a new message thread for 
> your topic. Would you please post this message again without the 
> header field?
of course, sorry about that
>
> Generally, receiving no reactions means that readers/reviewers don't 
> get enough information for your idea yet. (Of course, there's a 
> probability that your work attracts no one...) In this case, 
> submitting more resources is better, rather than requesting comments 
> to them. For instance, you can point links to backend/frontend 
> implementation as para-virtualization drivers which use the new 
> feature of interface, if you did work for it. Indicating procedure to 
> use a series of your work is better for test, if possible.
>
will do
> [1] https://www.ietf.org/rfc/rfc0822.txt
>
> Regards
>
> Takashi Sakamoto
Thank you,
Oleksandr

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2017-11-02  9:55 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-07 12:22 [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Oleksandr Andrushchenko
2017-08-07 12:22 ` [PATCH RESEND1 01/12] ALSA: vsnd: Introduce Xen para-virtualized sound " Oleksandr Andrushchenko
2017-08-07 12:22 ` [PATCH RESEND1 02/12] ALSA: vsnd: Implement driver's probe/remove Oleksandr Andrushchenko
2017-08-07 12:22 ` [PATCH RESEND1 03/12] ALSA: vsnd: Implement Xen bus state handling Oleksandr Andrushchenko
2017-08-07 12:22 ` [PATCH RESEND1 04/12] ALSA: vsnd: Read sound driver configuration from Xen store Oleksandr Andrushchenko
2017-08-07 12:22 ` [PATCH RESEND1 05/12] ALSA: vsnd: Implement Xen event channel handling Oleksandr Andrushchenko
2017-08-07 12:22 ` [PATCH RESEND1 06/12] ALSA: vsnd: Implement handling of shared buffers Oleksandr Andrushchenko
2017-08-07 12:22 ` [PATCH RESEND1 07/12] ALSA: vsnd: Introduce ALSA virtual sound driver Oleksandr Andrushchenko
2017-08-07 12:22 ` [PATCH RESEND1 08/12] ALSA: vsnd: Initialize virtul sound card Oleksandr Andrushchenko
2017-08-07 12:22 ` [PATCH RESEND1 09/12] ALSA: vsnd: Add timer for period interrupt emulation Oleksandr Andrushchenko
2017-08-07 12:22 ` [PATCH RESEND1 10/12] ALSA: vsnd: Implement ALSA PCM operations Oleksandr Andrushchenko
2017-08-07 12:22 ` [PATCH RESEND1 11/12] ALSA: vsnd: Implement communication with backend Oleksandr Andrushchenko
2017-08-07 12:22 ` [PATCH RESEND1 12/12] ALSA: vsnd: Introduce Kconfig option to enable Xen PV sound Oleksandr Andrushchenko
2017-08-10  3:14 ` [PATCH RESEND1 00/12] ALSA: vsnd: Add Xen para-virtualized frontend driver Takashi Sakamoto
2017-08-10  8:10   ` Oleksandr Andrushchenko
2017-08-17 10:05     ` [Xen-devel] " Oleksandr Grytsov
2017-08-18  5:43       ` Takashi Sakamoto
2017-08-18  5:56         ` Oleksandr Andrushchenko
2017-08-18  7:17           ` Takashi Sakamoto
2017-08-18  7:23             ` Oleksandr Andrushchenko
2017-08-22  2:43               ` Takashi Sakamoto
2017-08-23 14:51                 ` Oleksandr Grytsov
2017-08-24  4:38                   ` Takashi Sakamoto
2017-08-24  7:04                     ` Oleksandr Andrushchenko
2017-09-04  7:21                       ` Oleksandr Andrushchenko
2017-09-05  7:24                       ` [alsa-devel] " Clemens Ladisch
2017-09-12  7:52                         ` Oleksandr Grytsov
2017-09-19  8:57                           ` Oleksandr Andrushchenko
2017-09-26 11:35                             ` Oleksandr Andrushchenko
2017-10-04  6:50                               ` Oleksandr Andrushchenko
2017-10-13  6:15                                 ` Oleksandr Andrushchenko
2017-10-30  6:33                                   ` [RFC] " Oleksandr Andrushchenko
2017-11-02  9:44                                     ` Takashi Sakamoto
2017-11-02  9:55                                       ` Oleksandr Andrushchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).