diff mbox

[v1,1/2] xen/kbdif: update protocol documentation

Message ID 1484817891-4883-2-git-send-email-andr2000@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Oleksandr Andrushchenko Jan. 19, 2017, 9:24 a.m. UTC
From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>

Signed-off-by: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>
---
 xen/include/public/io/kbdif.h | 248 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 221 insertions(+), 27 deletions(-)

Comments

Stefano Stabellini Jan. 19, 2017, 6:56 p.m. UTC | #1
On Thu, 19 Jan 2017, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>
> 
> Signed-off-by: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>
> ---
>  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 <sstabellini@kernel.org>


> + * 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:         <uint>
> + *
> + *      Backends, which support reporting of absolute coordinates for pointer
> + *      device should set this to 1.
> + *
> + *------------------------- Pointer Device Parameters ------------------------
> + *
> + * width
> + *      Values:         <uint>
> + *
> + *      Maximum X coordinate (width) to be used by the frontend
> + *      while reporting input events, pixels, [0; UINT32_MAX].
> + *
> + * height
> + *      Values:         <uint>
> + *
> + *      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:         <uint>
> + *
> + *      Request backend to report absolute pointer coordinates
> + *      (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION).
> + *
> + *----------------------- Request Transport Parameters -----------------------
> + *
> + * event-channel
> + *      Values:         <uint>
> + *
> + *      The identifier of the Xen event channel used to signal activity
> + *      in the ring buffer.
> + *
> + * page-gref
> + *      Values:         <uint>
> + *
> + *      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:         <uint>
> + *
> + *      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_KEY                3
> +#define XENKBD_TYPE_POS                4
> +
>  /*
> - * 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"
> +
> +/*
> + *****************************************************************************
> + * Description of the protocol between frontend and backend driver.
> + *****************************************************************************
> + *
> + * The two halves of a Para-virtual driver communicate with
> + * each other using a shared page and an event channel.
> + * Shared page contains a ring with event structures.
> + *
> + * All reserved fields in the structures below must be 0.
> + *
> + *****************************************************************************
> + *                           Backend to frontend events
> + *****************************************************************************
> + *
> + * Frontends should ignore unknown in events.
> + * All event packets have the same length (40 octets)
> + * All event packets have common header:
> + *
> + *          0         octet
> + * +-----------------+
> + * |       type      |
> + * +-----------------+
> + * type - uint8_t, event code, XENKBD_TYPE_???
> + *
> + *
> + * Pointer relative movement event
> + *         0                1                 2               3        octet
> + * +----------------+----------------+----------------+----------------+
> + * |  _TYPE_MOTION  |                     reserved                     | 4
> + * +----------------+----------------+----------------+----------------+
> + * |                               rel_x                               | 8
> + * +----------------+----------------+----------------+----------------+
> + * |                               rel_y                               | 12
> + * +----------------+----------------+----------------+----------------+
> + * |                               rel_z                               | 16
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 20
> + * +----------------+----------------+----------------+----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 40
> + * +----------------+----------------+----------------+----------------+
> + *
> + * rel_x - int32_t, relative X motion
> + * rel_y - int32_t, relative Y motion
> + * rel_z - int32_t, relative Z motion (wheel)
>   */
> -#define XENKBD_TYPE_POS     4
>  
>  struct xenkbd_motion
>  {
> -    uint8_t type;        /* XENKBD_TYPE_MOTION */
> -    int32_t rel_x;       /* relative X motion */
> -    int32_t rel_y;       /* relative Y motion */
> -    int32_t rel_z;       /* relative Z motion (wheel) */
> +    uint8_t type;
> +    int32_t rel_x;
> +    int32_t rel_y;
> +    int32_t rel_z;
>  };
>  
> +/*
> + * Key event (includes pointer buttons)
> + *         0                1                 2               3        octet
> + * +----------------+----------------+----------------+----------------+
> + * |  _TYPE_KEY     |     pressed    |            reserved             | 4
> + * +----------------+----------------+----------------+----------------+
> + * |                              keycode                              | 8
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 12
> + * +----------------+----------------+----------------+----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 40
> + * +----------------+----------------+----------------+----------------+
> + *
> + * pressed - uint8_t, 1 if pressed; 0 otherwise
> + * keycode - uint32_t, KEY_* from linux/input.h
> + */
> +
>  struct xenkbd_key
>  {
> -    uint8_t type;         /* XENKBD_TYPE_KEY */
> -    uint8_t pressed;      /* 1 if pressed; 0 otherwise */
> -    uint32_t keycode;     /* KEY_* from linux/input.h */
> +    uint8_t type;
> +    uint8_t pressed;
> +    uint32_t keycode;
>  };
>  
> +/*
> + * Pointer absolute position event
> + *         0                1                 2               3        octet
> + * +----------------+----------------+----------------+----------------+
> + * |  _TYPE_POS     |                     reserved                     | 4
> + * +----------------+----------------+----------------+----------------+
> + * |                               abs_x                               | 8
> + * +----------------+----------------+----------------+----------------+
> + * |                               abs_y                               | 12
> + * +----------------+----------------+----------------+----------------+
> + * |                               rel_z                               | 16
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 20
> + * +----------------+----------------+----------------+----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 40
> + * +----------------+----------------+----------------+----------------+
> + *
> + * abs_x - int32_t, absolute X position (in FB pixels)
> + * abs_y - int32_t, absolute Y position (in FB pixels)
> + * rel_z - int32_t, relative Z motion (wheel)
> + */
> +
>  struct xenkbd_position
>  {
> -    uint8_t type;        /* XENKBD_TYPE_POS */
> -    int32_t abs_x;       /* absolute X position (in FB pixels) */
> -    int32_t abs_y;       /* absolute Y position (in FB pixels) */
> -    int32_t rel_z;       /* relative Z motion (wheel) */
> +    uint8_t type;
> +    int32_t abs_x;
> +    int32_t abs_y;
> +    int32_t rel_z;
>  };
>  
>  #define XENKBD_IN_EVENT_SIZE 40
> @@ -79,12 +259,22 @@ union xenkbd_in_event
>      char pad[XENKBD_IN_EVENT_SIZE];
>  };
>  
> -/* Out events (frontend -> backend) */
> -
>  /*
> + *****************************************************************************
> + *                            Frontend to backend events
> + *****************************************************************************
> + *
>   * Out events may be sent only when requested by backend, and receipt
>   * of an unknown out event is an error.
>   * No out events currently defined.
> +
> + * All event packets have the same length (40 octets)
> + * All event packets have common header:
> + *          0         octet
> + * +-----------------+
> + * |       type      |
> + * +-----------------+
> + * type - uint8_t, event code
>   */
>  
>  #define XENKBD_OUT_EVENT_SIZE 40
> @@ -95,7 +285,11 @@ union xenkbd_out_event
>      char pad[XENKBD_OUT_EVENT_SIZE];
>  };
>  
> -/* shared page */
> +/*
> + *****************************************************************************
> + *                            Shared page
> + *****************************************************************************
> + */
>  
>  #define XENKBD_IN_RING_SIZE 2048
>  #define XENKBD_IN_RING_LEN (XENKBD_IN_RING_SIZE / XENKBD_IN_EVENT_SIZE)
> @@ -119,7 +313,7 @@ struct xenkbd_page
>      uint32_t out_cons, out_prod;
>  };
>  
> -#endif
> +#endif /* __XEN_PUBLIC_IO_KBDIF_H__ */
>  
>  /*
>   * Local variables:
> -- 
> 2.7.4
>
Oleksandr Andrushchenko Jan. 20, 2017, 6:58 a.m. UTC | #2
On 01/19/2017 08:56 PM, Stefano Stabellini wrote:
> On Thu, 19 Jan 2017, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>
>>
>> Signed-off-by: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>
>> ---
>>   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 <sstabellini@kernel.org>
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:         <uint>
>> + *
>> + *      Backends, which support reporting of absolute coordinates for pointer
>> + *      device should set this to 1.
>> + *
>> + *------------------------- Pointer Device Parameters ------------------------
>> + *
>> + * width
>> + *      Values:         <uint>
>> + *
>> + *      Maximum X coordinate (width) to be used by the frontend
>> + *      while reporting input events, pixels, [0; UINT32_MAX].
>> + *
>> + * height
>> + *      Values:         <uint>
>> + *
>> + *      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:         <uint>
>> + *
>> + *      Request backend to report absolute pointer coordinates
>> + *      (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION).
>> + *
>> + *----------------------- Request Transport Parameters -----------------------
>> + *
>> + * event-channel
>> + *      Values:         <uint>
>> + *
>> + *      The identifier of the Xen event channel used to signal activity
>> + *      in the ring buffer.
>> + *
>> + * page-gref
>> + *      Values:         <uint>
>> + *
>> + *      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:         <uint>
>> + *
>> + *      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_KEY                3
>> +#define XENKBD_TYPE_POS                4
>> +
>>   /*
>> - * 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"
>> +
>> +/*
>> + *****************************************************************************
>> + * Description of the protocol between frontend and backend driver.
>> + *****************************************************************************
>> + *
>> + * The two halves of a Para-virtual driver communicate with
>> + * each other using a shared page and an event channel.
>> + * Shared page contains a ring with event structures.
>> + *
>> + * All reserved fields in the structures below must be 0.
>> + *
>> + *****************************************************************************
>> + *                           Backend to frontend events
>> + *****************************************************************************
>> + *
>> + * Frontends should ignore unknown in events.
>> + * All event packets have the same length (40 octets)
>> + * All event packets have common header:
>> + *
>> + *          0         octet
>> + * +-----------------+
>> + * |       type      |
>> + * +-----------------+
>> + * type - uint8_t, event code, XENKBD_TYPE_???
>> + *
>> + *
>> + * Pointer relative movement event
>> + *         0                1                 2               3        octet
>> + * +----------------+----------------+----------------+----------------+
>> + * |  _TYPE_MOTION  |                     reserved                     | 4
>> + * +----------------+----------------+----------------+----------------+
>> + * |                               rel_x                               | 8
>> + * +----------------+----------------+----------------+----------------+
>> + * |                               rel_y                               | 12
>> + * +----------------+----------------+----------------+----------------+
>> + * |                               rel_z                               | 16
>> + * +----------------+----------------+----------------+----------------+
>> + * |                             reserved                              | 20
>> + * +----------------+----------------+----------------+----------------+
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +----------------+----------------+----------------+----------------+
>> + * |                             reserved                              | 40
>> + * +----------------+----------------+----------------+----------------+
>> + *
>> + * rel_x - int32_t, relative X motion
>> + * rel_y - int32_t, relative Y motion
>> + * rel_z - int32_t, relative Z motion (wheel)
>>    */
>> -#define XENKBD_TYPE_POS     4
>>   
>>   struct xenkbd_motion
>>   {
>> -    uint8_t type;        /* XENKBD_TYPE_MOTION */
>> -    int32_t rel_x;       /* relative X motion */
>> -    int32_t rel_y;       /* relative Y motion */
>> -    int32_t rel_z;       /* relative Z motion (wheel) */
>> +    uint8_t type;
>> +    int32_t rel_x;
>> +    int32_t rel_y;
>> +    int32_t rel_z;
>>   };
>>   
>> +/*
>> + * Key event (includes pointer buttons)
>> + *         0                1                 2               3        octet
>> + * +----------------+----------------+----------------+----------------+
>> + * |  _TYPE_KEY     |     pressed    |            reserved             | 4
>> + * +----------------+----------------+----------------+----------------+
>> + * |                              keycode                              | 8
>> + * +----------------+----------------+----------------+----------------+
>> + * |                             reserved                              | 12
>> + * +----------------+----------------+----------------+----------------+
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +----------------+----------------+----------------+----------------+
>> + * |                             reserved                              | 40
>> + * +----------------+----------------+----------------+----------------+
>> + *
>> + * pressed - uint8_t, 1 if pressed; 0 otherwise
>> + * keycode - uint32_t, KEY_* from linux/input.h
>> + */
>> +
>>   struct xenkbd_key
>>   {
>> -    uint8_t type;         /* XENKBD_TYPE_KEY */
>> -    uint8_t pressed;      /* 1 if pressed; 0 otherwise */
>> -    uint32_t keycode;     /* KEY_* from linux/input.h */
>> +    uint8_t type;
>> +    uint8_t pressed;
>> +    uint32_t keycode;
>>   };
>>   
>> +/*
>> + * Pointer absolute position event
>> + *         0                1                 2               3        octet
>> + * +----------------+----------------+----------------+----------------+
>> + * |  _TYPE_POS     |                     reserved                     | 4
>> + * +----------------+----------------+----------------+----------------+
>> + * |                               abs_x                               | 8
>> + * +----------------+----------------+----------------+----------------+
>> + * |                               abs_y                               | 12
>> + * +----------------+----------------+----------------+----------------+
>> + * |                               rel_z                               | 16
>> + * +----------------+----------------+----------------+----------------+
>> + * |                             reserved                              | 20
>> + * +----------------+----------------+----------------+----------------+
>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
>> + * +----------------+----------------+----------------+----------------+
>> + * |                             reserved                              | 40
>> + * +----------------+----------------+----------------+----------------+
>> + *
>> + * abs_x - int32_t, absolute X position (in FB pixels)
>> + * abs_y - int32_t, absolute Y position (in FB pixels)
>> + * rel_z - int32_t, relative Z motion (wheel)
>> + */
>> +
>>   struct xenkbd_position
>>   {
>> -    uint8_t type;        /* XENKBD_TYPE_POS */
>> -    int32_t abs_x;       /* absolute X position (in FB pixels) */
>> -    int32_t abs_y;       /* absolute Y position (in FB pixels) */
>> -    int32_t rel_z;       /* relative Z motion (wheel) */
>> +    uint8_t type;
>> +    int32_t abs_x;
>> +    int32_t abs_y;
>> +    int32_t rel_z;
>>   };
>>   
>>   #define XENKBD_IN_EVENT_SIZE 40
>> @@ -79,12 +259,22 @@ union xenkbd_in_event
>>       char pad[XENKBD_IN_EVENT_SIZE];
>>   };
>>   
>> -/* Out events (frontend -> backend) */
>> -
>>   /*
>> + *****************************************************************************
>> + *                            Frontend to backend events
>> + *****************************************************************************
>> + *
>>    * Out events may be sent only when requested by backend, and receipt
>>    * of an unknown out event is an error.
>>    * No out events currently defined.
>> +
>> + * All event packets have the same length (40 octets)
>> + * All event packets have common header:
>> + *          0         octet
>> + * +-----------------+
>> + * |       type      |
>> + * +-----------------+
>> + * type - uint8_t, event code
>>    */
>>   
>>   #define XENKBD_OUT_EVENT_SIZE 40
>> @@ -95,7 +285,11 @@ union xenkbd_out_event
>>       char pad[XENKBD_OUT_EVENT_SIZE];
>>   };
>>   
>> -/* shared page */
>> +/*
>> + *****************************************************************************
>> + *                            Shared page
>> + *****************************************************************************
>> + */
>>   
>>   #define XENKBD_IN_RING_SIZE 2048
>>   #define XENKBD_IN_RING_LEN (XENKBD_IN_RING_SIZE / XENKBD_IN_EVENT_SIZE)
>> @@ -119,7 +313,7 @@ struct xenkbd_page
>>       uint32_t out_cons, out_prod;
>>   };
>>   
>> -#endif
>> +#endif /* __XEN_PUBLIC_IO_KBDIF_H__ */
>>   
>>   /*
>>    * Local variables:
>> -- 
>> 2.7.4
>>
Stefano Stabellini Jan. 20, 2017, 5:43 p.m. UTC | #3
On Fri, 20 Jan 2017, Oleksandr Andrushchenko wrote:
> On 01/19/2017 08:56 PM, Stefano Stabellini wrote:
> > On Thu, 19 Jan 2017, Oleksandr Andrushchenko wrote:
> > > From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>
> > > 
> > > Signed-off-by: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@epam.com>
> > > ---
> > >   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 <sstabellini@kernel.org>
> Thanks,
> with this change done, can I put your Reviewed-by
> into the next version of this patch?

Of course. Thanks,

Stefano

> > > + * 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:         <uint>
> > > + *
> > > + *      Backends, which support reporting of absolute coordinates for
> > > pointer
> > > + *      device should set this to 1.
> > > + *
> > > + *------------------------- Pointer Device Parameters
> > > ------------------------
> > > + *
> > > + * width
> > > + *      Values:         <uint>
> > > + *
> > > + *      Maximum X coordinate (width) to be used by the frontend
> > > + *      while reporting input events, pixels, [0; UINT32_MAX].
> > > + *
> > > + * height
> > > + *      Values:         <uint>
> > > + *
> > > + *      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:         <uint>
> > > + *
> > > + *      Request backend to report absolute pointer coordinates
> > > + *      (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION).
> > > + *
> > > + *----------------------- Request Transport Parameters
> > > -----------------------
> > > + *
> > > + * event-channel
> > > + *      Values:         <uint>
> > > + *
> > > + *      The identifier of the Xen event channel used to signal activity
> > > + *      in the ring buffer.
> > > + *
> > > + * page-gref
> > > + *      Values:         <uint>
> > > + *
> > > + *      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:         <uint>
> > > + *
> > > + *      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_KEY                3
> > > +#define XENKBD_TYPE_POS                4
> > > +
> > >   /*
> > > - * 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"
> > > +
> > > +/*
> > > +
> > > *****************************************************************************
> > > + * Description of the protocol between frontend and backend driver.
> > > +
> > > *****************************************************************************
> > > + *
> > > + * The two halves of a Para-virtual driver communicate with
> > > + * each other using a shared page and an event channel.
> > > + * Shared page contains a ring with event structures.
> > > + *
> > > + * All reserved fields in the structures below must be 0.
> > > + *
> > > +
> > > *****************************************************************************
> > > + *                           Backend to frontend events
> > > +
> > > *****************************************************************************
> > > + *
> > > + * Frontends should ignore unknown in events.
> > > + * All event packets have the same length (40 octets)
> > > + * All event packets have common header:
> > > + *
> > > + *          0         octet
> > > + * +-----------------+
> > > + * |       type      |
> > > + * +-----------------+
> > > + * type - uint8_t, event code, XENKBD_TYPE_???
> > > + *
> > > + *
> > > + * Pointer relative movement event
> > > + *         0                1                 2               3
> > > octet
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |  _TYPE_MOTION  |                     reserved                     |
> > > 4
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                               rel_x                               |
> > > 8
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                               rel_y                               |
> > > 12
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                               rel_z                               |
> > > 16
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                             reserved                              |
> > > 20
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                             reserved                              |
> > > 40
> > > + * +----------------+----------------+----------------+----------------+
> > > + *
> > > + * rel_x - int32_t, relative X motion
> > > + * rel_y - int32_t, relative Y motion
> > > + * rel_z - int32_t, relative Z motion (wheel)
> > >    */
> > > -#define XENKBD_TYPE_POS     4
> > >     struct xenkbd_motion
> > >   {
> > > -    uint8_t type;        /* XENKBD_TYPE_MOTION */
> > > -    int32_t rel_x;       /* relative X motion */
> > > -    int32_t rel_y;       /* relative Y motion */
> > > -    int32_t rel_z;       /* relative Z motion (wheel) */
> > > +    uint8_t type;
> > > +    int32_t rel_x;
> > > +    int32_t rel_y;
> > > +    int32_t rel_z;
> > >   };
> > >   +/*
> > > + * Key event (includes pointer buttons)
> > > + *         0                1                 2               3
> > > octet
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |  _TYPE_KEY     |     pressed    |            reserved             |
> > > 4
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                              keycode                              |
> > > 8
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                             reserved                              |
> > > 12
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                             reserved                              |
> > > 40
> > > + * +----------------+----------------+----------------+----------------+
> > > + *
> > > + * pressed - uint8_t, 1 if pressed; 0 otherwise
> > > + * keycode - uint32_t, KEY_* from linux/input.h
> > > + */
> > > +
> > >   struct xenkbd_key
> > >   {
> > > -    uint8_t type;         /* XENKBD_TYPE_KEY */
> > > -    uint8_t pressed;      /* 1 if pressed; 0 otherwise */
> > > -    uint32_t keycode;     /* KEY_* from linux/input.h */
> > > +    uint8_t type;
> > > +    uint8_t pressed;
> > > +    uint32_t keycode;
> > >   };
> > >   +/*
> > > + * Pointer absolute position event
> > > + *         0                1                 2               3
> > > octet
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |  _TYPE_POS     |                     reserved                     |
> > > 4
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                               abs_x                               |
> > > 8
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                               abs_y                               |
> > > 12
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                               rel_z                               |
> > > 16
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                             reserved                              |
> > > 20
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> > > + * +----------------+----------------+----------------+----------------+
> > > + * |                             reserved                              |
> > > 40
> > > + * +----------------+----------------+----------------+----------------+
> > > + *
> > > + * abs_x - int32_t, absolute X position (in FB pixels)
> > > + * abs_y - int32_t, absolute Y position (in FB pixels)
> > > + * rel_z - int32_t, relative Z motion (wheel)
> > > + */
> > > +
> > >   struct xenkbd_position
> > >   {
> > > -    uint8_t type;        /* XENKBD_TYPE_POS */
> > > -    int32_t abs_x;       /* absolute X position (in FB pixels) */
> > > -    int32_t abs_y;       /* absolute Y position (in FB pixels) */
> > > -    int32_t rel_z;       /* relative Z motion (wheel) */
> > > +    uint8_t type;
> > > +    int32_t abs_x;
> > > +    int32_t abs_y;
> > > +    int32_t rel_z;
> > >   };
> > >     #define XENKBD_IN_EVENT_SIZE 40
> > > @@ -79,12 +259,22 @@ union xenkbd_in_event
> > >       char pad[XENKBD_IN_EVENT_SIZE];
> > >   };
> > >   -/* Out events (frontend -> backend) */
> > > -
> > >   /*
> > > +
> > > *****************************************************************************
> > > + *                            Frontend to backend events
> > > +
> > > *****************************************************************************
> > > + *
> > >    * Out events may be sent only when requested by backend, and receipt
> > >    * of an unknown out event is an error.
> > >    * No out events currently defined.
> > > +
> > > + * All event packets have the same length (40 octets)
> > > + * All event packets have common header:
> > > + *          0         octet
> > > + * +-----------------+
> > > + * |       type      |
> > > + * +-----------------+
> > > + * type - uint8_t, event code
> > >    */
> > >     #define XENKBD_OUT_EVENT_SIZE 40
> > > @@ -95,7 +285,11 @@ union xenkbd_out_event
> > >       char pad[XENKBD_OUT_EVENT_SIZE];
> > >   };
> > >   -/* shared page */
> > > +/*
> > > +
> > > *****************************************************************************
> > > + *                            Shared page
> > > +
> > > *****************************************************************************
> > > + */
> > >     #define XENKBD_IN_RING_SIZE 2048
> > >   #define XENKBD_IN_RING_LEN (XENKBD_IN_RING_SIZE / XENKBD_IN_EVENT_SIZE)
> > > @@ -119,7 +313,7 @@ struct xenkbd_page
> > >       uint32_t out_cons, out_prod;
> > >   };
> > >   -#endif
> > > +#endif /* __XEN_PUBLIC_IO_KBDIF_H__ */
> > >     /*
> > >    * Local variables:
> > > -- 
> > > 2.7.4
> > > 
>
diff mbox

Patch

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:         <uint>
+ *
+ *      Backends, which support reporting of absolute coordinates for pointer
+ *      device should set this to 1.
+ *
+ *------------------------- Pointer Device Parameters ------------------------
+ *
+ * width
+ *      Values:         <uint>
+ *
+ *      Maximum X coordinate (width) to be used by the frontend
+ *      while reporting input events, pixels, [0; UINT32_MAX].
+ *
+ * height
+ *      Values:         <uint>
+ *
+ *      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:         <uint>
+ *
+ *      Request backend to report absolute pointer coordinates
+ *      (XENKBD_TYPE_POS) instead of relative ones (XENKBD_TYPE_MOTION).
+ *
+ *----------------------- Request Transport Parameters -----------------------
+ *
+ * event-channel
+ *      Values:         <uint>
+ *
+ *      The identifier of the Xen event channel used to signal activity
+ *      in the ring buffer.
+ *
+ * page-gref
+ *      Values:         <uint>
+ *
+ *      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:         <uint>
+ *
+ *      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_KEY                3
+#define XENKBD_TYPE_POS                4
+
 /*
- * 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"
+
+/*
+ *****************************************************************************
+ * Description of the protocol between frontend and backend driver.
+ *****************************************************************************
+ *
+ * The two halves of a Para-virtual driver communicate with
+ * each other using a shared page and an event channel.
+ * Shared page contains a ring with event structures.
+ *
+ * All reserved fields in the structures below must be 0.
+ *
+ *****************************************************************************
+ *                           Backend to frontend events
+ *****************************************************************************
+ *
+ * Frontends should ignore unknown in events.
+ * All event packets have the same length (40 octets)
+ * All event packets have common header:
+ *
+ *          0         octet
+ * +-----------------+
+ * |       type      |
+ * +-----------------+
+ * type - uint8_t, event code, XENKBD_TYPE_???
+ *
+ *
+ * Pointer relative movement event
+ *         0                1                 2               3        octet
+ * +----------------+----------------+----------------+----------------+
+ * |  _TYPE_MOTION  |                     reserved                     | 4
+ * +----------------+----------------+----------------+----------------+
+ * |                               rel_x                               | 8
+ * +----------------+----------------+----------------+----------------+
+ * |                               rel_y                               | 12
+ * +----------------+----------------+----------------+----------------+
+ * |                               rel_z                               | 16
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 20
+ * +----------------+----------------+----------------+----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 40
+ * +----------------+----------------+----------------+----------------+
+ *
+ * rel_x - int32_t, relative X motion
+ * rel_y - int32_t, relative Y motion
+ * rel_z - int32_t, relative Z motion (wheel)
  */
-#define XENKBD_TYPE_POS     4
 
 struct xenkbd_motion
 {
-    uint8_t type;        /* XENKBD_TYPE_MOTION */
-    int32_t rel_x;       /* relative X motion */
-    int32_t rel_y;       /* relative Y motion */
-    int32_t rel_z;       /* relative Z motion (wheel) */
+    uint8_t type;
+    int32_t rel_x;
+    int32_t rel_y;
+    int32_t rel_z;
 };
 
+/*
+ * Key event (includes pointer buttons)
+ *         0                1                 2               3        octet
+ * +----------------+----------------+----------------+----------------+
+ * |  _TYPE_KEY     |     pressed    |            reserved             | 4
+ * +----------------+----------------+----------------+----------------+
+ * |                              keycode                              | 8
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 12
+ * +----------------+----------------+----------------+----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 40
+ * +----------------+----------------+----------------+----------------+
+ *
+ * pressed - uint8_t, 1 if pressed; 0 otherwise
+ * keycode - uint32_t, KEY_* from linux/input.h
+ */
+
 struct xenkbd_key
 {
-    uint8_t type;         /* XENKBD_TYPE_KEY */
-    uint8_t pressed;      /* 1 if pressed; 0 otherwise */
-    uint32_t keycode;     /* KEY_* from linux/input.h */
+    uint8_t type;
+    uint8_t pressed;
+    uint32_t keycode;
 };
 
+/*
+ * Pointer absolute position event
+ *         0                1                 2               3        octet
+ * +----------------+----------------+----------------+----------------+
+ * |  _TYPE_POS     |                     reserved                     | 4
+ * +----------------+----------------+----------------+----------------+
+ * |                               abs_x                               | 8
+ * +----------------+----------------+----------------+----------------+
+ * |                               abs_y                               | 12
+ * +----------------+----------------+----------------+----------------+
+ * |                               rel_z                               | 16
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 20
+ * +----------------+----------------+----------------+----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 40
+ * +----------------+----------------+----------------+----------------+
+ *
+ * abs_x - int32_t, absolute X position (in FB pixels)
+ * abs_y - int32_t, absolute Y position (in FB pixels)
+ * rel_z - int32_t, relative Z motion (wheel)
+ */
+
 struct xenkbd_position
 {
-    uint8_t type;        /* XENKBD_TYPE_POS */
-    int32_t abs_x;       /* absolute X position (in FB pixels) */
-    int32_t abs_y;       /* absolute Y position (in FB pixels) */
-    int32_t rel_z;       /* relative Z motion (wheel) */
+    uint8_t type;
+    int32_t abs_x;
+    int32_t abs_y;
+    int32_t rel_z;
 };
 
 #define XENKBD_IN_EVENT_SIZE 40
@@ -79,12 +259,22 @@  union xenkbd_in_event
     char pad[XENKBD_IN_EVENT_SIZE];
 };
 
-/* Out events (frontend -> backend) */
-
 /*
+ *****************************************************************************
+ *                            Frontend to backend events
+ *****************************************************************************
+ *
  * Out events may be sent only when requested by backend, and receipt
  * of an unknown out event is an error.
  * No out events currently defined.
+
+ * All event packets have the same length (40 octets)
+ * All event packets have common header:
+ *          0         octet
+ * +-----------------+
+ * |       type      |
+ * +-----------------+
+ * type - uint8_t, event code
  */
 
 #define XENKBD_OUT_EVENT_SIZE 40
@@ -95,7 +285,11 @@  union xenkbd_out_event
     char pad[XENKBD_OUT_EVENT_SIZE];
 };
 
-/* shared page */
+/*
+ *****************************************************************************
+ *                            Shared page
+ *****************************************************************************
+ */
 
 #define XENKBD_IN_RING_SIZE 2048
 #define XENKBD_IN_RING_LEN (XENKBD_IN_RING_SIZE / XENKBD_IN_EVENT_SIZE)
@@ -119,7 +313,7 @@  struct xenkbd_page
     uint32_t out_cons, out_prod;
 };
 
-#endif
+#endif /* __XEN_PUBLIC_IO_KBDIF_H__ */
 
 /*
  * Local variables: