[Xen-devel] [RFC] kbdif: add multi-touch support
From: Oleksandr Andrushchenko Hi, all! I am working on adding multi-touch support to the existing kbdif protocol and would like to request for comments on the below patch. The aim is to provide multi-touch support to unprivileged domains which may employ multiple virtual displays. Thus, the protocol should handle multiple virtual touch input devices withing the same domain, so those can be mapped to different virtual displays. Thank you in advance, Oleksandr Oleksandr Andrushchenko (1): kbdif: add multi-touch support xen/include/public/io/kbdif.h | 64 +++ 1 file changed, 64 insertions(+) -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [RFC] kbdif: add multi-touch support
From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 64 +++ 1 file changed, 64 insertions(+) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index 2d2aebd..ad94b53 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -45,6 +45,19 @@ */ #define XENKBD_TYPE_POS 4 +/* + * Multi-touch event + * Capable backend sets feature-multi-touch in xenstore. + * Frontend requests feature by setting request-multi-touch in xenstore. + * Frontend supports up to XENKBD_MT_NUM_DEV virtual multi-touch input devices, + * configured by the backend in xenstore under mt-%d folder, %d being + * a sequential number of the virtual input device: + * o num-contacts - number of simultaneous touches supported + * o width - width of the touch area in pixels + * o height - height of the touch area in pixels + */ +#define XENKBD_TYPE_MTOUCH 5 + struct xenkbd_motion { uint8_t type;/* XENKBD_TYPE_MOTION */ @@ -68,6 +81,56 @@ struct xenkbd_position int32_t rel_z; /* relative Z motion (wheel) */ }; +/* number of simultaneously supported multi-touch virtual input devices */ +#define XENKBD_MT_NUM_DEV 4 + +/* Sent when a new touch is made: touch is assigned a unique contact + * ID, sent with this and consequent events related to this touch. + * Contact ID will be reused after XENKBD_MT_EV_UP event. + */ +#define XENKBD_MT_EV_DOWN 0 +/* Touch point has been released */ +#define XENKBD_MT_EV_UP 1 +/* Touch point has changed its coordinate(s) */ +#define XENKBD_MT_EV_MOTION 2 +/* Input synchronization event: shows end of a set of events + * which logically belong together. + */ +#define XENKBD_MT_EV_SYN3 +/* Touch point has changed its shape. Shape is approximated by an ellipse + * through the major and minor axis lengths: major is the longer diameter + * of the ellipse and minor is the shorter one. Center of the ellipse is + * reported via XENKBD_MT_EV_DOWN/XENKBD_MT_EV_MOTION events. + */ +#define XENKBD_MT_EV_SHAPE 4 +/* Touch point's shape has changed its orientation: calculated as a clockwise + * angle between the major axis of the ellipse and positive Y axis in degrees, + * [-180; +180]. + */ +#define XENKBD_MT_EV_ORIENT 5 + +struct xenkbd_mtouch { +uint8_t type; /* XENKBD_TYPE_MTOUCH */ +uint8_t dev_idx; /* index of the multi-touch device */ +uint8_t event_type; /* XENKBD_MT_EV_??? */ +uint8_t reserved; /* reserved for the future use */ +int32_t contact_id; /* contact ID, [0; num-contacts - 1] */ +union { +/* XENKBD_MT_EV_DOWN/XENKBD_MT_EV_MOTION */ +struct { +int32_t abs_x;/* absolute X position, pixels */ +int32_t abs_y;/* absolute Y position, pixels */ +} pos; +/* XENKBD_MT_EV_SHAPE */ +struct { +uint32_t major; /* length of the major axis, pixels */ +uint32_t minor; /* length of the minor axis, pixels */ +} shape; +/* XENKBD_MT_EV_ORIENT */ +uint16_t orientation; /* clockwise angle of the major axis */ +} u; +}; + #define XENKBD_IN_EVENT_SIZE 40 union xenkbd_in_event @@ -76,6 +139,7 @@ union xenkbd_in_event struct xenkbd_motion motion; struct xenkbd_key key; struct xenkbd_position pos; +struct xenkbd_mtouch mtouch; char pad[XENKBD_IN_EVENT_SIZE]; }; -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC] kbdif: add multi-touch support
On 01/03/2017 06:28 PM, Jan Beulich wrote: On 03.01.17 at 16:39, wrote: --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -45,6 +45,19 @@ */ #define XENKBD_TYPE_POS 4 +/* + * Multi-touch event + * Capable backend sets feature-multi-touch in xenstore. + * Frontend requests feature by setting request-multi-touch in xenstore. + * Frontend supports up to XENKBD_MT_NUM_DEV virtual multi-touch input devices, + * configured by the backend in xenstore under mt-%d folder, %d being + * a sequential number of the virtual input device: + * o num-contacts - number of simultaneous touches supported + * o width - width of the touch area in pixels + * o height - height of the touch area in pixels + */ +#define XENKBD_TYPE_MTOUCH 5 + struct xenkbd_motion { uint8_t type;/* XENKBD_TYPE_MOTION */ @@ -68,6 +81,56 @@ struct xenkbd_position int32_t rel_z; /* relative Z motion (wheel) */ }; +/* number of simultaneously supported multi-touch virtual input devices */ +#define XENKBD_MT_NUM_DEV 4 Why is this limit needed? There's no use of it within the other interface additions you make. Jan Well, the only reason for that was a shy attempt to somewhat simplify changes to the existing frontend, e.g. handling fixed number of mt input devices rather than allocating all those dynamically, finding the number of devices configured at run-time etc. I will happily remove this limitation though Oleksandr ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC] kbdif: add multi-touch support
First of all, thank you for comments On 01/04/2017 03:03 AM, Stefano Stabellini wrote: On Tue, 3 Jan 2017, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 64 +++ 1 file changed, 64 insertions(+) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index 2d2aebd..ad94b53 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -45,6 +45,19 @@ */ #define XENKBD_TYPE_POS 4 +/* + * Multi-touch event + * Capable backend sets feature-multi-touch in xenstore. + * Frontend requests feature by setting request-multi-touch in xenstore. + * Frontend supports up to XENKBD_MT_NUM_DEV virtual multi-touch input devices, + * configured by the backend in xenstore under mt-%d folder, %d being + * a sequential number of the virtual input device: + * o num-contacts - number of simultaneous touches supported + * o width - width of the touch area in pixels + * o height - height of the touch area in pixels Please write down the max width and height supported by the protocol, keeping in mind that motion events below use int32_t for coordinates. I will put "width(height) of the... in pixels, 32-bit signed integer" Are there any benefits of this compared to just setting up multiple kbd connections, one per multi-touch device? The only benefit I can think of is saving few pages. Well, not only saving a few pages, but somewhat simplifying handling of the protocol on both back and front ends. But, you are probably right as current protocol is capable of holding (XENKBD_IN_RING_SIZE - XENKBD_IN_RING_OFFS) / XENKBD_IN_EVENT_SIZE == (2048 - 1024) / 40 = 25 incoming events which may not be enough for multiple mt devices delivering hundreds of events per second. Will set-up dedicated rings per mt device then. Will also remove uint8_t dev_idx;/* index of the multi-touch device */ as every device will have its own ring. Will extend XenStore configuration for mt devices with: o page-ref (?) o page-gref o event-channel as it is done by the Linux xen-kbdfront driver. BTW, is there any reason we need "page-ref"? My understanding is that the pair of page-gref + event-channel is enough to establish and uniquely identify the connection. + */ +#define XENKBD_TYPE_MTOUCH 5 struct xenkbd_motion { uint8_t type;/* XENKBD_TYPE_MOTION */ @@ -68,6 +81,56 @@ struct xenkbd_position int32_t rel_z; /* relative Z motion (wheel) */ }; +/* number of simultaneously supported multi-touch virtual input devices */ +#define XENKBD_MT_NUM_DEV 4 If it turns out that supporting multiple devices per connection is a good idea, then I suggest that the max number of devices is a backend property on xenstore. Instead of setting the number of mt devices front can easily find this value by reading "mt-%d" XenStore entries. The requirement in the protocol is that "...%d being a sequential number of the virtual input device". Thus, first entry which we fail to sequentially read will indicate end of configured devices. Of course, xenbus_scanf return value needs to be checked if it has failed because there is no such value in XenStore or for any other reason. +/* Sent when a new touch is made: touch is assigned a unique contact + * ID, sent with this and consequent events related to this touch. + * Contact ID will be reused after XENKBD_MT_EV_UP event. Will be reused or can be reused? I would probably say "may be reused" as it depends on how and if new touches/contacts are made Please provide an example of a Contact ID lifecycle. Do you want it to be described in the protocol or just here? If the latter then, for example, as Wayland documentation describes it [1]: "Touch interactions can consist of one or more contacts. For each contact, a series of events is generated, starting with a down event, followed by zero or more motion events, and ending with an up event. Events relating to the same contact point can be identified by the ID of the sequence." So, if there is contact/touch a free Contact ID is assigned to this contact(sequence) and it is "released" when contact is done, e.g. after up event. From this point contact ID may be reused. I was basing the mt additions to the protocol on Wayland [1], Linux [2] and Windows [3] multi-touch support, so those may better explain the idea What is the max Contact ID? /* contact ID, [0; num-contacts - 1] */ num-contacts - number of simultaneous touches supported + */ +#define XENKBD_MT_EV_DOWN 0 +/* Touch point has been released */ +#define XENKBD_MT_EV_UP 1 +/* Touch point has changed its coordinate(s) */ +#define XENKBD_MT_EV_MOTION 2 +/* Input synchronization event: shows end of a set of events + * which logically belong together. + */ +#define XENKBD_MT_EV_SYN3 +/*
Re: [Xen-devel] Xen Community Call on new PV protocols, 11th Jan 9AM PST
On 01/04/2017 01:19 PM, Wei Liu wrote: On Wed, Dec 21, 2016 at 02:29:17PM -0800, Stefano Stabellini wrote: Hi all, the 4.9 development cycle is open, and we a number of new outstanding PV driver protocols on the agenda. I published a proposal on the lifecycle of PV drivers that might help speed up the process. I suggest we have a conference call to discuss how to proceed. Which PV protocol designs should we expect to make it into the next release? What steps should we take to ensure they are going to be accepted on time? I suggest we chat on Wednesday the 11th of January at 9AM PST. Does the time and date work for you? WFM. Works for EPAM as well Thanks, Stefano P.S. Julien is CC'ed in quality of release manager ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel -- *Oleksandr Andrushchenko* *Lead Software Engineer* *Office: *+380 44 390 5457 *x* 66825 *Cell: *+380 66 968 1747 *Email: *oleksandr_andrushche...@epam.com <mailto:oleksandr_andrushche...@epam.com> *Kyiv**,* *Ukraine *(GMT+3)*epam.com <http://www.epam.com>* CONFIDENTIALITY CAUTION AND DISCLAIMER This message is intended only for the use of the individual(s) or entity(ies) to which it is addressed and contains information that is legally privileged and confidential. If you are not the intended recipient, or the person responsible for delivering the message to the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. All unintended recipients are obliged to delete this message and destroy any printed copies. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [DOC v2] Xen transport for 9pfs
If this is not too late for comments... On 12/06/2016 03:33 AM, Stefano Stabellini wrote: Changes in v2: - fix copy/paste error - rename ring-ref- to ring-ref - fix memory barriers - add "verify prod/cons against local copy" - add a paragraph on high level design - add a note on the maximum possible max-ring-page-order value - add mechanisms to avoid unnecessary evtchn notifications --- # Xen transport for 9pfs version 1 ## Background 9pfs is a network filesystem protocol developed for Plan 9. 9pfs is very simple and describes a series of commands and responses. It is completely independent from the communication channels, in fact many clients and servers support multiple channels, usually called "transports". For example the Linux client supports tcp and unix sockets, fds, virtio and rdma. ### 9pfs protocol This document won't cover the full 9pfs specification. Please refer to this [paper] and this [website] for a detailed description of it. However it is useful to know that each 9pfs request and response has the following header: struct header { uint32_t size; uint8_t id; uint16_t tag; } __attribute__((packed)); As per my previous experience with sndif/displif __attribute__((packed)); is not expected to be in a generic protocol 0 4 57 +-+--++ | size |id|tag | +-+--++ - *size* The size of the request or response. - *id* The 9pfs request or response operation. - *tag* Unique id that identifies a specific request/response pair. It is used to multiplex operations on a single channel. It is possible to have multiple requests in-flight at any given time. ## Rationale This document describes a Xen based transport for 9pfs, in the traditional PV frontend and backend format. The PV frontend is used by the client to send commands to the server. The PV backend is used by the 9pfs server to receive commands from clients and send back responses. The transport protocol supports multiple rings up to the maximum supported by the backend. The size of every ring is also configurable and can span multiple pages, up to the maximum supported by the backend (although it cannot be more than 2MB). The design is to exploit parallelism at the vCPU level and support multiple outstanding requests simultaneously. This document does not cover the 9pfs client/server design or implementation, only the transport for it. ## Xenstore The frontend and the backend connect via xenstore to exchange information. The toolstack creates front and back nodes with state [XenbusStateInitialising]. The protocol node name is **9pfs**. Multiple rings are supported for each frontend and backend connection. ### Frontend XenBus Nodes num-rings port and ring-ref both have indices, thus allowing to find out how many rings are there, so why do we need to specify it explicitly? Values: Number of rings. It needs to be lower or equal to max-rings. port- (port-0, port-1, etc) Correct me if I'm wrong, but "event-channel" is most used name in the protocols, not "port" Values: The identifier of the Xen event channel used to signal activity in the ring buffer. One for each ring. Here you refer to port as to event channel... So, please consider changing it accordingly ring-ref (ring-ref0, ring-ref1, etc) Values: The Xen grant reference granting permission for the backend to map a page with information to setup a share ring. One for each ring. ### Backend XenBus Nodes Backend specific properties, written by the backend, read by the frontend: version Values: Protocol version supported by the backend. Currently the value is 1. max-rings Values: The maximum supported number of rings. Per frontend? If not, how does a frontend know how many it is allowed to use? max-ring-page-order Are there any specific requirements that this is order, not size? IMHO size allows better control on memory allocations and gives more flexibility. The only requirement on size I see is that it should be even value (because you divide allocated space for in and out) Values: The maximum supported size of a memory allocation in units of lb(machine pages), e.g. 0 == 1 page, 1 = 2 pages, 2 == 4 pages, etc. Backend configuration nodes, written by the toolstack, read by the backend: path Values: Host filesystem path to share. tag Values: Alphanumeric tag that identifies the 9pfs share. The client needs to know the tag to be able to mount it. security_model Values: "none" *none*: files are stored usin
Re: [Xen-devel] [DOC v2] Xen transport for 9pfs
On 12/12/2016 02:00 PM, Wei Liu wrote: On Mon, Dec 05, 2016 at 05:33:23PM -0800, Stefano Stabellini wrote: [...] ## Xenstore The frontend and the backend connect via xenstore to exchange information. The toolstack creates front and back nodes with state [XenbusStateInitialising]. The protocol node name is **9pfs**. Multiple rings are supported for each frontend and backend connection. It would help if you can state if a specific node is mandatory or optional. ### Frontend XenBus Nodes ### Backend XenBus Nodes [...] The producer always notifies the consumer after incrementing **prod**. However in some circumstances the producer is allowed not to notify the consumer, just as a performance improvement, and still maintain correctness. These are the steps to do it: after incrementing *prod*, the producer reads *cons* a second time; if the value is changed from the previous read and it is different from *prod* before the update, then the notification can be avoided. These are the producer steps, with the optimization: - read *prod* (old_prod), *cons* (old_cons) from shared memory - general memory barrier - verify *prod* against local copy (consumer shouldn't change it) - write to array at position *prod* up to *cons*, wrapping around the circular buffer when necessary - write memory barrier - increase *prod* (new_prod) - general memory barrier - read *cons* (new_cons) - if new_cons == old_cons or new_cons == old_prod, then notify the consumer I think it would be valuable to extract this section to a generic "how to write driver" doc. But that's probably something for another day. I do support this idea which will save lots of time and efforts for the PV developers... Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1] displif: add ABI for para-virtual display
Bug fix On 12/22/2016 10:12 AM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko This is the ABI for the two halves of a para-virtualized display driver. Changes since initial: * DRM changed to DISPL, protocol made generic * major re-work addressing issues raised for sndif Signed-off-by: Oleksandr Andrushchenko Signed-off-by: Oleksandr Grytsov --- +struct xendispl_event_page { +uint32_t in_cons; +uint32_t in_prod; +uint8_t reserved[60]; uint8_t reserved[56]; ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC] kbdif: add multi-touch support
On 01/04/2017 08:23 PM, Stefano Stabellini wrote: On Wed, 4 Jan 2017, Oleksandr Andrushchenko wrote: First of all, thank you for comments You are welcome :-) On 01/04/2017 03:03 AM, Stefano Stabellini wrote: On Tue, 3 Jan 2017, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 64 +++ 1 file changed, 64 insertions(+) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index 2d2aebd..ad94b53 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -45,6 +45,19 @@ */ #define XENKBD_TYPE_POS 4 +/* + * Multi-touch event + * Capable backend sets feature-multi-touch in xenstore. + * Frontend requests feature by setting request-multi-touch in xenstore. + * Frontend supports up to XENKBD_MT_NUM_DEV virtual multi-touch input devices, + * configured by the backend in xenstore under mt-%d folder, %d being + * a sequential number of the virtual input device: + * o num-contacts - number of simultaneous touches supported + * o width - width of the touch area in pixels + * o height - height of the touch area in pixels Please write down the max width and height supported by the protocol, keeping in mind that motion events below use int32_t for coordinates. I will put "width(height) of the... in pixels, 32-bit signed integer" I don't think I understand what you wrote here. To clarify, I meant that the doc should say what is the theoretical maximum for width and height, for example INT32_MAX. How about: * o width - width of the touch area in pixels, in * [INT_LEAST32_MIN; INT32_MAX] range * o height - height of the touch area in pixels, in * [INT_LEAST32_MIN; INT32_MAX] range Are there any benefits of this compared to just setting up multiple kbd connections, one per multi-touch device? The only benefit I can think of is saving few pages. Well, not only saving a few pages, but somewhat simplifying handling of the protocol on both back and front ends. But, you are probably right as current protocol is capable of holding (XENKBD_IN_RING_SIZE - XENKBD_IN_RING_OFFS) / XENKBD_IN_EVENT_SIZE == (2048 - 1024) / 40 = 25 incoming events which may not be enough for multiple mt devices delivering hundreds of events per second. Will set-up dedicated rings per mt device then. I think you'll find it is going to be simpler and faster for little extra memory costs. Well, I have implemented that yesterday and after some cleanup it will look ok Will also remove uint8_t dev_idx;/* index of the multi-touch device */ as every device will have its own ring. Right Will extend XenStore configuration for mt devices with: o page-ref (?) o page-gref o event-channel as it is done by the Linux xen-kbdfront driver. BTW, is there any reason we need "page-ref"? My understanding is that the pair of page-gref + event-channel is enough to establish and uniquely identify the connection. It's page-gref that is superfluous. I don't know why the Linux frontend writes it, in fact the QEMU backend doesn't even read it. I'll keep it for consistency. I have refactored the original kbdfront so there is common code for ring and event channel handling which creates all the above. If we decide that page-ref can be dropped it will be dropped for all the devices at a time. +/* Sent when a new touch is made: touch is assigned a unique contact + * ID, sent with this and consequent events related to this touch. + * Contact ID will be reused after XENKBD_MT_EV_UP event. Will be reused or can be reused? I would probably say "may be reused" as it depends on how and if new touches/contacts are made Please provide an example of a Contact ID lifecycle. Do you want it to be described in the protocol or just here? If the latter then, for example, as Wayland documentation describes it [1]: "Touch interactions can consist of one or more contacts. For each contact, a series of events is generated, starting with a down event, followed by zero or more motion events, and ending with an up event. Events relating to the same contact point can be identified by the ID of the sequence." So, if there is contact/touch a free Contact ID is assigned to this contact(sequence) and it is "released" when contact is done, e.g. after up event. From this point contact ID may be reused. This is very useful info for people not familiar with Wayland. Please write this in the doc, and link to the Wayland documentation. I have put some description, but not sure if it is a good idea to put links to Wayland or any other documentation from the Web: it might happen that in a little while the link disappears or changes I was basing the mt additions to the protocol on Wayland [1], Linux [2] and Windows [3] multi-touch support, so those may better exp
Re: [Xen-devel] [PATCH v1] displif: add ABI for para-virtual display
On 01/05/2017 05:45 PM, Jan Beulich wrote: On 22.12.16 at 09:12, wrote: +struct xendispl_pg_flip_evt { +uint64_t fb_cookie; Considering that apparently all operations have this cookie, I think it would better go ... +}; + +struct xendispl_req { +uint16_t id; +uint8_t operation; +uint8_t reserved[5]; ... here. If someone adds another event which doesn't need it? IMO, this is ok to reside where it is. Other than that the primary thing I'm missing (as I think I've mentioned elsewhere already) is a rationale of why this new protocol is needed (and the existing xenfb one can't be extended). "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" I tried to evaluate what would it be like to extend existing fbif... It looks like having 2 different protocols in a single file. What is more fbif can be used together with displif running at the same time, e.g. on Linux one provides framebuffer and another DRM Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1] displif: add ABI for para-virtual display
On 01/05/2017 06:12 PM, Jan Beulich wrote: On 05.01.17 at 17:03, wrote: On 01/05/2017 05:45 PM, Jan Beulich wrote: On 22.12.16 at 09:12, wrote: Other than that the primary thing I'm missing (as I think I've mentioned elsewhere already) is a rationale of why this new protocol is needed (and the existing xenfb one can't be extended). "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" Well, that's all stuff you had spelled out in the accompanying mail, but that's all items which could be taken care of by a protocol extension too. of course I tried to evaluate what would it be like to extend existing fbif... It looks like having 2 different protocols in a single file. This is what I'd like you to expand on. To start with: 1. In/out event sizes o fbif - 40 octets o displif - 40 octets It fits now, but this is only the initial version of the displif protocol which means that there could be requests which will not fit (we are thinking of 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. This makes me believe if we extend fbif it is better to have separate structures/rings from the start. 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 I believe 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. BTW, I can add framebuffer's update and resize into displif, so it could probably supersede fbif at some point What is more fbif can be used together with displif running at the same time, e.g. on Linux one provides framebuffer and another DRM And this is certainly a valid argument (which hence should be spelled out in the description). ok Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC] kbdif: add multi-touch support
On 01/05/2017 09:19 PM, Stefano Stabellini wrote: On Thu, 5 Jan 2017, Oleksandr Andrushchenko wrote: On 01/04/2017 08:23 PM, Stefano Stabellini wrote: On Wed, 4 Jan 2017, Oleksandr Andrushchenko wrote: First of all, thank you for comments You are welcome :-) On 01/04/2017 03:03 AM, Stefano Stabellini wrote: On Tue, 3 Jan 2017, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 64 +++ 1 file changed, 64 insertions(+) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index 2d2aebd..ad94b53 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -45,6 +45,19 @@ */ #define XENKBD_TYPE_POS 4 +/* + * Multi-touch event + * Capable backend sets feature-multi-touch in xenstore. + * Frontend requests feature by setting request-multi-touch in xenstore. + * Frontend supports up to XENKBD_MT_NUM_DEV virtual multi-touch input devices, + * configured by the backend in xenstore under mt-%d folder, %d being + * a sequential number of the virtual input device: + * o num-contacts - number of simultaneous touches supported + * o width - width of the touch area in pixels + * o height - height of the touch area in pixels Please write down the max width and height supported by the protocol, keeping in mind that motion events below use int32_t for coordinates. I will put "width(height) of the... in pixels, 32-bit signed integer" I don't think I understand what you wrote here. To clarify, I meant that the doc should say what is the theoretical maximum for width and height, for example INT32_MAX. How about: * o width - width of the touch area in pixels, in * [INT_LEAST32_MIN; INT32_MAX] range * o height - height of the touch area in pixels, in * [INT_LEAST32_MIN; INT32_MAX] range Yes, that's what I had in mind. But I think that height and width shouldn't have negative values here (movements should, but size measurements shouldn't). Maybe: width [0, INT32_MAX] height [0, INT32_MAX] even UINT32_MAX I think (width an height will be uint32_t) Are there any benefits of this compared to just setting up multiple kbd connections, one per multi-touch device? The only benefit I can think of is saving few pages. Well, not only saving a few pages, but somewhat simplifying handling of the protocol on both back and front ends. But, you are probably right as current protocol is capable of holding (XENKBD_IN_RING_SIZE - XENKBD_IN_RING_OFFS) / XENKBD_IN_EVENT_SIZE == (2048 - 1024) / 40 = 25 incoming events which may not be enough for multiple mt devices delivering hundreds of events per second. Will set-up dedicated rings per mt device then. I think you'll find it is going to be simpler and faster for little extra memory costs. Well, I have implemented that yesterday and after some cleanup it will look ok Good! Will also remove uint8_t dev_idx;/* index of the multi-touch device */ as every device will have its own ring. Right Will extend XenStore configuration for mt devices with: o page-ref (?) o page-gref o event-channel as it is done by the Linux xen-kbdfront driver. BTW, is there any reason we need "page-ref"? My understanding is that the pair of page-gref + event-channel is enough to establish and uniquely identify the connection. It's page-gref that is superfluous. I don't know why the Linux frontend writes it, in fact the QEMU backend doesn't even read it. I'll keep it for consistency. I have refactored the original kbdfront so there is common code for ring and event channel handling which creates all the above. If we decide that page-ref can be dropped it will be dropped for all the devices at a time. OK. Unfortunately the xenstore protocol for xenkbd is not documented, so we'll never be able to get rid of it :-( +/* Sent when a new touch is made: touch is assigned a unique contact + * ID, sent with this and consequent events related to this touch. + * Contact ID will be reused after XENKBD_MT_EV_UP event. Will be reused or can be reused? I would probably say "may be reused" as it depends on how and if new touches/contacts are made Please provide an example of a Contact ID lifecycle. Do you want it to be described in the protocol or just here? If the latter then, for example, as Wayland documentation describes it [1]: "Touch interactions can consist of one or more contacts. For each contact, a series of events is generated, starting with a down event, followed by zero or more motion events, and ending with an up event. Events relating to the same contact point can be identified by the ID of the sequence." So, if there is contact/touch a free Contact ID is assigned to this contact(sequence) and it is "released" when conta
[Xen-devel] [PATCH 1/2] xen/kbdif: update protocol documentation
From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 249 +- 1 file changed, 222 insertions(+), 27 deletions(-) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index 2d2aebd..0e19a40 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -26,46 +26,227 @@ #ifndef __XEN_PUBLIC_IO_KBDIF_H__ #define __XEN_PUBLIC_IO_KBDIF_H__ -/* In events (backend -> frontend) */ +/* + * + * Feature and Parameter Negotiation + * + * + * The two halves of a para-virtual driver utilize nodes within the + * XenStore to communicate capabilities and to negotiate operating parameters. + * This section enumerates these nodes which reside in the respective front and + * backend portions of the XenStore, following the XenBus convention. + * + * All data in the XenStore is stored as strings. Nodes specifying numeric + * values are encoded in decimal. Integer value ranges listed below are + * expressed as fixed sized integer types capable of storing the conversion + * of a properly formated node string, without loss of information. + * + * + *Backend XenBus Nodes + * + * + * Features supported + * + * Capable backend advertises supported features by publishing + * corresponding entries in XenStore and puts 1 as the value of the entry. + * If feature is not supported then 0 must be set or feature entry omitted. + * + * feature-abs-pointer + * Values: + * + * Backends, which support reporting of absolute coordinates for pointer + * device should set this to 1. + * + *- Pointer Device Parameters + * + * width + * Values: + * + * Maximum X coordinate (width) to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * height + * Values: + * + * Maximum Y coordinate (height) to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * + *Frontend XenBus Nodes + * + * + *-- Feature request - + * + * Capable frontend requests features from backend via setting corresponding + * entries to 1 in XenStore. Requests for features not advertised as supported + * by the backend have no effect. + * + * request-abs-pointer + * Values: + * + * Request from backend to report absolute pointer coordinates + * (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION). + * + *--- Request Transport Parameters --- + * + * event-channel + * Values: + * + * The identifier of the Xen event channel used to signal activity + * in the ring buffer. + * + * page-gref + * Values: + * + * The Xen grant reference granting permission for the backend to map + * a sole page in a single page sized event ring buffer. + * + * page-ref + * Values: + * + * OBSOLETE, not recommended for use. + * A unique (within the guest domain given) value identifying event + * channel and page reference pair. + */ /* - * Frontends should ignore unknown in events. + * EVENT CODES. */ -/* Pointer movement event */ -#define XENKBD_TYPE_MOTION 1 -/* Event type 2 currently not used */ -/* Key event (includes pointer buttons) */ -#define XENKBD_TYPE_KEY 3 +#define XENKBD_TYPE_MOTION 1 +#define XENKBD_TYPE_RESERVED 2 +#define XENKBD_TYPE_KEY3 +#define XENKBD_TYPE_POS4 + /* - * Pointer position event - * Capable backend sets feature-abs-pointer in xenstore. - * Frontend requests ot instead of XENKBD_TYPE_MOTION by setting - * request-abs-update in xenstore. + * CONSTANTS, XENSTORE FIELD AND PATH NAME STRINGS, HELPERS. + */ + +#define XENKBD_DRIVER_NAME "vkbd" + +#define XENKBD_FIELD_FEAT_ABS_POINTER "feature-abs-pointer" +#define XENKBD_FIELD_REQ_ABS_POINTER "request-abs-pointer" +#define XENKBD_FIELD_RING_GREF "page-gref" +#define XENKBD_FIELD_EVT_CHANNEL "event-channel" +#define XENKBD_FIELD_WIDTH "width" +#define XENKBD_FIELD_HEIGHT"height" + +/* OBSOLETE, not
[Xen-devel] [PATCH 0/2] xen/kbdif: add multi-touch support
From: Oleksandr Andrushchenko Hi, all! This series updates existing kbdif protocol documentation and adds multi-touch support Thank you, Oleksandr Andrushchenko Oleksandr Andrushchenko (2): xen/kbdif: update protocol documentation xen/kbdif: add multi-touch support xen/include/public/io/kbdif.h | 477 +++--- 1 file changed, 450 insertions(+), 27 deletions(-) -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 2/2] xen/kbdif: add multi-touch support
From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 228 ++ 1 file changed, 228 insertions(+) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index 0e19a40..1b446f9 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -57,6 +57,12 @@ * Backends, which support reporting of absolute coordinates for pointer * device should set this to 1. * + * feature-multi-touch + * Values: + * + * Backends, which support reporting of multi-touch events + * should set this to 1. + * *- Pointer Device Parameters * * width @@ -87,6 +93,11 @@ * Request from backend to report absolute pointer coordinates * (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION). * + * request-multi-touch + * Values: + * + * Request from backend to report multi-touch events. + * *--- Request Transport Parameters --- * * event-channel @@ -107,6 +118,43 @@ * OBSOLETE, not recommended for use. * A unique (within the guest domain given) value identifying event * channel and page reference pair. + * + *--- Multi-touch Device Parameters --- + * + * Every multi-touch input device uses a dedicated event ring and is + * configured via XenStore properties under XENKBD_PATH_MTOUCH folder. + * The values below are per emulated multi-touch input device: + * + * num-contacts + * Values: + * + * Number of simultaneous touches reported. + * + * width + * Values: + * + * Width of the touch area to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * height + * Values: + * + * Height of the touch area to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + *--- Multi-touch Transport Parameters + * + * event-channel + * Values: + * + * The identifier of the Xen event channel used to signal activity + * in the ring buffer. + * + * page-gref + * Values: + * + * The Xen grant reference granting permission for the backend to map + * a sole page in a single page sized event ring buffer. */ /* @@ -117,6 +165,16 @@ #define XENKBD_TYPE_RESERVED 2 #define XENKBD_TYPE_KEY3 #define XENKBD_TYPE_POS4 +#define XENKBD_TYPE_MTOUCH 5 + +/* Multi-touch event sub-codes */ + +#define XENKBD_MT_EV_DOWN 0 +#define XENKBD_MT_EV_UP1 +#define XENKBD_MT_EV_MOTION2 +#define XENKBD_MT_EV_SYN 3 +#define XENKBD_MT_EV_SHAPE 4 +#define XENKBD_MT_EV_ORIENT5 /* * CONSTANTS, XENSTORE FIELD AND PATH NAME STRINGS, HELPERS. @@ -125,11 +183,17 @@ #define XENKBD_DRIVER_NAME "vkbd" #define XENKBD_FIELD_FEAT_ABS_POINTER "feature-abs-pointer" +#define XENKBD_FIELD_FEAT_MTOUCH "feature-multi-touch" #define XENKBD_FIELD_REQ_ABS_POINTER "request-abs-pointer" +#define XENKBD_FIELD_REQ_MTOUCH"request-multi-touch" #define XENKBD_FIELD_RING_GREF "page-gref" #define XENKBD_FIELD_EVT_CHANNEL "event-channel" #define XENKBD_FIELD_WIDTH "width" #define XENKBD_FIELD_HEIGHT"height" +#define XENKBD_FIELD_NUM_CONTACTS "num-contacts" + +/* Path entries */ +#define XENKBD_PATH_MTOUCH "mtouch" /* OBSOLETE, not recommended for use */ #define XENKBD_FIELD_RING_REF "page-ref" @@ -249,6 +313,170 @@ struct xenkbd_position int32_t rel_z; }; +/* + * Multi-touch event and its sub-types + * + * All multi-touch event packets have common header: + * + * 0 1 23 octet + * +-+-+-+-+ + * | _TYPE_MTOUCH |event_type |contact_id | reserved| + * +-+-+-+-+ + * | reserved| + * +-+-+-+-+ + * + * event_type - unt8_t, multi-touch event sub-type, XENKBD_MT_EV_??? + * contact_id - unt8_t, ID of the contact + * + * Touch interactions can consist of one or more contacts. + * For each contact, a series of events is generated, starting + * with a down event, followed by zero or more motion events, + * and ending with an up event. Events relating to the same + * contact point can be identified by the ID of the sequence: contact
Re: [Xen-devel] [PATCH 1/2] xen/kbdif: update protocol documentation
On 01/07/2017 12:20 AM, Stefano Stabellini wrote: On Fri, 6 Jan 2017, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 249 +- 1 file changed, 222 insertions(+), 27 deletions(-) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index 2d2aebd..0e19a40 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -26,46 +26,227 @@ #ifndef __XEN_PUBLIC_IO_KBDIF_H__ #define __XEN_PUBLIC_IO_KBDIF_H__ -/* In events (backend -> frontend) */ +/* + * + * Feature and Parameter Negotiation + * + * + * The two halves of a para-virtual driver utilize nodes within the It's just "XenStore", not "the XenStore", all across the doc. Fixed + * XenStore to communicate capabilities and to negotiate operating parameters. + * This section enumerates these nodes which reside in the respective front and + * backend portions of the XenStore, following the XenBus convention. + * + * All data in the XenStore is stored as strings. Nodes specifying numeric + * values are encoded in decimal. Integer value ranges listed below are + * expressed as fixed sized integer types capable of storing the conversion + * of a properly formated node string, without loss of information. + * + * + *Backend XenBus Nodes + * + * + * Features supported + * + * Capable backend advertises supported features by publishing + * corresponding entries in XenStore and puts 1 as the value of the entry. + * If feature is not supported then 0 must be set or feature entry omitted. ^ a feature Fixed + * + * feature-abs-pointer + * Values: + * + * Backends, which support reporting of absolute coordinates for pointer + * device should set this to 1. + * + *- Pointer Device Parameters + * + * width + * Values: + * + * Maximum X coordinate (width) to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * height + * Values: + * + * Maximum Y coordinate (height) to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * + *Frontend XenBus Nodes + * + * + *-- Feature request - + * + * Capable frontend requests features from backend via setting corresponding + * entries to 1 in XenStore. Requests for features not advertised as supported + * by the backend have no effect. + * + * request-abs-pointer + * Values: + * + * Request from backend to report absolute pointer coordinates + * (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION). + * + *--- Request Transport Parameters --- + * + * event-channel + * Values: + * + * The identifier of the Xen event channel used to signal activity + * in the ring buffer. + * + * page-gref + * Values: + * + * The Xen grant reference granting permission for the backend to map + * a sole page in a single page sized event ring buffer. + * + * page-ref + * Values: + * + * OBSOLETE, not recommended for use. + * A unique (within the guest domain given) value identifying event + * channel and page reference pair. Actually page-ref is just the pfn of the page, it doesn't have any event channel info. Yes, I see this in the front driver which is concrete front's implementation(?). For that reason I put something about unique value. But I'll change that to "PFN of the shared page." + */ /* - * Frontends should ignore unknown in events. + * EVENT CODES. */ -/* Pointer movement event */ -#define XENKBD_TYPE_MOTION 1 -/* Event type 2 currently not used */ -/* Key event (includes pointer buttons) */ -#define XENKBD_TYPE_KEY 3 +#define XENKBD_TYPE_MOTION 1 +#define XENKBD_TYPE_RESERVED 2 +#define XENKBD_TYPE_KEY3 +#define XENKBD_TYPE_POS4 + /* - * Pointer position event - * Capable backend sets feature-abs-pointer in xenstore. - * Frontend requests ot instead of XENKBD_TYPE_MOTION by setting - * request-abs-update in xenstore. + * CONSTANTS, XENSTORE
Re: [Xen-devel] [PATCH 2/2] xen/kbdif: add multi-touch support
On 01/07/2017 12:37 AM, Stefano Stabellini wrote: On Fri, 6 Jan 2017, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 228 ++ 1 file changed, 228 insertions(+) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index 0e19a40..1b446f9 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -57,6 +57,12 @@ * Backends, which support reporting of absolute coordinates for pointer * device should set this to 1. * + * feature-multi-touch + * Values: + * + * Backends, which support reporting of multi-touch events + * should set this to 1. + * *- Pointer Device Parameters * * width @@ -87,6 +93,11 @@ * Request from backend to report absolute pointer coordinates * (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION). * + * request-multi-touch + * Values: + * + * Request from backend to report multi-touch events. I think in English should be "request backend to report multi-touch events". Same above. Done + * *--- Request Transport Parameters --- * * event-channel @@ -107,6 +118,43 @@ * OBSOLETE, not recommended for use. * A unique (within the guest domain given) value identifying event * channel and page reference pair. + * + *--- Multi-touch Device Parameters --- + * + * Every multi-touch input device uses a dedicated event ring and is For clarity I would say "Every multi-touch input device uses a dedicated frontend/backend connection". That includes a ring, an event channel, and xenstore entries. Done + * configured via XenStore properties under XENKBD_PATH_MTOUCH folder. I don't understand why we need a new xenstore folder. Why do we need XENKBD_PATH_MTOUCH? This is just another (IMO, cleaner) approach to define properties in XenStore for multiple instances. Let's see what vif does on my PC: /local/domain/11/device/vif/0/queue-0/tx-ring-ref = "2304" ... /local/domain/11/device/vif/0/queue-1/tx-ring-ref = "2306" ... /local/domain/11/device/vif/0/queue-2/tx-ring-ref = "2308" ... /local/domain/11/device/vif/0/queue-3/tx-ring-ref = "2310" ... /local/domain/11/device/vif/0/request-rx-copy = "1" We have folders "queue-%d" per queue which in my case are under XENKBD_PATH_MTOUCH. /local/domain/11/device/vkbd/0/mtouch/0/width = "3200" /local/domain/11/device/vkbd/0/mtouch/0/height = "3200" /local/domain/11/device/vkbd/0/mtouch/0/num-contacts = "5" /local/domain/11/device/vkbd/0/mtouch/1/width = "6400" /local/domain/11/device/vkbd/0/mtouch/1/height = "6400" /local/domain/11/device/vkbd/0/mtouch/1/num-contacts = "10" Namely, instead of "mtouch-%d" I use "mtouch/%d/. This is just like using "/local/domain/%d" but not "/local/domain-%d", so "mtouch/%d" seem to be more consistent. + * The values below are per emulated multi-touch input device: + * + * num-contacts + * Values: + * + * Number of simultaneous touches reported. + * + * width + * Values: + * + * Width of the touch area to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * height + * Values: + * + * Height of the touch area to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. This is OK. + *--- Multi-touch Transport Parameters + * + * event-channel + * Values: + * + * The identifier of the Xen event channel used to signal activity + * in the ring buffer. + * + * page-gref + * Values: + * + * The Xen grant reference granting permission for the backend to map + * a sole page in a single page sized event ring buffer. */ No need for this. If request-multi-touch, the usual xenkbd ring will be used for multi touch events. We can use the event-channel and page-gref already specified under "Request Transport Parameters". Ok, will remove /* @@ -117,6 +165,16 @@ #define XENKBD_TYPE_RESERVED 2 #define XENKBD_TYPE_KEY3 #define XENKBD_TYPE_POS4 +#define XENKBD_TYPE_MTOUCH 5 + +/* Multi-touch event sub-codes */ + +#define XENKBD_MT_EV_DOWN 0 +#define XENKBD_MT_EV_UP1 +#define XENKBD_MT_EV_MOTION2 +#define XENKBD_MT_EV_SYN 3 +#define XENKBD_MT_EV_SHAPE 4 +#define XENKBD_MT_EV_ORIENT5 /* * CONSTANTS, XENSTORE FIELD AND PATH NAM
Re: [Xen-devel] [PATCH v1] displif: add ABI for para-virtual display
As agreed on PV call PFA pahole results On 12/22/2016 10:12 AM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko 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 Oleksandr Andrushchenko (1): displif: add ABI for para-virtual display xen/include/public/io/displif.h | 730 1 file changed, 730 insertions(+) create mode 100644 xen/include/public/io/displif.h struct xendispl_dbuf_create_req { uint64_t dbuf_cookie; /* 0 8 */ uint32_t width;/* 8 4 */ uint32_t height; /*12 4 */ uint32_t bpp; /*16 4 */ uint32_t buffer_sz;/*20 4 */ uint32_t flags;/*24 4 */ grant_ref_tgref_directory_start; /*28 4 */ /* size: 32, cachelines: 1, members: 7 */ /* last cacheline: 32 bytes */ }; struct xendispl_page_directory { grant_ref_tgref_dir_next_page; /* 0 4 */ grant_ref_tgref[1]; /* 4 4 */ /* size: 8, cachelines: 1, members: 2 */ /* last cacheline: 8 bytes */ }; struct xendispl_dbuf_destroy_req { uint64_t dbuf_cookie; /* 0 8 */ /* size: 8, cachelines: 1, members: 1 */ /* last cacheline: 8 bytes */ }; struct xendispl_fb_attach_req { uint64_t dbuf_cookie; /* 0 8 */ uint64_t fb_cookie;/* 8 8 */ uint32_t width;/*16 4 */ uint32_t height; /*20 4 */ uint32_t pixel_format; /*24 4 */ /* size: 28, cachelines: 1, members: 5 */ /* last cacheline: 28 bytes */ }; struct xendispl_fb_detach_req { uint64_t fb_cookie;/* 0 8 */ /* size: 8, cachelines: 1, members: 1 */ /* last cacheline: 8 bytes */ }; struct xendispl_set_config_req { uint64_t fb_cookie;/* 0 8 */ uint32_t x;/* 8 4 */ uint32_t y;/*12 4 */ uint32_t width;/*16 4 */ uint32_t height; /*20 4 */ uint32_t bpp; /*24 4 */ /* size: 28, cachelines: 1, members: 6 */ /* last cacheline: 28 bytes */ }; struct xendispl_page_flip_req { uint64_t fb_cookie;/* 0 8 */ /* size: 8, cachelines: 1, members: 1 */ /* last cacheline: 8 bytes */ }; struct xendispl_pg_flip_evt { uint64_t fb_cookie;/* 0 8 */ /* size: 8, cachelines: 1, members: 1 */ /* last cacheline: 8 bytes */ }; struct xendispl_req { uint16_t id; /* 0 2 */ uint8_toperation;/* 2 1 */ uint8_treserved[5]; /* 3 5 */ union { struct xendispl_dbuf_create_req dbuf_create; /* 32 */ struct xendispl_dbuf_destroy_req dbuf_destroy; /* 8 */ struct xendispl_fb_attach_req fb_attach; /* 28 */ struct xendispl_fb_detach_req fb_detach; /* 8 */ struct xendispl_set_config_req set_config; /* 28 */ struct xendispl_page_flip_req pg_flip; /* 8 */ uint8_treserved[56]; /* 56 */ } op;/* 856 */ /* size: 64, cachelines: 1, members: 4 */ }; struct xendispl_resp { uint16_t id; /* 0 2 */ uint8_toperation;/* 2 1 */ int8_t status; /* 3 1 */ uint8_treserved[60]; /* 460 */ /* size: 64, cachelines: 1, members: 4 */ }; struct xendispl_evt { uint16_t id; /* 0 2 */ uint8_ttype
Re: [Xen-devel] [PATCH v15] sndif: add ABI for para-virtual sound
As agreed on PV call PFA pahole results On 12/05/2016 03:05 PM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Hi, all! Please find the next version of the ABI for the PV sound after addressing review comments. Thank you, Oleksandr Andrushchenko Oleksandr Grytsov Oleksandr Andrushchenko (1): This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other. xen/include/public/io/sndif.h | 671 ++ 1 file changed, 671 insertions(+) create mode 100644 xen/include/public/io/sndif.h struct xensnd_open_req { uint32_t pcm_rate; /* 0 4 */ uint8_tpcm_format; /* 4 1 */ uint8_tpcm_channels; /* 5 1 */ uint16_t reserved; /* 6 2 */ uint32_t buffer_sz;/* 8 4 */ grant_ref_tgref_directory_start; /*12 4 */ /* size: 16, cachelines: 1, members: 6 */ /* last cacheline: 16 bytes */ }; struct xensnd_page_directory { grant_ref_tgref_dir_next_page; /* 0 4 */ grant_ref_tgref[1]; /* 4 4 */ /* size: 8, cachelines: 1, members: 2 */ /* last cacheline: 8 bytes */ }; struct xensnd_rw_req { uint32_t offset; /* 0 4 */ uint32_t len; /* 4 4 */ /* size: 8, cachelines: 1, members: 2 */ /* last cacheline: 8 bytes */ }; struct xensnd_req { uint16_t id; /* 0 2 */ uint8_toperation;/* 2 1 */ uint8_tstream_idx; /* 3 1 */ uint32_t reserved; /* 4 4 */ union { struct xensnd_open_req open; /* 16 */ struct xensnd_rw_req rw; /* 8 */ uint8_treserved[24]; /* 24 */ } op;/* 824 */ /* size: 32, cachelines: 1, members: 5 */ /* last cacheline: 32 bytes */ }; struct xensnd_resp { uint16_t id; /* 0 2 */ uint8_toperation;/* 2 1 */ uint8_tstream_idx; /* 3 1 */ int8_t status; /* 4 1 */ uint8_treserved[27]; /* 527 */ /* size: 32, cachelines: 1, members: 5 */ /* last cacheline: 32 bytes */ }; struct xensnd_open_req { uint32_t pcm_rate; /* 0 4 */ uint8_tpcm_format; /* 4 1 */ uint8_tpcm_channels; /* 5 1 */ uint16_t reserved; /* 6 2 */ uint32_t buffer_sz;/* 8 4 */ grant_ref_tgref_directory_start; /*12 4 */ /* size: 16, cachelines: 1, members: 6 */ /* last cacheline: 16 bytes */ }; struct xensnd_page_directory { grant_ref_tgref_dir_next_page; /* 0 4 */ grant_ref_tgref[1]; /* 4 4 */ /* size: 8, cachelines: 1, members: 2 */ /* last cacheline: 8 bytes */ }; struct xensnd_rw_req { uint32_t offset; /* 0 4 */ uint32_t len; /* 4 4 */ /* size: 8, cachelines: 1, members: 2 */ /* last cacheline: 8 bytes */ }; struct xensnd_req { uint16_t id; /* 0 2 */ uint8_toperation;/* 2 1 */ uint8_tstream_idx; /* 3 1 */ uint32_t reserved; /* 4 4 */ union { struct xensnd_open_req open; /* 16 */ struct xensnd_rw_req rw; /* 8 */ uint8_treserved[24]; /* 24 */ } op;/* 824 */ /* size: 32, cachelines: 1, members: 5 */ /* last cacheline: 32 bytes */ }; struct xensnd_resp { uint16_t id; /* 0 2 */ uint8_toperation;/* 2 1 */ uint8_tstream_idx; /* 3
Re: [Xen-devel] [PATCH 0/2] xen/kbdif: add multi-touch support
As agreed on PV call PFA pahole results On 01/06/2017 11:32 AM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Hi, all! This series updates existing kbdif protocol documentation and adds multi-touch support Thank you, Oleksandr Andrushchenko Oleksandr Andrushchenko (2): xen/kbdif: update protocol documentation xen/kbdif: add multi-touch support xen/include/public/io/kbdif.h | 477 +++--- 1 file changed, 450 insertions(+), 27 deletions(-) struct xenkbd_motion { uint8_ttype; /* 0 1 */ /* XXX 3 bytes hole, try to pack */ int32_trel_x;/* 4 4 */ int32_trel_y;/* 8 4 */ int32_trel_z;/*12 4 */ /* size: 16, cachelines: 1, members: 4 */ /* sum members: 13, holes: 1, sum holes: 3 */ /* last cacheline: 16 bytes */ }; struct xenkbd_key { uint8_ttype; /* 0 1 */ uint8_tpressed; /* 1 1 */ /* XXX 2 bytes hole, try to pack */ uint32_t keycode; /* 4 4 */ /* size: 8, cachelines: 1, members: 3 */ /* sum members: 6, holes: 1, sum holes: 2 */ /* last cacheline: 8 bytes */ }; struct xenkbd_position { uint8_ttype; /* 0 1 */ /* XXX 3 bytes hole, try to pack */ int32_tabs_x;/* 4 4 */ int32_tabs_y;/* 8 4 */ int32_trel_z;/*12 4 */ /* size: 16, cachelines: 1, members: 4 */ /* sum members: 13, holes: 1, sum holes: 3 */ /* last cacheline: 16 bytes */ }; struct xenkbd_mtouch { uint8_ttype; /* 0 1 */ uint8_tevent_type; /* 1 1 */ uint8_tcontact_id; /* 2 1 */ uint8_treserved[5]; /* 3 5 */ union { struct { int32_tabs_x;/* 8 4 */ int32_tabs_y;/*12 4 */ } pos; /* 8 */ struct { uint32_t major;/* 8 4 */ uint32_t minor;/*12 4 */ } shape; /* 8 */ int16_torientation; /* 2 */ } u; /* 8 8 */ /* size: 16, cachelines: 1, members: 5 */ /* last cacheline: 16 bytes */ }; struct xenkbd_page { uint32_t in_cons; /* 0 4 */ uint32_t in_prod; /* 4 4 */ uint32_t out_cons; /* 8 4 */ uint32_t out_prod; /*12 4 */ /* size: 16, cachelines: 1, members: 4 */ /* last cacheline: 16 bytes */ }; struct xenkbd_motion { uint8_ttype; /* 0 1 */ /* XXX 3 bytes hole, try to pack */ int32_trel_x;/* 4 4 */ int32_trel_y;/* 8 4 */ int32_trel_z;/*12 4 */ /* size: 16, cachelines: 1, members: 4 */ /* sum members: 13, holes: 1, sum holes: 3 */ /* last cacheline: 16 bytes */ }; struct xenkbd_key { uint8_ttype; /* 0 1 */ uint8_tpressed; /* 1 1 */ /* XXX 2 bytes hole, try to pack */ uint32_t keycode; /* 4 4 */ /* size: 8, cachelines: 1, members: 3 */ /* sum members: 6, holes: 1, sum holes: 2 */ /* last cacheline: 8 bytes */ }; struct xenkbd_position { uint8_ttype; /* 0 1 */ /* XXX 3 bytes hole, try to pack */ int32_tabs_x;/* 4 4 */ int32_tabs_y;/* 8 4 */ int32_trel_z;/*12 4 */ /* size: 16, cachelines: 1, members: 4 */ /* sum members: 13, holes: 1, sum holes: 3 */ /* last cacheline: 16 bytes */ }; struct xenkbd_mtouch { uint8_t
Re: [Xen-devel] [PATCH 2/2] xen/kbdif: add multi-touch support
On 01/11/2017 02:29 AM, Stefano Stabellini wrote: On Tue, 10 Jan 2017, Oleksandr Andrushchenko wrote: On 01/07/2017 12:37 AM, Stefano Stabellini wrote: On Fri, 6 Jan 2017, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 228 ++ 1 file changed, 228 insertions(+) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index 0e19a40..1b446f9 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -57,6 +57,12 @@ * Backends, which support reporting of absolute coordinates for pointer * device should set this to 1. * + * feature-multi-touch + * Values: + * + * Backends, which support reporting of multi-touch events + * should set this to 1. + * *- Pointer Device Parameters * * width @@ -87,6 +93,11 @@ * Request from backend to report absolute pointer coordinates * (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION). * + * request-multi-touch + * Values: + * + * Request from backend to report multi-touch events. I think in English should be "request backend to report multi-touch events". Same above. Done + * *--- Request Transport Parameters --- * * event-channel @@ -107,6 +118,43 @@ * OBSOLETE, not recommended for use. * A unique (within the guest domain given) value identifying event * channel and page reference pair. + * + *--- Multi-touch Device Parameters --- + * + * Every multi-touch input device uses a dedicated event ring and is For clarity I would say "Every multi-touch input device uses a dedicated frontend/backend connection". That includes a ring, an event channel, and xenstore entries. Done + * configured via XenStore properties under XENKBD_PATH_MTOUCH folder. I don't understand why we need a new xenstore folder. Why do we need XENKBD_PATH_MTOUCH? This is just another (IMO, cleaner) approach to define properties in XenStore for multiple instances. Let's see what vif does on my PC: /local/domain/11/device/vif/0/queue-0/tx-ring-ref = "2304" ... /local/domain/11/device/vif/0/queue-1/tx-ring-ref = "2306" ... /local/domain/11/device/vif/0/queue-2/tx-ring-ref = "2308" ... /local/domain/11/device/vif/0/queue-3/tx-ring-ref = "2310" ... /local/domain/11/device/vif/0/request-rx-copy = "1" We have folders "queue-%d" per queue which in my case are under XENKBD_PATH_MTOUCH. /local/domain/11/device/vkbd/0/mtouch/0/width = "3200" /local/domain/11/device/vkbd/0/mtouch/0/height = "3200" /local/domain/11/device/vkbd/0/mtouch/0/num-contacts = "5" /local/domain/11/device/vkbd/0/mtouch/1/width = "6400" /local/domain/11/device/vkbd/0/mtouch/1/height = "6400" /local/domain/11/device/vkbd/0/mtouch/1/num-contacts = "10" Namely, instead of "mtouch-%d" I use "mtouch/%d/. This is just like using "/local/domain/%d" but not "/local/domain-%d", so "mtouch/%d" seem to be more consistent. I agree that mtouch/%d is better than mtouch-%d, but it's only necessary if we have more than one mtouch per vkbd. I would configure it so that one can only have one mtouch per vkbd: /local/domain/11/device/vkbd/0/mtouch/width = "3200" /local/domain/11/device/vkbd/0/mtouch/height = "3200" /local/domain/11/device/vkbd/1/mtouch/width = "6400" /local/domain/11/device/vkbd/1/mtouch/height = "6400" correct me if I'm wrong, but this notation implies multiple drivers support, not a single driver with multiple devices. If so, *DISCLAIMER* *Linux* I see no simple solution to do that in the front driver. Could you please clarify? This is also what I was referring to when I wrote: It is useful to distinguish mouse events from keyboard events, from multitouch events more easily, but I don't think we should support multiple keyboards or multiple mice on the same xenkbd ring. If we need two mice, we can setup two xenkdb rings. The same applies to multitouch devices, I think. But maybe you have good reasons for supporting multiple multitouch devices on a single vkbd connection? How many do you expect to have on a single VM? well, let me put the whole tree from xenstore: /local/domain/0/backend/vkbd/2/0/frontend-id = "2" /local/domain/0/backend/vkbd/2/0/frontend = "/local/domain/2/device/vkbd/0" /local/domain/0/backend/vkbd/2/0/dev_id = "0" /local/domain/0/backend/vkbd/2/0/state = "6" /local/domain/0/backend/vkbd/2/0/feature-abs-pointer = "1" /local/domain/0/backend/vkb
Re: [Xen-devel] [PATCH 1/2] xen/kbdif: update protocol documentation
On 01/11/2017 07:35 PM, Dario Faggioli wrote: On Tue, 2017-01-10 at 09:21 +0200, Oleksandr Andrushchenko wrote: On 01/07/2017 12:20 AM, Stefano Stabellini wrote: On Fri, 6 Jan 2017, Oleksandr Andrushchenko wrote: | reserved | + * +-+-+-+ -+ + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\ /\/\/\/| + * +-+-+-+ -+ I guess this means that we are skipping some bytes because they are all reserved, right? If so, it would be useful to write the byte count at this point. What's the total size of the event struct? IMO, we shouldn't put any sizes here because: 1. Above we say "All event packets have the same length (40 octets)" 2. All the event structures are part of the union xenkbd_in_event, which has char pad[XENKBD_IN_EVENT_SIZE]; which effectively regulates the size of the event. In which case, you can use either 40 or XENKBD_IN_EVENT_SIZE (probably the latter). It's indeed a repetition, but a good one, IMO: it helps the reader, as she won't have to go back to figure out how big the struct was, how the macro was call and to what value it was defined). I am still not convinced that we should put it. Probably we can go the way other RFCs do, like TCP [1], 802.11 [2] etc: those do not define any reserved fields at the bottom of structures, (which are effectively padding?) but are limited to only those fields which are defined. This means that, for example, instead of * 0 1 2 3octet * +-+-+-+-+ * | _TYPE_MOTION | reserved | * +-+-+-+-+ * | rel_x | * +-+-+-+-+ * | rel_y | * +-+-+-+-+ * | rel_z | * +-+-+-+-+ * | reserved| * +-+-+-+-+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| * +-+-+-+-+ * | reserved| * +-+-+-+-+ simply put * 0 1 2 3octet * +-+-+-+-+ * | _TYPE_MOTION | reserved | * +-+-+-+-+ * | rel_x | * +-+-+-+-+ * | rel_y | * +-+-+-+-+ * | rel_z | * +-+-+-+-+ What do you think? [1] https://www.ietf.org/rfc/rfc793.txt [2] https://tools.ietf.org/html/rfc5416 Regards, Dario Thank you, Oleksandr ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC] kbdif: add multi-touch support
I know it is cumbersome, and I might not be a fun of it myself, but it is required for new Xen protocol changes. I wrote all of the binary representations manually but if you find a tool to do it, please let me know :-) Letting you know ;) there is a project in Python which can do this [1] I'm going to give it a try [1] https://github.com/luismartingarcia/protocol ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] xen/kbdif: update protocol documentation
On 01/12/2017 12:50 AM, Dario Faggioli wrote: On Wed, 2017-01-11 at 20:40 +0200, Oleksandr Andrushchenko wrote: On 01/11/2017 07:35 PM, Dario Faggioli wrote: It's indeed a repetition, but a good one, IMO: it helps the reader, as she won't have to go back to figure out how big the struct was, how the macro was call and to what value it was defined). I am still not convinced that we should put it. Probably we can go the way other RFCs do, like TCP [1], 802.11 [2] etc: those do not define any reserved fields at the bottom of structures, (which are effectively padding?) but are limited to only those fields which are defined. In principle, I like the idea of following the example of those RFCs. However, I'd say that what we should value most is consistency within our own source tree. But, TBH, there aren't many binary diagram already committed in include/public/io, so it's hard to tell. FWIW, I still think that providing a clue to the reader about the size --even if already specified somewhere else-- would be beneficial, but it's a rather minor thing, and I certainly can leave with whatever you and the maintainer(s) agree upon. fair enough Regards, Dario Konrad, could you please define what that ASCII box notation should look like? Thank you, Oleksandr ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] PV audio drivers for Linux
On 01/18/2017 06:31 AM, Ughreja, Rakesh A wrote: -Original Message- From: Stefano Stabellini [mailto:sstabell...@kernel.org] Sent: Wednesday, January 18, 2017 5:41 AM To: Ughreja, Rakesh A Cc: xen-devel@lists.xen.org; oleksandr_andrushche...@epam.com; oleksandr_gryt...@epam.com; oleksandr.dmytrys...@globallogic.com; iurii.konovale...@globallogic.com; konrad.w...@oracle.com Subject: Re: [Xen-devel] PV audio drivers for Linux On Tue, 17 Jan 2017, Ughreja, Rakesh A wrote: Hi, I am trying to develop PV audio drivers and facing one issue to achieve zero copy of the buffers between Front End (DOM1) and Back End (DOM0) drivers. You might want to take a look at the existing PV sound proposal: http://marc.info/?l=xen-devel&m=148094319010445 Sure, let me look into this. Thank you very much for the quick reply and the reference. And here we are working on sound, display (DRM), multi-touch PV frontends [1], generic backend user-space library [2] and the corresponding user-space backends [3], [4]. Please take a look at the Dom0 zero-copy driver for DRM [5] Please note, that we haven't considered zero-copy for sound (ALSA) yet, but for DRM only. [1] https://github.com/andr2000/linux/tree/xen-pv-devel-4.10-rc2 [2] https://github.com/al1img/libxenbe [3] https://github.com/al1img/displ_be [4] https://github.com/al1img/snd_be [5] https://github.com/andr2000/linux/blob/xen-pv-devel-4.10-rc2/drivers/gpu/drm/xen/xen_drm_zcopy.c ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] PV audio drivers for Linux
On 01/18/2017 08:52 PM, Stefano Stabellini wrote: On Wed, 18 Jan 2017, Ughreja, Rakesh A wrote: -Original Message- From: Stefano Stabellini [mailto:sstabell...@kernel.org] Sent: Wednesday, January 18, 2017 5:41 AM To: Ughreja, Rakesh A Cc: xen-devel@lists.xen.org; oleksandr_andrushche...@epam.com; oleksandr_gryt...@epam.com; oleksandr.dmytrys...@globallogic.com; iurii.konovale...@globallogic.com; konrad.w...@oracle.com Subject: Re: [Xen-devel] PV audio drivers for Linux On Tue, 17 Jan 2017, Ughreja, Rakesh A wrote: Hi, I am trying to develop PV audio drivers and facing one issue to achieve zero copy of the buffers between Front End (DOM1) and Back End (DOM0) drivers. You might want to take a look at the existing PV sound proposal: http://marc.info/?l=xen-devel&m=148094319010445 Sure, let me look into this. Thank you very much for the quick reply and the reference. When the buffer is allocated using __get_free_pages() on the DOM0 OS, I am able to grant the access using gnttab_grant_foreign_access() to DOM1 as well as I am able to map it in the DOM1 virtual space using xenbus_map_ring_valloc(). However the existing audio driver allocates buffer using dma_alloc_coherent(). In that case I am able to grant the access using gnttab_grant_foreign_access() to DOM1 but when I try to map in the DOM1 virtual space using xenbus_map_ring_valloc(), it returns an error. [1] Code returns from here. 507 xenbus_dev_fatal(dev, map[i].status, 508 "mapping in shared page %d from domain %d", 509 gnt_refs[i], dev->otherend_id); gnttab_batch_map(map, i) is unable to map the page, but I am unable to understand why. May be its due to the difference in the way buffers are allocated dma_alloc_coherent() vs __get_free_pages(). Since I don't want to touch existing audio driver, I need to figure out how to map buffer to DOM1 space with dma_alloc_coherent(). Any pointers would be really helpful. Thank you in advance. Pages allocated by dma_alloc_coherent can be a bit special. Are you going through the swiotlb-xen (drivers/xen/swiotlb-xen.c:xen_swiotlb_alloc_coherent) in Dom0? No, I am not using this. Keep in mind that when swiotlb-xen is used, it is transparent from the device driver point of view. You might be using it without knowing. I suggest you add a printk in there to be sure. Actually I am trying to reuse the existing HDA driver and just opening the ALSA streams at kernel level in PV Backend driver. Buffers are allocated by the existing HDA driver. http://lxr.free-electrons.com/source/sound/core/memalloc.c#L83 I would probably add a few printks to Xen in xen/common/grant_table.c:do_grant_table_op to understand what is the error exactly. In the gnttab_retry_eagain_gop function, when it tries to get the status it always receives status as GNTST_eagain. After the retries the status is marked as GNTST_bad_page. I am unable to figure out what properties of dma_alloc_coherent allocated buffers makes it un-mappable at Dom1. Nothing comes to mind, but maybe the x86 maintainers (CC'ed) know. just a guess. please check [1]. I had problems mapping dma_alloc_coherent pages in DomU because of vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); which cannot be done in unprivileged domain Regards, Oleksandr ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel [1] https://lists.xen.org/archives/html/xen-devel/2016-11/msg02882.html ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] xen/kbdif: update protocol documentation
On 01/12/2017 08:36 AM, Oleksandr Andrushchenko wrote: On 01/12/2017 12:50 AM, Dario Faggioli wrote: On Wed, 2017-01-11 at 20:40 +0200, Oleksandr Andrushchenko wrote: On 01/11/2017 07:35 PM, Dario Faggioli wrote: It's indeed a repetition, but a good one, IMO: it helps the reader, as she won't have to go back to figure out how big the struct was, how the macro was call and to what value it was defined). I am still not convinced that we should put it. Probably we can go the way other RFCs do, like TCP [1], 802.11 [2] etc: those do not define any reserved fields at the bottom of structures, (which are effectively padding?) but are limited to only those fields which are defined. In principle, I like the idea of following the example of those RFCs. However, I'd say that what we should value most is consistency within our own source tree. But, TBH, there aren't many binary diagram already committed in include/public/io, so it's hard to tell. FWIW, I still think that providing a clue to the reader about the size --even if already specified somewhere else-- would be beneficial, but it's a rather minor thing, and I certainly can leave with whatever you and the maintainer(s) agree upon. fair enough Regards, Dario Konrad, could you please define what that ASCII box notation should look like? Stefano, Konrad As per my understanding this is the only thing blocking multi-touch and updated kbdif protocol from being upgraded/extended ) Could you please make some decision on this any time soon? Thank you, Oleksandr ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v1 0/2] xen/kbdif: add multi-touch support
From: Oleksandr Andrushchenko Hi, all! This series updates existing kbdif protocol documentation and adds multi-touch support Thank you, Oleksandr Andrushchenko Oleksandr Andrushchenko (2): xen/kbdif: update protocol documentation xen/kbdif: add multi-touch support xen/include/public/io/kbdif.h | 464 +++--- 1 file changed, 437 insertions(+), 27 deletions(-) -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v1 1/2] xen/kbdif: update protocol documentation
From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 248 +- 1 file changed, 221 insertions(+), 27 deletions(-) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index 2d2aebdd3f28..c00faa3af5d2 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -26,46 +26,226 @@ #ifndef __XEN_PUBLIC_IO_KBDIF_H__ #define __XEN_PUBLIC_IO_KBDIF_H__ -/* In events (backend -> frontend) */ +/* + * + * Feature and Parameter Negotiation + * + * + * The two halves of a para-virtual driver utilize nodes within the + * XenStore to communicate capabilities and to negotiate operating parameters. + * This section enumerates these nodes which reside in the respective front and + * backend portions of XenStore, following 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. + * + * + *Backend XenBus Nodes + * + * + * Features supported + * + * Capable backend advertises supported features by publishing + * corresponding entries in XenStore and puts 1 as the value of the entry. + * If a feature is not supported then 0 must be set or feature entry omitted. + * + * feature-abs-pointer + * Values: + * + * Backends, which support reporting of absolute coordinates for pointer + * device should set this to 1. + * + *- Pointer Device Parameters + * + * width + * Values: + * + * Maximum X coordinate (width) to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * height + * Values: + * + * Maximum Y coordinate (height) to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * + *Frontend XenBus Nodes + * + * + *-- Feature request - + * + * Capable frontend requests features from backend via setting corresponding + * entries to 1 in XenStore. Requests for features not advertised as supported + * by the backend have no effect. + * + * request-abs-pointer + * Values: + * + * Request backend to report absolute pointer coordinates + * (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION). + * + *--- Request Transport Parameters --- + * + * event-channel + * Values: + * + * The identifier of the Xen event channel used to signal activity + * in the ring buffer. + * + * page-gref + * Values: + * + * The Xen grant reference granting permission for the backend to map + * a sole page in a single page sized event ring buffer. + * + * page-ref + * Values: + * + * OBSOLETE, not recommended for use. + * PFN of the shared page. + */ /* - * Frontends should ignore unknown in events. + * EVENT CODES. */ -/* Pointer movement event */ -#define XENKBD_TYPE_MOTION 1 -/* Event type 2 currently not used */ -/* Key event (includes pointer buttons) */ -#define XENKBD_TYPE_KEY 3 +#define XENKBD_TYPE_MOTION 1 +#define XENKBD_TYPE_RESERVED 2 +#define XENKBD_TYPE_KEY3 +#define XENKBD_TYPE_POS4 + /* - * Pointer position event - * Capable backend sets feature-abs-pointer in xenstore. - * Frontend requests ot instead of XENKBD_TYPE_MOTION by setting - * request-abs-update in xenstore. + * CONSTANTS, XENSTORE FIELD AND PATH NAME STRINGS, HELPERS. + */ + +#define XENKBD_DRIVER_NAME "vkbd" + +#define XENKBD_FIELD_FEAT_ABS_POINTER "feature-abs-pointer" +#define XENKBD_FIELD_REQ_ABS_POINTER "request-abs-pointer" +#define XENKBD_FIELD_RING_GREF "page-gref" +#define XENKBD_FIELD_EVT_CHANNEL "event-channel" +#define XENKBD_FIELD_WIDTH "width" +#define XENKBD_FIELD_HEIGHT"height" + +/* OBSOLETE, not recommended for use */ +#define XENKBD_FIELD_RING_REF "page-ref" + +/* + **
[Xen-devel] [PATCH v1 2/2] xen/kbdif: add multi-touch support
From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 216 ++ 1 file changed, 216 insertions(+) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index c00faa3af5d2..8d8ba9af1cb1 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -57,6 +57,12 @@ * Backends, which support reporting of absolute coordinates for pointer * device should set this to 1. * + * feature-multi-touch + * Values: + * + * Backends, which support reporting of multi-touch events + * should set this to 1. + * *- Pointer Device Parameters * * width @@ -87,6 +93,11 @@ * Request backend to report absolute pointer coordinates * (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION). * + * request-multi-touch + * Values: + * + * Request backend to report multi-touch events. + * *--- Request Transport Parameters --- * * event-channel @@ -106,6 +117,30 @@ * * OBSOLETE, not recommended for use. * PFN of the shared page. + * + *--- Multi-touch Device Parameters --- + * + * Every multi-touch input device uses a dedicated event frontend/backend + * connection configured via XenStore properties under + * XENKBD_PATH_MTOUCH folder. The values below are per emulated multi-touch + * input device: + * + * num-contacts + * Values: + * + * Number of simultaneous touches reported. + * + * width + * Values: + * + * Width of the touch area to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * height + * Values: + * + * Height of the touch area to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. */ /* @@ -116,6 +151,16 @@ #define XENKBD_TYPE_RESERVED 2 #define XENKBD_TYPE_KEY3 #define XENKBD_TYPE_POS4 +#define XENKBD_TYPE_MTOUCH 5 + +/* Multi-touch event sub-codes */ + +#define XENKBD_MT_EV_DOWN 0 +#define XENKBD_MT_EV_UP1 +#define XENKBD_MT_EV_MOTION2 +#define XENKBD_MT_EV_SYN 3 +#define XENKBD_MT_EV_SHAPE 4 +#define XENKBD_MT_EV_ORIENT5 /* * CONSTANTS, XENSTORE FIELD AND PATH NAME STRINGS, HELPERS. @@ -124,11 +169,17 @@ #define XENKBD_DRIVER_NAME "vkbd" #define XENKBD_FIELD_FEAT_ABS_POINTER "feature-abs-pointer" +#define XENKBD_FIELD_FEAT_MTOUCH "feature-multi-touch" #define XENKBD_FIELD_REQ_ABS_POINTER "request-abs-pointer" +#define XENKBD_FIELD_REQ_MTOUCH"request-multi-touch" #define XENKBD_FIELD_RING_GREF "page-gref" #define XENKBD_FIELD_EVT_CHANNEL "event-channel" #define XENKBD_FIELD_WIDTH "width" #define XENKBD_FIELD_HEIGHT"height" +#define XENKBD_FIELD_NUM_CONTACTS "num-contacts" + +/* Path entries */ +#define XENKBD_PATH_MTOUCH "mtouch" /* OBSOLETE, not recommended for use */ #define XENKBD_FIELD_RING_REF "page-ref" @@ -248,6 +299,170 @@ struct xenkbd_position int32_t rel_z; }; +/* + * Multi-touch event and its sub-types + * + * All multi-touch event packets have common header: + * + * 01 2 3octet + * +++++ + * | _TYPE_MTOUCH | event_type | contact_id |reserved| 4 + * +++++ + * | reserved | 8 + * +++++ + * + * event_type - unt8_t, multi-touch event sub-type, XENKBD_MT_EV_??? + * contact_id - unt8_t, ID of the contact + * + * Touch interactions can consist of one or more contacts. + * For each contact, a series of events is generated, starting + * with a down event, followed by zero or more motion events, + * and ending with an up event. Events relating to the same + * contact point can be identified by the ID of the sequence: contact ID. + * Contact ID may be reused after XENKBD_MT_EV_UP event and + * is in the [0; XENKBD_FIELD_NUM_CONTACTS - 1] range. + * + * For further information please refer to documentation on Wayland [1], + * Linux [2] and Windows [3] multi-touch support. + * + * [1] https://cgit.freedesktop.org/wayland/wayland/tree/protocol/wayland.xml + * [2] https://www.kernel.org/doc/Documentation/input/multi-touch-protocol.txt + * [3] https://msdn.microsoft.com/en-us/library/jj151564(v=vs.85).aspx + * + * + * Multi-to
Re: [Xen-devel] [PATCH v1 1/2] xen/kbdif: update protocol documentation
On 01/19/2017 08:56 PM, Stefano Stabellini wrote: On Thu, 19 Jan 2017, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 248 +- 1 file changed, 221 insertions(+), 27 deletions(-) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index 2d2aebdd3f28..c00faa3af5d2 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -26,46 +26,226 @@ #ifndef __XEN_PUBLIC_IO_KBDIF_H__ #define __XEN_PUBLIC_IO_KBDIF_H__ -/* In events (backend -> frontend) */ +/* + * + * Feature and Parameter Negotiation + * + * + * The two halves of a para-virtual driver utilize nodes within the within "XenStore", drop "the" Aside from this minor this: Reviewed-by: Stefano Stabellini Thanks, with this change done, can I put your Reviewed-by into the next version of this patch? + * 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 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. + * + * + *Backend XenBus Nodes + * + * + * Features supported + * + * Capable backend advertises supported features by publishing + * corresponding entries in XenStore and puts 1 as the value of the entry. + * If a feature is not supported then 0 must be set or feature entry omitted. + * + * feature-abs-pointer + * Values: + * + * Backends, which support reporting of absolute coordinates for pointer + * device should set this to 1. + * + *- Pointer Device Parameters + * + * width + * Values: + * + * Maximum X coordinate (width) to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * height + * Values: + * + * Maximum Y coordinate (height) to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * + *Frontend XenBus Nodes + * + * + *-- Feature request - + * + * Capable frontend requests features from backend via setting corresponding + * entries to 1 in XenStore. Requests for features not advertised as supported + * by the backend have no effect. + * + * request-abs-pointer + * Values: + * + * Request backend to report absolute pointer coordinates + * (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION). + * + *--- Request Transport Parameters --- + * + * event-channel + * Values: + * + * The identifier of the Xen event channel used to signal activity + * in the ring buffer. + * + * page-gref + * Values: + * + * The Xen grant reference granting permission for the backend to map + * a sole page in a single page sized event ring buffer. + * + * page-ref + * Values: + * + * OBSOLETE, not recommended for use. + * PFN of the shared page. + */ /* - * Frontends should ignore unknown in events. + * EVENT CODES. */ -/* Pointer movement event */ -#define XENKBD_TYPE_MOTION 1 -/* Event type 2 currently not used */ -/* Key event (includes pointer buttons) */ -#define XENKBD_TYPE_KEY 3 +#define XENKBD_TYPE_MOTION 1 +#define XENKBD_TYPE_RESERVED 2 +#define XENKBD_TYPE_KEY3 +#define XENKBD_TYPE_POS4 + /* - * Pointer position event - * Capable backend sets feature-abs-pointer in xenstore. - * Frontend requests ot instead of XENKBD_TYPE_MOTION by setting - * request-abs-update in xenstore. + * CONSTANTS, XENSTORE FIELD AND PATH NAME STRINGS, HELPERS. + */ + +#define XENKBD_DRIVER_NAME "vkbd" + +#define XENKBD_FIELD_FEAT_ABS_POINTER "feature-abs-pointer" +#define XENKBD_FIELD_REQ_ABS_POINTER "request-abs-pointer" +#define XENKBD_FIELD_RING_GREF "page-gr
Re: [Xen-devel] [PATCH v1 2/2] xen/kbdif: add multi-touch support
On 01/20/2017 12:22 AM, Stefano Stabellini wrote: On Thu, 19 Jan 2017, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 216 ++ 1 file changed, 216 insertions(+) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index c00faa3af5d2..8d8ba9af1cb1 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -57,6 +57,12 @@ * Backends, which support reporting of absolute coordinates for pointer * device should set this to 1. * + * feature-multi-touch + * Values: + * + * Backends, which support reporting of multi-touch events + * should set this to 1. + * *- Pointer Device Parameters * * width @@ -87,6 +93,11 @@ * Request backend to report absolute pointer coordinates * (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION). * + * request-multi-touch + * Values: + * + * Request backend to report multi-touch events. + * *--- Request Transport Parameters --- * * event-channel @@ -106,6 +117,30 @@ * * OBSOLETE, not recommended for use. * PFN of the shared page. + * + *--- Multi-touch Device Parameters --- + * + * Every multi-touch input device uses a dedicated event frontend/backend + * connection configured via XenStore properties under + * XENKBD_PATH_MTOUCH folder. This sentence doesn't seem to match the rest of the patch: lets break it into smaller parts it looks like multi-touch devices use the same ring and evtchn used by the pointer and keyboard. *Every* multi-touch input device uses a *dedicated* event frontend/backend *connection* So, it is clearly stated that there is a dedicated/separate connection (which consists of a ring and corresponding event channel: here we have already discussed this part: http://marc.info/?l=xen-devel&m=148412731428686): + * Every multi-touch input device uses a dedicated event ring and is For clarity I would say "Every multi-touch input device uses a dedicated frontend/backend connection". That includes a ring, an event channel, and xenstore entries. Also, it is not clear whether multiple multitouch devices are supported with a single frontend/backend connection (as you described in http://marc.info/?l=xen-devel&m=148412731428686) or not. Please clarify. Same as above: *Every* multi-touch input device uses a *dedicated* event frontend/backend *connection* If only one device is supported, do we need a XENKBD_PATH_MTOUCH folder? For consistency and simplicity: at http://marc.info/?l=xen-devel&m=148412731428686 I have an example of how XenStore entries look like. If you define multiple mtouch devices then you'll iterate over folder contents, e.g. /local/domain/11/device/vkbd/0/mtouch/0/width = "3200" /local/domain/11/device/vkbd/0/mtouch/1/width = "6400" So, you'll have something like for (i = 0; i < num_mtouch_devices; i++) { process(i); } If you have a single mtouch device nothing changes. Thus, I see no reason in making any difference for single or multiple-devices. The values below are per emulated multi-touch + * input device: + * + * num-contacts + * Values: + * + * Number of simultaneous touches reported. + * + * width + * Values: + * + * Width of the touch area to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * height + * Values: + * + * Height of the touch area to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. */ /* @@ -116,6 +151,16 @@ #define XENKBD_TYPE_RESERVED 2 #define XENKBD_TYPE_KEY3 #define XENKBD_TYPE_POS4 +#define XENKBD_TYPE_MTOUCH 5 + +/* Multi-touch event sub-codes */ + +#define XENKBD_MT_EV_DOWN 0 +#define XENKBD_MT_EV_UP1 +#define XENKBD_MT_EV_MOTION2 +#define XENKBD_MT_EV_SYN 3 +#define XENKBD_MT_EV_SHAPE 4 +#define XENKBD_MT_EV_ORIENT5 /* * CONSTANTS, XENSTORE FIELD AND PATH NAME STRINGS, HELPERS. @@ -124,11 +169,17 @@ #define XENKBD_DRIVER_NAME "vkbd" #define XENKBD_FIELD_FEAT_ABS_POINTER "feature-abs-pointer" +#define XENKBD_FIELD_FEAT_MTOUCH "feature-multi-touch" #define XENKBD_FIELD_REQ_ABS_POINTER "request-abs-pointer" +#define XENKBD_FIELD_REQ_MTOUCH"request-multi-touch" #define XENKBD_FIELD_RING_GREF "page-gref" #define XENKBD_FIELD_EVT_CHANNEL "event-channel" #define XENKBD_FIELD_WIDTH
Re: [Xen-devel] [PATCH v1 2/2] xen/kbdif: add multi-touch support
On 01/20/2017 07:52 PM, Stefano Stabellini wrote: On Fri, 20 Jan 2017, Oleksandr Andrushchenko wrote: On 01/20/2017 12:22 AM, Stefano Stabellini wrote: On Thu, 19 Jan 2017, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 216 ++ 1 file changed, 216 insertions(+) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index c00faa3af5d2..8d8ba9af1cb1 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -57,6 +57,12 @@ * Backends, which support reporting of absolute coordinates for pointer * device should set this to 1. * + * feature-multi-touch + * Values: + * + * Backends, which support reporting of multi-touch events + * should set this to 1. + * *- Pointer Device Parameters * * width @@ -87,6 +93,11 @@ * Request backend to report absolute pointer coordinates * (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION). * + * request-multi-touch + * Values: + * + * Request backend to report multi-touch events. + * *--- Request Transport Parameters --- * * event-channel @@ -106,6 +117,30 @@ * * OBSOLETE, not recommended for use. * PFN of the shared page. + * + *--- Multi-touch Device Parameters --- + * + * Every multi-touch input device uses a dedicated event frontend/backend + * connection configured via XenStore properties under + * XENKBD_PATH_MTOUCH folder. This sentence doesn't seem to match the rest of the patch: lets break it into smaller parts it looks like multi-touch devices use the same ring and evtchn used by the pointer and keyboard. *Every* multi-touch input device uses a *dedicated* event frontend/backend *connection* So, it is clearly stated that there is a dedicated/separate connection (which consists of a ring and corresponding event channel: here we have already discussed this part: http://marc.info/?l=xen-devel&m=148412731428686): + * Every multi-touch input device uses a dedicated event ring and is For clarity I would say "Every multi-touch input device uses a dedicated frontend/backend connection". That includes a ring, an event channel, and xenstore entries. Also, it is not clear whether multiple multitouch devices are supported with a single frontend/backend connection (as you described in http://marc.info/?l=xen-devel&m=148412731428686) or not. Please clarify. Same as above: *Every* multi-touch input device uses a *dedicated* event frontend/backend *connection* If only one device is supported, do we need a XENKBD_PATH_MTOUCH folder? For consistency and simplicity: at http://marc.info/?l=xen-devel&m=148412731428686 I have an example of how XenStore entries look like. If you define multiple mtouch devices then you'll iterate over folder contents, e.g. /local/domain/11/device/vkbd/0/mtouch/0/width = "3200" /local/domain/11/device/vkbd/0/mtouch/1/width = "6400" So, you'll have something like for (i = 0; i < num_mtouch_devices; i++) { process(i); } If you have a single mtouch device nothing changes. Thus, I see no reason in making any difference for single or multiple-devices. All right. I got confused because I read first the other email where you still suggested to use the other approach. Sorry. The wording is clear enough. No problem, glad we are on the same page now Aside from clarity, XENKBD_PATH_MTOUCH is important to distinguish the width and height parameters of the mtouch device from the ones of the pointers device, is that right? Exactly. Also for ring-ref and event-channel This patch is OK as is. Reviewed-by: Stefano Stabellini Thank you, can I also put this "Reviewed-by" into the patch? Anyways, I have to wait for some other comments if any and publish another version of the series with comments addressed ("the" in the previous patch) The values below are per emulated multi-touch + * input device: + * + * num-contacts + * Values: + * + * Number of simultaneous touches reported. + * + * width + * Values: + * + * Width of the touch area to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * height + * Values: + * + * Height of the touch area to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. */ /* @@ -116,6 +151,16 @@ #define XENKBD_TYPE_RESERVED 2 #define XENKBD_TYPE_KEY3 #define XENKBD_TYPE_POS4 +#define XENKBD_TYPE_MTOUCH 5 + +/* Multi-touch event sub-codes */ + +#define XENKBD_MT_EV_DOWN 0 +#d
Re: [Xen-devel] [PATCH v1 2/2] xen/kbdif: add multi-touch support
On 01/21/2017 01:01 AM, Stefano Stabellini wrote: On Fri, 20 Jan 2017, Oleksandr Andrushchenko wrote: On 01/20/2017 07:52 PM, Stefano Stabellini wrote: On Fri, 20 Jan 2017, Oleksandr Andrushchenko wrote: On 01/20/2017 12:22 AM, Stefano Stabellini wrote: On Thu, 19 Jan 2017, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 216 ++ 1 file changed, 216 insertions(+) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index c00faa3af5d2..8d8ba9af1cb1 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -57,6 +57,12 @@ * Backends, which support reporting of absolute coordinates for pointer * device should set this to 1. * + * feature-multi-touch + * Values: + * + * Backends, which support reporting of multi-touch events + * should set this to 1. + * *- Pointer Device Parameters * * width @@ -87,6 +93,11 @@ * Request backend to report absolute pointer coordinates * (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION). * + * request-multi-touch + * Values: + * + * Request backend to report multi-touch events. + * *--- Request Transport Parameters --- * * event-channel @@ -106,6 +117,30 @@ * * OBSOLETE, not recommended for use. * PFN of the shared page. + * + *--- Multi-touch Device Parameters --- + * + * Every multi-touch input device uses a dedicated event frontend/backend + * connection configured via XenStore properties under + * XENKBD_PATH_MTOUCH folder. This sentence doesn't seem to match the rest of the patch: lets break it into smaller parts it looks like multi-touch devices use the same ring and evtchn used by the pointer and keyboard. *Every* multi-touch input device uses a *dedicated* event frontend/backend *connection* So, it is clearly stated that there is a dedicated/separate connection (which consists of a ring and corresponding event channel: here we have already discussed this part: http://marc.info/?l=xen-devel&m=148412731428686): + * Every multi-touch input device uses a dedicated event ring and is For clarity I would say "Every multi-touch input device uses a dedicated frontend/backend connection". That includes a ring, an event channel, and xenstore entries. Also, it is not clear whether multiple multitouch devices are supported with a single frontend/backend connection (as you described in http://marc.info/?l=xen-devel&m=148412731428686) or not. Please clarify. Same as above: *Every* multi-touch input device uses a *dedicated* event frontend/backend *connection* If only one device is supported, do we need a XENKBD_PATH_MTOUCH folder? For consistency and simplicity: at http://marc.info/?l=xen-devel&m=148412731428686 I have an example of how XenStore entries look like. If you define multiple mtouch devices then you'll iterate over folder contents, e.g. /local/domain/11/device/vkbd/0/mtouch/0/width = "3200" /local/domain/11/device/vkbd/0/mtouch/1/width = "6400" So, you'll have something like for (i = 0; i < num_mtouch_devices; i++) { process(i); } If you have a single mtouch device nothing changes. Thus, I see no reason in making any difference for single or multiple-devices. All right. I got confused because I read first the other email where you still suggested to use the other approach. Sorry. The wording is clear enough. No problem, glad we are on the same page now Aside from clarity, XENKBD_PATH_MTOUCH is important to distinguish the width and height parameters of the mtouch device from the ones of the pointers device, is that right? Exactly. Also for ring-ref and event-channel Sorry, now I am confused again :-S In the mail-thread you mentioned above there is a picture of the xenstore entries and conclusion: 1. No change to the existing kbd+ptr: 1.1. They still share a dedicated (single) connection channel as they did before multi-touch: page-ref + event-channel: /local/domain/2/device/vkbd/0/page-ref = "1248025" /local/domain/2/device/vkbd/0/page-gref = "336" /local/domain/2/device/vkbd/0/event-channel = "43" 1.2. No multi-touch events via that connection 1.3. Old backends/frontends will see no difference after multi-touch 2. Each multi-touch device has its own: 2.1. Configuration (width x height, num-contacts) 2.2. Own connection channel (page-ref, event-channel) /local/domain/2/device/vkbd/0/mtouch/0/width = "3200" /local/domain/2/device/vkbd/0/mtouch/0/height = "3200" /local/domain/2/device/vkbd/0/mtouch/0/num-contacts = "5" /local/domain/2/device/vkbd/0/mtouc
Re: [Xen-devel] [PATCH v1 2/2] xen/kbdif: add multi-touch support
On 01/23/2017 09:49 PM, Stefano Stabellini wrote: On Sat, 21 Jan 2017, Oleksandr Andrushchenko wrote: In the mail-thread you mentioned above there is a picture of the xenstore entries and conclusion: 1. No change to the existing kbd+ptr: 1.1. They still share a dedicated (single) connection channel as they did before multi-touch: page-ref + event-channel: /local/domain/2/device/vkbd/0/page-ref = "1248025" /local/domain/2/device/vkbd/0/page-gref = "336" /local/domain/2/device/vkbd/0/event-channel = "43" 1.2. No multi-touch events via that connection 1.3. Old backends/frontends will see no difference after multi-touch 2. Each multi-touch device has its own: 2.1. Configuration (width x height, num-contacts) 2.2. Own connection channel (page-ref, event-channel) /local/domain/2/device/vkbd/0/mtouch/0/width = "3200" /local/domain/2/device/vkbd/0/mtouch/0/height = "3200" /local/domain/2/device/vkbd/0/mtouch/0/num-contacts = "5" /local/domain/2/device/vkbd/0/mtouch/0/page-ref = "1252386" /local/domain/2/device/vkbd/0/mtouch/0/page-gref = "335" /local/domain/2/device/vkbd/0/mtouch/0/event-channel = "44" /local/domain/2/device/vkbd/0/mtouch/1/width = "6400" /local/domain/2/device/vkbd/0/mtouch/1/height = "6400" /local/domain/2/device/vkbd/0/mtouch/1/num-contacts = "10" /local/domain/2/device/vkbd/0/mtouch/1/page-ref = "2376038" /local/domain/2/device/vkbd/0/mtouch/1/page-gref = "337" /local/domain/2/device/vkbd/0/mtouch/1/event-channel = "45" So, for the example above: 1 kbd + 1 ptr + 2 mt devices within a single driver, *3 connections* There are no ring-ref and event-channel under mtouch, are there? there are ring-refs and event-channels under mtouch *per multi-touch* device Now, it is clear. Sorry it took so many back and forth. np page-ref and event-channel should definitely be described under "Multi-touch Device Parameters". When I asked you to remove them, I meant not just from the description but also from the protocol. This is where the confusion originated: in the current patch the two parameters are not described, hence I assumed they didn't exist and you were reusing the existing ring (/local/domain/2/device/vkbd/0/page-ref). understood Aside from new message structs, which look fine to me, we have two important choices to make: a) number of multitouch devices per PV frontend/backend connection (By frontend/backend connection, I mean by xenstore device, such as /local/domain/2/device/vkbd/0.) b) number of multitouch devices sharing the same ring (By ring I mean page-ref + event channel.) Both these choices need to be motivated. As I wrote in the past, I would go for 1 multitouch device per frontend/backend connection, reusing the existing ring (/local/domain/2/device/vkbd/0/page-ref and /local/domain/2/device/vkbd/0/event-channel), because I assume that there won't be many multitouch devices, less than 5, yes, I have 2 mt devices in my use-case so we can afford to create new PV devices for each of them. see at the bottom I am also assuming that it is fine for them to share ring with the keyboard or pointer devices without effects on performance because they should be all low frequency devices. sounds reasonable, but see below The current proposal supports multiple multitouch devices per frontend/backnend connection and allocates a new dedicated ring for each multitouch device. Your proposal might very well be the best solution, but why? The architectural choices need to be motivated. How many multitouch devices is reasonable to expect to be present on a platform? What is the multitouch events frequency we expect from them? The answer to the first question motivates choice a), the answer to the second question motivates choice b). Please add them both to the protocol description. It is OK to add the explanation to kbdif.h. You also mentioned something about multiple PV devices causing troubles to Linux. If that is an issue somehow, please add info about that too. Well, all the above sounds reasonable, some comments: 1. Event frequency: here we are talking about 60-120(?)Hz scan frequency of a touch screen generating at least 2 events for each contact: frame (syn) and position. So, we can estimate for 10 fingers the value to be 60 * 2 * 10 = 1200 events a second per multi-touch device. Of course, this is rough estimate which I believe would be smaller in real life. 2. "create new PV devices for each of them" 2.1. Driver This is something which you are seeing in xenstore as /local/domain/2/device/vkbd/0 0 - being index of the *driver*. And the challenge here is that if you want multiple *devices* to be allocated in this way: /local/domain/2/device/vkbd/0 /local/domain/2/device/vkbd/1 /local/domain/2/device/vkbd/2 then you have to register multiple *DRIVERS*. What we normally do in the PV
Re: [Xen-devel] [PATCH v1 2/2] xen/kbdif: add multi-touch support
On 01/25/2017 01:05 AM, Stefano Stabellini wrote: On Tue, 24 Jan 2017, Oleksandr Andrushchenko wrote: On 01/23/2017 09:49 PM, Stefano Stabellini wrote: On Sat, 21 Jan 2017, Oleksandr Andrushchenko wrote: In the mail-thread you mentioned above there is a picture of the xenstore entries and conclusion: 1. No change to the existing kbd+ptr: 1.1. They still share a dedicated (single) connection channel as they did before multi-touch: page-ref + event-channel: /local/domain/2/device/vkbd/0/page-ref = "1248025" /local/domain/2/device/vkbd/0/page-gref = "336" /local/domain/2/device/vkbd/0/event-channel = "43" 1.2. No multi-touch events via that connection 1.3. Old backends/frontends will see no difference after multi-touch 2. Each multi-touch device has its own: 2.1. Configuration (width x height, num-contacts) 2.2. Own connection channel (page-ref, event-channel) /local/domain/2/device/vkbd/0/mtouch/0/width = "3200" /local/domain/2/device/vkbd/0/mtouch/0/height = "3200" /local/domain/2/device/vkbd/0/mtouch/0/num-contacts = "5" /local/domain/2/device/vkbd/0/mtouch/0/page-ref = "1252386" /local/domain/2/device/vkbd/0/mtouch/0/page-gref = "335" /local/domain/2/device/vkbd/0/mtouch/0/event-channel = "44" /local/domain/2/device/vkbd/0/mtouch/1/width = "6400" /local/domain/2/device/vkbd/0/mtouch/1/height = "6400" /local/domain/2/device/vkbd/0/mtouch/1/num-contacts = "10" /local/domain/2/device/vkbd/0/mtouch/1/page-ref = "2376038" /local/domain/2/device/vkbd/0/mtouch/1/page-gref = "337" /local/domain/2/device/vkbd/0/mtouch/1/event-channel = "45" So, for the example above: 1 kbd + 1 ptr + 2 mt devices within a single driver, *3 connections* There are no ring-ref and event-channel under mtouch, are there? there are ring-refs and event-channels under mtouch *per multi-touch* device Now, it is clear. Sorry it took so many back and forth. np page-ref and event-channel should definitely be described under "Multi-touch Device Parameters". When I asked you to remove them, I meant not just from the description but also from the protocol. This is where the confusion originated: in the current patch the two parameters are not described, hence I assumed they didn't exist and you were reusing the existing ring (/local/domain/2/device/vkbd/0/page-ref). understood Aside from new message structs, which look fine to me, we have two important choices to make: a) number of multitouch devices per PV frontend/backend connection (By frontend/backend connection, I mean by xenstore device, such as /local/domain/2/device/vkbd/0.) b) number of multitouch devices sharing the same ring (By ring I mean page-ref + event channel.) Both these choices need to be motivated. As I wrote in the past, I would go for 1 multitouch device per frontend/backend connection, reusing the existing ring (/local/domain/2/device/vkbd/0/page-ref and /local/domain/2/device/vkbd/0/event-channel), because I assume that there won't be many multitouch devices, less than 5, yes, I have 2 mt devices in my use-case so we can afford to create new PV devices for each of them. see at the bottom I am also assuming that it is fine for them to share ring with the keyboard or pointer devices without effects on performance because they should be all low frequency devices. sounds reasonable, but see below The current proposal supports multiple multitouch devices per frontend/backnend connection and allocates a new dedicated ring for each multitouch device. Your proposal might very well be the best solution, but why? The architectural choices need to be motivated. How many multitouch devices is reasonable to expect to be present on a platform? What is the multitouch events frequency we expect from them? The answer to the first question motivates choice a), the answer to the second question motivates choice b). Please add them both to the protocol description. It is OK to add the explanation to kbdif.h. You also mentioned something about multiple PV devices causing troubles to Linux. If that is an issue somehow, please add info about that too. Well, all the above sounds reasonable, some comments: 1. Event frequency: here we are talking about 60-120(?)Hz scan frequency of a touch screen generating at least 2 events for each contact: frame (syn) and position. So, we can estimate for 10 fingers the value to be 60 * 2 * 10 = 1200 events a second per multi-touch device. Of course, this is rough estimate which I believe would be smaller in real life. 2. "create new PV devices for each of them" 2.1. Driver This is something which you are seeing in xenstore as /local/domain/2/device/vkbd/0 0 - being index of the *driver*. And the challenge here is that if you want multiple *devices* to be allocated in this way: /local/domain/2/device/vkbd/0 /local/domain/2/device/vkbd/1
[Xen-devel] [PATCH v2 0/2] xen/kbdif: add multi-touch support
From: Oleksandr Andrushchenko Hi, all! This series updates existing kbdif protocol documentation and adds multi-touch support Thank you, Oleksandr Andrushchenko Changes since v1: * removed mtouch folder * changed mtouch xenstore parameters' names * single multi-touch device per driver, if more needed multiple instances of the driver should be configured via xenstore entries Oleksandr Andrushchenko (2): xen/kbdif: update protocol documentation xen/kbdif: add multi-touch support xen/include/public/io/kbdif.h | 458 +++--- 1 file changed, 431 insertions(+), 27 deletions(-) -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 1/2] xen/kbdif: update protocol documentation
From: Oleksandr Andrushchenko Reviewed-by: Stefano Stabellini Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 248 +- 1 file changed, 221 insertions(+), 27 deletions(-) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index 2d2aebdd3f28..446aed2478b5 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -26,46 +26,226 @@ #ifndef __XEN_PUBLIC_IO_KBDIF_H__ #define __XEN_PUBLIC_IO_KBDIF_H__ -/* In events (backend -> frontend) */ +/* + * + * Feature and Parameter Negotiation + * + * + * The two halves of a para-virtual 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 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. + * + * + *Backend XenBus Nodes + * + * + * Features supported + * + * Capable backend advertises supported features by publishing + * corresponding entries in XenStore and puts 1 as the value of the entry. + * If a feature is not supported then 0 must be set or feature entry omitted. + * + * feature-abs-pointer + * Values: + * + * Backends, which support reporting of absolute coordinates for pointer + * device should set this to 1. + * + *- Pointer Device Parameters + * + * width + * Values: + * + * Maximum X coordinate (width) to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * height + * Values: + * + * Maximum Y coordinate (height) to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * + *Frontend XenBus Nodes + * + * + *-- Feature request - + * + * Capable frontend requests features from backend via setting corresponding + * entries to 1 in XenStore. Requests for features not advertised as supported + * by the backend have no effect. + * + * request-abs-pointer + * Values: + * + * Request backend to report absolute pointer coordinates + * (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION). + * + *--- Request Transport Parameters --- + * + * event-channel + * Values: + * + * The identifier of the Xen event channel used to signal activity + * in the ring buffer. + * + * page-gref + * Values: + * + * The Xen grant reference granting permission for the backend to map + * a sole page in a single page sized event ring buffer. + * + * page-ref + * Values: + * + * OBSOLETE, not recommended for use. + * PFN of the shared page. + */ /* - * Frontends should ignore unknown in events. + * EVENT CODES. */ -/* Pointer movement event */ -#define XENKBD_TYPE_MOTION 1 -/* Event type 2 currently not used */ -/* Key event (includes pointer buttons) */ -#define XENKBD_TYPE_KEY 3 +#define XENKBD_TYPE_MOTION 1 +#define XENKBD_TYPE_RESERVED 2 +#define XENKBD_TYPE_KEY3 +#define XENKBD_TYPE_POS4 + /* - * Pointer position event - * Capable backend sets feature-abs-pointer in xenstore. - * Frontend requests ot instead of XENKBD_TYPE_MOTION by setting - * request-abs-update in xenstore. + * CONSTANTS, XENSTORE FIELD AND PATH NAME STRINGS, HELPERS. + */ + +#define XENKBD_DRIVER_NAME "vkbd" + +#define XENKBD_FIELD_FEAT_ABS_POINTER "feature-abs-pointer" +#define XENKBD_FIELD_REQ_ABS_POINTER "request-abs-pointer" +#define XENKBD_FIELD_RING_GREF "page-gref" +#define XENKBD_FIELD_EVT_CHANNEL "event-channel" +#define XENKBD_FIELD_WIDTH "width" +#define XENKBD_FIELD_HEIGHT"height" + +/* OBSOLETE, not
[Xen-devel] [PATCH v2 2/2] xen/kbdif: add multi-touch support
From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 210 ++ 1 file changed, 210 insertions(+) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index 446aed2478b5..74883267d6e6 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -57,6 +57,12 @@ * Backends, which support reporting of absolute coordinates for pointer * device should set this to 1. * + * feature-multi-touch + * Values: + * + * Backends, which support reporting of multi-touch events + * should set this to 1. + * *- Pointer Device Parameters * * width @@ -87,6 +93,11 @@ * Request backend to report absolute pointer coordinates * (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION). * + * request-multi-touch + * Values: + * + * Request backend to report multi-touch events. + * *--- Request Transport Parameters --- * * event-channel @@ -106,6 +117,25 @@ * * OBSOLETE, not recommended for use. * PFN of the shared page. + * + *--- Multi-touch Device Parameters --- + * + * mt-num-contacts + * Values: + * + * Number of simultaneous touches reported. + * + * mt-width + * Values: + * + * Width of the touch area to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * mt-height + * Values: + * + * Height of the touch area to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. */ /* @@ -116,6 +146,16 @@ #define XENKBD_TYPE_RESERVED 2 #define XENKBD_TYPE_KEY3 #define XENKBD_TYPE_POS4 +#define XENKBD_TYPE_MTOUCH 5 + +/* Multi-touch event sub-codes */ + +#define XENKBD_MT_EV_DOWN 0 +#define XENKBD_MT_EV_UP1 +#define XENKBD_MT_EV_MOTION2 +#define XENKBD_MT_EV_SYN 3 +#define XENKBD_MT_EV_SHAPE 4 +#define XENKBD_MT_EV_ORIENT5 /* * CONSTANTS, XENSTORE FIELD AND PATH NAME STRINGS, HELPERS. @@ -124,11 +164,16 @@ #define XENKBD_DRIVER_NAME "vkbd" #define XENKBD_FIELD_FEAT_ABS_POINTER "feature-abs-pointer" +#define XENKBD_FIELD_FEAT_MTOUCH "feature-multi-touch" #define XENKBD_FIELD_REQ_ABS_POINTER "request-abs-pointer" +#define XENKBD_FIELD_REQ_MTOUCH"request-multi-touch" #define XENKBD_FIELD_RING_GREF "page-gref" #define XENKBD_FIELD_EVT_CHANNEL "event-channel" #define XENKBD_FIELD_WIDTH "width" #define XENKBD_FIELD_HEIGHT"height" +#define XENKBD_FIELD_MT_WIDTH "mt-width" +#define XENKBD_FIELD_MT_HEIGHT "mt-height" +#define XENKBD_FIELD_MT_NUM_CONTACTS "mt-num-contacts" /* OBSOLETE, not recommended for use */ #define XENKBD_FIELD_RING_REF "page-ref" @@ -248,6 +293,170 @@ struct xenkbd_position int32_t rel_z; }; +/* + * Multi-touch event and its sub-types + * + * All multi-touch event packets have common header: + * + * 01 2 3octet + * +++++ + * | _TYPE_MTOUCH | event_type | contact_id |reserved| 4 + * +++++ + * | reserved | 8 + * +++++ + * + * event_type - unt8_t, multi-touch event sub-type, XENKBD_MT_EV_??? + * contact_id - unt8_t, ID of the contact + * + * Touch interactions can consist of one or more contacts. + * For each contact, a series of events is generated, starting + * with a down event, followed by zero or more motion events, + * and ending with an up event. Events relating to the same + * contact point can be identified by the ID of the sequence: contact ID. + * Contact ID may be reused after XENKBD_MT_EV_UP event and + * is in the [0; XENKBD_FIELD_NUM_CONTACTS - 1] range. + * + * For further information please refer to documentation on Wayland [1], + * Linux [2] and Windows [3] multi-touch support. + * + * [1] https://cgit.freedesktop.org/wayland/wayland/tree/protocol/wayland.xml + * [2] https://www.kernel.org/doc/Documentation/input/multi-touch-protocol.txt + * [3] https://msdn.microsoft.com/en-us/library/jj151564(v=vs.85).aspx + * + * + * Multi-touch down event - sent when a new touch is made: touch is assigned + * a unique contact ID, sent with t
Re: [Xen-devel] [PATCH v15] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
Hi, Konrad! First of all thank you very much for the valuable comments and your time! The number of changes (mostly in description) is going to be huge, so do you think I can publish something like "RFC v16" so we can discuss the updated patch? Thank you, Oleksandr On 01/24/2017 09:13 PM, Konrad Rzeszutek Wilk wrote: On Mon, Dec 05, 2016 at 03:05:29PM +0200, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Usually one also puts somethign in the commit description. When I applied this to me tree I got: [konrad@char xen]$ git log --oneline HEAD^.. ecc7711 This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other. [konrad@char xen]$ fixed Also, thank you for the including the pahole output! np Signed-off-by: Oleksandr Andrushchenko Signed-off-by: Oleksandr Grytsov Signed-off-by: Oleksandr Dmytryshyn Signed-off-by: Iurii Konovalenko --- Changes since v1: * removed __attribute__((__packed__)) from all structures definitions Changes since v2: * removed all C structures * added protocol description between frontend and backend drivers Changes since v3: * fixed some typos * renamed XENSND_PCM_FORMAT_FLOAT_** to XENSND_PCM_FORMAT_F32_** * renamed XENSND_PCM_FORMAT_FLOAT64_** to XENSND_PCM_FORMAT_F64_** * added 'id' field to the request and response packets * renamed 'stream_id' to 'stream' in the packets description * renamed 'pcm_data_rate' to 'pcm_rate' in the packets description * renamed 'pcm_stream_type' to 'pcm_type' in the packets description * removed 'stream_id' field from the response packets Changes since v4: * renamed 'stream_id' back to the to 'stream' in the packets description * moved 'id' field to the upper position in the response packets Changes since v5: * Slightly reworked request/response packets * Size of the request/response packet is changed to the 64 bytes * Now parameters for the XENSND_OP_SET_VOLUME/XENSND_OP_GET_VOLUME are passed via shared page * Added parameters for the XenBus nodes (now each stream can be mapped to the defined sound device in the backend using those parameters) * Added XenBus state diagrams description Changes since v6: * Reworked streams description in the Backend XenBus Nodes Changes since v7: * re-worked backend device parameters to be more generic and flexible * extended frontend device parameters * slightly updated state machine description added mute/unmute commands * added constants for XenStore configuration strings (fields, PCM formats etc.) * changed request/response structure size from 64 octets to 16 * introduced dynamic buffer allocation instead of static XENSND_MAX_PAGES_PER_REQUEST * re-worked open request to allow dynamic buffer allocation * re-worked read/write/volume requests, so they don't pass grefs: buffer from the open request is used for these operations to pass data * specified type of the volume value to be a signed value in steps of 0.001 dBm, while 0 being 0dBm. * added Linux include file with structure definitions Changes since v8: * changed frontend-id to frontend_id * single sound card support, configured with bunch of devices/streams * clarifucation made on sample rates and formats expressed as decimals w/o any particular ordering * put description of migration/disconnection state * replaced __attribute__((packed)) to __packed * changed padding of ring structures to 64 to fit cache line * removeed #ifdef __KERNEL * explicitly stated which indices in XenStore configuration are contiguous * added description to what frontend's defaults are * made names of virtual card/devices optional * removed PCM_FORMAT_SPECIAL * changed volume units from dBm to dB Changes since v9: * removed sndif_linux.h * moved all structures from sndif_linux.h to sndif.h * structures padded where needed * fixed Hz comment Changes since v10: * fixed tabs to 4 spaces to comply with Xen coding style * added placeholders to empty structures (C89 concern) * added missing header includes Changes since v11: * added XENSND_RSP_NOTSUPP error code * changed gref[0] to gref[1] with comment * modified comments on empty structures * removed "__" from member names * fixed indentation * added padding in union xensnd_resp * changed __XEN_PUBLIC_IO_XENSND_H__ to __XEN_PUBLIC_IO_SNDIF_H__ Changes since v12: * changed indentation for defines * missed ";" after gref[1] * documentation changes * changed req/resp structures * changed xensnd_page_directory structure * pass buffer size in open request Changes since v13: * note on usage of grant ref 0 * all page sizes are XEN_PAGE_SIZE * padding/reserved cleanup/fixes * removed empty structures Changes since v14: * turn padding in
Re: [Xen-devel] [PATCH v15] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
On 01/26/2017 01:09 PM, Dario Faggioli wrote: On Thu, 2017-01-26 at 12:02 +0200, Oleksandr Andrushchenko wrote: Hi, Konrad! First of all thank you very much for the valuable comments and your time! The number of changes (mostly in description) is going to be huge, so do you think I can publish something like "RFC v16" so we can discuss the updated patch? Konrad's call, but why you want to introduce the 'RFC' tag now? I'd just go with v16... no particular reason, will use v16 ) On 01/24/2017 09:13 PM, Konrad Rzeszutek Wilk wrote: On Mon, Dec 05, 2016 at 03:05:29PM +0200, Oleksandr Andrushchenko wrote: + * Example for the frontend running in domain 5, instance of the driver + * in the front is 0 (single or first PV driver), device id 2, + * first stream (0): + * /local/domain//device/vsnd// + * device//stream//type = "p" + * /local/domain/5/device/vsnd/0/device/2/stream/0/type = "p" Why do you need 'device' ? just for clarity: the hierarchy of the sound driver would be that a device may have multiple different streams. So, from readability POV I would still have "device" in place From xenstore documentation: "Data should generally be human-readable for ease of management and debugging " I assume this also applies to the structure as well Perhaps: /local/domain//device/vsnd//dev-/stream-/... Could not this be: /local/domain/5/device/vsnd/0/2/stream/0/type = "p" ? then one has to know that "2" stands for device. see above, I would keep "device" here /local/domain/5/device/vsnd/0/dev-2/stream-0/type = "p" Or, with no '-': /local/domain/5/device/vsnd/0/dev2/stream0/type = "p" Just my 2 cents... 1. Well, the only reason I have "device" here is for clarity and consistency: sound card owns PCM devices, PCM device owns streams We could probably have "pcm-dev" instead of "device" here, so we do not collide with xen device. 2. "dev-%d" or "dev%d", "stream-%d" or "stream%d" IMO, we already have indices employed in xenstore, e.g. "domain/5", not "domain-5" or "domain5" So, is the PCM device in question any different from domain from this POV? To me - not, so this is why I use "device/%d" Dario Thank you, Oleksandr ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v15] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
On 01/26/2017 01:54 PM, Dario Faggioli wrote: On Thu, 2017-01-26 at 13:23 +0200, Oleksandr Andrushchenko wrote: On 01/26/2017 01:09 PM, Dario Faggioli wrote: On 01/24/2017 09:13 PM, Konrad Rzeszutek Wilk wrote: On Mon, Dec 05, 2016 at 03:05:29PM +0200, Oleksandr Andrushchenko wrote: + * Example for the frontend running in domain 5, instance of the driver + * in the front is 0 (single or first PV driver), device id 2, + * first stream (0): + * /local/domain//device/vsnd// + * device//stream//type = "p" + * /local/domain/5/device/vsnd/0/device/2/stream/0/type = "p" Why do you need 'device' ? Could not this be: /local/domain/5/device/vsnd/0/2/stream/0/type = "p" ? then one has to know that "2" stands for device. see above, I would keep "device" here /local/domain/5/device/vsnd/0/dev-2/stream-0/type = "p" Or, with no '-': /local/domain/5/device/vsnd/0/dev2/stream0/type = "p" Just my 2 cents... 1. Well, the only reason I have "device" here is for clarity and consistency: sound card owns PCM devices, PCM device owns streams We could probably have "pcm-dev" instead of "device" here, so we do not collide with xen device. Sure. Or maybe even just 'pcm' (matter of taste, to large extent). I would stick to "pcm-dev" then 2. "dev-%d" or "dev%d", "stream-%d" or "stream%d" IMO, we already have indices employed in xenstore, e.g. "domain/5", not "domain-5" or "domain5" So, is the PCM device in question any different from domain from this POV? To me - not, so this is why I use "device/%d" True. Well, actually, have both. For instance, blkif, when multiqueue is available are enabled, looks like this: /local/domain/1/device/vbd/0/multi-queue-num-queues = "2" /local/domain/1/device/vbd/0/queue-0 = "" /local/domain/1/device/vbd/0/queue-0/ring-ref = "" /local/domain/1/device/vbd/0/queue-0/event-channel = "" /local/domain/1/device/vbd/0/queue-1 = "" /local/domain/1/device/vbd/0/queue-1/ring-ref = "" /local/domain/1/device/vbd/0/queue-1/event-channel = "" Yeap, I saw this and was in doubt So, while I after all thing I agree with you on point 1) (i.e., on having device, or pcm-dev, or pcm, the latter being my prefernce), I think it would be ok to manage streams like blkif manages queues, and hence using stream-0, stream-1, etc. Ok, then we could have a formal rule for this: the last enumeration should follow "XXX-%d" format, e.g. "queue-%d", "stream-%d" etc. But entries, before this enum should follow "YYY/%d" format. Regards, Dario ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] POSIX error names and codes in PV protocols
Hi, all! There is some work happening on new PV protocols: sndif [1], displif [2], PV calls [3] and the common part of those is that error/status codes must be returned as a part of a response packet. For that Konrad suggested [1] (and Stefano already used in [3]) POSIX to be employed here instead of defining protocol specific error codes. The problem I see here is that POSIX only defines names of the errors, but not values [4]. So, in order to use POSIX one still needs to define the values (names must be the same, but values may differ for different OSes). So, the question is what would be the best option to a) have those numbers defined in OS agnostic way b) have those defined for all PV protocols Stefano has already defined the error code values in his work [3], but for other protocols this should be reimplemented again. Thank you, Oleksandr [1] https://marc.info/?l=xen-devel&m=148528530727366&w=2 [2] https://marc.info/?l=xen-devel&m=148239440624498&w=2 [3] https://marc.info/?l=xen-devel&m=148530334732400&w=2 [4] http://pubs.opengroup.org/onlinepubs/007908799/xsh/errno.h.html ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] POSIX error names and codes in PV protocols
On 01/26/2017 05:44 PM, Jan Beulich wrote: On 26.01.17 at 15:40, wrote: There is some work happening on new PV protocols: sndif [1], displif [2], PV calls [3] and the common part of those is that error/status codes must be returned as a part of a response packet. For that Konrad suggested [1] (and Stefano already used in [3]) POSIX to be employed here instead of defining protocol specific error codes. The problem I see here is that POSIX only defines names of the errors, but not values [4]. So, in order to use POSIX one still needs to define the values (names must be the same, but values may differ for different OSes). So, the question is what would be the best option to a) have those numbers defined in OS agnostic way b) have those defined for all PV protocols Stefano has already defined the error code values in his work [3], but for other protocols this should be reimplemented again. Aren't these simply what public/errno.h provides? Why would any Xen specific protocol want to define their own, now that we have this base set? Jan Indeed, thank you The problem is that it is not exposed to Linux, but I can see it in FreeBSD [1] and the helper to convert error codes [2] there as well. Is there any reason these are not available in Linux? Besides this fact, Konrad, Stefano, are you ok that we say in the protocol file that these are the error codes used? As error codes are unsigned ints, what should be used in packets? uint16_t or uint32_t? Thank you, Oleksandr [1] https://github.com/freebsd/freebsd/blob/master/sys/xen/interface/errno.h [2] https://github.com/freebsd/freebsd/blob/master/sys/xen/error.h ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] POSIX error names and codes in PV protocols
On 01/26/2017 07:38 PM, Roger Pau Monné wrote: On Thu, Jan 26, 2017 at 07:28:44PM +0200, Oleksandr Andrushchenko wrote: On 01/26/2017 05:44 PM, Jan Beulich wrote: On 26.01.17 at 15:40, wrote: There is some work happening on new PV protocols: sndif [1], displif [2], PV calls [3] and the common part of those is that error/status codes must be returned as a part of a response packet. For that Konrad suggested [1] (and Stefano already used in [3]) POSIX to be employed here instead of defining protocol specific error codes. The problem I see here is that POSIX only defines names of the errors, but not values [4]. So, in order to use POSIX one still needs to define the values (names must be the same, but values may differ for different OSes). So, the question is what would be the best option to a) have those numbers defined in OS agnostic way b) have those defined for all PV protocols Stefano has already defined the error code values in his work [3], but for other protocols this should be reimplemented again. Aren't these simply what public/errno.h provides? Why would any Xen specific protocol want to define their own, now that we have this base set? Jan Indeed, thank you The problem is that it is not exposed to Linux, but I can see it in FreeBSD [1] and the helper to convert error codes [2] there as well. Is there any reason these are not available in Linux? Xen error codes are Linux error codes, so I guess there's basically no need to use them on Linux (although it would be good, just so that people is aware that Xen and Linux are in different theoretical spaces, which happen to match in Linux's case). Roger. Thank you for clarifications. Does it mean that I can state in PV protocols that XEN_E??? error codes are used and still use Linux error codes directly in Linux front/back w/o complains from the community because of no conversion? :) ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1] displif: add ABI for para-virtual display
Hi, Jan! Does the below answer your question? Thank you, Oleksandr On 01/05/2017 08:07 PM, Oleksandr Andrushchenko wrote: On 01/05/2017 06:12 PM, Jan Beulich wrote: On 05.01.17 at 17:03, wrote: On 01/05/2017 05:45 PM, Jan Beulich wrote: On 22.12.16 at 09:12, wrote: Other than that the primary thing I'm missing (as I think I've mentioned elsewhere already) is a rationale of why this new protocol is needed (and the existing xenfb one can't be extended). "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" Well, that's all stuff you had spelled out in the accompanying mail, but that's all items which could be taken care of by a protocol extension too. of course I tried to evaluate what would it be like to extend existing fbif... It looks like having 2 different protocols in a single file. This is what I'd like you to expand on. To start with: 1. In/out event sizes o fbif - 40 octets o displif - 40 octets It fits now, but this is only the initial version of the displif protocol which means that there could be requests which will not fit (we are thinking of 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. This makes me believe if we extend fbif it is better to have separate structures/rings from the start. 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 I believe 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. BTW, I can add framebuffer's update and resize into displif, so it could probably supersede fbif at some point What is more fbif can be used together with displif running at the same time, e.g. on Linux one provides framebuffer and another DRM And this is certainly a valid argument (which hence should be spelled out in the description). ok Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] POSIX error names and codes in PV protocols
On 01/26/2017 08:16 PM, Roger Pau Monné wrote: On Thu, Jan 26, 2017 at 08:14:10PM +0200, Oleksandr Andrushchenko wrote: On 01/26/2017 07:38 PM, Roger Pau Monné wrote: On Thu, Jan 26, 2017 at 07:28:44PM +0200, Oleksandr Andrushchenko wrote: On 01/26/2017 05:44 PM, Jan Beulich wrote: On 26.01.17 at 15:40, wrote: There is some work happening on new PV protocols: sndif [1], displif [2], PV calls [3] and the common part of those is that error/status codes must be returned as a part of a response packet. For that Konrad suggested [1] (and Stefano already used in [3]) POSIX to be employed here instead of defining protocol specific error codes. The problem I see here is that POSIX only defines names of the errors, but not values [4]. So, in order to use POSIX one still needs to define the values (names must be the same, but values may differ for different OSes). So, the question is what would be the best option to a) have those numbers defined in OS agnostic way b) have those defined for all PV protocols Stefano has already defined the error code values in his work [3], but for other protocols this should be reimplemented again. Aren't these simply what public/errno.h provides? Why would any Xen specific protocol want to define their own, now that we have this base set? Jan Indeed, thank you The problem is that it is not exposed to Linux, but I can see it in FreeBSD [1] and the helper to convert error codes [2] there as well. Is there any reason these are not available in Linux? Xen error codes are Linux error codes, so I guess there's basically no need to use them on Linux (although it would be good, just so that people is aware that Xen and Linux are in different theoretical spaces, which happen to match in Linux's case). Roger. Thank you for clarifications. Does it mean that I can state in PV protocols that XEN_E??? error codes are used and still use Linux error codes directly in Linux front/back w/o complains from the community because of no conversion? :) I guess if it's inside of the Linux kernel that's fine (although that's a question for the Linux maintainers). If it runs in user-space you certainly need to use the XEN_E error codes. clear Roger. Thank you ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [DOC v8] PV Calls protocol design
Hi, Stefano! Error numbers The numbers corresponding to the error names specified by POSIX are: [EPERM] -1 [ENOENT]-2 Don't you want to use Xen's errno.h here as described in [1]? So we have error codes consistent for all PV protocols? Thanks, Oleksandr [1] https://marc.info/?l=xen-devel&m=148545604312317&w=2 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen, input: try to read screen resolution for xen-kbdfront
On 01/27/2017 09:12 AM, Juergen Gross wrote: Instead of using the default resolution of 800*600 for the pointing device of xen-kbdfront try to read the resolution of the (virtual) framebuffer device. Use the default as fallback only. Signed-off-by: Juergen Gross --- V2: get framebuffer resolution only if CONFIG_FB (Dmitry Torokhov) --- drivers/input/misc/xen-kbdfront.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c index 3900875..3aae9b4 100644 --- a/drivers/input/misc/xen-kbdfront.c +++ b/drivers/input/misc/xen-kbdfront.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -108,7 +109,7 @@ static irqreturn_t input_handler(int rq, void *dev_id) static int xenkbd_probe(struct xenbus_device *dev, const struct xenbus_device_id *id) { - int ret, i; + int ret, i, width, height; unsigned int abs; struct xenkbd_info *info; struct input_dev *kbd, *ptr; @@ -173,9 +174,17 @@ static int xenkbd_probe(struct xenbus_device *dev, ptr->id.product = 0xfffe; if (abs) { + width = XENFB_WIDTH; + height = XENFB_HEIGHT; +#ifdef CONFIG_FB + if (registered_fb[0]) { This still will not help if FB gets registered after kbd+ptr + width = registered_fb[0]->var.xres_virtual; + height = registered_fb[0]->var.yres_virtual; + } +#endif __set_bit(EV_ABS, ptr->evbit); - input_set_abs_params(ptr, ABS_X, 0, XENFB_WIDTH, 0, 0); - input_set_abs_params(ptr, ABS_Y, 0, XENFB_HEIGHT, 0, 0); + input_set_abs_params(ptr, ABS_X, 0, width, 0, 0); + input_set_abs_params(ptr, ABS_Y, 0, height, 0, 0); } else { input_set_capability(ptr, EV_REL, REL_X); input_set_capability(ptr, EV_REL, REL_Y); ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] Xen on lager for DOMU
On 01/26/2017 09:11 PM, Julien Grall wrote: On 24/01/2017 13:05, George John wrote: Hi all, Hello, I was able to bring up Dom0 in lager board by steps followed by charles. What could be the steps I could follow to bring up DomU in xen for lager board.?.. You can give a look to: - https://wiki.xen.org/wiki/Mainline_Linux_Kernel_Configs - https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions#DomU_kernel_and_DTS Alternatively, you can find a distribution where the kernel has Xen options enabled. For testing and some development on ARM64 I use Ubuntu for DomU [2], [3] It may be the case in Yocto, or you can look at centos (see [1]). How to bring xen-utils in the filesystem(yocto build) of Dom0 linux for configuring DomU. I don't know much about Yocto. I have CCed Iurri who I think is using Yocto. It might be able to help you here. Cheers, [1] https://blog.xenproject.org/2015/10/05/xen-in-centos-7-aarch64/ [2] https://github.com/xen-troops/manifests/wiki/DomU-Ubuntu-on-H3-M3 [3] https://help.ubuntu.com/community/Xen ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen, input: try to read screen resolution for xen-kbdfront
On 01/27/2017 09:46 AM, Juergen Gross wrote: On 27/01/17 08:21, Oleksandr Andrushchenko wrote: On 01/27/2017 09:12 AM, Juergen Gross wrote: Instead of using the default resolution of 800*600 for the pointing device of xen-kbdfront try to read the resolution of the (virtual) framebuffer device. Use the default as fallback only. Signed-off-by: Juergen Gross --- V2: get framebuffer resolution only if CONFIG_FB (Dmitry Torokhov) --- drivers/input/misc/xen-kbdfront.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c index 3900875..3aae9b4 100644 --- a/drivers/input/misc/xen-kbdfront.c +++ b/drivers/input/misc/xen-kbdfront.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -108,7 +109,7 @@ static irqreturn_t input_handler(int rq, void *dev_id) static int xenkbd_probe(struct xenbus_device *dev, const struct xenbus_device_id *id) { -int ret, i; +int ret, i, width, height; unsigned int abs; struct xenkbd_info *info; struct input_dev *kbd, *ptr; @@ -173,9 +174,17 @@ static int xenkbd_probe(struct xenbus_device *dev, ptr->id.product = 0xfffe; if (abs) { +width = XENFB_WIDTH; +height = XENFB_HEIGHT; +#ifdef CONFIG_FB +if (registered_fb[0]) { This still will not help if FB gets registered after kbd+ptr Hmm, so you think I should add a call to fb_register_client() to get events for new registered framebuffer devices? yes, but also pay attention to CONFIG_FB_NOTIFY: you may still end up w/o notification. This would probably work. I'll have a try. Thanks, Juergen My bigger concern here is that we try to tie keyboard and pointer device to the framebuffer. IMO, these are independent parts of the system and the relation depends on the use-case. One can have graphics enabled w/o framebuffer at all, e.g. DRM/KMS + OpenGLES + Weston + kbd + ptr... ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] POSIX error names and codes in PV protocols
On 01/27/2017 10:01 AM, Jan Beulich wrote: On 26.01.17 at 18:28, wrote: The problem is that it is not exposed to Linux, but I can see it in FreeBSD [1] and the helper to convert error codes [2] there as well. Is there any reason these are not available in Linux? Well, the header should eventually get added there (but see also Julien's reply as to why no-one may have cared so far). Besides this fact, Konrad, Stefano, are you ok that we say in the protocol file that these are the error codes used? As error codes are unsigned ints, what should be used in packets? uint16_t or uint32_t? Conventionally error codes are signed ints, and the width you need should be mostly dictated by other structure size constraints (i.e. if you have ample of space, int32_t may be more efficient to access, but if you have space restrictions, then int16_t should be fine as it can represent quite a bit larger a range than error codes will cover in the foreseeable future). yes, what I put in sndif now is: /* * STATUS RETURN CODES. * * Status return code is zero on success and -XEN_EXX on failure. */ ... * status - int32_t, response status, zero on success and -XEN_EXX on failure ... Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1] displif: add ABI for para-virtual display
BTW, what do you think about adding FB functionality into DISPLIF protocol? Of course it will duplicate FB, but allow future extensions On 01/27/2017 09:56 AM, Jan Beulich wrote: On 26.01.17 at 19:39, wrote: Does the below answer your question? I think that's fine, once added to the actual patch description. Jan On 01/05/2017 08:07 PM, Oleksandr Andrushchenko wrote: On 01/05/2017 06:12 PM, Jan Beulich wrote: On 05.01.17 at 17:03, wrote: On 01/05/2017 05:45 PM, Jan Beulich wrote: On 22.12.16 at 09:12, wrote: Other than that the primary thing I'm missing (as I think I've mentioned elsewhere already) is a rationale of why this new protocol is needed (and the existing xenfb one can't be extended). "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" Well, that's all stuff you had spelled out in the accompanying mail, but that's all items which could be taken care of by a protocol extension too. of course I tried to evaluate what would it be like to extend existing fbif... It looks like having 2 different protocols in a single file. This is what I'd like you to expand on. To start with: 1. In/out event sizes o fbif - 40 octets o displif - 40 octets It fits now, but this is only the initial version of the displif protocol which means that there could be requests which will not fit (we are thinking of 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. This makes me believe if we extend fbif it is better to have separate structures/rings from the start. 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 I believe 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. BTW, I can add framebuffer's update and resize into displif, so it could probably supersede fbif at some point What is more fbif can be used together with displif running at the same time, e.g. on Linux one provides framebuffer and another DRM And this is certainly a valid argument (which hence should be spelled out in the description). ok Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen, input: try to read screen resolution for xen-kbdfront
On 01/27/2017 10:14 AM, Juergen Gross wrote: On 27/01/17 08:53, Oleksandr Andrushchenko wrote: On 01/27/2017 09:46 AM, Juergen Gross wrote: On 27/01/17 08:21, Oleksandr Andrushchenko wrote: On 01/27/2017 09:12 AM, Juergen Gross wrote: Instead of using the default resolution of 800*600 for the pointing device of xen-kbdfront try to read the resolution of the (virtual) framebuffer device. Use the default as fallback only. Signed-off-by: Juergen Gross --- V2: get framebuffer resolution only if CONFIG_FB (Dmitry Torokhov) --- drivers/input/misc/xen-kbdfront.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c index 3900875..3aae9b4 100644 --- a/drivers/input/misc/xen-kbdfront.c +++ b/drivers/input/misc/xen-kbdfront.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -108,7 +109,7 @@ static irqreturn_t input_handler(int rq, void *dev_id) static int xenkbd_probe(struct xenbus_device *dev, const struct xenbus_device_id *id) { -int ret, i; +int ret, i, width, height; unsigned int abs; struct xenkbd_info *info; struct input_dev *kbd, *ptr; @@ -173,9 +174,17 @@ static int xenkbd_probe(struct xenbus_device *dev, ptr->id.product = 0xfffe; if (abs) { +width = XENFB_WIDTH; +height = XENFB_HEIGHT; +#ifdef CONFIG_FB +if (registered_fb[0]) { This still will not help if FB gets registered after kbd+ptr Hmm, so you think I should add a call to fb_register_client() to get events for new registered framebuffer devices? yes, but also pay attention to CONFIG_FB_NOTIFY: you may still end up w/o notification. Okay, that's not worse than today. agree This would probably work. I'll have a try. Thanks, Juergen My bigger concern here is that we try to tie keyboard and pointer device to the framebuffer. IMO, these are independent parts of the system and the relation depends on the use-case. One can have graphics enabled w/o framebuffer at all, e.g. DRM/KMS + OpenGLES + Weston + kbd + ptr... Again: that's a use case which will work as today. The current defaults are being used. The question is whether we should add a module parameter switching off the automatic adaption of the resolution as there might be use cases where we don't want this feature. I think for those who doesn't want this resolution there is still a possibility to change it on backend's XenbusStateConnected So, no need for module parameter, IMO Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v16] sndif: add ABI for para-virtual sound
From: Oleksandr Andrushchenko Hi, all! Please find the next version of the ABI for the PV sound after addressing review comments. Thank you, Oleksandr Andrushchenko Oleksandr Grytsov Oleksandr Andrushchenko (1): xen/sndif: Add sound-device ABI xen/include/public/io/sndif.h | 725 ++ 1 file changed, 725 insertions(+) create mode 100644 xen/include/public/io/sndif.h -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v16] xen/sndif: Add sound-device ABI
From: Oleksandr Andrushchenko This is the ABI for the two halves of a para-virtualized sound driver to communicate with each other. The ABI allows implementing audio playback and capture as well as volume control and possibility to mute/unmute audio sources. Note: depending on the use-case backend can expose more sound cards and PCM devices/streams than the underlying HW physically has by employing SW mixers, configuring virtual sound streams, channels etc. Thus, allowing fine tunned configurations per frontend. Signed-off-by: Oleksandr Andrushchenko Signed-off-by: Oleksandr Grytsov Signed-off-by: Oleksandr Dmytryshyn Signed-off-by: Iurii Konovalenko --- Changes since v1: * removed __attribute__((__packed__)) from all structures definitions Changes since v2: * removed all C structures * added protocol description between frontend and backend drivers Changes since v3: * fixed some typos * renamed XENSND_PCM_FORMAT_FLOAT_** to XENSND_PCM_FORMAT_F32_** * renamed XENSND_PCM_FORMAT_FLOAT64_** to XENSND_PCM_FORMAT_F64_** * added 'id' field to the request and response packets * renamed 'stream_id' to 'stream' in the packets description * renamed 'pcm_data_rate' to 'pcm_rate' in the packets description * renamed 'pcm_stream_type' to 'pcm_type' in the packets description * removed 'stream_id' field from the response packets Changes since v4: * renamed 'stream_id' back to the to 'stream' in the packets description * moved 'id' field to the upper position in the response packets Changes since v5: * Slightly reworked request/response packets * Size of the request/response packet is changed to the 64 bytes * Now parameters for the XENSND_OP_SET_VOLUME/XENSND_OP_GET_VOLUME are passed via shared page * Added parameters for the XenBus nodes (now each stream can be mapped to the defined sound device in the backend using those parameters) * Added XenBus state diagrams description Changes since v6: * Reworked streams description in the Backend XenBus Nodes Changes since v7: * re-worked backend device parameters to be more generic and flexible * extended frontend device parameters * slightly updated state machine description added mute/unmute commands * added constants for XenStore configuration strings (fields, PCM formats etc.) * changed request/response structure size from 64 octets to 16 * introduced dynamic buffer allocation instead of static XENSND_MAX_PAGES_PER_REQUEST * re-worked open request to allow dynamic buffer allocation * re-worked read/write/volume requests, so they don't pass grefs: buffer from the open request is used for these operations to pass data * specified type of the volume value to be a signed value in steps of 0.001 dBm, while 0 being 0dBm. * added Linux include file with structure definitions Changes since v8: * changed frontend-id to frontend_id * single sound card support, configured with bunch of devices/streams * clarifucation made on sample rates and formats expressed as decimals w/o any particular ordering * put description of migration/disconnection state * replaced __attribute__((packed)) to __packed * changed padding of ring structures to 64 to fit cache line * removeed #ifdef __KERNEL * explicitly stated which indices in XenStore configuration are contiguous * added description to what frontend's defaults are * made names of virtual card/devices optional * removed PCM_FORMAT_SPECIAL * changed volume units from dBm to dB Changes since v9: * removed sndif_linux.h * moved all structures from sndif_linux.h to sndif.h * structures padded where needed * fixed Hz comment Changes since v10: * fixed tabs to 4 spaces to comply with Xen coding style * added placeholders to empty structures (C89 concern) * added missing header includes Changes since v11: * added XENSND_RSP_NOTSUPP error code * changed gref[0] to gref[1] with comment * modified comments on empty structures * removed "__" from member names * fixed indentation * added padding in union xensnd_resp * changed __XEN_PUBLIC_IO_XENSND_H__ to __XEN_PUBLIC_IO_SNDIF_H__ Changes since v12: * changed indentation for defines * missed ";" after gref[1] * documentation changes * changed req/resp structures * changed xensnd_page_directory structure * pass buffer size in open request Changes since v13: * note on usage of grant ref 0 * all page sizes are XEN_PAGE_SIZE * padding/reserved cleanup/fixes * removed empty structures Changes since v14: * turn padding into reserved Changes since v15: * protocol description reworked * added offset to block notations * used -XEN_EXX for status reporting * added protocol versioning * removed stream_idx from the packet structure * changed xenstore paths --- Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/sndif.h | 725 +
[Xen-devel] [PATCH v2] displif: add ABI for para-virtual display
From: Oleksandr Andrushchenko 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 than vice versa. displif at the moment has 6 requests and 1 event, multiple connector support, etc. Changes since initial: * DRM changed to DISPL, protocol made generic * major re-work addressing issues raised for sndif Signed-off-by: Oleksandr Grytsov Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/displif.h | 745 1 file changed, 745 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 ..98321223e5c2 --- /dev/null +++ b/xen/include/public/io/displif.h @@ -0,0 +1,745 @@ +/** + * 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 Grytsov + */ + +#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
[Xen-devel] [PATCH v2] displif: add ABI for para-virtual display
From: Oleksandr Andrushchenko 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 Oleksandr Andrushchenko (1): displif: add ABI for para-virtual display xen/include/public/io/displif.h | 745 1 file changed, 745 insertions(+) create mode 100644 xen/include/public/io/displif.h -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] displif: add ABI for para-virtual display
PFA pahole output On 01/27/2017 03:08 PM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko 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 Oleksandr Andrushchenko (1): displif: add ABI for para-virtual display xen/include/public/io/displif.h | 745 1 file changed, 745 insertions(+) create mode 100644 xen/include/public/io/displif.h struct xendispl_dbuf_create_req { uint64_t dbuf_cookie; /* 0 8 */ uint32_t width;/* 8 4 */ uint32_t height; /*12 4 */ uint32_t bpp; /*16 4 */ uint32_t buffer_sz;/*20 4 */ uint32_t flags;/*24 4 */ grant_ref_tgref_directory_start; /*28 4 */ /* size: 32, cachelines: 1, members: 7 */ /* last cacheline: 32 bytes */ }; struct xendispl_page_directory { grant_ref_tgref_dir_next_page; /* 0 4 */ grant_ref_tgref[1]; /* 4 4 */ /* size: 8, cachelines: 1, members: 2 */ /* last cacheline: 8 bytes */ }; struct xendispl_dbuf_destroy_req { uint64_t dbuf_cookie; /* 0 8 */ /* size: 8, cachelines: 1, members: 1 */ /* last cacheline: 8 bytes */ }; struct xendispl_fb_attach_req { uint64_t dbuf_cookie; /* 0 8 */ uint64_t fb_cookie;/* 8 8 */ uint32_t width;/*16 4 */ uint32_t height; /*20 4 */ uint32_t pixel_format; /*24 4 */ /* size: 28, cachelines: 1, members: 5 */ /* last cacheline: 28 bytes */ }; struct xendispl_fb_detach_req { uint64_t fb_cookie;/* 0 8 */ /* size: 8, cachelines: 1, members: 1 */ /* last cacheline: 8 bytes */ }; struct xendispl_set_config_req { uint64_t fb_cookie;/* 0 8 */ uint32_t x;/* 8 4 */ uint32_t y;/*12 4 */ uint32_t width;/*16 4 */ uint32_t height; /*20 4 */ uint32_t bpp; /*24 4 */ /* size: 28, cachelines: 1, members: 6 */ /* last cacheline: 28 bytes */ }; struct xendispl_page_flip_req { uint64_t fb_cookie;/* 0 8 */ /* size: 8, cachelines: 1, members: 1 */ /* last cacheline: 8 bytes */ }; struct xendispl_pg_flip_evt { uint64_t fb_cookie;/* 0 8 */ /* size: 8, cachelines: 1, members: 1 */ /* last cacheline: 8 bytes */ }; struct xendispl_req { uint16_t id; /* 0 2 */ uint8_toperation;/* 2 1 */ uint8_treserved[5]; /* 3 5 */ union { struct xendispl_dbuf_create_req dbuf_create; /* 32 */ struct xendispl_dbuf_destroy_req dbuf_destroy; /* 8 */ struct xendispl_fb_attach_req fb_attach; /* 28 */ struct xendispl_fb_detach_req fb_detach; /* 8 */ struct xendispl_set_config_req set_config; /* 28 */ struct xendispl_page_flip_req pg_flip; /* 8 */ uint8_treserved[56]; /* 56 */ } op;/* 856 */ /* size: 64, cachelines: 1, members: 4 */ }; struct xendispl_resp { uint16_t id; /* 0
Re: [Xen-devel] [PATCH v16] sndif: add ABI for para-virtual sound
PFA pahole output On 01/27/2017 03:03 PM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Hi, all! Please find the next version of the ABI for the PV sound after addressing review comments. Thank you, Oleksandr Andrushchenko Oleksandr Grytsov Oleksandr Andrushchenko (1): xen/sndif: Add sound-device ABI xen/include/public/io/sndif.h | 725 ++ 1 file changed, 725 insertions(+) create mode 100644 xen/include/public/io/sndif.h struct xensnd_open_req { uint32_t pcm_rate; /* 0 4 */ uint8_tpcm_format; /* 4 1 */ uint8_tpcm_channels; /* 5 1 */ uint16_t reserved; /* 6 2 */ uint32_t buffer_sz;/* 8 4 */ grant_ref_tgref_directory_start; /*12 4 */ /* size: 16, cachelines: 1, members: 6 */ /* last cacheline: 16 bytes */ }; struct xensnd_page_directory { grant_ref_tgref_dir_next_page; /* 0 4 */ grant_ref_tgref[1]; /* 4 4 */ /* size: 8, cachelines: 1, members: 2 */ /* last cacheline: 8 bytes */ }; struct xensnd_rw_req { uint32_t offset; /* 0 4 */ uint32_t len; /* 4 4 */ /* size: 8, cachelines: 1, members: 2 */ /* last cacheline: 8 bytes */ }; struct xensnd_req { uint16_t id; /* 0 2 */ uint8_toperation;/* 2 1 */ uint8_treserved[5]; /* 3 5 */ union { struct xensnd_open_req open; /* 16 */ struct xensnd_rw_req rw; /* 8 */ uint8_treserved[24]; /* 24 */ } op;/* 824 */ /* size: 32, cachelines: 1, members: 4 */ /* last cacheline: 32 bytes */ }; struct xensnd_resp { uint16_t id; /* 0 2 */ uint8_toperation;/* 2 1 */ uint8_treserved; /* 3 1 */ int32_tstatus; /* 4 4 */ uint8_treserved1[24];/* 824 */ /* size: 32, cachelines: 1, members: 5 */ /* last cacheline: 32 bytes */ }; struct xensnd_open_req { uint32_t pcm_rate; /* 0 4 */ uint8_tpcm_format; /* 4 1 */ uint8_tpcm_channels; /* 5 1 */ uint16_t reserved; /* 6 2 */ uint32_t buffer_sz;/* 8 4 */ grant_ref_tgref_directory_start; /*12 4 */ /* size: 16, cachelines: 1, members: 6 */ /* last cacheline: 16 bytes */ }; struct xensnd_page_directory { grant_ref_tgref_dir_next_page; /* 0 4 */ grant_ref_tgref[1]; /* 4 4 */ /* size: 8, cachelines: 1, members: 2 */ /* last cacheline: 8 bytes */ }; struct xensnd_rw_req { uint32_t offset; /* 0 4 */ uint32_t len; /* 4 4 */ /* size: 8, cachelines: 1, members: 2 */ /* last cacheline: 8 bytes */ }; struct xensnd_req { uint16_t id; /* 0 2 */ uint8_toperation;/* 2 1 */ uint8_treserved[5]; /* 3 5 */ union { struct xensnd_open_req open; /* 16 */ struct xensnd_rw_req rw; /* 8 */ uint8_treserved[24]; /* 24 */ } op;/* 824 */ /* size: 32, cachelines: 1, members: 4 */ /* last cacheline: 32 bytes */ }; struct xensnd_resp { uint16_t id; /* 0 2 */ uint8_toperation;/* 2 1 */ uint8_treserved; /* 3 1 */ int32_tstatus; /* 4 4 */ uint8_treserved1[24];/* 824 */ /* size: 32, cachelines: 1, members: 5 */ /* last cacheline: 32 bytes
Re: [Xen-devel] [PATCH v15] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
thank you for comments, please find answers below Can we please switch to v16 discussion as v15 vs v16 is a big change? On 01/27/2017 05:14 PM, Konrad Rzeszutek Wilk wrote: On Thu, Jan 26, 2017 at 12:02:49PM +0200, Oleksandr Andrushchenko wrote: Hi, Konrad! First of all thank you very much for the valuable comments and your time! The number of changes (mostly in description) is going to be huge, so do you think I can publish something like "RFC v16" so we can discuss the updated patch? RFC sadly means folks are going to mostly ignore it. I would prefer you did not use RFC at this stage but just did v16. ..snip.. sure + * Example for the frontend running in domain 5, instance of the driver + * in the front is 0 (single or first PV driver), device id 2, + * first stream (0): + * /local/domain//device/vsnd// + * device//stream//type = "p" + * /local/domain/5/device/vsnd/0/device/2/stream/0/type = "p" Why do you need 'device' ? just for clarity: the hierarchy of the sound driver would be that a device may have multiple different streams. And it just occured to me that you could also imply that each device has an stream without the 'stream' in it. So /local/domain/5/device/vsnd/0/2/0/type = "p" And the format is: /local/domain//device/vsnd/// ok, so we'll end up with something like: - Backend --- /local/domain/0/backend/vsnd/1/0/frontend-id = "1" /local/domain/0/backend/vsnd/1/0/frontend = "/local/domain/1/device/vsnd/0" /local/domain/0/backend/vsnd/1/0/state = "4" /local/domain/0/backend/vsnd/1/0/versions = "1,2" - Card /local/domain/1/device/vsnd/0/version = "1" /local/domain/1/device/vsnd/0/short-name = "Card short name" /local/domain/1/device/vsnd/0/long-name = "Card long name" /local/domain/1/device/vsnd/0/sample-rates = "8000,32000,44100,48000,96000" /local/domain/1/device/vsnd/0/sample-formats = "s8,u8,s16_le,s16_be" /local/domain/1/device/vsnd/0/buffer-size = "262144" - PCM device 0 /local/domain/1/device/vsnd/0/0/name = "General analog" /local/domain/1/device/vsnd/0/0/channels-max = "5" - Stream 0, playback /local/domain/1/device/vsnd/0/0/0/type = "p" /local/domain/1/device/vsnd/0/0/0/sample-formats = "s8,u8" /local/domain/1/device/vsnd/0/0/0/unique-id = "0" /local/domain/1/device/vsnd/0/0/0/ring-ref = "386" /local/domain/1/device/vsnd/0/0/0/event-channel = "15" --- PCM device 3 /local/domain/1/device/vsnd/0/3/name = "HDMI-0" /local/domain/1/device/vsnd/0/3/sample-rates = "8000,32000,44100" -- Stream 0, capture /local/domain/1/device/vsnd/0/3/0/type = "c" /local/domain/1/device/vsnd/0/3/0/unique-id = "2" /local/domain/1/device/vsnd/0/3/0/ring-ref = "387" /local/domain/1/device/vsnd/0/3/0/event-channel = "151" Is this what you would like to see? IMO, all these values do not help understanding what it is, e.g. this is equal to me if we have /local/domain/1/device/vbd/51712/queue-0/ring-ref = "8" /local/domain/1/device/vbd/51712/queue-0/event-channel = "3" /local/domain/1/device/vbd/51712/queue-1 = "" /local/domain/1/device/vbd/51712/queue-1/ring-ref = "9" and then decided to go with /local/domain/1/device/vbd/51712/0/ring-ref = "8" /local/domain/1/device/vbd/51712/0/event-channel = "3" /local/domain/1/device/vbd/51712/1/ring-ref = "9" Can one easily tell what 0 or 1 after "51712/" is? So, what is the final decision then? Though I have a little of trouble with the 'instance of the driver'. Are you suggesting you would have multiple PV drivers of 'vsnd'? Can't the multiple device ids fulfill this? it is possible, but the main use-case will have a single PV driver with multiple PCM devices/streams So, from readability POV I would still have "device" in place From xenstore documentation: "Data should generally be human-readable for ease of management and debugging " I assume this also applies to the structure as well Could not this be: /local/domain/5/device/vsnd/0/2/stream/0/type = "p" ? then one has to know that "2" stands for device. see above, I would keep "device" here + * + *--- PCM settings + * + * Every virtualized sound frontend has set of devices and streams, each frontend or back
Re: [Xen-devel] [PATCH v15] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
On 01/27/2017 06:36 PM, Konrad Rzeszutek Wilk wrote: On Fri, Jan 27, 2017 at 05:50:36PM +0200, Oleksandr Andrushchenko wrote: thank you for comments, please find answers below Can we please switch to v16 discussion as v15 vs v16 is a big change? This channel seemed appropiate to hash this out. sorry about that I will look at v16 next week (out of time for reviews for today). thank you for your time See below. On 01/27/2017 05:14 PM, Konrad Rzeszutek Wilk wrote: On Thu, Jan 26, 2017 at 12:02:49PM +0200, Oleksandr Andrushchenko wrote: Hi, Konrad! First of all thank you very much for the valuable comments and your time! The number of changes (mostly in description) is going to be huge, so do you think I can publish something like "RFC v16" so we can discuss the updated patch? RFC sadly means folks are going to mostly ignore it. I would prefer you did not use RFC at this stage but just did v16. ..snip.. sure + * Example for the frontend running in domain 5, instance of the driver + * in the front is 0 (single or first PV driver), device id 2, + * first stream (0): + * /local/domain//device/vsnd// + * device//stream//type = "p" + * /local/domain/5/device/vsnd/0/device/2/stream/0/type = "p" Why do you need 'device' ? just for clarity: the hierarchy of the sound driver would be that a device may have multiple different streams. And it just occured to me that you could also imply that each device has an stream without the 'stream' in it. So /local/domain/5/device/vsnd/0/2/0/type = "p" And the format is: /local/domain//device/vsnd/// ok, so we'll end up with something like: - Backend --- /local/domain/0/backend/vsnd/1/0/frontend-id = "1" /local/domain/0/backend/vsnd/1/0/frontend = "/local/domain/1/device/vsnd/0" /local/domain/0/backend/vsnd/1/0/state = "4" /local/domain/0/backend/vsnd/1/0/versions = "1,2" - Card /local/domain/1/device/vsnd/0/version = "1" /local/domain/1/device/vsnd/0/short-name = "Card short name" /local/domain/1/device/vsnd/0/long-name = "Card long name" /local/domain/1/device/vsnd/0/sample-rates = "8000,32000,44100,48000,96000" /local/domain/1/device/vsnd/0/sample-formats = "s8,u8,s16_le,s16_be" /local/domain/1/device/vsnd/0/buffer-size = "262144" - PCM device 0 /local/domain/1/device/vsnd/0/0/name = "General analog" /local/domain/1/device/vsnd/0/0/channels-max = "5" - Stream 0, playback /local/domain/1/device/vsnd/0/0/0/type = "p" /local/domain/1/device/vsnd/0/0/0/sample-formats = "s8,u8" /local/domain/1/device/vsnd/0/0/0/unique-id = "0" /local/domain/1/device/vsnd/0/0/0/ring-ref = "386" /local/domain/1/device/vsnd/0/0/0/event-channel = "15" --- PCM device 3 /local/domain/1/device/vsnd/0/3/name = "HDMI-0" /local/domain/1/device/vsnd/0/3/sample-rates = "8000,32000,44100" -- Stream 0, capture /local/domain/1/device/vsnd/0/3/0/type = "c" /local/domain/1/device/vsnd/0/3/0/unique-id = "2" /local/domain/1/device/vsnd/0/3/0/ring-ref = "387" /local/domain/1/device/vsnd/0/3/0/event-channel = "151" Is this what you would like to see? Yes. ok, then I will use "/local/domain/1/device/vsnd" as the pattern, no "device", "stream" or whatever IMO, all these values do not help understanding what it is, e.g. this is equal to me if we have /local/domain/1/device/vbd/51712/queue-0/ring-ref = "8" /local/domain/1/device/vbd/51712/queue-0/event-channel = "3" /local/domain/1/device/vbd/51712/queue-1 = "" /local/domain/1/device/vbd/51712/queue-1/ring-ref = "9" and then decided to go with /local/domain/1/device/vbd/51712/0/ring-ref = "8" /local/domain/1/device/vbd/51712/0/event-channel = "3" /local/domain/1/device/vbd/51712/1/ring-ref = "9" Can one easily tell what 0 or 1 after "51712/" is? I do. But maybe my brain has been swimming in this too much? I am looking at this from FAE's or integrator's POV, when one would need to touch different parts of the system. "/0/0/0" makes me feel sad just because either you have to keep all those numbers in mind (like you do) or have documentation available (and know where it is, e.g. sources of Xen or kernel). I have a strong feeling that if you can change configuration without knowing what index stands for it is always
Re: [Xen-devel] [PATCH v15] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
On 01/27/2017 08:13 PM, Konrad Rzeszutek Wilk wrote: .snip.. I am looking at this from FAE's or integrator's POV, when one would need FAE? Field Applications Engineer to touch different parts of the system. "/0/0/0" makes me feel sad just because either you have to keep all those numbers in mind (like you do) or have documentation available (and know where it is, e.g. sources of Xen or kernel). I have a strong feeling that if you can change configuration without knowing what index stands for it is always better and fail-safer then just having numbers... Not sure I follow that. How would you change configuration without knowing the index? ..snip.. if one looks at .../pcm-dev-0/stream-1/... most probably he/she will understand this w/o any documentation, because it is human readable if one looks at .../0/1/... well, I believe you can almost do nothing w/o looking at the documentation ok, then struct xensnd_rw_req { uint32_t offset; uint32_t len; }; covers all the requests, but open/close Do you want me to keep the same structure name (xensnd_rw_req) or rename it to something like xensnd_io_req? The name is fine. Thank you, Oleksandr ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v15] This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other.
On 01/27/2017 08:57 PM, Konrad Rzeszutek Wilk wrote: On Fri, Jan 27, 2017 at 08:38:29PM +0200, Oleksandr Andrushchenko wrote: On 01/27/2017 08:13 PM, Konrad Rzeszutek Wilk wrote: .snip.. I am looking at this from FAE's or integrator's POV, when one would need FAE? Field Applications Engineer to touch different parts of the system. "/0/0/0" makes me feel sad just because either you have to keep all those numbers in mind (like you do) or have documentation available (and know where it is, e.g. sources of Xen or kernel). I have a strong feeling that if you can change configuration without knowing what index stands for it is always better and fail-safer then just having numbers... Not sure I follow that. How would you change configuration without knowing the index? ..snip.. if one looks at .../pcm-dev-0/stream-1/... most probably he/she will understand this w/o any documentation, because it is human readable if one looks at .../0/1/... well, I believe you can almost do nothing w/o looking at the documentation I can see the beaty of it. I can also see the beaty of the old implied mechanism from a maintaince perspective. I am maintainer so I am leaning towards the second one as having less "special" cases. Sorry, I feel like I am mutilating your baby with this old boring view of "maintaince" and "conform to the old standard" :-( np, so I will use ".../0/0/0/..." pattern here and in displif as well. I will update "Addressing" section so it is documented ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [DOC v8] PV Calls protocol design
On 01/27/2017 08:48 PM, Stefano Stabellini wrote: On Fri, 27 Jan 2017, Oleksandr Andrushchenko wrote: Hi, Stefano! Error numbers The numbers corresponding to the error names specified by POSIX are: [EPERM] -1 [ENOENT]-2 Don't you want to use Xen's errno.h here as described in [1]? So we have error codes consistent for all PV protocols? Thanks, Oleksandr [1] https://marc.info/?l=xen-devel&m=148545604312317&w=2 Hi Oleksandr, PVCalls is a bit different, because the protocol is meant to send POSIX calls to the backend, therefore, I have to use POSIX error names in the protocol. I could assign any numbers to the names though. It makes sense to use the Xen/Linux error numbers for simplicity. Whether I declare them directly as numbers as I have done here, or indirectly as XEN_ERRNO, I don't think it matters much. But I think that using numbers is clearer, that's why I did it that way. got it, thanks ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 2/2] xen/kbdif: add multi-touch support
Stefano, does the below look like you expected? All, any comments/objections? Thank you, Oleksandr On 01/26/2017 09:46 AM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko Signed-off-by: Oleksandr Andrushchenko --- xen/include/public/io/kbdif.h | 210 ++ 1 file changed, 210 insertions(+) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index 446aed2478b5..74883267d6e6 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -57,6 +57,12 @@ * Backends, which support reporting of absolute coordinates for pointer * device should set this to 1. * + * feature-multi-touch + * Values: + * + * Backends, which support reporting of multi-touch events + * should set this to 1. + * *- Pointer Device Parameters * * width @@ -87,6 +93,11 @@ * Request backend to report absolute pointer coordinates * (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION). * + * request-multi-touch + * Values: + * + * Request backend to report multi-touch events. + * *--- Request Transport Parameters --- * * event-channel @@ -106,6 +117,25 @@ * * OBSOLETE, not recommended for use. * PFN of the shared page. + * + *--- Multi-touch Device Parameters --- + * + * mt-num-contacts + * Values: + * + * Number of simultaneous touches reported. + * + * mt-width + * Values: + * + * Width of the touch area to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. + * + * mt-height + * Values: + * + * Height of the touch area to be used by the frontend + * while reporting input events, pixels, [0; UINT32_MAX]. */ /* @@ -116,6 +146,16 @@ #define XENKBD_TYPE_RESERVED 2 #define XENKBD_TYPE_KEY3 #define XENKBD_TYPE_POS4 +#define XENKBD_TYPE_MTOUCH 5 + +/* Multi-touch event sub-codes */ + +#define XENKBD_MT_EV_DOWN 0 +#define XENKBD_MT_EV_UP1 +#define XENKBD_MT_EV_MOTION2 +#define XENKBD_MT_EV_SYN 3 +#define XENKBD_MT_EV_SHAPE 4 +#define XENKBD_MT_EV_ORIENT5 /* * CONSTANTS, XENSTORE FIELD AND PATH NAME STRINGS, HELPERS. @@ -124,11 +164,16 @@ #define XENKBD_DRIVER_NAME "vkbd" #define XENKBD_FIELD_FEAT_ABS_POINTER "feature-abs-pointer" +#define XENKBD_FIELD_FEAT_MTOUCH "feature-multi-touch" #define XENKBD_FIELD_REQ_ABS_POINTER "request-abs-pointer" +#define XENKBD_FIELD_REQ_MTOUCH"request-multi-touch" #define XENKBD_FIELD_RING_GREF "page-gref" #define XENKBD_FIELD_EVT_CHANNEL "event-channel" #define XENKBD_FIELD_WIDTH "width" #define XENKBD_FIELD_HEIGHT"height" +#define XENKBD_FIELD_MT_WIDTH "mt-width" +#define XENKBD_FIELD_MT_HEIGHT "mt-height" +#define XENKBD_FIELD_MT_NUM_CONTACTS "mt-num-contacts" /* OBSOLETE, not recommended for use */ #define XENKBD_FIELD_RING_REF "page-ref" @@ -248,6 +293,170 @@ struct xenkbd_position int32_t rel_z; }; +/* + * Multi-touch event and its sub-types + * + * All multi-touch event packets have common header: + * + * 01 2 3octet + * +++++ + * | _TYPE_MTOUCH | event_type | contact_id |reserved| 4 + * +++++ + * | reserved | 8 + * +++++ + * + * event_type - unt8_t, multi-touch event sub-type, XENKBD_MT_EV_??? + * contact_id - unt8_t, ID of the contact + * + * Touch interactions can consist of one or more contacts. + * For each contact, a series of events is generated, starting + * with a down event, followed by zero or more motion events, + * and ending with an up event. Events relating to the same + * contact point can be identified by the ID of the sequence: contact ID. + * Contact ID may be reused after XENKBD_MT_EV_UP event and + * is in the [0; XENKBD_FIELD_NUM_CONTACTS - 1] range. + * + * For further information please refer to documentation on Wayland [1], + * Linux [2] and Windows [3] multi-touch support. + * + * [1] https://cgit.freedesktop.org/wayland/wayland/tree/protocol/wayland.xml + * [2] https://www.kernel.org/doc/Documentation/input/multi-touch-protocol.txt + * [3] https://msdn.microsoft.com/en-us/library/jj151564(v=vs
Re: [Xen-devel] [PATCH v2] xen, input: try to read screen resolution for xen-kbdfront
On 01/30/2017 01:23 PM, Juergen Gross wrote: On 27/01/17 17:10, Dmitry Torokhov wrote: On January 27, 2017 12:31:19 AM PST, Juergen Gross wrote: On 27/01/17 09:26, Oleksandr Andrushchenko wrote: On 01/27/2017 10:14 AM, Juergen Gross wrote: On 27/01/17 08:53, Oleksandr Andrushchenko wrote: On 01/27/2017 09:46 AM, Juergen Gross wrote: On 27/01/17 08:21, Oleksandr Andrushchenko wrote: On 01/27/2017 09:12 AM, Juergen Gross wrote: Instead of using the default resolution of 800*600 for the pointing device of xen-kbdfront try to read the resolution of the (virtual) framebuffer device. Use the default as fallback only. Signed-off-by: Juergen Gross --- V2: get framebuffer resolution only if CONFIG_FB (Dmitry Torokhov) --- drivers/input/misc/xen-kbdfront.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c index 3900875..3aae9b4 100644 --- a/drivers/input/misc/xen-kbdfront.c +++ b/drivers/input/misc/xen-kbdfront.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -108,7 +109,7 @@ static irqreturn_t input_handler(int rq, void *dev_id) static int xenkbd_probe(struct xenbus_device *dev, const struct xenbus_device_id *id) { -int ret, i; +int ret, i, width, height; unsigned int abs; struct xenkbd_info *info; struct input_dev *kbd, *ptr; @@ -173,9 +174,17 @@ static int xenkbd_probe(struct xenbus_device *dev, ptr->id.product = 0xfffe; if (abs) { +width = XENFB_WIDTH; +height = XENFB_HEIGHT; +#ifdef CONFIG_FB +if (registered_fb[0]) { This still will not help if FB gets registered after kbd+ptr Hmm, so you think I should add a call to fb_register_client() to get events for new registered framebuffer devices? yes, but also pay attention to CONFIG_FB_NOTIFY: you may still end up w/o notification. Okay, that's not worse than today. agree This would probably work. I'll have a try. Thanks, Juergen My bigger concern here is that we try to tie keyboard and pointer device to the framebuffer. IMO, these are independent parts of the system and the relation depends on the use-case. One can have graphics enabled w/o framebuffer at all, e.g. DRM/KMS + OpenGLES + Weston + kbd + ptr... Again: that's a use case which will work as today. The current defaults are being used. The question is whether we should add a module parameter switching off the automatic adaption of the resolution as there might be use cases where we don't want this feature. I think for those who doesn't want this resolution there is still a possibility to change it on backend's XenbusStateConnected So, no need for module parameter, IMO Fine. I'll send V3 soon. How about you do the axis adjustment from userspace (udev rule), and leave kernel as is? Hmm, is this a good idea? I'd need a udev rule to trigger when either the pointing device or a new frame buffer is showing up. In both cases I need to read the geometry of the frame buffer (in case it exists) and set the geometry of the pointing device (in case it exists) to the same values. This seems to be much more complicated than the required changes in the driver. I could be wrong, of course, especially as I'm no expert in writing udev rules. :-) And you may also end up with thin Dom0 w/o udev at all... Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Embedded-pv-devel] [PATCH v8] sndif: add ABI for Para-virtual sound
Hi, Lars! Thank you for your inputs, I've just added the persons you mentioned to the CC list and hope that someone will have time to review the protocol. Thank you, Oleksandr On 11/08/2016 03:11 PM, Lars Kurth wrote: Oleksandr, you may want to update the CC list, as there have been changes to maintainership. Remove Ian Campell and Keir Fraser, add Stefano Stabellini and Julien Grall . Also, as this is a change in the Hypervisor, you probably want to use xen-devel@ as the main mailing list. embedded-pv-devel is not properly moderated, which means that it may take time until a mail which is posted gets to everyone. The same applies to the other mail. Regards Lars On 4 Nov 2016, at 20:51, Andrushchenko, Oleksandr wrote: Hi, there! We would like to bring back to life this thread. We do hope that the re-worked version of the original patch addresses all the issues discussed before. What is more we already have a working version of the frontend and backend which prove that the protocol works (those are in the process of preparing to be pushed to the community as well). Hope you can review the changes and provide some feedback, so finally sound devices find their way in the world of Xen Best regards, Andrushchenko, Oleksandr Grytsov, Oleksandr Andrushchenko, Oleksandr (1): This is ABI for the two halves of a Para-virtual sound driver to communicate with each to other. xen/include/public/io/sndif.h | 583 xen/include/public/io/sndif_linux.h | 114 +++ 2 files changed, 697 insertions(+) create mode 100644 xen/include/public/io/sndif.h create mode 100644 xen/include/public/io/sndif_linux.h -- 2.7.4 ___ Embedded-pv-devel mailing list embedded-pv-de...@lists.xenproject.org https://lists.xenproject.org/cgi-bin/mailman/listinfo/embedded-pv-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v8] This is ABI for the two halves of a Para-virtual sound driver to communicate with each to other.
with XENSND_OP_OPEN is used to exchange mute/unmute >> + * values: >> + * >> + * 0 1 23 >> octet >> + * +-+-+-+-+ >> + * | channel[0]| channel[1]| channel[2]| channel[3]| >> + * +-+-+-+-+ >> + * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >> + * +-+-+-+-+ >> + * | channel[i]| channel[i+1] | channel[i+2] | channel[i+3] | >> + * +-+-+-+-+ >> + * +/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >> + * +-+-+-+-+ >> + * >> + * channel[i] - uint8_t, non-zero if i-th channel needs to be muted/unmuted >> + * Number of channels passed is equal to XENSND_OP_OPEN request pcm_channels >> + * field >> + * >> + * >> + * All response packets have the same length (64 bytes) >> + * >> + * Response for all requests: >> + * 0 1 23 >> octet >> + * +-+-+-+-+ >> + * | id|operation| stream_idx | >> + * +-+-+-+-+ >> + * | status | reserved | >> + * +-+-+-+-+ >> + * | reserved | >> + * +-+-+-+-+ >> + * | reserved | >> + * +-+-+-+-+ >> + * >> + * id - uint16_t, copied from the request >> + * stream_idx - uint8_t, copied from request >> + * operation - uint8_t, XENSND_OP_XXX - copied from request >> + * status - int8_t, response status (XENSND_RSP_???) > > Hmm, what about also including an 'int errno' which is more of an OS specific > value. > As per previous discussions we must stay away from some OS specifics and be agnostic to the OSes used by back and front. That said, what if front is Windows and back QNX? What if errno values differ? >> + */ >> + >> +struct xensnd_request { >> +uint8_t raw[16]; >> +}; > > How come you pick 16, not say 32 or something more cache-line friendly? > I was thinking about cache lines while picking 16. If I pick 32 or 64 will it always be of the cache line size? On ARM? On x86? On some other platform (remember, the requirement is to be agnostic...) > It is OK to add padding. >> + >> +struct xensnd_response { >> +uint8_t raw[16]; >> +}; >> + >> +#endif /* __XEN_PUBLIC_IO_XENSND_H__ */ >> diff --git a/xen/include/public/io/sndif_linux.h >> b/xen/include/public/io/sndif_linux.h >> new file mode 100644 >> index 000..58ec96c >> --- /dev/null >> +++ b/xen/include/public/io/sndif_linux.h >> @@ -0,0 +1,114 @@ >> +/* >> + * Unified sound-device I/O interface for Xen guest OSes >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 >> USA > > > You are putting an GPL header in the 'public' directory. > > The public headers should be BSD, see COPYING in the root of Xen > directory. > > Your previous file had the BSD, how come you are doing GPL here? > > Will fix > >> + * >> + * Copyright (C) 2016 EPAM Systems Inc. >> + */ >> + >> +#ifndef __XEN_PUBLIC_IO_XENSND_LINUX_H__ >> +#define __XEN_PUBLIC_IO_XENSND_LINUX_H__ >> + >> +#ifdef __KERNEL__ >> +#include >> +#include >> +#else >> +#include >> +#include >> +#endif >> +#if 0 >> +#include >> +#else >> +#warning "TODO: properly include base protocol header" > > Aha. Presumarily that should go away? > It has already in the PATCH v9 >> +#include "sndif.h" >> +#endif >> + >> +struct xensnd_open_req { >> + uint32_t pcm_rate; >> + uint8_t pcm_format; >> + uint8_t pcm_channels; >> + /* in Hz */ >> + uint16_t __reserved0; >> + grant_ref_t gref_directory_start; >> +} __attribute__((packed)); > > Hm, I recall folks frowning on using GCC-ism like this. These are > public headers and can be ingested by compilers like Microsoft > or other that don't grok __packet__. > > Could you remove it and still make sure the structure has the right > size/offset? pahole helps a lot. > As was requested in the previous discussions of this protocol we should have structs and packaging options in the protocol header file. For that same reason I have created a Linux specific file which can be used on Unix systems with gcc. So, once someone needs to run on Windows they will add their own platform specific header... >> + >> +struct xensnd_page_directory { >> + grant_ref_t gref_dir_next_page; >> + uint32_t num_grefs; >> + grant_ref_t gref[0]; >> +} __attribute__((packed)); >> + >> +struct xensnd_close_req { >> +} __attribute__((packed)); >> + >> +struct xensnd_write_req { >> + uint32_t offset; >> + uint32_t len; >> +} __attribute__((packed)); >> + >> +struct xensnd_read_req { >> + uint32_t offset; >> + uint32_t len; >> +} __attribute__((packed)); >> + >> +struct xensnd_get_vol_req { >> +} __attribute__((packed)); >> + >> +struct xensnd_set_vol_req { >> +} __attribute__((packed)); >> + >> +struct xensnd_mute_req { >> +} __attribute__((packed)); >> + >> +struct xensnd_unmute_req { >> +} __attribute__((packed)); >> + >> +struct xensnd_req { >> + union { >> + struct xensnd_request raw; >> + struct { >> + uint16_t id; >> + uint8_t operation; >> + uint8_t stream_idx; >> + union { >> + struct xensnd_open_req open; >> + struct xensnd_close_req close; >> + struct xensnd_write_req write; >> + struct xensnd_read_req read; >> + struct xensnd_get_vol_req get_vol; >> + struct xensnd_set_vol_req set_vol; >> + struct xensnd_mute_req mute; >> + struct xensnd_unmute_req unmute; >> + } op; >> + } data; >> + } u; >> +}; >> + >> +struct xensnd_resp { >> + union { >> + struct xensnd_response raw; >> + struct { >> + uint16_t id; >> + uint8_t operation; >> + uint8_t stream_idx; >> + int8_t status; >> + } data; >> + } u; >> +}; >> + >> +DEFINE_RING_TYPES(xen_sndif, struct xensnd_req, >> + struct xensnd_resp); >> + >> +#endif /* __XEN_PUBLIC_IO_XENSND_LINUX_H__ */ >> -- >> 2.7.4 >> >> >> ___ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> https://lists.xen.org/xen-devel -- Best regards, Oleksandr Andrushchenko ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] DomU application crashes while mmap'ing device memory on x86_64
der cc_compiler: gcc (Debian 6.2.0-10) 6.2.0 20161027 cc_compile_by : ijackson cc_compile_domain : chiark.greenend.org.uk cc_compile_date: Tue Nov 1 18:11:16 UTC 2016 build_id : 3744fa5e7a5b01a0439ba4413e41a7a1c505d5ee xend_config_format : 4 2. DomU Linux DomU 4.9.0-040900rc3-generic #201610291831 SMP Sat Oct 29 22:32:46 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Could anyone please give me any hint on what needs to be checked and how this can be resolved? Thank you, Oleksandr Andrushchenko ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v8] This is ABI for the two halves of a Para-virtual sound driver to communicate with each to other.
If these are all the comments then I'll start preparing patch v9 Thank you all for reviewing and providing feedback Oleksandr Andrushchenko ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v8] This is ABI for the two halves of a Para-virtual sound driver to communicate with each to other.
Thank you all for the review and comments! I guess only these are not addressed (or still need comments): > 1. You can only say how many channels a stream has, not what the > channels correspond to (e.g., "left", "right" for a 2 channel stereo > stream; or "left-front", "rear-right", etc. for a 6 channel 5.1 surround > sound stream). PV driver may be configured to have number of channels which are not supported by the back's HW. In that case this is up to back how to treat those. If we introduce some specific ordering or mapping that will only be a recommendation to the back, which it may follow. Thus, I would stick to what we have now > 3. For the PCM formats you need to precisely specify the format or > reference external specifications that do so. For example, "mpeg" is a > bit vague as I'm sure this standard body has produced many different > audio formats. The specifications don't need to be freely available, > just correctly referenced. I do understand the intent here, but I blieve we cannot make the protocol being simple and fit all at once. So, I would suggest leaving it as is at least for now or removing it. > 4. 8 bits for stream index, operation and (particularly) pcm format feel > a little limiting. 8 bits mean you can have 256 streams, operations and formats (which are indices). I would prefer to stay with 8 bits > 5. For the read/write operations is the stream buffer considered to be > circular? I think it probably should be. I guess it shouldn't. The reason of having offset is the fact that software on front side may mmap part of the buffer and tell the driver the offset of the "dirty" region of the buffer. Please find updated list of TODOs for the next version: 1. Change frontend-id to frontend_id 2. Have a single sound card configured with bunch of different devices/streams 3. State that sample rates and formats expressed as decimals w/o any particular ordering 4. Put description of migration/disconnection state 5. Change __attribute__((packed)) to __packed (scripts/checkpatch.pl) 6. Padding to fit cache line? What is its size (ARM/x86...) - need to discuss 7. Change GPL header to BSD 8. Remove #ifdef __KERNEL 9. Remove "Multiple PV drivers are allowed in the domain at the same time." 10. Support a single card as device/stream configuration allows fine tuning already 11. Explicitly say which indices in XenStore configuration are contiguous 12. For strings "If not defined then use frontend's default." add more description: "This default is depends on concrete PV frontend implementation" 13. Make names of cards/devices optional 14. Remove PCM_FORMAT_SPECIAL 15. Change volume units from dBm to dB Thank you, Oleksandr ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] DomU application crashes while mmap'ing device memory on x86_64
Hi, just wanted to bump this as I also have the same issue on real HW now (x86_64) Nov 14 10:30:18 DomU kernel: [ 1169.569936] [] xen_mc_flush+0x19c/0x1b0 Thank you in advnce, Oleksandr On Mon, Nov 14, 2016 at 6:07 PM, Oleksandr Andrushchenko wrote: > Hi, there! > > Sorry for the long read ahead, but it seems I've got stuck... > > I am working on a PV driver and facing an mmap issue. > This actually happens when user-space tries to mmap > the memory allocated by the driver: > > cma_obj->vaddr = dma_alloc_wc(drm->dev, size, &cma_obj->paddr, > GFP_KERNEL | __GFP_NOWARN); > > and maping: > > vma->vm_flags &= ~VM_PFNMAP; > vma->vm_pgoff = 0; > > ret = dma_mmap_wc(cma_obj->base.dev->dev, vma, cma_obj->vaddr, > cma_obj->paddr, vma->vm_end - vma->vm_start); > > Return of the dma_mmap_wc is 0, but I see in the DomU kernel logs: > > Nov 14 10:30:18 DomU kernel: [ 1169.569909] [ cut here > ] > Nov 14 10:30:18 DomU kernel: [ 1169.569911] WARNING: CPU: 1 PID: 5146 at > /home/kernel/COD/linux/arch/x86/xen/multicalls.c:129 > xen_mc_flush+0x19c/0x1b0 > Nov 14 10:30:18 DomU kernel: [ 1169.569912] Modules linked in: > xen_drmfront(OE) drm_kms_helper(OE) drm(OE) fb_sys_fops syscopyarea > sysfillrect sysimgblt crct10dif_pclmul crc32_pclmul ghash_clmulni_intel > aesni_intel aes_x86_64 lrw glue_helper ablk_helper cryptd intel_rapl_perf > autofs4 [last unloaded: xen_drmfront] > Nov 14 10:30:18 DomU kernel: [ 1169.569919] CPU: 1 PID: 5146 Comm: > lt-modetest Tainted: GW OE 4.9.0-040900rc3-generic #201610291831 > Nov 14 10:30:18 DomU kernel: [ 1169.569920] c900406ffb10 > 81416bf2 > Nov 14 10:30:18 DomU kernel: [ 1169.569923] c900406ffb50 > 8108361b 0081406ffb30 88003f90b8e0 > Nov 14 10:30:18 DomU kernel: [ 1169.569925] 0001 > 0010 0201 > Nov 14 10:30:18 DomU kernel: [ 1169.569928] Call Trace: > Nov 14 10:30:18 DomU kernel: [ 1169.569930] [] > dump_stack+0x63/0x81 > Nov 14 10:30:18 DomU kernel: [ 1169.569932] [] > __warn+0xcb/0xf0 > Nov 14 10:30:18 DomU kernel: [ 1169.569934] [] > warn_slowpath_null+0x1d/0x20 > Nov 14 10:30:18 DomU kernel: [ 1169.569936] [] > xen_mc_flush+0x19c/0x1b0 > Nov 14 10:30:18 DomU kernel: [ 1169.569938] [] > __xen_mc_entry+0xf6/0x150 > Nov 14 10:30:18 DomU kernel: [ 1169.569940] [] > xen_extend_mmu_update+0x56/0xd0 > Nov 14 10:30:18 DomU kernel: [ 1169.569942] [] > xen_set_pte_at+0x177/0x2f0 > Nov 14 10:30:18 DomU kernel: [ 1169.569944] [] > remap_pfn_range+0x30b/0x430 > Nov 14 10:30:18 DomU kernel: [ 1169.569946] [] > dma_common_mmap+0x87/0xa0 > Nov 14 10:30:18 DomU kernel: [ 1169.569953] [] > drm_gem_cma_mmap_obj+0x8f/0xa0 [drm] > Nov 14 10:30:18 DomU kernel: [ 1169.569960] [] > drm_gem_cma_mmap+0x25/0x30 [drm] > Nov 14 10:30:18 DomU kernel: [ 1169.569962] [] > mmap_region+0x3a5/0x640 > Nov 14 10:30:18 DomU kernel: [ 1169.569964] [] > do_mmap+0x446/0x530 > Nov 14 10:30:18 DomU kernel: [ 1169.569966] [] ? > common_mmap+0x45/0x50 > Nov 14 10:30:18 DomU kernel: [ 1169.569968] [] ? > apparmor_mmap_file+0x16/0x20 > Nov 14 10:30:18 DomU kernel: [ 1169.569970] [] ? > security_mmap_file+0xdd/0xf0 > Nov 14 10:30:18 DomU kernel: [ 1169.569972] [] > vm_mmap_pgoff+0xba/0xf0 > Nov 14 10:30:18 DomU kernel: [ 1169.569974] [] > SyS_mmap_pgoff+0x1c1/0x290 > Nov 14 10:30:18 DomU kernel: [ 1169.569976] [] > SyS_mmap+0x1b/0x30 > Nov 14 10:30:18 DomU kernel: [ 1169.569978] [] > entry_SYSCALL_64_fastpath+0x1e/0xad > Nov 14 10:30:18 DomU kernel: [ 1169.569979] ---[ end trace > ce1796cb265ebe08 ]--- > Nov 14 10:30:18 DomU kernel: [ 1169.569982] [ cut here > ] > > > And output of xl dmesg says: > > (XEN) memory.c:226:d0v0 Could not allocate order=9 extent: id=31 > memflags=0x40 (488 of 512) > (d31) mapping kernel into physical memory > (d31) about to get started... > (XEN) d31 attempted to change d31v0's CR4 flags 0620 -> 00040660 > (XEN) d31 attempted to change d31v1's CR4 flags 0620 -> 00040660 > (XEN) traps.c:3657: GPF (): 82d0801a1a09 -> 82d08024b970 > (XEN) mm.c:1893:d31v0 Bad L1 flags 90 > (XEN) mm.c:1893:d31v0 Bad L1 flags 90 > (XEN) mm.c:1893:d31v0 Bad L1 flags 90 > (XEN) mm.c:1893:d31v0 Bad L1 flags 90 > > My setup is a little bit tricky... I am using a Xen setup running > inside VirtualBox: > > 1. xl info: > host : Dom0 > release: 4.4.0-45-generic > version: #66-Ubuntu SMP Wed Oct 19 14:12:37 UTC 2016 > machine: x86_64 > nr
[Xen-devel] [PATCH v9] xen: add para-virtual sound interface header files
This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other. Signed-off-by: Oleksandr Andrushchenko Signed-off-by: Oleksandr Grytsov Signed-off-by: Oleksandr Dmytryshyn Signed-off-by: Iurii Konovalenko --- Changes since v1: * removed __attribute__((__packed__)) from all structures definitions Changes since v2: * removed all C structures * added protocol description between frontend and backend drivers Changes since v3: * fixed some typos * renamed XENSND_PCM_FORMAT_FLOAT_** to XENSND_PCM_FORMAT_F32_** * renamed XENSND_PCM_FORMAT_FLOAT64_** to XENSND_PCM_FORMAT_F64_** * added 'id' field to the request and response packets * renamed 'stream_id' to 'stream' in the packets description * renamed 'pcm_data_rate' to 'pcm_rate' in the packets description * renamed 'pcm_stream_type' to 'pcm_type' in the packets description * removed 'stream_id' field from the response packets Changes since v4: * renamed 'stream_id' back to the to 'stream' in the packets description * moved 'id' field to the upper position in the response packets Changes since v5: * Slightly reworked request/response packets * Size of the request/response packet is changed to the 64 bytes * Now parameters for the XENSND_OP_SET_VOLUME/XENSND_OP_GET_VOLUME are passed via shared page * Added parameters for the XenBus nodes (now each stream can be mapped to the defined sound device in the backend using those parameters) * Added XenBus state diagrams description Changes since v6: * Reworked streams description in the Backend XenBus Nodes Changes since v7: * re-worked backend device parameters to be more generic and flexible * extended frontend device parameters * slightly updated state machine description added mute/unmute commands * added constants for XenStore configuration strings (fields, PCM formats etc.) * changed request/response structure size from 64 octets to 16 * introduced dynamic buffer allocation instead of static XENSND_MAX_PAGES_PER_REQUEST * re-worked open request to allow dynamic buffer allocation * re-worked read/write/volume requests, so they don't pass grefs: buffer from the open request is used for these operations to pass data * specified type of the volume value to be a signed value in steps of 0.001 dBm, while 0 being 0dBm. * added Linux include file with structure definitions Changes since v8: * changed frontend-id to frontend_id * single sound card support, configured with bunch of devices/streams * clarifucation made on sample rates and formats expressed as decimals w/o any particular ordering * put description of migration/disconnection state * replaced __attribute__((packed)) to __packed * changed padding of ring structures to 64 to fit cache line * removeed #ifdef __KERNEL * explicitly stated which indices in XenStore configuration are contiguous * added description to what frontend's defaults are * made names of virtual card/devices optional * removed PCM_FORMAT_SPECIAL * changed volume units from dBm to dB --- include/xen/interface/io/sndif.h | 609 + include/xen/interface/io/sndif_linux.h | 112 ++ 2 files changed, 721 insertions(+) create mode 100644 include/xen/interface/io/sndif.h create mode 100644 include/xen/interface/io/sndif_linux.h diff --git a/include/xen/interface/io/sndif.h b/include/xen/interface/io/sndif.h new file mode 100644 index 000..06079c7 --- /dev/null +++ b/include/xen/interface/io/sndif.h @@ -0,0 +1,609 @@ +/** + * sndif.h + * + * Unified sound-device I/O interface for Xen guest OSes. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * + * Copyright (C) 2013-2015 GlobalLogic Inc. + * Copy
[Xen-devel] [PATCH v9] sndif: add ABI for para-virtual sound
Hi, all! First of all we would like to thank you for valuable comments! Please find the next version of the ABI for the PV sound. Thank you, Oleksandr Andrushchenko Oleksandr Grytsov Oleksandr Andrushchenko (1): xen: add para-virtual sound interface header files include/xen/interface/io/sndif.h | 609 + include/xen/interface/io/sndif_linux.h | 112 ++ 2 files changed, 721 insertions(+) create mode 100644 include/xen/interface/io/sndif.h create mode 100644 include/xen/interface/io/sndif_linux.h -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] drmif: add ABI for para-virtual DRM/KMS
Hi, all! This patch adds support for para-virtualized DRM/KMS. Comments, ideas are more than welcome. With best regards, Oleksandr Andrushchenko Oleksandr Grytsov Oleksandr Andrushchenko (1): drmif: add ABI for para-virtual DRM/KMS include/xen/interface/io/drmif.h | 505 + include/xen/interface/io/drmif_linux.h | 142 + 2 files changed, 647 insertions(+) create mode 100644 include/xen/interface/io/drmif.h create mode 100644 include/xen/interface/io/drmif_linux.h -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] drmif: add ABI for para-virtual DRM/KMS
This is the ABI for the two halves of a para-virtualized DRM/KMS driver. Signed-off-by: Oleksandr Andrushchenko Signed-off-by: Oleksandr Grytsov --- include/xen/interface/io/drmif.h | 505 + include/xen/interface/io/drmif_linux.h | 142 + 2 files changed, 647 insertions(+) create mode 100644 include/xen/interface/io/drmif.h create mode 100644 include/xen/interface/io/drmif_linux.h diff --git a/include/xen/interface/io/drmif.h b/include/xen/interface/io/drmif.h new file mode 100644 index 000..82f49e0 --- /dev/null +++ b/include/xen/interface/io/drmif.h @@ -0,0 +1,505 @@ +/** + * drmif.h + * + * Unified DRM-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 EPAM Systems Inc. + * + * Authors: Oleksandr Andrushchenko + * Oleksandr Grytsov + */ + +#ifndef __XEN_PUBLIC_IO_XENDRM_H__ +#define __XEN_PUBLIC_IO_XENDRM_H__ + +/* + * Front->back notifications: When enqueuing a new request, sending a + * notification can be made conditional on req_event (i.e., the generic + * hold-off mechanism provided by the ring macros). Backends must set + * req_event appropriately (e.g., using RING_FINAL_CHECK_FOR_REQUESTS()). + * + * Back->front notifications: When enqueuing a new response, sending a + * notification can be made conditional on rsp_event (i.e., the generic + * hold-off mechanism provided by the ring macros). Frontends must set + * rsp_event appropriately (e.g., using RING_FINAL_CHECK_FOR_RESPONSES()). + */ + +/* + * Feature and Parameter Negotiation + * = + * The two halves of a para-virtual DRM driver utilize nodes within the + * XenStore to communicate capabilities and to negotiate operating parameters. + * This section enumerates these nodes which reside in the respective front and + * backend portions of the XenStore, following the XenBus convention. + * + * All data in the XenStore is stored as strings. Nodes specifying numeric + * values are encoded in decimal. Integer value ranges listed below are + * expressed as fixed sized integer types capable of storing the conversion + * of a properly formated node string, without loss of information. + * + * + *Backend XenBus Nodes + * + * + * Addressing - + * + * Indices used to address frontends, driver instances, cards, + * devices and streams. + * + * frontend_id + * Values: + * + * Domain ID of the sound frontend. + * + * drv_idx + * Values: + * + * Zero based contiguous index of the virtualized DRM driver instance in + * this domain. Multiple PV drivers are allowed in the domain + * at the same time. + * + * conn_id + * Values: + * + * Zero based contiguous index of the connector within the card. + * + *- Connector settings - + * resolution + * Values: <[width]x[height]> + * + * Width and height for the connector in pixels separated by + * XENDRM_RESOLUTION_SEPARATOR. For example, + * vdrm/0/connector/0/resolution = "800x600" + * + * + * + *Frontend XenBus Nodes + * + * + *--- Request Transport Parameters --- + * + * These are per stream. + * + * ctrl-channel + * Values: + * + * The identifier of the Xen connector's control ev
Re: [Xen-devel] [PATCH v9] xen: add para-virtual sound interface header files
1. As per previous discussions on this there was a request that protocol files must be platform agnostic meaning that no specific compiler/platform code allowed. For that reason we have created 2 separate files: protocol itself + companion Linux specific file with structure definitions, packaging etc. 2. My intention was that these files go into Linux kernel tree and the corresponding Xen headers are all under include/xen/interface/io folder. Best regards, Oleksandr Andrushchenko ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Embedded-pv-devel] [PATCH v9] xen: add para-virtual sound interface header files
> It is not clear to me to which field this comment applies. Is it > pcm_channels? It is for pcm_rate, fixed, thanks ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v9] xen: add para-virtual sound interface header files
> You still talk about "Xen headers", i.e. plural, so I don't think your > reply addresses my earlier question. IOW I continue to think there > should be only one header in the patch here. Ok, makes sense. I will create 2 patches: one for sndif.h which is a generic protocol (platform agnostic) and sndif_linux.h which is Linux specific. Does this address your concern? ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Embedded-pv-devel] [PATCH v9] xen: add para-virtual sound interface header files
> Any people who wants to use the PV driver for other OS than Linux will have > to do it. So, may I ask why don't you write a platform agnostic header? > > This would also make easier to share the PV driver code between different > OS. There are 2 files: sndif.h which is platform agnostic and sndif_linux.h which is Linux platform specific (this is exactly the file which every platform will need to define, e.g. sndif_windows.h). In my case I need it to at least define Linux specific __packed attribute which is not supported by Win AFAIK. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Embedded-pv-devel] [PATCH v9] xen: add para-virtual sound interface header files
> I don't consider sndif.h been agnostic. It is just a bunch of array > definition. > I am not sure I'm following you here. sndif.h doesn't have any platform specific definitions, but sndif_linux.h does. Both don't define any methods, but only structures (and __packed as of now). >Each OS would have to implement their own headers in order to > easily access command fields. This is what I did for Linux, the result is sndif_linux.h > All the other PV protocols are able to define structure that are platform > agnostic. I would like to understand why you cannot do this here. > > Not mentioning that using __packed is usually a bad idea because it can > mess-up with the natural alignment of the fields and makes the programmer's > life more miserable (see an example in drm PV drivers). I need some more time for that ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Embedded-pv-devel] [PATCH] drmif: add ABI for para-virtual DRM/KMS
I believe these concerns are exactly the same as for para-virtual audio, so I would suggest that these being addressed after sndif has some resolution on packaging and structure (one or two files) Other than that I would also hear comments on the protocol itself, e.g. related to DRM/KMS Thank you, Oleksandr ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Embedded-pv-devel] [PATCH v9] xen: add para-virtual sound interface header files
> This is not critical/important ATM, and completely disconnected from the > purpose of the patch, so I agree it has to be rewritten according to > current practices. Agree, will resend new patch v10 soon Thank you all ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] drmif: add ABI for para-virtual DRM/KMS
Sorry about that, will fix On Thu, Nov 24, 2016 at 5:05 PM, Lars Kurth wrote: > > Oleksandr, > > > On 24/11/2016 11:30, "Oleksandr Andrushchenko" wrote: > >>This is the ABI for the two halves of a para-virtualized >>DRM/KMS driver. >> >>Signed-off-by: Oleksandr Andrushchenko >>Signed-off-by: Oleksandr Grytsov > > ... > >>+ * >>+ * Copyright (C) 2016 EPAM Systems Inc. >>+ * >>+ * Authors: Oleksandr Andrushchenko >>+ * Oleksandr Grytsov >>+ */ > > I have a slight concern here about the use of private e-mail addresses in > the Signed-off-by tags and authors field in the (c) notice. It contradicts > "Copyright (C) 2016 EPAM Systems Inc." and will make it very hard to > execute licenses changes, if that needs to be done in future. > > The Signed-off-by tag is a way to identify the (c) holder of a patch. I am > assuming that in this case the (c) holder would be EPAM. Is this correct? > > If yes, please change the Signed-off-by: and author fields to an EPAM > e-mail address. You can still send from gmail addresses though. > If not, please change the "Copyright (C) 2016 EPAM Systems Inc." > accordingly > > I am assuming there is a similar issue in the other patches. > Please make sure you fix all of them. > > Best Regards > Lars > -- Best regards, Oleksandr Andrushchenko ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Embedded-pv-devel] [PATCH] drmif: add ABI for para-virtual DRM/KMS
> One thing which I think you should have clarified up front is why > the existing fbfront is neither sufficient nor possible to extend > suitably. Framebuffer device is rather outdated way for Linux user-space to draw. All modern software expects DRM/KMS drivers and as a fallback *may* use fbdev. For that reason, there is a demand on DRM support for guests. So, it doesn't replace fbdev, but rather extend > Which gets me to a second aspect: The chosen name is > rather Linux centric - DRM has quite different a meaning in the > Windows world afaik. > Well, that was my intent: define ABI for *Linux DRM/KMS* protocol That said, it is still possible to implement back or front on Windows with this protocol if need be > And then a more general request: Please stop deleting all context > of the mails you reply to. As much as keeping all context (especially > when replying to long mails), deleting all of it makes reading harder. > Sorry about that, I am new to Open Source and sometimes do things wrong ;) > Jan > -- Best regards, Oleksandr Andrushchenko ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v10] sndif: add ABI for para-virtual sound
From: Oleksandr Andrushchenko Hi, all! Please find the next version of the ABI for the PV sound after addressing review comments. Thank you, Oleksandr Andrushchenko Oleksandr Grytsov Oleksandr Andrushchenko (1): xen: add para-virtual sound interface header file include/xen/interface/io/sndif.h | 700 +++ 1 file changed, 700 insertions(+) create mode 100644 include/xen/interface/io/sndif.h -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v10] xen: add para-virtual sound interface header file
From: Oleksandr Andrushchenko This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other. Signed-off-by: Oleksandr Andrushchenko Signed-off-by: Oleksandr Grytsov Signed-off-by: Oleksandr Dmytryshyn Signed-off-by: Iurii Konovalenko --- Changes since v1: * removed __attribute__((__packed__)) from all structures definitions Changes since v2: * removed all C structures * added protocol description between frontend and backend drivers Changes since v3: * fixed some typos * renamed XENSND_PCM_FORMAT_FLOAT_** to XENSND_PCM_FORMAT_F32_** * renamed XENSND_PCM_FORMAT_FLOAT64_** to XENSND_PCM_FORMAT_F64_** * added 'id' field to the request and response packets * renamed 'stream_id' to 'stream' in the packets description * renamed 'pcm_data_rate' to 'pcm_rate' in the packets description * renamed 'pcm_stream_type' to 'pcm_type' in the packets description * removed 'stream_id' field from the response packets Changes since v4: * renamed 'stream_id' back to the to 'stream' in the packets description * moved 'id' field to the upper position in the response packets Changes since v5: * Slightly reworked request/response packets * Size of the request/response packet is changed to the 64 bytes * Now parameters for the XENSND_OP_SET_VOLUME/XENSND_OP_GET_VOLUME are passed via shared page * Added parameters for the XenBus nodes (now each stream can be mapped to the defined sound device in the backend using those parameters) * Added XenBus state diagrams description Changes since v6: * Reworked streams description in the Backend XenBus Nodes Changes since v7: * re-worked backend device parameters to be more generic and flexible * extended frontend device parameters * slightly updated state machine description added mute/unmute commands * added constants for XenStore configuration strings (fields, PCM formats etc.) * changed request/response structure size from 64 octets to 16 * introduced dynamic buffer allocation instead of static XENSND_MAX_PAGES_PER_REQUEST * re-worked open request to allow dynamic buffer allocation * re-worked read/write/volume requests, so they don't pass grefs: buffer from the open request is used for these operations to pass data * specified type of the volume value to be a signed value in steps of 0.001 dBm, while 0 being 0dBm. * added Linux include file with structure definitions Changes since v8: * changed frontend-id to frontend_id * single sound card support, configured with bunch of devices/streams * clarifucation made on sample rates and formats expressed as decimals w/o any particular ordering * put description of migration/disconnection state * replaced __attribute__((packed)) to __packed * changed padding of ring structures to 64 to fit cache line * removeed #ifdef __KERNEL * explicitly stated which indices in XenStore configuration are contiguous * added description to what frontend's defaults are * made names of virtual card/devices optional * removed PCM_FORMAT_SPECIAL * changed volume units from dBm to dB Changes since v9: * removed sndif_linux.h * moved all structures from sndif_linux.h to sndif.h * structures padded where needed * fixed Hz comment --- include/xen/interface/io/sndif.h | 700 +++ 1 file changed, 700 insertions(+) create mode 100644 include/xen/interface/io/sndif.h diff --git a/include/xen/interface/io/sndif.h b/include/xen/interface/io/sndif.h new file mode 100644 index 000..82a655b --- /dev/null +++ b/include/xen/interface/io/sndif.h @@ -0,0 +1,700 @@ +/** + * sndif.h + * + * Unified sound-device I/O interface for Xen guest OSes. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DE
Re: [Xen-devel] [PATCH v10] xen: add para-virtual sound interface header file
>> +struct xensnd_close_req { >> +}; > > I'm afraid structures without any members aren't permitted by C89. > There are 2 options I see: 1. Remove empty structures 2. Leave them in, but add a comment and add a dummy place holder I would prefer option 1 as this way the interface seems to be clearer, like all requests are defined etc. >> +struct xensnd_request { >> + uint8_t raw[64]; >> +}; >> + >> +struct xensnd_response { >> + uint8_t raw[64]; >> +}; > > What are these two needed for now? If you want to enforce a > minimum size, that would better go ... > That is because we want to be cache line aligned. What it means in practice is not clear as we have so many HW around, so it is not possible to fit all. So the options could be: 1. 32 or 64 2. 48 I think is not an option here I would stick to 32 >> +union xensnd_req { >> + struct xensnd_request raw; >> + struct { >> + uint16_t id; >> + uint8_t operation; >> + uint8_t stream_idx; >> + uint32_t padding; >> + union { >> + struct xensnd_open_req open; >> + struct xensnd_close_req close; >> + struct xensnd_write_req write; >> + struct xensnd_read_req read; >> + struct xensnd_get_vol_req get_vol; >> + struct xensnd_set_vol_req set_vol; >> + struct xensnd_mute_req mute; >> + struct xensnd_unmute_req unmute; > > ... here. > > Also please observe ./CODING_STYLE (we don't use tabs for > indentation). > My fault, was thinking Linux kernel codestyle as the patch is in the kernel, but actully platform independent - will fix > Jan > Oleksandr Andrushchenko ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v10] xen: add para-virtual sound interface header file
>>>> +struct xensnd_close_req { >>>> +}; >>> >>> >>> I'm afraid structures without any members aren't permitted by C89. >>> >> There are 2 options I see: >> 1. Remove empty structures >> 2. Leave them in, but add a comment and add a dummy place holder >> I would prefer option 1 as this way the interface seems to be clearer, >> like all requests are defined etc. Typo: I would prefer option 2 with placeholders and comments >>>> >>>> +struct xensnd_request { >>>> + uint8_t raw[64]; >>>> +}; >>>> + >>>> +struct xensnd_response { >>>> + uint8_t raw[64]; >>>> +}; >>> >>> >>> What are these two needed for now? If you want to enforce a >>> minimum size, that would better go ... >>> >> That is because we want to be cache line aligned. > > > But this is only accurate to your platform. There is HW available with 128 > bytes. > > For instance on Xen ARM, all the structures are aligned to 128 bytes in > order to fit in most of cache line. Although technically it would be > possible have bigger one (see the definition of CSSIDR). > So, then I will align to 32 >> What it means in practice >> is not clear as we have so many HW around, so it is not possible to >> fit all. So the options could be: >> 1. 32 or 64 >> 2. 48 I think is not an option here > > > Why 48? It is not an option, saw something like this in fbif.h, but it is 40 actually there: #define XENFB_OUT_EVENT_SIZE 40 > > Regards, > > -- > Julien Grall Best regards, Oleksandr Andrushchenko ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v10] xen: add para-virtual sound interface header file
>>>>>> +struct xensnd_close_req { >>>>>> +}; >>>>> >>>>> >>>>> >>>>> I'm afraid structures without any members aren't permitted by C89. >>>>> >>>> There are 2 options I see: >>>> 1. Remove empty structures >>>> 2. Leave them in, but add a comment and add a dummy place holder >>>> I would prefer option 1 as this way the interface seems to be clearer, >>>> like all requests are defined etc. >> >> Typo: I would prefer option 2 with placeholders and comments >>>>>> >>>>>> >>>>>> +struct xensnd_request { >>>>>> + uint8_t raw[64]; >>>>>> +}; >>>>>> + >>>>>> +struct xensnd_response { >>>>>> + uint8_t raw[64]; >>>>>> +}; >>>>> >>>>> >>>>> >>>>> What are these two needed for now? If you want to enforce a >>>>> minimum size, that would better go ... >>>>> >>>> That is because we want to be cache line aligned. >>> >>> >>> >>> But this is only accurate to your platform. There is HW available with >>> 128 >>> bytes. >>> >>> For instance on Xen ARM, all the structures are aligned to 128 bytes in >>> order to fit in most of cache line. Although technically it would be >>> possible have bigger one (see the definition of CSSIDR). >>> >> So, then I will align to 32 > > > To confirm, you don't plan to reduce the size of the command packet to 32 > octet? > Well, actually this is what I was going to do... So, what is the correct value to be used here? >>>> What it means in practice >>>> is not clear as we have so many HW around, so it is not possible to >>>> fit all. So the options could be: >>>> 1. 32 or 64 >>>> 2. 48 I think is not an option here >>> >>> >>> >>> Why 48? >> >> It is not an option, saw something like this in fbif.h, but it is 40 >> actually there: >> #define XENFB_OUT_EVENT_SIZE 40 > > > But your 32 value seems to be as random as 40. I guess they define a size > that would fit all the command and would potentially allow the addition of > commands. > Yes, you are right here > -- > Julien Grall Best regards, Oleksandr Andrushchenko ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Embedded-pv-devel] [PATCH] drmif: add ABI for para-virtual DRM/KMS
>>> One thing which I think you should have clarified up front is why >>> the existing fbfront is neither sufficient nor possible to extend >>> suitably. >> Framebuffer device is rather outdated way for Linux user-space >> to draw. All modern software expects DRM/KMS drivers and as >> a fallback *may* use fbdev. For that reason, there is a demand >> on DRM support for guests. So, it doesn't replace fbdev, but rather extend >>> Which gets me to a second aspect: The chosen name is >>> rather Linux centric - DRM has quite different a meaning in the >>> Windows world afaik. >>> >> Well, that was my intent: define ABI for *Linux DRM/KMS* protocol >> That said, it is still possible to implement back or front on Windows >> with this protocol if need be > > Hmm, I think you want a PV Linux DRM/KMS driver, but that doesn't > mean you want/need a protocol by that name. The interface has > to describe virtual hardware, and I don't think you'd call a graphics > card "DRM/KMS card"? Good point, then I would suggest we name it dspl for display (PV display), e.g. vdspl, not vdrm. >Hence also the question whether the existing > fbfront protocol couldn't be extended - after all modern graphics > (3D) cards have also evolved from simple frame buffer (2D) ones. > The proposed protocol is almost totally diferent from what existing framebuffer offers. So that was the reason to create a new one which better fits modern graphics and doesn't alter fbif. What is more, real DRM drivers usually support framebuffer emulation, so I was thinking of some flexible solution: 1. If FB is not needed then only DRM/DSPL is in use 2. If also FB is needed then we use existing protocol to add this functionality to guest along with DSPL Nothing tells me that these couldn't be different back and front drivers/applications for even better flexibility. > Jan > Best regards, Oleksandr Andrushchenko ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v10] xen: add para-virtual sound interface header file
>>>>>>>> >>>>>>>> +struct xensnd_close_req { >>>>>>>> +}; >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> I'm afraid structures without any members aren't permitted by C89. >>>>>>> >>>>>> There are 2 options I see: >>>>>> 1. Remove empty structures >>>>>> 2. Leave them in, but add a comment and add a dummy place holder >>>>>> I would prefer option 1 as this way the interface seems to be clearer, >>>>>> like all requests are defined etc. >>>> >>>> >>>> Typo: I would prefer option 2 with placeholders and comments >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> +struct xensnd_request { >>>>>>>> + uint8_t raw[64]; >>>>>>>> +}; >>>>>>>> + >>>>>>>> +struct xensnd_response { >>>>>>>> + uint8_t raw[64]; >>>>>>>> +}; >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> What are these two needed for now? If you want to enforce a >>>>>>> minimum size, that would better go ... >>>>>>> >>>>>> That is because we want to be cache line aligned. >>>>> >>>>> >>>>> >>>>> >>>>> But this is only accurate to your platform. There is HW available with >>>>> 128 >>>>> bytes. >>>>> >>>>> For instance on Xen ARM, all the structures are aligned to 128 bytes in >>>>> order to fit in most of cache line. Although technically it would be >>>>> possible have bigger one (see the definition of CSSIDR). >>>>> >>>> So, then I will align to 32 >>> >>> >>> >>> To confirm, you don't plan to reduce the size of the command packet to 32 >>> octet? >>> >> Well, actually this is what I was going to do... >> So, what is the correct value to be used here? > > > I don't know. What is the expected maximum size of a command? > With the current design I don't expect it to be more than 32. Actually we have already implemented a Linux PV snd driver and feel comfortable with 32. So, I would suggest we stick to 32 > Regards, > > -- > Julien Grall -- Best regards, Oleksandr Andrushchenko ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Embedded-pv-devel] [PATCH] drmif: add ABI for para-virtual DRM/KMS
On 11/25/2016 09:20 AM, Jan Beulich wrote: On 24.11.16 at 23:14, wrote: On Thu, 2016-11-24 at 20:31 +0200, Oleksandr Andrushchenko wrote: Hmm, I think you want a PV Linux DRM/KMS driver, but that doesn't mean you want/need a protocol by that name. The interface has to describe virtual hardware, and I don't think you'd call a graphics card "DRM/KMS card"? Good point, then I would suggest we name it dspl for display (PV display), e.g. vdspl, not vdrm. What about DISPL / vDISPL, which is a little easier to pronounce, remember, and map back to 'display' (while dspl sounds to me a lot like the name of one of those obscure ACPI tables, or something like that! :-P). Or, if you want it to be consonant only, how about GFX / vGFX, for 'graphics' ('graphix'). Yeah, I'd personally like gfx (and hence gfxif.h) better. But again I'll leave the final word to the slated maintainer. I think that GFX may mislead and make people think it is somehow related to what GPU normally does. So, I would vote for DISPL/vDISPL Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v10] xen: add para-virtual sound interface header file
On 11/25/2016 09:14 AM, Jan Beulich wrote: On 24.11.16 at 18:25, wrote: On 24/11/16 17:11, Oleksandr Andrushchenko wrote: That is because we want to be cache line aligned. But this is only accurate to your platform. There is HW available with 128 bytes. For instance on Xen ARM, all the structures are aligned to 128 bytes in order to fit in most of cache line. Although technically it would be possible have bigger one (see the definition of CSSIDR). I think for "excessive" cache line sizes it is a good idea to balance between the positive performance effect of having a single entry per cache line and the amount of wasted space on the shared ring. With that I think 64 is a reasonable compromise, which had been agreed to also in earlier discussions on other interfaces (e.g. the blkif indirect descriptor extension iirc). Sounds good, will use 64 Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v10] xen: add para-virtual sound interface header file
On 11/25/2016 09:11 AM, Jan Beulich wrote: On 24.11.16 at 18:11, wrote: +struct xensnd_close_req { +}; I'm afraid structures without any members aren't permitted by C89. There are 2 options I see: 1. Remove empty structures 2. Leave them in, but add a comment and add a dummy place holder I would prefer option 1 as this way the interface seems to be clearer, like all requests are defined etc. I'm not really sure which option is better, so I'd leave this to the (soon to be sole) maintainer of those files (Konrad - I don't think I've seen v2 of that patch yet). I am preparing a new patch now, will publish soon +struct xensnd_request { + uint8_t raw[64]; +}; + +struct xensnd_response { + uint8_t raw[64]; +}; What are these two needed for now? If you want to enforce a minimum size, that would better go ... That is because we want to be cache line aligned. What it means in practice is not clear as we have so many HW around, so it is not possible to fit all. So the options could be: 1. 32 or 64 2. 48 I think is not an option here I would stick to 32 I don't understand why you switched over to discussing size, when all I've asked for is to change the placement of these size enforcing constructs (after all by themselves they're not useful types). You are absolutely right, will fix Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v11] sndif: add ABI for para-virtual sound
From: Oleksandr Andrushchenko Hi, all! Please find the next version of the ABI for the PV sound after addressing review comments. N.B.: after changing tabs to spaces Linux kernel is now not so happy: check patch says: NOTE: Whitespace errors detected. total: 19 errors, 42 warnings, 705 lines checked Thank you, Oleksandr Andrushchenko Oleksandr Grytsov Oleksandr Andrushchenko (1): xen: add para-virtual sound interface header file include/xen/interface/io/sndif.h | 705 +++ 1 file changed, 705 insertions(+) create mode 100644 include/xen/interface/io/sndif.h -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v11] xen: add para-virtual sound interface header file
From: Oleksandr Andrushchenko This is the ABI for the two halves of a para-virtualized sound driver to communicate with each to other. Signed-off-by: Oleksandr Andrushchenko Signed-off-by: Oleksandr Grytsov Signed-off-by: Oleksandr Dmytryshyn Signed-off-by: Iurii Konovalenko --- Changes since v1: * removed __attribute__((__packed__)) from all structures definitions Changes since v2: * removed all C structures * added protocol description between frontend and backend drivers Changes since v3: * fixed some typos * renamed XENSND_PCM_FORMAT_FLOAT_** to XENSND_PCM_FORMAT_F32_** * renamed XENSND_PCM_FORMAT_FLOAT64_** to XENSND_PCM_FORMAT_F64_** * added 'id' field to the request and response packets * renamed 'stream_id' to 'stream' in the packets description * renamed 'pcm_data_rate' to 'pcm_rate' in the packets description * renamed 'pcm_stream_type' to 'pcm_type' in the packets description * removed 'stream_id' field from the response packets Changes since v4: * renamed 'stream_id' back to the to 'stream' in the packets description * moved 'id' field to the upper position in the response packets Changes since v5: * Slightly reworked request/response packets * Size of the request/response packet is changed to the 64 bytes * Now parameters for the XENSND_OP_SET_VOLUME/XENSND_OP_GET_VOLUME are passed via shared page * Added parameters for the XenBus nodes (now each stream can be mapped to the defined sound device in the backend using those parameters) * Added XenBus state diagrams description Changes since v6: * Reworked streams description in the Backend XenBus Nodes Changes since v7: * re-worked backend device parameters to be more generic and flexible * extended frontend device parameters * slightly updated state machine description added mute/unmute commands * added constants for XenStore configuration strings (fields, PCM formats etc.) * changed request/response structure size from 64 octets to 16 * introduced dynamic buffer allocation instead of static XENSND_MAX_PAGES_PER_REQUEST * re-worked open request to allow dynamic buffer allocation * re-worked read/write/volume requests, so they don't pass grefs: buffer from the open request is used for these operations to pass data * specified type of the volume value to be a signed value in steps of 0.001 dBm, while 0 being 0dBm. * added Linux include file with structure definitions Changes since v8: * changed frontend-id to frontend_id * single sound card support, configured with bunch of devices/streams * clarifucation made on sample rates and formats expressed as decimals w/o any particular ordering * put description of migration/disconnection state * replaced __attribute__((packed)) to __packed * changed padding of ring structures to 64 to fit cache line * removeed #ifdef __KERNEL * explicitly stated which indices in XenStore configuration are contiguous * added description to what frontend's defaults are * made names of virtual card/devices optional * removed PCM_FORMAT_SPECIAL * changed volume units from dBm to dB Changes since v9: * removed sndif_linux.h * moved all structures from sndif_linux.h to sndif.h * structures padded where needed * fixed Hz comment Changes since v10: * fixed tabs to 4 spaces to comply with Xen coding style * added placeholders to empty structures (C89 concern) * added missing header includes --- include/xen/interface/io/sndif.h | 705 +++ 1 file changed, 705 insertions(+) create mode 100644 include/xen/interface/io/sndif.h diff --git a/include/xen/interface/io/sndif.h b/include/xen/interface/io/sndif.h new file mode 100644 index 000..d2cf2ab --- /dev/null +++ b/include/xen/interface/io/sndif.h @@ -0,0 +1,705 @@ +/** + * sndif.h + * + * Unified sound-device I/O interface for Xen guest OSes. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
Re: [Xen-devel] [PATCH v11] sndif: add ABI for para-virtual sound
On 11/25/2016 10:32 AM, Jan Beulich wrote: On 25.11.16 at 09:03, wrote: check patch says: NOTE: Whitespace errors detected. total: 19 errors, 42 warnings, 705 lines checked Presumably your primary problem here is that you're patching the wrong tree: include/xen/interface/io/sndif.h | 705 +++ 1 file changed, 705 insertions(+) create mode 100644 include/xen/interface/io/sndif.h This wants to be xen/include/public/io/sndif.h in the main Xen tree. The Linux tree would get updated only by propagating whatever lands in the Xen tree, and whether that involves re-formatting is up to you and the Linux maintainers. got you, will resend v11 with the right path Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v11][RESEND] sndif: add ABI for para-virtual sound
From: Oleksandr Andrushchenko Hi, all! Please find the next version of the ABI for the PV sound after addressing review comments. Thank you, Oleksandr Andrushchenko Oleksandr Grytsov Oleksandr Andrushchenko (1): xen: add para-virtual sound interface header file xen/include/public/io/sndif.h | 705 +++ 1 file changed, 705 insertions(+) create mode 100644 xen/include/public/io/sndif.h -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel