[Qemu-devel] [PATCH 0/2] xenfb: Document public headers

[Qemu-devel] [PATCH 0/2] xenfb: Document public headers

[Owen smith]

Add comments documenting the xenstore nodes used by the vfb and vkbd devices.
Documents changes in qemu patch series posted here
http://lists.gnu.org/archive/html/qemu-devel/2014-09/msg03472.html

Owen smith (2):
  xenfb: Add comment documentation
  xenfb: Add comment documentation

 xen/include/public/io/fbif.h  | 25 +++++++++++++++
 xen/include/public/io/kbdif.h | 74 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+)

-- 
2.1.0

[Qemu-devel] [PATCH 1/2] xenfb: Add comment documentation

[Owen smith]

Add documentation for feature-abs-pointer, feature-no-abs-rescale,
feature-no-console, page-ref, page-gref and event-channel

Signed-off-by: Owen smith <owen.smith@citrix.com>
---
 xen/include/public/io/kbdif.h | 74 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
index 2d2aebd..b29bc12 100644
--- a/xen/include/public/io/kbdif.h
+++ b/xen/include/public/io/kbdif.h
@@ -26,6 +26,80 @@
 #ifndef __XEN_PUBLIC_IO_KBDIF_H__
 #define __XEN_PUBLIC_IO_KBDIF_H__
 
+/*
+ * Backend Xenstore Nodes
+ * ----------------------
+ *
+ * feature-abs-pointer
+ *   Values: 0/1 (boolean)
+ *   Default: 0
+ *
+ *   When set to "1", backend supports supplying absolute coordinates via
+ *   XENKBD_TYPE_POS messages. When set to "0", backend can only supply
+ *   relative movements via XENKBD_TYPE_MOTION messages.
+ *
+ * feature-no-abs-rescale
+ *   Values: 0/1 (boolean)
+ *   Default: 0
+ *
+ *   When set to "1", backend supports unscaled absolute coordinates. Unscaled
+ *   coordinates are in the range [0, 0x7fff]. When set to "0", backend can
+ *   only supply scaled coordinates. Scaled coordinates are scaled to the
+ *   'screen size' of the console. If "feature-abs-pointer" is "0", this value
+ *   has no effect.
+ *
+ * feature-no-console
+ *   Values: 0/1 (boolean)
+ *   Default: 0
+ *
+ *   When set to "1", backend supports connection without a console. When
+ *   running without a console, scaled values maximum is undefined. When
+ *   set to "0", backend will wait for a console before connecting.
+ *
+ * Frontend XenStore Nodes
+ * -----------------------
+ *
+ * request-abs-pointer
+ *   Values: 0/1 (boolean)
+ *   Default: 0
+ *
+ *   When set to "1", frontend wants absolute coordinates delivered with the
+ *   XENKBD_TYPE_POS message.
+ *
+ * request-no-abs-rescale
+ *   Values: 0/1 (boolean)
+ *   Default: 0
+ *
+ *   When set to "1", frontend wants unscaled absolute coordinates. If
+ *   "request-abs-pointer" is "0", this value has no effect.
+ *
+ * request-no-console
+ *   Values: 0/1 (boolean)
+ *   Default: 0
+ *
+ *   When set to "1", frontend does not require a console for connection.
+ *
+ * page-ref
+ *   Values: <uint64_t>
+ *   Optional, "page-gref" is used if "page-ref" is not set.
+ *
+ *   The MFN of a page of memory for the shared ring structures. If not
+ *   present, "page-gref" must be set. "page-ref" overrides "page-gref".
+ *
+ * page-gref
+ *   Values: <uint32_t>
+ *   Only required if "page-ref" is NOT set.
+ *
+ *   A grant reference to the memory page to be mapped for the shared ring
+ *   structures. Must be present if "page-ref" is not present.
+ *
+ * event-channel
+ *   Values: <uint32_t>
+ *
+ *   An event channel identifier, which is triggered when the shared page
+ *   is updated.
+ */
+
 /* In events (backend -> frontend) */
 
 /*
-- 
2.1.0

Re: [Qemu-devel] [PATCH 1/2] xenfb: Add comment documentation

[Stefano Stabellini]

On Mon, 22 Sep 2014, Owen smith wrote:
> Add documentation for feature-abs-pointer, feature-no-abs-rescale,
> feature-no-console, page-ref, page-gref and event-channel
> 
> Signed-off-by: Owen smith <owen.smith@citrix.com>

I would suggest resending this patch series removing the new options you
are introducing with
1410964242-3341-1-git-send-email-owen.smith@citrix.com.
This series should be a precursor and could go in the 4.5 release.


>  xen/include/public/io/kbdif.h | 74 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
> 
> diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
> index 2d2aebd..b29bc12 100644
> --- a/xen/include/public/io/kbdif.h
> +++ b/xen/include/public/io/kbdif.h
> @@ -26,6 +26,80 @@
>  #ifndef __XEN_PUBLIC_IO_KBDIF_H__
>  #define __XEN_PUBLIC_IO_KBDIF_H__
>  
> +/*
> + * Backend Xenstore Nodes
> + * ----------------------
> + *
> + * feature-abs-pointer
> + *   Values: 0/1 (boolean)
> + *   Default: 0
> + *
> + *   When set to "1", backend supports supplying absolute coordinates via
> + *   XENKBD_TYPE_POS messages. When set to "0", backend can only supply
> + *   relative movements via XENKBD_TYPE_MOTION messages.
> + *
> + * feature-no-abs-rescale
> + *   Values: 0/1 (boolean)
> + *   Default: 0
> + *
> + *   When set to "1", backend supports unscaled absolute coordinates. Unscaled
> + *   coordinates are in the range [0, 0x7fff]. When set to "0", backend can
> + *   only supply scaled coordinates. Scaled coordinates are scaled to the
> + *   'screen size' of the console. If "feature-abs-pointer" is "0", this value
> + *   has no effect.
> + *
> + * feature-no-console
> + *   Values: 0/1 (boolean)
> + *   Default: 0
> + *
> + *   When set to "1", backend supports connection without a console. When
> + *   running without a console, scaled values maximum is undefined. When
> + *   set to "0", backend will wait for a console before connecting.
> + *
> + * Frontend XenStore Nodes
> + * -----------------------
> + *
> + * request-abs-pointer
> + *   Values: 0/1 (boolean)
> + *   Default: 0
> + *
> + *   When set to "1", frontend wants absolute coordinates delivered with the
> + *   XENKBD_TYPE_POS message.
> + *
> + * request-no-abs-rescale
> + *   Values: 0/1 (boolean)
> + *   Default: 0
> + *
> + *   When set to "1", frontend wants unscaled absolute coordinates. If
> + *   "request-abs-pointer" is "0", this value has no effect.
> + *
> + * request-no-console
> + *   Values: 0/1 (boolean)
> + *   Default: 0
> + *
> + *   When set to "1", frontend does not require a console for connection.
> + *
> + * page-ref
> + *   Values: <uint64_t>
> + *   Optional, "page-gref" is used if "page-ref" is not set.
> + *
> + *   The MFN of a page of memory for the shared ring structures. If not
> + *   present, "page-gref" must be set. "page-ref" overrides "page-gref".
> + *
> + * page-gref
> + *   Values: <uint32_t>
> + *   Only required if "page-ref" is NOT set.
> + *
> + *   A grant reference to the memory page to be mapped for the shared ring
> + *   structures. Must be present if "page-ref" is not present.
> + *
> + * event-channel
> + *   Values: <uint32_t>
> + *
> + *   An event channel identifier, which is triggered when the shared page
> + *   is updated.
> + */
> +
>  /* In events (backend -> frontend) */
>  
>  /*
> -- 
> 2.1.0
> 

[Qemu-devel] [PATCH 2/2] xenfb: Add comment documentation

[Owen smith]

Add documentation for page-ref, page-gref and event-channel.

Signed-off-by: Owen smith <owen.smith@citrix.com>
---
 xen/include/public/io/fbif.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/xen/include/public/io/fbif.h b/xen/include/public/io/fbif.h
index cc25aab..ba3f524 100644
--- a/xen/include/public/io/fbif.h
+++ b/xen/include/public/io/fbif.h
@@ -26,6 +26,31 @@
 #ifndef __XEN_PUBLIC_IO_FBIF_H__
 #define __XEN_PUBLIC_IO_FBIF_H__
 
+/*
+ * Frontend XenStore Nodes
+ * -----------------------
+ *
+ * page-ref
+ *   Values: <uint64_t>
+ *   Optional, "page-gref" is used if "page-ref" is not set.
+ *
+ *   The MFN of a page of memory for the shared ring structures. If not
+ *   present, "page-gref" must be set.page-ref" overrides "page-gref".
+ *
+ * page-gref
+ *   Values: <uint32_t>
+ *   Only required if "page-ref" is NOT set.
+ *
+ *   A grant reference to the memory page to be mapped for the shared ring
+ *   structures. Must be present if "page-ref" is not present.
+ *
+ * event-channel
+ *   Values: <uint32_t>
+ *
+ *   An event channel identifier, which is triggered when the shared page
+ *   is updated.
+ */
+
 /* Out events (frontend -> backend) */
 
 /*
-- 
2.1.0

Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xenfb: Add comment documentation

[David Vrabel]

On 22/09/14 10:04, Owen smith wrote:
> Add documentation for page-ref, page-gref and event-channel.
> 
> Signed-off-by: Owen smith <owen.smith@citrix.com>
> ---
>  xen/include/public/io/fbif.h | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/xen/include/public/io/fbif.h b/xen/include/public/io/fbif.h
> index cc25aab..ba3f524 100644
> --- a/xen/include/public/io/fbif.h
> +++ b/xen/include/public/io/fbif.h
> @@ -26,6 +26,31 @@
>  #ifndef __XEN_PUBLIC_IO_FBIF_H__
>  #define __XEN_PUBLIC_IO_FBIF_H__
>  
> +/*
> + * Frontend XenStore Nodes
> + * -----------------------
> + *
> + * page-ref
> + *   Values: <uint64_t>
> + *   Optional, "page-gref" is used if "page-ref" is not set.
> + *
> + *   The MFN of a page of memory for the shared ring structures. If not
> + *   present, "page-gref" must be set.page-ref" overrides "page-gref".

I think you mean GFN here, not MFN.

> + * page-gref
> + *   Values: <uint32_t>
> + *   Only required if "page-ref" is NOT set.
> + *
> + *   A grant reference to the memory page to be mapped for the shared ring
> + *   structures. Must be present if "page-ref" is not present.

Should there be some negotiation for which of page-ref or page-gref is
required/preferred by the backend?

The same applies to kbdif as well.

David

Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xenfb: Add comment documentation

[Owen Smith]

> -----Original Message-----
> From: David Vrabel
> Sent: 22 September 2014 11:16
> To: Owen Smith; xen-devel@lists.xen.org
> Cc: Stefano Stabellini; qemu-devel@nongnu.org; Ian Campbell
> Subject: Re: [Xen-devel] [PATCH 2/2] xenfb: Add comment documentation
> 
> On 22/09/14 10:04, Owen smith wrote:
> > Add documentation for page-ref, page-gref and event-channel.
> >
> > Signed-off-by: Owen smith <owen.smith@citrix.com>
> > ---
> >  xen/include/public/io/fbif.h | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/xen/include/public/io/fbif.h
> > b/xen/include/public/io/fbif.h index cc25aab..ba3f524 100644
> > --- a/xen/include/public/io/fbif.h
> > +++ b/xen/include/public/io/fbif.h
> > @@ -26,6 +26,31 @@
> >  #ifndef __XEN_PUBLIC_IO_FBIF_H__
> >  #define __XEN_PUBLIC_IO_FBIF_H__
> >
> > +/*
> > + * Frontend XenStore Nodes
> > + * -----------------------
> > + *
> > + * page-ref
> > + *   Values: <uint64_t>
> > + *   Optional, "page-gref" is used if "page-ref" is not set.
> > + *
> > + *   The MFN of a page of memory for the shared ring structures. If not
> > + *   present, "page-gref" must be set.page-ref" overrides "page-gref".
> 
> I think you mean GFN here, not MFN.

GFN would be more correct.

> 
> > + * page-gref
> > + *   Values: <uint32_t>
> > + *   Only required if "page-ref" is NOT set.
> > + *
> > + *   A grant reference to the memory page to be mapped for the shared
> ring
> > + *   structures. Must be present if "page-ref" is not present.
> 
> Should there be some negotiation for which of page-ref or page-gref is
> required/preferred by the backend?
> 
> The same applies to kbdif as well.
> 

I've not added negotiation, as I wanted minimum changes when "page-ref" is specified.
With the kbdif device only, not investigated the vfb, a backend only supporting "page-gref" 
will break a lot of frontends (I saw breaks in Ubuntu, which advertises both if I forced page-gref)

> David

Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xenfb: Add comment documentation

[Ian Campbell]

On Fri, 2014-09-26 at 11:20 +0100, Owen Smith wrote:
> > > + * page-gref
> > > + *   Values: <uint32_t>
> > > + *   Only required if "page-ref" is NOT set.
> > > + *
> > > + *   A grant reference to the memory page to be mapped for the shared
> > ring
> > > + *   structures. Must be present if "page-ref" is not present.
> > 
> > Should there be some negotiation for which of page-ref or page-gref is
> > required/preferred by the backend?
> > 
> > The same applies to kbdif as well.
> > 
> 
> I've not added negotiation, as I wanted minimum changes when "page-ref" is specified.
> With the kbdif device only, not investigated the vfb, a backend only supporting "page-gref" 
> will break a lot of frontends (I saw breaks in Ubuntu, which advertises both if I forced page-gref)

Is this patch/series documenting existing behaviour or the new behaviour
which you are currently implementing? Both patches have the same title
and essentially the same commit message (list of fields), so I can't
tell.

Re: [Qemu-devel] [PATCH 2/2] xenfb: Add comment documentation

[Stefano Stabellini]

On Mon, 22 Sep 2014, Owen smith wrote:
> Add documentation for page-ref, page-gref and event-channel.
> 
> Signed-off-by: Owen smith <owen.smith@citrix.com>

thanks for the patch


>  xen/include/public/io/fbif.h | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/xen/include/public/io/fbif.h b/xen/include/public/io/fbif.h
> index cc25aab..ba3f524 100644
> --- a/xen/include/public/io/fbif.h
> +++ b/xen/include/public/io/fbif.h
> @@ -26,6 +26,31 @@
>  #ifndef __XEN_PUBLIC_IO_FBIF_H__
>  #define __XEN_PUBLIC_IO_FBIF_H__
>  
> +/*
> + * Frontend XenStore Nodes
> + * -----------------------
> + *
> + * page-ref
> + *   Values: <uint64_t>
> + *   Optional, "page-gref" is used if "page-ref" is not set.
> + *
> + *   The MFN of a page of memory for the shared ring structures. If not
> + *   present, "page-gref" must be set.page-ref" overrides "page-gref".
> + *
> + * page-gref
> + *   Values: <uint32_t>
> + *   Only required if "page-ref" is NOT set.
> + *
> + *   A grant reference to the memory page to be mapped for the shared ring
> + *   structures. Must be present if "page-ref" is not present.
> + *
> + * event-channel
> + *   Values: <uint32_t>
> + *
> + *   An event channel identifier, which is triggered when the shared page
> + *   is updated.
> + */

You are missing videoram, feature-update, feature-resize and
request-update.


>  /* Out events (frontend -> backend) */
>  
>  /*
> -- 
> 2.1.0
>