sndif: add ABI for para-virtual sound
PATCH v13 1/1 sndif: add ABI for para-virtual sound
From: Oleksandr Andrushchenko <[email protected]>
Hi, all!
Please find the next version of the ABI for the PV sound after addressing review comments.
Thank you,
Oleksandr Andrushchenko
Oleksandr Grytsov
Oleksandr Andrushchenko (1):
This is the ABI for the two halves of a para-virtualized sound
driver to communicate with each to other.
xen/include/public/io/sndif.h | 698 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 698 insertions(+) create mode 100644 xen/include/public/io/sndif.h
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xen.org/xen-devel
PATCH v13 1/1 This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
From: Oleksandr Andrushchenko <[email protected]>
---
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
Changes since v6:
* Reworked streams description in the Backend XenBus Nodes
Changes since v7:
* re-worked backend device parameters to be more generic and flexible
* extended frontend device parameters
* slightly updated state machine description added mute/unmute commands
* added constants for XenStore configuration strings (fields, PCM formats etc.)
* changed request/response structure size from 64 octets to 16
* introduced dynamic buffer allocation instead of
static XENSND_MAX_PAGES_PER_REQUEST
* re-worked open request to allow dynamic buffer allocation
* re-worked read/write/volume requests, so they don't pass grefs:
buffer from the open request is used for these operations to pass data
* specified type of the volume value to be a signed value in steps of 0.001 dBm, while 0 being 0dBm.
* added Linux include file with structure definitions
Changes since v8:
* changed frontend-id to frontend_id
* single sound card support, configured with bunch of devices/streams
* clarifucation made on sample rates and formats expressed as decimals w/o any particular ordering
* put description of migration/disconnection state
* replaced __attribute__((packed)) to __packed
* changed padding of ring structures to 64 to fit cache line
* removeed #ifdef __KERNEL
* explicitly stated which indices in XenStore configuration are contiguous
* added description to what frontend's defaults are
* made names of virtual card/devices optional
* removed PCM_FORMAT_SPECIAL
* changed volume units from dBm to dB
Changes since v9:
* removed sndif_linux.h
* moved all structures from sndif_linux.h to sndif.h
* structures padded where needed
* fixed Hz comment
Changes since v10:
* fixed tabs to 4 spaces to comply with Xen coding style
* added placeholders to empty structures (C89 concern)
* added missing header includes
Changes since v11:
* added XENSND_RSP_NOTSUPP error code
* changed gref[0] to gref[1] with comment
* modified comments on empty structures
* removed "__" from member names
* fixed indentation
* added padding in union xensnd_resp
* changed __XEN_PUBLIC_IO_XENSND_H__ to __XEN_PUBLIC_IO_SNDIF_H__
Changes since v12:
* changed indentation for defines
* missed ";" after gref[1]
* documentation changes
* changed req/resp structures
* changed xensnd_page_directory structure
* pass buffer size in open request
---
--- xen/include/public/io/sndif.h | 698 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 698 insertions(+) create mode 100644 xen/include/public/io/sndif.h
xen/include/public/io/sndif.h new file +698 -0
@@ -0,0 +1,698 @@ +/****************************************************************************** +* 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. +* Copyright (C) 2016 EPAM Systems Inc. +* +* Authors: Oleksandr Andrushchenko <[email protected]> +* Oleksandr Grytsov <[email protected]> +* Oleksandr Dmytryshyn <[email protected]> +* Iurii Konovalenko <[email protected]> +*/ + +#ifndef __XEN_PUBLIC_IO_SNDIF_H__ +#define __XEN_PUBLIC_IO_SNDIF_H__ + +#include "ring.h" +#include "../grant_table.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 +***************************************************************************** +* +*-------------------------------- Addressing --------------------------------- +* +* Indices used to address frontends, driver instances, cards, +* devices and streams. +* +* frontend_id +* Values: <uint> +* +* Domain ID of the sound frontend. +* +* drv_idx +* Values: <uint> +* +* Zero based contiguous index of the virtualized sound driver instance in +* this domain. Multiple PV drivers are allowed in the domain +* at the same time. +* +* dev_id +* Values: <uint> +* +* Unique device ID. +* Doesn't have to be zero based and/or to be contiguous. +* +* stream_idx +* Values: <uint> +* +* Zero based contiguous index of the stream of the device. +* +* Example for the frontend running in domain 5, instance of the driver +* in the front is 0 (single or first PV driver), device id 2, +* first stream (0): +* /local/domain/<frontend_id>/device/vsnd/<drv_idx>/ +* device/<dev_id>/stream/<stream_idx>/type = "p" +* /local/domain/5/device/vsnd/0/device/2/stream/0/type = "p" +* +*------------------------------- PCM settings -------------------------------- +* +* Every virtualized sound frontend has set of devices and streams, each +* is individually configured. Part of the PCM configuration can be defined at +* higher level and be fully or partially re-used by the underlying layers. +* These configuration values are: +* o number of channels (min/max) +* o supported sample rates +* o supported sample formats. +* E.g. one can define these values for the whole driver, device or stream. +* Every underlying layer in turn can re-define some or all of them to better +* fit its needs. For example, driver may define number of channels to be +* in [1; 8] range, and some particular stream may be limited to [1; 2] only. +* The rule is that the underlying layer must be a subset of the upper layer +* range. +* +* Note: if any of the values are not defined then PV driver should use +* its default values instead. +* +* channels-min +* Values: <uint> +* +* The minimum amount of channels that is supported. +* Must be at least 1. If not defined then use frontend's default. +* +* channels-max +* Values: <uint> +* +* The maximum amount of channels that is supported. +* Must be at least <channels-min>. If not defined then use frontend's +* default. +* +* sample-rates +* Values: <list of uints> +* +* List of supported sample rates separated by XENSND_LIST_SEPARATOR. +* If not defined then use frontend's default. Sample rates are expressed +* as a list of decimal values w/o any ordering requirement. +* +* sample-formats +* Values: <list of XENSND_PCM_FORMAT_XXX_STR> +* +* List of supported sample formats separated by XENSND_LIST_SEPARATOR. +* If not defined then use frontend's default. +* +* buffer-size +* Values: <uint> +* +* The maximum size in octets of the buffer to allocate per stream. +* +* Example configuration: +* +* Driver configuration used by all streams: +* /local/domain/5/device/vsnd/0/sample-formats = "s8;u8;s16_le;s16_be" +* Stream overrides sample rates supported: +* /local/domain/5/device/vsnd/0/device/2/stream/0/sample-rates = +* "8000;22050;44100;48000" +* +*----------------------- Virtual sound card settings -------------------------- +* short-name +* Values: <char[32]> +* +* Short name of the virtual sound card. Optional. +* +* long-name +* Values: <char[80]> +* +* Long name of the virtual sound card. Optional. +* +* For example, +* /local/domain/5/device/vsnd/0/short-name = "Virtual audio" +* /local/domain/5/device/vsnd/0/long-name = +* "Virtual audio at center stack" +* +*----------------------------- Device settings -------------------------------- +* name +* Values: <char[80]> +* +* Name of the sound device within the virtual sound card. Optional. +* +* For example, +* /local/domain/5/device/vsnd/0/device/0/name = "General analog" +* +*----------------------------- Stream settings ------------------------------- +* +* type +* Values: "p", "c" +* +* Stream type: "p" - playback stream, "c" - capture stream +* +* If both capture and playback are needed then two streams need to be +* defined under the same device. For example, +* /local/domain/5/device/vsnd/0/device/0/stream/0/type = "p" +* /local/domain/5/device/vsnd/0/device/0/stream/1/type = "c" +* +***************************************************************************** +* Frontend XenBus Nodes +***************************************************************************** +* +*----------------------- Request Transport Parameters ----------------------- +* +* These are per stream. +* +* event-channel +* Values: <uint> +* +* The identifier of the Xen event channel used to signal activity +* in the ring buffer. +* +* ring-ref +* Values: <uint> +* +* The Xen grant reference granting permission for the backend to map +* a sole page in a single page sized ring buffer. +* +* index +* Values: <uint> +* +* After stream initialization it is assigned a unique ID (within the front +* driver), so every stream of the frontend can be identified by the +* backend by this ID. This is not equal to stream_idx as the later is +* zero based within a device, but this index is contiguous within the +* driver. +*/ + +/* +* STATE DIAGRAMS +* +***************************************************************************** +* Startup * +***************************************************************************** +* +* Tool stack creates front and back state nodes with initial state +* XenbusStateInitialising. +* Tool stack creates and sets up frontend sound configuration nodes per domain. +* +* Front Back +* ================================= ===================================== +* XenbusStateInitialising XenbusStateInitialising +* o Query backend device identification +* data. +* o Open and validate backend device. +* | +* | +* V +* XenbusStateInitWait +* +* o Query frontend configuration +* o Allocate and initialize +* event channels per configured +* playback/capture stream. +* o Publish transport parameters +* that will be in effect during +* this connection. +* | +* | +* V +* XenbusStateInitialised +* +* o Query frontend transport parameters. +* o Connect to the event channels. +* | +* | +* V +* XenbusStateConnected +* +* o Create and initialize OS +* virtual sound device instances +* as per configuration. +* | +* | +* V +* XenbusStateConnected +* +* XenbusStateUnknown +* XenbusStateClosed +* XenbusStateClosing +* o Remove virtual sound device +* o Remove event channels +* | +* | +* V +* XenbusStateClosed +* +*/ + +/* +* 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 + +/* +* 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 +#define XENSND_OP_MUTE 6 +#define XENSND_OP_UNMUTE 7 + +/* +* XENSTORE FIELD AND PATH NAME STRINGS, HELPERS. +*/ +#define XENSND_DRIVER_NAME "vsnd" + +#define XENSND_LIST_SEPARATOR ";" +/* Path entries */ +#define XENSND_PATH_DEVICE "device" +#define XENSND_PATH_STREAM "stream" +/* Field names */ +#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_DEVICE_NAME "name" +#define XENSND_FIELD_TYPE "type" +#define XENSND_FIELD_STREAM_INDEX "index" +#define XENSND_FIELD_CHANNELS_MIN "channels-min" +#define XENSND_FIELD_CHANNELS_MAX "channels-max" +#define XENSND_FIELD_SAMPLE_RATES "sample-rates" +#define XENSND_FIELD_SAMPLE_FORMATS "sample-formats" +#define XENSND_FIELD_BUFFER_SIZE "buffer-size" + +/* Stream type field values. */ +#define XENSND_STREAM_TYPE_PLAYBACK "p" +#define XENSND_STREAM_TYPE_CAPTURE "c" +/* Sample rate max string length */ +#define XENSND_SAMPLE_RATE_MAX_LEN 6 +/* Sample format field values */ +#define XENSND_SAMPLE_FORMAT_MAX_LEN 24 + +#define XENSND_PCM_FORMAT_S8_STR "s8" +#define XENSND_PCM_FORMAT_U8_STR "u8" +#define XENSND_PCM_FORMAT_S16_LE_STR "s16_le" +#define XENSND_PCM_FORMAT_S16_BE_STR "s16_be" +#define XENSND_PCM_FORMAT_U16_LE_STR "u16_le" +#define XENSND_PCM_FORMAT_U16_BE_STR "u16_be" +#define XENSND_PCM_FORMAT_S24_LE_STR "s24_le" +#define XENSND_PCM_FORMAT_S24_BE_STR "s24_be" +#define XENSND_PCM_FORMAT_U24_LE_STR "u24_le" +#define XENSND_PCM_FORMAT_U24_BE_STR "u24_be" +#define XENSND_PCM_FORMAT_S32_LE_STR "s32_le" +#define XENSND_PCM_FORMAT_S32_BE_STR "s32_be" +#define XENSND_PCM_FORMAT_U32_LE_STR "u32_le" +#define XENSND_PCM_FORMAT_U32_BE_STR "u32_be" +#define XENSND_PCM_FORMAT_F32_LE_STR "float_le" +#define XENSND_PCM_FORMAT_F32_BE_STR "float_be" +#define XENSND_PCM_FORMAT_F64_LE_STR "float64_le" +#define XENSND_PCM_FORMAT_F64_BE_STR "float64_be" +#define XENSND_PCM_FORMAT_IEC958_SUBFRAME_LE_STR "iec958_subframe_le" +#define XENSND_PCM_FORMAT_IEC958_SUBFRAME_BE_STR "iec958_subframe_be" +#define XENSND_PCM_FORMAT_MU_LAW_STR "mu_law" +#define XENSND_PCM_FORMAT_A_LAW_STR "a_law" +#define XENSND_PCM_FORMAT_IMA_ADPCM_STR "ima_adpcm" +#define XENSND_PCM_FORMAT_MPEG_STR "mpeg" +#define XENSND_PCM_FORMAT_GSM_STR "gsm" + +/* +* STATUS RETURN CODES. +*/ +/* Operation not supported. */ +#define XENSND_RSP_NOTSUPP (-2) +/* 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 other using a shared page and an event channel. +* Shared page contains a ring with request/response packets. +* +* All reserved and padding fields in the structures below must be 0. +* +* All request packets have the same length (32 octets) +* All request packets have common header: +* 0 1 2 3 octet +* +-----------------+-----------------+-----------------+-----------------+ +* | id | operation | stream_idx | +* +-----------------+-----------------+-----------------+-----------------+ +* id - uint16_t, private guest value, echoed in response +* operation - uint8_t, operation code +* stream_idx - uint8_t, index of the stream ("streams_idx" XenStore entry +* of the stream) +* +* +* Request open - open a PCM stream for playback or capture: +* 0 1 2 3 octet +* +-----------------+-----------------+-----------------+-----------------+ +* | id | XENSND_OP_OPEN | stream_idx | +* +-----------------+-----------------+-----------------+-----------------+ +* | padding | +* +-----------------+-----------------+-----------------+-----------------+ +* | pcm_rate | +* +-----------------+-----------------+-----------------+-----------------+ +* | pcm_format | pcm_channels | reserved | +* +-----------------+-----------------+-----------------+-----------------+ +* | buffer_sz | +* +-----------------+-----------------+-----------------+-----------------+ +* | gref_directory_start | +* +-----------------+-----------------+-----------------+-----------------+ +* |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| +* +-----------------+-----------------+-----------------+-----------------+ +* | reserved | +* +-----------------+-----------------+-----------------+-----------------+ +* +* pcm_rate - uint32_t, stream data rate, Hz +* pcm_format - uint8_t, XENSND_PCM_FORMAT_XXX value +* pcm_channels - uint8_t, number of channels of this stream +* buffer_sz - uint32_t, buffer size to be allocated in octets +* gref_directory_start - grant_ref_t, a reference to the first shared page +* describing shared buffer references. At least one page exists. If shared +* buffer size exceeds what can be addressed by this single page, then +* reference to the next page must be supplied (see gref_dir_next_page below) +*/ + +struct xensnd_open_req { +uint32_t pcm_rate;/* in Hz */ +uint8_t pcm_format; +uint8_t pcm_channels; +uint16_t reserved; +uint32_t buffer_sz; +grant_ref_t gref_directory_start; +}; + +/* +* Shared page for XENSND_OP_OPEN buffer descriptor (gref_directory in the +* request) employs a list of pages, describing all pages of the shared data +* buffer: +* 0 1 2 3 octet +* +-----------------+-----------------+-----------------+-----------------+ +* | gref_dir_next_page | +* +-----------------+-----------------+-----------------+-----------------+ +* | gref[0] | +* +-----------------+-----------------+-----------------+-----------------+ +* |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| +* +-----------------+-----------------+-----------------+-----------------+ +* | gref[i] | +* +-----------------+-----------------+-----------------+-----------------+ +* |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| +* +-----------------+-----------------+-----------------+-----------------+ +* | gref[N -1] | +* +-----------------+-----------------+-----------------+-----------------+ +* +* gref_dir_next_page - grant_ref_t, reference to the next page describing +* page directory. Must be 0 if no more pages in the list. +* gref[i] - grant_ref_t, reference to a shared page of the buffer +* allocated at XENSND_OP_OPEN +* +* Number of grant_ref_t entries in the whole page directory is not +* passed, but instead can be calculated as: +* num_grefs_total = DIV_ROUND_UP(XENSND_OP_OPEN.buffer_sz, PAGE_SIZE); +*/ + +struct xensnd_page_directory { +grant_ref_t gref_dir_next_page; +grant_ref_t gref[1 ];/* Variable length */ +}; + +/* +* Request close - close an opened pcm stream: +* 0 1 2 3 octet +* +-----------------+-----------------+-----------------+-----------------+ +* | id | XENSND_OP_CLOSE | stream_idx | +* +-----------------+-----------------+-----------------+-----------------+ +* | padding | +* +-----------------+-----------------+-----------------+-----------------+ +* | reserved | +* +-----------------+-----------------+-----------------+-----------------+ +* |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| +* +-----------------+-----------------+-----------------+-----------------+ +* | reserved | +* +-----------------+-----------------+-----------------+-----------------+ +*/ + +struct xensnd_close_req { +/* place holder, remove if changing the structure */ +uint8_t placeholder; +}; + +/* +* Request read/write - used for read (for capture) or write (for playback): +* 0 1 2 3 octet +* +-----------------+-----------------+-----------------+-----------------+ +* | id | operation | stream_idx | +* +-----------------+-----------------+-----------------+-----------------+ +* | padding | +* +-----------------+-----------------+-----------------+-----------------+ +* | offset | +* +-----------------+-----------------+-----------------+-----------------+ +* | length | +* +-----------------+-----------------+-----------------+-----------------+ +* | reserved | +* +-----------------+-----------------+-----------------+-----------------+ +* |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| +* +-----------------+-----------------+-----------------+-----------------+ +* | reserved | +* +-----------------+-----------------+-----------------+-----------------+ +* +* operation - XENSND_OP_READ for read or XENSND_OP_WRITE for write +* offset - uint32_t, read or write data offset within the shared buffer +* passed with XENSND_OP_OPEN request +* length - uint32_t, read or write data length +*/ + +struct xensnd_rw_req { +uint32_t offset; +uint32_t len; +}; + +/* +* Request set/get volume - set/get channels' volume of the stream given: +* 0 1 2 3 octet +* +-----------------+-----------------+-----------------+-----------------+ +* | id | operation | stream_idx | +* +-----------------+-----------------+-----------------+-----------------+ +* | padding | +* +-----------------+-----------------+-----------------+-----------------+ +* | reserved | +* +-----------------+-----------------+-----------------+-----------------+ +* |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| +* +-----------------+-----------------+-----------------+-----------------+ +* | reserved | +* +-----------------+-----------------+-----------------+-----------------+ +* +* operation - XENSND_OP_SET_VOLUME for volume set +* or XENSND_OP_GET_VOLUME for volume get +* Buffer passed with XENSND_OP_OPEN is used to exchange volume +* values: +* +* 0 1 2 3 octet +* +-----------------+-----------------+-----------------+-----------------+ +* | channel[0] | +* +-----------------+-----------------+-----------------+-----------------+ +* +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| +* +-----------------+-----------------+-----------------+-----------------+ +* | channel[i] | +* +-----------------+-----------------+-----------------+-----------------+ +* +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| +* +-----------------+-----------------+-----------------+-----------------+ +* channel[XENSND_OP_OPEN.pcm_channels - 1] | +* +-----------------+-----------------+-----------------+-----------------+ +* +* channel[i] - sint32_t, volume of i-th channel +* Volume is expressed as a signed value in steps of 0.001 dB, +* while 0 being 0 dB. +*/ + +struct xensnd_get_vol_req { +/* place holder, remove if changing the structure */ +uint8_t placeholder; +}; + +struct xensnd_set_vol_req { +/* place holder, remove if changing the structure */ +uint8_t placeholder; +}; + +/* +* Request mute/unmute - mute/unmute stream: +* 0 1 2 3 octet +* +-----------------+-----------------+-----------------+-----------------+ +* | id | operation | stream_idx | +* +-----------------+-----------------+-----------------+-----------------+ +* | padding | +* +-----------------+-----------------+-----------------+-----------------+ +* | reserved | +* +-----------------+-----------------+-----------------+-----------------+ +* |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| +* +-----------------+-----------------+-----------------+-----------------+ +* | reserved | +* +-----------------+-----------------+-----------------+-----------------+ +* +* operation - XENSND_OP_MUTE for mute or XENSND_OP_UNMUTE for unmute +* Buffer passed with XENSND_OP_OPEN is used to exchange mute/unmute +* values: +* +* 0 1 2 3 octet +* +-----------------+-----------------+-----------------+-----------------+ +* | channel[0] | channel[1] | channel[2] | channel[3] | +* +-----------------+-----------------+-----------------+-----------------+ +* +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| +* +-----------------+-----------------+-----------------+-----------------+ +* | channel[i] | channel[i+1] | channel[i+2] | channel[i+3] | +* +-----------------+-----------------+-----------------+-----------------+ +* +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| +* +-----------------+-----------------+-----------------+-----------------+ +* +* channel[i] - uint8_t, non-zero if i-th channel needs to be muted/unmuted +* Number of channels passed is equal to XENSND_OP_OPEN request pcm_channels +* field +*/ + +struct xensnd_mute_req { +/* place holder, remove if changing the structure */ +uint8_t placeholder; +}; + +struct xensnd_unmute_req { +/* place holder, remove if changing the structure */ +uint8_t placeholder; +}; + +/* +* All response packets have the same length (32 octets) +* +* Response for all requests: +* 0 1 2 3 octet +* +-----------------+-----------------+-----------------+-----------------+ +* | id | operation | stream_idx | +* +-----------------+-----------------+-----------------+-----------------+ +* | padding | +* +-----------------+-----------------+-----------------+-----------------+ +* | status | reserved | +* +-----------------+-----------------+-----------------+-----------------+ +* | reserved | +* +-----------------+-----------------+-----------------+-----------------+ +* |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| +* +-----------------+-----------------+-----------------+-----------------+ +* | reserved | +* +-----------------+-----------------+-----------------+-----------------+ +* +* id - uint16_t, copied from the request +* stream_idx - uint8_t, copied from request +* operation - uint8_t, XENSND_OP_XXX - copied from request +* status - int8_t, response status (XENSND_RSP_???) +*/ + +struct xensnd_req { +uint16_t id; +uint8_t operation; +uint8_t stream_idx; +uint32_t reserved; +union { +struct xensnd_open_req open; +struct xensnd_close_req close; +struct xensnd_rw_req write; +struct xensnd_rw_req read; +struct xensnd_get_vol_req get_vol; +struct xensnd_set_vol_req set_vol; +struct xensnd_mute_req mute; +struct xensnd_unmute_req unmute; +uint8_t padding[24 ]; + } op; +}; + +struct xensnd_resp { +uint16_t id; +uint8_t operation; +uint8_t stream_idx; +int8_t status; +uint8_t padding[26 ]; +}; + +DEFINE_RING_TYPES(xen_sndif, struct xensnd_req, struct xensnd_resp); + +#endif /* __XEN_PUBLIC_IO_SNDIF_H__ */
Signature · 7 lines
2.7.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xen.org/xen-devel
Re: [PATCH v13] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
Quoted 7 lines
Quoted 1 line
Quoted 1 line · On 28.11.16 at 13:30, <[email protected]> wrote:
+ * Request open - open a PCM stream for playback or capture:
+ * 0 1 2 3 octet
+ * +-----------------+-----------------+-----------------+-----------------+
+ * | id |XENSND_OP_OPEN|stream_idx|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * | padding |
This still say "padding" instead of "reserved". Also isn't this still part of the common header?
Quoted 10 lines
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |pcm_rate|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |pcm_format|pcm_channels| reserved |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |buffer_sz|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |gref_directory_start|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
Don't you elsewhere use this as an "and so one" marker? That's not what you want here afaict, because of ...
Quoted 3 lines
+ * +-----------------+-----------------+-----------------+-----------------+
+ * | reserved |
+ * +-----------------+-----------------+-----------------+-----------------+
... this.
Quoted 19 lines
+/*
+ * Shared page forXENSND_OP_OPENbuffer descriptor (gref_directoryin the
+ * request) employs a list of pages, describing all pages of the shared data
+ * buffer:
+ * 0 1 2 3 octet
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |gref_dir_next_page|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * | gref[0] |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * | gref[i] |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * | gref[N -1] |
+ * +-----------------+-----------------+-----------------+-----------------+
+ *
Quoted 8 lines
+ *
gref_dir_next_page-grant_ref_t, reference to the next page describing
+ * page directory. Must be 0 if no more pages in the list.
+ * gref[i] -grant_ref_t, reference to a shared page of the buffer
+ * allocated atXENSND_OP_OPEN
+ *
+ * Number ofgrant_ref_tentries in the whole page directory is not
+ * passed, but instead can be calculated as:
+ *num_grefs_total=DIV_ROUND_UP(XENSND_OP_OPEN.buffer_sz, PAGE_SIZE);
The header should be self contained, and there's no DIV_ROUND_UP() anywhere under public/io/ for a reader to refer to. Please express this using mathematical terms plus, if needed, standard C library ones.
Quoted 6 lines
+ * operation -
XENSND_OP_MUTEfor mute orXENSND_OP_UNMUTEfor unmute
+ * Buffer passed withXENSND_OP_OPENis used to exchange mute/unmute
+ * values:
+ *
+ * 0 1 2 3 octet
+ * +-----------------+-----------------+-----------------+-----------------+
Quoted 8 lines
+ * | channel[0] | channel[1] | channel[2] | channel[3] |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * | channel[i] | channel[i+1] | channel[i+2] | channel[i+3] |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
Iirc you had confirmed to change this, but you didn't: I can only repeat that other than e.g. for the volume operations, it is unclear what the upper bound here is, as you don't use N.
xen/include/public/io/sndif.h line 679
Quoted 10 lines
xen/include/public/io/sndif.h new file +10
-0+struct xensnd_req { +uint16_t id; +uint8_t operation; +uint8_t stream_idx; +uint32_t reserved; +union { +struct xensnd_open_req open; +struct xensnd_close_req close; +struct xensnd_rw_req write; +struct xensnd_rw_req read;
I guess this has been this way before, but - why two of them? Just like thy type can be shared, the field needs to be here just once (perhaps named "rw").
xen/include/public/io/sndif.h line 681
Quoted 2 lines
xen/include/public/io/sndif.h new file +2
-0+struct xensnd_get_vol_req get_vol; +struct xensnd_set_vol_req set_vol;
Same here - if these dummy structures really need to survive, then I don't think you need multiple for the volume operations or ...
xen/include/public/io/sndif.h line 683
Quoted 2 lines
xen/include/public/io/sndif.h new file +2
-0+struct xensnd_mute_req mute; +struct xensnd_unmute_req unmute;
... or the muting ones. In fact, if these dummy structures are needed in the first place, perhaps just one total would suffice?
xen/include/public/io/sndif.h line 694
Quoted 7 lines
xen/include/public/io/sndif.h new file +7
-0+struct xensnd_resp { +uint16_t id; +uint8_t operation; +uint8_t stream_idx; +int8_t status; +uint8_t padding[26 ]; +};
2 + 1 + 1 + 1 + 26 = 31, which I don't think is what you want.
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Re: [PATCH v13] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
Quoted 9 lines · Jan Beulich
Quoted 7 lines
Quoted 1 line
Quoted 1 line · On 28.11.16 at 13:30, <[email protected]> wrote:
+ * Request open - open a PCM stream for playback or capture:
+ * 0 1 2 3 octet
+ * +-----------------+-----------------+-----------------+-----------------+
+ * | id |XENSND_OP_OPEN|stream_idx|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * | padding |This still say "padding" instead of "reserved". Also isn't this still part of the common header?
done
Quoted 10 lines
Quoted 10 lines
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |pcm_rate|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |pcm_format|pcm_channels| reserved |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |buffer_sz|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |gref_directory_start|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
Quoted 7 lines
Don't you elsewhere use this as an "and so one" marker? That's not what you want here afaict, because of ...
Quoted 3 lines
+ * +-----------------+-----------------+-----------------+-----------------+
+ * | reserved |
+ * +-----------------+-----------------+-----------------+-----------------+... this.
done
Quoted 27 lines
Quoted 19 lines
+/*
+ * Shared page forXENSND_OP_OPENbuffer descriptor (gref_directoryin the
+ * request) employs a list of pages, describing all pages of the shared data
+ * buffer:
+ * 0 1 2 3 octet
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |gref_dir_next_page|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * | gref[0] |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * | gref[i] |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * | gref[N -1] |
+ * +-----------------+-----------------+-----------------+-----------------+
+ *Quoted 8 lines
+ *
gref_dir_next_page-grant_ref_t, reference to the next page describing
+ * page directory. Must be 0 if no more pages in the list.
+ * gref[i] -grant_ref_t, reference to a shared page of the buffer
+ * allocated atXENSND_OP_OPEN
+ *
+ * Number ofgrant_ref_tentries in the whole page directory is not
+ * passed, but instead can be calculated as:
+ *num_grefs_total=DIV_ROUND_UP(XENSND_OP_OPEN.buffer_sz, PAGE_SIZE);
Quoted 3 lines
The header should be self contained, and there's no
DIV_ROUND_UP()anywhere under public/io/ for a reader to refer to. Please express this using mathematical terms plus, if needed, standard C library ones.
done, will put:num_grefs_total = (XENSND_OP_OPEN.buffer_sz + PAGE_SIZE - 1) / PAGE_SIZE
Quoted 14 lines
Quoted 6 lines
+ * operation -
XENSND_OP_MUTEfor mute orXENSND_OP_UNMUTEfor unmute
+ * Buffer passed withXENSND_OP_OPENis used to exchange mute/unmute
+ * values:
+ *
+ * 0 1 2 3 octet
+ * +-----------------+-----------------+-----------------+-----------------+Quoted 8 lines
+ * | channel[0] | channel[1] | channel[2] | channel[3] |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
+ * | channel[i] | channel[i+1] | channel[i+2] | channel[i+3] |
+ * +-----------------+-----------------+-----------------+-----------------+
+ * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +-----------------+-----------------+-----------------+-----------------+
Quoted 3 lines
Iirc you had confirmed to change this, but you didn't: I can only repeat that other than e.g. for the volume operations, it is unclear what the upper bound here is, as you don't use N.
done
Quoted 10 lines
xen/include/public/io/sndif.h line 679
Quoted 10 lines
xen/include/public/io/sndif.h new file +10
-0+struct xensnd_req { +uint16_t id; +uint8_t operation; +uint8_t stream_idx; +uint32_t reserved; +union { +struct xensnd_open_req open; +struct xensnd_close_req close; +struct xensnd_rw_req write; +struct xensnd_rw_req read;
Quoted 3 lines
I guess this has been this way before, but - why two of them? Just like thy type can be shared, the field needs to be here just once (perhaps named "rw").
done
Quoted 9 lines
xen/include/public/io/sndif.h line 681
Quoted 2 lines
xen/include/public/io/sndif.h new file +2
-0+struct xensnd_get_vol_req get_vol; +struct xensnd_set_vol_req set_vol;Same here - if these dummy structures really need to survive, then I don't think you need multiple for the volume operations or ...
xen/include/public/io/sndif.h line 683
Quoted 2 lines
xen/include/public/io/sndif.h new file +2
-0+struct xensnd_mute_req mute; +struct xensnd_unmute_req unmute;... or the muting ones. In fact, if these dummy structures are needed in the first place, perhaps just one total would suffice?
I will remove all empty structures
Quoted 8 lines
xen/include/public/io/sndif.h line 694
Quoted 7 lines
xen/include/public/io/sndif.h new file +7
-0+struct xensnd_resp { +uint16_t id; +uint8_t operation; +uint8_t stream_idx; +int8_t status; +uint8_t padding[26 ]; +};2 + 1 + 1 + 1 + 26 = 31, which I don't think is what you want.
changed 26 to 27
Quoted 2 lines
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Re: [PATCH v13] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
Hi Oleksandr,
Quoted 6 lines · Oleksandr Andrushchenko
Quoted 4 lines · Jan Beulich
Quoted 4 lines
+ *
+ *gref_dir_next_page-grant_ref_t, reference to the next page describing
+ * page directory. Must be 0 if no more pages in the list.
If I am not mistaken 0 is a valid grant.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
+ * gref[i] -
grant_ref_t, reference to a shared page of the buffer
+ * allocated atXENSND_OP_OPEN
+ *
+ * Number ofgrant_ref_tentries in the whole page directory is not
+ * passed, but instead can be calculated as:
+ *num_grefs_total= DIV_ROUND_UP(XENSND_OP_OPEN.buffer_sz,PAGE_SIZE);
Quoted 5 lines
Quoted 3 lines
The header should be self contained, and there's no
DIV_ROUND_UP()anywhere under public/io/ for a reader to refer to. Please express this using mathematical terms plus, if needed, standard C library ones.done, will put:
num_grefs_total= (XENSND_OP_OPEN.buffer_sz +PAGE_SIZE- 1) /PAGE_SIZE
Can we avoid to use PAGE_SIZE in the header? Xen, the front-end and the back-end may have different page size.
Regards,
Signature · 6 lines
Julien Grall _______________________________________________ Xen-devel mailing list [email protected] https://lists.xen.org/xen-devel
Re: [PATCH v13] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
Quoted 4 lines · Julien Grall
Hi Oleksandr,
Quoted 1 line · Oleksandr Andrushchenko
Quoted 8 lines
Quoted 5 lines · Jan Beulich
Quoted 4 lines
Quoted 4 lines
+ *
+ *gref_dir_next_page-grant_ref_t, reference to the next page describing
+ * page directory. Must be 0 if no more pages in the list.If I am not mistaken 0 is a valid grant.
Then I will remove this sentence, anyways BE knows how many grefs there are for the buffer size given
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
+ * gref[i] -
grant_ref_t, reference to a shared page of the buffer
+ * allocated atXENSND_OP_OPEN
+ *
+ * Number ofgrant_ref_tentries in the whole page directory is not
+ * passed, but instead can be calculated as:
+ *num_grefs_total= DIV_ROUND_UP(XENSND_OP_OPEN.buffer_sz,PAGE_SIZE);
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
The header should be self contained, and there's no
DIV_ROUND_UP()anywhere under public/io/ for a reader to refer to. Please express this using mathematical terms plus, if needed, standard C library ones.done, will put:
num_grefs_total= (XENSND_OP_OPEN.buffer_sz +PAGE_SIZE- 1) /PAGE_SIZECan we avoid to use
PAGE_SIZEin the header? Xen, the front-end and the back-end may have different page size.
then, I believe, the protocol should implement something like blkif does? Multi-page buffer which depends on front and back page sizes? (blkif: max-ring-page-order/ring-ref%u/ring-page-order)
Is this what you mean?
Quoted 2 lines
Regards,
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Re: [PATCH v13] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
Hi Oleksandr,
Quoted 28 lines · Oleksandr Andrushchenko
Quoted 4 lines · Julien Grall
Hi Oleksandr,
Quoted 1 line · Oleksandr Andrushchenko
Quoted 8 lines
Quoted 5 lines · Jan Beulich
Quoted 4 lines
Quoted 4 lines
+ *
+ *gref_dir_next_page-grant_ref_t, reference to the next page describing
+ * page directory. Must be 0 if no more pages in the list.If I am not mistaken 0 is a valid grant.
Then I will remove this sentence, anyways BE knows how many grefs there are for the buffer size given
Quoted 13 lines
Quoted 12 lines
Quoted 10 lines
Quoted 7 lines
+ * gref[i] -
grant_ref_t, reference to a shared page of the buffer
+ * allocated atXENSND_OP_OPEN
+ *
+ * Number ofgrant_ref_tentries in the whole page directory is not
+ * passed, but instead can be calculated as:
+ *num_grefs_total= DIV_ROUND_UP(XENSND_OP_OPEN.buffer_sz,PAGE_SIZE);The header should be self contained, and there's no
DIV_ROUND_UP()anywhere under public/io/ for a reader to refer to. Please express this using mathematical terms plus, if needed, standard C library ones.done, will put:
num_grefs_total= (XENSND_OP_OPEN.buffer_sz +PAGE_SIZE- 1) /PAGE_SIZE
Quoted 7 lines
Quoted 3 lines
Can we avoid to use
PAGE_SIZEin the header? Xen, the front-end and the back-end may have different page size.then, I believe, the protocol should implement something like blkif does? Multi-page buffer which depends on front and back page sizes? (blkif: max-ring-page-order/ring-ref%u/ring-page-order)
Is this what you mean?
It is not what I meant. I asked to define PAGE_SIZE. Is it the the PAGE_SIZE of Xen? PAGE_SIZE of the backend? PAGE_SIZE of the frontend?
Currently PV driver PAGE_SIZE is based on the size of a grant page.
Regards,
Signature · 6 lines
Julien Grall _______________________________________________ Xen-devel mailing list [email protected] https://lists.xen.org/xen-devel
Re: [PATCH v13] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
Quoted 18 lines · Julien Grall
Hi Oleksandr,
Quoted 15 lines · Oleksandr Andrushchenko
Quoted 4 lines · Julien Grall
Hi Oleksandr,
Quoted 1 line · Oleksandr Andrushchenko
Quoted 8 lines
Quoted 5 lines · Jan Beulich
Quoted 4 lines
Quoted 4 lines
+ *
+ *gref_dir_next_page-grant_ref_t, reference to the next page describing
+ * page directory. Must be 0 if no more pages in the list.If I am not mistaken 0 is a valid grant.
Then I will remove this sentence, anyways BE knows how many grefs there are for the buffer size given
BTW, xen-blkfrint.c:
#define GRANT_INVALID_REF 0
this is from where I got "Must be 0 if no more pages in the list."
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
+ * gref[i] -
grant_ref_t, reference to a shared page of the buffer
+ * allocated atXENSND_OP_OPEN
+ *
+ * Number ofgrant_ref_tentries in the whole page directory is not
+ * passed, but instead can be calculated as:
+ *num_grefs_total= DIV_ROUND_UP(XENSND_OP_OPEN.buffer_sz,PAGE_SIZE);Quoted 5 lines
Quoted 3 lines
The header should be self contained, and there's no
DIV_ROUND_UP()anywhere under public/io/ for a reader to refer to. Please express this using mathematical terms plus, if needed, standard C library ones.done, will put:
num_grefs_total= (XENSND_OP_OPEN.buffer_sz +PAGE_SIZE- 1) /PAGE_SIZECan we avoid to use
PAGE_SIZEin the header? Xen, the front-end and the back-end may have different page size.
Quoted 8 lines
Quoted 4 lines
then, I believe, the protocol should implement something like blkif does? Multi-page buffer which depends on front and back page sizes? (blkif: max-ring-page-order/ring-ref%u/ring-page-order)
Is this what you mean?It is not what I meant. I asked to define
PAGE_SIZE. Is it the thePAGE_SIZEof Xen?PAGE_SIZEof the backend?PAGE_SIZEof the frontend?Currently PV driver
PAGE_SIZEis based on the size of a grant page.
As per my understanding, I can put XEN_PAGE_SIZE here, because xen/page.h says:
" * We assume that PAGE_SIZE is a multiple of XEN_PAGE_SIZE
* XXX: Add a BUILD_BUG_ON?
"
meaning that PAGE_SIZE on either side cannot be less than XEN_PAGE_SIZE Will XEN_PAGE_SIZE fit then?
Quoted 2 lines
Regards,
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Re: [PATCH v13] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
Hi Oleksandr,
Quoted 49 lines · Oleksandr Andrushchenko
Quoted 18 lines · Julien Grall
Hi Oleksandr,
Quoted 15 lines · Oleksandr Andrushchenko
Quoted 4 lines · Julien Grall
Hi Oleksandr,
Quoted 1 line · Oleksandr Andrushchenko
Quoted 8 lines
Quoted 5 lines · Jan Beulich
Quoted 4 lines
Quoted 4 lines
+ *
+ *gref_dir_next_page-grant_ref_t, reference to the next page describing
+ * page directory. Must be 0 if no more pages in the list.If I am not mistaken 0 is a valid grant.
Then I will remove this sentence, anyways BE knows how many grefs there are for the buffer size given
BTW,
xen-blkfrint.c:
#defineGRANT_INVALID_REF0
this is from where I got "Must be 0 if no more pages in the list."Quoted 27 lines
Quoted 15 lines
Quoted 15 lines
Quoted 14 lines
Quoted 11 lines
Quoted 7 lines
+ * gref[i] -
grant_ref_t, reference to a shared page of the buffer
+ * allocated atXENSND_OP_OPEN
+ *
+ * Number ofgrant_ref_tentries in the whole page directory is not
+ * passed, but instead can be calculated as:
+ *num_grefs_total= DIV_ROUND_UP(XENSND_OP_OPEN.buffer_sz,PAGE_SIZE);The header should be self contained, and there's no
DIV_ROUND_UP()anywhere under public/io/ for a reader to refer to. Please express this
using mathematical terms plus, if needed, standard C library ones.done, will put:
num_grefs_total= (XENSND_OP_OPEN.buffer_sz +PAGE_SIZE- 1) /PAGE_SIZEQuoted 8 lines
Quoted 3 lines
Can we avoid to use
PAGE_SIZEin the header? Xen, the front-end and the back-end may have different page size.then, I believe, the protocol should implement something like blkif does?
Multi-page buffer which depends on front and back page sizes? (blkif: max-ring-page-order/ring-ref%u/ring-page-order) Is this what you mean?It is not what I meant. I asked to define
PAGE_SIZE. Is it the thePAGE_SIZEof Xen?PAGE_SIZEof the backend?PAGE_SIZEof the frontend?
Quoted 8 lines
Quoted 1 line
Currently PV driver
PAGE_SIZEis based on the size of a grant page.As per my understanding, I can put
XEN_PAGE_SIZEhere, becausexen/page.hsays:
" * We assume thatPAGE_SIZEis a multiple ofXEN_PAGE_SIZE
* XXX: Add aBUILD_BUG_ON?
"
meaning thatPAGE_SIZEon either side cannot be less thanXEN_PAGE_SIZEWillXEN_PAGE_SIZEfit then?
Yes. FWIW, this is the page granularity used by every PV drivers.
Regards,
Signature · 6 lines
Julien Grall _______________________________________________ Xen-devel mailing list [email protected] https://lists.xen.org/xen-devel
Re: [PATCH v13] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
In reply to Oleksandr AndrushchenkoHi,
Quoted 15 lines · Oleksandr Andrushchenko
Quoted 14 lines · Julien Grall
Hi Oleksandr,
Quoted 11 lines · Oleksandr Andrushchenko
Quoted 10 lines · Julien Grall
Hi Oleksandr,
Quoted 6 lines · Oleksandr Andrushchenko
Quoted 4 lines · Jan Beulich
Quoted 4 lines
+ *
+ *gref_dir_next_page-grant_ref_t, reference to the next page describing
+ * page directory. Must be 0 if no more pages in the list.
Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
If I am not mistaken 0 is a valid grant.
Then I will remove this sentence, anyways BE knows how many grefs there are for the buffer size given
BTW,
xen-blkfrint.c:
#defineGRANT_INVALID_REF0
this is from where I got "Must be 0 if no more pages in the list."
GRANT_INVALID_REF is internally to Linux and never exposed in the PV driver. So for me it is implementation details because ref 0 could be allocated (log dump by Xen):
(XEN) -------- active -------- -------- shared --------
(XEN) [ref] localdom mfn pin localdom gmfn flags
(XEN) grant-table for remote domain: 2 (v1)
(XEN) [ 0] 0 0x99bf35 0x00000001 0 0x039000 0x19 (XEN) [ 1] 0 0x99bf33 0x00000001 0 0x039001 0x19
Cheers,
Signature · 6 lines
Julien Grall _______________________________________________ Xen-devel mailing list [email protected] https://lists.xen.org/xen-devel
Re: [PATCH v13] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
Quoted 31 lines · Julien Grall
Hi,
Quoted 15 lines · Oleksandr Andrushchenko
Quoted 14 lines · Julien Grall
Hi Oleksandr,
Quoted 11 lines · Oleksandr Andrushchenko
Quoted 10 lines · Julien Grall
Hi Oleksandr,
Quoted 6 lines · Oleksandr Andrushchenko
Quoted 4 lines · Jan Beulich
Quoted 4 lines
+ *
+ *gref_dir_next_page-grant_ref_t, reference to the next page describing
+ * page directory. Must be 0 if no more pages in the list.Quoted 8 lines
Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
If I am not mistaken 0 is a valid grant.
Then I will remove this sentence, anyways BE knows how many grefs there
are for the buffer size givenBTW,
xen-blkfrint.c:
#defineGRANT_INVALID_REF0
this is from where I got "Must be 0 if no more pages in the list."
GRANT_INVALID_REFis internally to Linux and never exposed in the PV driver. So for me it is implementation details because ref 0 could be allocated (log dump by Xen):
Quoted 5 lines
(XEN) -------- active -------- -------- shared --------
(XEN) [ref] localdom mfn pin localdom gmfn flags
(XEN) grant-table for remote domain: 2 (v1)(XEN) [ 0] 0 0x99bf35 0x00000001 0 0x039000 0x19 (XEN) [ 1] 0 0x99bf33 0x00000001 0 0x039001 0x19
Grant reference 0 is reserved in the ABI for the paravirtual console.
This reuse looks erroneous on behalf of blkfront.
~Andrew
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel
Re: [PATCH v13] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
Quoted 32 lines · Andrew Cooper
Quoted 31 lines · Julien Grall
Hi,
Quoted 15 lines · Oleksandr Andrushchenko
Quoted 14 lines · Julien Grall
Hi Oleksandr,
Quoted 11 lines · Oleksandr Andrushchenko
Quoted 10 lines · Julien Grall
Hi Oleksandr,
Quoted 6 lines · Oleksandr Andrushchenko
Quoted 4 lines · Jan Beulich
Quoted 4 lines
+ *
+ *gref_dir_next_page-grant_ref_t, reference to the next page describing
+ * page directory. Must be 0 if no more pages in the list.Quoted 8 lines
Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
If I am not mistaken 0 is a valid grant.
Then I will remove this sentence, anyways BE knows how many grefs there
are for the buffer size givenBTW,
xen-blkfrint.c:
#defineGRANT_INVALID_REF0
this is from where I got "Must be 0 if no more pages in the list."
GRANT_INVALID_REFis internally to Linux and never exposed in the PV driver. So for me it is implementation details because ref 0 could be allocated (log dump by Xen):
Quoted 7 lines
Quoted 5 lines
(XEN) -------- active -------- -------- shared --------
(XEN) [ref] localdom mfn pin localdom gmfn flags
(XEN) grant-table for remote domain: 2 (v1)(XEN) [ 0] 0 0x99bf35 0x00000001 0 0x039000 0x19 (XEN) [ 1] 0 0x99bf33 0x00000001 0 0x039001 0x19Grant reference 0 is reserved in the ABI for the paravirtual console.
Oh, so considering grant ref 0 as invalid in a PV protocol would be fine?
Cheers,
Signature · 6 lines
Julien Grall _______________________________________________ Xen-devel mailing list [email protected] https://lists.xen.org/xen-devel
Re: [PATCH v13] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
Quoted 41 lines · Julien Grall
Quoted 38 lines · Andrew Cooper
Quoted 36 lines · Julien Grall
Hi,
Quoted 15 lines · Oleksandr Andrushchenko
Quoted 14 lines · Julien Grall
Hi Oleksandr,
Quoted 11 lines · Oleksandr Andrushchenko
Quoted 10 lines · Julien Grall
Hi Oleksandr,
Quoted 6 lines · Oleksandr Andrushchenko
Quoted 4 lines · Jan Beulich
Quoted 4 lines
+ *
+ *gref_dir_next_page-grant_ref_t, reference to the next page describing
+ * page directory. Must be 0 if no more pages in the list.Quoted 8 lines
Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
If I am not mistaken 0 is a valid grant.
Then I will remove this sentence, anyways BE knows how many grefs there
are for the buffer size givenBTW,
xen-blkfrint.c:
#defineGRANT_INVALID_REF0
this is from where I got "Must be 0 if no more pages in the list."
GRANT_INVALID_REFis internally to Linux and never exposed in the PV driver. So for me it is implementation details because ref 0 could be allocated (log dump by Xen):(XEN) -------- active -------- -------- shared --------
(XEN) [ref] localdom mfn pin localdom gmfn flags
(XEN) grant-table for remote domain: 2 (v1)(XEN) [ 0] 0 0x99bf35 0x00000001 0 0x039000 0x19 (XEN) [ 1] 0 0x99bf33 0x00000001 0 0x039001 0x19
Quoted 3 lines
Quoted 1 line
Grant reference 0 is reserved in the ABI for the paravirtual console.
Oh, so considering grant ref 0 as invalid in a PV protocol would be fine?
Just to summarize: strictly speaking, grant reference 0 is valid, but never exposed to a PV driver, because of the fact that it is already in use/reserved for the PV console. Taking into account this fact we can assume that 0 is GRANT_INVALID_REF and can be used in PV drivers just like xen-blkfront.c
does.
I will add a note on this in the protocol
Quoted 3 lines
Cheers,
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xen.org/xen-devel