| Message ID | 1486711798-32322-2-git-send-email-andr2000@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 10, 2017 at 09:29:58AM +0200, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > This is the ABI for the two halves of a para-virtualized > display driver. > > This protocol aims to provide a unified protocol which fits more > sophisticated use-cases than a framebuffer device can handle. At the > moment basic functionality is supported with the intention to extend: > o multiple dynamically allocated/destroyed framebuffers > o buffers of arbitrary sizes > o better configuration options including multiple display support > > Note: existing fbif can be used together with displif running at the > same time, e.g. on Linux one provides framebuffer and another DRM/KMS > > Future extensions to the existing protocol may include: > o allow display/connector cloning > o allow allocating objects other than display buffers > o add planes/overlays support > o support scaling > o support rotation > > ================================================== > Rationale for introducing this protocol instead of > using the existing fbif: > ================================================== > > 1. In/out event sizes > o fbif - 40 octets > o displif - 40 octets > This is only the initial version of the displif protocol > which means that there could be requests which will not fit > (WRT introducing some GPU related functionality > later on). In that case we cannot alter fbif sizes as we need to > be backward compatible an will be forced to handle those > apart of fbif. > > 2. Shared page > Displif doesn't use anything like struct xenfb_page, but > DEFINE_RING_TYPES(xen_displif, struct xendispl_req, struct > xendispl_resp) which is a better and more common way. > Output events use a shared page which only has in_cons and in_prod > and all the rest is used for incoming events. Here struct xenfb_page > could probably be used as is despite the fact that it only has a half > of a page for incoming events which is only 50 events. (consider > something like 60Hz display) > > 3. Amount of changes. > fbif only provides XENFB_TYPE_UPDATE and XENFB_TYPE_RESIZE > events, so it looks like it is easier to get fb support into displif .. would it make sense to reserve some of those values (2, 3) in the XENDISPL_OP_ values? So that if this happens there is a nice fit in there? Thought looking at the structure there is no easy way to 'overlay' the xenfb_out_event structure as it is missing the 'id'. I guess one can get creative. Or you could swap positions of 'id' and 'type'? And then it would fit much nicer? > than vice versa. displif at the moment has 6 requests and 1 event, > multiple connector support, etc. > > Changes since v2: > * updated XenStore configuration template/pattern > * added "Recovery flow" to state diagram description > * renamed gref_directory_start to gref_directory > * added missing "versions" and "version" string constants > > Changes since v1: > * fixed xendispl_event_page padding size > * added versioning support > * explicitly define value types for XenStore fields > * text decoration re-work > * added offsets to ASCII box notation > > Changes since initial: > * DRM changed to DISPL, protocol made generic > * major re-work addressing issues raised for sndif > > Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > --- > xen/include/public/io/displif.h | 778 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 778 insertions(+) > create mode 100644 xen/include/public/io/displif.h > > diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h > new file mode 100644 > index 000000000000..849f27fe5f1d > --- /dev/null > +++ b/xen/include/public/io/displif.h > @@ -0,0 +1,778 @@ > +/****************************************************************************** > + * displif.h > + * > + * Unified display 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) 2016-2017 EPAM Systems Inc. > + * > + * Authors: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > + * Oleksandr Grytsov <oleksandr_grytsov@epam.com> > + */ > + > +#ifndef __XEN_PUBLIC_IO_DISPLIF_H__ > +#define __XEN_PUBLIC_IO_DISPLIF_H__ > + > +#include "ring.h" > +#include "../grant_table.h" > + > +/* > + ****************************************************************************** > + * Main features provided by the protocol > + ****************************************************************************** > + * This protocol aims to provide a unified protocol which fits more > + * sophisticated use-cases than a framebuffer device can handle. At the > + * moment basic functionality is supported with the intention to be extended: > + * o multiple dynamically allocated/destroyed framebuffers > + * o buffers of arbitrary sizes > + * o buffer allocation at either back or front end > + * o better configuration options including multiple display support > + * > + * Note: existing fbif can be used together with displif running at the > + * same time, e.g. on Linux one provides framebuffer and another DRM/KMS > + * > + ****************************************************************************** > + * Direction of improvements > + ****************************************************************************** > + * Future extensions to the existing protocol may include: > + * o display/connector cloning > + * o allocation of objects other than display buffers > + * o plane/overlay support > + * o scaling support > + * o rotation support > + * > + ****************************************************************************** > + * Feature and Parameter Negotiation > + ****************************************************************************** > + * > + * Front->back notifications: when enqueuing a new request, sending a > + * notification can be made conditional on xendispl_req (i.e., the generic > + * hold-off mechanism provided by the ring macros). Backends must set > + * xendispl_req 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 xendispl_resp (i.e., the generic > + * hold-off mechanism provided by the ring macros). Frontends must set > + * xendispl_resp appropriately (e.g., using RING_FINAL_CHECK_FOR_RESPONSES()). > + * > + * The two halves of a para-virtual display driver utilize nodes within > + * XenStore to communicate capabilities and to negotiate operating parameters. > + * This section enumerates these nodes which reside in the respective front and > + * backend portions of XenStore, following the XenBus convention. > + * > + * All data in 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. > + * > + ****************************************************************************** > + * Example configuration > + ****************************************************************************** > + * > + * Note: depending on the use-case backend can expose more display connectors > + * than the underlying HW physically has by employing SW graphics compositors > + * > + * This is an example of backend and frontend configuration: > + * > + *--------------------------------- Backend ----------------------------------- > + * > + * /local/domain/0/backend/vdispl/1/0/frontend-id = "1" > + * /local/domain/0/backend/vdispl/1/0/frontend = "/local/domain/1/device/vdispl/0" > + * /local/domain/0/backend/vdispl/1/0/state = "4" > + * /local/domain/0/backend/vdispl/1/0/versions = "1,2" > + * /local/domain/0/backend/vdispl/1/0/features = "be_alloc" > + * > + *--------------------------------- Frontend ---------------------------------- > + * > + * /local/domain/1/device/vdispl/0/backend-id = "0" > + * /local/domain/1/device/vdispl/0/backend = "/local/domain/0/backend/vdispl/1/0" > + * /local/domain/1/device/vdispl/0/state = "4" > + * /local/domain/1/device/vdispl/0/version = "1" > + * > + *-------------------------- Connector 0 configuration ------------------------ > + * > + * /local/domain/1/device/vdispl/0/0/resolution = "1920x1080" > + * /local/domain/1/device/vdispl/0/0/ctrl-ring-ref = "2832" > + * /local/domain/1/device/vdispl/0/0/ctrl-channel = "15" > + * /local/domain/1/device/vdispl/0/0/event-ring-ref = "387" > + * /local/domain/1/device/vdispl/0/0/event-channel = "16" > + * > + *-------------------------- Connector 1 configuration ------------------------ > + * > + * /local/domain/1/device/vdispl/0/1/resolution = "800x600" > + * /local/domain/1/device/vdispl/0/1/ctrl-ring-ref = "2833" > + * /local/domain/1/device/vdispl/0/1/ctrl-channel = "17" > + * /local/domain/1/device/vdispl/0/1/event-ring-ref = "388" > + * /local/domain/1/device/vdispl/0/1/event-channel = "18" > + * > + ****************************************************************************** > + * Backend XenBus Nodes > + ****************************************************************************** > + * > + *----------------------------- Protocol version ------------------------------ > + * > + * versions > + * Values: <string> > + * > + * List of XENDISPL_LIST_SEPARATOR separated protocol versions supported > + * by the backend. For example "1,2,3". > + * > + * features > + * Values: <list of strings> > + * > + * XENDISPL_LIST_SEPARATOR separated list of features > + * XENDISPL_FEATURE_BE_??? that the backend supports. > + * > + * be_alloc > + * Backend can be a buffer provider/allocator during > + * XENDISPL_OP_DBUF_CREATE operation (see below for negotiation). > + * > + ****************************************************************************** > + * Frontend XenBus Nodes > + ****************************************************************************** > + * > + *-------------------------------- Addressing --------------------------------- > + * > + * dom-id > + * Values: <uint16_t> > + * > + * Domain identifier. > + * > + * dev-id > + * Values: <uint16_t> > + * > + * Device identifier. > + * > + * conn-idx > + * Values: <uint8_t> > + * > + * Zero based contigous index of the connector. > + * /local/domain/<dom-id>/device/vdispl/<dev-id>/<conn-idx>/... > + * > + *----------------------------- Protocol version ------------------------------ > + * > + * version > + * Values: <string> > + * > + * Protocol version, chosen among the ones supported by the backend. > + * > + *----------------------------- Connector settings ---------------------------- > + * > + * resolution > + * Values: <width, uint32_t>x<height, uint32_t> > + * > + * Width and height of the connector in pixels separated by > + * XENDISPL_RESOLUTION_SEPARATOR. Is there an default depth? 32? Or should that also be negotiated? > + * > + *------------------ Connector Request Transport Parameters ------------------- > + * > + * ctrl-channel Please call it 'ctrl-event-channel' > + * Values: <uint32_t> > + * > + * The identifier of the Xen connector's control event channel > + * used to signal activity in the ring buffer. > + * > + * ctrl-ring-ref > + * Values: <uint32_t> > + * > + * The Xen grant reference granting permission for the backend to map > + * a sole page in a single page sized connector's control ring buffer. > + * > + * event-channel > + * Values: <uint32_t> > + * > + * The identifier of the Xen connector's event channel > + * used to signal activity in the ring buffer. > + * > + * event-ring-ref > + * Values: <uint32_t> > + * > + * The Xen grant reference granting permission for the backend to map > + * a sole page in a single page sized connector's event ring buffer. So you are making two rings and two event channels - and it looks like the operations are going via the control ring.. but what goes through the the other ring buffer? Or is the other ring buffer the one that is created via 'gref_directory' ? If so, wouldn't you need mulitple event channels? And also the size of those rings is much much bigger. But it is not really a ring. It looks to hold some opaque data? -ECONFUSED. > + */ > + > +/* > + ****************************************************************************** > + * STATE DIAGRAMS > + ****************************************************************************** > + * > + * Tool stack creates front and back state nodes with initial state > + * XenbusStateInitialising. > + * Tool stack creates and sets up frontend display configuration > + * nodes per domain. > + * > + *-------------------------------- Normal flow -------------------------------- > + * > + * 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 > + * connector. > + * 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 display connectors > + * as per configuration. > + * | > + * | > + * V > + * XenbusStateConnected > + * > + * XenbusStateUnknown > + * XenbusStateClosed > + * XenbusStateClosing > + * o Remove virtual display device > + * o Remove event channels > + * | > + * | > + * V > + * XenbusStateClosed > + * > + *------------------------------- Recovery flow ------------------------------- > + * > + * In case of frontend unrecoverable errors backend handles that as > + * if frontend goes into the XenbusStateClosed state. > + * > + * In case of backend unrecoverable errors frontend tries removing > + * the emulated device. If this is possible at the moment of error, What emulated device? > + * then frontend goes into the XenbusStateInitialising state and is ready for > + * new connection with backend. If the emulated device is still in use and emulated device? > + * cannot be removed, then frontend goes into the XenbusStateReconfiguring state .. and what is done in XenbusStateReconfiguring state? I am assuming 'resolution' is examined? > + * until either the emulated device removed or backend initiates a new > + * connection. On the emulated device removal frontend goes into the emulated device? > + * XenbusStateInitialising state. > + * > + ****************************************************************************** > + * REQUEST CODES > + ****************************************************************************** > + */ > +#define XENDISPL_OP_DBUF_CREATE 0 > +#define XENDISPL_OP_DBUF_DESTROY 1 > +#define XENDISPL_OP_FB_ATTACH 2 > +#define XENDISPL_OP_FB_DETACH 3 > +#define XENDISPL_OP_SET_CONFIG 4 > +#define XENDISPL_OP_PG_FLIP 5 > + > +/* > + ****************************************************************************** > + * EVENT CODES > + ****************************************************************************** > + */ > +#define XENDISPL_EVT_PG_FLIP 0 > + > +/* > + ****************************************************************************** > + * XENSTORE FIELD AND PATH NAME STRINGS, HELPERS > + ****************************************************************************** > + */ > +#define XENDISPL_DRIVER_NAME "vdispl" > + > +#define XENDISPL_LIST_SEPARATOR "," > +#define XENDISPL_RESOLUTION_SEPARATOR "x" > +/* Field names */ > +#define XENDISPL_FIELD_BE_VERSIONS "versions" > +#define XENDISPL_FIELD_FE_VERSION "version" > +#define XENDISPL_FIELD_FEATURES "features" > +#define XENDISPL_FIELD_CTRL_RING_REF "ctrl-ring-ref" > +#define XENDISPL_FIELD_CTRL_CHANNEL "ctrl-channel" > +#define XENDISPL_FIELD_EVT_RING_REF "event-ring-ref" > +#define XENDISPL_FIELD_EVT_CHANNEL "event-channel" > +#define XENDISPL_FIELD_RESOLUTION "resolution" > + > +#define XENDISPL_FEATURE_BE_ALLOC "be_alloc" > + > +/* > + ****************************************************************************** > + * STATUS RETURN CODES > + ****************************************************************************** > + * > + * Status return code is zero on success and -XEN_EXX on failure. > + * > + ****************************************************************************** > + * Assumptions > + ****************************************************************************** > + * o usage of grant reference 0 as invalid grant reference: > + * grant reference 0 is valid, but never exposed to a PV driver, > + * because of the fact it is already in use/reserved by the PV console. > + * o all references in this document to page sizes must be treated > + * as pages of size XEN_PAGE_SIZE unless otherwise noted. > + * > + ****************************************************************************** > + * Description of the protocol between frontend and backend driver > + ****************************************************************************** > + * > + * The two halves of a Para-virtual display driver communicate with > + * each other using shared pages and event channels. > + * Shared page contains a ring with request/response packets. > + * > + * All reserved fields in the structures below must be 0. > + * Display buffers's cookie of value 0 treated as invalid. > + * Framebuffer's cookie of value 0 treated as invalid. > + * > + *---------------------------------- Requests --------------------------------- > + * > + * All requests/responses, which are not connector specific, must be sent over > + * control ring of the connector with index 0. > + * > + * For all request/response/event packets that use cookies: > + * dbuf_cookie - uint64_t, unique to guest domain value used by the backend > + * to map remote display buffer to its local one > + * fb_cookie - uint64_t, unique to guest domain value used by the backend > + * to map remote framebuffer to its local one > + * > + * All request packets have the same length (64 octets) > + * All request packets have common header: > + * 0 1 2 3 octet > + * +----------------+----------------+----------------+----------------+ > + * | id | operation | reserved | 4 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 8 > + * +----------------+----------------+----------------+----------------+ > + * id - uint16_t, private guest value, echoed in response > + * operation - uint8_t, operation code, XENDISPL_OP_??? > + * > + * Request dbuf creation - request creation of a display buffer. > + * 0 1 2 3 octet > + * +----------------+----------------+----------------+----------------+ > + * | id |_OP_DBUF_CREATE | reserved | 4 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 8 > + * +----------------+----------------+----------------+----------------+ > + * | dbuf_cookie low 32-bit | 12 > + * +----------------+----------------+----------------+----------------+ > + * | dbuf_cookie high 32-bit | 16 > + * +----------------+----------------+----------------+----------------+ > + * | width | 20 > + * +----------------+----------------+----------------+----------------+ > + * | height | 24 > + * +----------------+----------------+----------------+----------------+ > + * | bpp | 28 > + * +----------------+----------------+----------------+----------------+ > + * | buffer_sz | 32 > + * +----------------+----------------+----------------+----------------+ > + * | flags | 36 > + * +----------------+----------------+----------------+----------------+ > + * | gref_directory | 40 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 44 > + * +----------------+----------------+----------------+----------------+ > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 64 > + * +----------------+----------------+----------------+----------------+ > + * > + * Must be sent over control ring of the connector with index 0. > + * > + * width - uint32_t, width in pixels > + * height - uint32_t, height in pixels Why? Isn't that in the 'resolution' XenBus entry? If not, why even have the 'resolution' field? And also - what if the width or height are bigger than 'resolution'? Should -EINVAL be returned? > + * bpp - uint32_t, bits per pixel > + * buffer_sz - uint32_t, buffer size to be allocated, octets > + * flags - uint32_t, flags of the operation > + * o XENDISPL_DBUF_FLG_REQ_ALLOC - if set, then backend is requested > + * to allocate the buffer with the parameters provided in this request. > + * Page directory is handled as follows: > + * Frontend on request: > + * o allocates pages for the directory > + * o grants permissions for the pages of the directory > + * o sets gref_dir_next_page fields > + * Backend on response: > + * o grants permissions for the pages of the buffer allocated <scratches his head> > + * o fills in page directory with grant references The backend provides the pages instead of the frontend? But you said 'allocated pages for the directory' in the frontned part - and since you say 'pages' that means more than one? Or did you mean 'the gref_directory' page and the 'gref_dir_next_page' pages? You may want to be explicit. And what if the 'flags' is zero? Or 0x314? Perhaps also mention the default one (0 I presume) which would make the backend map the frontend pages provided in 'gref_directory'? > + * gref_directory - grant_ref_t, a reference to the first shared page > + * describing shared buffer references. At least one page exists. If shared > + * buffer size (buffer_sz) exceeds what can be addressed by this single page, ^- extra space > + * then reference to the next page must be supplied (see gref_dir_next_page > + * below) > + */ What are some of the return errors ? Would it make sense to mention some? What to for example if there are multiple _OP_DBUF_CREATE with the same dbuf_cookie but with different width/height? > + > +#define XENDISPL_DBUF_FLG_REQ_ALLOC 0x0001 > + > +struct xendispl_dbuf_create_req { > + uint64_t dbuf_cookie; > + uint32_t width; > + uint32_t height; > + uint32_t bpp; > + uint32_t buffer_sz; > + uint32_t flags; > + grant_ref_t gref_directory; > +}; > + > +/* > + * Shared page for XENDISPL_OP_DBUF_CREATE 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 | 4 > + * +----------------+----------------+----------------+----------------+ > + * | gref[0] | 8 > + * +----------------+----------------+----------------+----------------+ > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > + * +----------------+----------------+----------------+----------------+ > + * | gref[i] | i*4+8 > + * +----------------+----------------+----------------+----------------+ > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > + * +----------------+----------------+----------------+----------------+ > + * | gref[N - 1] | N*4+8 > + * +----------------+----------------+----------------+----------------+ > + * > + * gref_dir_next_page - grant_ref_t, reference to the next page describing > + * page directory. Must be 0 if there are no more pages in the list. > + * gref[i] - grant_ref_t, reference to a shared page of the buffer > + * allocated at XENDISPL_OP_DBUF_CREATE > + * > + * Number of grant_ref_t entries in the whole page directory is not > + * passed, but instead can be calculated as: > + * num_grefs_total = (XENDISPL_OP_DBUF_CREATE.buffer_sz + XEN_PAGE_SIZE - 1) / > + * XEN_PAGE_SIZE > + */ > + > +struct xendispl_page_directory { > + grant_ref_t gref_dir_next_page; > + grant_ref_t gref[1]; /* Variable length */ > +}; > + > +/* > + * Request dbuf destruction - destroy a previously allocated display buffer: > + * 0 1 2 3 octet > + * +----------------+----------------+----------------+----------------+ > + * | id |_OP_DBUF_DESTROY| reserved | 4 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 8 > + * +----------------+----------------+----------------+----------------+ > + * | dbuf_cookie low 32-bit | 12 > + * +----------------+----------------+----------------+----------------+ > + * | dbuf_cookie high 32-bit | 16 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 20 > + * +----------------+----------------+----------------+----------------+ > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 64 > + * +----------------+----------------+----------------+----------------+ > + * > + * Must be sent over control ring of the connector with index 0. > + */ > + > +struct xendispl_dbuf_destroy_req { > + uint64_t dbuf_cookie; > +}; > + > +/* > + * Request framebuffer attachment - request attachment of a framebuffer to > + * previously created display buffer. > + * 0 1 2 3 octet > + * +----------------+----------------+----------------+----------------+ > + * | id | _OP_FB_ATTACH | reserved | 4 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 8 > + * +----------------+----------------+----------------+----------------+ > + * | dbuf_cookie low 32-bit | 12 > + * +----------------+----------------+----------------+----------------+ > + * | dbuf_cookie high 32-bit | 16 > + * +----------------+----------------+----------------+----------------+ > + * | fb_cookie low 32-bit | 20 > + * +----------------+----------------+----------------+----------------+ > + * | fb_cookie high 32-bit | 24 > + * +----------------+----------------+----------------+----------------+ > + * | width | 28 > + * +----------------+----------------+----------------+----------------+ > + * | height | 32 > + * +----------------+----------------+----------------+----------------+ > + * | pixel_format | 36 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 40 > + * +----------------+----------------+----------------+----------------+ > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 64 > + * +----------------+----------------+----------------+----------------+ > + * > + * Must be sent over control ring of the connector with index 0. > + * > + * width - uint32_t, width in pixels > + * height - uint32_t, height in pixels Why? The _OP_DBUF_CREATE already specified the height/width. Why have this? Or are you thinking to have an smaller (or bigger?) resolution framebuffer than the display? I suppose you can do that which would imply that the display would switch modes? Perhaps mention this? Also what if these values are bigger (or lesser) than 'resolution' ? > + * pixel_format - uint32_t, pixel format of the framebuffer, FOURCC code You did not mention the fb_cookie? > + */ > + > +struct xendispl_fb_attach_req { > + uint64_t dbuf_cookie; > + uint64_t fb_cookie; > + uint32_t width; > + uint32_t height; > + uint32_t pixel_format; > +}; > + > +/* > + * Request framebuffer detach - detach a previously > + * attached framebuffer from the display buffer in request: > + * 0 1 2 3 octet > + * +----------------+----------------+----------------+----------------+ > + * | id | _OP_FB_DETACH | reserved | 4 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 8 > + * +----------------+----------------+----------------+----------------+ > + * | fb_cookie low 32-bit | 12 > + * +----------------+----------------+----------------+----------------+ > + * | fb_cookie high 32-bit | 16 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 20 > + * +----------------+----------------+----------------+----------------+ > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 64 > + * +----------------+----------------+----------------+----------------+ > + * > + * Must be sent over control ring of the connector with index 0. > + */ > + > +struct xendispl_fb_detach_req { > + uint64_t fb_cookie; > +}; > + > +/* > + * Request configuration set/reset - request to set or reset > + * the configuration/mode of the display: > + * 0 1 2 3 octet > + * +----------------+----------------+----------------+----------------+ > + * | id | _OP_SET_CONFIG | reserved | 4 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 8 > + * +----------------+----------------+----------------+----------------+ > + * | fb_cookie low 32-bit | 12 > + * +----------------+----------------+----------------+----------------+ > + * | fb_cookie high 32-bit | 16 > + * +----------------+----------------+----------------+----------------+ > + * | x | 20 > + * +----------------+----------------+----------------+----------------+ > + * | y | 24 > + * +----------------+----------------+----------------+----------------+ > + * | width | 28 > + * +----------------+----------------+----------------+----------------+ > + * | height | 32 > + * +----------------+----------------+----------------+----------------+ > + * | bpp | 40 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 44 > + * +----------------+----------------+----------------+----------------+ > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 64 > + * +----------------+----------------+----------------+----------------+ > + * > + * Pass all zeros to reset, otherwise command is treated as > + * configuration set. All zeros? Including fb_cookie? > + * If this is a set configuration request then framebuffer's cookie tells Not sure if the first part of the sentence is needed. This is after all the section that talks about setting configurations. > + * the display which framebuffer/dbuf must be displayed while enabling display > + * (applying configuration). Can you disable an display without changing this? > + * > + * x - uint32_t, starting position in pixels by X axis > + * y - uint32_t, starting position in pixels by Y axis Which is bound by 'resolution' ? Or by the display command? > + * width - uint32_t, width in pixels > + * height - uint32_t, height in pixels > + * bpp - uint32_t, bits per pixel Shouldn't there be an stride as well? > + */ > + > +struct xendispl_set_config_req { > + uint64_t fb_cookie; > + uint32_t x; > + uint32_t y; > + uint32_t width; > + uint32_t height; > + uint32_t bpp; > +}; > + > +/* > + * Request page flip - request to flip a page identified by the framebuffer > + * cookie: > + * 0 1 2 3 octet > + * +----------------+----------------+----------------+----------------+ > + * | id | _OP_PG_FLIP | reserved | 4 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 8 > + * +----------------+----------------+----------------+----------------+ > + * | fb_cookie low 32-bit | 12 > + * +----------------+----------------+----------------+----------------+ > + * | fb_cookie high 32-bit | 16 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 20 > + * +----------------+----------------+----------------+----------------+ > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 64 > + * +----------------+----------------+----------------+----------------+ > + */ > + > +struct xendispl_page_flip_req { > + uint64_t fb_cookie; > +}; > + > +/* > + *---------------------------------- Responses -------------------------------- > + * > + * All response packets have the same length (64 octets) > + * > + * All response packets have common header: > + * 0 1 2 3 octet > + * +----------------+----------------+----------------+----------------+ > + * | id | reserved | 4 > + * +----------------+----------------+----------------+----------------+ > + * | status | 8 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 12 > + * +----------------+----------------+----------------+----------------+ > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 64 > + * +----------------+----------------+----------------+----------------+ > + * > + * id - uint16_t, private guest value, echoed from request > + * status - int32_t, response status, zero on success and -XEN_EXX on failure > + * > + *----------------------------------- Events ---------------------------------- > + * > + * Events are sent via a shared page allocated by the front and propagated by > + * event-channel/event-ring-ref XenStore entries > + * All event packets have the same length (64 octets) > + * All event packets have common header: > + * 0 1 2 3 octet > + * +----------------+----------------+----------------+----------------+ > + * | id | type | reserved | 4 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 8 > + * +----------------+----------------+----------------+----------------+ > + * > + * id - uint16_t, event id, may be used by front > + * type - uint8_t, type of the event > + * > + * > + * Page flip complete event - event from back to front on page flip completed: > + * 0 1 2 3 octet > + * +----------------+----------------+----------------+----------------+ > + * | id | _EVT_PG_FLIP | reserved | 4 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 8 > + * +----------------+----------------+----------------+----------------+ > + * | fb_cookie low 32-bit | 12 > + * +----------------+----------------+----------------+----------------+ > + * | fb_cookie high 32-bit | 16 > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 20 > + * +----------------+----------------+----------------+----------------+ > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > + * +----------------+----------------+----------------+----------------+ > + * | reserved | 64 > + * +----------------+----------------+----------------+----------------+ > + */ > + > +struct xendispl_pg_flip_evt { > + uint64_t fb_cookie; > +}; > + > +struct xendispl_req { > + uint16_t id; > + uint8_t operation; > + uint8_t reserved[5]; > + union { > + struct xendispl_dbuf_create_req dbuf_create; > + struct xendispl_dbuf_destroy_req dbuf_destroy; > + struct xendispl_fb_attach_req fb_attach; > + struct xendispl_fb_detach_req fb_detach; > + struct xendispl_set_config_req set_config; > + struct xendispl_page_flip_req pg_flip; > + uint8_t reserved[56]; > + } op; > +}; > + > +struct xendispl_resp { > + uint16_t id; > + uint8_t operation; > + uint8_t reserved; > + int32_t status; > + uint8_t reserved1[56]; > +}; > + > +struct xendispl_evt { > + uint16_t id; > + uint8_t type; > + uint8_t reserved[5]; > + union { > + struct xendispl_pg_flip_evt pg_flip; > + uint8_t reserved[56]; > + } op; > +}; > + > +DEFINE_RING_TYPES(xen_displif, struct xendispl_req, struct xendispl_resp); > + > +/* > + ****************************************************************************** > + * Back to front events delivery > + ****************************************************************************** > + * In order to deliver asynchronous events from back to front a shared page is > + * allocated by front and its grefs propagated to back via XenStore entries > + * (event-XXX). ? You mean event-channel ? Or control-event-channel? > + * This page has a common header used by both front and back to synchronize > + * access and control event's ring buffer, while back being a producer of the > + * events and front being a consumer. The rest of the page after the header > + * is used for event packets. > + * > + * Upon reception of an event(s) front may confirm its reception > + * for either each event, group of events or none. > + */ > + > +struct xendispl_event_page { > + uint32_t in_cons; > + uint32_t in_prod; > + uint8_t reserved[56]; > +}; > + > +#define XENDISPL_EVENT_PAGE_SIZE 4096 > +#define XENDISPL_IN_RING_OFFS (sizeof(struct xendispl_event_page)) > +#define XENDISPL_IN_RING_SIZE (XENDISPL_EVENT_PAGE_SIZE - XENDISPL_IN_RING_OFFS) > +#define XENDISPL_IN_RING_LEN (XENDISPL_IN_RING_SIZE / sizeof(struct xendispl_evt)) > +#define XENDISPL_IN_RING(page) \ > + ((struct xendispl_evt *)((char *)(page) + XENDISPL_IN_RING_OFFS)) > +#define XENDISPL_IN_RING_REF(page, idx) \ > + (XENDISPL_IN_RING((page))[(idx) % XENDISPL_IN_RING_LEN]) Why not use the ring.h macros? Especially as the top of this talks about "RING_FINAL_CHECK_FOR_REQUESTS"? > + > +#endif /* __XEN_PUBLIC_IO_DISPLIF_H__ */ Could we also have this header in: /* * Local variables: * mode: C * c-file-style: "BSD" * c-basic-offset: 4 * tab-width: 4 * indent-tabs-mode: nil * End: */ please? > -- > 2.7.4 >
Hi, Konrad! Thank you for reviewing this. On 02/10/2017 11:27 PM, Konrad Rzeszutek Wilk wrote: > On Fri, Feb 10, 2017 at 09:29:58AM +0200, Oleksandr Andrushchenko wrote: >> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> >> This is the ABI for the two halves of a para-virtualized >> display driver. >> >> This protocol aims to provide a unified protocol which fits more >> sophisticated use-cases than a framebuffer device can handle. At the >> moment basic functionality is supported with the intention to extend: >> o multiple dynamically allocated/destroyed framebuffers >> o buffers of arbitrary sizes >> o better configuration options including multiple display support >> >> Note: existing fbif can be used together with displif running at the >> same time, e.g. on Linux one provides framebuffer and another DRM/KMS >> >> Future extensions to the existing protocol may include: >> o allow display/connector cloning >> o allow allocating objects other than display buffers >> o add planes/overlays support >> o support scaling >> o support rotation >> >> ================================================== >> Rationale for introducing this protocol instead of >> using the existing fbif: >> ================================================== >> >> 1. In/out event sizes >> o fbif - 40 octets >> o displif - 40 octets >> This is only the initial version of the displif protocol >> which means that there could be requests which will not fit >> (WRT introducing some GPU related functionality >> later on). In that case we cannot alter fbif sizes as we need to >> be backward compatible an will be forced to handle those >> apart of fbif. >> >> 2. Shared page >> Displif doesn't use anything like struct xenfb_page, but >> DEFINE_RING_TYPES(xen_displif, struct xendispl_req, struct >> xendispl_resp) which is a better and more common way. >> Output events use a shared page which only has in_cons and in_prod >> and all the rest is used for incoming events. Here struct xenfb_page >> could probably be used as is despite the fact that it only has a half >> of a page for incoming events which is only 50 events. (consider >> something like 60Hz display) >> >> 3. Amount of changes. >> fbif only provides XENFB_TYPE_UPDATE and XENFB_TYPE_RESIZE >> events, so it looks like it is easier to get fb support into displif > .. would it make sense to reserve some of those values (2, 3) > in the XENDISPL_OP_ values? So that if this happens there is a nice > fit in there? Thought looking at the structure there is no easy > way to 'overlay' the xenfb_out_event structure as it is missing the 'id'. > > I guess one can get creative. > > Or you could swap positions of 'id' and 'type'? And then it would fit much > nicer? yeap, in order no one needs to be creative, why not explicitly define those? Anyways, it won't be possible to simply lay the structures from fbif on top of displif (different structure, size, workflow etc.), what is more that would give some room to find workarounds, rather than have well defined solution. BTW, there was an attempt [1] to update fbdev to meet modern application needs. If we decide to add FBDEV functionality into this protocol, then I can re-use already proven to work solution from [1] into DISPLIF, but defining structures and events to fit the current protocol. What do you think? >> than vice versa. displif at the moment has 6 requests and 1 event, >> multiple connector support, etc. >> >> Changes since v2: >> * updated XenStore configuration template/pattern >> * added "Recovery flow" to state diagram description >> * renamed gref_directory_start to gref_directory >> * added missing "versions" and "version" string constants >> >> Changes since v1: >> * fixed xendispl_event_page padding size >> * added versioning support >> * explicitly define value types for XenStore fields >> * text decoration re-work >> * added offsets to ASCII box notation >> >> Changes since initial: >> * DRM changed to DISPL, protocol made generic >> * major re-work addressing issues raised for sndif >> >> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> --- >> xen/include/public/io/displif.h | 778 ++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 778 insertions(+) >> create mode 100644 xen/include/public/io/displif.h >> >> diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h >> new file mode 100644 >> index 000000000000..849f27fe5f1d >> --- /dev/null >> +++ b/xen/include/public/io/displif.h >> @@ -0,0 +1,778 @@ >> +/****************************************************************************** >> + * displif.h >> + * >> + * Unified display 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) 2016-2017 EPAM Systems Inc. >> + * >> + * Authors: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> + * Oleksandr Grytsov <oleksandr_grytsov@epam.com> >> + */ >> + >> +#ifndef __XEN_PUBLIC_IO_DISPLIF_H__ >> +#define __XEN_PUBLIC_IO_DISPLIF_H__ >> + >> +#include "ring.h" >> +#include "../grant_table.h" >> + >> +/* >> + ****************************************************************************** >> + * Main features provided by the protocol >> + ****************************************************************************** >> + * This protocol aims to provide a unified protocol which fits more >> + * sophisticated use-cases than a framebuffer device can handle. At the >> + * moment basic functionality is supported with the intention to be extended: >> + * o multiple dynamically allocated/destroyed framebuffers >> + * o buffers of arbitrary sizes >> + * o buffer allocation at either back or front end >> + * o better configuration options including multiple display support >> + * >> + * Note: existing fbif can be used together with displif running at the >> + * same time, e.g. on Linux one provides framebuffer and another DRM/KMS >> + * >> + ****************************************************************************** >> + * Direction of improvements >> + ****************************************************************************** >> + * Future extensions to the existing protocol may include: >> + * o display/connector cloning >> + * o allocation of objects other than display buffers >> + * o plane/overlay support >> + * o scaling support >> + * o rotation support >> + * >> + ****************************************************************************** >> + * Feature and Parameter Negotiation >> + ****************************************************************************** >> + * >> + * Front->back notifications: when enqueuing a new request, sending a >> + * notification can be made conditional on xendispl_req (i.e., the generic >> + * hold-off mechanism provided by the ring macros). Backends must set >> + * xendispl_req 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 xendispl_resp (i.e., the generic >> + * hold-off mechanism provided by the ring macros). Frontends must set >> + * xendispl_resp appropriately (e.g., using RING_FINAL_CHECK_FOR_RESPONSES()). >> + * >> + * The two halves of a para-virtual display driver utilize nodes within >> + * XenStore to communicate capabilities and to negotiate operating parameters. >> + * This section enumerates these nodes which reside in the respective front and >> + * backend portions of XenStore, following the XenBus convention. >> + * >> + * All data in 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. >> + * >> + ****************************************************************************** >> + * Example configuration >> + ****************************************************************************** >> + * >> + * Note: depending on the use-case backend can expose more display connectors >> + * than the underlying HW physically has by employing SW graphics compositors >> + * >> + * This is an example of backend and frontend configuration: >> + * >> + *--------------------------------- Backend ----------------------------------- >> + * >> + * /local/domain/0/backend/vdispl/1/0/frontend-id = "1" >> + * /local/domain/0/backend/vdispl/1/0/frontend = "/local/domain/1/device/vdispl/0" >> + * /local/domain/0/backend/vdispl/1/0/state = "4" >> + * /local/domain/0/backend/vdispl/1/0/versions = "1,2" >> + * /local/domain/0/backend/vdispl/1/0/features = "be_alloc" >> + * >> + *--------------------------------- Frontend ---------------------------------- >> + * >> + * /local/domain/1/device/vdispl/0/backend-id = "0" >> + * /local/domain/1/device/vdispl/0/backend = "/local/domain/0/backend/vdispl/1/0" >> + * /local/domain/1/device/vdispl/0/state = "4" >> + * /local/domain/1/device/vdispl/0/version = "1" >> + * >> + *-------------------------- Connector 0 configuration ------------------------ >> + * >> + * /local/domain/1/device/vdispl/0/0/resolution = "1920x1080" >> + * /local/domain/1/device/vdispl/0/0/ctrl-ring-ref = "2832" >> + * /local/domain/1/device/vdispl/0/0/ctrl-channel = "15" >> + * /local/domain/1/device/vdispl/0/0/event-ring-ref = "387" >> + * /local/domain/1/device/vdispl/0/0/event-channel = "16" >> + * >> + *-------------------------- Connector 1 configuration ------------------------ >> + * >> + * /local/domain/1/device/vdispl/0/1/resolution = "800x600" >> + * /local/domain/1/device/vdispl/0/1/ctrl-ring-ref = "2833" >> + * /local/domain/1/device/vdispl/0/1/ctrl-channel = "17" >> + * /local/domain/1/device/vdispl/0/1/event-ring-ref = "388" >> + * /local/domain/1/device/vdispl/0/1/event-channel = "18" >> + * >> + ****************************************************************************** >> + * Backend XenBus Nodes >> + ****************************************************************************** >> + * >> + *----------------------------- Protocol version ------------------------------ >> + * >> + * versions >> + * Values: <string> >> + * >> + * List of XENDISPL_LIST_SEPARATOR separated protocol versions supported >> + * by the backend. For example "1,2,3". >> + * >> + * features >> + * Values: <list of strings> >> + * >> + * XENDISPL_LIST_SEPARATOR separated list of features >> + * XENDISPL_FEATURE_BE_??? that the backend supports. >> + * >> + * be_alloc >> + * Backend can be a buffer provider/allocator during >> + * XENDISPL_OP_DBUF_CREATE operation (see below for negotiation). >> + * >> + ****************************************************************************** >> + * Frontend XenBus Nodes >> + ****************************************************************************** >> + * >> + *-------------------------------- Addressing --------------------------------- >> + * >> + * dom-id >> + * Values: <uint16_t> >> + * >> + * Domain identifier. >> + * >> + * dev-id >> + * Values: <uint16_t> >> + * >> + * Device identifier. >> + * >> + * conn-idx >> + * Values: <uint8_t> >> + * >> + * Zero based contigous index of the connector. >> + * /local/domain/<dom-id>/device/vdispl/<dev-id>/<conn-idx>/... >> + * >> + *----------------------------- Protocol version ------------------------------ >> + * >> + * version >> + * Values: <string> >> + * >> + * Protocol version, chosen among the ones supported by the backend. >> + * >> + *----------------------------- Connector settings ---------------------------- >> + * >> + * resolution >> + * Values: <width, uint32_t>x<height, uint32_t> >> + * >> + * Width and height of the connector in pixels separated by >> + * XENDISPL_RESOLUTION_SEPARATOR. > Is there an default depth? no > 32? no defaults > Or should that also be negotiated? frontend requests the desired value of it, backend provides it or rejects the request, so front will try other value. So, yes, this is kind of negotiation here (EINVAL based :) I will also put a note that "resolution" only defines visible area, thus display and frame buffer sizes can be different from this value >> + * >> + *------------------ Connector Request Transport Parameters ------------------- >> + * >> + * ctrl-channel > Please call it 'ctrl-event-channel' I will >> + * Values: <uint32_t> >> + * >> + * The identifier of the Xen connector's control event channel >> + * used to signal activity in the ring buffer. >> + * >> + * ctrl-ring-ref >> + * Values: <uint32_t> >> + * >> + * The Xen grant reference granting permission for the backend to map >> + * a sole page in a single page sized connector's control ring buffer. >> + * >> + * event-channel >> + * Values: <uint32_t> >> + * >> + * The identifier of the Xen connector's event channel >> + * used to signal activity in the ring buffer. >> + * >> + * event-ring-ref >> + * Values: <uint32_t> >> + * >> + * The Xen grant reference granting permission for the backend to map >> + * a sole page in a single page sized connector's event ring buffer. > So you are making two rings and two event channels - and it looks like the > operations are going via the control ring.. but what goes through the > the other ring buffer? Events from backend to frontend, currently only XENDISPL_EVT_PG_FLIP I will define 2 sections: *------------------ Connector Request Transport Parameters ------------------- * * ctrl-event-channel * ctrl-ring-ref * *------------------- Connector Event Transport Parameters -------------------- * * event-channel * event-ring-ref > Or is the other ring buffer the one that is created via 'gref_directory' ? no At the bottom: * In order to deliver asynchronous events from back to front a shared page is * allocated by front and its gref propagated to back via XenStore entries * (event-XXX). > If so, wouldn't you need mulitple event channels? And also the size of those > rings is much much bigger. But it is not really a ring. It looks to hold > some opaque data? > > -ECONFUSED. >> + */ >> + >> +/* >> + ****************************************************************************** >> + * STATE DIAGRAMS >> + ****************************************************************************** >> + * >> + * Tool stack creates front and back state nodes with initial state >> + * XenbusStateInitialising. >> + * Tool stack creates and sets up frontend display configuration >> + * nodes per domain. >> + * >> + *-------------------------------- Normal flow -------------------------------- >> + * >> + * 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 >> + * connector. >> + * 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 display connectors >> + * as per configuration. >> + * | >> + * | >> + * V >> + * XenbusStateConnected >> + * >> + * XenbusStateUnknown >> + * XenbusStateClosed >> + * XenbusStateClosing >> + * o Remove virtual display device >> + * o Remove event channels >> + * | >> + * | >> + * V >> + * XenbusStateClosed >> + * >> + *------------------------------- Recovery flow ------------------------------- >> + * >> + * In case of frontend unrecoverable errors backend handles that as >> + * if frontend goes into the XenbusStateClosed state. >> + * >> + * In case of backend unrecoverable errors frontend tries removing >> + * the emulated device. If this is possible at the moment of error, > What emulated device? s/emulated/virtualized/g for all? > >> + * then frontend goes into the XenbusStateInitialising state and is ready for >> + * new connection with backend. If the emulated device is still in use and > emulated device? > >> + * cannot be removed, then frontend goes into the XenbusStateReconfiguring state > .. and what is done in XenbusStateReconfiguring state? > > I am assuming 'resolution' is examined? > >> + * until either the emulated device removed or backend initiates a new >> + * connection. On the emulated device removal frontend goes into the > emulated device? The whole idea here is to let frontend recover from backend failure. What happens when backend dies: frontend cannot send requests to the backend and thus cannot emulate (virtualize) display device anymore. There is no simple solution like disconnecting rings and waiting for the backend to re-connect, as display device, just like sound devices, does hold some state: configuration in use, number of display buffers, framebuffers, application state, framework "watchdogs" (like vertical blank time-outs and recovery at framework level) etc. So, in most cases, it would require frontend to implement complex recovery logic. Instead, what I suggest, that we go into some state, which tells that we are not yet ready to go into Initialized state and still need some time: at this time frontend will try to make sure no new clients from user-space are accepted, it allows the current client to exit gracefully by returning error codes etc. Once all the clients are gone frontend can reinitialize the virtualized device, e.g. DRM/KMS driver, and get into Initialized state again. Thus, I was thinking of XenbusStateReconfiguringstate as appropriate in this case >> + * XenbusStateInitialising state. >> + * >> + ****************************************************************************** >> + * REQUEST CODES >> + ****************************************************************************** >> + */ >> +#define XENDISPL_OP_DBUF_CREATE 0 >> +#define XENDISPL_OP_DBUF_DESTROY 1 >> +#define XENDISPL_OP_FB_ATTACH 2 >> +#define XENDISPL_OP_FB_DETACH 3 >> +#define XENDISPL_OP_SET_CONFIG 4 >> +#define XENDISPL_OP_PG_FLIP 5 >> + >> +/* >> + ****************************************************************************** >> + * EVENT CODES >> + ****************************************************************************** >> + */ >> +#define XENDISPL_EVT_PG_FLIP 0 >> + >> +/* >> + ****************************************************************************** >> + * XENSTORE FIELD AND PATH NAME STRINGS, HELPERS >> + ****************************************************************************** >> + */ >> +#define XENDISPL_DRIVER_NAME "vdispl" >> + >> +#define XENDISPL_LIST_SEPARATOR "," >> +#define XENDISPL_RESOLUTION_SEPARATOR "x" >> +/* Field names */ >> +#define XENDISPL_FIELD_BE_VERSIONS "versions" >> +#define XENDISPL_FIELD_FE_VERSION "version" >> +#define XENDISPL_FIELD_FEATURES "features" >> +#define XENDISPL_FIELD_CTRL_RING_REF "ctrl-ring-ref" >> +#define XENDISPL_FIELD_CTRL_CHANNEL "ctrl-channel" >> +#define XENDISPL_FIELD_EVT_RING_REF "event-ring-ref" >> +#define XENDISPL_FIELD_EVT_CHANNEL "event-channel" >> +#define XENDISPL_FIELD_RESOLUTION "resolution" >> + >> +#define XENDISPL_FEATURE_BE_ALLOC "be_alloc" >> + >> +/* >> + ****************************************************************************** >> + * STATUS RETURN CODES >> + ****************************************************************************** >> + * >> + * Status return code is zero on success and -XEN_EXX on failure. >> + * >> + ****************************************************************************** >> + * Assumptions >> + ****************************************************************************** >> + * o usage of grant reference 0 as invalid grant reference: >> + * grant reference 0 is valid, but never exposed to a PV driver, >> + * because of the fact it is already in use/reserved by the PV console. >> + * o all references in this document to page sizes must be treated >> + * as pages of size XEN_PAGE_SIZE unless otherwise noted. >> + * >> + ****************************************************************************** >> + * Description of the protocol between frontend and backend driver >> + ****************************************************************************** >> + * >> + * The two halves of a Para-virtual display driver communicate with >> + * each other using shared pages and event channels. >> + * Shared page contains a ring with request/response packets. >> + * >> + * All reserved fields in the structures below must be 0. >> + * Display buffers's cookie of value 0 treated as invalid. >> + * Framebuffer's cookie of value 0 treated as invalid. >> + * >> + *---------------------------------- Requests --------------------------------- >> + * >> + * All requests/responses, which are not connector specific, must be sent over >> + * control ring of the connector with index 0. >> + * >> + * For all request/response/event packets that use cookies: >> + * dbuf_cookie - uint64_t, unique to guest domain value used by the backend >> + * to map remote display buffer to its local one >> + * fb_cookie - uint64_t, unique to guest domain value used by the backend >> + * to map remote framebuffer to its local one >> + * >> + * All request packets have the same length (64 octets) >> + * All request packets have common header: >> + * 0 1 2 3 octet >> + * +----------------+----------------+----------------+----------------+ >> + * | id | operation | reserved | 4 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 8 >> + * +----------------+----------------+----------------+----------------+ >> + * id - uint16_t, private guest value, echoed in response >> + * operation - uint8_t, operation code, XENDISPL_OP_??? >> + * >> + * Request dbuf creation - request creation of a display buffer. >> + * 0 1 2 3 octet >> + * +----------------+----------------+----------------+----------------+ >> + * | id |_OP_DBUF_CREATE | reserved | 4 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 8 >> + * +----------------+----------------+----------------+----------------+ >> + * | dbuf_cookie low 32-bit | 12 >> + * +----------------+----------------+----------------+----------------+ >> + * | dbuf_cookie high 32-bit | 16 >> + * +----------------+----------------+----------------+----------------+ >> + * | width | 20 >> + * +----------------+----------------+----------------+----------------+ >> + * | height | 24 >> + * +----------------+----------------+----------------+----------------+ >> + * | bpp | 28 >> + * +----------------+----------------+----------------+----------------+ >> + * | buffer_sz | 32 >> + * +----------------+----------------+----------------+----------------+ >> + * | flags | 36 >> + * +----------------+----------------+----------------+----------------+ >> + * | gref_directory | 40 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 44 >> + * +----------------+----------------+----------------+----------------+ >> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 64 >> + * +----------------+----------------+----------------+----------------+ >> + * >> + * Must be sent over control ring of the connector with index 0. >> + * >> + * width - uint32_t, width in pixels >> + * height - uint32_t, height in pixels > Why? Isn't that in the 'resolution' XenBus entry? It can be bigger that the resolution, for example if scaling will take place. > > If not, why even have the 'resolution' field? The confusion comes from the fact that the resolution is visible area: will state that clearly. > And also - what if the width or height are bigger than 'resolution'? > Should -EINVAL be returned? > no problem here: we can have buffers bigger than visible area >> + * bpp - uint32_t, bits per pixel >> + * buffer_sz - uint32_t, buffer size to be allocated, octets >> + * flags - uint32_t, flags of the operation >> + * o XENDISPL_DBUF_FLG_REQ_ALLOC - if set, then backend is requested >> + * to allocate the buffer with the parameters provided in this request. >> + * Page directory is handled as follows: >> + * Frontend on request: >> + * o allocates pages for the directory >> + * o grants permissions for the pages of the directory >> + * o sets gref_dir_next_page fields >> + * Backend on response: >> + * o grants permissions for the pages of the buffer allocated > <scratches his head> >> + * o fills in page directory with grant references > The backend provides the pages instead of the frontend? But you said > 'allocated pages for the directory' in the frontned part - and since you > say 'pages' that means more than one? Or did you mean 'the gref_directory' > page and the 'gref_dir_next_page' pages? You may want to be explicit. > sure, I will state it clearly that only directory pages are allocated: gref_directory + gref_dir_next_page(s) > And what if the 'flags' is zero? it would mean that even that the backend can allocate buffers for us we don't want it to do so, we will allocate on our own: flag is not set > Or 0x314? Perhaps also mention > the default one (0 I presume) which would make the backend map > the frontend pages provided in 'gref_directory'? yes, I'll put a note on defaults > >> + * gref_directory - grant_ref_t, a reference to the first shared page >> + * describing shared buffer references. At least one page exists. If shared >> + * buffer size (buffer_sz) exceeds what can be addressed by this single page, > ^- extra space done >> + * then reference to the next page must be supplied (see gref_dir_next_page >> + * below) >> + */ > > What are some of the return errors ? Would it make sense to mention > some? not sure, this way will dictate implementation details, but we are defining protocol > > What to for example if there are multiple _OP_DBUF_CREATE > with the same dbuf_cookie but with different width/height? it will be an error, I will clearly state this: * An attempt to create multiple display buffers with the same dbuf_cookie is * an error. dbuf_cookie can be re-used after destroying the corresponding * display buffer. >> + >> +#define XENDISPL_DBUF_FLG_REQ_ALLOC 0x0001 >> + >> +struct xendispl_dbuf_create_req { >> + uint64_t dbuf_cookie; >> + uint32_t width; >> + uint32_t height; >> + uint32_t bpp; >> + uint32_t buffer_sz; >> + uint32_t flags; >> + grant_ref_t gref_directory; >> +}; >> + >> +/* >> + * Shared page for XENDISPL_OP_DBUF_CREATE 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 | 4 >> + * +----------------+----------------+----------------+----------------+ >> + * | gref[0] | 8 >> + * +----------------+----------------+----------------+----------------+ >> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >> + * +----------------+----------------+----------------+----------------+ >> + * | gref[i] | i*4+8 >> + * +----------------+----------------+----------------+----------------+ >> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >> + * +----------------+----------------+----------------+----------------+ >> + * | gref[N - 1] | N*4+8 >> + * +----------------+----------------+----------------+----------------+ >> + * >> + * gref_dir_next_page - grant_ref_t, reference to the next page describing >> + * page directory. Must be 0 if there are no more pages in the list. >> + * gref[i] - grant_ref_t, reference to a shared page of the buffer >> + * allocated at XENDISPL_OP_DBUF_CREATE >> + * >> + * Number of grant_ref_t entries in the whole page directory is not >> + * passed, but instead can be calculated as: >> + * num_grefs_total = (XENDISPL_OP_DBUF_CREATE.buffer_sz + XEN_PAGE_SIZE - 1) / >> + * XEN_PAGE_SIZE >> + */ >> + >> +struct xendispl_page_directory { >> + grant_ref_t gref_dir_next_page; >> + grant_ref_t gref[1]; /* Variable length */ >> +}; >> + >> +/* >> + * Request dbuf destruction - destroy a previously allocated display buffer: >> + * 0 1 2 3 octet >> + * +----------------+----------------+----------------+----------------+ >> + * | id |_OP_DBUF_DESTROY| reserved | 4 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 8 >> + * +----------------+----------------+----------------+----------------+ >> + * | dbuf_cookie low 32-bit | 12 >> + * +----------------+----------------+----------------+----------------+ >> + * | dbuf_cookie high 32-bit | 16 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 20 >> + * +----------------+----------------+----------------+----------------+ >> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 64 >> + * +----------------+----------------+----------------+----------------+ >> + * >> + * Must be sent over control ring of the connector with index 0. >> + */ >> + >> +struct xendispl_dbuf_destroy_req { >> + uint64_t dbuf_cookie; >> +}; >> + >> +/* >> + * Request framebuffer attachment - request attachment of a framebuffer to >> + * previously created display buffer. >> + * 0 1 2 3 octet >> + * +----------------+----------------+----------------+----------------+ >> + * | id | _OP_FB_ATTACH | reserved | 4 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 8 >> + * +----------------+----------------+----------------+----------------+ >> + * | dbuf_cookie low 32-bit | 12 >> + * +----------------+----------------+----------------+----------------+ >> + * | dbuf_cookie high 32-bit | 16 >> + * +----------------+----------------+----------------+----------------+ >> + * | fb_cookie low 32-bit | 20 >> + * +----------------+----------------+----------------+----------------+ >> + * | fb_cookie high 32-bit | 24 >> + * +----------------+----------------+----------------+----------------+ >> + * | width | 28 >> + * +----------------+----------------+----------------+----------------+ >> + * | height | 32 >> + * +----------------+----------------+----------------+----------------+ >> + * | pixel_format | 36 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 40 >> + * +----------------+----------------+----------------+----------------+ >> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 64 >> + * +----------------+----------------+----------------+----------------+ >> + * >> + * Must be sent over control ring of the connector with index 0. >> + * >> + * width - uint32_t, width in pixels >> + * height - uint32_t, height in pixels > Why? The _OP_DBUF_CREATE already specified the height/width. Why have this? > > Or are you thinking to have an smaller (or bigger?) resolution framebuffer > than the display? I suppose you can do that which would imply that the display > would switch modes? Perhaps mention this? > > Also what if these values are bigger (or lesser) than 'resolution' ? > > >> + * pixel_format - uint32_t, pixel format of the framebuffer, FOURCC code > You did not mention the fb_cookie? I have a common section: * For all request/response/event packets that use cookies: * dbuf_cookie - uint64_t, unique to guest domain value used by the backend * to map remote display buffer to its local one * fb_cookie - uint64_t, unique to guest domain value used by the backend * to map remote framebuffer to its local one But it is in the "Requests" section - I'll pull it out to be common for all >> + */ >> + >> +struct xendispl_fb_attach_req { >> + uint64_t dbuf_cookie; >> + uint64_t fb_cookie; >> + uint32_t width; >> + uint32_t height; >> + uint32_t pixel_format; >> +}; >> + >> +/* >> + * Request framebuffer detach - detach a previously >> + * attached framebuffer from the display buffer in request: >> + * 0 1 2 3 octet >> + * +----------------+----------------+----------------+----------------+ >> + * | id | _OP_FB_DETACH | reserved | 4 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 8 >> + * +----------------+----------------+----------------+----------------+ >> + * | fb_cookie low 32-bit | 12 >> + * +----------------+----------------+----------------+----------------+ >> + * | fb_cookie high 32-bit | 16 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 20 >> + * +----------------+----------------+----------------+----------------+ >> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 64 >> + * +----------------+----------------+----------------+----------------+ >> + * >> + * Must be sent over control ring of the connector with index 0. >> + */ >> + >> +struct xendispl_fb_detach_req { >> + uint64_t fb_cookie; >> +}; >> + >> +/* >> + * Request configuration set/reset - request to set or reset >> + * the configuration/mode of the display: >> + * 0 1 2 3 octet >> + * +----------------+----------------+----------------+----------------+ >> + * | id | _OP_SET_CONFIG | reserved | 4 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 8 >> + * +----------------+----------------+----------------+----------------+ >> + * | fb_cookie low 32-bit | 12 >> + * +----------------+----------------+----------------+----------------+ >> + * | fb_cookie high 32-bit | 16 >> + * +----------------+----------------+----------------+----------------+ >> + * | x | 20 >> + * +----------------+----------------+----------------+----------------+ >> + * | y | 24 >> + * +----------------+----------------+----------------+----------------+ >> + * | width | 28 >> + * +----------------+----------------+----------------+----------------+ >> + * | height | 32 >> + * +----------------+----------------+----------------+----------------+ >> + * | bpp | 40 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 44 >> + * +----------------+----------------+----------------+----------------+ >> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 64 >> + * +----------------+----------------+----------------+----------------+ >> + * >> + * Pass all zeros to reset, otherwise command is treated as >> + * configuration set. > All zeros? Including fb_cookie? Yes, fb_cookie is 0 when we disable the display, as at this point there could be no fb at all (all released by that time) > >> + * If this is a set configuration request then framebuffer's cookie tells > Not sure if the first part of the sentence is needed. This is after all > the section that talks about setting configurations. ok, will remove it > >> + * the display which framebuffer/dbuf must be displayed while enabling display >> + * (applying configuration). > Can you disable an display without changing this? You pass ALL zeros, including fb_cookie to disable/reset. >> + * >> + * x - uint32_t, starting position in pixels by X axis >> + * y - uint32_t, starting position in pixels by Y axis > Which is bound by 'resolution' ? Or by the display command? these are limited by the "resolution" - will state that: * x, y, width and height are bound by the connector resolution and must not * exceed it. > >> + * width - uint32_t, width in pixels >> + * height - uint32_t, height in pixels >> + * bpp - uint32_t, bits per pixel > Shouldn't there be an stride as well? no, stride is kind of implementation detail which shouldn't be in the protocol >> + */ >> + >> +struct xendispl_set_config_req { >> + uint64_t fb_cookie; >> + uint32_t x; >> + uint32_t y; >> + uint32_t width; >> + uint32_t height; >> + uint32_t bpp; >> +}; >> + >> +/* >> + * Request page flip - request to flip a page identified by the framebuffer >> + * cookie: >> + * 0 1 2 3 octet >> + * +----------------+----------------+----------------+----------------+ >> + * | id | _OP_PG_FLIP | reserved | 4 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 8 >> + * +----------------+----------------+----------------+----------------+ >> + * | fb_cookie low 32-bit | 12 >> + * +----------------+----------------+----------------+----------------+ >> + * | fb_cookie high 32-bit | 16 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 20 >> + * +----------------+----------------+----------------+----------------+ >> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 64 >> + * +----------------+----------------+----------------+----------------+ >> + */ >> + >> +struct xendispl_page_flip_req { >> + uint64_t fb_cookie; >> +}; >> + >> +/* >> + *---------------------------------- Responses -------------------------------- >> + * >> + * All response packets have the same length (64 octets) >> + * >> + * All response packets have common header: >> + * 0 1 2 3 octet >> + * +----------------+----------------+----------------+----------------+ >> + * | id | reserved | 4 >> + * +----------------+----------------+----------------+----------------+ >> + * | status | 8 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 12 >> + * +----------------+----------------+----------------+----------------+ >> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 64 >> + * +----------------+----------------+----------------+----------------+ >> + * >> + * id - uint16_t, private guest value, echoed from request >> + * status - int32_t, response status, zero on success and -XEN_EXX on failure >> + * >> + *----------------------------------- Events ---------------------------------- >> + * >> + * Events are sent via a shared page allocated by the front and propagated by >> + * event-channel/event-ring-ref XenStore entries >> + * All event packets have the same length (64 octets) >> + * All event packets have common header: >> + * 0 1 2 3 octet >> + * +----------------+----------------+----------------+----------------+ >> + * | id | type | reserved | 4 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 8 >> + * +----------------+----------------+----------------+----------------+ >> + * >> + * id - uint16_t, event id, may be used by front >> + * type - uint8_t, type of the event >> + * >> + * >> + * Page flip complete event - event from back to front on page flip completed: >> + * 0 1 2 3 octet >> + * +----------------+----------------+----------------+----------------+ >> + * | id | _EVT_PG_FLIP | reserved | 4 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 8 >> + * +----------------+----------------+----------------+----------------+ >> + * | fb_cookie low 32-bit | 12 >> + * +----------------+----------------+----------------+----------------+ >> + * | fb_cookie high 32-bit | 16 >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 20 >> + * +----------------+----------------+----------------+----------------+ >> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >> + * +----------------+----------------+----------------+----------------+ >> + * | reserved | 64 >> + * +----------------+----------------+----------------+----------------+ >> + */ >> + >> +struct xendispl_pg_flip_evt { >> + uint64_t fb_cookie; >> +}; >> + >> +struct xendispl_req { >> + uint16_t id; >> + uint8_t operation; >> + uint8_t reserved[5]; >> + union { >> + struct xendispl_dbuf_create_req dbuf_create; >> + struct xendispl_dbuf_destroy_req dbuf_destroy; >> + struct xendispl_fb_attach_req fb_attach; >> + struct xendispl_fb_detach_req fb_detach; >> + struct xendispl_set_config_req set_config; >> + struct xendispl_page_flip_req pg_flip; >> + uint8_t reserved[56]; >> + } op; >> +}; >> + >> +struct xendispl_resp { >> + uint16_t id; >> + uint8_t operation; >> + uint8_t reserved; >> + int32_t status; >> + uint8_t reserved1[56]; >> +}; >> + >> +struct xendispl_evt { >> + uint16_t id; >> + uint8_t type; >> + uint8_t reserved[5]; >> + union { >> + struct xendispl_pg_flip_evt pg_flip; >> + uint8_t reserved[56]; >> + } op; >> +}; >> + >> +DEFINE_RING_TYPES(xen_displif, struct xendispl_req, struct xendispl_resp); >> + >> +/* >> + ****************************************************************************** >> + * Back to front events delivery >> + ****************************************************************************** >> + * In order to deliver asynchronous events from back to front a shared page is >> + * allocated by front and its grefs propagated to back via XenStore entries >> + * (event-XXX). > ? > > You mean event-channel ? Or control-event-channel? event-channel which is used for async events from back to front, control is always used to pass req/resp. >> + * This page has a common header used by both front and back to synchronize >> + * access and control event's ring buffer, while back being a producer of the >> + * events and front being a consumer. The rest of the page after the header >> + * is used for event packets. >> + * >> + * Upon reception of an event(s) front may confirm its reception >> + * for either each event, group of events or none. >> + */ >> + >> +struct xendispl_event_page { >> + uint32_t in_cons; >> + uint32_t in_prod; >> + uint8_t reserved[56]; >> +}; >> + >> +#define XENDISPL_EVENT_PAGE_SIZE 4096 >> +#define XENDISPL_IN_RING_OFFS (sizeof(struct xendispl_event_page)) >> +#define XENDISPL_IN_RING_SIZE (XENDISPL_EVENT_PAGE_SIZE - XENDISPL_IN_RING_OFFS) >> +#define XENDISPL_IN_RING_LEN (XENDISPL_IN_RING_SIZE / sizeof(struct xendispl_evt)) >> +#define XENDISPL_IN_RING(page) \ >> + ((struct xendispl_evt *)((char *)(page) + XENDISPL_IN_RING_OFFS)) >> +#define XENDISPL_IN_RING_REF(page, idx) \ >> + (XENDISPL_IN_RING((page))[(idx) % XENDISPL_IN_RING_LEN]) > Why not use the ring.h macros? well, this is the way the same structs are defined for fbif/kbif and I am not sure what can be re-used here from ring.h as it is designed to work with req/resp, not events. > > Especially as the top of this talks about "RING_FINAL_CHECK_FOR_REQUESTS"? I can remove this >> + >> +#endif /* __XEN_PUBLIC_IO_DISPLIF_H__ */ > Could we also have this header in: sure, I will add the same to sndif as well > > /* > * Local variables: > * mode: C > * c-file-style: "BSD" > * c-basic-offset: 4 > * tab-width: 4 > * indent-tabs-mode: nil > * End: > */ > > please? >> -- >> 2.7.4 >> [1] http://marc.info/?l=xen-devel&m=142167153324052&w=4
On Mon, Feb 13, 2017 at 10:50:49AM +0200, Oleksandr Andrushchenko wrote: > Hi, Konrad! > > Thank you for reviewing this. > > On 02/10/2017 11:27 PM, Konrad Rzeszutek Wilk wrote: > > On Fri, Feb 10, 2017 at 09:29:58AM +0200, Oleksandr Andrushchenko wrote: > > > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > > > > > This is the ABI for the two halves of a para-virtualized > > > display driver. > > > > > > This protocol aims to provide a unified protocol which fits more > > > sophisticated use-cases than a framebuffer device can handle. At the > > > moment basic functionality is supported with the intention to extend: > > > o multiple dynamically allocated/destroyed framebuffers > > > o buffers of arbitrary sizes > > > o better configuration options including multiple display support > > > > > > Note: existing fbif can be used together with displif running at the > > > same time, e.g. on Linux one provides framebuffer and another DRM/KMS > > > > > > Future extensions to the existing protocol may include: > > > o allow display/connector cloning > > > o allow allocating objects other than display buffers > > > o add planes/overlays support > > > o support scaling > > > o support rotation > > > > > > ================================================== > > > Rationale for introducing this protocol instead of > > > using the existing fbif: > > > ================================================== > > > > > > 1. In/out event sizes > > > o fbif - 40 octets > > > o displif - 40 octets > > > This is only the initial version of the displif protocol > > > which means that there could be requests which will not fit > > > (WRT introducing some GPU related functionality > > > later on). In that case we cannot alter fbif sizes as we need to > > > be backward compatible an will be forced to handle those > > > apart of fbif. > > > > > > 2. Shared page > > > Displif doesn't use anything like struct xenfb_page, but > > > DEFINE_RING_TYPES(xen_displif, struct xendispl_req, struct > > > xendispl_resp) which is a better and more common way. > > > Output events use a shared page which only has in_cons and in_prod > > > and all the rest is used for incoming events. Here struct xenfb_page > > > could probably be used as is despite the fact that it only has a half > > > of a page for incoming events which is only 50 events. (consider > > > something like 60Hz display) > > > > > > 3. Amount of changes. > > > fbif only provides XENFB_TYPE_UPDATE and XENFB_TYPE_RESIZE > > > events, so it looks like it is easier to get fb support into displif > > .. would it make sense to reserve some of those values (2, 3) > > in the XENDISPL_OP_ values? So that if this happens there is a nice > > fit in there? Thought looking at the structure there is no easy > > way to 'overlay' the xenfb_out_event structure as it is missing the 'id'. > > > > I guess one can get creative. > > > > Or you could swap positions of 'id' and 'type'? And then it would fit much > > nicer? > yeap, in order no one needs to be creative, why not > explicitly define those? > Anyways, it won't be possible to simply lay the structures from > fbif on top of displif (different structure, size, workflow etc.), > what is more that would give some room to find workarounds, rather than > have well defined solution. BTW, there was an attempt [1] to update > fbdev to meet modern application needs. If we decide to add FBDEV > functionality into this protocol, then I can re-use already > proven to work solution from [1] into DISPLIF, but defining > structures and events to fit the current protocol. I was thinking that since the size of the structure is 64bytes you have enough space to jam in the old structures too. Naturally the driver would need adjustments as it offsets of where this goes are all wrong. > What do you think? > > > than vice versa. displif at the moment has 6 requests and 1 event, > > > multiple connector support, etc. > > > > > > Changes since v2: > > > * updated XenStore configuration template/pattern > > > * added "Recovery flow" to state diagram description > > > * renamed gref_directory_start to gref_directory > > > * added missing "versions" and "version" string constants > > > > > > Changes since v1: > > > * fixed xendispl_event_page padding size > > > * added versioning support > > > * explicitly define value types for XenStore fields > > > * text decoration re-work > > > * added offsets to ASCII box notation > > > > > > Changes since initial: > > > * DRM changed to DISPL, protocol made generic > > > * major re-work addressing issues raised for sndif > > > > > > Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> > > > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > > --- > > > xen/include/public/io/displif.h | 778 ++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 778 insertions(+) > > > create mode 100644 xen/include/public/io/displif.h > > > > > > diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h > > > new file mode 100644 > > > index 000000000000..849f27fe5f1d > > > --- /dev/null > > > +++ b/xen/include/public/io/displif.h > > > @@ -0,0 +1,778 @@ > > > +/****************************************************************************** > > > + * displif.h > > > + * > > > + * Unified display 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) 2016-2017 EPAM Systems Inc. > > > + * > > > + * Authors: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > > + * Oleksandr Grytsov <oleksandr_grytsov@epam.com> > > > + */ > > > + > > > +#ifndef __XEN_PUBLIC_IO_DISPLIF_H__ > > > +#define __XEN_PUBLIC_IO_DISPLIF_H__ > > > + > > > +#include "ring.h" > > > +#include "../grant_table.h" > > > + > > > +/* > > > + ****************************************************************************** > > > + * Main features provided by the protocol > > > + ****************************************************************************** > > > + * This protocol aims to provide a unified protocol which fits more > > > + * sophisticated use-cases than a framebuffer device can handle. At the > > > + * moment basic functionality is supported with the intention to be extended: > > > + * o multiple dynamically allocated/destroyed framebuffers > > > + * o buffers of arbitrary sizes > > > + * o buffer allocation at either back or front end > > > + * o better configuration options including multiple display support > > > + * > > > + * Note: existing fbif can be used together with displif running at the > > > + * same time, e.g. on Linux one provides framebuffer and another DRM/KMS > > > + * > > > + ****************************************************************************** > > > + * Direction of improvements > > > + ****************************************************************************** > > > + * Future extensions to the existing protocol may include: > > > + * o display/connector cloning > > > + * o allocation of objects other than display buffers > > > + * o plane/overlay support > > > + * o scaling support > > > + * o rotation support > > > + * > > > + ****************************************************************************** > > > + * Feature and Parameter Negotiation > > > + ****************************************************************************** > > > + * > > > + * Front->back notifications: when enqueuing a new request, sending a > > > + * notification can be made conditional on xendispl_req (i.e., the generic > > > + * hold-off mechanism provided by the ring macros). Backends must set > > > + * xendispl_req 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 xendispl_resp (i.e., the generic > > > + * hold-off mechanism provided by the ring macros). Frontends must set > > > + * xendispl_resp appropriately (e.g., using RING_FINAL_CHECK_FOR_RESPONSES()). > > > + * > > > + * The two halves of a para-virtual display driver utilize nodes within > > > + * XenStore to communicate capabilities and to negotiate operating parameters. > > > + * This section enumerates these nodes which reside in the respective front and > > > + * backend portions of XenStore, following the XenBus convention. > > > + * > > > + * All data in 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. > > > + * > > > + ****************************************************************************** > > > + * Example configuration > > > + ****************************************************************************** > > > + * > > > + * Note: depending on the use-case backend can expose more display connectors > > > + * than the underlying HW physically has by employing SW graphics compositors > > > + * > > > + * This is an example of backend and frontend configuration: > > > + * > > > + *--------------------------------- Backend ----------------------------------- > > > + * > > > + * /local/domain/0/backend/vdispl/1/0/frontend-id = "1" > > > + * /local/domain/0/backend/vdispl/1/0/frontend = "/local/domain/1/device/vdispl/0" > > > + * /local/domain/0/backend/vdispl/1/0/state = "4" > > > + * /local/domain/0/backend/vdispl/1/0/versions = "1,2" > > > + * /local/domain/0/backend/vdispl/1/0/features = "be_alloc" > > > + * > > > + *--------------------------------- Frontend ---------------------------------- > > > + * > > > + * /local/domain/1/device/vdispl/0/backend-id = "0" > > > + * /local/domain/1/device/vdispl/0/backend = "/local/domain/0/backend/vdispl/1/0" > > > + * /local/domain/1/device/vdispl/0/state = "4" > > > + * /local/domain/1/device/vdispl/0/version = "1" > > > + * > > > + *-------------------------- Connector 0 configuration ------------------------ > > > + * > > > + * /local/domain/1/device/vdispl/0/0/resolution = "1920x1080" > > > + * /local/domain/1/device/vdispl/0/0/ctrl-ring-ref = "2832" > > > + * /local/domain/1/device/vdispl/0/0/ctrl-channel = "15" > > > + * /local/domain/1/device/vdispl/0/0/event-ring-ref = "387" > > > + * /local/domain/1/device/vdispl/0/0/event-channel = "16" > > > + * > > > + *-------------------------- Connector 1 configuration ------------------------ > > > + * > > > + * /local/domain/1/device/vdispl/0/1/resolution = "800x600" > > > + * /local/domain/1/device/vdispl/0/1/ctrl-ring-ref = "2833" > > > + * /local/domain/1/device/vdispl/0/1/ctrl-channel = "17" > > > + * /local/domain/1/device/vdispl/0/1/event-ring-ref = "388" > > > + * /local/domain/1/device/vdispl/0/1/event-channel = "18" > > > + * > > > + ****************************************************************************** > > > + * Backend XenBus Nodes > > > + ****************************************************************************** > > > + * > > > + *----------------------------- Protocol version ------------------------------ > > > + * > > > + * versions > > > + * Values: <string> > > > + * > > > + * List of XENDISPL_LIST_SEPARATOR separated protocol versions supported > > > + * by the backend. For example "1,2,3". > > > + * > > > + * features > > > + * Values: <list of strings> > > > + * > > > + * XENDISPL_LIST_SEPARATOR separated list of features > > > + * XENDISPL_FEATURE_BE_??? that the backend supports. > > > + * > > > + * be_alloc > > > + * Backend can be a buffer provider/allocator during > > > + * XENDISPL_OP_DBUF_CREATE operation (see below for negotiation). > > > + * > > > + ****************************************************************************** > > > + * Frontend XenBus Nodes > > > + ****************************************************************************** > > > + * > > > + *-------------------------------- Addressing --------------------------------- > > > + * > > > + * dom-id > > > + * Values: <uint16_t> > > > + * > > > + * Domain identifier. > > > + * > > > + * dev-id > > > + * Values: <uint16_t> > > > + * > > > + * Device identifier. > > > + * > > > + * conn-idx > > > + * Values: <uint8_t> > > > + * > > > + * Zero based contigous index of the connector. > > > + * /local/domain/<dom-id>/device/vdispl/<dev-id>/<conn-idx>/... > > > + * > > > + *----------------------------- Protocol version ------------------------------ > > > + * > > > + * version > > > + * Values: <string> > > > + * > > > + * Protocol version, chosen among the ones supported by the backend. > > > + * > > > + *----------------------------- Connector settings ---------------------------- > > > + * > > > + * resolution > > > + * Values: <width, uint32_t>x<height, uint32_t> > > > + * > > > + * Width and height of the connector in pixels separated by > > > + * XENDISPL_RESOLUTION_SEPARATOR. > > Is there an default depth? > no > > 32? > no defaults Should we have a default? And perhaps also say that all the operations are bounded by these? > > Or should that also be negotiated? > frontend requests the desired value of it, backend provides it > or rejects the request, so front will try other value. > So, yes, this is kind of negotiation here (EINVAL based :) > > I will also put a note that "resolution" only defines visible area, > thus display and frame buffer sizes can be different from this value OK, which I suspect means that it can only be smaller, not larger. > > > + * > > > + *------------------ Connector Request Transport Parameters ------------------- > > > + * > > > + * ctrl-channel > > Please call it 'ctrl-event-channel' > I will > > > + * Values: <uint32_t> > > > + * > > > + * The identifier of the Xen connector's control event channel > > > + * used to signal activity in the ring buffer. > > > + * > > > + * ctrl-ring-ref > > > + * Values: <uint32_t> > > > + * > > > + * The Xen grant reference granting permission for the backend to map > > > + * a sole page in a single page sized connector's control ring buffer. > > > + * > > > + * event-channel > > > + * Values: <uint32_t> > > > + * > > > + * The identifier of the Xen connector's event channel > > > + * used to signal activity in the ring buffer. > > > + * > > > + * event-ring-ref > > > + * Values: <uint32_t> > > > + * > > > + * The Xen grant reference granting permission for the backend to map > > > + * a sole page in a single page sized connector's event ring buffer. > > So you are making two rings and two event channels - and it looks like the > > operations are going via the control ring.. but what goes through the > > the other ring buffer? > Events from backend to frontend, currently only XENDISPL_EVT_PG_FLIP Aaaaah! In that case I would change it from 'ctrl-' to 'req-alloc' to match with. > I will define 2 sections: > *------------------ Connector Request Transport Parameters > ------------------- > * > * ctrl-event-channel > * ctrl-ring-ref > * > *------------------- Connector Event Transport Parameters > -------------------- > * > * event-channel > * event-ring-ref > > > Or is the other ring buffer the one that is created via 'gref_directory' ? > no > At the bottom: > * In order to deliver asynchronous events from back to front a shared page > is > * allocated by front and its gref propagated to back via XenStore entries > * (event-XXX). AAnd you may want to say this is guarded by REQ_ALLOC feature right? > > > If so, wouldn't you need mulitple event channels? And also the size of those > > rings is much much bigger. But it is not really a ring. It looks to hold > > some opaque data? > > > > -ECONFUSED. > > > + */ > > > + > > > +/* > > > + ****************************************************************************** > > > + * STATE DIAGRAMS > > > + ****************************************************************************** > > > + * > > > + * Tool stack creates front and back state nodes with initial state > > > + * XenbusStateInitialising. > > > + * Tool stack creates and sets up frontend display configuration > > > + * nodes per domain. > > > + * > > > + *-------------------------------- Normal flow -------------------------------- > > > + * > > > + * 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 > > > + * connector. > > > + * 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 display connectors > > > + * as per configuration. > > > + * | > > > + * | > > > + * V > > > + * XenbusStateConnected > > > + * > > > + * XenbusStateUnknown > > > + * XenbusStateClosed > > > + * XenbusStateClosing > > > + * o Remove virtual display device > > > + * o Remove event channels > > > + * | > > > + * | > > > + * V > > > + * XenbusStateClosed > > > + * > > > + *------------------------------- Recovery flow ------------------------------- > > > + * > > > + * In case of frontend unrecoverable errors backend handles that as > > > + * if frontend goes into the XenbusStateClosed state. > > > + * > > > + * In case of backend unrecoverable errors frontend tries removing > > > + * the emulated device. If this is possible at the moment of error, > > What emulated device? > s/emulated/virtualized/g for all? /me nods. > > > > > + * then frontend goes into the XenbusStateInitialising state and is ready for > > > + * new connection with backend. If the emulated device is still in use and > > emulated device? > > > > > + * cannot be removed, then frontend goes into the XenbusStateReconfiguring state > > .. and what is done in XenbusStateReconfiguring state? > > > > I am assuming 'resolution' is examined? > > > > > + * until either the emulated device removed or backend initiates a new > > > + * connection. On the emulated device removal frontend goes into the > > emulated device? > The whole idea here is to let frontend recover from backend failure. > What happens when backend dies: frontend cannot send requests > to the backend and thus cannot emulate (virtualize) display > device anymore. There is no simple solution like disconnecting rings > and waiting for the backend to re-connect, as display device, just like > sound devices, does hold some state: configuration in use, number of display > buffers, framebuffers, application state, framework "watchdogs" (like > vertical blank time-outs and recovery at framework level) etc. > So, in most cases, it would require frontend to implement complex recovery > logic. Instead, what I suggest, that we go into some state, which tells that > we are not yet ready to go into Initialized state and still need some time: > at this time frontend will try to make sure no new clients from user-space > are accepted, it allows the current client to exit gracefully by returning > error codes etc. Once all the clients are gone frontend can reinitialize > the virtualized device, e.g. DRM/KMS driver, and get into Initialized state > again. That is fine. > Thus, I was thinking of XenbusStateReconfiguringstate as appropriate in this > case Right, but somebody has to move to this state. Who would do it? Would the frontend have some form of timer to make sure that the backend is still alive? And if it had died then move to Reconfiguring? > > > + * XenbusStateInitialising state. > > > + * > > > + ****************************************************************************** > > > + * REQUEST CODES > > > + ****************************************************************************** > > > + */ > > > +#define XENDISPL_OP_DBUF_CREATE 0 > > > +#define XENDISPL_OP_DBUF_DESTROY 1 > > > +#define XENDISPL_OP_FB_ATTACH 2 > > > +#define XENDISPL_OP_FB_DETACH 3 > > > +#define XENDISPL_OP_SET_CONFIG 4 > > > +#define XENDISPL_OP_PG_FLIP 5 > > > + > > > +/* > > > + ****************************************************************************** > > > + * EVENT CODES > > > + ****************************************************************************** > > > + */ > > > +#define XENDISPL_EVT_PG_FLIP 0 > > > + > > > +/* > > > + ****************************************************************************** > > > + * XENSTORE FIELD AND PATH NAME STRINGS, HELPERS > > > + ****************************************************************************** > > > + */ > > > +#define XENDISPL_DRIVER_NAME "vdispl" > > > + > > > +#define XENDISPL_LIST_SEPARATOR "," > > > +#define XENDISPL_RESOLUTION_SEPARATOR "x" > > > +/* Field names */ > > > +#define XENDISPL_FIELD_BE_VERSIONS "versions" > > > +#define XENDISPL_FIELD_FE_VERSION "version" > > > +#define XENDISPL_FIELD_FEATURES "features" > > > +#define XENDISPL_FIELD_CTRL_RING_REF "ctrl-ring-ref" > > > +#define XENDISPL_FIELD_CTRL_CHANNEL "ctrl-channel" > > > +#define XENDISPL_FIELD_EVT_RING_REF "event-ring-ref" > > > +#define XENDISPL_FIELD_EVT_CHANNEL "event-channel" > > > +#define XENDISPL_FIELD_RESOLUTION "resolution" > > > + > > > +#define XENDISPL_FEATURE_BE_ALLOC "be_alloc" > > > + > > > +/* > > > + ****************************************************************************** > > > + * STATUS RETURN CODES > > > + ****************************************************************************** > > > + * > > > + * Status return code is zero on success and -XEN_EXX on failure. > > > + * > > > + ****************************************************************************** > > > + * Assumptions > > > + ****************************************************************************** > > > + * o usage of grant reference 0 as invalid grant reference: > > > + * grant reference 0 is valid, but never exposed to a PV driver, > > > + * because of the fact it is already in use/reserved by the PV console. > > > + * o all references in this document to page sizes must be treated > > > + * as pages of size XEN_PAGE_SIZE unless otherwise noted. > > > + * > > > + ****************************************************************************** > > > + * Description of the protocol between frontend and backend driver > > > + ****************************************************************************** > > > + * > > > + * The two halves of a Para-virtual display driver communicate with > > > + * each other using shared pages and event channels. > > > + * Shared page contains a ring with request/response packets. > > > + * > > > + * All reserved fields in the structures below must be 0. > > > + * Display buffers's cookie of value 0 treated as invalid. > > > + * Framebuffer's cookie of value 0 treated as invalid. > > > + * > > > + *---------------------------------- Requests --------------------------------- > > > + * > > > + * All requests/responses, which are not connector specific, must be sent over > > > + * control ring of the connector with index 0. > > > + * > > > + * For all request/response/event packets that use cookies: > > > + * dbuf_cookie - uint64_t, unique to guest domain value used by the backend > > > + * to map remote display buffer to its local one > > > + * fb_cookie - uint64_t, unique to guest domain value used by the backend > > > + * to map remote framebuffer to its local one > > > + * > > > + * All request packets have the same length (64 octets) > > > + * All request packets have common header: > > > + * 0 1 2 3 octet > > > + * +----------------+----------------+----------------+----------------+ > > > + * | id | operation | reserved | 4 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 8 > > > + * +----------------+----------------+----------------+----------------+ > > > + * id - uint16_t, private guest value, echoed in response > > > + * operation - uint8_t, operation code, XENDISPL_OP_??? > > > + * > > > + * Request dbuf creation - request creation of a display buffer. > > > + * 0 1 2 3 octet > > > + * +----------------+----------------+----------------+----------------+ > > > + * | id |_OP_DBUF_CREATE | reserved | 4 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 8 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | dbuf_cookie low 32-bit | 12 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | dbuf_cookie high 32-bit | 16 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | width | 20 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | height | 24 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | bpp | 28 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | buffer_sz | 32 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | flags | 36 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | gref_directory | 40 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 44 > > > + * +----------------+----------------+----------------+----------------+ > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 64 > > > + * +----------------+----------------+----------------+----------------+ > > > + * > > > + * Must be sent over control ring of the connector with index 0. > > > + * > > > + * width - uint32_t, width in pixels > > > + * height - uint32_t, height in pixels > > Why? Isn't that in the 'resolution' XenBus entry? > It can be bigger that the resolution, for example > if scaling will take place. How would you reflect the scaling part of this? Is that something that is assumed if the XenBus resolution value < widthxheight values? > > > > If not, why even have the 'resolution' field? > The confusion comes from the fact that the resolution is visible area: > will state that clearly. > > And also - what if the width or height are bigger than 'resolution'? > > Should -EINVAL be returned? > > > no problem here: we can have buffers bigger than visible area > > > + * bpp - uint32_t, bits per pixel > > > + * buffer_sz - uint32_t, buffer size to be allocated, octets > > > + * flags - uint32_t, flags of the operation > > > + * o XENDISPL_DBUF_FLG_REQ_ALLOC - if set, then backend is requested > > > + * to allocate the buffer with the parameters provided in this request. > > > + * Page directory is handled as follows: > > > + * Frontend on request: > > > + * o allocates pages for the directory > > > + * o grants permissions for the pages of the directory > > > + * o sets gref_dir_next_page fields > > > + * Backend on response: > > > + * o grants permissions for the pages of the buffer allocated > > <scratches his head> > > > + * o fills in page directory with grant references > > The backend provides the pages instead of the frontend? But you said > > 'allocated pages for the directory' in the frontned part - and since you > > say 'pages' that means more than one? Or did you mean 'the gref_directory' > > page and the 'gref_dir_next_page' pages? You may want to be explicit. > > > sure, I will state it clearly that only directory pages > are allocated: gref_directory + gref_dir_next_page(s) Thx > > And what if the 'flags' is zero? > it would mean that even that the backend can allocate buffers > for us we don't want it to do so, we will allocate on our own: > flag is not set > > Or 0x314? Perhaps also mention > > the default one (0 I presume) which would make the backend map > > the frontend pages provided in 'gref_directory'? > yes, I'll put a note on defaults Awesome! > > > > > + * gref_directory - grant_ref_t, a reference to the first shared page > > > + * describing shared buffer references. At least one page exists. If shared > > > + * buffer size (buffer_sz) exceeds what can be addressed by this single page, > > ^- extra space > done > > > + * then reference to the next page must be supplied (see gref_dir_next_page > > > + * below) > > > + */ > > > > What are some of the return errors ? Would it make sense to mention > > some? > not sure, this way will dictate implementation details, > but we are defining protocol True to an extent. > > > > What to for example if there are multiple _OP_DBUF_CREATE > > with the same dbuf_cookie but with different width/height? > it will be an error, I will clearly state this: > * An attempt to create multiple display buffers with the same dbuf_cookie > is > * an error. dbuf_cookie can be re-used after destroying the corresponding > * display buffer. May also want to say what error value it should be. -EAGAIN? No probably something different. > > > > + > > > +#define XENDISPL_DBUF_FLG_REQ_ALLOC 0x0001 > > > + > > > +struct xendispl_dbuf_create_req { > > > + uint64_t dbuf_cookie; > > > + uint32_t width; > > > + uint32_t height; > > > + uint32_t bpp; > > > + uint32_t buffer_sz; > > > + uint32_t flags; > > > + grant_ref_t gref_directory; > > > +}; > > > + > > > +/* > > > + * Shared page for XENDISPL_OP_DBUF_CREATE 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 | 4 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | gref[0] | 8 > > > + * +----------------+----------------+----------------+----------------+ > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > + * +----------------+----------------+----------------+----------------+ > > > + * | gref[i] | i*4+8 > > > + * +----------------+----------------+----------------+----------------+ > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > + * +----------------+----------------+----------------+----------------+ > > > + * | gref[N - 1] | N*4+8 > > > + * +----------------+----------------+----------------+----------------+ > > > + * > > > + * gref_dir_next_page - grant_ref_t, reference to the next page describing > > > + * page directory. Must be 0 if there are no more pages in the list. > > > + * gref[i] - grant_ref_t, reference to a shared page of the buffer > > > + * allocated at XENDISPL_OP_DBUF_CREATE > > > + * > > > + * Number of grant_ref_t entries in the whole page directory is not > > > + * passed, but instead can be calculated as: > > > + * num_grefs_total = (XENDISPL_OP_DBUF_CREATE.buffer_sz + XEN_PAGE_SIZE - 1) / > > > + * XEN_PAGE_SIZE > > > + */ > > > + > > > +struct xendispl_page_directory { > > > + grant_ref_t gref_dir_next_page; > > > + grant_ref_t gref[1]; /* Variable length */ > > > +}; > > > + > > > +/* > > > + * Request dbuf destruction - destroy a previously allocated display buffer: > > > + * 0 1 2 3 octet > > > + * +----------------+----------------+----------------+----------------+ > > > + * | id |_OP_DBUF_DESTROY| reserved | 4 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 8 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | dbuf_cookie low 32-bit | 12 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | dbuf_cookie high 32-bit | 16 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 20 > > > + * +----------------+----------------+----------------+----------------+ > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 64 > > > + * +----------------+----------------+----------------+----------------+ > > > + * > > > + * Must be sent over control ring of the connector with index 0. > > > + */ > > > + > > > +struct xendispl_dbuf_destroy_req { > > > + uint64_t dbuf_cookie; > > > +}; > > > + > > > +/* > > > + * Request framebuffer attachment - request attachment of a framebuffer to > > > + * previously created display buffer. > > > + * 0 1 2 3 octet > > > + * +----------------+----------------+----------------+----------------+ > > > + * | id | _OP_FB_ATTACH | reserved | 4 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 8 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | dbuf_cookie low 32-bit | 12 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | dbuf_cookie high 32-bit | 16 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | fb_cookie low 32-bit | 20 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | fb_cookie high 32-bit | 24 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | width | 28 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | height | 32 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | pixel_format | 36 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 40 > > > + * +----------------+----------------+----------------+----------------+ > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 64 > > > + * +----------------+----------------+----------------+----------------+ > > > + * > > > + * Must be sent over control ring of the connector with index 0. > > > + * > > > + * width - uint32_t, width in pixels > > > + * height - uint32_t, height in pixels > > Why? The _OP_DBUF_CREATE already specified the height/width. Why have this? > > > > Or are you thinking to have an smaller (or bigger?) resolution framebuffer > > than the display? I suppose you can do that which would imply that the display > > would switch modes? Perhaps mention this? > > > > Also what if these values are bigger (or lesser) than 'resolution' ? > > > > > > > + * pixel_format - uint32_t, pixel format of the framebuffer, FOURCC code > > You did not mention the fb_cookie? > I have a common section: > * For all request/response/event packets that use cookies: > * dbuf_cookie - uint64_t, unique to guest domain value used by the > backend > * to map remote display buffer to its local one > * fb_cookie - uint64_t, unique to guest domain value used by the backend > * to map remote framebuffer to its local one > But it is in the "Requests" section - I'll pull it out to be common for all Aaaah! > > > + */ > > > + > > > +struct xendispl_fb_attach_req { > > > + uint64_t dbuf_cookie; > > > + uint64_t fb_cookie; > > > + uint32_t width; > > > + uint32_t height; > > > + uint32_t pixel_format; > > > +}; > > > + > > > +/* > > > + * Request framebuffer detach - detach a previously > > > + * attached framebuffer from the display buffer in request: > > > + * 0 1 2 3 octet > > > + * +----------------+----------------+----------------+----------------+ > > > + * | id | _OP_FB_DETACH | reserved | 4 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 8 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | fb_cookie low 32-bit | 12 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | fb_cookie high 32-bit | 16 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 20 > > > + * +----------------+----------------+----------------+----------------+ > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 64 > > > + * +----------------+----------------+----------------+----------------+ > > > + * > > > + * Must be sent over control ring of the connector with index 0. > > > + */ > > > + > > > +struct xendispl_fb_detach_req { > > > + uint64_t fb_cookie; > > > +}; > > > + > > > +/* > > > + * Request configuration set/reset - request to set or reset > > > + * the configuration/mode of the display: > > > + * 0 1 2 3 octet > > > + * +----------------+----------------+----------------+----------------+ > > > + * | id | _OP_SET_CONFIG | reserved | 4 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 8 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | fb_cookie low 32-bit | 12 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | fb_cookie high 32-bit | 16 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | x | 20 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | y | 24 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | width | 28 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | height | 32 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | bpp | 40 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 44 > > > + * +----------------+----------------+----------------+----------------+ > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 64 > > > + * +----------------+----------------+----------------+----------------+ > > > + * > > > + * Pass all zeros to reset, otherwise command is treated as > > > + * configuration set. > > All zeros? Including fb_cookie? > Yes, fb_cookie is 0 when we disable the display, as at this point there > could be no fb at all (all released by that time) > > > > > + * If this is a set configuration request then framebuffer's cookie tells > > Not sure if the first part of the sentence is needed. This is after all > > the section that talks about setting configurations. > ok, will remove it > > > > > + * the display which framebuffer/dbuf must be displayed while enabling display > > > + * (applying configuration). > > Can you disable an display without changing this? > You pass ALL zeros, including fb_cookie to disable/reset. > > > + * > > > + * x - uint32_t, starting position in pixels by X axis > > > + * y - uint32_t, starting position in pixels by Y axis > > Which is bound by 'resolution' ? Or by the display command? > these are limited by the "resolution" - will state that: > * x, y, width and height are bound by the connector resolution and must not > * exceed it. Thanks! > > > > > > + * width - uint32_t, width in pixels > > > + * height - uint32_t, height in pixels > > > + * bpp - uint32_t, bits per pixel > > Shouldn't there be an stride as well? > no, stride is kind of implementation detail which shouldn't > be in the protocol > > > + */ > > > + > > > +struct xendispl_set_config_req { > > > + uint64_t fb_cookie; > > > + uint32_t x; > > > + uint32_t y; > > > + uint32_t width; > > > + uint32_t height; > > > + uint32_t bpp; > > > +}; > > > + > > > +/* > > > + * Request page flip - request to flip a page identified by the framebuffer > > > + * cookie: > > > + * 0 1 2 3 octet > > > + * +----------------+----------------+----------------+----------------+ > > > + * | id | _OP_PG_FLIP | reserved | 4 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 8 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | fb_cookie low 32-bit | 12 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | fb_cookie high 32-bit | 16 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 20 > > > + * +----------------+----------------+----------------+----------------+ > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 64 > > > + * +----------------+----------------+----------------+----------------+ > > > + */ > > > + > > > +struct xendispl_page_flip_req { > > > + uint64_t fb_cookie; > > > +}; > > > + > > > +/* > > > + *---------------------------------- Responses -------------------------------- > > > + * > > > + * All response packets have the same length (64 octets) > > > + * > > > + * All response packets have common header: > > > + * 0 1 2 3 octet > > > + * +----------------+----------------+----------------+----------------+ > > > + * | id | reserved | 4 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | status | 8 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 12 > > > + * +----------------+----------------+----------------+----------------+ > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 64 > > > + * +----------------+----------------+----------------+----------------+ > > > + * > > > + * id - uint16_t, private guest value, echoed from request > > > + * status - int32_t, response status, zero on success and -XEN_EXX on failure > > > + * > > > + *----------------------------------- Events ---------------------------------- > > > + * > > > + * Events are sent via a shared page allocated by the front and propagated by > > > + * event-channel/event-ring-ref XenStore entries > > > + * All event packets have the same length (64 octets) > > > + * All event packets have common header: > > > + * 0 1 2 3 octet > > > + * +----------------+----------------+----------------+----------------+ > > > + * | id | type | reserved | 4 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 8 > > > + * +----------------+----------------+----------------+----------------+ > > > + * > > > + * id - uint16_t, event id, may be used by front > > > + * type - uint8_t, type of the event > > > + * > > > + * > > > + * Page flip complete event - event from back to front on page flip completed: > > > + * 0 1 2 3 octet > > > + * +----------------+----------------+----------------+----------------+ > > > + * | id | _EVT_PG_FLIP | reserved | 4 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 8 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | fb_cookie low 32-bit | 12 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | fb_cookie high 32-bit | 16 > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 20 > > > + * +----------------+----------------+----------------+----------------+ > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > + * +----------------+----------------+----------------+----------------+ > > > + * | reserved | 64 > > > + * +----------------+----------------+----------------+----------------+ > > > + */ > > > + > > > +struct xendispl_pg_flip_evt { > > > + uint64_t fb_cookie; > > > +}; > > > + > > > +struct xendispl_req { > > > + uint16_t id; > > > + uint8_t operation; > > > + uint8_t reserved[5]; > > > + union { > > > + struct xendispl_dbuf_create_req dbuf_create; > > > + struct xendispl_dbuf_destroy_req dbuf_destroy; > > > + struct xendispl_fb_attach_req fb_attach; > > > + struct xendispl_fb_detach_req fb_detach; > > > + struct xendispl_set_config_req set_config; > > > + struct xendispl_page_flip_req pg_flip; > > > + uint8_t reserved[56]; > > > + } op; > > > +}; > > > + > > > +struct xendispl_resp { > > > + uint16_t id; > > > + uint8_t operation; > > > + uint8_t reserved; > > > + int32_t status; > > > + uint8_t reserved1[56]; > > > +}; > > > + > > > +struct xendispl_evt { > > > + uint16_t id; > > > + uint8_t type; > > > + uint8_t reserved[5]; > > > + union { > > > + struct xendispl_pg_flip_evt pg_flip; > > > + uint8_t reserved[56]; > > > + } op; > > > +}; > > > + > > > +DEFINE_RING_TYPES(xen_displif, struct xendispl_req, struct xendispl_resp); > > > + > > > +/* > > > + ****************************************************************************** > > > + * Back to front events delivery > > > + ****************************************************************************** > > > + * In order to deliver asynchronous events from back to front a shared page is > > > + * allocated by front and its grefs propagated to back via XenStore entries > > > + * (event-XXX). > > ? > > > > You mean event-channel ? Or control-event-channel? > event-channel which is used for async events from back to front, > control is always used to pass req/resp. > > > + * This page has a common header used by both front and back to synchronize > > > + * access and control event's ring buffer, while back being a producer of the > > > + * events and front being a consumer. The rest of the page after the header > > > + * is used for event packets. > > > + * > > > + * Upon reception of an event(s) front may confirm its reception > > > + * for either each event, group of events or none. > > > + */ > > > + > > > +struct xendispl_event_page { > > > + uint32_t in_cons; > > > + uint32_t in_prod; > > > + uint8_t reserved[56]; > > > +}; > > > + > > > +#define XENDISPL_EVENT_PAGE_SIZE 4096 > > > +#define XENDISPL_IN_RING_OFFS (sizeof(struct xendispl_event_page)) > > > +#define XENDISPL_IN_RING_SIZE (XENDISPL_EVENT_PAGE_SIZE - XENDISPL_IN_RING_OFFS) > > > +#define XENDISPL_IN_RING_LEN (XENDISPL_IN_RING_SIZE / sizeof(struct xendispl_evt)) > > > +#define XENDISPL_IN_RING(page) \ > > > + ((struct xendispl_evt *)((char *)(page) + XENDISPL_IN_RING_OFFS)) > > > +#define XENDISPL_IN_RING_REF(page, idx) \ > > > + (XENDISPL_IN_RING((page))[(idx) % XENDISPL_IN_RING_LEN]) > > Why not use the ring.h macros? > well, this is the way the same structs are defined for fbif/kbif > and I am not sure what can be re-used here from ring.h as it is > designed to work with req/resp, not events. > > > > Especially as the top of this talks about "RING_FINAL_CHECK_FOR_REQUESTS"? > I can remove this > > > + > > > +#endif /* __XEN_PUBLIC_IO_DISPLIF_H__ */ > > Could we also have this header in: > sure, I will add the same to sndif as well > > > > /* > > * Local variables: > > * mode: C > > * c-file-style: "BSD" > > * c-basic-offset: 4 > > * tab-width: 4 > > * indent-tabs-mode: nil > > * End: > > */ > > > > please? > > > -- > > > 2.7.4 > > > > [1] http://marc.info/?l=xen-devel&m=142167153324052&w=4 >
On 02/14/2017 10:27 PM, Konrad Rzeszutek Wilk wrote: > On Mon, Feb 13, 2017 at 10:50:49AM +0200, Oleksandr Andrushchenko wrote: >> Hi, Konrad! >> >> Thank you for reviewing this. >> >> On 02/10/2017 11:27 PM, Konrad Rzeszutek Wilk wrote: >>> On Fri, Feb 10, 2017 at 09:29:58AM +0200, Oleksandr Andrushchenko wrote: >>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>>> >>>> This is the ABI for the two halves of a para-virtualized >>>> display driver. >>>> >>>> This protocol aims to provide a unified protocol which fits more >>>> sophisticated use-cases than a framebuffer device can handle. At the >>>> moment basic functionality is supported with the intention to extend: >>>> o multiple dynamically allocated/destroyed framebuffers >>>> o buffers of arbitrary sizes >>>> o better configuration options including multiple display support >>>> >>>> Note: existing fbif can be used together with displif running at the >>>> same time, e.g. on Linux one provides framebuffer and another DRM/KMS >>>> >>>> Future extensions to the existing protocol may include: >>>> o allow display/connector cloning >>>> o allow allocating objects other than display buffers >>>> o add planes/overlays support >>>> o support scaling >>>> o support rotation >>>> >>>> ================================================== >>>> Rationale for introducing this protocol instead of >>>> using the existing fbif: >>>> ================================================== >>>> >>>> 1. In/out event sizes >>>> o fbif - 40 octets >>>> o displif - 40 octets >>>> This is only the initial version of the displif protocol >>>> which means that there could be requests which will not fit >>>> (WRT introducing some GPU related functionality >>>> later on). In that case we cannot alter fbif sizes as we need to >>>> be backward compatible an will be forced to handle those >>>> apart of fbif. >>>> >>>> 2. Shared page >>>> Displif doesn't use anything like struct xenfb_page, but >>>> DEFINE_RING_TYPES(xen_displif, struct xendispl_req, struct >>>> xendispl_resp) which is a better and more common way. >>>> Output events use a shared page which only has in_cons and in_prod >>>> and all the rest is used for incoming events. Here struct xenfb_page >>>> could probably be used as is despite the fact that it only has a half >>>> of a page for incoming events which is only 50 events. (consider >>>> something like 60Hz display) >>>> >>>> 3. Amount of changes. >>>> fbif only provides XENFB_TYPE_UPDATE and XENFB_TYPE_RESIZE >>>> events, so it looks like it is easier to get fb support into displif >>> .. would it make sense to reserve some of those values (2, 3) >>> in the XENDISPL_OP_ values? So that if this happens there is a nice >>> fit in there? Thought looking at the structure there is no easy >>> way to 'overlay' the xenfb_out_event structure as it is missing the 'id'. >>> >>> I guess one can get creative. >>> >>> Or you could swap positions of 'id' and 'type'? And then it would fit much >>> nicer? >> yeap, in order no one needs to be creative, why not >> explicitly define those? >> Anyways, it won't be possible to simply lay the structures from >> fbif on top of displif (different structure, size, workflow etc.), >> what is more that would give some room to find workarounds, rather than >> have well defined solution. BTW, there was an attempt [1] to update >> fbdev to meet modern application needs. If we decide to add FBDEV >> functionality into this protocol, then I can re-use already >> proven to work solution from [1] into DISPLIF, but defining >> structures and events to fit the current protocol. > I was thinking that since the size of the structure is 64bytes > you have enough space to jam in the old structures too. yes, I understand your intention > > Naturally the driver would need adjustments as it offsets > of where this goes are all wrong. exactly > >> What do you think? so, what is the decision? Options: 1) Leave the protocol as is, if need be it can be extended later 2) Implement something similar to [1] 3) Give room for workarounds and reserve XENDISPL_OP_XXX for what current fbif uses >>>> than vice versa. displif at the moment has 6 requests and 1 event, >>>> multiple connector support, etc. >>>> >>>> Changes since v2: >>>> * updated XenStore configuration template/pattern >>>> * added "Recovery flow" to state diagram description >>>> * renamed gref_directory_start to gref_directory >>>> * added missing "versions" and "version" string constants >>>> >>>> Changes since v1: >>>> * fixed xendispl_event_page padding size >>>> * added versioning support >>>> * explicitly define value types for XenStore fields >>>> * text decoration re-work >>>> * added offsets to ASCII box notation >>>> >>>> Changes since initial: >>>> * DRM changed to DISPL, protocol made generic >>>> * major re-work addressing issues raised for sndif >>>> >>>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> >>>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>>> --- >>>> xen/include/public/io/displif.h | 778 ++++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 778 insertions(+) >>>> create mode 100644 xen/include/public/io/displif.h >>>> >>>> diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h >>>> new file mode 100644 >>>> index 000000000000..849f27fe5f1d >>>> --- /dev/null >>>> +++ b/xen/include/public/io/displif.h >>>> @@ -0,0 +1,778 @@ >>>> +/****************************************************************************** >>>> + * displif.h >>>> + * >>>> + * Unified display 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) 2016-2017 EPAM Systems Inc. >>>> + * >>>> + * Authors: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>>> + * Oleksandr Grytsov <oleksandr_grytsov@epam.com> >>>> + */ >>>> + >>>> +#ifndef __XEN_PUBLIC_IO_DISPLIF_H__ >>>> +#define __XEN_PUBLIC_IO_DISPLIF_H__ >>>> + >>>> +#include "ring.h" >>>> +#include "../grant_table.h" >>>> + >>>> +/* >>>> + ****************************************************************************** >>>> + * Main features provided by the protocol >>>> + ****************************************************************************** >>>> + * This protocol aims to provide a unified protocol which fits more >>>> + * sophisticated use-cases than a framebuffer device can handle. At the >>>> + * moment basic functionality is supported with the intention to be extended: >>>> + * o multiple dynamically allocated/destroyed framebuffers >>>> + * o buffers of arbitrary sizes >>>> + * o buffer allocation at either back or front end >>>> + * o better configuration options including multiple display support >>>> + * >>>> + * Note: existing fbif can be used together with displif running at the >>>> + * same time, e.g. on Linux one provides framebuffer and another DRM/KMS >>>> + * >>>> + ****************************************************************************** >>>> + * Direction of improvements >>>> + ****************************************************************************** >>>> + * Future extensions to the existing protocol may include: >>>> + * o display/connector cloning >>>> + * o allocation of objects other than display buffers >>>> + * o plane/overlay support >>>> + * o scaling support >>>> + * o rotation support >>>> + * >>>> + ****************************************************************************** >>>> + * Feature and Parameter Negotiation >>>> + ****************************************************************************** >>>> + * >>>> + * Front->back notifications: when enqueuing a new request, sending a >>>> + * notification can be made conditional on xendispl_req (i.e., the generic >>>> + * hold-off mechanism provided by the ring macros). Backends must set >>>> + * xendispl_req 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 xendispl_resp (i.e., the generic >>>> + * hold-off mechanism provided by the ring macros). Frontends must set >>>> + * xendispl_resp appropriately (e.g., using RING_FINAL_CHECK_FOR_RESPONSES()). >>>> + * >>>> + * The two halves of a para-virtual display driver utilize nodes within >>>> + * XenStore to communicate capabilities and to negotiate operating parameters. >>>> + * This section enumerates these nodes which reside in the respective front and >>>> + * backend portions of XenStore, following the XenBus convention. >>>> + * >>>> + * All data in 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. >>>> + * >>>> + ****************************************************************************** >>>> + * Example configuration >>>> + ****************************************************************************** >>>> + * >>>> + * Note: depending on the use-case backend can expose more display connectors >>>> + * than the underlying HW physically has by employing SW graphics compositors >>>> + * >>>> + * This is an example of backend and frontend configuration: >>>> + * >>>> + *--------------------------------- Backend ----------------------------------- >>>> + * >>>> + * /local/domain/0/backend/vdispl/1/0/frontend-id = "1" >>>> + * /local/domain/0/backend/vdispl/1/0/frontend = "/local/domain/1/device/vdispl/0" >>>> + * /local/domain/0/backend/vdispl/1/0/state = "4" >>>> + * /local/domain/0/backend/vdispl/1/0/versions = "1,2" >>>> + * /local/domain/0/backend/vdispl/1/0/features = "be_alloc" >>>> + * >>>> + *--------------------------------- Frontend ---------------------------------- >>>> + * >>>> + * /local/domain/1/device/vdispl/0/backend-id = "0" >>>> + * /local/domain/1/device/vdispl/0/backend = "/local/domain/0/backend/vdispl/1/0" >>>> + * /local/domain/1/device/vdispl/0/state = "4" >>>> + * /local/domain/1/device/vdispl/0/version = "1" >>>> + * >>>> + *-------------------------- Connector 0 configuration ------------------------ >>>> + * >>>> + * /local/domain/1/device/vdispl/0/0/resolution = "1920x1080" >>>> + * /local/domain/1/device/vdispl/0/0/ctrl-ring-ref = "2832" >>>> + * /local/domain/1/device/vdispl/0/0/ctrl-channel = "15" >>>> + * /local/domain/1/device/vdispl/0/0/event-ring-ref = "387" >>>> + * /local/domain/1/device/vdispl/0/0/event-channel = "16" >>>> + * >>>> + *-------------------------- Connector 1 configuration ------------------------ >>>> + * >>>> + * /local/domain/1/device/vdispl/0/1/resolution = "800x600" >>>> + * /local/domain/1/device/vdispl/0/1/ctrl-ring-ref = "2833" >>>> + * /local/domain/1/device/vdispl/0/1/ctrl-channel = "17" >>>> + * /local/domain/1/device/vdispl/0/1/event-ring-ref = "388" >>>> + * /local/domain/1/device/vdispl/0/1/event-channel = "18" >>>> + * >>>> + ****************************************************************************** >>>> + * Backend XenBus Nodes >>>> + ****************************************************************************** >>>> + * >>>> + *----------------------------- Protocol version ------------------------------ >>>> + * >>>> + * versions >>>> + * Values: <string> >>>> + * >>>> + * List of XENDISPL_LIST_SEPARATOR separated protocol versions supported >>>> + * by the backend. For example "1,2,3". >>>> + * >>>> + * features >>>> + * Values: <list of strings> >>>> + * >>>> + * XENDISPL_LIST_SEPARATOR separated list of features >>>> + * XENDISPL_FEATURE_BE_??? that the backend supports. >>>> + * >>>> + * be_alloc >>>> + * Backend can be a buffer provider/allocator during >>>> + * XENDISPL_OP_DBUF_CREATE operation (see below for negotiation). >>>> + * >>>> + ****************************************************************************** >>>> + * Frontend XenBus Nodes >>>> + ****************************************************************************** >>>> + * >>>> + *-------------------------------- Addressing --------------------------------- >>>> + * >>>> + * dom-id >>>> + * Values: <uint16_t> >>>> + * >>>> + * Domain identifier. >>>> + * >>>> + * dev-id >>>> + * Values: <uint16_t> >>>> + * >>>> + * Device identifier. >>>> + * >>>> + * conn-idx >>>> + * Values: <uint8_t> >>>> + * >>>> + * Zero based contigous index of the connector. >>>> + * /local/domain/<dom-id>/device/vdispl/<dev-id>/<conn-idx>/... >>>> + * >>>> + *----------------------------- Protocol version ------------------------------ >>>> + * >>>> + * version >>>> + * Values: <string> >>>> + * >>>> + * Protocol version, chosen among the ones supported by the backend. >>>> + * >>>> + *----------------------------- Connector settings ---------------------------- >>>> + * >>>> + * resolution >>>> + * Values: <width, uint32_t>x<height, uint32_t> >>>> + * >>>> + * Width and height of the connector in pixels separated by >>>> + * XENDISPL_RESOLUTION_SEPARATOR. >>> Is there an default depth? >> no >>> 32? >> no defaults > Should we have a default? > > And perhaps also say that all the operations are bounded by these? not sure we need default here: this is *up to backend* what to do with *depths* and *resolutions* which are different from what HW display supports, e.g. those buffers can be just a part of the final composition. For example, backend utilizes a SW compositor which makes screen compositing with 1920x1080x32 (lets say, for example, this is the most suitable one for the GPU used and also this is used for a remote desktop - use-cases may differ) But, the real HW display is only 1200x800x24. So, once the composition is done, it will be downscaled/resampled by any means to what HW display wants. Hope, this better explains why buffers may differ from what "resolution" says and depth used: buffer is not exactly what you see finally, so it can be bigger, smaller or equal - all valid >>> Or should that also be negotiated? >> frontend requests the desired value of it, backend provides it >> or rejects the request, so front will try other value. >> So, yes, this is kind of negotiation here (EINVAL based :) >> I will also put a note that "resolution" only defines visible area, >> thus display and frame buffer sizes can be different from this value > OK, which I suspect means that it can only be smaller, not larger. See above, no influence on buffer size >>>> + * >>>> + *------------------ Connector Request Transport Parameters ------------------- >>>> + * >>>> + * ctrl-channel >>> Please call it 'ctrl-event-channel' >> I will >>>> + * Values: <uint32_t> >>>> + * >>>> + * The identifier of the Xen connector's control event channel >>>> + * used to signal activity in the ring buffer. >>>> + * >>>> + * ctrl-ring-ref >>>> + * Values: <uint32_t> >>>> + * >>>> + * The Xen grant reference granting permission for the backend to map >>>> + * a sole page in a single page sized connector's control ring buffer. >>>> + * >>>> + * event-channel >>>> + * Values: <uint32_t> >>>> + * >>>> + * The identifier of the Xen connector's event channel >>>> + * used to signal activity in the ring buffer. >>>> + * >>>> + * event-ring-ref >>>> + * Values: <uint32_t> >>>> + * >>>> + * The Xen grant reference granting permission for the backend to map >>>> + * a sole page in a single page sized connector's event ring buffer. >>> So you are making two rings and two event channels - and it looks like the >>> operations are going via the control ring.. but what goes through the >>> the other ring buffer? >> Events from backend to frontend, currently only XENDISPL_EVT_PG_FLIP > Aaaaah! In that case I would change it from 'ctrl-' to > 'req-alloc' to match with. well, this is the control/command path, for that reason I still see its name as "ctrl/cmd-". What is more, I can agree to "req", but why "alloc"? This path is not only used for allocations, but configuration as well. In the future, I would expect other commands/control be passed over it. So, there is a command/control path ("ctrl-") and event path ("event-") which I believe do reflect their usage. >> I will define 2 sections: >> *------------------ Connector Request Transport Parameters >> ------------------- >> * >> * ctrl-event-channel >> * ctrl-ring-ref >> * >> *------------------- Connector Event Transport Parameters >> -------------------- >> * >> * event-channel >> * event-ring-ref >> >>> Or is the other ring buffer the one that is created via 'gref_directory' ? >> no >> At the bottom: >> * In order to deliver asynchronous events from back to front a shared page >> is >> * allocated by front and its gref propagated to back via XenStore entries >> * (event-XXX). > AAnd you may want to say this is guarded by REQ_ALLOC feature right? Not sure I understood you. Event path is totally independent from any feature, e.g. REQ_ALLOC. It just provides means to send async events from back to front, "page flip done" in my case. >>> If so, wouldn't you need mulitple event channels? And also the size of those >>> rings is much much bigger. But it is not really a ring. It looks to hold >>> some opaque data? >>> >>> -ECONFUSED. >>>> + */ >>>> + >>>> +/* >>>> + ****************************************************************************** >>>> + * STATE DIAGRAMS >>>> + ****************************************************************************** >>>> + * >>>> + * Tool stack creates front and back state nodes with initial state >>>> + * XenbusStateInitialising. >>>> + * Tool stack creates and sets up frontend display configuration >>>> + * nodes per domain. >>>> + * >>>> + *-------------------------------- Normal flow -------------------------------- >>>> + * >>>> + * 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 >>>> + * connector. >>>> + * 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 display connectors >>>> + * as per configuration. >>>> + * | >>>> + * | >>>> + * V >>>> + * XenbusStateConnected >>>> + * >>>> + * XenbusStateUnknown >>>> + * XenbusStateClosed >>>> + * XenbusStateClosing >>>> + * o Remove virtual display device >>>> + * o Remove event channels >>>> + * | >>>> + * | >>>> + * V >>>> + * XenbusStateClosed >>>> + * >>>> + *------------------------------- Recovery flow ------------------------------- >>>> + * >>>> + * In case of frontend unrecoverable errors backend handles that as >>>> + * if frontend goes into the XenbusStateClosed state. >>>> + * >>>> + * In case of backend unrecoverable errors frontend tries removing >>>> + * the emulated device. If this is possible at the moment of error, >>> What emulated device? >> s/emulated/virtualized/g for all? > /me nods. done >>>> + * then frontend goes into the XenbusStateInitialising state and is ready for >>>> + * new connection with backend. If the emulated device is still in use and >>> emulated device? >>> >>>> + * cannot be removed, then frontend goes into the XenbusStateReconfiguring state >>> .. and what is done in XenbusStateReconfiguring state? >>> >>> I am assuming 'resolution' is examined? >>> >>>> + * until either the emulated device removed or backend initiates a new >>>> + * connection. On the emulated device removal frontend goes into the >>> emulated device? >> The whole idea here is to let frontend recover from backend failure. >> What happens when backend dies: frontend cannot send requests >> to the backend and thus cannot emulate (virtualize) display >> device anymore. There is no simple solution like disconnecting rings >> and waiting for the backend to re-connect, as display device, just like >> sound devices, does hold some state: configuration in use, number of display >> buffers, framebuffers, application state, framework "watchdogs" (like >> vertical blank time-outs and recovery at framework level) etc. >> So, in most cases, it would require frontend to implement complex recovery >> logic. Instead, what I suggest, that we go into some state, which tells that >> we are not yet ready to go into Initialized state and still need some time: >> at this time frontend will try to make sure no new clients from user-space >> are accepted, it allows the current client to exit gracefully by returning >> error codes etc. Once all the clients are gone frontend can reinitialize >> the virtualized device, e.g. DRM/KMS driver, and get into Initialized state >> again. > That is fine. >> Thus, I was thinking of XenbusStateReconfiguringstate as appropriate in this >> case > Right, but somebody has to move to this state. Who would do it? when backend dies its state changes to "closed". At this moment front tries to remove virtualized device and if it is possible/done, then it goes into "initialized" state. If not - "reconfiguring". So, you would ask how does the front go from "reconfiguring" into "initialized" state? This is OS/front specific, but: 1. the underlying framework, e.g. DRM/KMS, ALSA, provide a callback(s) to signal that the last client to the virtualized device has gone and the driver can be removed (equivalent to module's usage counter 0) 2. one can schedule a delayed work (timer/tasklet/workqueue) to periodically check if this is the right time to re-try the removal and remove In both cases, after the virtualized device has been removed we move into "initialized" state again and are ready for new connections with backend (if it arose from the dead :) > Would the > frontend have some form of timer to make sure that the backend is still > alive? And if it had died then move to Reconfiguring? There are at least 2 ways to understand if back is dead: 1. XenBus state change (back is closed) 2. time-outs on requests from front to back Bottom line on the "recovery flow": what I describe could be put in the description as an example to clarify more this. >>>> + * XenbusStateInitialising state. >>>> + * >>>> + ****************************************************************************** >>>> + * REQUEST CODES >>>> + ****************************************************************************** >>>> + */ >>>> +#define XENDISPL_OP_DBUF_CREATE 0 >>>> +#define XENDISPL_OP_DBUF_DESTROY 1 >>>> +#define XENDISPL_OP_FB_ATTACH 2 >>>> +#define XENDISPL_OP_FB_DETACH 3 >>>> +#define XENDISPL_OP_SET_CONFIG 4 >>>> +#define XENDISPL_OP_PG_FLIP 5 >>>> + >>>> +/* >>>> + ****************************************************************************** >>>> + * EVENT CODES >>>> + ****************************************************************************** >>>> + */ >>>> +#define XENDISPL_EVT_PG_FLIP 0 >>>> + >>>> +/* >>>> + ****************************************************************************** >>>> + * XENSTORE FIELD AND PATH NAME STRINGS, HELPERS >>>> + ****************************************************************************** >>>> + */ >>>> +#define XENDISPL_DRIVER_NAME "vdispl" >>>> + >>>> +#define XENDISPL_LIST_SEPARATOR "," >>>> +#define XENDISPL_RESOLUTION_SEPARATOR "x" >>>> +/* Field names */ >>>> +#define XENDISPL_FIELD_BE_VERSIONS "versions" >>>> +#define XENDISPL_FIELD_FE_VERSION "version" >>>> +#define XENDISPL_FIELD_FEATURES "features" >>>> +#define XENDISPL_FIELD_CTRL_RING_REF "ctrl-ring-ref" >>>> +#define XENDISPL_FIELD_CTRL_CHANNEL "ctrl-channel" >>>> +#define XENDISPL_FIELD_EVT_RING_REF "event-ring-ref" >>>> +#define XENDISPL_FIELD_EVT_CHANNEL "event-channel" >>>> +#define XENDISPL_FIELD_RESOLUTION "resolution" >>>> + >>>> +#define XENDISPL_FEATURE_BE_ALLOC "be_alloc" >>>> + >>>> +/* >>>> + ****************************************************************************** >>>> + * STATUS RETURN CODES >>>> + ****************************************************************************** >>>> + * >>>> + * Status return code is zero on success and -XEN_EXX on failure. >>>> + * >>>> + ****************************************************************************** >>>> + * Assumptions >>>> + ****************************************************************************** >>>> + * o usage of grant reference 0 as invalid grant reference: >>>> + * grant reference 0 is valid, but never exposed to a PV driver, >>>> + * because of the fact it is already in use/reserved by the PV console. >>>> + * o all references in this document to page sizes must be treated >>>> + * as pages of size XEN_PAGE_SIZE unless otherwise noted. >>>> + * >>>> + ****************************************************************************** >>>> + * Description of the protocol between frontend and backend driver >>>> + ****************************************************************************** >>>> + * >>>> + * The two halves of a Para-virtual display driver communicate with >>>> + * each other using shared pages and event channels. >>>> + * Shared page contains a ring with request/response packets. >>>> + * >>>> + * All reserved fields in the structures below must be 0. >>>> + * Display buffers's cookie of value 0 treated as invalid. >>>> + * Framebuffer's cookie of value 0 treated as invalid. >>>> + * >>>> + *---------------------------------- Requests --------------------------------- >>>> + * >>>> + * All requests/responses, which are not connector specific, must be sent over >>>> + * control ring of the connector with index 0. >>>> + * >>>> + * For all request/response/event packets that use cookies: >>>> + * dbuf_cookie - uint64_t, unique to guest domain value used by the backend >>>> + * to map remote display buffer to its local one >>>> + * fb_cookie - uint64_t, unique to guest domain value used by the backend >>>> + * to map remote framebuffer to its local one >>>> + * >>>> + * All request packets have the same length (64 octets) >>>> + * All request packets have common header: >>>> + * 0 1 2 3 octet >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | id | operation | reserved | 4 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 8 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * id - uint16_t, private guest value, echoed in response >>>> + * operation - uint8_t, operation code, XENDISPL_OP_??? >>>> + * >>>> + * Request dbuf creation - request creation of a display buffer. >>>> + * 0 1 2 3 octet >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | id |_OP_DBUF_CREATE | reserved | 4 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 8 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | dbuf_cookie low 32-bit | 12 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | dbuf_cookie high 32-bit | 16 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | width | 20 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | height | 24 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | bpp | 28 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | buffer_sz | 32 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | flags | 36 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | gref_directory | 40 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 44 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 64 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * >>>> + * Must be sent over control ring of the connector with index 0. >>>> + * >>>> + * width - uint32_t, width in pixels >>>> + * height - uint32_t, height in pixels >>> Why? Isn't that in the 'resolution' XenBus entry? >> It can be bigger that the resolution, for example >> if scaling will take place. > How would you reflect the scaling part of this? Is that something > that is assumed if the XenBus resolution value < widthxheight values? As in the above example with back utilizing an SW compositor, scaling will be done by the backend, front nows nothing about the destiny of the buffers it wants to show So, no need to worry about widthXheightXdepth of the buffers >>> If not, why even have the 'resolution' field? >> The confusion comes from the fact that the resolution is visible area: >> will state that clearly. >>> And also - what if the width or height are bigger than 'resolution'? >>> Should -EINVAL be returned? >>> >> no problem here: we can have buffers bigger than visible area >>>> + * bpp - uint32_t, bits per pixel >>>> + * buffer_sz - uint32_t, buffer size to be allocated, octets >>>> + * flags - uint32_t, flags of the operation >>>> + * o XENDISPL_DBUF_FLG_REQ_ALLOC - if set, then backend is requested >>>> + * to allocate the buffer with the parameters provided in this request. >>>> + * Page directory is handled as follows: >>>> + * Frontend on request: >>>> + * o allocates pages for the directory >>>> + * o grants permissions for the pages of the directory >>>> + * o sets gref_dir_next_page fields >>>> + * Backend on response: >>>> + * o grants permissions for the pages of the buffer allocated >>> <scratches his head> >>>> + * o fills in page directory with grant references >>> The backend provides the pages instead of the frontend? But you said >>> 'allocated pages for the directory' in the frontned part - and since you >>> say 'pages' that means more than one? Or did you mean 'the gref_directory' >>> page and the 'gref_dir_next_page' pages? You may want to be explicit. >>> >> sure, I will state it clearly that only directory pages >> are allocated: gref_directory + gref_dir_next_page(s) > Thx >>> And what if the 'flags' is zero? >> it would mean that even that the backend can allocate buffers >> for us we don't want it to do so, we will allocate on our own: >> flag is not set >>> Or 0x314? Perhaps also mention >>> the default one (0 I presume) which would make the backend map >>> the frontend pages provided in 'gref_directory'? >> yes, I'll put a note on defaults > Awesome! >>>> + * gref_directory - grant_ref_t, a reference to the first shared page >>>> + * describing shared buffer references. At least one page exists. If shared >>>> + * buffer size (buffer_sz) exceeds what can be addressed by this single page, >>> ^- extra space >> done >>>> + * then reference to the next page must be supplied (see gref_dir_next_page >>>> + * below) >>>> + */ >>> What are some of the return errors ? Would it make sense to mention >>> some? >> not sure, this way will dictate implementation details, >> but we are defining protocol > True to an extent. And that extent makes me believe we shouldn't dictate error codes here. POSIX error codes are (sometimes :) more than descriptive ) >>> What to for example if there are multiple _OP_DBUF_CREATE >>> with the same dbuf_cookie but with different width/height? >> it will be an error, I will clearly state this: >> * An attempt to create multiple display buffers with the same dbuf_cookie >> is >> * an error. dbuf_cookie can be re-used after destroying the corresponding >> * display buffer. > May also want to say what error value it should be. -EAGAIN? No probably something > different. As above, I would prefer to not put anything >>>> + >>>> +#define XENDISPL_DBUF_FLG_REQ_ALLOC 0x0001 >>>> + >>>> +struct xendispl_dbuf_create_req { >>>> + uint64_t dbuf_cookie; >>>> + uint32_t width; >>>> + uint32_t height; >>>> + uint32_t bpp; >>>> + uint32_t buffer_sz; >>>> + uint32_t flags; >>>> + grant_ref_t gref_directory; >>>> +}; >>>> + >>>> +/* >>>> + * Shared page for XENDISPL_OP_DBUF_CREATE 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 | 4 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | gref[0] | 8 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | gref[i] | i*4+8 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | gref[N - 1] | N*4+8 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * >>>> + * gref_dir_next_page - grant_ref_t, reference to the next page describing >>>> + * page directory. Must be 0 if there are no more pages in the list. >>>> + * gref[i] - grant_ref_t, reference to a shared page of the buffer >>>> + * allocated at XENDISPL_OP_DBUF_CREATE >>>> + * >>>> + * Number of grant_ref_t entries in the whole page directory is not >>>> + * passed, but instead can be calculated as: >>>> + * num_grefs_total = (XENDISPL_OP_DBUF_CREATE.buffer_sz + XEN_PAGE_SIZE - 1) / >>>> + * XEN_PAGE_SIZE >>>> + */ >>>> + >>>> +struct xendispl_page_directory { >>>> + grant_ref_t gref_dir_next_page; >>>> + grant_ref_t gref[1]; /* Variable length */ >>>> +}; >>>> + >>>> +/* >>>> + * Request dbuf destruction - destroy a previously allocated display buffer: >>>> + * 0 1 2 3 octet >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | id |_OP_DBUF_DESTROY| reserved | 4 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 8 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | dbuf_cookie low 32-bit | 12 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | dbuf_cookie high 32-bit | 16 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 20 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 64 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * >>>> + * Must be sent over control ring of the connector with index 0. >>>> + */ >>>> + >>>> +struct xendispl_dbuf_destroy_req { >>>> + uint64_t dbuf_cookie; >>>> +}; >>>> + >>>> +/* >>>> + * Request framebuffer attachment - request attachment of a framebuffer to >>>> + * previously created display buffer. >>>> + * 0 1 2 3 octet >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | id | _OP_FB_ATTACH | reserved | 4 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 8 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | dbuf_cookie low 32-bit | 12 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | dbuf_cookie high 32-bit | 16 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | fb_cookie low 32-bit | 20 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | fb_cookie high 32-bit | 24 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | width | 28 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | height | 32 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | pixel_format | 36 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 40 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 64 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * >>>> + * Must be sent over control ring of the connector with index 0. >>>> + * >>>> + * width - uint32_t, width in pixels >>>> + * height - uint32_t, height in pixels >>> Why? The _OP_DBUF_CREATE already specified the height/width. Why have this? >>> >>> Or are you thinking to have an smaller (or bigger?) resolution framebuffer >>> than the display? I suppose you can do that which would imply that the display >>> would switch modes? Perhaps mention this? >>> >>> Also what if these values are bigger (or lesser) than 'resolution' ? >>> >>> >>>> + * pixel_format - uint32_t, pixel format of the framebuffer, FOURCC code >>> You did not mention the fb_cookie? >> I have a common section: >> * For all request/response/event packets that use cookies: >> * dbuf_cookie - uint64_t, unique to guest domain value used by the >> backend >> * to map remote display buffer to its local one >> * fb_cookie - uint64_t, unique to guest domain value used by the backend >> * to map remote framebuffer to its local one >> But it is in the "Requests" section - I'll pull it out to be common for all > Aaaah! >>>> + */ >>>> + >>>> +struct xendispl_fb_attach_req { >>>> + uint64_t dbuf_cookie; >>>> + uint64_t fb_cookie; >>>> + uint32_t width; >>>> + uint32_t height; >>>> + uint32_t pixel_format; >>>> +}; >>>> + >>>> +/* >>>> + * Request framebuffer detach - detach a previously >>>> + * attached framebuffer from the display buffer in request: >>>> + * 0 1 2 3 octet >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | id | _OP_FB_DETACH | reserved | 4 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 8 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | fb_cookie low 32-bit | 12 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | fb_cookie high 32-bit | 16 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 20 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 64 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * >>>> + * Must be sent over control ring of the connector with index 0. >>>> + */ >>>> + >>>> +struct xendispl_fb_detach_req { >>>> + uint64_t fb_cookie; >>>> +}; >>>> + >>>> +/* >>>> + * Request configuration set/reset - request to set or reset >>>> + * the configuration/mode of the display: >>>> + * 0 1 2 3 octet >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | id | _OP_SET_CONFIG | reserved | 4 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 8 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | fb_cookie low 32-bit | 12 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | fb_cookie high 32-bit | 16 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | x | 20 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | y | 24 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | width | 28 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | height | 32 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | bpp | 40 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 44 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 64 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * >>>> + * Pass all zeros to reset, otherwise command is treated as >>>> + * configuration set. >>> All zeros? Including fb_cookie? >> Yes, fb_cookie is 0 when we disable the display, as at this point there >> could be no fb at all (all released by that time) >>>> + * If this is a set configuration request then framebuffer's cookie tells >>> Not sure if the first part of the sentence is needed. This is after all >>> the section that talks about setting configurations. >> ok, will remove it >>>> + * the display which framebuffer/dbuf must be displayed while enabling display >>>> + * (applying configuration). >>> Can you disable an display without changing this? >> You pass ALL zeros, including fb_cookie to disable/reset. >>>> + * >>>> + * x - uint32_t, starting position in pixels by X axis >>>> + * y - uint32_t, starting position in pixels by Y axis >>> Which is bound by 'resolution' ? Or by the display command? >> these are limited by the "resolution" - will state that: >> * x, y, width and height are bound by the connector resolution and must not >> * exceed it. > Thanks! BTW, this is the only place ATM where we stick to resolution from XenStore... And it will clearly be stated >>>> + * width - uint32_t, width in pixels >>>> + * height - uint32_t, height in pixels >>>> + * bpp - uint32_t, bits per pixel >>> Shouldn't there be an stride as well? >> no, stride is kind of implementation detail which shouldn't >> be in the protocol >>>> + */ >>>> + >>>> +struct xendispl_set_config_req { >>>> + uint64_t fb_cookie; >>>> + uint32_t x; >>>> + uint32_t y; >>>> + uint32_t width; >>>> + uint32_t height; >>>> + uint32_t bpp; >>>> +}; >>>> + >>>> +/* >>>> + * Request page flip - request to flip a page identified by the framebuffer >>>> + * cookie: >>>> + * 0 1 2 3 octet >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | id | _OP_PG_FLIP | reserved | 4 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 8 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | fb_cookie low 32-bit | 12 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | fb_cookie high 32-bit | 16 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 20 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 64 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + */ >>>> + >>>> +struct xendispl_page_flip_req { >>>> + uint64_t fb_cookie; >>>> +}; >>>> + >>>> +/* >>>> + *---------------------------------- Responses -------------------------------- >>>> + * >>>> + * All response packets have the same length (64 octets) >>>> + * >>>> + * All response packets have common header: >>>> + * 0 1 2 3 octet >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | id | reserved | 4 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | status | 8 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 12 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 64 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * >>>> + * id - uint16_t, private guest value, echoed from request >>>> + * status - int32_t, response status, zero on success and -XEN_EXX on failure >>>> + * >>>> + *----------------------------------- Events ---------------------------------- >>>> + * >>>> + * Events are sent via a shared page allocated by the front and propagated by >>>> + * event-channel/event-ring-ref XenStore entries >>>> + * All event packets have the same length (64 octets) >>>> + * All event packets have common header: >>>> + * 0 1 2 3 octet >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | id | type | reserved | 4 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 8 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * >>>> + * id - uint16_t, event id, may be used by front >>>> + * type - uint8_t, type of the event >>>> + * >>>> + * >>>> + * Page flip complete event - event from back to front on page flip completed: >>>> + * 0 1 2 3 octet >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | id | _EVT_PG_FLIP | reserved | 4 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 8 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | fb_cookie low 32-bit | 12 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | fb_cookie high 32-bit | 16 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 20 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>> + * +----------------+----------------+----------------+----------------+ >>>> + * | reserved | 64 >>>> + * +----------------+----------------+----------------+----------------+ >>>> + */ >>>> + >>>> +struct xendispl_pg_flip_evt { >>>> + uint64_t fb_cookie; >>>> +}; >>>> + >>>> +struct xendispl_req { >>>> + uint16_t id; >>>> + uint8_t operation; >>>> + uint8_t reserved[5]; >>>> + union { >>>> + struct xendispl_dbuf_create_req dbuf_create; >>>> + struct xendispl_dbuf_destroy_req dbuf_destroy; >>>> + struct xendispl_fb_attach_req fb_attach; >>>> + struct xendispl_fb_detach_req fb_detach; >>>> + struct xendispl_set_config_req set_config; >>>> + struct xendispl_page_flip_req pg_flip; >>>> + uint8_t reserved[56]; >>>> + } op; >>>> +}; >>>> + >>>> +struct xendispl_resp { >>>> + uint16_t id; >>>> + uint8_t operation; >>>> + uint8_t reserved; >>>> + int32_t status; >>>> + uint8_t reserved1[56]; >>>> +}; >>>> + >>>> +struct xendispl_evt { >>>> + uint16_t id; >>>> + uint8_t type; >>>> + uint8_t reserved[5]; >>>> + union { >>>> + struct xendispl_pg_flip_evt pg_flip; >>>> + uint8_t reserved[56]; >>>> + } op; >>>> +}; >>>> + >>>> +DEFINE_RING_TYPES(xen_displif, struct xendispl_req, struct xendispl_resp); >>>> + >>>> +/* >>>> + ****************************************************************************** >>>> + * Back to front events delivery >>>> + ****************************************************************************** >>>> + * In order to deliver asynchronous events from back to front a shared page is >>>> + * allocated by front and its grefs propagated to back via XenStore entries >>>> + * (event-XXX). >>> ? >>> >>> You mean event-channel ? Or control-event-channel? >> event-channel which is used for async events from back to front, >> control is always used to pass req/resp. >>>> + * This page has a common header used by both front and back to synchronize >>>> + * access and control event's ring buffer, while back being a producer of the >>>> + * events and front being a consumer. The rest of the page after the header >>>> + * is used for event packets. >>>> + * >>>> + * Upon reception of an event(s) front may confirm its reception >>>> + * for either each event, group of events or none. >>>> + */ >>>> + >>>> +struct xendispl_event_page { >>>> + uint32_t in_cons; >>>> + uint32_t in_prod; >>>> + uint8_t reserved[56]; >>>> +}; >>>> + >>>> +#define XENDISPL_EVENT_PAGE_SIZE 4096 >>>> +#define XENDISPL_IN_RING_OFFS (sizeof(struct xendispl_event_page)) >>>> +#define XENDISPL_IN_RING_SIZE (XENDISPL_EVENT_PAGE_SIZE - XENDISPL_IN_RING_OFFS) >>>> +#define XENDISPL_IN_RING_LEN (XENDISPL_IN_RING_SIZE / sizeof(struct xendispl_evt)) >>>> +#define XENDISPL_IN_RING(page) \ >>>> + ((struct xendispl_evt *)((char *)(page) + XENDISPL_IN_RING_OFFS)) >>>> +#define XENDISPL_IN_RING_REF(page, idx) \ >>>> + (XENDISPL_IN_RING((page))[(idx) % XENDISPL_IN_RING_LEN]) >>> Why not use the ring.h macros? >> well, this is the way the same structs are defined for fbif/kbif >> and I am not sure what can be re-used here from ring.h as it is >> designed to work with req/resp, not events. >>> Especially as the top of this talks about "RING_FINAL_CHECK_FOR_REQUESTS"? >> I can remove this >>>> + >>>> +#endif /* __XEN_PUBLIC_IO_DISPLIF_H__ */ >>> Could we also have this header in: >> sure, I will add the same to sndif as well >>> /* >>> * Local variables: >>> * mode: C >>> * c-file-style: "BSD" >>> * c-basic-offset: 4 >>> * tab-width: 4 >>> * indent-tabs-mode: nil >>> * End: >>> */ >>> >>> please? >>>> -- >>>> 2.7.4 >>>> >> [1] http://marc.info/?l=xen-devel&m=142167153324052&w=4 >> Thank you, Oleksandr
On Wed, Feb 15, 2017 at 09:33:41AM +0200, Oleksandr Andrushchenko wrote: > On 02/14/2017 10:27 PM, Konrad Rzeszutek Wilk wrote: > > On Mon, Feb 13, 2017 at 10:50:49AM +0200, Oleksandr Andrushchenko wrote: > > > Hi, Konrad! > > > > > > Thank you for reviewing this. > > > > > > On 02/10/2017 11:27 PM, Konrad Rzeszutek Wilk wrote: > > > > On Fri, Feb 10, 2017 at 09:29:58AM +0200, Oleksandr Andrushchenko wrote: > > > > > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > > > > > > > > > This is the ABI for the two halves of a para-virtualized > > > > > display driver. > > > > > > > > > > This protocol aims to provide a unified protocol which fits more > > > > > sophisticated use-cases than a framebuffer device can handle. At the > > > > > moment basic functionality is supported with the intention to extend: > > > > > o multiple dynamically allocated/destroyed framebuffers > > > > > o buffers of arbitrary sizes > > > > > o better configuration options including multiple display support > > > > > > > > > > Note: existing fbif can be used together with displif running at the > > > > > same time, e.g. on Linux one provides framebuffer and another DRM/KMS > > > > > > > > > > Future extensions to the existing protocol may include: > > > > > o allow display/connector cloning > > > > > o allow allocating objects other than display buffers > > > > > o add planes/overlays support > > > > > o support scaling > > > > > o support rotation > > > > > > > > > > ================================================== > > > > > Rationale for introducing this protocol instead of > > > > > using the existing fbif: > > > > > ================================================== > > > > > > > > > > 1. In/out event sizes > > > > > o fbif - 40 octets > > > > > o displif - 40 octets > > > > > This is only the initial version of the displif protocol > > > > > which means that there could be requests which will not fit > > > > > (WRT introducing some GPU related functionality > > > > > later on). In that case we cannot alter fbif sizes as we need to > > > > > be backward compatible an will be forced to handle those > > > > > apart of fbif. > > > > > > > > > > 2. Shared page > > > > > Displif doesn't use anything like struct xenfb_page, but > > > > > DEFINE_RING_TYPES(xen_displif, struct xendispl_req, struct > > > > > xendispl_resp) which is a better and more common way. > > > > > Output events use a shared page which only has in_cons and in_prod > > > > > and all the rest is used for incoming events. Here struct xenfb_page > > > > > could probably be used as is despite the fact that it only has a half > > > > > of a page for incoming events which is only 50 events. (consider > > > > > something like 60Hz display) > > > > > > > > > > 3. Amount of changes. > > > > > fbif only provides XENFB_TYPE_UPDATE and XENFB_TYPE_RESIZE > > > > > events, so it looks like it is easier to get fb support into displif > > > > .. would it make sense to reserve some of those values (2, 3) > > > > in the XENDISPL_OP_ values? So that if this happens there is a nice > > > > fit in there? Thought looking at the structure there is no easy > > > > way to 'overlay' the xenfb_out_event structure as it is missing the 'id'. > > > > > > > > I guess one can get creative. > > > > > > > > Or you could swap positions of 'id' and 'type'? And then it would fit much > > > > nicer? > > > yeap, in order no one needs to be creative, why not > > > explicitly define those? > > > Anyways, it won't be possible to simply lay the structures from > > > fbif on top of displif (different structure, size, workflow etc.), > > > what is more that would give some room to find workarounds, rather than > > > have well defined solution. BTW, there was an attempt [1] to update > > > fbdev to meet modern application needs. If we decide to add FBDEV > > > functionality into this protocol, then I can re-use already > > > proven to work solution from [1] into DISPLIF, but defining > > > structures and events to fit the current protocol. > > I was thinking that since the size of the structure is 64bytes > > you have enough space to jam in the old structures too. > yes, I understand your intention > > > > Naturally the driver would need adjustments as it offsets > > of where this goes are all wrong. > exactly > > > > > What do you think? > so, what is the decision? Options: > 1) Leave the protocol as is, if need be it can be extended later > 2) Implement something similar to [1] > 3) Give room for workarounds and reserve XENDISPL_OP_XXX for > what current fbif uses I would do 3) so that it becomes easier to migrate the old code over. > > > > > than vice versa. displif at the moment has 6 requests and 1 event, > > > > > multiple connector support, etc. > > > > > > > > > > Changes since v2: > > > > > * updated XenStore configuration template/pattern > > > > > * added "Recovery flow" to state diagram description > > > > > * renamed gref_directory_start to gref_directory > > > > > * added missing "versions" and "version" string constants > > > > > > > > > > Changes since v1: > > > > > * fixed xendispl_event_page padding size > > > > > * added versioning support > > > > > * explicitly define value types for XenStore fields > > > > > * text decoration re-work > > > > > * added offsets to ASCII box notation > > > > > > > > > > Changes since initial: > > > > > * DRM changed to DISPL, protocol made generic > > > > > * major re-work addressing issues raised for sndif > > > > > > > > > > Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> > > > > > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > > > > --- > > > > > xen/include/public/io/displif.h | 778 ++++++++++++++++++++++++++++++++++++++++ > > > > > 1 file changed, 778 insertions(+) > > > > > create mode 100644 xen/include/public/io/displif.h > > > > > > > > > > diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h > > > > > new file mode 100644 > > > > > index 000000000000..849f27fe5f1d > > > > > --- /dev/null > > > > > +++ b/xen/include/public/io/displif.h > > > > > @@ -0,0 +1,778 @@ > > > > > +/****************************************************************************** > > > > > + * displif.h > > > > > + * > > > > > + * Unified display 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) 2016-2017 EPAM Systems Inc. > > > > > + * > > > > > + * Authors: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > > > > + * Oleksandr Grytsov <oleksandr_grytsov@epam.com> > > > > > + */ > > > > > + > > > > > +#ifndef __XEN_PUBLIC_IO_DISPLIF_H__ > > > > > +#define __XEN_PUBLIC_IO_DISPLIF_H__ > > > > > + > > > > > +#include "ring.h" > > > > > +#include "../grant_table.h" > > > > > + > > > > > +/* > > > > > + ****************************************************************************** > > > > > + * Main features provided by the protocol > > > > > + ****************************************************************************** > > > > > + * This protocol aims to provide a unified protocol which fits more > > > > > + * sophisticated use-cases than a framebuffer device can handle. At the > > > > > + * moment basic functionality is supported with the intention to be extended: > > > > > + * o multiple dynamically allocated/destroyed framebuffers > > > > > + * o buffers of arbitrary sizes > > > > > + * o buffer allocation at either back or front end > > > > > + * o better configuration options including multiple display support > > > > > + * > > > > > + * Note: existing fbif can be used together with displif running at the > > > > > + * same time, e.g. on Linux one provides framebuffer and another DRM/KMS > > > > > + * > > > > > + ****************************************************************************** > > > > > + * Direction of improvements > > > > > + ****************************************************************************** > > > > > + * Future extensions to the existing protocol may include: > > > > > + * o display/connector cloning > > > > > + * o allocation of objects other than display buffers > > > > > + * o plane/overlay support > > > > > + * o scaling support > > > > > + * o rotation support > > > > > + * > > > > > + ****************************************************************************** > > > > > + * Feature and Parameter Negotiation > > > > > + ****************************************************************************** > > > > > + * > > > > > + * Front->back notifications: when enqueuing a new request, sending a > > > > > + * notification can be made conditional on xendispl_req (i.e., the generic > > > > > + * hold-off mechanism provided by the ring macros). Backends must set > > > > > + * xendispl_req 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 xendispl_resp (i.e., the generic > > > > > + * hold-off mechanism provided by the ring macros). Frontends must set > > > > > + * xendispl_resp appropriately (e.g., using RING_FINAL_CHECK_FOR_RESPONSES()). > > > > > + * > > > > > + * The two halves of a para-virtual display driver utilize nodes within > > > > > + * XenStore to communicate capabilities and to negotiate operating parameters. > > > > > + * This section enumerates these nodes which reside in the respective front and > > > > > + * backend portions of XenStore, following the XenBus convention. > > > > > + * > > > > > + * All data in 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. > > > > > + * > > > > > + ****************************************************************************** > > > > > + * Example configuration > > > > > + ****************************************************************************** > > > > > + * > > > > > + * Note: depending on the use-case backend can expose more display connectors > > > > > + * than the underlying HW physically has by employing SW graphics compositors > > > > > + * > > > > > + * This is an example of backend and frontend configuration: > > > > > + * > > > > > + *--------------------------------- Backend ----------------------------------- > > > > > + * > > > > > + * /local/domain/0/backend/vdispl/1/0/frontend-id = "1" > > > > > + * /local/domain/0/backend/vdispl/1/0/frontend = "/local/domain/1/device/vdispl/0" > > > > > + * /local/domain/0/backend/vdispl/1/0/state = "4" > > > > > + * /local/domain/0/backend/vdispl/1/0/versions = "1,2" > > > > > + * /local/domain/0/backend/vdispl/1/0/features = "be_alloc" > > > > > + * > > > > > + *--------------------------------- Frontend ---------------------------------- > > > > > + * > > > > > + * /local/domain/1/device/vdispl/0/backend-id = "0" > > > > > + * /local/domain/1/device/vdispl/0/backend = "/local/domain/0/backend/vdispl/1/0" > > > > > + * /local/domain/1/device/vdispl/0/state = "4" > > > > > + * /local/domain/1/device/vdispl/0/version = "1" > > > > > + * > > > > > + *-------------------------- Connector 0 configuration ------------------------ > > > > > + * > > > > > + * /local/domain/1/device/vdispl/0/0/resolution = "1920x1080" > > > > > + * /local/domain/1/device/vdispl/0/0/ctrl-ring-ref = "2832" > > > > > + * /local/domain/1/device/vdispl/0/0/ctrl-channel = "15" > > > > > + * /local/domain/1/device/vdispl/0/0/event-ring-ref = "387" > > > > > + * /local/domain/1/device/vdispl/0/0/event-channel = "16" > > > > > + * > > > > > + *-------------------------- Connector 1 configuration ------------------------ > > > > > + * > > > > > + * /local/domain/1/device/vdispl/0/1/resolution = "800x600" > > > > > + * /local/domain/1/device/vdispl/0/1/ctrl-ring-ref = "2833" > > > > > + * /local/domain/1/device/vdispl/0/1/ctrl-channel = "17" > > > > > + * /local/domain/1/device/vdispl/0/1/event-ring-ref = "388" > > > > > + * /local/domain/1/device/vdispl/0/1/event-channel = "18" > > > > > + * > > > > > + ****************************************************************************** > > > > > + * Backend XenBus Nodes > > > > > + ****************************************************************************** > > > > > + * > > > > > + *----------------------------- Protocol version ------------------------------ > > > > > + * > > > > > + * versions > > > > > + * Values: <string> > > > > > + * > > > > > + * List of XENDISPL_LIST_SEPARATOR separated protocol versions supported > > > > > + * by the backend. For example "1,2,3". > > > > > + * > > > > > + * features > > > > > + * Values: <list of strings> > > > > > + * > > > > > + * XENDISPL_LIST_SEPARATOR separated list of features > > > > > + * XENDISPL_FEATURE_BE_??? that the backend supports. > > > > > + * > > > > > + * be_alloc > > > > > + * Backend can be a buffer provider/allocator during > > > > > + * XENDISPL_OP_DBUF_CREATE operation (see below for negotiation). > > > > > + * > > > > > + ****************************************************************************** > > > > > + * Frontend XenBus Nodes > > > > > + ****************************************************************************** > > > > > + * > > > > > + *-------------------------------- Addressing --------------------------------- > > > > > + * > > > > > + * dom-id > > > > > + * Values: <uint16_t> > > > > > + * > > > > > + * Domain identifier. > > > > > + * > > > > > + * dev-id > > > > > + * Values: <uint16_t> > > > > > + * > > > > > + * Device identifier. > > > > > + * > > > > > + * conn-idx > > > > > + * Values: <uint8_t> > > > > > + * > > > > > + * Zero based contigous index of the connector. > > > > > + * /local/domain/<dom-id>/device/vdispl/<dev-id>/<conn-idx>/... > > > > > + * > > > > > + *----------------------------- Protocol version ------------------------------ > > > > > + * > > > > > + * version > > > > > + * Values: <string> > > > > > + * > > > > > + * Protocol version, chosen among the ones supported by the backend. > > > > > + * > > > > > + *----------------------------- Connector settings ---------------------------- > > > > > + * > > > > > + * resolution > > > > > + * Values: <width, uint32_t>x<height, uint32_t> > > > > > + * > > > > > + * Width and height of the connector in pixels separated by > > > > > + * XENDISPL_RESOLUTION_SEPARATOR. > > > > Is there an default depth? > > > no > > > > 32? > > > no defaults > > Should we have a default? > > > > And perhaps also say that all the operations are bounded by these? > not sure we need default here: this is *up to backend* > what to do with *depths* and *resolutions* which are different > from what HW display supports, e.g. those buffers > can be just a part of the final composition. > For example, backend utilizes a SW compositor which > makes screen compositing with 1920x1080x32 (lets say, for > example, this is the most suitable one for the GPU used > and also this is used for a remote desktop - use-cases may > differ) > But, the real HW display is only 1200x800x24. So, once the > composition is done, it will be downscaled/resampled by any > means to what HW display wants. > > Hope, this better explains why buffers may differ from > what "resolution" says and depth used: buffer is not > exactly what you see finally, so it can be bigger, smaller > or equal - all valid OK, you may to include this explanation in the document. > > > > Or should that also be negotiated? > > > frontend requests the desired value of it, backend provides it > > > or rejects the request, so front will try other value. > > > So, yes, this is kind of negotiation here (EINVAL based :) > > > I will also put a note that "resolution" only defines visible area, > > > thus display and frame buffer sizes can be different from this value > > OK, which I suspect means that it can only be smaller, not larger. > See above, no influence on buffer size > > > > > + * > > > > > + *------------------ Connector Request Transport Parameters ------------------- > > > > > + * > > > > > + * ctrl-channel > > > > Please call it 'ctrl-event-channel' > > > I will > > > > > + * Values: <uint32_t> > > > > > + * > > > > > + * The identifier of the Xen connector's control event channel > > > > > + * used to signal activity in the ring buffer. > > > > > + * > > > > > + * ctrl-ring-ref > > > > > + * Values: <uint32_t> > > > > > + * > > > > > + * The Xen grant reference granting permission for the backend to map > > > > > + * a sole page in a single page sized connector's control ring buffer. > > > > > + * > > > > > + * event-channel > > > > > + * Values: <uint32_t> > > > > > + * > > > > > + * The identifier of the Xen connector's event channel > > > > > + * used to signal activity in the ring buffer. > > > > > + * > > > > > + * event-ring-ref > > > > > + * Values: <uint32_t> > > > > > + * > > > > > + * The Xen grant reference granting permission for the backend to map > > > > > + * a sole page in a single page sized connector's event ring buffer. > > > > So you are making two rings and two event channels - and it looks like the > > > > operations are going via the control ring.. but what goes through the > > > > the other ring buffer? > > > Events from backend to frontend, currently only XENDISPL_EVT_PG_FLIP > > Aaaaah! In that case I would change it from 'ctrl-' to > > 'req-alloc' to match with. > well, this is the control/command path, for that reason I > still see its name as "ctrl/cmd-". > What is more, I can agree to "req", but why "alloc"? > This path is not only used for allocations, but configuration as well. > In the future, I would expect other commands/control be passed > over it. > So, there is a command/control path ("ctrl-") and event path ("event-") > which I believe do reflect their usage. Isn't this tied to 'XENDISPL_FEATURE_BE_ALLOC' feature? If so I would call it with that name in mind. > > > I will define 2 sections: > > > *------------------ Connector Request Transport Parameters > > > ------------------- > > > * > > > * ctrl-event-channel > > > * ctrl-ring-ref > > > * > > > *------------------- Connector Event Transport Parameters > > > -------------------- > > > * > > > * event-channel > > > * event-ring-ref > > > > > > > Or is the other ring buffer the one that is created via 'gref_directory' ? > > > no > > > At the bottom: > > > * In order to deliver asynchronous events from back to front a shared page > > > is > > > * allocated by front and its gref propagated to back via XenStore entries > > > * (event-XXX). > > AAnd you may want to say this is guarded by REQ_ALLOC feature right? > Not sure I understood you. Event path is totally independent > from any feature, e.g. REQ_ALLOC. > It just provides means to send async events > from back to front, "page flip done" in my case. <scratche his head> Why do you need a seperate ring to send responses back? Why not use the same ring on which requests were sent > > > > If so, wouldn't you need mulitple event channels? And also the size of those > > > > rings is much much bigger. But it is not really a ring. It looks to hold > > > > some opaque data? > > > > > > > > -ECONFUSED. > > > > > + */ > > > > > + > > > > > +/* > > > > > + ****************************************************************************** > > > > > + * STATE DIAGRAMS > > > > > + ****************************************************************************** > > > > > + * > > > > > + * Tool stack creates front and back state nodes with initial state > > > > > + * XenbusStateInitialising. > > > > > + * Tool stack creates and sets up frontend display configuration > > > > > + * nodes per domain. > > > > > + * > > > > > + *-------------------------------- Normal flow -------------------------------- > > > > > + * > > > > > + * 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 > > > > > + * connector. > > > > > + * 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 display connectors > > > > > + * as per configuration. > > > > > + * | > > > > > + * | > > > > > + * V > > > > > + * XenbusStateConnected > > > > > + * > > > > > + * XenbusStateUnknown > > > > > + * XenbusStateClosed > > > > > + * XenbusStateClosing > > > > > + * o Remove virtual display device > > > > > + * o Remove event channels > > > > > + * | > > > > > + * | > > > > > + * V > > > > > + * XenbusStateClosed > > > > > + * > > > > > + *------------------------------- Recovery flow ------------------------------- > > > > > + * > > > > > + * In case of frontend unrecoverable errors backend handles that as > > > > > + * if frontend goes into the XenbusStateClosed state. > > > > > + * > > > > > + * In case of backend unrecoverable errors frontend tries removing > > > > > + * the emulated device. If this is possible at the moment of error, > > > > What emulated device? > > > s/emulated/virtualized/g for all? > > /me nods. > done > > > > > + * then frontend goes into the XenbusStateInitialising state and is ready for > > > > > + * new connection with backend. If the emulated device is still in use and > > > > emulated device? > > > > > > > > > + * cannot be removed, then frontend goes into the XenbusStateReconfiguring state > > > > .. and what is done in XenbusStateReconfiguring state? > > > > > > > > I am assuming 'resolution' is examined? > > > > > > > > > + * until either the emulated device removed or backend initiates a new > > > > > + * connection. On the emulated device removal frontend goes into the > > > > emulated device? > > > The whole idea here is to let frontend recover from backend failure. > > > What happens when backend dies: frontend cannot send requests > > > to the backend and thus cannot emulate (virtualize) display > > > device anymore. There is no simple solution like disconnecting rings > > > and waiting for the backend to re-connect, as display device, just like > > > sound devices, does hold some state: configuration in use, number of display > > > buffers, framebuffers, application state, framework "watchdogs" (like > > > vertical blank time-outs and recovery at framework level) etc. > > > So, in most cases, it would require frontend to implement complex recovery > > > logic. Instead, what I suggest, that we go into some state, which tells that > > > we are not yet ready to go into Initialized state and still need some time: > > > at this time frontend will try to make sure no new clients from user-space > > > are accepted, it allows the current client to exit gracefully by returning > > > error codes etc. Once all the clients are gone frontend can reinitialize > > > the virtualized device, e.g. DRM/KMS driver, and get into Initialized state > > > again. > > That is fine. > > > Thus, I was thinking of XenbusStateReconfiguringstate as appropriate in this > > > case > > Right, but somebody has to move to this state. Who would do it? > when backend dies its state changes to "closed". > At this moment front tries to remove virtualized device > and if it is possible/done, then it goes into "initialized" > state. If not - "reconfiguring". > So, you would ask how does the front go from "reconfiguring" > into "initialized" state? This is OS/front specific, but: > 1. the underlying framework, e.g. DRM/KMS, ALSA, provide > a callback(s) to signal that the last client to the > virtualized device has gone and the driver can be removed > (equivalent to module's usage counter 0) > 2. one can schedule a delayed work (timer/tasklet/workqueue) > to periodically check if this is the right time to re-try > the removal and remove > > In both cases, after the virtualized device has been removed we move > into "initialized" state again and are ready for new connections > with backend (if it arose from the dead :) > > Would the > > frontend have some form of timer to make sure that the backend is still > > alive? And if it had died then move to Reconfiguring? > There are at least 2 ways to understand if back is dead: > 1. XenBus state change (back is closed) .. If the backend does a nice shutdown.. > 2. time-outs on requests from front to back > Right. > > Bottom line on the "recovery flow": what I describe could be put in > the description as an example to clarify more this. > > > > > + * XenbusStateInitialising state. > > > > > + * > > > > > + ****************************************************************************** > > > > > + * REQUEST CODES > > > > > + ****************************************************************************** > > > > > + */ > > > > > +#define XENDISPL_OP_DBUF_CREATE 0 > > > > > +#define XENDISPL_OP_DBUF_DESTROY 1 > > > > > +#define XENDISPL_OP_FB_ATTACH 2 > > > > > +#define XENDISPL_OP_FB_DETACH 3 > > > > > +#define XENDISPL_OP_SET_CONFIG 4 > > > > > +#define XENDISPL_OP_PG_FLIP 5 > > > > > + > > > > > +/* > > > > > + ****************************************************************************** > > > > > + * EVENT CODES > > > > > + ****************************************************************************** > > > > > + */ > > > > > +#define XENDISPL_EVT_PG_FLIP 0 > > > > > + > > > > > +/* > > > > > + ****************************************************************************** > > > > > + * XENSTORE FIELD AND PATH NAME STRINGS, HELPERS > > > > > + ****************************************************************************** > > > > > + */ > > > > > +#define XENDISPL_DRIVER_NAME "vdispl" > > > > > + > > > > > +#define XENDISPL_LIST_SEPARATOR "," > > > > > +#define XENDISPL_RESOLUTION_SEPARATOR "x" > > > > > +/* Field names */ > > > > > +#define XENDISPL_FIELD_BE_VERSIONS "versions" > > > > > +#define XENDISPL_FIELD_FE_VERSION "version" > > > > > +#define XENDISPL_FIELD_FEATURES "features" > > > > > +#define XENDISPL_FIELD_CTRL_RING_REF "ctrl-ring-ref" > > > > > +#define XENDISPL_FIELD_CTRL_CHANNEL "ctrl-channel" > > > > > +#define XENDISPL_FIELD_EVT_RING_REF "event-ring-ref" > > > > > +#define XENDISPL_FIELD_EVT_CHANNEL "event-channel" > > > > > +#define XENDISPL_FIELD_RESOLUTION "resolution" > > > > > + > > > > > +#define XENDISPL_FEATURE_BE_ALLOC "be_alloc" > > > > > + > > > > > +/* > > > > > + ****************************************************************************** > > > > > + * STATUS RETURN CODES > > > > > + ****************************************************************************** > > > > > + * > > > > > + * Status return code is zero on success and -XEN_EXX on failure. > > > > > + * > > > > > + ****************************************************************************** > > > > > + * Assumptions > > > > > + ****************************************************************************** > > > > > + * o usage of grant reference 0 as invalid grant reference: > > > > > + * grant reference 0 is valid, but never exposed to a PV driver, > > > > > + * because of the fact it is already in use/reserved by the PV console. > > > > > + * o all references in this document to page sizes must be treated > > > > > + * as pages of size XEN_PAGE_SIZE unless otherwise noted. > > > > > + * > > > > > + ****************************************************************************** > > > > > + * Description of the protocol between frontend and backend driver > > > > > + ****************************************************************************** > > > > > + * > > > > > + * The two halves of a Para-virtual display driver communicate with > > > > > + * each other using shared pages and event channels. > > > > > + * Shared page contains a ring with request/response packets. > > > > > + * > > > > > + * All reserved fields in the structures below must be 0. > > > > > + * Display buffers's cookie of value 0 treated as invalid. > > > > > + * Framebuffer's cookie of value 0 treated as invalid. > > > > > + * > > > > > + *---------------------------------- Requests --------------------------------- > > > > > + * > > > > > + * All requests/responses, which are not connector specific, must be sent over > > > > > + * control ring of the connector with index 0. > > > > > + * > > > > > + * For all request/response/event packets that use cookies: > > > > > + * dbuf_cookie - uint64_t, unique to guest domain value used by the backend > > > > > + * to map remote display buffer to its local one > > > > > + * fb_cookie - uint64_t, unique to guest domain value used by the backend > > > > > + * to map remote framebuffer to its local one > > > > > + * > > > > > + * All request packets have the same length (64 octets) > > > > > + * All request packets have common header: > > > > > + * 0 1 2 3 octet > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | id | operation | reserved | 4 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 8 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * id - uint16_t, private guest value, echoed in response > > > > > + * operation - uint8_t, operation code, XENDISPL_OP_??? > > > > > + * > > > > > + * Request dbuf creation - request creation of a display buffer. > > > > > + * 0 1 2 3 octet > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | id |_OP_DBUF_CREATE | reserved | 4 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 8 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | dbuf_cookie low 32-bit | 12 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | dbuf_cookie high 32-bit | 16 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | width | 20 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | height | 24 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | bpp | 28 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | buffer_sz | 32 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | flags | 36 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | gref_directory | 40 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 44 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 64 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * > > > > > + * Must be sent over control ring of the connector with index 0. > > > > > + * > > > > > + * width - uint32_t, width in pixels > > > > > + * height - uint32_t, height in pixels > > > > Why? Isn't that in the 'resolution' XenBus entry? > > > It can be bigger that the resolution, for example > > > if scaling will take place. > > How would you reflect the scaling part of this? Is that something > > that is assumed if the XenBus resolution value < widthxheight values? > As in the above example with back utilizing an SW compositor, > scaling will be done by the backend, front nows nothing > about the destiny of the buffers it wants to show > So, no need to worry about widthXheightXdepth of the buffers > > > > If not, why even have the 'resolution' field? > > > The confusion comes from the fact that the resolution is visible area: > > > will state that clearly. > > > > And also - what if the width or height are bigger than 'resolution'? > > > > Should -EINVAL be returned? > > > > > > > no problem here: we can have buffers bigger than visible area > > > > > + * bpp - uint32_t, bits per pixel > > > > > + * buffer_sz - uint32_t, buffer size to be allocated, octets > > > > > + * flags - uint32_t, flags of the operation > > > > > + * o XENDISPL_DBUF_FLG_REQ_ALLOC - if set, then backend is requested > > > > > + * to allocate the buffer with the parameters provided in this request. > > > > > + * Page directory is handled as follows: > > > > > + * Frontend on request: > > > > > + * o allocates pages for the directory > > > > > + * o grants permissions for the pages of the directory > > > > > + * o sets gref_dir_next_page fields > > > > > + * Backend on response: > > > > > + * o grants permissions for the pages of the buffer allocated > > > > <scratches his head> > > > > > + * o fills in page directory with grant references > > > > The backend provides the pages instead of the frontend? But you said > > > > 'allocated pages for the directory' in the frontned part - and since you > > > > say 'pages' that means more than one? Or did you mean 'the gref_directory' > > > > page and the 'gref_dir_next_page' pages? You may want to be explicit. > > > > > > > sure, I will state it clearly that only directory pages > > > are allocated: gref_directory + gref_dir_next_page(s) > > Thx > > > > And what if the 'flags' is zero? > > > it would mean that even that the backend can allocate buffers > > > for us we don't want it to do so, we will allocate on our own: > > > flag is not set > > > > Or 0x314? Perhaps also mention > > > > the default one (0 I presume) which would make the backend map > > > > the frontend pages provided in 'gref_directory'? > > > yes, I'll put a note on defaults > > Awesome! > > > > > + * gref_directory - grant_ref_t, a reference to the first shared page > > > > > + * describing shared buffer references. At least one page exists. If shared > > > > > + * buffer size (buffer_sz) exceeds what can be addressed by this single page, > > > > ^- extra space > > > done > > > > > + * then reference to the next page must be supplied (see gref_dir_next_page > > > > > + * below) > > > > > + */ > > > > What are some of the return errors ? Would it make sense to mention > > > > some? > > > not sure, this way will dictate implementation details, > > > but we are defining protocol > > True to an extent. > And that extent makes me believe we shouldn't dictate > error codes here. POSIX error codes are (sometimes :) > more than descriptive ) > > > > What to for example if there are multiple _OP_DBUF_CREATE > > > > with the same dbuf_cookie but with different width/height? > > > it will be an error, I will clearly state this: > > > * An attempt to create multiple display buffers with the same dbuf_cookie > > > is > > > * an error. dbuf_cookie can be re-used after destroying the corresponding > > > * display buffer. > > May also want to say what error value it should be. -EAGAIN? No probably something > > different. > As above, I would prefer to not put anything OK. > > > > > + > > > > > +#define XENDISPL_DBUF_FLG_REQ_ALLOC 0x0001 > > > > > + > > > > > +struct xendispl_dbuf_create_req { > > > > > + uint64_t dbuf_cookie; > > > > > + uint32_t width; > > > > > + uint32_t height; > > > > > + uint32_t bpp; > > > > > + uint32_t buffer_sz; > > > > > + uint32_t flags; > > > > > + grant_ref_t gref_directory; > > > > > +}; > > > > > + > > > > > +/* > > > > > + * Shared page for XENDISPL_OP_DBUF_CREATE 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 | 4 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | gref[0] | 8 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | gref[i] | i*4+8 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | gref[N - 1] | N*4+8 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * > > > > > + * gref_dir_next_page - grant_ref_t, reference to the next page describing > > > > > + * page directory. Must be 0 if there are no more pages in the list. > > > > > + * gref[i] - grant_ref_t, reference to a shared page of the buffer > > > > > + * allocated at XENDISPL_OP_DBUF_CREATE > > > > > + * > > > > > + * Number of grant_ref_t entries in the whole page directory is not > > > > > + * passed, but instead can be calculated as: > > > > > + * num_grefs_total = (XENDISPL_OP_DBUF_CREATE.buffer_sz + XEN_PAGE_SIZE - 1) / > > > > > + * XEN_PAGE_SIZE > > > > > + */ > > > > > + > > > > > +struct xendispl_page_directory { > > > > > + grant_ref_t gref_dir_next_page; > > > > > + grant_ref_t gref[1]; /* Variable length */ > > > > > +}; > > > > > + > > > > > +/* > > > > > + * Request dbuf destruction - destroy a previously allocated display buffer: > > > > > + * 0 1 2 3 octet > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | id |_OP_DBUF_DESTROY| reserved | 4 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 8 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | dbuf_cookie low 32-bit | 12 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | dbuf_cookie high 32-bit | 16 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 20 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 64 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * > > > > > + * Must be sent over control ring of the connector with index 0. > > > > > + */ > > > > > + > > > > > +struct xendispl_dbuf_destroy_req { > > > > > + uint64_t dbuf_cookie; > > > > > +}; > > > > > + > > > > > +/* > > > > > + * Request framebuffer attachment - request attachment of a framebuffer to > > > > > + * previously created display buffer. > > > > > + * 0 1 2 3 octet > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | id | _OP_FB_ATTACH | reserved | 4 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 8 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | dbuf_cookie low 32-bit | 12 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | dbuf_cookie high 32-bit | 16 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | fb_cookie low 32-bit | 20 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | fb_cookie high 32-bit | 24 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | width | 28 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | height | 32 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | pixel_format | 36 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 40 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 64 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * > > > > > + * Must be sent over control ring of the connector with index 0. > > > > > + * > > > > > + * width - uint32_t, width in pixels > > > > > + * height - uint32_t, height in pixels > > > > Why? The _OP_DBUF_CREATE already specified the height/width. Why have this? > > > > > > > > Or are you thinking to have an smaller (or bigger?) resolution framebuffer > > > > than the display? I suppose you can do that which would imply that the display > > > > would switch modes? Perhaps mention this? > > > > > > > > Also what if these values are bigger (or lesser) than 'resolution' ? > > > > > > > > > > > > > + * pixel_format - uint32_t, pixel format of the framebuffer, FOURCC code > > > > You did not mention the fb_cookie? > > > I have a common section: > > > * For all request/response/event packets that use cookies: > > > * dbuf_cookie - uint64_t, unique to guest domain value used by the > > > backend > > > * to map remote display buffer to its local one > > > * fb_cookie - uint64_t, unique to guest domain value used by the backend > > > * to map remote framebuffer to its local one > > > But it is in the "Requests" section - I'll pull it out to be common for all > > Aaaah! > > > > > + */ > > > > > + > > > > > +struct xendispl_fb_attach_req { > > > > > + uint64_t dbuf_cookie; > > > > > + uint64_t fb_cookie; > > > > > + uint32_t width; > > > > > + uint32_t height; > > > > > + uint32_t pixel_format; > > > > > +}; > > > > > + > > > > > +/* > > > > > + * Request framebuffer detach - detach a previously > > > > > + * attached framebuffer from the display buffer in request: > > > > > + * 0 1 2 3 octet > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | id | _OP_FB_DETACH | reserved | 4 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 8 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | fb_cookie low 32-bit | 12 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | fb_cookie high 32-bit | 16 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 20 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 64 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * > > > > > + * Must be sent over control ring of the connector with index 0. > > > > > + */ > > > > > + > > > > > +struct xendispl_fb_detach_req { > > > > > + uint64_t fb_cookie; > > > > > +}; > > > > > + > > > > > +/* > > > > > + * Request configuration set/reset - request to set or reset > > > > > + * the configuration/mode of the display: > > > > > + * 0 1 2 3 octet > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | id | _OP_SET_CONFIG | reserved | 4 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 8 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | fb_cookie low 32-bit | 12 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | fb_cookie high 32-bit | 16 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | x | 20 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | y | 24 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | width | 28 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | height | 32 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | bpp | 40 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 44 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 64 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * > > > > > + * Pass all zeros to reset, otherwise command is treated as > > > > > + * configuration set. > > > > All zeros? Including fb_cookie? > > > Yes, fb_cookie is 0 when we disable the display, as at this point there > > > could be no fb at all (all released by that time) > > > > > + * If this is a set configuration request then framebuffer's cookie tells > > > > Not sure if the first part of the sentence is needed. This is after all > > > > the section that talks about setting configurations. > > > ok, will remove it > > > > > + * the display which framebuffer/dbuf must be displayed while enabling display > > > > > + * (applying configuration). > > > > Can you disable an display without changing this? > > > You pass ALL zeros, including fb_cookie to disable/reset. > > > > > + * > > > > > + * x - uint32_t, starting position in pixels by X axis > > > > > + * y - uint32_t, starting position in pixels by Y axis > > > > Which is bound by 'resolution' ? Or by the display command? > > > these are limited by the "resolution" - will state that: > > > * x, y, width and height are bound by the connector resolution and must not > > > * exceed it. > > Thanks! > BTW, this is the only place ATM where we stick to resolution > from XenStore... And it will clearly be stated /me nods > > > > > + * width - uint32_t, width in pixels > > > > > + * height - uint32_t, height in pixels > > > > > + * bpp - uint32_t, bits per pixel > > > > Shouldn't there be an stride as well? > > > no, stride is kind of implementation detail which shouldn't > > > be in the protocol > > > > > + */ > > > > > + > > > > > +struct xendispl_set_config_req { > > > > > + uint64_t fb_cookie; > > > > > + uint32_t x; > > > > > + uint32_t y; > > > > > + uint32_t width; > > > > > + uint32_t height; > > > > > + uint32_t bpp; > > > > > +}; > > > > > + > > > > > +/* > > > > > + * Request page flip - request to flip a page identified by the framebuffer > > > > > + * cookie: > > > > > + * 0 1 2 3 octet > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | id | _OP_PG_FLIP | reserved | 4 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 8 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | fb_cookie low 32-bit | 12 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | fb_cookie high 32-bit | 16 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 20 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 64 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + */ > > > > > + > > > > > +struct xendispl_page_flip_req { > > > > > + uint64_t fb_cookie; > > > > > +}; > > > > > + > > > > > +/* > > > > > + *---------------------------------- Responses -------------------------------- > > > > > + * > > > > > + * All response packets have the same length (64 octets) > > > > > + * > > > > > + * All response packets have common header: > > > > > + * 0 1 2 3 octet > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | id | reserved | 4 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | status | 8 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 12 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 64 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * > > > > > + * id - uint16_t, private guest value, echoed from request > > > > > + * status - int32_t, response status, zero on success and -XEN_EXX on failure > > > > > + * > > > > > + *----------------------------------- Events ---------------------------------- > > > > > + * > > > > > + * Events are sent via a shared page allocated by the front and propagated by > > > > > + * event-channel/event-ring-ref XenStore entries > > > > > + * All event packets have the same length (64 octets) > > > > > + * All event packets have common header: > > > > > + * 0 1 2 3 octet > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | id | type | reserved | 4 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 8 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * > > > > > + * id - uint16_t, event id, may be used by front > > > > > + * type - uint8_t, type of the event > > > > > + * > > > > > + * > > > > > + * Page flip complete event - event from back to front on page flip completed: > > > > > + * 0 1 2 3 octet > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | id | _EVT_PG_FLIP | reserved | 4 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 8 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | fb_cookie low 32-bit | 12 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | fb_cookie high 32-bit | 16 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 20 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + * | reserved | 64 > > > > > + * +----------------+----------------+----------------+----------------+ > > > > > + */ > > > > > + > > > > > +struct xendispl_pg_flip_evt { > > > > > + uint64_t fb_cookie; > > > > > +}; > > > > > + > > > > > +struct xendispl_req { > > > > > + uint16_t id; > > > > > + uint8_t operation; > > > > > + uint8_t reserved[5]; > > > > > + union { > > > > > + struct xendispl_dbuf_create_req dbuf_create; > > > > > + struct xendispl_dbuf_destroy_req dbuf_destroy; > > > > > + struct xendispl_fb_attach_req fb_attach; > > > > > + struct xendispl_fb_detach_req fb_detach; > > > > > + struct xendispl_set_config_req set_config; > > > > > + struct xendispl_page_flip_req pg_flip; > > > > > + uint8_t reserved[56]; > > > > > + } op; > > > > > +}; > > > > > + > > > > > +struct xendispl_resp { > > > > > + uint16_t id; > > > > > + uint8_t operation; > > > > > + uint8_t reserved; > > > > > + int32_t status; > > > > > + uint8_t reserved1[56]; > > > > > +}; > > > > > + > > > > > +struct xendispl_evt { > > > > > + uint16_t id; > > > > > + uint8_t type; > > > > > + uint8_t reserved[5]; > > > > > + union { > > > > > + struct xendispl_pg_flip_evt pg_flip; > > > > > + uint8_t reserved[56]; > > > > > + } op; > > > > > +}; > > > > > + > > > > > +DEFINE_RING_TYPES(xen_displif, struct xendispl_req, struct xendispl_resp); > > > > > + > > > > > +/* > > > > > + ****************************************************************************** > > > > > + * Back to front events delivery > > > > > + ****************************************************************************** > > > > > + * In order to deliver asynchronous events from back to front a shared page is > > > > > + * allocated by front and its grefs propagated to back via XenStore entries > > > > > + * (event-XXX). > > > > ? > > > > > > > > You mean event-channel ? Or control-event-channel? > > > event-channel which is used for async events from back to front, > > > control is always used to pass req/resp. > > > > > + * This page has a common header used by both front and back to synchronize > > > > > + * access and control event's ring buffer, while back being a producer of the > > > > > + * events and front being a consumer. The rest of the page after the header > > > > > + * is used for event packets. > > > > > + * > > > > > + * Upon reception of an event(s) front may confirm its reception > > > > > + * for either each event, group of events or none. > > > > > + */ > > > > > + > > > > > +struct xendispl_event_page { > > > > > + uint32_t in_cons; > > > > > + uint32_t in_prod; > > > > > + uint8_t reserved[56]; > > > > > +}; > > > > > + > > > > > +#define XENDISPL_EVENT_PAGE_SIZE 4096 > > > > > +#define XENDISPL_IN_RING_OFFS (sizeof(struct xendispl_event_page)) > > > > > +#define XENDISPL_IN_RING_SIZE (XENDISPL_EVENT_PAGE_SIZE - XENDISPL_IN_RING_OFFS) > > > > > +#define XENDISPL_IN_RING_LEN (XENDISPL_IN_RING_SIZE / sizeof(struct xendispl_evt)) > > > > > +#define XENDISPL_IN_RING(page) \ > > > > > + ((struct xendispl_evt *)((char *)(page) + XENDISPL_IN_RING_OFFS)) > > > > > +#define XENDISPL_IN_RING_REF(page, idx) \ > > > > > + (XENDISPL_IN_RING((page))[(idx) % XENDISPL_IN_RING_LEN]) > > > > Why not use the ring.h macros? > > > well, this is the way the same structs are defined for fbif/kbif > > > and I am not sure what can be re-used here from ring.h as it is > > > designed to work with req/resp, not events. > > > > Especially as the top of this talks about "RING_FINAL_CHECK_FOR_REQUESTS"? > > > I can remove this > > > > > + > > > > > +#endif /* __XEN_PUBLIC_IO_DISPLIF_H__ */ > > > > Could we also have this header in: > > > sure, I will add the same to sndif as well > > > > /* > > > > * Local variables: > > > > * mode: C > > > > * c-file-style: "BSD" > > > > * c-basic-offset: 4 > > > > * tab-width: 4 > > > > * indent-tabs-mode: nil > > > > * End: > > > > */ > > > > > > > > please? > > > > > -- > > > > > 2.7.4 > > > > > > > > [1] http://marc.info/?l=xen-devel&m=142167153324052&w=4 > > > > Thank you, > Oleksandr
On 02/15/2017 05:45 PM, Konrad Rzeszutek Wilk wrote: > On Wed, Feb 15, 2017 at 09:33:41AM +0200, Oleksandr Andrushchenko wrote: >> On 02/14/2017 10:27 PM, Konrad Rzeszutek Wilk wrote: >>> On Mon, Feb 13, 2017 at 10:50:49AM +0200, Oleksandr Andrushchenko wrote: >>>> Hi, Konrad! >>>> >>>> Thank you for reviewing this. >>>> >>>> On 02/10/2017 11:27 PM, Konrad Rzeszutek Wilk wrote: >>>>> On Fri, Feb 10, 2017 at 09:29:58AM +0200, Oleksandr Andrushchenko wrote: >>>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>>>>> >>>>>> This is the ABI for the two halves of a para-virtualized >>>>>> display driver. >>>>>> >>>>>> This protocol aims to provide a unified protocol which fits more >>>>>> sophisticated use-cases than a framebuffer device can handle. At the >>>>>> moment basic functionality is supported with the intention to extend: >>>>>> o multiple dynamically allocated/destroyed framebuffers >>>>>> o buffers of arbitrary sizes >>>>>> o better configuration options including multiple display support >>>>>> >>>>>> Note: existing fbif can be used together with displif running at the >>>>>> same time, e.g. on Linux one provides framebuffer and another DRM/KMS >>>>>> >>>>>> Future extensions to the existing protocol may include: >>>>>> o allow display/connector cloning >>>>>> o allow allocating objects other than display buffers >>>>>> o add planes/overlays support >>>>>> o support scaling >>>>>> o support rotation >>>>>> >>>>>> ================================================== >>>>>> Rationale for introducing this protocol instead of >>>>>> using the existing fbif: >>>>>> ================================================== >>>>>> >>>>>> 1. In/out event sizes >>>>>> o fbif - 40 octets >>>>>> o displif - 40 octets >>>>>> This is only the initial version of the displif protocol >>>>>> which means that there could be requests which will not fit >>>>>> (WRT introducing some GPU related functionality >>>>>> later on). In that case we cannot alter fbif sizes as we need to >>>>>> be backward compatible an will be forced to handle those >>>>>> apart of fbif. >>>>>> >>>>>> 2. Shared page >>>>>> Displif doesn't use anything like struct xenfb_page, but >>>>>> DEFINE_RING_TYPES(xen_displif, struct xendispl_req, struct >>>>>> xendispl_resp) which is a better and more common way. >>>>>> Output events use a shared page which only has in_cons and in_prod >>>>>> and all the rest is used for incoming events. Here struct xenfb_page >>>>>> could probably be used as is despite the fact that it only has a half >>>>>> of a page for incoming events which is only 50 events. (consider >>>>>> something like 60Hz display) >>>>>> >>>>>> 3. Amount of changes. >>>>>> fbif only provides XENFB_TYPE_UPDATE and XENFB_TYPE_RESIZE >>>>>> events, so it looks like it is easier to get fb support into displif >>>>> .. would it make sense to reserve some of those values (2, 3) >>>>> in the XENDISPL_OP_ values? So that if this happens there is a nice >>>>> fit in there? Thought looking at the structure there is no easy >>>>> way to 'overlay' the xenfb_out_event structure as it is missing the 'id'. >>>>> >>>>> I guess one can get creative. >>>>> >>>>> Or you could swap positions of 'id' and 'type'? And then it would fit much >>>>> nicer? >>>> yeap, in order no one needs to be creative, why not >>>> explicitly define those? >>>> Anyways, it won't be possible to simply lay the structures from >>>> fbif on top of displif (different structure, size, workflow etc.), >>>> what is more that would give some room to find workarounds, rather than >>>> have well defined solution. BTW, there was an attempt [1] to update >>>> fbdev to meet modern application needs. If we decide to add FBDEV >>>> functionality into this protocol, then I can re-use already >>>> proven to work solution from [1] into DISPLIF, but defining >>>> structures and events to fit the current protocol. >>> I was thinking that since the size of the structure is 64bytes >>> you have enough space to jam in the old structures too. >> yes, I understand your intention >>> Naturally the driver would need adjustments as it offsets >>> of where this goes are all wrong. >> exactly >>>> What do you think? >> so, what is the decision? Options: >> 1) Leave the protocol as is, if need be it can be extended later >> 2) Implement something similar to [1] >> 3) Give room for workarounds and reserve XENDISPL_OP_XXX for >> what current fbif uses > I would do 3) so that it becomes easier to migrate the old code over. Ok, so I will reserve XENDISPL_OP_XXX to fit fbif, namely: I'll put XENDISPL_OP_RESERVED{0|1...} for those used by fbif >>>>>> than vice versa. displif at the moment has 6 requests and 1 event, >>>>>> multiple connector support, etc. >>>>>> >>>>>> Changes since v2: >>>>>> * updated XenStore configuration template/pattern >>>>>> * added "Recovery flow" to state diagram description >>>>>> * renamed gref_directory_start to gref_directory >>>>>> * added missing "versions" and "version" string constants >>>>>> >>>>>> Changes since v1: >>>>>> * fixed xendispl_event_page padding size >>>>>> * added versioning support >>>>>> * explicitly define value types for XenStore fields >>>>>> * text decoration re-work >>>>>> * added offsets to ASCII box notation >>>>>> >>>>>> Changes since initial: >>>>>> * DRM changed to DISPL, protocol made generic >>>>>> * major re-work addressing issues raised for sndif >>>>>> >>>>>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> >>>>>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>>>>> --- >>>>>> xen/include/public/io/displif.h | 778 ++++++++++++++++++++++++++++++++++++++++ >>>>>> 1 file changed, 778 insertions(+) >>>>>> create mode 100644 xen/include/public/io/displif.h >>>>>> >>>>>> diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h >>>>>> new file mode 100644 >>>>>> index 000000000000..849f27fe5f1d >>>>>> --- /dev/null >>>>>> +++ b/xen/include/public/io/displif.h >>>>>> @@ -0,0 +1,778 @@ >>>>>> +/****************************************************************************** >>>>>> + * displif.h >>>>>> + * >>>>>> + * Unified display 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) 2016-2017 EPAM Systems Inc. >>>>>> + * >>>>>> + * Authors: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>>>>> + * Oleksandr Grytsov <oleksandr_grytsov@epam.com> >>>>>> + */ >>>>>> + >>>>>> +#ifndef __XEN_PUBLIC_IO_DISPLIF_H__ >>>>>> +#define __XEN_PUBLIC_IO_DISPLIF_H__ >>>>>> + >>>>>> +#include "ring.h" >>>>>> +#include "../grant_table.h" >>>>>> + >>>>>> +/* >>>>>> + ****************************************************************************** >>>>>> + * Main features provided by the protocol >>>>>> + ****************************************************************************** >>>>>> + * This protocol aims to provide a unified protocol which fits more >>>>>> + * sophisticated use-cases than a framebuffer device can handle. At the >>>>>> + * moment basic functionality is supported with the intention to be extended: >>>>>> + * o multiple dynamically allocated/destroyed framebuffers >>>>>> + * o buffers of arbitrary sizes >>>>>> + * o buffer allocation at either back or front end >>>>>> + * o better configuration options including multiple display support >>>>>> + * >>>>>> + * Note: existing fbif can be used together with displif running at the >>>>>> + * same time, e.g. on Linux one provides framebuffer and another DRM/KMS >>>>>> + * >>>>>> + ****************************************************************************** >>>>>> + * Direction of improvements >>>>>> + ****************************************************************************** >>>>>> + * Future extensions to the existing protocol may include: >>>>>> + * o display/connector cloning >>>>>> + * o allocation of objects other than display buffers >>>>>> + * o plane/overlay support >>>>>> + * o scaling support >>>>>> + * o rotation support >>>>>> + * >>>>>> + ****************************************************************************** >>>>>> + * Feature and Parameter Negotiation >>>>>> + ****************************************************************************** >>>>>> + * >>>>>> + * Front->back notifications: when enqueuing a new request, sending a >>>>>> + * notification can be made conditional on xendispl_req (i.e., the generic >>>>>> + * hold-off mechanism provided by the ring macros). Backends must set >>>>>> + * xendispl_req 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 xendispl_resp (i.e., the generic >>>>>> + * hold-off mechanism provided by the ring macros). Frontends must set >>>>>> + * xendispl_resp appropriately (e.g., using RING_FINAL_CHECK_FOR_RESPONSES()). >>>>>> + * >>>>>> + * The two halves of a para-virtual display driver utilize nodes within >>>>>> + * XenStore to communicate capabilities and to negotiate operating parameters. >>>>>> + * This section enumerates these nodes which reside in the respective front and >>>>>> + * backend portions of XenStore, following the XenBus convention. >>>>>> + * >>>>>> + * All data in 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. >>>>>> + * >>>>>> + ****************************************************************************** >>>>>> + * Example configuration >>>>>> + ****************************************************************************** >>>>>> + * >>>>>> + * Note: depending on the use-case backend can expose more display connectors >>>>>> + * than the underlying HW physically has by employing SW graphics compositors >>>>>> + * >>>>>> + * This is an example of backend and frontend configuration: >>>>>> + * >>>>>> + *--------------------------------- Backend ----------------------------------- >>>>>> + * >>>>>> + * /local/domain/0/backend/vdispl/1/0/frontend-id = "1" >>>>>> + * /local/domain/0/backend/vdispl/1/0/frontend = "/local/domain/1/device/vdispl/0" >>>>>> + * /local/domain/0/backend/vdispl/1/0/state = "4" >>>>>> + * /local/domain/0/backend/vdispl/1/0/versions = "1,2" >>>>>> + * /local/domain/0/backend/vdispl/1/0/features = "be_alloc" >>>>>> + * >>>>>> + *--------------------------------- Frontend ---------------------------------- >>>>>> + * >>>>>> + * /local/domain/1/device/vdispl/0/backend-id = "0" >>>>>> + * /local/domain/1/device/vdispl/0/backend = "/local/domain/0/backend/vdispl/1/0" >>>>>> + * /local/domain/1/device/vdispl/0/state = "4" >>>>>> + * /local/domain/1/device/vdispl/0/version = "1" >>>>>> + * >>>>>> + *-------------------------- Connector 0 configuration ------------------------ >>>>>> + * >>>>>> + * /local/domain/1/device/vdispl/0/0/resolution = "1920x1080" >>>>>> + * /local/domain/1/device/vdispl/0/0/ctrl-ring-ref = "2832" >>>>>> + * /local/domain/1/device/vdispl/0/0/ctrl-channel = "15" >>>>>> + * /local/domain/1/device/vdispl/0/0/event-ring-ref = "387" >>>>>> + * /local/domain/1/device/vdispl/0/0/event-channel = "16" >>>>>> + * >>>>>> + *-------------------------- Connector 1 configuration ------------------------ >>>>>> + * >>>>>> + * /local/domain/1/device/vdispl/0/1/resolution = "800x600" >>>>>> + * /local/domain/1/device/vdispl/0/1/ctrl-ring-ref = "2833" >>>>>> + * /local/domain/1/device/vdispl/0/1/ctrl-channel = "17" >>>>>> + * /local/domain/1/device/vdispl/0/1/event-ring-ref = "388" >>>>>> + * /local/domain/1/device/vdispl/0/1/event-channel = "18" >>>>>> + * >>>>>> + ****************************************************************************** >>>>>> + * Backend XenBus Nodes >>>>>> + ****************************************************************************** >>>>>> + * >>>>>> + *----------------------------- Protocol version ------------------------------ >>>>>> + * >>>>>> + * versions >>>>>> + * Values: <string> >>>>>> + * >>>>>> + * List of XENDISPL_LIST_SEPARATOR separated protocol versions supported >>>>>> + * by the backend. For example "1,2,3". >>>>>> + * >>>>>> + * features >>>>>> + * Values: <list of strings> >>>>>> + * >>>>>> + * XENDISPL_LIST_SEPARATOR separated list of features >>>>>> + * XENDISPL_FEATURE_BE_??? that the backend supports. >>>>>> + * >>>>>> + * be_alloc >>>>>> + * Backend can be a buffer provider/allocator during >>>>>> + * XENDISPL_OP_DBUF_CREATE operation (see below for negotiation). >>>>>> + * >>>>>> + ****************************************************************************** >>>>>> + * Frontend XenBus Nodes >>>>>> + ****************************************************************************** >>>>>> + * >>>>>> + *-------------------------------- Addressing --------------------------------- >>>>>> + * >>>>>> + * dom-id >>>>>> + * Values: <uint16_t> >>>>>> + * >>>>>> + * Domain identifier. >>>>>> + * >>>>>> + * dev-id >>>>>> + * Values: <uint16_t> >>>>>> + * >>>>>> + * Device identifier. >>>>>> + * >>>>>> + * conn-idx >>>>>> + * Values: <uint8_t> >>>>>> + * >>>>>> + * Zero based contigous index of the connector. >>>>>> + * /local/domain/<dom-id>/device/vdispl/<dev-id>/<conn-idx>/... >>>>>> + * >>>>>> + *----------------------------- Protocol version ------------------------------ >>>>>> + * >>>>>> + * version >>>>>> + * Values: <string> >>>>>> + * >>>>>> + * Protocol version, chosen among the ones supported by the backend. >>>>>> + * >>>>>> + *----------------------------- Connector settings ---------------------------- >>>>>> + * >>>>>> + * resolution >>>>>> + * Values: <width, uint32_t>x<height, uint32_t> >>>>>> + * >>>>>> + * Width and height of the connector in pixels separated by >>>>>> + * XENDISPL_RESOLUTION_SEPARATOR. >>>>> Is there an default depth? >>>> no >>>>> 32? >>>> no defaults >>> Should we have a default? >>> >>> And perhaps also say that all the operations are bounded by these? >> not sure we need default here: this is *up to backend* >> what to do with *depths* and *resolutions* which are different >> from what HW display supports, e.g. those buffers >> can be just a part of the final composition. >> For example, backend utilizes a SW compositor which >> makes screen compositing with 1920x1080x32 (lets say, for >> example, this is the most suitable one for the GPU used >> and also this is used for a remote desktop - use-cases may >> differ) >> But, the real HW display is only 1200x800x24. So, once the >> composition is done, it will be downscaled/resampled by any >> means to what HW display wants. >> >> Hope, this better explains why buffers may differ from >> what "resolution" says and depth used: buffer is not >> exactly what you see finally, so it can be bigger, smaller >> or equal - all valid > OK, you may to include this explanation in the document. > Ok, I will make the description more suitable for the documentation and put it in >>>>> Or should that also be negotiated? >>>> frontend requests the desired value of it, backend provides it >>>> or rejects the request, so front will try other value. >>>> So, yes, this is kind of negotiation here (EINVAL based :) >>>> I will also put a note that "resolution" only defines visible area, >>>> thus display and frame buffer sizes can be different from this value >>> OK, which I suspect means that it can only be smaller, not larger. >> See above, no influence on buffer size >>>>>> + * >>>>>> + *------------------ Connector Request Transport Parameters ------------------- >>>>>> + * >>>>>> + * ctrl-channel >>>>> Please call it 'ctrl-event-channel' >>>> I will >>>>>> + * Values: <uint32_t> >>>>>> + * >>>>>> + * The identifier of the Xen connector's control event channel >>>>>> + * used to signal activity in the ring buffer. >>>>>> + * >>>>>> + * ctrl-ring-ref >>>>>> + * Values: <uint32_t> >>>>>> + * >>>>>> + * The Xen grant reference granting permission for the backend to map >>>>>> + * a sole page in a single page sized connector's control ring buffer. >>>>>> + * >>>>>> + * event-channel >>>>>> + * Values: <uint32_t> >>>>>> + * >>>>>> + * The identifier of the Xen connector's event channel >>>>>> + * used to signal activity in the ring buffer. >>>>>> + * >>>>>> + * event-ring-ref >>>>>> + * Values: <uint32_t> >>>>>> + * >>>>>> + * The Xen grant reference granting permission for the backend to map >>>>>> + * a sole page in a single page sized connector's event ring buffer. >>>>> So you are making two rings and two event channels - and it looks like the >>>>> operations are going via the control ring.. but what goes through the >>>>> the other ring buffer? >>>> Events from backend to frontend, currently only XENDISPL_EVT_PG_FLIP >>> Aaaaah! In that case I would change it from 'ctrl-' to >>> 'req-alloc' to match with. >> well, this is the control/command path, for that reason I >> still see its name as "ctrl/cmd-". >> What is more, I can agree to "req", but why "alloc"? >> This path is not only used for allocations, but configuration as well. >> In the future, I would expect other commands/control be passed >> over it. >> So, there is a command/control path ("ctrl-") and event path ("event-") >> which I believe do reflect their usage. > Isn't this tied to 'XENDISPL_FEATURE_BE_ALLOC' feature? If so I would > call it with that name in mind. No. XENDISPL_FEATURE_BE_ALLOC just says, that buffers can be allocated by the backend and shared with frontend. This is specifically for zero-copy use-case which may have some security concerns: front will map pages allocated by the back for such buffers. So, I would still call it either "ctrl-" or "cmd-" Please see description of the event channels/ring below >>>> I will define 2 sections: >>>> *------------------ Connector Request Transport Parameters >>>> ------------------- >>>> * >>>> * ctrl-event-channel >>>> * ctrl-ring-ref >>>> * >>>> *------------------- Connector Event Transport Parameters >>>> -------------------- >>>> * >>>> * event-channel >>>> * event-ring-ref >>>> >>>>> Or is the other ring buffer the one that is created via 'gref_directory' ? >>>> no >>>> At the bottom: >>>> * In order to deliver asynchronous events from back to front a shared page >>>> is >>>> * allocated by front and its gref propagated to back via XenStore entries >>>> * (event-XXX). >>> AAnd you may want to say this is guarded by REQ_ALLOC feature right? >> Not sure I understood you. Event path is totally independent >> from any feature, e.g. REQ_ALLOC. >> It just provides means to send async events >> from back to front, "page flip done" in my case. > <scratche his head> Why do you need a seperate ring to send > responses back? Why not use the same ring on which requests > were sent Ok, it seems we are not on the same page for rings/channels usage. Let me describe how those are used: 1. Command/control event channel and its corresponding ring are used to pass requests from front to back (XENDISPL_OP_XXX) and get responses from the back. These are synchronous, use macros from ring.h: ctrl-event-channel + ctrl-ring-ref I call them "ctrl-" because this way front controls back, or sends commands if you will. Maybe "cmd-" would fit better here? 2. Event channel - asynchronous path for the backend to signal activity to the frontend, currently used for "page flip done" event which is sent at some point of time after back has actually completed the page flip requested (so, before that the corresponding request was sent and response received, but operation didn't complete yet, instead it was scheduled) No macros exist for this use-case in ring.h (kbdif+fbif implement this on their own, so do I) These are: event-channel + event-ring-ref Probably here is the point from where confusion comes, naming. We can have something like "be-to-fe-event-channel" or anything else more cute and descriptive. Hope this explains the need for 2 paths >>>>> If so, wouldn't you need mulitple event channels? And also the size of those >>>>> rings is much much bigger. But it is not really a ring. It looks to hold >>>>> some opaque data? >>>>> >>>>> -ECONFUSED. >>>>>> + */ >>>>>> + >>>>>> +/* >>>>>> + ****************************************************************************** >>>>>> + * STATE DIAGRAMS >>>>>> + ****************************************************************************** >>>>>> + * >>>>>> + * Tool stack creates front and back state nodes with initial state >>>>>> + * XenbusStateInitialising. >>>>>> + * Tool stack creates and sets up frontend display configuration >>>>>> + * nodes per domain. >>>>>> + * >>>>>> + *-------------------------------- Normal flow -------------------------------- >>>>>> + * >>>>>> + * 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 >>>>>> + * connector. >>>>>> + * 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 display connectors >>>>>> + * as per configuration. >>>>>> + * | >>>>>> + * | >>>>>> + * V >>>>>> + * XenbusStateConnected >>>>>> + * >>>>>> + * XenbusStateUnknown >>>>>> + * XenbusStateClosed >>>>>> + * XenbusStateClosing >>>>>> + * o Remove virtual display device >>>>>> + * o Remove event channels >>>>>> + * | >>>>>> + * | >>>>>> + * V >>>>>> + * XenbusStateClosed >>>>>> + * >>>>>> + *------------------------------- Recovery flow ------------------------------- >>>>>> + * >>>>>> + * In case of frontend unrecoverable errors backend handles that as >>>>>> + * if frontend goes into the XenbusStateClosed state. >>>>>> + * >>>>>> + * In case of backend unrecoverable errors frontend tries removing >>>>>> + * the emulated device. If this is possible at the moment of error, >>>>> What emulated device? >>>> s/emulated/virtualized/g for all? >>> /me nods. >> done >>>>>> + * then frontend goes into the XenbusStateInitialising state and is ready for >>>>>> + * new connection with backend. If the emulated device is still in use and >>>>> emulated device? >>>>> >>>>>> + * cannot be removed, then frontend goes into the XenbusStateReconfiguring state >>>>> .. and what is done in XenbusStateReconfiguring state? >>>>> >>>>> I am assuming 'resolution' is examined? >>>>> >>>>>> + * until either the emulated device removed or backend initiates a new >>>>>> + * connection. On the emulated device removal frontend goes into the >>>>> emulated device? >>>> The whole idea here is to let frontend recover from backend failure. >>>> What happens when backend dies: frontend cannot send requests >>>> to the backend and thus cannot emulate (virtualize) display >>>> device anymore. There is no simple solution like disconnecting rings >>>> and waiting for the backend to re-connect, as display device, just like >>>> sound devices, does hold some state: configuration in use, number of display >>>> buffers, framebuffers, application state, framework "watchdogs" (like >>>> vertical blank time-outs and recovery at framework level) etc. >>>> So, in most cases, it would require frontend to implement complex recovery >>>> logic. Instead, what I suggest, that we go into some state, which tells that >>>> we are not yet ready to go into Initialized state and still need some time: >>>> at this time frontend will try to make sure no new clients from user-space >>>> are accepted, it allows the current client to exit gracefully by returning >>>> error codes etc. Once all the clients are gone frontend can reinitialize >>>> the virtualized device, e.g. DRM/KMS driver, and get into Initialized state >>>> again. >>> That is fine. >>>> Thus, I was thinking of XenbusStateReconfiguringstate as appropriate in this >>>> case >>> Right, but somebody has to move to this state. Who would do it? >> when backend dies its state changes to "closed". >> At this moment front tries to remove virtualized device >> and if it is possible/done, then it goes into "initialized" >> state. If not - "reconfiguring". >> So, you would ask how does the front go from "reconfiguring" >> into "initialized" state? This is OS/front specific, but: >> 1. the underlying framework, e.g. DRM/KMS, ALSA, provide >> a callback(s) to signal that the last client to the >> virtualized device has gone and the driver can be removed >> (equivalent to module's usage counter 0) >> 2. one can schedule a delayed work (timer/tasklet/workqueue) >> to periodically check if this is the right time to re-try >> the removal and remove >> >> In both cases, after the virtualized device has been removed we move >> into "initialized" state again and are ready for new connections >> with backend (if it arose from the dead :) >>> Would the >>> frontend have some form of timer to make sure that the backend is still >>> alive? And if it had died then move to Reconfiguring? >> There are at least 2 ways to understand if back is dead: >> 1. XenBus state change (back is closed) > .. If the backend does a nice shutdown.. hm, on Linux I can kill -9 backend and XenBus driver seems to be able to turn back's state into "closed" isn't this expected behavior? > >> 2. time-outs on requests from front to back >> > Right. >> Bottom line on the "recovery flow": what I describe could be put in >> the description as an example to clarify more this. >>>>>> + * XenbusStateInitialising state. >>>>>> + * >>>>>> + ****************************************************************************** >>>>>> + * REQUEST CODES >>>>>> + ****************************************************************************** >>>>>> + */ >>>>>> +#define XENDISPL_OP_DBUF_CREATE 0 >>>>>> +#define XENDISPL_OP_DBUF_DESTROY 1 >>>>>> +#define XENDISPL_OP_FB_ATTACH 2 >>>>>> +#define XENDISPL_OP_FB_DETACH 3 >>>>>> +#define XENDISPL_OP_SET_CONFIG 4 >>>>>> +#define XENDISPL_OP_PG_FLIP 5 >>>>>> + >>>>>> +/* >>>>>> + ****************************************************************************** >>>>>> + * EVENT CODES >>>>>> + ****************************************************************************** >>>>>> + */ >>>>>> +#define XENDISPL_EVT_PG_FLIP 0 >>>>>> + >>>>>> +/* >>>>>> + ****************************************************************************** >>>>>> + * XENSTORE FIELD AND PATH NAME STRINGS, HELPERS >>>>>> + ****************************************************************************** >>>>>> + */ >>>>>> +#define XENDISPL_DRIVER_NAME "vdispl" >>>>>> + >>>>>> +#define XENDISPL_LIST_SEPARATOR "," >>>>>> +#define XENDISPL_RESOLUTION_SEPARATOR "x" >>>>>> +/* Field names */ >>>>>> +#define XENDISPL_FIELD_BE_VERSIONS "versions" >>>>>> +#define XENDISPL_FIELD_FE_VERSION "version" >>>>>> +#define XENDISPL_FIELD_FEATURES "features" >>>>>> +#define XENDISPL_FIELD_CTRL_RING_REF "ctrl-ring-ref" >>>>>> +#define XENDISPL_FIELD_CTRL_CHANNEL "ctrl-channel" >>>>>> +#define XENDISPL_FIELD_EVT_RING_REF "event-ring-ref" >>>>>> +#define XENDISPL_FIELD_EVT_CHANNEL "event-channel" >>>>>> +#define XENDISPL_FIELD_RESOLUTION "resolution" >>>>>> + >>>>>> +#define XENDISPL_FEATURE_BE_ALLOC "be_alloc" >>>>>> + >>>>>> +/* >>>>>> + ****************************************************************************** >>>>>> + * STATUS RETURN CODES >>>>>> + ****************************************************************************** >>>>>> + * >>>>>> + * Status return code is zero on success and -XEN_EXX on failure. >>>>>> + * >>>>>> + ****************************************************************************** >>>>>> + * Assumptions >>>>>> + ****************************************************************************** >>>>>> + * o usage of grant reference 0 as invalid grant reference: >>>>>> + * grant reference 0 is valid, but never exposed to a PV driver, >>>>>> + * because of the fact it is already in use/reserved by the PV console. >>>>>> + * o all references in this document to page sizes must be treated >>>>>> + * as pages of size XEN_PAGE_SIZE unless otherwise noted. >>>>>> + * >>>>>> + ****************************************************************************** >>>>>> + * Description of the protocol between frontend and backend driver >>>>>> + ****************************************************************************** >>>>>> + * >>>>>> + * The two halves of a Para-virtual display driver communicate with >>>>>> + * each other using shared pages and event channels. >>>>>> + * Shared page contains a ring with request/response packets. >>>>>> + * >>>>>> + * All reserved fields in the structures below must be 0. >>>>>> + * Display buffers's cookie of value 0 treated as invalid. >>>>>> + * Framebuffer's cookie of value 0 treated as invalid. >>>>>> + * >>>>>> + *---------------------------------- Requests --------------------------------- >>>>>> + * >>>>>> + * All requests/responses, which are not connector specific, must be sent over >>>>>> + * control ring of the connector with index 0. >>>>>> + * >>>>>> + * For all request/response/event packets that use cookies: >>>>>> + * dbuf_cookie - uint64_t, unique to guest domain value used by the backend >>>>>> + * to map remote display buffer to its local one >>>>>> + * fb_cookie - uint64_t, unique to guest domain value used by the backend >>>>>> + * to map remote framebuffer to its local one >>>>>> + * >>>>>> + * All request packets have the same length (64 octets) >>>>>> + * All request packets have common header: >>>>>> + * 0 1 2 3 octet >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | id | operation | reserved | 4 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 8 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * id - uint16_t, private guest value, echoed in response >>>>>> + * operation - uint8_t, operation code, XENDISPL_OP_??? >>>>>> + * >>>>>> + * Request dbuf creation - request creation of a display buffer. >>>>>> + * 0 1 2 3 octet >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | id |_OP_DBUF_CREATE | reserved | 4 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 8 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | dbuf_cookie low 32-bit | 12 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | dbuf_cookie high 32-bit | 16 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | width | 20 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | height | 24 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | bpp | 28 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | buffer_sz | 32 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | flags | 36 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | gref_directory | 40 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 44 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 64 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * >>>>>> + * Must be sent over control ring of the connector with index 0. >>>>>> + * >>>>>> + * width - uint32_t, width in pixels >>>>>> + * height - uint32_t, height in pixels >>>>> Why? Isn't that in the 'resolution' XenBus entry? >>>> It can be bigger that the resolution, for example >>>> if scaling will take place. >>> How would you reflect the scaling part of this? Is that something >>> that is assumed if the XenBus resolution value < widthxheight values? >> As in the above example with back utilizing an SW compositor, >> scaling will be done by the backend, front nows nothing >> about the destiny of the buffers it wants to show >> So, no need to worry about widthXheightXdepth of the buffers >>>>> If not, why even have the 'resolution' field? >>>> The confusion comes from the fact that the resolution is visible area: >>>> will state that clearly. >>>>> And also - what if the width or height are bigger than 'resolution'? >>>>> Should -EINVAL be returned? >>>>> >>>> no problem here: we can have buffers bigger than visible area >>>>>> + * bpp - uint32_t, bits per pixel >>>>>> + * buffer_sz - uint32_t, buffer size to be allocated, octets >>>>>> + * flags - uint32_t, flags of the operation >>>>>> + * o XENDISPL_DBUF_FLG_REQ_ALLOC - if set, then backend is requested >>>>>> + * to allocate the buffer with the parameters provided in this request. >>>>>> + * Page directory is handled as follows: >>>>>> + * Frontend on request: >>>>>> + * o allocates pages for the directory >>>>>> + * o grants permissions for the pages of the directory >>>>>> + * o sets gref_dir_next_page fields >>>>>> + * Backend on response: >>>>>> + * o grants permissions for the pages of the buffer allocated >>>>> <scratches his head> >>>>>> + * o fills in page directory with grant references >>>>> The backend provides the pages instead of the frontend? But you said >>>>> 'allocated pages for the directory' in the frontned part - and since you >>>>> say 'pages' that means more than one? Or did you mean 'the gref_directory' >>>>> page and the 'gref_dir_next_page' pages? You may want to be explicit. >>>>> >>>> sure, I will state it clearly that only directory pages >>>> are allocated: gref_directory + gref_dir_next_page(s) >>> Thx >>>>> And what if the 'flags' is zero? >>>> it would mean that even that the backend can allocate buffers >>>> for us we don't want it to do so, we will allocate on our own: >>>> flag is not set >>>>> Or 0x314? Perhaps also mention >>>>> the default one (0 I presume) which would make the backend map >>>>> the frontend pages provided in 'gref_directory'? >>>> yes, I'll put a note on defaults >>> Awesome! >>>>>> + * gref_directory - grant_ref_t, a reference to the first shared page >>>>>> + * describing shared buffer references. At least one page exists. If shared >>>>>> + * buffer size (buffer_sz) exceeds what can be addressed by this single page, >>>>> ^- extra space >>>> done >>>>>> + * then reference to the next page must be supplied (see gref_dir_next_page >>>>>> + * below) >>>>>> + */ >>>>> What are some of the return errors ? Would it make sense to mention >>>>> some? >>>> not sure, this way will dictate implementation details, >>>> but we are defining protocol >>> True to an extent. >> And that extent makes me believe we shouldn't dictate >> error codes here. POSIX error codes are (sometimes :) >> more than descriptive ) >>>>> What to for example if there are multiple _OP_DBUF_CREATE >>>>> with the same dbuf_cookie but with different width/height? >>>> it will be an error, I will clearly state this: >>>> * An attempt to create multiple display buffers with the same dbuf_cookie >>>> is >>>> * an error. dbuf_cookie can be re-used after destroying the corresponding >>>> * display buffer. >>> May also want to say what error value it should be. -EAGAIN? No probably something >>> different. >> As above, I would prefer to not put anything > OK. >>>>>> + >>>>>> +#define XENDISPL_DBUF_FLG_REQ_ALLOC 0x0001 >>>>>> + >>>>>> +struct xendispl_dbuf_create_req { >>>>>> + uint64_t dbuf_cookie; >>>>>> + uint32_t width; >>>>>> + uint32_t height; >>>>>> + uint32_t bpp; >>>>>> + uint32_t buffer_sz; >>>>>> + uint32_t flags; >>>>>> + grant_ref_t gref_directory; >>>>>> +}; >>>>>> + >>>>>> +/* >>>>>> + * Shared page for XENDISPL_OP_DBUF_CREATE 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 | 4 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | gref[0] | 8 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | gref[i] | i*4+8 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | gref[N - 1] | N*4+8 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * >>>>>> + * gref_dir_next_page - grant_ref_t, reference to the next page describing >>>>>> + * page directory. Must be 0 if there are no more pages in the list. >>>>>> + * gref[i] - grant_ref_t, reference to a shared page of the buffer >>>>>> + * allocated at XENDISPL_OP_DBUF_CREATE >>>>>> + * >>>>>> + * Number of grant_ref_t entries in the whole page directory is not >>>>>> + * passed, but instead can be calculated as: >>>>>> + * num_grefs_total = (XENDISPL_OP_DBUF_CREATE.buffer_sz + XEN_PAGE_SIZE - 1) / >>>>>> + * XEN_PAGE_SIZE >>>>>> + */ >>>>>> + >>>>>> +struct xendispl_page_directory { >>>>>> + grant_ref_t gref_dir_next_page; >>>>>> + grant_ref_t gref[1]; /* Variable length */ >>>>>> +}; >>>>>> + >>>>>> +/* >>>>>> + * Request dbuf destruction - destroy a previously allocated display buffer: >>>>>> + * 0 1 2 3 octet >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | id |_OP_DBUF_DESTROY| reserved | 4 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 8 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | dbuf_cookie low 32-bit | 12 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | dbuf_cookie high 32-bit | 16 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 20 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 64 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * >>>>>> + * Must be sent over control ring of the connector with index 0. >>>>>> + */ >>>>>> + >>>>>> +struct xendispl_dbuf_destroy_req { >>>>>> + uint64_t dbuf_cookie; >>>>>> +}; >>>>>> + >>>>>> +/* >>>>>> + * Request framebuffer attachment - request attachment of a framebuffer to >>>>>> + * previously created display buffer. >>>>>> + * 0 1 2 3 octet >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | id | _OP_FB_ATTACH | reserved | 4 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 8 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | dbuf_cookie low 32-bit | 12 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | dbuf_cookie high 32-bit | 16 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | fb_cookie low 32-bit | 20 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | fb_cookie high 32-bit | 24 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | width | 28 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | height | 32 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | pixel_format | 36 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 40 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 64 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * >>>>>> + * Must be sent over control ring of the connector with index 0. >>>>>> + * >>>>>> + * width - uint32_t, width in pixels >>>>>> + * height - uint32_t, height in pixels >>>>> Why? The _OP_DBUF_CREATE already specified the height/width. Why have this? >>>>> >>>>> Or are you thinking to have an smaller (or bigger?) resolution framebuffer >>>>> than the display? I suppose you can do that which would imply that the display >>>>> would switch modes? Perhaps mention this? >>>>> >>>>> Also what if these values are bigger (or lesser) than 'resolution' ? >>>>> >>>>> >>>>>> + * pixel_format - uint32_t, pixel format of the framebuffer, FOURCC code >>>>> You did not mention the fb_cookie? >>>> I have a common section: >>>> * For all request/response/event packets that use cookies: >>>> * dbuf_cookie - uint64_t, unique to guest domain value used by the >>>> backend >>>> * to map remote display buffer to its local one >>>> * fb_cookie - uint64_t, unique to guest domain value used by the backend >>>> * to map remote framebuffer to its local one >>>> But it is in the "Requests" section - I'll pull it out to be common for all >>> Aaaah! >>>>>> + */ >>>>>> + >>>>>> +struct xendispl_fb_attach_req { >>>>>> + uint64_t dbuf_cookie; >>>>>> + uint64_t fb_cookie; >>>>>> + uint32_t width; >>>>>> + uint32_t height; >>>>>> + uint32_t pixel_format; >>>>>> +}; >>>>>> + >>>>>> +/* >>>>>> + * Request framebuffer detach - detach a previously >>>>>> + * attached framebuffer from the display buffer in request: >>>>>> + * 0 1 2 3 octet >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | id | _OP_FB_DETACH | reserved | 4 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 8 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | fb_cookie low 32-bit | 12 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | fb_cookie high 32-bit | 16 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 20 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 64 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * >>>>>> + * Must be sent over control ring of the connector with index 0. >>>>>> + */ >>>>>> + >>>>>> +struct xendispl_fb_detach_req { >>>>>> + uint64_t fb_cookie; >>>>>> +}; >>>>>> + >>>>>> +/* >>>>>> + * Request configuration set/reset - request to set or reset >>>>>> + * the configuration/mode of the display: >>>>>> + * 0 1 2 3 octet >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | id | _OP_SET_CONFIG | reserved | 4 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 8 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | fb_cookie low 32-bit | 12 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | fb_cookie high 32-bit | 16 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | x | 20 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | y | 24 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | width | 28 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | height | 32 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | bpp | 40 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 44 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 64 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * >>>>>> + * Pass all zeros to reset, otherwise command is treated as >>>>>> + * configuration set. >>>>> All zeros? Including fb_cookie? >>>> Yes, fb_cookie is 0 when we disable the display, as at this point there >>>> could be no fb at all (all released by that time) >>>>>> + * If this is a set configuration request then framebuffer's cookie tells >>>>> Not sure if the first part of the sentence is needed. This is after all >>>>> the section that talks about setting configurations. >>>> ok, will remove it >>>>>> + * the display which framebuffer/dbuf must be displayed while enabling display >>>>>> + * (applying configuration). >>>>> Can you disable an display without changing this? >>>> You pass ALL zeros, including fb_cookie to disable/reset. >>>>>> + * >>>>>> + * x - uint32_t, starting position in pixels by X axis >>>>>> + * y - uint32_t, starting position in pixels by Y axis >>>>> Which is bound by 'resolution' ? Or by the display command? >>>> these are limited by the "resolution" - will state that: >>>> * x, y, width and height are bound by the connector resolution and must not >>>> * exceed it. >>> Thanks! >> BTW, this is the only place ATM where we stick to resolution >> from XenStore... And it will clearly be stated > /me nods >>>>>> + * width - uint32_t, width in pixels >>>>>> + * height - uint32_t, height in pixels >>>>>> + * bpp - uint32_t, bits per pixel >>>>> Shouldn't there be an stride as well? >>>> no, stride is kind of implementation detail which shouldn't >>>> be in the protocol >>>>>> + */ >>>>>> + >>>>>> +struct xendispl_set_config_req { >>>>>> + uint64_t fb_cookie; >>>>>> + uint32_t x; >>>>>> + uint32_t y; >>>>>> + uint32_t width; >>>>>> + uint32_t height; >>>>>> + uint32_t bpp; >>>>>> +}; >>>>>> + >>>>>> +/* >>>>>> + * Request page flip - request to flip a page identified by the framebuffer >>>>>> + * cookie: >>>>>> + * 0 1 2 3 octet >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | id | _OP_PG_FLIP | reserved | 4 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 8 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | fb_cookie low 32-bit | 12 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | fb_cookie high 32-bit | 16 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 20 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 64 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + */ >>>>>> + >>>>>> +struct xendispl_page_flip_req { >>>>>> + uint64_t fb_cookie; >>>>>> +}; >>>>>> + >>>>>> +/* >>>>>> + *---------------------------------- Responses -------------------------------- >>>>>> + * >>>>>> + * All response packets have the same length (64 octets) >>>>>> + * >>>>>> + * All response packets have common header: >>>>>> + * 0 1 2 3 octet >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | id | reserved | 4 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | status | 8 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 12 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 64 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * >>>>>> + * id - uint16_t, private guest value, echoed from request >>>>>> + * status - int32_t, response status, zero on success and -XEN_EXX on failure >>>>>> + * >>>>>> + *----------------------------------- Events ---------------------------------- >>>>>> + * >>>>>> + * Events are sent via a shared page allocated by the front and propagated by >>>>>> + * event-channel/event-ring-ref XenStore entries >>>>>> + * All event packets have the same length (64 octets) >>>>>> + * All event packets have common header: >>>>>> + * 0 1 2 3 octet >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | id | type | reserved | 4 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 8 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * >>>>>> + * id - uint16_t, event id, may be used by front >>>>>> + * type - uint8_t, type of the event >>>>>> + * >>>>>> + * >>>>>> + * Page flip complete event - event from back to front on page flip completed: >>>>>> + * 0 1 2 3 octet >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | id | _EVT_PG_FLIP | reserved | 4 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 8 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | fb_cookie low 32-bit | 12 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | fb_cookie high 32-bit | 16 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 20 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + * | reserved | 64 >>>>>> + * +----------------+----------------+----------------+----------------+ >>>>>> + */ >>>>>> + >>>>>> +struct xendispl_pg_flip_evt { >>>>>> + uint64_t fb_cookie; >>>>>> +}; >>>>>> + >>>>>> +struct xendispl_req { >>>>>> + uint16_t id; >>>>>> + uint8_t operation; >>>>>> + uint8_t reserved[5]; >>>>>> + union { >>>>>> + struct xendispl_dbuf_create_req dbuf_create; >>>>>> + struct xendispl_dbuf_destroy_req dbuf_destroy; >>>>>> + struct xendispl_fb_attach_req fb_attach; >>>>>> + struct xendispl_fb_detach_req fb_detach; >>>>>> + struct xendispl_set_config_req set_config; >>>>>> + struct xendispl_page_flip_req pg_flip; >>>>>> + uint8_t reserved[56]; >>>>>> + } op; >>>>>> +}; >>>>>> + >>>>>> +struct xendispl_resp { >>>>>> + uint16_t id; >>>>>> + uint8_t operation; >>>>>> + uint8_t reserved; >>>>>> + int32_t status; >>>>>> + uint8_t reserved1[56]; >>>>>> +}; >>>>>> + >>>>>> +struct xendispl_evt { >>>>>> + uint16_t id; >>>>>> + uint8_t type; >>>>>> + uint8_t reserved[5]; >>>>>> + union { >>>>>> + struct xendispl_pg_flip_evt pg_flip; >>>>>> + uint8_t reserved[56]; >>>>>> + } op; >>>>>> +}; >>>>>> + >>>>>> +DEFINE_RING_TYPES(xen_displif, struct xendispl_req, struct xendispl_resp); >>>>>> + >>>>>> +/* >>>>>> + ****************************************************************************** >>>>>> + * Back to front events delivery >>>>>> + ****************************************************************************** >>>>>> + * In order to deliver asynchronous events from back to front a shared page is >>>>>> + * allocated by front and its grefs propagated to back via XenStore entries >>>>>> + * (event-XXX). >>>>> ? >>>>> >>>>> You mean event-channel ? Or control-event-channel? >>>> event-channel which is used for async events from back to front, >>>> control is always used to pass req/resp. >>>>>> + * This page has a common header used by both front and back to synchronize >>>>>> + * access and control event's ring buffer, while back being a producer of the >>>>>> + * events and front being a consumer. The rest of the page after the header >>>>>> + * is used for event packets. >>>>>> + * >>>>>> + * Upon reception of an event(s) front may confirm its reception >>>>>> + * for either each event, group of events or none. >>>>>> + */ >>>>>> + >>>>>> +struct xendispl_event_page { >>>>>> + uint32_t in_cons; >>>>>> + uint32_t in_prod; >>>>>> + uint8_t reserved[56]; >>>>>> +}; >>>>>> + >>>>>> +#define XENDISPL_EVENT_PAGE_SIZE 4096 >>>>>> +#define XENDISPL_IN_RING_OFFS (sizeof(struct xendispl_event_page)) >>>>>> +#define XENDISPL_IN_RING_SIZE (XENDISPL_EVENT_PAGE_SIZE - XENDISPL_IN_RING_OFFS) >>>>>> +#define XENDISPL_IN_RING_LEN (XENDISPL_IN_RING_SIZE / sizeof(struct xendispl_evt)) >>>>>> +#define XENDISPL_IN_RING(page) \ >>>>>> + ((struct xendispl_evt *)((char *)(page) + XENDISPL_IN_RING_OFFS)) >>>>>> +#define XENDISPL_IN_RING_REF(page, idx) \ >>>>>> + (XENDISPL_IN_RING((page))[(idx) % XENDISPL_IN_RING_LEN]) >>>>> Why not use the ring.h macros? >>>> well, this is the way the same structs are defined for fbif/kbif >>>> and I am not sure what can be re-used here from ring.h as it is >>>> designed to work with req/resp, not events. >>>>> Especially as the top of this talks about "RING_FINAL_CHECK_FOR_REQUESTS"? >>>> I can remove this >>>>>> + >>>>>> +#endif /* __XEN_PUBLIC_IO_DISPLIF_H__ */ >>>>> Could we also have this header in: >>>> sure, I will add the same to sndif as well >>>>> /* >>>>> * Local variables: >>>>> * mode: C >>>>> * c-file-style: "BSD" >>>>> * c-basic-offset: 4 >>>>> * tab-width: 4 >>>>> * indent-tabs-mode: nil >>>>> * End: >>>>> */ >>>>> >>>>> please? >>>>>> -- >>>>>> 2.7.4 >>>>>> >>>> [1] http://marc.info/?l=xen-devel&m=142167153324052&w=4 >>>> >> Thank you, >> Oleksandr
.snip.. > > > > > I will define 2 sections: > > > > > *------------------ Connector Request Transport Parameters > > > > > ------------------- > > > > > * > > > > > * ctrl-event-channel > > > > > * ctrl-ring-ref > > > > > * > > > > > *------------------- Connector Event Transport Parameters > > > > > -------------------- > > > > > * > > > > > * event-channel > > > > > * event-ring-ref > > > > > > > > > > > Or is the other ring buffer the one that is created via 'gref_directory' ? > > > > > no > > > > > At the bottom: > > > > > * In order to deliver asynchronous events from back to front a shared page > > > > > is > > > > > * allocated by front and its gref propagated to back via XenStore entries > > > > > * (event-XXX). > > > > AAnd you may want to say this is guarded by REQ_ALLOC feature right? > > > Not sure I understood you. Event path is totally independent > > > from any feature, e.g. REQ_ALLOC. > > > It just provides means to send async events > > > from back to front, "page flip done" in my case. > > <scratche his head> Why do you need a seperate ring to send > > responses back? Why not use the same ring on which requests > > were sent > Ok, it seems we are not on the same page for rings/channels usage. > Let me describe how those are used: > > 1. Command/control event channel and its corresponding ring are used > to pass requests from front to back (XENDISPL_OP_XXX) and get responses > from the back. These are synchronous, use macros from ring.h: > ctrl-event-channel + ctrl-ring-ref > I call them "ctrl-" because this way front controls back, or sends commands > if you will. Maybe "cmd-" would fit better here? > > 2. Event channel - asynchronous path for the backend to signal activity > to the frontend, currently used for "page flip done" event which is sent > at some point of time after back has actually completed the page flip > requested > (so, before that the corresponding request was sent and response received, > but > operation didn't complete yet, instead it was scheduled) > No macros exist for this use-case in ring.h (kbdif+fbif implement > this on their own, so do I) > These are: event-channel + event-ring-ref > Probably here is the point from where confusion comes, naming. > We can have something like "be-to-fe-event-channel" or anything else > more cute and descriptive. > > Hope this explains the need for 2 paths Aha! So this is like the network where there is an 'rx' and 'tx'! Now I get it. In that case why not just prefix it with 'in' and 'out'? Such as: 'out-ring-ref' and 'out-event-channel' and 'in-ring-ref' along with 'in-event-channel'. Or perhaps better - borrow the same idea that Stefano came up for 9pfs and PV calls - where his ring does both. Then you just need 'ring-ref', 'event-channel', 'max-page-ring-order' (which must be 1 or larger). And you split the ring-ref in two - one for 'in' events and the other part for 'out' events? ..giant snip.. > > > > > Thus, I was thinking of XenbusStateReconfiguringstate as appropriate in this > > > > > case > > > > Right, but somebody has to move to this state. Who would do it? > > > when backend dies its state changes to "closed". > > > At this moment front tries to remove virtualized device > > > and if it is possible/done, then it goes into "initialized" > > > state. If not - "reconfiguring". > > > So, you would ask how does the front go from "reconfiguring" > > > into "initialized" state? This is OS/front specific, but: > > > 1. the underlying framework, e.g. DRM/KMS, ALSA, provide > > > a callback(s) to signal that the last client to the > > > virtualized device has gone and the driver can be removed > > > (equivalent to module's usage counter 0) > > > 2. one can schedule a delayed work (timer/tasklet/workqueue) > > > to periodically check if this is the right time to re-try > > > the removal and remove > > > > > > In both cases, after the virtualized device has been removed we move > > > into "initialized" state again and are ready for new connections > > > with backend (if it arose from the dead :) > > > > Would the > > > > frontend have some form of timer to make sure that the backend is still > > > > alive? And if it had died then move to Reconfiguring? > > > There are at least 2 ways to understand if back is dead: > > > 1. XenBus state change (back is closed) > > .. If the backend does a nice shutdown.. > hm, on Linux I can kill -9 backend and XenBus driver seems > to be able to turn back's state into "closed" > isn't this expected behavior? That is the expected behavior. I was thinking more of a backend being a guest - and the guest completly going away and nobody clearing its XenStore keys. In which case your second option of doing a timeout will work. But you may need an 'PING' type request to figure this out?
================================================== Rationale for introducing this protocol instead of using the existing fbif: ================================================== 1. In/out event sizes o fbif - 40 octets o displif - 40 octets This is only the initial version of the displif protocol which means that there could be requests which will not fit (WRT introducing some GPU related functionality later on). In that case we cannot alter fbif sizes as we need to be backward compatible an will be forced to handle those apart of fbif. 2. Shared page Displif doesn't use anything like struct xenfb_page, but DEFINE_RING_TYPES(xen_displif, struct xendispl_req, struct xendispl_resp) which is a better and more common way. Output events use a shared page which only has in_cons and in_prod and all the rest is used for incoming events. Here struct xenfb_page could probably be used as is despite the fact that it only has a half of a page for incoming events which is only 50 events. (consider something like 60Hz display) 3. Amount of changes. fbif only provides XENFB_TYPE_UPDATE and XENFB_TYPE_RESIZE events, so it looks like it is easier to get fb support into displif than vice versa. displif at the moment has 6 requests and 1 event, multiple connector support, etc. Changes since v2: * updated XenStore configuration template/pattern * added "Recovery flow" to state diagram description * renamed gref_directory_start to gref_directory * added missing "versions" and "version" string constants Changes since v1: * fixed xendispl_event_page padding size * added versioning support * explicitly define value types for XenStore fields * text decoration re-work * added offsets to ASCII box notation Changes since initial: * DRM changed to DISPL, protocol made generic * major re-work addressing issues raised for sndif Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> --- xen/include/public/io/displif.h | 778 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 778 insertions(+) create mode 100644 xen/include/public/io/displif.h diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h new file mode 100644 index 000000000000..849f27fe5f1d --- /dev/null +++ b/xen/include/public/io/displif.h @@ -0,0 +1,778 @@ +/****************************************************************************** + * displif.h + * + * Unified display 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) 2016-2017 EPAM Systems Inc. + * + * Authors: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> + * Oleksandr Grytsov <oleksandr_grytsov@epam.com> + */ + +#ifndef __XEN_PUBLIC_IO_DISPLIF_H__ +#define __XEN_PUBLIC_IO_DISPLIF_H__ + +#include "ring.h" +#include "../grant_table.h" + +/* + ****************************************************************************** + * Main features provided by the protocol + ****************************************************************************** + * This protocol aims to provide a unified protocol which fits more + * sophisticated use-cases than a framebuffer device can handle. At the + * moment basic functionality is supported with the intention to be extended: + * o multiple dynamically allocated/destroyed framebuffers + * o buffers of arbitrary sizes + * o buffer allocation at either back or front end + * o better configuration options including multiple display support + * + * Note: existing fbif can be used together with displif running at the + * same time, e.g. on Linux one provides framebuffer and another DRM/KMS + * + ****************************************************************************** + * Direction of improvements + ****************************************************************************** + * Future extensions to the existing protocol may include: + * o display/connector cloning + * o allocation of objects other than display buffers + * o plane/overlay support + * o scaling support + * o rotation support + * + ****************************************************************************** + * Feature and Parameter Negotiation + ****************************************************************************** + * + * Front->back notifications: when enqueuing a new request, sending a + * notification can be made conditional on xendispl_req (i.e., the generic + * hold-off mechanism provided by the ring macros). Backends must set + * xendispl_req 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 xendispl_resp (i.e., the generic + * hold-off mechanism provided by the ring macros). Frontends must set + * xendispl_resp appropriately (e.g., using RING_FINAL_CHECK_FOR_RESPONSES()). + * + * The two halves of a para-virtual display driver utilize nodes within + * XenStore to communicate capabilities and to negotiate operating parameters. + * This section enumerates these nodes which reside in the respective front and + * backend portions of XenStore, following the XenBus convention. + * + * All data in 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. + * + ****************************************************************************** + * Example configuration + ****************************************************************************** + * + * Note: depending on the use-case backend can expose more display connectors + * than the underlying HW physically has by employing SW graphics compositors + * + * This is an example of backend and frontend configuration: + * + *--------------------------------- Backend ----------------------------------- + * + * /local/domain/0/backend/vdispl/1/0/frontend-id = "1" + * /local/domain/0/backend/vdispl/1/0/frontend = "/local/domain/1/device/vdispl/0" + * /local/domain/0/backend/vdispl/1/0/state = "4" + * /local/domain/0/backend/vdispl/1/0/versions = "1,2" + * /local/domain/0/backend/vdispl/1/0/features = "be_alloc" + * + *--------------------------------- Frontend ---------------------------------- + * + * /local/domain/1/device/vdispl/0/backend-id = "0" + * /local/domain/1/device/vdispl/0/backend = "/local/domain/0/backend/vdispl/1/0" + * /local/domain/1/device/vdispl/0/state = "4" + * /local/domain/1/device/vdispl/0/version = "1" + * + *-------------------------- Connector 0 configuration ------------------------ + * + * /local/domain/1/device/vdispl/0/0/resolution = "1920x1080" + * /local/domain/1/device/vdispl/0/0/ctrl-ring-ref = "2832" + * /local/domain/1/device/vdispl/0/0/ctrl-channel = "15" + * /local/domain/1/device/vdispl/0/0/event-ring-ref = "387" + * /local/domain/1/device/vdispl/0/0/event-channel = "16" + * + *-------------------------- Connector 1 configuration ------------------------ + * + * /local/domain/1/device/vdispl/0/1/resolution = "800x600" + * /local/domain/1/device/vdispl/0/1/ctrl-ring-ref = "2833" + * /local/domain/1/device/vdispl/0/1/ctrl-channel = "17" + * /local/domain/1/device/vdispl/0/1/event-ring-ref = "388" + * /local/domain/1/device/vdispl/0/1/event-channel = "18" + * + ****************************************************************************** + * Backend XenBus Nodes + ****************************************************************************** + * + *----------------------------- Protocol version ------------------------------ + * + * versions + * Values: <string> + * + * List of XENDISPL_LIST_SEPARATOR separated protocol versions supported + * by the backend. For example "1,2,3". + * + * features + * Values: <list of strings> + * + * XENDISPL_LIST_SEPARATOR separated list of features + * XENDISPL_FEATURE_BE_??? that the backend supports. + * + * be_alloc + * Backend can be a buffer provider/allocator during + * XENDISPL_OP_DBUF_CREATE operation (see below for negotiation). + * + ****************************************************************************** + * Frontend XenBus Nodes + ****************************************************************************** + * + *-------------------------------- Addressing --------------------------------- + * + * dom-id + * Values: <uint16_t> + * + * Domain identifier. + * + * dev-id + * Values: <uint16_t> + * + * Device identifier. + * + * conn-idx + * Values: <uint8_t> + * + * Zero based contigous index of the connector. + * /local/domain/<dom-id>/device/vdispl/<dev-id>/<conn-idx>/... + * + *----------------------------- Protocol version ------------------------------ + * + * version + * Values: <string> + * + * Protocol version, chosen among the ones supported by the backend. + * + *----------------------------- Connector settings ---------------------------- + * + * resolution + * Values: <width, uint32_t>x<height, uint32_t> + * + * Width and height of the connector in pixels separated by + * XENDISPL_RESOLUTION_SEPARATOR. + * + *------------------ Connector Request Transport Parameters ------------------- + * + * ctrl-channel + * Values: <uint32_t> + * + * The identifier of the Xen connector's control event channel + * used to signal activity in the ring buffer. + * + * ctrl-ring-ref + * Values: <uint32_t> + * + * The Xen grant reference granting permission for the backend to map + * a sole page in a single page sized connector's control ring buffer. + * + * event-channel + * Values: <uint32_t> + * + * The identifier of the Xen connector's event channel + * used to signal activity in the ring buffer. + * + * event-ring-ref + * Values: <uint32_t> + * + * The Xen grant reference granting permission for the backend to map + * a sole page in a single page sized connector's event ring buffer. + */ + +/* + ****************************************************************************** + * STATE DIAGRAMS + ****************************************************************************** + * + * Tool stack creates front and back state nodes with initial state + * XenbusStateInitialising. + * Tool stack creates and sets up frontend display configuration + * nodes per domain. + * + *-------------------------------- Normal flow -------------------------------- + * + * 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 + * connector. + * 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 display connectors + * as per configuration. + * | + * | + * V + * XenbusStateConnected + * + * XenbusStateUnknown + * XenbusStateClosed + * XenbusStateClosing + * o Remove virtual display device + * o Remove event channels + * | + * | + * V + * XenbusStateClosed + * + *------------------------------- Recovery flow ------------------------------- + * + * In case of frontend unrecoverable errors backend handles that as + * if frontend goes into the XenbusStateClosed state. + * + * In case of backend unrecoverable errors frontend tries removing + * the emulated device. If this is possible at the moment of error, + * then frontend goes into the XenbusStateInitialising state and is ready for + * new connection with backend. If the emulated device is still in use and + * cannot be removed, then frontend goes into the XenbusStateReconfiguring state + * until either the emulated device removed or backend initiates a new + * connection. On the emulated device removal frontend goes into the + * XenbusStateInitialising state. + * + ****************************************************************************** + * REQUEST CODES + ****************************************************************************** + */ +#define XENDISPL_OP_DBUF_CREATE 0 +#define XENDISPL_OP_DBUF_DESTROY 1 +#define XENDISPL_OP_FB_ATTACH 2 +#define XENDISPL_OP_FB_DETACH 3 +#define XENDISPL_OP_SET_CONFIG 4 +#define XENDISPL_OP_PG_FLIP 5 + +/* + ****************************************************************************** + * EVENT CODES + ****************************************************************************** + */ +#define XENDISPL_EVT_PG_FLIP 0 + +/* + ****************************************************************************** + * XENSTORE FIELD AND PATH NAME STRINGS, HELPERS + ****************************************************************************** + */ +#define XENDISPL_DRIVER_NAME "vdispl" + +#define XENDISPL_LIST_SEPARATOR "," +#define XENDISPL_RESOLUTION_SEPARATOR "x" +/* Field names */ +#define XENDISPL_FIELD_BE_VERSIONS "versions" +#define XENDISPL_FIELD_FE_VERSION "version" +#define XENDISPL_FIELD_FEATURES "features" +#define XENDISPL_FIELD_CTRL_RING_REF "ctrl-ring-ref" +#define XENDISPL_FIELD_CTRL_CHANNEL "ctrl-channel" +#define XENDISPL_FIELD_EVT_RING_REF "event-ring-ref" +#define XENDISPL_FIELD_EVT_CHANNEL "event-channel" +#define XENDISPL_FIELD_RESOLUTION "resolution" + +#define XENDISPL_FEATURE_BE_ALLOC "be_alloc" + +/* + ****************************************************************************** + * STATUS RETURN CODES + ****************************************************************************** + * + * Status return code is zero on success and -XEN_EXX on failure. + * + ****************************************************************************** + * Assumptions + ****************************************************************************** + * o usage of grant reference 0 as invalid grant reference: + * grant reference 0 is valid, but never exposed to a PV driver, + * because of the fact it is already in use/reserved by the PV console. + * o all references in this document to page sizes must be treated + * as pages of size XEN_PAGE_SIZE unless otherwise noted. + * + ****************************************************************************** + * Description of the protocol between frontend and backend driver + ****************************************************************************** + * + * The two halves of a Para-virtual display driver communicate with + * each other using shared pages and event channels. + * Shared page contains a ring with request/response packets. + * + * All reserved fields in the structures below must be 0. + * Display buffers's cookie of value 0 treated as invalid. + * Framebuffer's cookie of value 0 treated as invalid. + * + *---------------------------------- Requests --------------------------------- + * + * All requests/responses, which are not connector specific, must be sent over + * control ring of the connector with index 0. + * + * For all request/response/event packets that use cookies: + * dbuf_cookie - uint64_t, unique to guest domain value used by the backend + * to map remote display buffer to its local one + * fb_cookie - uint64_t, unique to guest domain value used by the backend + * to map remote framebuffer to its local one + * + * All request packets have the same length (64 octets) + * All request packets have common header: + * 0 1 2 3 octet + * +----------------+----------------+----------------+----------------+ + * | id | operation | reserved | 4 + * +----------------+----------------+----------------+----------------+ + * | reserved | 8 + * +----------------+----------------+----------------+----------------+ + * id - uint16_t, private guest value, echoed in response + * operation - uint8_t, operation code, XENDISPL_OP_??? + * + * Request dbuf creation - request creation of a display buffer. + * 0 1 2 3 octet + * +----------------+----------------+----------------+----------------+ + * | id |_OP_DBUF_CREATE | reserved | 4 + * +----------------+----------------+----------------+----------------+ + * | reserved | 8 + * +----------------+----------------+----------------+----------------+ + * | dbuf_cookie low 32-bit | 12 + * +----------------+----------------+----------------+----------------+ + * | dbuf_cookie high 32-bit | 16 + * +----------------+----------------+----------------+----------------+ + * | width | 20 + * +----------------+----------------+----------------+----------------+ + * | height | 24 + * +----------------+----------------+----------------+----------------+ + * | bpp | 28 + * +----------------+----------------+----------------+----------------+ + * | buffer_sz | 32 + * +----------------+----------------+----------------+----------------+ + * | flags | 36 + * +----------------+----------------+----------------+----------------+ + * | gref_directory | 40 + * +----------------+----------------+----------------+----------------+ + * | reserved | 44 + * +----------------+----------------+----------------+----------------+ + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| + * +----------------+----------------+----------------+----------------+ + * | reserved | 64 + * +----------------+----------------+----------------+----------------+ + * + * Must be sent over control ring of the connector with index 0. + * + * width - uint32_t, width in pixels + * height - uint32_t, height in pixels + * bpp - uint32_t, bits per pixel + * buffer_sz - uint32_t, buffer size to be allocated, octets + * flags - uint32_t, flags of the operation + * o XENDISPL_DBUF_FLG_REQ_ALLOC - if set, then backend is requested + * to allocate the buffer with the parameters provided in this request. + * Page directory is handled as follows: + * Frontend on request: + * o allocates pages for the directory + * o grants permissions for the pages of the directory + * o sets gref_dir_next_page fields + * Backend on response: + * o grants permissions for the pages of the buffer allocated + * o fills in page directory with grant references + * gref_directory - grant_ref_t, a reference to the first shared page + * describing shared buffer references. At least one page exists. If shared + * buffer size (buffer_sz) exceeds what can be addressed by this single page, + * then reference to the next page must be supplied (see gref_dir_next_page + * below) + */ + +#define XENDISPL_DBUF_FLG_REQ_ALLOC 0x0001 + +struct xendispl_dbuf_create_req { + uint64_t dbuf_cookie; + uint32_t width; + uint32_t height; + uint32_t bpp; + uint32_t buffer_sz; + uint32_t flags; + grant_ref_t gref_directory; +}; + +/* + * Shared page for XENDISPL_OP_DBUF_CREATE 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 | 4 + * +----------------+----------------+----------------+----------------+ + * | gref[0] | 8 + * +----------------+----------------+----------------+----------------+ + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| + * +----------------+----------------+----------------+----------------+ + * | gref[i] | i*4+8 + * +----------------+----------------+----------------+----------------+ + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| + * +----------------+----------------+----------------+----------------+ + * | gref[N - 1] | N*4+8 + * +----------------+----------------+----------------+----------------+ + * + * gref_dir_next_page - grant_ref_t, reference to the next page describing + * page directory. Must be 0 if there are no more pages in the list. + * gref[i] - grant_ref_t, reference to a shared page of the buffer + * allocated at XENDISPL_OP_DBUF_CREATE + * + * Number of grant_ref_t entries in the whole page directory is not + * passed, but instead can be calculated as: + * num_grefs_total = (XENDISPL_OP_DBUF_CREATE.buffer_sz + XEN_PAGE_SIZE - 1) / + * XEN_PAGE_SIZE + */ + +struct xendispl_page_directory { + grant_ref_t gref_dir_next_page; + grant_ref_t gref[1]; /* Variable length */ +}; + +/* + * Request dbuf destruction - destroy a previously allocated display buffer: + * 0 1 2 3 octet + * +----------------+----------------+----------------+----------------+ + * | id |_OP_DBUF_DESTROY| reserved | 4 + * +----------------+----------------+----------------+----------------+ + * | reserved | 8 + * +----------------+----------------+----------------+----------------+ + * | dbuf_cookie low 32-bit | 12 + * +----------------+----------------+----------------+----------------+ + * | dbuf_cookie high 32-bit | 16 + * +----------------+----------------+----------------+----------------+ + * | reserved | 20 + * +----------------+----------------+----------------+----------------+ + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| + * +----------------+----------------+----------------+----------------+ + * | reserved | 64 + * +----------------+----------------+----------------+----------------+ + * + * Must be sent over control ring of the connector with index 0. + */ + +struct xendispl_dbuf_destroy_req { + uint64_t dbuf_cookie; +}; + +/* + * Request framebuffer attachment - request attachment of a framebuffer to + * previously created display buffer. + * 0 1 2 3 octet + * +----------------+----------------+----------------+----------------+ + * | id | _OP_FB_ATTACH | reserved | 4 + * +----------------+----------------+----------------+----------------+ + * | reserved | 8 + * +----------------+----------------+----------------+----------------+ + * | dbuf_cookie low 32-bit | 12 + * +----------------+----------------+----------------+----------------+ + * | dbuf_cookie high 32-bit | 16 + * +----------------+----------------+----------------+----------------+ + * | fb_cookie low 32-bit | 20 + * +----------------+----------------+----------------+----------------+ + * | fb_cookie high 32-bit | 24 + * +----------------+----------------+----------------+----------------+ + * | width | 28 + * +----------------+----------------+----------------+----------------+ + * | height | 32 + * +----------------+----------------+----------------+----------------+ + * | pixel_format | 36 + * +----------------+----------------+----------------+----------------+ + * | reserved | 40 + * +----------------+----------------+----------------+----------------+ + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| + * +----------------+----------------+----------------+----------------+ + * | reserved | 64 + * +----------------+----------------+----------------+----------------+ + * + * Must be sent over control ring of the connector with index 0. + * + * width - uint32_t, width in pixels + * height - uint32_t, height in pixels + * pixel_format - uint32_t, pixel format of the framebuffer, FOURCC code + */ + +struct xendispl_fb_attach_req { + uint64_t dbuf_cookie; + uint64_t fb_cookie; + uint32_t width; + uint32_t height; + uint32_t pixel_format; +}; + +/* + * Request framebuffer detach - detach a previously + * attached framebuffer from the display buffer in request: + * 0 1 2 3 octet + * +----------------+----------------+----------------+----------------+ + * | id | _OP_FB_DETACH | reserved | 4 + * +----------------+----------------+----------------+----------------+ + * | reserved | 8 + * +----------------+----------------+----------------+----------------+ + * | fb_cookie low 32-bit | 12 + * +----------------+----------------+----------------+----------------+ + * | fb_cookie high 32-bit | 16 + * +----------------+----------------+----------------+----------------+ + * | reserved | 20 + * +----------------+----------------+----------------+----------------+ + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| + * +----------------+----------------+----------------+----------------+ + * | reserved | 64 + * +----------------+----------------+----------------+----------------+ + * + * Must be sent over control ring of the connector with index 0. + */ + +struct xendispl_fb_detach_req { + uint64_t fb_cookie; +}; + +/* + * Request configuration set/reset - request to set or reset + * the configuration/mode of the display: + * 0 1 2 3 octet + * +----------------+----------------+----------------+----------------+ + * | id | _OP_SET_CONFIG | reserved | 4 + * +----------------+----------------+----------------+----------------+ + * | reserved | 8 + * +----------------+----------------+----------------+----------------+ + * | fb_cookie low 32-bit | 12 + * +----------------+----------------+----------------+----------------+ + * | fb_cookie high 32-bit | 16 + * +----------------+----------------+----------------+----------------+ + * | x | 20 + * +----------------+----------------+----------------+----------------+ + * | y | 24 + * +----------------+----------------+----------------+----------------+ + * | width | 28 + * +----------------+----------------+----------------+----------------+ + * | height | 32 + * +----------------+----------------+----------------+----------------+ + * | bpp | 40 + * +----------------+----------------+----------------+----------------+ + * | reserved | 44 + * +----------------+----------------+----------------+----------------+ + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| + * +----------------+----------------+----------------+----------------+ + * | reserved | 64 + * +----------------+----------------+----------------+----------------+ + * + * Pass all zeros to reset, otherwise command is treated as + * configuration set. + * If this is a set configuration request then framebuffer's cookie tells + * the display which framebuffer/dbuf must be displayed while enabling display + * (applying configuration). + * + * x - uint32_t, starting position in pixels by X axis + * y - uint32_t, starting position in pixels by Y axis + * width - uint32_t, width in pixels + * height - uint32_t, height in pixels + * bpp - uint32_t, bits per pixel + */ + +struct xendispl_set_config_req { + uint64_t fb_cookie; + uint32_t x; + uint32_t y; + uint32_t width; + uint32_t height; + uint32_t bpp; +}; + +/* + * Request page flip - request to flip a page identified by the framebuffer + * cookie: + * 0 1 2 3 octet + * +----------------+----------------+----------------+----------------+ + * | id | _OP_PG_FLIP | reserved | 4 + * +----------------+----------------+----------------+----------------+ + * | reserved | 8 + * +----------------+----------------+----------------+----------------+ + * | fb_cookie low 32-bit | 12 + * +----------------+----------------+----------------+----------------+ + * | fb_cookie high 32-bit | 16 + * +----------------+----------------+----------------+----------------+ + * | reserved | 20 + * +----------------+----------------+----------------+----------------+ + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| + * +----------------+----------------+----------------+----------------+ + * | reserved | 64 + * +----------------+----------------+----------------+----------------+ + */ + +struct xendispl_page_flip_req { + uint64_t fb_cookie; +}; + +/* + *---------------------------------- Responses -------------------------------- + * + * All response packets have the same length (64 octets) + * + * All response packets have common header: + * 0 1 2 3 octet + * +----------------+----------------+----------------+----------------+ + * | id | reserved | 4 + * +----------------+----------------+----------------+----------------+ + * | status | 8 + * +----------------+----------------+----------------+----------------+ + * | reserved | 12 + * +----------------+----------------+----------------+----------------+ + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| + * +----------------+----------------+----------------+----------------+ + * | reserved | 64 + * +----------------+----------------+----------------+----------------+ + * + * id - uint16_t, private guest value, echoed from request + * status - int32_t, response status, zero on success and -XEN_EXX on failure + * + *----------------------------------- Events ---------------------------------- + * + * Events are sent via a shared page allocated by the front and propagated by + * event-channel/event-ring-ref XenStore entries + * All event packets have the same length (64 octets) + * All event packets have common header: + * 0 1 2 3 octet + * +----------------+----------------+----------------+----------------+ + * | id | type | reserved | 4 + * +----------------+----------------+----------------+----------------+ + * | reserved | 8 + * +----------------+----------------+----------------+----------------+ + * + * id - uint16_t, event id, may be used by front + * type - uint8_t, type of the event + * + * + * Page flip complete event - event from back to front on page flip completed: + * 0 1 2 3 octet + * +----------------+----------------+----------------+----------------+ + * | id | _EVT_PG_FLIP | reserved | 4 + * +----------------+----------------+----------------+----------------+ + * | reserved | 8 + * +----------------+----------------+----------------+----------------+ + * | fb_cookie low 32-bit | 12 + * +----------------+----------------+----------------+----------------+ + * | fb_cookie high 32-bit | 16 + * +----------------+----------------+----------------+----------------+ + * | reserved | 20 + * +----------------+----------------+----------------+----------------+ + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| + * +----------------+----------------+----------------+----------------+ + * | reserved | 64 + * +----------------+----------------+----------------+----------------+ + */ + +struct xendispl_pg_flip_evt { + uint64_t fb_cookie; +}; + +struct xendispl_req { + uint16_t id; + uint8_t operation; + uint8_t reserved[5]; + union { + struct xendispl_dbuf_create_req dbuf_create; + struct xendispl_dbuf_destroy_req dbuf_destroy; + struct xendispl_fb_attach_req fb_attach; + struct xendispl_fb_detach_req fb_detach; + struct xendispl_set_config_req set_config; + struct xendispl_page_flip_req pg_flip; + uint8_t reserved[56]; + } op; +}; + +struct xendispl_resp { + uint16_t id; + uint8_t operation; + uint8_t reserved; + int32_t status; + uint8_t reserved1[56]; +}; + +struct xendispl_evt { + uint16_t id; + uint8_t type; + uint8_t reserved[5]; + union { + struct xendispl_pg_flip_evt pg_flip; + uint8_t reserved[56]; + } op; +}; + +DEFINE_RING_TYPES(xen_displif, struct xendispl_req, struct xendispl_resp); + +/* + ****************************************************************************** + * Back to front events delivery + ****************************************************************************** + * In order to deliver asynchronous events from back to front a shared page is + * allocated by front and its grefs propagated to back via XenStore entries + * (event-XXX). + * This page has a common header used by both front and back to synchronize + * access and control event's ring buffer, while back being a producer of the + * events and front being a consumer. The rest of the page after the header + * is used for event packets. + * + * Upon reception of an event(s) front may confirm its reception + * for either each event, group of events or none. + */ + +struct xendispl_event_page { + uint32_t in_cons; + uint32_t in_prod; + uint8_t reserved[56]; +}; + +#define XENDISPL_EVENT_PAGE_SIZE 4096 +#define XENDISPL_IN_RING_OFFS (sizeof(struct xendispl_event_page)) +#define XENDISPL_IN_RING_SIZE (XENDISPL_EVENT_PAGE_SIZE - XENDISPL_IN_RING_OFFS) +#define XENDISPL_IN_RING_LEN (XENDISPL_IN_RING_SIZE / sizeof(struct xendispl_evt)) +#define XENDISPL_IN_RING(page) \ + ((struct xendispl_evt *)((char *)(page) + XENDISPL_IN_RING_OFFS)) +#define XENDISPL_IN_RING_REF(page, idx) \ + (XENDISPL_IN_RING((page))[(idx) % XENDISPL_IN_RING_LEN]) + +#endif /* __XEN_PUBLIC_IO_DISPLIF_H__ */
From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> This is the ABI for the two halves of a para-virtualized display driver. This protocol aims to provide a unified protocol which fits more sophisticated use-cases than a framebuffer device can handle. At the moment basic functionality is supported with the intention to extend: o multiple dynamically allocated/destroyed framebuffers o buffers of arbitrary sizes o better configuration options including multiple display support Note: existing fbif can be used together with displif running at the same time, e.g. on Linux one provides framebuffer and another DRM/KMS Future extensions to the existing protocol may include: o allow display/connector cloning o allow allocating objects other than display buffers o add planes/overlays support o support scaling o support rotation