Re: [Xen-devel] [PATCH v3] sndif: add ABI for Para-virtual sound

2015-01-22 Thread Jan Beulich
 On 22.01.15 at 11:52, oleksandr.dmytrys...@globallogic.com wrote:
 On Thu, Jan 22, 2015 at 12:24 PM, Jan Beulich jbeul...@suse.com wrote:
 On 22.01.15 at 10:56, oleksandr.dmytrys...@globallogic.com wrote:
 + * Responce for all requests exept the XENSND_GET_VOLUME:
 + * 01 2 3 4 5 6 7  octet
 + * +-+-+-+-+-+-+-+-+
 + * |   stream_id   |   operation   |

 Is there a really need for echoing the operation, i.e. isn't the echoed
 ID sufficient? Because if you'd be able to drop this, request and
 response sizes would match up, avoiding wasted space on the
 shared page(s).
 In our implementation there is no matter if is echoed the operation or no.
 Frontend sends one request and waits a response. So I can simply remove
 the echoed operation. But in the future if this echoed the operation
 will be needed
 it can be simply restored.

Question here is whether stream_id really means what its name
says. If it does, you certainly need another field with a request ID,
which then also need echoing (and then the question is whether
stream_id need echoing too).

Jan


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


Re: [Xen-devel] [PATCH v3] sndif: add ABI for Para-virtual sound

2015-01-22 Thread Oleksandr Dmytryshyn
On Thu, Jan 22, 2015 at 1:08 PM, Jan Beulich jbeul...@suse.com wrote:
 On 22.01.15 at 11:52, oleksandr.dmytrys...@globallogic.com wrote:
 On Thu, Jan 22, 2015 at 12:24 PM, Jan Beulich jbeul...@suse.com wrote:
 On 22.01.15 at 10:56, oleksandr.dmytrys...@globallogic.com wrote:
 + * Responce for all requests exept the XENSND_GET_VOLUME:
 + * 01 2 3 4 5 6 7  octet
 + * +-+-+-+-+-+-+-+-+
 + * |   stream_id   |   operation   |

 Is there a really need for echoing the operation, i.e. isn't the echoed
 ID sufficient? Because if you'd be able to drop this, request and
 response sizes would match up, avoiding wasted space on the
 shared page(s).
 In our implementation there is no matter if is echoed the operation or no.
 Frontend sends one request and waits a response. So I can simply remove
 the echoed operation. But in the future if this echoed the operation
 will be needed
 it can be simply restored.

 Question here is whether stream_id really means what its name
 says. If it does, you certainly need another field with a request ID,
 which then also need echoing (and then the question is whether
 stream_id need echoing too).
Our implementation doesn't use a request ID, but this field
may be added to the protocol and in addition this request ID will be echoed.
And stream_id didn't need be echoed.
I'll think how to do it better in the next-patch-set.

 Jan


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


Re: [Xen-devel] [PATCH v3] sndif: add ABI for Para-virtual sound

2015-01-22 Thread Jan Beulich
 On 22.01.15 at 12:17, oleksandr.dmytrys...@globallogic.com wrote:
 On Thu, Jan 22, 2015 at 1:08 PM, Jan Beulich jbeul...@suse.com wrote:
 On 22.01.15 at 11:52, oleksandr.dmytrys...@globallogic.com wrote:
 On Thu, Jan 22, 2015 at 12:24 PM, Jan Beulich jbeul...@suse.com wrote:
 On 22.01.15 at 10:56, oleksandr.dmytrys...@globallogic.com wrote:
 + * Responce for all requests exept the XENSND_GET_VOLUME:
 + * 01 2 3 4 5 6 7  octet
 + * +-+-+-+-+-+-+-+-+
 + * |   stream_id   |   operation   |

 Is there a really need for echoing the operation, i.e. isn't the echoed
 ID sufficient? Because if you'd be able to drop this, request and
 response sizes would match up, avoiding wasted space on the
 shared page(s).
 In our implementation there is no matter if is echoed the operation or no.
 Frontend sends one request and waits a response. So I can simply remove
 the echoed operation. But in the future if this echoed the operation
 will be needed
 it can be simply restored.

 Question here is whether stream_id really means what its name
 says. If it does, you certainly need another field with a request ID,
 which then also need echoing (and then the question is whether
 stream_id need echoing too).
 Our implementation doesn't use a request ID, 

Please don't design the interface with just your implementation in mind.

Jan


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


[Xen-devel] [PATCH v3] sndif: add ABI for Para-virtual sound

2015-01-22 Thread Oleksandr Dmytryshyn
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.dmytrys...@globallogic.com
Signed-off-by: Iurii Konovalenko iurii.konovale...@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

 xen/include/public/io/sndif.h | 323 ++
 1 file changed, 323 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 000..8a78501
--- /dev/null
+++ b/xen/include/public/io/sndif.h
@@ -0,0 +1,323 @@
+/**
+ * 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__
+
+/*
+ * Feature and Parameter Negotiation
+ * =
+ * The two halves of a Para-virtual sound 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.
+ *
+ * XenStore nodes in sections marked PRIVATE are solely for use by the
+ * driver side whose XenBus tree contains them.
+ *
+ *
+ *Backend XenBus Nodes
+ *
+ *
+ *-- Backend Device Identification (PRIVATE) --
+ *
+ * stream_id
+ *  Values: uint32_t
+ *
+ *  Each virtualized stream has own id starting from 0.
+ *  Within each frontend these ids must be unique.
+ *  Each stream can be opened only for playback or capture
+ *  at the same time.
+ *
+ * channels_cnt
+ *  Values: uint32_t
+ *
+ *  The maximum amount of channels that can be supported by this stream.
+ *
+ *
+ *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.
+ */
+
+/*
+ * PCM FORMATS
+ *
+ * XENSND_PCM_FORMAT_format[_endian]
+ *
+ * format: S/U/FLOAT[bits] or name
+ * S - signed, U - unsigned, FLOAT - float
+ * bits - 8, 16, 24, 32 or absent
+ * name - MU_LAW, GSM, etc.
+ *
+ * endian: LE/BE, may be absent
+ * LE - Little endian, BE - Big endian
+ */
+#define XENSND_PCM_FORMAT_S80
+#define XENSND_PCM_FORMAT_U81
+#define XENSND_PCM_FORMAT_S16_LE2
+#define XENSND_PCM_FORMAT_S16_BE3
+#define XENSND_PCM_FORMAT_U16_LE4
+#define XENSND_PCM_FORMAT_U16_BE5
+#define XENSND_PCM_FORMAT_S24_LE6
+#define XENSND_PCM_FORMAT_S24_BE

Re: [Xen-devel] [PATCH v3] sndif: add ABI for Para-virtual sound

2015-01-22 Thread Oleksandr Dmytryshyn
On Thu, Jan 22, 2015 at 12:24 PM, Jan Beulich jbeul...@suse.com wrote:
 On 22.01.15 at 10:56, oleksandr.dmytrys...@globallogic.com wrote:
 +/*
 + * PCM FORMATS
 + *
 + * XENSND_PCM_FORMAT_format[_endian]
 + *
 + * format: S/U/FLOAT[bits] or name
 + * S - signed, U - unsigned, FLOAT - float
 + * bits - 8, 16, 24, 32 or absent
 + * name - MU_LAW, GSM, etc.
 + *
 + * endian: LE/BE, may be absent
 + * LE - Little endian, BE - Big endian
 + */
 +#define XENSND_PCM_FORMAT_S80
 +#define XENSND_PCM_FORMAT_U81
 +#define XENSND_PCM_FORMAT_S16_LE2
 +#define XENSND_PCM_FORMAT_S16_BE3
 +#define XENSND_PCM_FORMAT_U16_LE4
 +#define XENSND_PCM_FORMAT_U16_BE5
 +#define XENSND_PCM_FORMAT_S24_LE6
 +#define XENSND_PCM_FORMAT_S24_BE7
 +#define XENSND_PCM_FORMAT_U24_LE8
 +#define XENSND_PCM_FORMAT_U24_BE9
 +#define XENSND_PCM_FORMAT_S32_LE10
 +#define XENSND_PCM_FORMAT_S32_BE11
 +#define XENSND_PCM_FORMAT_U32_LE12
 +#define XENSND_PCM_FORMAT_U32_BE13
 +#define XENSND_PCM_FORMAT_FLOAT_LE  14 /* 4-byte float, IEEE-754 
 32-bit, */
 +#define XENSND_PCM_FORMAT_FLOAT_BE  15 /* range -1.0 to 1.0 
  */

 I think these should carry 32 in their names. I also wonder why
 it needs to be FLOAT instead of just F.
I'll rename those defines in the next patch-set.

 + * Request open - open an pcm stream for playback or capture:
 + * 01 2 3 4 5 6 7  octet
 + * +-+-+-+-+-+-+-+-+
 + * |   operation   |   stream_id   |
 + * +-+-+-+-+-+-+-+-+
 + * |  pcm_format   |  pcm_channes  |

 pcm_channel(s)?
I'll fix this in the next patch-set

 + * All responce packets have the same length (76 bytes)

 response
I'll fix this in the next patch-set

 + * Responce for all requests exept the XENSND_GET_VOLUME:
 + * 01 2 3 4 5 6 7  octet
 + * +-+-+-+-+-+-+-+-+
 + * |   stream_id   |   operation   |

 Is there a really need for echoing the operation, i.e. isn't the echoed
 ID sufficient? Because if you'd be able to drop this, request and
 response sizes would match up, avoiding wasted space on the
 shared page(s).
In our implementation there is no matter if is echoed the operation or no.
Frontend sends one request and waits a response. So I can simply remove
the echoed operation. But in the future if this echoed the operation
will be needed
it can be simply restored.


 + * +-+-+-+-+-+-+-+-+
 Jan


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