[Xen-devel] [RFC] kbdif: add multi-touch support

2017-01-03 Thread Oleksandr Andrushchenko
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

2017-01-03 Thread Oleksandr Andrushchenko
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

2017-01-03 Thread Oleksandr Andrushchenko


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

2017-01-03 Thread Oleksandr Andrushchenko

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

2017-01-04 Thread Oleksandr Andrushchenko



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

2017-01-04 Thread Oleksandr Andrushchenko

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

2017-01-04 Thread Oleksandr Andrushchenko

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

2017-01-04 Thread Oleksandr Andrushchenko

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

2017-01-04 Thread Oleksandr Andrushchenko

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

2017-01-05 Thread Oleksandr Andrushchenko

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

2017-01-05 Thread Oleksandr Andrushchenko

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

2017-01-05 Thread Oleksandr Andrushchenko

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

2017-01-06 Thread Oleksandr Andrushchenko
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

2017-01-06 Thread Oleksandr Andrushchenko
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

2017-01-06 Thread Oleksandr Andrushchenko
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

2017-01-09 Thread Oleksandr Andrushchenko


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

2017-01-09 Thread Oleksandr Andrushchenko


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

2017-01-11 Thread Oleksandr Andrushchenko

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

2017-01-11 Thread Oleksandr Andrushchenko

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

2017-01-11 Thread Oleksandr Andrushchenko

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

2017-01-11 Thread Oleksandr Andrushchenko

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

2017-01-11 Thread Oleksandr Andrushchenko

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

2017-01-11 Thread Oleksandr Andrushchenko



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

2017-01-11 Thread Oleksandr Andrushchenko

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

2017-01-18 Thread Oleksandr Andrushchenko

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

2017-01-18 Thread Oleksandr Andrushchenko

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

2017-01-18 Thread Oleksandr Andrushchenko

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

2017-01-19 Thread Oleksandr Andrushchenko
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

2017-01-19 Thread Oleksandr Andrushchenko
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

2017-01-19 Thread Oleksandr Andrushchenko
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

2017-01-19 Thread Oleksandr Andrushchenko

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

2017-01-19 Thread Oleksandr Andrushchenko

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

2017-01-20 Thread Oleksandr Andrushchenko

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

2017-01-21 Thread Oleksandr Andrushchenko

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

2017-01-23 Thread Oleksandr Andrushchenko

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

2017-01-25 Thread Oleksandr Andrushchenko

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

2017-01-25 Thread Oleksandr Andrushchenko
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

2017-01-25 Thread Oleksandr Andrushchenko
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

2017-01-25 Thread Oleksandr Andrushchenko
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.

2017-01-26 Thread Oleksandr Andrushchenko

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.

2017-01-26 Thread Oleksandr Andrushchenko

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.

2017-01-26 Thread Oleksandr Andrushchenko

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

2017-01-26 Thread Oleksandr Andrushchenko

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

2017-01-26 Thread Oleksandr Andrushchenko

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

2017-01-26 Thread Oleksandr Andrushchenko

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

2017-01-26 Thread Oleksandr Andrushchenko

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

2017-01-26 Thread Oleksandr Andrushchenko



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

2017-01-26 Thread Oleksandr Andrushchenko

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

2017-01-26 Thread Oleksandr Andrushchenko

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

2017-01-26 Thread Oleksandr Andrushchenko

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

2017-01-26 Thread Oleksandr Andrushchenko

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

2017-01-27 Thread Oleksandr Andrushchenko

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

2017-01-27 Thread Oleksandr Andrushchenko

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

2017-01-27 Thread Oleksandr Andrushchenko

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

2017-01-27 Thread Oleksandr Andrushchenko
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

2017-01-27 Thread Oleksandr Andrushchenko
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

2017-01-27 Thread Oleksandr Andrushchenko
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

2017-01-27 Thread Oleksandr Andrushchenko
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

2017-01-27 Thread Oleksandr Andrushchenko

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

2017-01-27 Thread Oleksandr Andrushchenko

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.

2017-01-27 Thread Oleksandr Andrushchenko

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.

2017-01-27 Thread Oleksandr Andrushchenko

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.

2017-01-27 Thread Oleksandr Andrushchenko



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.

2017-01-29 Thread Oleksandr Andrushchenko

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

2017-01-29 Thread Oleksandr Andrushchenko



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

2017-01-30 Thread Oleksandr Andrushchenko

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

2017-01-30 Thread Oleksandr Andrushchenko

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

2016-11-08 Thread Oleksandr Andrushchenko

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.

2016-11-12 Thread Oleksandr Andrushchenko
 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

2016-11-14 Thread Oleksandr Andrushchenko
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.

2016-11-15 Thread Oleksandr Andrushchenko
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.

2016-11-17 Thread Oleksandr Andrushchenko
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

2016-11-22 Thread Oleksandr Andrushchenko
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

2016-11-23 Thread 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
---
 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

2016-11-23 Thread Oleksandr Andrushchenko
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

2016-11-24 Thread Oleksandr Andrushchenko
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

2016-11-24 Thread Oleksandr Andrushchenko
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

2016-11-24 Thread Oleksandr Andrushchenko
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

2016-11-24 Thread Oleksandr Andrushchenko
> 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

2016-11-24 Thread Oleksandr Andrushchenko
> 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

2016-11-24 Thread Oleksandr Andrushchenko
> 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

2016-11-24 Thread Oleksandr Andrushchenko
> 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

2016-11-24 Thread Oleksandr Andrushchenko
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

2016-11-24 Thread Oleksandr Andrushchenko
> 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

2016-11-24 Thread Oleksandr Andrushchenko
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

2016-11-24 Thread Oleksandr Andrushchenko
> 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

2016-11-24 Thread Oleksandr Andrushchenko
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

2016-11-24 Thread Oleksandr Andrushchenko
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

2016-11-24 Thread Oleksandr Andrushchenko
>> +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

2016-11-24 Thread Oleksandr Andrushchenko
>>>> +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

2016-11-24 Thread Oleksandr Andrushchenko
>>>>>> +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

2016-11-24 Thread Oleksandr Andrushchenko
>>>  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

2016-11-24 Thread Oleksandr Andrushchenko
>>>>>>>>
>>>>>>>> +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

2016-11-24 Thread Oleksandr Andrushchenko

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

2016-11-24 Thread Oleksandr Andrushchenko



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

2016-11-24 Thread Oleksandr Andrushchenko



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

2016-11-25 Thread Oleksandr Andrushchenko
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

2016-11-25 Thread Oleksandr Andrushchenko
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

2016-11-25 Thread Oleksandr Andrushchenko

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

2016-11-25 Thread Oleksandr Andrushchenko
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


  1   2   3   4   >