All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6] sndif: add ABI for Para-virtual sound
@ 2015-01-29 11:01 Oleksandr Dmytryshyn
  2015-02-02 18:52 ` [Embedded-pv-devel] " Dario Faggioli
  2015-02-04 13:54 ` Oleksandr Dmytryshyn
  0 siblings, 2 replies; 6+ messages in thread
From: Oleksandr Dmytryshyn @ 2015-01-29 11:01 UTC (permalink / raw)
  To: xen-devel, embedded-pv-devel
  Cc: Iurii Konovalenko, Keir Fraser, Ian Campbell, Tim Deegan,
	Ian Jackson, Jan Beulich

This is ABI for the two halves of a Para-virtual
sound driver to communicate with each to other.

Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com>
Signed-off-by: Iurii Konovalenko <iurii.konovalenko@globallogic.com>
---
Changes since v1:
 * removed __attribute__((__packed__)) from all structures definitions

Changes since v2:
 * removed all C structures
 * added protocol description between frontend and backend drivers

Changes since v3:
 * fixed some typos
 * renamed XENSND_PCM_FORMAT_FLOAT_** to XENSND_PCM_FORMAT_F32_**
 * renamed XENSND_PCM_FORMAT_FLOAT64_** to XENSND_PCM_FORMAT_F64_**
 * added 'id' field to the request and response packets
 * renamed 'stream_id' to 'stream' in the packets description
 * renamed 'pcm_data_rate' to 'pcm_rate' in the packets description
 * renamed 'pcm_stream_type' to 'pcm_type' in the packets description
 * removed 'stream_id' field from the response packets

Changes since v4:
 * renamed 'stream_id' back to the to 'stream' in the packets description
 * moved 'id' field to the upper position in the response packets

Changes since v5:
 * Slightly reworked request/response packets
 * Size of the request/response packet is changed to the 64 bytes
 * Now parameters for the XENSND_OP_SET_VOLUME/XENSND_OP_GET_VOLUME are
   passed via shared page
 * Added parameters for the XenBus nodes (now each stream can be mapped
   to the defined sound device in the backend using those parameters)
 * Added XenBus state diagrams description

 xen/include/public/io/sndif.h | 421 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 421 insertions(+)
 create mode 100644 xen/include/public/io/sndif.h

diff --git a/xen/include/public/io/sndif.h b/xen/include/public/io/sndif.h
new file mode 100644
index 0000000..f2e080b
--- /dev/null
+++ b/xen/include/public/io/sndif.h
@@ -0,0 +1,421 @@
+/******************************************************************************
+ * sndif.h
+ *
+ * Unified sound-device I/O interface for Xen guest OSes.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Copyright (C) 2013-2015 GlobalLogic Inc.
+ */
+
+#ifndef __XEN_PUBLIC_IO_XENSND_H__
+#define __XEN_PUBLIC_IO_XENSND_H__
+
+/*
+ * Front->back notifications: When enqueuing a new request, sending a
+ * notification can be made conditional on req_event (i.e., the generic
+ * hold-off mechanism provided by the ring macros). Backends must set
+ * req_event appropriately (e.g., using RING_FINAL_CHECK_FOR_REQUESTS()).
+ *
+ * Back->front notifications: When enqueuing a new response, sending a
+ * notification can be made conditional on rsp_event (i.e., the generic
+ * hold-off mechanism provided by the ring macros). Frontends must set
+ * rsp_event appropriately (e.g., using RING_FINAL_CHECK_FOR_RESPONSES()).
+ */
+
+/*
+ * Feature and Parameter Negotiation
+ * =================================
+ * The two halves of a Para-virtual sound card driver utilize nodes within the
+ * XenStore to communicate capabilities and to negotiate operating parameters.
+ * This section enumerates these nodes which reside in the respective front and
+ * backend portions of the XenStore, following the XenBus convention.
+ *
+ * All data in the XenStore is stored as strings.  Nodes specifying numeric
+ * values are encoded in decimal.  Integer value ranges listed below are
+ * expressed as fixed sized integer types capable of storing the conversion
+ * of a properly formated node string, without loss of information.
+ *
+ *****************************************************************************
+ *                            Backend XenBus Nodes
+ *****************************************************************************
+ *
+ *------------------------- Backend Device parameters -------------------------
+ *
+ * devid
+ *      Values:         <uint32_t>
+ *
+ *      Index of the soundcard which will be created in the frontend. This
+ *      index is zero based.
+ *
+ * streams_cnt
+ *      Values:         <uint32_t>
+ *
+ *      Streams count for virtualized soundcard.
+ *
+ *----------------------------- Streams settings ------------------------------
+ *
+ * Every virtualized device has own set of the sound streams. Each stream
+ * parameter is with index "%u" and defined as 'stream%u_???'. Stream index is
+ * zero based and should be continuous in range from 0 to 'streams_cnt' - 1.
+ *
+ * stream%u_channels
+ *      Values:         <uint32_t>
+ *
+ *      The maximum amount of channels that can be supported by this stream.
+ *      Should be from 1 to XENSND_MAX_CHANNELS_PER_STREAM.
+ *
+ * stream%u_type
+ *      Values:         "p", "c"
+ *
+ *      Stream type: "p" - playback stream, "c" - capture stream.
+ *
+ * stream%u_bedev
+ *      Values:         string
+ *
+ *      Name of the sound device which is mapped to this stream by the backend.
+ *
+ * stream%u_devid
+ *      Values:         <uint32_t>
+ *
+ *      Index of the PCM instance which is created by the soundcard
+ *      in the frontend.
+ *
+ *****************************************************************************
+ *                            Frontend XenBus Nodes
+ *****************************************************************************
+ *
+ *----------------------- Request Transport Parameters -----------------------
+ *
+ * event-channel
+ *      Values:         <uint32_t>
+ *
+ *      The identifier of the Xen event channel used to signal activity
+ *      in the ring buffer.
+ *
+ * ring-ref
+ *      Values:         <uint32_t>
+ *
+ *      The Xen grant reference granting permission for the backend to map
+ *      the sole page in a single page sized ring buffer.
+ */
+
+/*
+ * STATE DIAGRAMS
+ *
+ *****************************************************************************
+ *                                   Startup                                 *
+ *****************************************************************************
+ *
+ * Tool stack creates front and back nodes with state XenbusStateInitialising.
+ *
+ * Front                                Back
+ * =================================    =====================================
+ * XenbusStateInitialising              XenbusStateInitialising
+ *  o Query virtual device               o Query backend device identification
+ *    properties.                          data.
+ *  o Setup OS device instance.          o Open and validate backend device.
+ *                                       o Publish backend features and
+ *                                         transport parameters.
+ *                                                      |
+ *                                                      |
+ *                                                      V
+ *                                      XenbusStateInitWait
+ *
+ * o Query backend features and
+ *   transport parameters.
+ * o Allocate and initialize the
+ *   request ring.
+ * o Publish transport parameters
+ *   that will be in effect during
+ *   this connection.
+ *              |
+ *              |
+ *              V
+ * XenbusStateInitialised
+ *
+ *                                       o Query frontend transport parameters.
+ *                                       o Connect to the request ring and
+ *                                         event channel.
+ *                                       o Publish backend device properties.
+ *                                                      |
+ *                                                      |
+ *                                                      V
+ *                                      XenbusStateConnected
+ *
+ *  o Query backend device properties.
+ *  o Finalize OS virtual device
+ *    instance.
+ *              |
+ *              |
+ *              V
+ * XenbusStateConnected
+ *
+ * Note: Drivers that do not support any optional features, or the negotiation
+ *       of transport parameters, can skip certain states in the state machine:
+ *
+ *       o A frontend may transition to XenbusStateInitialised without
+ *         waiting for the backend to enter XenbusStateInitWait.  In this
+ *         case, default transport parameters are in effect and any
+ *         transport parameters published by the frontend must contain
+ *         their default values.
+ *
+ *       o A backend may transition to XenbusStateInitialised, bypassing
+ *         XenbusStateInitWait, without waiting for the frontend to first
+ *         enter the XenbusStateInitialised state.  In this case, default
+ *         transport parameters are in effect and any transport parameters
+ *         published by the backend must contain their default values.
+ *
+ *       Drivers that support optional features and/or transport parameter
+ *       negotiation must tolerate these additional state transition paths.
+ *       In general this means performing the work of any skipped state
+ *       transition, if it has not already been performed, in addition to the
+ *       work associated with entry into the current state.
+ */
+
+/*
+ * PCM FORMATS
+ *
+ * XENSND_PCM_FORMAT_<format>[_<endian>]
+ *
+ * format: <S/U/F><bits> or <name>
+ *     S - signed, U - unsigned, F - float
+ *     bits - 8, 16, 24, 32
+ *     name - MU_LAW, GSM, etc.
+ *
+ * endian: <LE/BE>, may be absent
+ *     LE - Little endian, BE - Big endian
+ */
+#define XENSND_PCM_FORMAT_S8            0
+#define XENSND_PCM_FORMAT_U8            1
+#define XENSND_PCM_FORMAT_S16_LE        2
+#define XENSND_PCM_FORMAT_S16_BE        3
+#define XENSND_PCM_FORMAT_U16_LE        4
+#define XENSND_PCM_FORMAT_U16_BE        5
+#define XENSND_PCM_FORMAT_S24_LE        6
+#define XENSND_PCM_FORMAT_S24_BE        7
+#define XENSND_PCM_FORMAT_U24_LE        8
+#define XENSND_PCM_FORMAT_U24_BE        9
+#define XENSND_PCM_FORMAT_S32_LE        10
+#define XENSND_PCM_FORMAT_S32_BE        11
+#define XENSND_PCM_FORMAT_U32_LE        12
+#define XENSND_PCM_FORMAT_U32_BE        13
+#define XENSND_PCM_FORMAT_F32_LE        14 /* 4-byte float, IEEE-754 32-bit, */
+#define XENSND_PCM_FORMAT_F32_BE        15 /* range -1.0 to 1.0              */
+#define XENSND_PCM_FORMAT_F64_LE        16 /* 8-byte float, IEEE-754 64-bit, */
+#define XENSND_PCM_FORMAT_F64_BE        17 /* range -1.0 to 1.0              */
+#define XENSND_PCM_FORMAT_IEC958_SUBFRAME_LE 18
+#define XENSND_PCM_FORMAT_IEC958_SUBFRAME_BE 19
+#define XENSND_PCM_FORMAT_MU_LAW        20
+#define XENSND_PCM_FORMAT_A_LAW         21
+#define XENSND_PCM_FORMAT_IMA_ADPCM     22
+#define XENSND_PCM_FORMAT_MPEG          23
+#define XENSND_PCM_FORMAT_GSM           24
+#define XENSND_PCM_FORMAT_SPECIAL       31 /* Any other unspecified format */
+
+/*
+ * REQUEST CODES.
+ */
+#define XENSND_OP_OPEN                  0
+#define XENSND_OP_CLOSE                 1
+#define XENSND_OP_READ                  2
+#define XENSND_OP_WRITE                 3
+#define XENSND_OP_SET_VOLUME            4
+#define XENSND_OP_GET_VOLUME            5
+
+/*
+ * The maximum amount of shared pages which can be used in any request
+ * from the frontend driver to the backend driver
+ */
+#define XENSND_MAX_PAGES_PER_REQUEST    10
+
+/* The maximum amount of channels per virtualized stream */
+#define XENSND_MAX_CHANNELS_PER_STREAM  128
+
+/*
+ * STATUS RETURN CODES.
+ */
+ /* Operation failed for some unspecified reason (e. g. -EIO). */
+#define XENSND_RSP_ERROR                 (-1)
+ /* Operation completed successfully. */
+#define XENSND_RSP_OKAY                  0
+
+/*
+ * Description of the protocol between frontend and backend driver.
+ *
+ * The two halves of a Para-virtual sound driver communicates with
+ * each to other using an shared page and event channel.
+ * Shared page contains a ring with request/response packets.
+ * All fields within the packet are always in little-endian byte order.
+ * Almost all fields within the packet are unsigned except
+ * the field 'status' in the responses packets which is signed.
+ *
+ *
+ * All request packets have the same length (64 bytes)
+ *
+ * Request open - open an pcm stream for playback or capture:
+ *     0    1     2     3     4     5     6     7  octet
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |                      id                       |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       operation       |      stream_idx       |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |      pcm_format       |      pcm_channels     |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       pcm_rate        |       reserved        |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       reserved        |       reserved        |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/+
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       reserved        |       reserved        |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ *
+ * id - private guest value, echoed in resp
+ * operation - XENSND_OP_OPEN
+ * stream_idx - index of the stream (from 0 to 'streams_cnt' - 1.
+ *   'streams_cnt' is read from the XenStore)
+ * pcm_format - XENSND_PCM_FORMAT_???
+ * pcm_channels - channels count in stream
+ * pcm_rate - stream data rate
+ *
+ *
+ * Request close - close an opened pcm stream:
+ *     0    1     2     3     4     5     6     7  octet
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |                      id                       |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       operation       |       stream_idx      |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       reserved        |       reserved        |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/+
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       reserved        |       reserved        |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ *
+ * id - private guest value, echoed in resp
+ * operation - XENSND_OP_CLOSE
+ * stream_idx - index of the stream (from 0 to 'streams_cnt' - 1.
+ *   'streams_cnt' is read from the XenStore)
+ *
+ *
+ * Request read/write - used for read (for capture) or write (for playback):
+ *     0    1     2     3     4     5     6     7  octet
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |                      id                       |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       operation       |       stream_idx      |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |         length        |         gref0         |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |         gref1         |         gref2         |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/+
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |          gref9        |       reserved        |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ *
+ * id - private guest value, echoed in resp
+ * operation - XENSND_OP_READ or XENSND_OP_WRITE
+ * stream_idx - index of the stream (from 0 to 'streams_cnt' - 1.
+ *   'streams_cnt' is read from the XenStore)
+ * length - read or write data length
+ * gref0 - gref9 - references to a grant entries for used pages in read/write
+ * request.
+ *
+ *
+ * Request set volume - set/get channels volume in stream:
+ *     0    1     2     3     4     5     6     7  octet
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |                      id                       |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       operation       |       stream_idx      |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |         gref          |       reserved        |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       reserved        |       reserved        |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/+
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       reserved        |       reserved        |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ *
+ * id - private guest value, echoed in resp
+ * operation - XENSND_OP_SET_VOLUME or XENSND_OP_GET_VOLUME
+ * stream_idx - index of the stream (from 0 to 'streams_cnt' - 1.
+ *   'streams_cnt' is read from the XenStore)
+ * gref - references to a grant entry for page with the volume values
+ *
+ *
+ * Shared page for set/get volume:
+ *
+ *     0    1     2     3     4     5     6     7  octet
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |        vol_ch0        |        vol_ch1        |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |        vol_ch2        |        vol_ch3        |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/+
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       vol_ch126       |       vol_ch127       |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ *
+ * vol_ch0 - vol_ch127 - volume for the channel from 0 to
+ *   XENSND_MAX_CHANNELS_PER_STREAM
+ * Please, note that only first 'stream%u_channels' are used in this command,
+ *   where 'stream%u_channels' is read from the XenStore (channels count for
+ *   stream with index '%u' which equals to 'stream_idx')
+ *
+ *
+ * All response packets have the same length (64 bytes)
+ *
+ * Response for all requests:
+ *     0    1     2     3     4     5     6     7  octet
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |                      id                       |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       operation       |         status        |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       stream_idx      |       reserved        |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       reserved        |       reserved        |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/+
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ * |       reserved        |       reserved        |
+ * +-----+-----+-----+-----+-----+-----+-----+-----+
+ *
+ * id - copied from request
+ * stream_idx - copied from request
+ * operation - XENSND_OP_??? - copied from request
+ * status - XENSND_RSP_???
+ */
+
+struct xensnd_request {
+    uint8_t raw[64];
+};
+
+struct xensnd_response {
+    uint8_t raw[64];
+};
+
+DEFINE_RING_TYPES(xensnd, struct xensnd_request, struct xensnd_response);
+
+#endif /* __XEN_PUBLIC_IO_XENSND_H__ */
-- 
1.9.1

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

* Re: [Embedded-pv-devel] [PATCH v6] sndif: add ABI for Para-virtual sound
  2015-01-29 11:01 [PATCH v6] sndif: add ABI for Para-virtual sound Oleksandr Dmytryshyn
@ 2015-02-02 18:52 ` Dario Faggioli
  2015-02-03  8:32   ` Oleksandr Dmytryshyn
  2015-02-05 19:47   ` Stefano Panella
  2015-02-04 13:54 ` Oleksandr Dmytryshyn
  1 sibling, 2 replies; 6+ messages in thread
From: Dario Faggioli @ 2015-02-02 18:52 UTC (permalink / raw)
  To: Oleksandr Dmytryshyn
  Cc: Iurii Konovalenko, Keir Fraser, Ian Campbell, Ian Jackson,
	Tim Deegan, xen-devel, embedded-pv-devel, stefano.panella,
	Jan Beulich


[-- Attachment #1.1: Type: text/plain, Size: 860 bytes --]

On Thu, 2015-01-29 at 13:01 +0200, Oleksandr Dmytryshyn wrote:
> This is ABI for the two halves of a Para-virtual
> sound driver to communicate with each to other.
> 
I wonder whether Stefano (Cc-ed), which has a great deal of experience
with --real, virtual and paravirtual-- audio, could find a few minutes
to have a look and help us here, by saying what he thinks :-)

Stefano, a bit of context (Oleksandr and Iurii, can add more, if needed,
I'm sure):

http://marc.info/?l=xen-devel&m=142165575819173
http://marc.info/?l=xen-devel&m=142194015107046

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

* Re: [Embedded-pv-devel] [PATCH v6] sndif: add ABI for Para-virtual sound
  2015-02-02 18:52 ` [Embedded-pv-devel] " Dario Faggioli
@ 2015-02-03  8:32   ` Oleksandr Dmytryshyn
  2015-02-05 19:47   ` Stefano Panella
  1 sibling, 0 replies; 6+ messages in thread
From: Oleksandr Dmytryshyn @ 2015-02-03  8:32 UTC (permalink / raw)
  To: Dario Faggioli
  Cc: Iurii Konovalenko, Keir Fraser, Ian Campbell, Ian Jackson,
	Tim Deegan, xen-devel, embedded-pv-devel, stefano.panella,
	Jan Beulich

On Mon, Feb 2, 2015 at 8:52 PM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
>
> On Thu, 2015-01-29 at 13:01 +0200, Oleksandr Dmytryshyn wrote:
> > This is ABI for the two halves of a Para-virtual
> > sound driver to communicate with each to other.
> >
> I wonder whether Stefano (Cc-ed), which has a great deal of experience
> with --real, virtual and paravirtual-- audio, could find a few minutes
> to have a look and help us here, by saying what he thinks :-)
>
> Stefano, a bit of context (Oleksandr and Iurii, can add more, if needed,
> I'm sure):
>
> http://marc.info/?l=xen-devel&m=142165575819173
> http://marc.info/?l=xen-devel&m=142194015107046
This version of the driver will be reworked due to the new sndif.h file.
This driver has hardcoded streams (stream '0' - playback, stream '1' - capture).
I'm reworking this driver now. New driver will read streams configuration
from the XenStore and will be able to map virtualized streams
to real devices.
The first thing is to conform a reworked 'sndif.h' file and then I'll rework
PV sound driver and I'll push it to the review.

> Thanks and Regards,
> Dario
>
> --
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -----------------------------------------------------------------
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
>

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

* Re: [PATCH v6] sndif: add ABI for Para-virtual sound
  2015-01-29 11:01 [PATCH v6] sndif: add ABI for Para-virtual sound Oleksandr Dmytryshyn
  2015-02-02 18:52 ` [Embedded-pv-devel] " Dario Faggioli
@ 2015-02-04 13:54 ` Oleksandr Dmytryshyn
  1 sibling, 0 replies; 6+ messages in thread
From: Oleksandr Dmytryshyn @ 2015-02-04 13:54 UTC (permalink / raw)
  To: xen-devel, embedded-pv-devel
  Cc: Iurii Konovalenko, Keir Fraser, Ian Campbell, Tim Deegan,
	Ian Jackson, Jan Beulich

On Thu, Jan 29, 2015 at 1:01 PM, Oleksandr Dmytryshyn
<oleksandr.dmytryshyn@globallogic.com> wrote:
>
> This is ABI for the two halves of a Para-virtual
> sound driver to communicate with each to other.
>
> Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com>
> Signed-off-by: Iurii Konovalenko <iurii.konovalenko@globallogic.com>
> ---
> Changes since v1:
>  * removed __attribute__((__packed__)) from all structures definitions
>
> Changes since v2:
>  * removed all C structures
>  * added protocol description between frontend and backend drivers
>
> Changes since v3:
>  * fixed some typos
>  * renamed XENSND_PCM_FORMAT_FLOAT_** to XENSND_PCM_FORMAT_F32_**
>  * renamed XENSND_PCM_FORMAT_FLOAT64_** to XENSND_PCM_FORMAT_F64_**
>  * added 'id' field to the request and response packets
>  * renamed 'stream_id' to 'stream' in the packets description
>  * renamed 'pcm_data_rate' to 'pcm_rate' in the packets description
>  * renamed 'pcm_stream_type' to 'pcm_type' in the packets description
>  * removed 'stream_id' field from the response packets
>
> Changes since v4:
>  * renamed 'stream_id' back to the to 'stream' in the packets description
>  * moved 'id' field to the upper position in the response packets
>
> Changes since v5:
>  * Slightly reworked request/response packets
>  * Size of the request/response packet is changed to the 64 bytes
>  * Now parameters for the XENSND_OP_SET_VOLUME/XENSND_OP_GET_VOLUME are
>    passed via shared page
>  * Added parameters for the XenBus nodes (now each stream can be mapped
>    to the defined sound device in the backend using those parameters)
>  * Added XenBus state diagrams description
>
>  xen/include/public/io/sndif.h | 421 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 421 insertions(+)
>  create mode 100644 xen/include/public/io/sndif.h
>
> diff --git a/xen/include/public/io/sndif.h b/xen/include/public/io/sndif.h
> new file mode 100644
> index 0000000..f2e080b
> --- /dev/null
> +++ b/xen/include/public/io/sndif.h
> @@ -0,0 +1,421 @@
> +/******************************************************************************
> + * sndif.h
> + *
> + * Unified sound-device I/O interface for Xen guest OSes.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to
> + * deal in the Software without restriction, including without limitation the
> + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * Copyright (C) 2013-2015 GlobalLogic Inc.
> + */
> +
> +#ifndef __XEN_PUBLIC_IO_XENSND_H__
> +#define __XEN_PUBLIC_IO_XENSND_H__
> +
> +/*
> + * Front->back notifications: When enqueuing a new request, sending a
> + * notification can be made conditional on req_event (i.e., the generic
> + * hold-off mechanism provided by the ring macros). Backends must set
> + * req_event appropriately (e.g., using RING_FINAL_CHECK_FOR_REQUESTS()).
> + *
> + * Back->front notifications: When enqueuing a new response, sending a
> + * notification can be made conditional on rsp_event (i.e., the generic
> + * hold-off mechanism provided by the ring macros). Frontends must set
> + * rsp_event appropriately (e.g., using RING_FINAL_CHECK_FOR_RESPONSES()).
> + */
> +
> +/*
> + * Feature and Parameter Negotiation
> + * =================================
> + * The two halves of a Para-virtual sound card driver utilize nodes within the
> + * XenStore to communicate capabilities and to negotiate operating parameters.
> + * This section enumerates these nodes which reside in the respective front and
> + * backend portions of the XenStore, following the XenBus convention.
> + *
> + * All data in the XenStore is stored as strings.  Nodes specifying numeric
> + * values are encoded in decimal.  Integer value ranges listed below are
> + * expressed as fixed sized integer types capable of storing the conversion
> + * of a properly formated node string, without loss of information.
> + *
> + *****************************************************************************
> + *                            Backend XenBus Nodes
> + *****************************************************************************
> + *
> + *------------------------- Backend Device parameters -------------------------
> + *
> + * devid
> + *      Values:         <uint32_t>
> + *
> + *      Index of the soundcard which will be created in the frontend. This
> + *      index is zero based.
> + *
> + * streams_cnt
> + *      Values:         <uint32_t>
> + *
> + *      Streams count for virtualized soundcard.
> + *
> + *----------------------------- Streams settings ------------------------------
> + *
> + * Every virtualized device has own set of the sound streams. Each stream
> + * parameter is with index "%u" and defined as 'stream%u_???'. Stream index is
> + * zero based and should be continuous in range from 0 to 'streams_cnt' - 1.
> + *
> + * stream%u_channels
> + *      Values:         <uint32_t>
> + *
> + *      The maximum amount of channels that can be supported by this stream.
> + *      Should be from 1 to XENSND_MAX_CHANNELS_PER_STREAM.
> + *
> + * stream%u_type
> + *      Values:         "p", "c"
> + *
> + *      Stream type: "p" - playback stream, "c" - capture stream.
> + *
> + * stream%u_bedev
> + *      Values:         string
> + *
> + *      Name of the sound device which is mapped to this stream by the backend.
> + *
> + * stream%u_devid
> + *      Values:         <uint32_t>
> + *
> + *      Index of the PCM instance which is created by the soundcard
> + *      in the frontend.
> + *
> + *****************************************************************************
> + *                            Frontend XenBus Nodes
> + *****************************************************************************
> + *
> + *----------------------- Request Transport Parameters -----------------------
> + *
> + * event-channel
> + *      Values:         <uint32_t>
> + *
> + *      The identifier of the Xen event channel used to signal activity
> + *      in the ring buffer.
> + *
> + * ring-ref
> + *      Values:         <uint32_t>
> + *
> + *      The Xen grant reference granting permission for the backend to map
> + *      the sole page in a single page sized ring buffer.
> + */
> +
> +/*
> + * STATE DIAGRAMS
> + *
> + *****************************************************************************
> + *                                   Startup                                 *
> + *****************************************************************************
> + *
> + * Tool stack creates front and back nodes with state XenbusStateInitialising.
> + *
> + * Front                                Back
> + * =================================    =====================================
> + * XenbusStateInitialising              XenbusStateInitialising
> + *  o Query virtual device               o Query backend device identification
> + *    properties.                          data.
> + *  o Setup OS device instance.          o Open and validate backend device.
> + *                                       o Publish backend features and
> + *                                         transport parameters.
> + *                                                      |
> + *                                                      |
> + *                                                      V
> + *                                      XenbusStateInitWait
> + *
> + * o Query backend features and
> + *   transport parameters.
> + * o Allocate and initialize the
> + *   request ring.
> + * o Publish transport parameters
> + *   that will be in effect during
> + *   this connection.
> + *              |
> + *              |
> + *              V
> + * XenbusStateInitialised
> + *
> + *                                       o Query frontend transport parameters.
> + *                                       o Connect to the request ring and
> + *                                         event channel.
> + *                                       o Publish backend device properties.
> + *                                                      |
> + *                                                      |
> + *                                                      V
> + *                                      XenbusStateConnected
> + *
> + *  o Query backend device properties.
> + *  o Finalize OS virtual device
> + *    instance.
> + *              |
> + *              |
> + *              V
> + * XenbusStateConnected
> + *
> + * Note: Drivers that do not support any optional features, or the negotiation
> + *       of transport parameters, can skip certain states in the state machine:
> + *
> + *       o A frontend may transition to XenbusStateInitialised without
> + *         waiting for the backend to enter XenbusStateInitWait.  In this
> + *         case, default transport parameters are in effect and any
> + *         transport parameters published by the frontend must contain
> + *         their default values.
> + *
> + *       o A backend may transition to XenbusStateInitialised, bypassing
> + *         XenbusStateInitWait, without waiting for the frontend to first
> + *         enter the XenbusStateInitialised state.  In this case, default
> + *         transport parameters are in effect and any transport parameters
> + *         published by the backend must contain their default values.
> + *
> + *       Drivers that support optional features and/or transport parameter
> + *       negotiation must tolerate these additional state transition paths.
> + *       In general this means performing the work of any skipped state
> + *       transition, if it has not already been performed, in addition to the
> + *       work associated with entry into the current state.
> + */
> +
> +/*
> + * PCM FORMATS
> + *
> + * XENSND_PCM_FORMAT_<format>[_<endian>]
> + *
> + * format: <S/U/F><bits> or <name>
> + *     S - signed, U - unsigned, F - float
> + *     bits - 8, 16, 24, 32
> + *     name - MU_LAW, GSM, etc.
> + *
> + * endian: <LE/BE>, may be absent
> + *     LE - Little endian, BE - Big endian
> + */
> +#define XENSND_PCM_FORMAT_S8            0
> +#define XENSND_PCM_FORMAT_U8            1
> +#define XENSND_PCM_FORMAT_S16_LE        2
> +#define XENSND_PCM_FORMAT_S16_BE        3
> +#define XENSND_PCM_FORMAT_U16_LE        4
> +#define XENSND_PCM_FORMAT_U16_BE        5
> +#define XENSND_PCM_FORMAT_S24_LE        6
> +#define XENSND_PCM_FORMAT_S24_BE        7
> +#define XENSND_PCM_FORMAT_U24_LE        8
> +#define XENSND_PCM_FORMAT_U24_BE        9
> +#define XENSND_PCM_FORMAT_S32_LE        10
> +#define XENSND_PCM_FORMAT_S32_BE        11
> +#define XENSND_PCM_FORMAT_U32_LE        12
> +#define XENSND_PCM_FORMAT_U32_BE        13
> +#define XENSND_PCM_FORMAT_F32_LE        14 /* 4-byte float, IEEE-754 32-bit, */
> +#define XENSND_PCM_FORMAT_F32_BE        15 /* range -1.0 to 1.0              */
> +#define XENSND_PCM_FORMAT_F64_LE        16 /* 8-byte float, IEEE-754 64-bit, */
> +#define XENSND_PCM_FORMAT_F64_BE        17 /* range -1.0 to 1.0              */
> +#define XENSND_PCM_FORMAT_IEC958_SUBFRAME_LE 18
> +#define XENSND_PCM_FORMAT_IEC958_SUBFRAME_BE 19
> +#define XENSND_PCM_FORMAT_MU_LAW        20
> +#define XENSND_PCM_FORMAT_A_LAW         21
> +#define XENSND_PCM_FORMAT_IMA_ADPCM     22
> +#define XENSND_PCM_FORMAT_MPEG          23
> +#define XENSND_PCM_FORMAT_GSM           24
> +#define XENSND_PCM_FORMAT_SPECIAL       31 /* Any other unspecified format */
> +
> +/*
> + * REQUEST CODES.
> + */
> +#define XENSND_OP_OPEN                  0
> +#define XENSND_OP_CLOSE                 1
> +#define XENSND_OP_READ                  2
> +#define XENSND_OP_WRITE                 3
> +#define XENSND_OP_SET_VOLUME            4
> +#define XENSND_OP_GET_VOLUME            5
> +
> +/*
> + * The maximum amount of shared pages which can be used in any request
> + * from the frontend driver to the backend driver
> + */
> +#define XENSND_MAX_PAGES_PER_REQUEST    10
> +
> +/* The maximum amount of channels per virtualized stream */
> +#define XENSND_MAX_CHANNELS_PER_STREAM  128
> +
> +/*
> + * STATUS RETURN CODES.
> + */
> + /* Operation failed for some unspecified reason (e. g. -EIO). */
> +#define XENSND_RSP_ERROR                 (-1)
> + /* Operation completed successfully. */
> +#define XENSND_RSP_OKAY                  0
> +
> +/*
> + * Description of the protocol between frontend and backend driver.
> + *
> + * The two halves of a Para-virtual sound driver communicates with
> + * each to other using an shared page and event channel.
> + * Shared page contains a ring with request/response packets.
> + * All fields within the packet are always in little-endian byte order.
> + * Almost all fields within the packet are unsigned except
> + * the field 'status' in the responses packets which is signed.
> + *
> + *
> + * All request packets have the same length (64 bytes)
> + *
> + * Request open - open an pcm stream for playback or capture:
> + *     0    1     2     3     4     5     6     7  octet
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |                      id                       |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       operation       |      stream_idx       |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |      pcm_format       |      pcm_channels     |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       pcm_rate        |       reserved        |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       reserved        |       reserved        |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/+
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       reserved        |       reserved        |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + *
> + * id - private guest value, echoed in resp
> + * operation - XENSND_OP_OPEN
> + * stream_idx - index of the stream (from 0 to 'streams_cnt' - 1.
> + *   'streams_cnt' is read from the XenStore)
> + * pcm_format - XENSND_PCM_FORMAT_???
> + * pcm_channels - channels count in stream
> + * pcm_rate - stream data rate
> + *
> + *
> + * Request close - close an opened pcm stream:
> + *     0    1     2     3     4     5     6     7  octet
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |                      id                       |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       operation       |       stream_idx      |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       reserved        |       reserved        |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/+
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       reserved        |       reserved        |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + *
> + * id - private guest value, echoed in resp
> + * operation - XENSND_OP_CLOSE
> + * stream_idx - index of the stream (from 0 to 'streams_cnt' - 1.
> + *   'streams_cnt' is read from the XenStore)
> + *
> + *
> + * Request read/write - used for read (for capture) or write (for playback):
> + *     0    1     2     3     4     5     6     7  octet
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |                      id                       |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       operation       |       stream_idx      |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |         length        |         gref0         |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |         gref1         |         gref2         |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/+
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |          gref9        |       reserved        |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + *
> + * id - private guest value, echoed in resp
> + * operation - XENSND_OP_READ or XENSND_OP_WRITE
> + * stream_idx - index of the stream (from 0 to 'streams_cnt' - 1.
> + *   'streams_cnt' is read from the XenStore)
> + * length - read or write data length
> + * gref0 - gref9 - references to a grant entries for used pages in read/write
> + * request.
> + *
> + *
> + * Request set volume - set/get channels volume in stream:
> + *     0    1     2     3     4     5     6     7  octet
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |                      id                       |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       operation       |       stream_idx      |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |         gref          |       reserved        |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       reserved        |       reserved        |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/+
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       reserved        |       reserved        |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + *
> + * id - private guest value, echoed in resp
> + * operation - XENSND_OP_SET_VOLUME or XENSND_OP_GET_VOLUME
> + * stream_idx - index of the stream (from 0 to 'streams_cnt' - 1.
> + *   'streams_cnt' is read from the XenStore)
> + * gref - references to a grant entry for page with the volume values
> + *
> + *
> + * Shared page for set/get volume:
> + *
> + *     0    1     2     3     4     5     6     7  octet
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |        vol_ch0        |        vol_ch1        |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |        vol_ch2        |        vol_ch3        |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/+
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       vol_ch126       |       vol_ch127       |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + *
> + * vol_ch0 - vol_ch127 - volume for the channel from 0 to
> + *   XENSND_MAX_CHANNELS_PER_STREAM
> + * Please, note that only first 'stream%u_channels' are used in this command,
> + *   where 'stream%u_channels' is read from the XenStore (channels count for
> + *   stream with index '%u' which equals to 'stream_idx')
> + *
> + *
> + * All response packets have the same length (64 bytes)
> + *
> + * Response for all requests:
> + *     0    1     2     3     4     5     6     7  octet
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |                      id                       |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       operation       |         status        |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       stream_idx      |       reserved        |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       reserved        |       reserved        |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/+
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + * |       reserved        |       reserved        |
> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> + *
> + * id - copied from request
> + * stream_idx - copied from request
> + * operation - XENSND_OP_??? - copied from request
> + * status - XENSND_RSP_???
> + */
> +
> +struct xensnd_request {
> +    uint8_t raw[64];
> +};
> +
> +struct xensnd_response {
> +    uint8_t raw[64];
> +};
> +
> +DEFINE_RING_TYPES(xensnd, struct xensnd_request, struct xensnd_response);
> +
> +#endif /* __XEN_PUBLIC_IO_XENSND_H__ */
> --
> 1.9.1
Hi to all.
Could anybody confirm that this interface could be used for PV sound driver?

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

* Re: [Embedded-pv-devel] [PATCH v6] sndif: add ABI for Para-virtual sound
  2015-02-02 18:52 ` [Embedded-pv-devel] " Dario Faggioli
  2015-02-03  8:32   ` Oleksandr Dmytryshyn
@ 2015-02-05 19:47   ` Stefano Panella
  2015-02-06 11:10     ` Oleksandr Dmytryshyn
  1 sibling, 1 reply; 6+ messages in thread
From: Stefano Panella @ 2015-02-05 19:47 UTC (permalink / raw)
  To: Dario Faggioli, Oleksandr Dmytryshyn
  Cc: Iurii Konovalenko, Keir (Xen.org), Ian Campbell, Tim (Xen.org),
	xen-devel, embedded-pv-devel, Jan Beulich, Ian Jackson

Hi all,

First of all I would like to say that:
- I am happy that PV audio is finding its way into xen and there is active development on it
- Defining a flexible and future proof PV audio protocol is very hard and will probably take a bit of trial and error in the beginning.
- I hope I will be able to save some time in the trial and error process sharing a bit of my xen audio related experiences.

I had a look at the patches sent and even if I am sure they are solving in a practical way a specific use case scenario, the protocol specified does not look general enough in my opinion and it does many assumptions which might or not be acceptable.

Also I could not find any discussion around the actual design of the protocol including motivations and scope.

I would like to think one could break the design protocol of a PV driver in different areas where different people can contribute at different levels bringing their particular expertise.

Anyway, first it would be good to start a discussion around some basic questions and then we could go into more detail.

- what is the aim of the protocol? Just pipe some sort of sound data from guest and dom0 or expose the guest with a real xen PV soundcard?
- how general/granular/configurable should it be in term of the potentially many and different sound devices present in dom0?
- audio, differently from any other Xen PV driver, is a real-time thing, should this be reflected in the protocol? Most sound driver API require to know when a particular sound sample is played on the Speaker or how old is a sample coming from a Mic (particularly useful for Acoustic Echo Cancellation)
- Should the protocol allow the backend to publish many capabilities so the frontend can chose how to best use them?
- how easy/feasible would be to write frontend and backend drivers using the protocol for different OSs?
- would the protocol map well with the current major sound driver models and API for the different guest Oss? (linux alsa driver, Windows WaveCiclic or WaveRT etc..)

I hope these questions will help moving the conversation in a constructive and productive way for now :)

Stefano

-----Original Message-----
From: Dario Faggioli [mailto:dario.faggioli@citrix.com] 
Sent: Monday, February 2, 2015 6:52 PM
To: Oleksandr Dmytryshyn
Cc: xen-devel@lists.xen.org; embedded-pv-devel@lists.xenproject.org; Iurii Konovalenko; Keir (Xen.org); Ian Campbell; Tim (Xen.org); Ian Jackson; Jan Beulich; Stefano Panella
Subject: Re: [Embedded-pv-devel] [PATCH v6] sndif: add ABI for Para-virtual sound

On Thu, 2015-01-29 at 13:01 +0200, Oleksandr Dmytryshyn wrote:
> This is ABI for the two halves of a Para-virtual sound driver to 
> communicate with each to other.
> 
I wonder whether Stefano (Cc-ed), which has a great deal of experience with --real, virtual and paravirtual-- audio, could find a few minutes to have a look and help us here, by saying what he thinks :-)

Stefano, a bit of context (Oleksandr and Iurii, can add more, if needed, I'm sure):

http://marc.info/?l=xen-devel&m=142165575819173
http://marc.info/?l=xen-devel&m=142194015107046

Thanks and Regards,
Dario

--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

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

* Re: [Embedded-pv-devel] [PATCH v6] sndif: add ABI for Para-virtual sound
  2015-02-05 19:47   ` Stefano Panella
@ 2015-02-06 11:10     ` Oleksandr Dmytryshyn
  0 siblings, 0 replies; 6+ messages in thread
From: Oleksandr Dmytryshyn @ 2015-02-06 11:10 UTC (permalink / raw)
  To: Stefano Panella
  Cc: Iurii Konovalenko, Keir (Xen.org),
	Ian Campbell, Dario Faggioli, Tim (Xen.org),
	xen-devel, embedded-pv-devel, Jan Beulich, Ian Jackson

Hi, Stefano.

Currently we have this configuration:
Dom0, DomD (driver domain), DomU (Android).
Sound driver is inside DomD. Backend uses ALSA for playback/capture.

On Thu, Feb 5, 2015 at 9:47 PM, Stefano Panella
<stefano.panella@citrix.com> wrote:
> Hi all,
>
> First of all I would like to say that:
> - I am happy that PV audio is finding its way into xen and there is active development on it
> - Defining a flexible and future proof PV audio protocol is very hard and will probably take a bit of trial and error in the beginning.
> - I hope I will be able to save some time in the trial and error process sharing a bit of my xen audio related experiences.
>
> I had a look at the patches sent and even if I am sure they are solving in a practical way a specific use case scenario, the protocol specified does not look general enough in my opinion and it does many assumptions which might or not be acceptable.
>
> Also I could not find any discussion around the actual design of the protocol including motivations and scope.
>
> I would like to think one could break the design protocol of a PV driver in different areas where different people can contribute at different levels bringing their particular expertise.
>
> Anyway, first it would be good to start a discussion around some basic questions and then we could go into more detail.
>
> - what is the aim of the protocol? Just pipe some sort of sound data from guest and dom0 or expose the guest with a real xen PV soundcard?
The aim is to expose the guest with a real xen PV soundcard.

> - how general/granular/configurable should it be in term of the potentially many and different sound devices present in dom0?
Each sound device in DomD has own name and it is mapped to the selected
sound device of the PV soundcard in the frontend. I'll little rework the stream
mappings description in this protocol in the next patch-set.

> - audio, differently from any other Xen PV driver, is a real-time thing, should this be reflected in the protocol? Most sound driver API require to know when a particular sound sample is played on the Speaker or how old is a sample coming from a Mic (particularly useful for Acoustic Echo Cancellation)
I don't know exactly how reflect it in the protocol. My PV driver adds
additional latency which is comparable to the context switching.

> - Should the protocol allow the backend to publish many capabilities so the frontend can chose how to best use them?
Yes, it should. Maybe this should be added a bit later to the protocol.
At first it will be simple version of the protocol description.

> - how easy/feasible would be to write frontend and backend drivers using the protocol for different OSs?
It is feasible to write backend and frontend drivers.
The main work should be done:
1. Learn how use ALSA to play/capture data (if ALSA is used for backend driver)
2. Learn how to create and use an virtual soundcard in selected OS
(linux, unix, etc.)
3. Learn how use an event channel and shared memory.

> - would the protocol map well with the current major sound driver models and API for the different guest Oss? (linux alsa driver, Windows WaveCiclic or WaveRT etc..)
At least the protocol map would be well with the alsa-like drivers
(Linux, QNX). BTW we've written a test version of the PV sound frontend
which works on the QNX. Unfurtunatelly I don't know about Windows WaveCiclic

>
> I hope these questions will help moving the conversation in a constructive and productive way for now :)
>
> Stefano
>
> -----Original Message-----
> From: Dario Faggioli [mailto:dario.faggioli@citrix.com]
> Sent: Monday, February 2, 2015 6:52 PM
> To: Oleksandr Dmytryshyn
> Cc: xen-devel@lists.xen.org; embedded-pv-devel@lists.xenproject.org; Iurii Konovalenko; Keir (Xen.org); Ian Campbell; Tim (Xen.org); Ian Jackson; Jan Beulich; Stefano Panella
> Subject: Re: [Embedded-pv-devel] [PATCH v6] sndif: add ABI for Para-virtual sound
>
> On Thu, 2015-01-29 at 13:01 +0200, Oleksandr Dmytryshyn wrote:
>> This is ABI for the two halves of a Para-virtual sound driver to
>> communicate with each to other.
>>
> I wonder whether Stefano (Cc-ed), which has a great deal of experience with --real, virtual and paravirtual-- audio, could find a few minutes to have a look and help us here, by saying what he thinks :-)
>
> Stefano, a bit of context (Oleksandr and Iurii, can add more, if needed, I'm sure):
>
> http://marc.info/?l=xen-devel&m=142165575819173
> http://marc.info/?l=xen-devel&m=142194015107046
>
> Thanks and Regards,
> Dario
>
> --
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -----------------------------------------------------------------
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
>

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

end of thread, other threads:[~2015-02-06 11:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-29 11:01 [PATCH v6] sndif: add ABI for Para-virtual sound Oleksandr Dmytryshyn
2015-02-02 18:52 ` [Embedded-pv-devel] " Dario Faggioli
2015-02-03  8:32   ` Oleksandr Dmytryshyn
2015-02-05 19:47   ` Stefano Panella
2015-02-06 11:10     ` Oleksandr Dmytryshyn
2015-02-04 13:54 ` Oleksandr Dmytryshyn

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.