Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] displif: Protocol version 2
@ 2020-05-20  9:04 Oleksandr Andrushchenko
  2020-05-20  9:04 ` [PATCH 1/2] xen/displif: " Oleksandr Andrushchenko
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Oleksandr Andrushchenko @ 2020-05-20  9:04 UTC (permalink / raw)
  To: xen-devel, jgross, ian.jackson, wei.liu2, konrad.wilk
  Cc: Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Hello all,

this series extends the existing displif protocol with new
request and adds additional parameter to the exiting one.
It also provides support for the new parameter in libgnttab
via ioctl. The relevant changes in the backend can be found at [1]
and the frontend at [2].

List of changes:

1. Change protocol version from string to integer

Version string, which is in fact an integer, is hard to handle in the
code that supports different protocol versions. To simplify that
make the version an integer.

2. Pass buffer offset with XENDISPL_OP_DBUF_CREATE

There are cases when display data buffer is created with non-zero
offset to the data start. Handle such cases and provide that offset
while creating a display buffer. Add the corresponding filed to the
protocol and handle it in libgnttab.
This change is required for bringing virtual display on iMX8
platform which uses offset of 64 bytes for the buffers allocated
on GPU side and then imported into PV DRM frontend. Otherwise the
final picture looks shifted.

3. Add XENDISPL_OP_GET_EDID command

Add an optional request for reading Extended Display Identification
Data (EDID) structure which allows better configuration of the
display connectors over the configuration set in XenStore.
With this change connectors may have multiple resolutions defined
with respect to detailed timing definitions and additional properties
normally provided by displays.

If this request is not supported by the backend then visible area
is defined by the relevant XenStore's "resolution" property.

If backend provides extended display identification data (EDID) with
XENDISPL_OP_GET_EDID request then EDID values must take precedence
over the resolutions defined in XenStore.

4. Bump protocol version to 2.

Open questions and notes on the changes:

1. gnttab minor version change from 2 to 3
As per my understanding it is required to bump the version when
I add the new functionality, but I would like to hear from the
maintainers on that.

2. gnttab and version 2 of the ioctls
Because we add an additional parameter (data offset) and the structures
used to pass ioctl arguments cannot be extended (there are no enough
reserved fields), so there are to ways to solve that:
- break the existing API and add data_ofs field into the existing
structures
- create a copy of the ioctl (v2) with the additional parameter.

It seems to be easier to go the first way, but this breaks things,
so I decided to introduce v2 of the same ioctl which behaves gracefully
with respect to the existing users, but adds some amount of
copy-paste code (I was trying to minimize copy-paste so it is easier
to maintain, but the result looked ugly to me because of lots of
"if (protocol v1)" constructs.

Please note that struct ioctl_gntdev_dmabuf_imp_to_refs, e.g.
version 1 of the ioctl, has uint32_t reserved field which can be
used for the data offset, but its counterpart (ioctl_gntdev_dmabuf_exp_from_refs)
doesn't have any, so it seems not to be aligned to only introduce
version 2 of the ioctl for the later.

The other question is if to keep that reserved field in
ioctl_gntdev_dmabuf_imp_to_refs_v2 structure or drop it.

3. displif protocol version string to int conversion
The existing protocol defines its version as a string "1"
which is ok, but makes it not so easy to be directly used by
C/C++ preprocessor which would like to see an integer for constructs
like "#if XENDISPL_PROTOCOL_VERSION > 2"

At the same time this change may break the existing users of the protocol
which still expect version as a string. I tried something like

#define STR_HELPER(x) #x
#define STR(x) STR_HELPER(x)

#define XENDISPL_PROTOCOL_VERSION_INT 1
#define XENDISPL_PROTOCOL_VERSION STR(XENDISPL_PROTOCOL_VERSION_INT)

but not sure if this is the right and good way to solve the issue,
so in this series I have deliberately changed the protocol version to
int.
Other possible way could be that every user of the header has its local
copy (this is what we now use in the display backend). This way the
local copy can be changed in a way suitable for the concrete user.
This cannot be done (?) for the Linux Kernel though.

Thank you,
Oleksandr

[1] https://github.com/xen-troops/displ_be
[2] https://github.com/xen-troops/linux/pull/87

Oleksandr Andrushchenko (2):
  xen/displif: Protocol version 2
  libgnttab: Add support for Linux dma-buf offset

 tools/include/xen-sys/Linux/gntdev.h  | 53 +++++++++++++++++
 tools/libs/gnttab/Makefile            |  2 +-
 tools/libs/gnttab/freebsd.c           | 15 +++++
 tools/libs/gnttab/gnttab_core.c       | 17 ++++++
 tools/libs/gnttab/include/xengnttab.h | 13 ++++
 tools/libs/gnttab/libxengnttab.map    |  6 ++
 tools/libs/gnttab/linux.c             | 86 +++++++++++++++++++++++++++
 tools/libs/gnttab/minios.c            | 15 +++++
 tools/libs/gnttab/private.h           |  9 +++
 xen/include/public/io/displif.h       | 83 +++++++++++++++++++++++++-
 10 files changed, 295 insertions(+), 4 deletions(-)

-- 
2.17.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] xen/displif: Protocol version 2
  2020-05-20  9:04 [PATCH 0/2] displif: Protocol version 2 Oleksandr Andrushchenko
@ 2020-05-20  9:04 ` Oleksandr Andrushchenko
  2020-06-29  7:02   ` Jürgen Groß
  2020-05-20  9:04 ` [PATCH 2/2] libgnttab: Add support for Linux dma-buf offset Oleksandr Andrushchenko
  2020-06-01 16:01 ` [PATCH 0/2] displif: Protocol version 2 Oleksandr Andrushchenko
  2 siblings, 1 reply; 13+ messages in thread
From: Oleksandr Andrushchenko @ 2020-05-20  9:04 UTC (permalink / raw)
  To: xen-devel, jgross, ian.jackson, wei.liu2, konrad.wilk
  Cc: Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

1. Change protocol version from string to integer

Version string, which is in fact an integer, is hard to handle in the
code that supports different protocol versions. To simplify that
make the version an integer.

2. Pass buffer offset with XENDISPL_OP_DBUF_CREATE

There are cases when display data buffer is created with non-zero
offset to the data start. Handle such cases and provide that offset
while creating a display buffer.

3. Add XENDISPL_OP_GET_EDID command

Add an optional request for reading Extended Display Identification
Data (EDID) structure which allows better configuration of the
display connectors over the configuration set in XenStore.
With this change connectors may have multiple resolutions defined
with respect to detailed timing definitions and additional properties
normally provided by displays.

If this request is not supported by the backend then visible area
is defined by the relevant XenStore's "resolution" property.

If backend provides extended display identification data (EDID) with
XENDISPL_OP_GET_EDID request then EDID values must take precedence
over the resolutions defined in XenStore.

4. Bump protocol version to 2.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
 xen/include/public/io/displif.h | 83 +++++++++++++++++++++++++++++++--
 1 file changed, 80 insertions(+), 3 deletions(-)

diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h
index cc5de9cb1f35..4d43ba5078c8 100644
--- a/xen/include/public/io/displif.h
+++ b/xen/include/public/io/displif.h
@@ -38,7 +38,7 @@
  *                           Protocol version
  ******************************************************************************
  */
-#define XENDISPL_PROTOCOL_VERSION     "1"
+#define XENDISPL_PROTOCOL_VERSION     2
 
 /*
  ******************************************************************************
@@ -202,6 +202,9 @@
  *      Width and height of the connector in pixels separated by
  *      XENDISPL_RESOLUTION_SEPARATOR. This defines visible area of the
  *      display.
+ *      If backend provides extended display identification data (EDID) with
+ *      XENDISPL_OP_GET_EDID request then EDID values must take precedence
+ *      over the resolutions defined here.
  *
  *------------------ Connector Request Transport Parameters -------------------
  *
@@ -349,6 +352,7 @@
 #define XENDISPL_OP_FB_DETACH         0x13
 #define XENDISPL_OP_SET_CONFIG        0x14
 #define XENDISPL_OP_PG_FLIP           0x15
+#define XENDISPL_OP_GET_EDID          0x16
 
 /*
  ******************************************************************************
@@ -377,6 +381,10 @@
 #define XENDISPL_FIELD_BE_ALLOC       "be-alloc"
 #define XENDISPL_FIELD_UNIQUE_ID      "unique-id"
 
+#define XENDISPL_EDID_BLOCK_SIZE      128
+#define XENDISPL_EDID_BLOCK_COUNT     256
+#define XENDISPL_EDID_MAX_SIZE        (XENDISPL_EDID_BLOCK_SIZE * XENDISPL_EDID_BLOCK_COUNT)
+
 /*
  ******************************************************************************
  *                          STATUS RETURN CODES
@@ -451,7 +459,9 @@
  * +----------------+----------------+----------------+----------------+
  * |                           gref_directory                          | 40
  * +----------------+----------------+----------------+----------------+
- * |                             reserved                              | 44
+ * |                             data_ofs                              | 44
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 48
  * +----------------+----------------+----------------+----------------+
  * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
  * +----------------+----------------+----------------+----------------+
@@ -494,6 +504,7 @@
  *   buffer size (buffer_sz) exceeds what can be addressed by this single page,
  *   then reference to the next page must be supplied (see gref_dir_next_page
  *   below)
+ * data_ofs - uint32_t, offset of the data in the buffer, octets
  */
 
 #define XENDISPL_DBUF_FLG_REQ_ALLOC       (1 << 0)
@@ -506,6 +517,7 @@ struct xendispl_dbuf_create_req {
     uint32_t buffer_sz;
     uint32_t flags;
     grant_ref_t gref_directory;
+    uint32_t data_ofs;
 };
 
 /*
@@ -731,6 +743,42 @@ struct xendispl_page_flip_req {
     uint64_t fb_cookie;
 };
 
+/*
+ * Request EDID - request EDID describing current connector:
+ *         0                1                 2               3        octet
+ * +----------------+----------------+----------------+----------------+
+ * |               id                | _OP_GET_EDID   |   reserved     | 4
+ * +----------------+----------------+----------------+----------------+
+ * |                             buffer_sz                             | 8
+ * +----------------+----------------+----------------+----------------+
+ * |                          gref_directory                           | 12
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 16
+ * +----------------+----------------+----------------+----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 64
+ * +----------------+----------------+----------------+----------------+
+ *
+ * Notes:
+ *   - This request is optional and if not supported then visible area
+ *     is defined by the relevant XenStore's "resolution" property.
+ *   - Shared buffer, allocated for EDID storage, must not be less then
+ *     XENDISPL_EDID_MAX_SIZE octets.
+ *
+ * buffer_sz - uint32_t, buffer size to be allocated, octets
+ * gref_directory - grant_ref_t, a reference to the first shared page
+ *   describing EDID buffer references. See XENDISPL_OP_DBUF_CREATE for
+ *   grant page directory structure (struct xendispl_page_directory).
+ *
+ * See response format for this request.
+ */
+
+struct xendispl_get_edid_req {
+    uint32_t buffer_sz;
+    grant_ref_t gref_directory;
+};
+
 /*
  *---------------------------------- Responses --------------------------------
  *
@@ -753,6 +801,31 @@ struct xendispl_page_flip_req {
  * id - uint16_t, private guest value, echoed from request
  * status - int32_t, response status, zero on success and -XEN_EXX on failure
  *
+ *
+ * Get EDID response - response for XENDISPL_OP_GET_EDID:
+ *         0                1                 2               3        octet
+ * +----------------+----------------+----------------+----------------+
+ * |               id                |    operation   |    reserved    | 4
+ * +----------------+----------------+----------------+----------------+
+ * |                              status                               | 8
+ * +----------------+----------------+----------------+----------------+
+ * |                             edid_sz                               | 12
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 16
+ * +----------------+----------------+----------------+----------------+
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +----------------+----------------+----------------+----------------+
+ * |                             reserved                              | 64
+ * +----------------+----------------+----------------+----------------+
+ *
+ * edid_sz - uint32_t, size of the EDID, octets
+ */
+
+struct xendispl_get_edid_resp {
+    uint32_t edid_sz;
+};
+
+/*
  *----------------------------------- Events ----------------------------------
  *
  * Events are sent via a shared page allocated by the front and propagated by
@@ -804,6 +877,7 @@ struct xendispl_req {
         struct xendispl_fb_detach_req fb_detach;
         struct xendispl_set_config_req set_config;
         struct xendispl_page_flip_req pg_flip;
+        struct xendispl_get_edid_req get_edid;
         uint8_t reserved[56];
     } op;
 };
@@ -813,7 +887,10 @@ struct xendispl_resp {
     uint8_t operation;
     uint8_t reserved;
     int32_t status;
-    uint8_t reserved1[56];
+    union {
+        struct xendispl_get_edid_resp get_edid;
+        uint8_t reserved1[56];
+    } op;
 };
 
 struct xendispl_evt {
-- 
2.17.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/2] libgnttab: Add support for Linux dma-buf offset
  2020-05-20  9:04 [PATCH 0/2] displif: Protocol version 2 Oleksandr Andrushchenko
  2020-05-20  9:04 ` [PATCH 1/2] xen/displif: " Oleksandr Andrushchenko
@ 2020-05-20  9:04 ` Oleksandr Andrushchenko
  2020-06-30  9:42   ` Oleksandr Andrushchenko
  2020-07-31 10:53   ` Oleksandr Andrushchenko
  2020-06-01 16:01 ` [PATCH 0/2] displif: Protocol version 2 Oleksandr Andrushchenko
  2 siblings, 2 replies; 13+ messages in thread
From: Oleksandr Andrushchenko @ 2020-05-20  9:04 UTC (permalink / raw)
  To: xen-devel, jgross, ian.jackson, wei.liu2, konrad.wilk
  Cc: Oleksandr Andrushchenko

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Add version 2 of the dma-buf ioctls which adds data_ofs parameter.

dma-buf is backed by a scatter-gather table and has offset parameter
which tells where the actual data starts. Relevant ioctls are extended
to support that offset:
  - when dma-buf is created (exported) from grant references then
    data_ofs is used to set the offset field in the scatter list
    of the new dma-buf
  - when dma-buf is imported and grant references provided then
    data_ofs is used to report that offset to user-space

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
---
 tools/include/xen-sys/Linux/gntdev.h  | 53 +++++++++++++++++
 tools/libs/gnttab/Makefile            |  2 +-
 tools/libs/gnttab/freebsd.c           | 15 +++++
 tools/libs/gnttab/gnttab_core.c       | 17 ++++++
 tools/libs/gnttab/include/xengnttab.h | 13 ++++
 tools/libs/gnttab/libxengnttab.map    |  6 ++
 tools/libs/gnttab/linux.c             | 86 +++++++++++++++++++++++++++
 tools/libs/gnttab/minios.c            | 15 +++++
 tools/libs/gnttab/private.h           |  9 +++
 9 files changed, 215 insertions(+), 1 deletion(-)

diff --git a/tools/include/xen-sys/Linux/gntdev.h b/tools/include/xen-sys/Linux/gntdev.h
index d16076044c71..0c43393cbee5 100644
--- a/tools/include/xen-sys/Linux/gntdev.h
+++ b/tools/include/xen-sys/Linux/gntdev.h
@@ -274,4 +274,57 @@ struct ioctl_gntdev_dmabuf_imp_release {
     uint32_t reserved;
 };
 
+/*
+ * Version 2 of the ioctls adds @data_ofs parameter.
+ *
+ * dma-buf is backed by a scatter-gather table and has offset
+ * parameter which tells where the actual data starts.
+ * Relevant ioctls are extended to support that offset:
+ *   - when dma-buf is created (exported) from grant references then
+ *     @data_ofs is used to set the offset field in the scatter list
+ *     of the new dma-buf
+ *   - when dma-buf is imported and grant references are provided then
+ *     @data_ofs is used to report that offset to user-space
+ */
+#define IOCTL_GNTDEV_DMABUF_EXP_FROM_REFS_V2 \
+    _IOC(_IOC_NONE, 'G', 13, \
+         sizeof(struct ioctl_gntdev_dmabuf_exp_from_refs_v2))
+struct ioctl_gntdev_dmabuf_exp_from_refs_v2 {
+    /* IN parameters. */
+    /* Specific options for this dma-buf: see GNTDEV_DMA_FLAG_XXX. */
+    uint32_t flags;
+    /* Number of grant references in @refs array. */
+    uint32_t count;
+    /* Offset of the data in the dma-buf. */
+    uint32_t data_ofs;
+    /* OUT parameters. */
+    /* File descriptor of the dma-buf. */
+    uint32_t fd;
+    /* The domain ID of the grant references to be mapped. */
+    uint32_t domid;
+    /* Variable IN parameter. */
+    /* Array of grant references of size @count. */
+    uint32_t refs[1];
+};
+
+#define IOCTL_GNTDEV_DMABUF_IMP_TO_REFS_V2 \
+    _IOC(_IOC_NONE, 'G', 14, \
+         sizeof(struct ioctl_gntdev_dmabuf_imp_to_refs_v2))
+struct ioctl_gntdev_dmabuf_imp_to_refs_v2 {
+    /* IN parameters. */
+    /* File descriptor of the dma-buf. */
+    uint32_t fd;
+    /* Number of grant references in @refs array. */
+    uint32_t count;
+    /* The domain ID for which references to be granted. */
+    uint32_t domid;
+    /* Reserved - must be zero. */
+    uint32_t reserved;
+    /* OUT parameters. */
+    /* Offset of the data in the dma-buf. */
+    uint32_t data_ofs;
+    /* Array of grant references of size @count. */
+    uint32_t refs[1];
+};
+
 #endif /* __LINUX_PUBLIC_GNTDEV_H__ */
diff --git a/tools/libs/gnttab/Makefile b/tools/libs/gnttab/Makefile
index 2da8fbbb7f6f..5ee2d965214f 100644
--- a/tools/libs/gnttab/Makefile
+++ b/tools/libs/gnttab/Makefile
@@ -2,7 +2,7 @@ XEN_ROOT = $(CURDIR)/../../..
 include $(XEN_ROOT)/tools/Rules.mk
 
 MAJOR    = 1
-MINOR    = 2
+MINOR    = 3
 LIBNAME  := gnttab
 USELIBS  := toollog toolcore
 
diff --git a/tools/libs/gnttab/freebsd.c b/tools/libs/gnttab/freebsd.c
index 886b588303a0..baf0f60aa4d3 100644
--- a/tools/libs/gnttab/freebsd.c
+++ b/tools/libs/gnttab/freebsd.c
@@ -319,6 +319,14 @@ int osdep_gnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
     abort();
 }
 
+int osdep_gnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
+                                         uint32_t flags, uint32_t count,
+                                         const uint32_t *refs,
+                                         uint32_t *dmabuf_fd, uint32_t data_ofs)
+{
+    abort();
+}
+
 int osdep_gnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt,
                                           uint32_t fd, uint32_t wait_to_ms)
 {
@@ -331,6 +339,13 @@ int osdep_gnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
     abort();
 }
 
+int osdep_gnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
+                                       uint32_t fd, uint32_t count,
+                                       uint32_t *refs, uint32_t *data_ofs)
+{
+    abort();
+}
+
 int osdep_gnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd)
 {
     abort();
diff --git a/tools/libs/gnttab/gnttab_core.c b/tools/libs/gnttab/gnttab_core.c
index 92e7228a2671..3af3cec80045 100644
--- a/tools/libs/gnttab/gnttab_core.c
+++ b/tools/libs/gnttab/gnttab_core.c
@@ -144,6 +144,15 @@ int xengnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
                                              refs, fd);
 }
 
+int xengnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
+                                      uint32_t flags, uint32_t count,
+                                      const uint32_t *refs, uint32_t *fd,
+                                      uint32_t data_ofs)
+{
+    return osdep_gnttab_dmabuf_exp_from_refs_v2(xgt, domid, flags, count,
+                                                refs, fd, data_ofs);
+}
+
 int xengnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt, uint32_t fd,
                                        uint32_t wait_to_ms)
 {
@@ -156,6 +165,14 @@ int xengnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
     return osdep_gnttab_dmabuf_imp_to_refs(xgt, domid, fd, count, refs);
 }
 
+int xengnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
+                                    uint32_t fd, uint32_t count, uint32_t *refs,
+                                    uint32_t *data_ofs)
+{
+    return osdep_gnttab_dmabuf_imp_to_refs_v2(xgt, domid, fd, count, refs,
+                                              data_ofs);
+}
+
 int xengnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd)
 {
     return osdep_gnttab_dmabuf_imp_release(xgt, fd);
diff --git a/tools/libs/gnttab/include/xengnttab.h b/tools/libs/gnttab/include/xengnttab.h
index 111fc88caeb3..0956bd91e0df 100644
--- a/tools/libs/gnttab/include/xengnttab.h
+++ b/tools/libs/gnttab/include/xengnttab.h
@@ -322,12 +322,19 @@ int xengnttab_grant_copy(xengnttab_handle *xgt,
  * Returns 0 if dma-buf was successfully created and the corresponding
  * dma-buf's file descriptor is returned in @fd.
  *
+ * Version 2 also accepts @data_ofs offset of the data in the buffer.
+ *
  * [1] https://elixir.bootlin.com/linux/latest/source/Documentation/driver-api/dma-buf.rst
  */
 int xengnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
                                    uint32_t flags, uint32_t count,
                                    const uint32_t *refs, uint32_t *fd);
 
+int xengnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
+                                      uint32_t flags, uint32_t count,
+                                      const uint32_t *refs, uint32_t *fd,
+                                      uint32_t data_ofs);
+
 /*
  * This will block until the dma-buf with the file descriptor @fd is
  * released. This is only valid for buffers created with
@@ -345,10 +352,16 @@ int xengnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt, uint32_t fd,
 /*
  * Import a dma-buf with file descriptor @fd and export granted references
  * to the pages of that dma-buf into array @refs of size @count.
+ *
+ * Version 2 also provides @data_ofs offset of the data in the buffer.
  */
 int xengnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
                                  uint32_t fd, uint32_t count, uint32_t *refs);
 
+int xengnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
+                                    uint32_t fd, uint32_t count, uint32_t *refs,
+                                    uint32_t *data_ofs);
+
 /*
  * This will close all references to an imported buffer, so it can be
  * released by the owner. This is only valid for buffers created with
diff --git a/tools/libs/gnttab/libxengnttab.map b/tools/libs/gnttab/libxengnttab.map
index d2a9b7e18bea..ddf77e064b08 100644
--- a/tools/libs/gnttab/libxengnttab.map
+++ b/tools/libs/gnttab/libxengnttab.map
@@ -36,3 +36,9 @@ VERS_1.2 {
 		xengnttab_dmabuf_imp_to_refs;
 		xengnttab_dmabuf_imp_release;
 } VERS_1.1;
+
+VERS_1.3 {
+    global:
+		xengnttab_dmabuf_exp_from_refs_v2;
+		xengnttab_dmabuf_imp_to_refs_v2;
+} VERS_1.2;
diff --git a/tools/libs/gnttab/linux.c b/tools/libs/gnttab/linux.c
index a01bb6c698c6..75e249fb3202 100644
--- a/tools/libs/gnttab/linux.c
+++ b/tools/libs/gnttab/linux.c
@@ -352,6 +352,51 @@ out:
     return rc;
 }
 
+int osdep_gnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
+                                         uint32_t flags, uint32_t count,
+                                         const uint32_t *refs,
+                                         uint32_t *dmabuf_fd,
+                                         uint32_t data_ofs)
+{
+    struct ioctl_gntdev_dmabuf_exp_from_refs_v2 *from_refs_v2 = NULL;
+    int rc = -1;
+
+    if ( !count )
+    {
+        errno = EINVAL;
+        goto out;
+    }
+
+    from_refs_v2 = malloc(sizeof(*from_refs_v2) +
+                          (count - 1) * sizeof(from_refs_v2->refs[0]));
+    if ( !from_refs_v2 )
+    {
+        errno = ENOMEM;
+        goto out;
+    }
+
+    from_refs_v2->flags = flags;
+    from_refs_v2->count = count;
+    from_refs_v2->domid = domid;
+    from_refs_v2->data_ofs = data_ofs;
+
+    memcpy(from_refs_v2->refs, refs, count * sizeof(from_refs_v2->refs[0]));
+
+    if ( (rc = ioctl(xgt->fd, IOCTL_GNTDEV_DMABUF_EXP_FROM_REFS_V2,
+                     from_refs_v2)) )
+    {
+        GTERROR(xgt->logger, "ioctl DMABUF_EXP_FROM_REFS_V2 failed");
+        goto out;
+    }
+
+    *dmabuf_fd = from_refs_v2->fd;
+    rc = 0;
+
+out:
+    free(from_refs_v2);
+    return rc;
+}
+
 int osdep_gnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt,
                                           uint32_t fd, uint32_t wait_to_ms)
 {
@@ -413,6 +458,47 @@ out:
     return rc;
 }
 
+int osdep_gnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
+                                       uint32_t fd, uint32_t count,
+                                       uint32_t *refs,
+                                       uint32_t *data_ofs)
+{
+    struct ioctl_gntdev_dmabuf_imp_to_refs_v2 *to_refs_v2 = NULL;
+    int rc = -1;
+
+    if ( !count )
+    {
+        errno = EINVAL;
+        goto out;
+    }
+
+    to_refs_v2 = malloc(sizeof(*to_refs_v2) +
+                        (count - 1) * sizeof(to_refs_v2->refs[0]));
+    if ( !to_refs_v2 )
+    {
+        errno = ENOMEM;
+        goto out;
+    }
+
+    to_refs_v2->fd = fd;
+    to_refs_v2->count = count;
+    to_refs_v2->domid = domid;
+
+    if ( (rc = ioctl(xgt->fd, IOCTL_GNTDEV_DMABUF_IMP_TO_REFS_V2, to_refs_v2)) )
+    {
+        GTERROR(xgt->logger, "ioctl DMABUF_IMP_TO_REFS_V2 failed");
+        goto out;
+    }
+
+    memcpy(refs, to_refs_v2->refs, count * sizeof(*refs));
+    *data_ofs = to_refs_v2->data_ofs;
+    rc = 0;
+
+out:
+    free(to_refs_v2);
+    return rc;
+}
+
 int osdep_gnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd)
 {
     struct ioctl_gntdev_dmabuf_imp_release release;
diff --git a/tools/libs/gnttab/minios.c b/tools/libs/gnttab/minios.c
index f78caadd3043..298416b2a98d 100644
--- a/tools/libs/gnttab/minios.c
+++ b/tools/libs/gnttab/minios.c
@@ -120,6 +120,14 @@ int osdep_gnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
     return -1;
 }
 
+int osdep_gnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
+                                         uint32_t flags, uint32_t count,
+                                         const uint32_t *refs, uint32_t *fd,
+                                         uint32_t data_ofs)
+{
+    return -1;
+}
+
 int osdep_gnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt,
                                           uint32_t fd, uint32_t wait_to_ms)
 {
@@ -133,6 +141,13 @@ int osdep_gnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
     return -1;
 }
 
+int osdep_gnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
+                                       uint32_t fd, uint32_t count,
+                                       uint32_t *refs, uint32_t *data_ofs)
+{
+    return -1;
+}
+
 int osdep_gnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd)
 {
     return -1;
diff --git a/tools/libs/gnttab/private.h b/tools/libs/gnttab/private.h
index c5e23639b141..07271637f609 100644
--- a/tools/libs/gnttab/private.h
+++ b/tools/libs/gnttab/private.h
@@ -39,6 +39,11 @@ int osdep_gnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
                                       uint32_t flags, uint32_t count,
                                       const uint32_t *refs, uint32_t *fd);
 
+int osdep_gnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
+                                         uint32_t flags, uint32_t count,
+                                         const uint32_t *refs, uint32_t *fd,
+                                         uint32_t data_ofs);
+
 int osdep_gnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt,
                                           uint32_t fd, uint32_t wait_to_ms);
 
@@ -46,6 +51,10 @@ int osdep_gnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
                                     uint32_t fd, uint32_t count,
                                     uint32_t *refs);
 
+int osdep_gnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
+                                       uint32_t fd, uint32_t count,
+                                       uint32_t *refs, uint32_t *data_ofs);
+
 int osdep_gnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd);
 
 int osdep_gntshr_open(xengntshr_handle *xgs);
-- 
2.17.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] displif: Protocol version 2
  2020-05-20  9:04 [PATCH 0/2] displif: Protocol version 2 Oleksandr Andrushchenko
  2020-05-20  9:04 ` [PATCH 1/2] xen/displif: " Oleksandr Andrushchenko
  2020-05-20  9:04 ` [PATCH 2/2] libgnttab: Add support for Linux dma-buf offset Oleksandr Andrushchenko
@ 2020-06-01 16:01 ` Oleksandr Andrushchenko
  2 siblings, 0 replies; 13+ messages in thread
From: Oleksandr Andrushchenko @ 2020-06-01 16:01 UTC (permalink / raw)
  To: xen-devel, jgross, ian.jackson, wl; +Cc: andr2000

ping

On 5/20/20 12:04 PM, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>
> Hello all,
>
> this series extends the existing displif protocol with new
> request and adds additional parameter to the exiting one.
> It also provides support for the new parameter in libgnttab
> via ioctl. The relevant changes in the backend can be found at [1]
> and the frontend at [2].
>
> List of changes:
>
> 1. Change protocol version from string to integer
>
> Version string, which is in fact an integer, is hard to handle in the
> code that supports different protocol versions. To simplify that
> make the version an integer.
>
> 2. Pass buffer offset with XENDISPL_OP_DBUF_CREATE
>
> There are cases when display data buffer is created with non-zero
> offset to the data start. Handle such cases and provide that offset
> while creating a display buffer. Add the corresponding filed to the
> protocol and handle it in libgnttab.
> This change is required for bringing virtual display on iMX8
> platform which uses offset of 64 bytes for the buffers allocated
> on GPU side and then imported into PV DRM frontend. Otherwise the
> final picture looks shifted.
>
> 3. Add XENDISPL_OP_GET_EDID command
>
> Add an optional request for reading Extended Display Identification
> Data (EDID) structure which allows better configuration of the
> display connectors over the configuration set in XenStore.
> With this change connectors may have multiple resolutions defined
> with respect to detailed timing definitions and additional properties
> normally provided by displays.
>
> If this request is not supported by the backend then visible area
> is defined by the relevant XenStore's "resolution" property.
>
> If backend provides extended display identification data (EDID) with
> XENDISPL_OP_GET_EDID request then EDID values must take precedence
> over the resolutions defined in XenStore.
>
> 4. Bump protocol version to 2.
>
> Open questions and notes on the changes:
>
> 1. gnttab minor version change from 2 to 3
> As per my understanding it is required to bump the version when
> I add the new functionality, but I would like to hear from the
> maintainers on that.
>
> 2. gnttab and version 2 of the ioctls
> Because we add an additional parameter (data offset) and the structures
> used to pass ioctl arguments cannot be extended (there are no enough
> reserved fields), so there are to ways to solve that:
> - break the existing API and add data_ofs field into the existing
> structures
> - create a copy of the ioctl (v2) with the additional parameter.
>
> It seems to be easier to go the first way, but this breaks things,
> so I decided to introduce v2 of the same ioctl which behaves gracefully
> with respect to the existing users, but adds some amount of
> copy-paste code (I was trying to minimize copy-paste so it is easier
> to maintain, but the result looked ugly to me because of lots of
> "if (protocol v1)" constructs.
>
> Please note that struct ioctl_gntdev_dmabuf_imp_to_refs, e.g.
> version 1 of the ioctl, has uint32_t reserved field which can be
> used for the data offset, but its counterpart (ioctl_gntdev_dmabuf_exp_from_refs)
> doesn't have any, so it seems not to be aligned to only introduce
> version 2 of the ioctl for the later.
>
> The other question is if to keep that reserved field in
> ioctl_gntdev_dmabuf_imp_to_refs_v2 structure or drop it.
>
> 3. displif protocol version string to int conversion
> The existing protocol defines its version as a string "1"
> which is ok, but makes it not so easy to be directly used by
> C/C++ preprocessor which would like to see an integer for constructs
> like "#if XENDISPL_PROTOCOL_VERSION > 2"
>
> At the same time this change may break the existing users of the protocol
> which still expect version as a string. I tried something like
>
> #define STR_HELPER(x) #x
> #define STR(x) STR_HELPER(x)
>
> #define XENDISPL_PROTOCOL_VERSION_INT 1
> #define XENDISPL_PROTOCOL_VERSION STR(XENDISPL_PROTOCOL_VERSION_INT)
>
> but not sure if this is the right and good way to solve the issue,
> so in this series I have deliberately changed the protocol version to
> int.
> Other possible way could be that every user of the header has its local
> copy (this is what we now use in the display backend). This way the
> local copy can be changed in a way suitable for the concrete user.
> This cannot be done (?) for the Linux Kernel though.
>
> Thank you,
> Oleksandr
>
> [1] https://github.com/xen-troops/displ_be
> [2] https://github.com/xen-troops/linux/pull/87
>
> Oleksandr Andrushchenko (2):
>    xen/displif: Protocol version 2
>    libgnttab: Add support for Linux dma-buf offset
>
>   tools/include/xen-sys/Linux/gntdev.h  | 53 +++++++++++++++++
>   tools/libs/gnttab/Makefile            |  2 +-
>   tools/libs/gnttab/freebsd.c           | 15 +++++
>   tools/libs/gnttab/gnttab_core.c       | 17 ++++++
>   tools/libs/gnttab/include/xengnttab.h | 13 ++++
>   tools/libs/gnttab/libxengnttab.map    |  6 ++
>   tools/libs/gnttab/linux.c             | 86 +++++++++++++++++++++++++++
>   tools/libs/gnttab/minios.c            | 15 +++++
>   tools/libs/gnttab/private.h           |  9 +++
>   xen/include/public/io/displif.h       | 83 +++++++++++++++++++++++++-
>   10 files changed, 295 insertions(+), 4 deletions(-)
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] xen/displif: Protocol version 2
  2020-05-20  9:04 ` [PATCH 1/2] xen/displif: " Oleksandr Andrushchenko
@ 2020-06-29  7:02   ` Jürgen Groß
  2020-06-30  6:13     ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 13+ messages in thread
From: Jürgen Groß @ 2020-06-29  7:02 UTC (permalink / raw)
  To: Oleksandr Andrushchenko, xen-devel, ian.jackson, wei.liu2, konrad.wilk
  Cc: Oleksandr Andrushchenko

On 20.05.20 11:04, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> 
> 1. Change protocol version from string to integer
> 
> Version string, which is in fact an integer, is hard to handle in the
> code that supports different protocol versions. To simplify that
> make the version an integer.
> 
> 2. Pass buffer offset with XENDISPL_OP_DBUF_CREATE
> 
> There are cases when display data buffer is created with non-zero
> offset to the data start. Handle such cases and provide that offset
> while creating a display buffer.
> 
> 3. Add XENDISPL_OP_GET_EDID command
> 
> Add an optional request for reading Extended Display Identification
> Data (EDID) structure which allows better configuration of the
> display connectors over the configuration set in XenStore.
> With this change connectors may have multiple resolutions defined
> with respect to detailed timing definitions and additional properties
> normally provided by displays.
> 
> If this request is not supported by the backend then visible area
> is defined by the relevant XenStore's "resolution" property.
> 
> If backend provides extended display identification data (EDID) with
> XENDISPL_OP_GET_EDID request then EDID values must take precedence
> over the resolutions defined in XenStore.
> 
> 4. Bump protocol version to 2.
> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> ---
>   xen/include/public/io/displif.h | 83 +++++++++++++++++++++++++++++++--
>   1 file changed, 80 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h
> index cc5de9cb1f35..4d43ba5078c8 100644
> --- a/xen/include/public/io/displif.h
> +++ b/xen/include/public/io/displif.h
> @@ -38,7 +38,7 @@
>    *                           Protocol version
>    ******************************************************************************
>    */
> -#define XENDISPL_PROTOCOL_VERSION     "1"
> +#define XENDISPL_PROTOCOL_VERSION     2

This is not very nice regarding compatibility.

Can't you add a new macro for the integer value?

And please add comments further down which additions are related to
the new version.


Juergen


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] xen/displif: Protocol version 2
  2020-06-29  7:02   ` Jürgen Groß
@ 2020-06-30  6:13     ` Oleksandr Andrushchenko
  2020-06-30  7:03       ` Jürgen Groß
  0 siblings, 1 reply; 13+ messages in thread
From: Oleksandr Andrushchenko @ 2020-06-30  6:13 UTC (permalink / raw)
  To: Jürgen Groß,
	Oleksandr Andrushchenko, xen-devel, ian.jackson, wei.liu2,
	konrad.wilk

On 6/29/20 10:02 AM, Jürgen Groß wrote:
> On 20.05.20 11:04, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>
>> 1. Change protocol version from string to integer
>>
>> Version string, which is in fact an integer, is hard to handle in the
>> code that supports different protocol versions. To simplify that
>> make the version an integer.
>>
>> 2. Pass buffer offset with XENDISPL_OP_DBUF_CREATE
>>
>> There are cases when display data buffer is created with non-zero
>> offset to the data start. Handle such cases and provide that offset
>> while creating a display buffer.
>>
>> 3. Add XENDISPL_OP_GET_EDID command
>>
>> Add an optional request for reading Extended Display Identification
>> Data (EDID) structure which allows better configuration of the
>> display connectors over the configuration set in XenStore.
>> With this change connectors may have multiple resolutions defined
>> with respect to detailed timing definitions and additional properties
>> normally provided by displays.
>>
>> If this request is not supported by the backend then visible area
>> is defined by the relevant XenStore's "resolution" property.
>>
>> If backend provides extended display identification data (EDID) with
>> XENDISPL_OP_GET_EDID request then EDID values must take precedence
>> over the resolutions defined in XenStore.
>>
>> 4. Bump protocol version to 2.
>>
>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> ---
>>   xen/include/public/io/displif.h | 83 +++++++++++++++++++++++++++++++--
>>   1 file changed, 80 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h
>> index cc5de9cb1f35..4d43ba5078c8 100644
>> --- a/xen/include/public/io/displif.h
>> +++ b/xen/include/public/io/displif.h
>> @@ -38,7 +38,7 @@
>>    *                           Protocol version
>> ******************************************************************************
>>    */
>> -#define XENDISPL_PROTOCOL_VERSION     "1"
>> +#define XENDISPL_PROTOCOL_VERSION     2
>
> This is not very nice regarding compatibility.
>
> Can't you add a new macro for the integer value?

We can leave it as is, e.g. define XENDISPL_PROTOCOL_VERSION as "2",

so we do not break the existing users. Then if every user of the header has

its local copy (this is what we now use in the display backend), then that
local copy can be changed in a way suitable for the concrete user, e.g. "2"

redefined to 2. This cannot be done (?) for the Linux Kernel though.

Or we can have

#define XENDISPL_PROTOCOL_VERSION     "2"

#define XENDISPL_PROTOCOL_VERSION_INT  2

Jurgen, what's your preference here?

>
> And please add comments further down which additions are related to
> the new version.
I will after the review is done and other comments are fixed
>
>
> Juergen

Thank you,

Oleksandr

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] xen/displif: Protocol version 2
  2020-06-30  6:13     ` Oleksandr Andrushchenko
@ 2020-06-30  7:03       ` Jürgen Groß
  2020-06-30  7:09         ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 13+ messages in thread
From: Jürgen Groß @ 2020-06-30  7:03 UTC (permalink / raw)
  To: Oleksandr Andrushchenko, Oleksandr Andrushchenko, xen-devel,
	ian.jackson, wei.liu2, konrad.wilk

On 30.06.20 08:13, Oleksandr Andrushchenko wrote:
> On 6/29/20 10:02 AM, Jürgen Groß wrote:
>> On 20.05.20 11:04, Oleksandr Andrushchenko wrote:
>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>
>>> 1. Change protocol version from string to integer
>>>
>>> Version string, which is in fact an integer, is hard to handle in the
>>> code that supports different protocol versions. To simplify that
>>> make the version an integer.
>>>
>>> 2. Pass buffer offset with XENDISPL_OP_DBUF_CREATE
>>>
>>> There are cases when display data buffer is created with non-zero
>>> offset to the data start. Handle such cases and provide that offset
>>> while creating a display buffer.
>>>
>>> 3. Add XENDISPL_OP_GET_EDID command
>>>
>>> Add an optional request for reading Extended Display Identification
>>> Data (EDID) structure which allows better configuration of the
>>> display connectors over the configuration set in XenStore.
>>> With this change connectors may have multiple resolutions defined
>>> with respect to detailed timing definitions and additional properties
>>> normally provided by displays.
>>>
>>> If this request is not supported by the backend then visible area
>>> is defined by the relevant XenStore's "resolution" property.
>>>
>>> If backend provides extended display identification data (EDID) with
>>> XENDISPL_OP_GET_EDID request then EDID values must take precedence
>>> over the resolutions defined in XenStore.
>>>
>>> 4. Bump protocol version to 2.
>>>
>>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>> ---
>>>    xen/include/public/io/displif.h | 83 +++++++++++++++++++++++++++++++--
>>>    1 file changed, 80 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h
>>> index cc5de9cb1f35..4d43ba5078c8 100644
>>> --- a/xen/include/public/io/displif.h
>>> +++ b/xen/include/public/io/displif.h
>>> @@ -38,7 +38,7 @@
>>>     *                           Protocol version
>>> ******************************************************************************
>>>     */
>>> -#define XENDISPL_PROTOCOL_VERSION     "1"
>>> +#define XENDISPL_PROTOCOL_VERSION     2
>>
>> This is not very nice regarding compatibility.
>>
>> Can't you add a new macro for the integer value?
> 
> We can leave it as is, e.g. define XENDISPL_PROTOCOL_VERSION as "2",
> 
> so we do not break the existing users. Then if every user of the header has
> 
> its local copy (this is what we now use in the display backend), then that
> local copy can be changed in a way suitable for the concrete user, e.g. "2"
> 
> redefined to 2. This cannot be done (?) for the Linux Kernel though.
> 
> Or we can have
> 
> #define XENDISPL_PROTOCOL_VERSION     "2"
> 
> #define XENDISPL_PROTOCOL_VERSION_INT  2
> 
> Jurgen, what's your preference here?

I would prefer the latter, as it avoids the need to modify the header
when copying it to a local project.


Juergen


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] xen/displif: Protocol version 2
  2020-06-30  7:03       ` Jürgen Groß
@ 2020-06-30  7:09         ` Oleksandr Andrushchenko
  2020-06-30  7:30           ` Jürgen Groß
  0 siblings, 1 reply; 13+ messages in thread
From: Oleksandr Andrushchenko @ 2020-06-30  7:09 UTC (permalink / raw)
  To: Jürgen Groß,
	Oleksandr Andrushchenko, xen-devel, ian.jackson, wl, konrad.wilk

On 6/30/20 10:03 AM, Jürgen Groß wrote:
> On 30.06.20 08:13, Oleksandr Andrushchenko wrote:
>> On 6/29/20 10:02 AM, Jürgen Groß wrote:
>>> On 20.05.20 11:04, Oleksandr Andrushchenko wrote:
>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>>
>>>> 1. Change protocol version from string to integer
>>>>
>>>> Version string, which is in fact an integer, is hard to handle in the
>>>> code that supports different protocol versions. To simplify that
>>>> make the version an integer.
>>>>
>>>> 2. Pass buffer offset with XENDISPL_OP_DBUF_CREATE
>>>>
>>>> There are cases when display data buffer is created with non-zero
>>>> offset to the data start. Handle such cases and provide that offset
>>>> while creating a display buffer.
>>>>
>>>> 3. Add XENDISPL_OP_GET_EDID command
>>>>
>>>> Add an optional request for reading Extended Display Identification
>>>> Data (EDID) structure which allows better configuration of the
>>>> display connectors over the configuration set in XenStore.
>>>> With this change connectors may have multiple resolutions defined
>>>> with respect to detailed timing definitions and additional properties
>>>> normally provided by displays.
>>>>
>>>> If this request is not supported by the backend then visible area
>>>> is defined by the relevant XenStore's "resolution" property.
>>>>
>>>> If backend provides extended display identification data (EDID) with
>>>> XENDISPL_OP_GET_EDID request then EDID values must take precedence
>>>> over the resolutions defined in XenStore.
>>>>
>>>> 4. Bump protocol version to 2.
>>>>
>>>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>> ---
>>>>    xen/include/public/io/displif.h | 83 +++++++++++++++++++++++++++++++--
>>>>    1 file changed, 80 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h
>>>> index cc5de9cb1f35..4d43ba5078c8 100644
>>>> --- a/xen/include/public/io/displif.h
>>>> +++ b/xen/include/public/io/displif.h
>>>> @@ -38,7 +38,7 @@
>>>>     *                           Protocol version
>>>> ******************************************************************************
>>>>     */
>>>> -#define XENDISPL_PROTOCOL_VERSION     "1"
>>>> +#define XENDISPL_PROTOCOL_VERSION     2
>>>
>>> This is not very nice regarding compatibility.
>>>
>>> Can't you add a new macro for the integer value?
>>
>> We can leave it as is, e.g. define XENDISPL_PROTOCOL_VERSION as "2",
>>
>> so we do not break the existing users. Then if every user of the header has
>>
>> its local copy (this is what we now use in the display backend), then that
>> local copy can be changed in a way suitable for the concrete user, e.g. "2"
>>
>> redefined to 2. This cannot be done (?) for the Linux Kernel though.
>>
>> Or we can have
>>
>> #define XENDISPL_PROTOCOL_VERSION     "2"
>>
>> #define XENDISPL_PROTOCOL_VERSION_INT  2
>>
>> Jurgen, what's your preference here?
>
> I would prefer the latter, as it avoids the need to modify the header
> when copying it to a local project.
>
Ok, I'll have 2 definitions then

Anything else (beside the comments on new functionality) I can add/change

before sending v2 of the patch?

>
> Juergen


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] xen/displif: Protocol version 2
  2020-06-30  7:09         ` Oleksandr Andrushchenko
@ 2020-06-30  7:30           ` Jürgen Groß
  2020-06-30  7:39             ` Oleksandr Andrushchenko
  0 siblings, 1 reply; 13+ messages in thread
From: Jürgen Groß @ 2020-06-30  7:30 UTC (permalink / raw)
  To: Oleksandr Andrushchenko, Oleksandr Andrushchenko, xen-devel,
	ian.jackson, wl, konrad.wilk

On 30.06.20 09:09, Oleksandr Andrushchenko wrote:
> On 6/30/20 10:03 AM, Jürgen Groß wrote:
>> On 30.06.20 08:13, Oleksandr Andrushchenko wrote:
>>> On 6/29/20 10:02 AM, Jürgen Groß wrote:
>>>> On 20.05.20 11:04, Oleksandr Andrushchenko wrote:
>>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>>>
>>>>> 1. Change protocol version from string to integer
>>>>>
>>>>> Version string, which is in fact an integer, is hard to handle in the
>>>>> code that supports different protocol versions. To simplify that
>>>>> make the version an integer.
>>>>>
>>>>> 2. Pass buffer offset with XENDISPL_OP_DBUF_CREATE
>>>>>
>>>>> There are cases when display data buffer is created with non-zero
>>>>> offset to the data start. Handle such cases and provide that offset
>>>>> while creating a display buffer.
>>>>>
>>>>> 3. Add XENDISPL_OP_GET_EDID command
>>>>>
>>>>> Add an optional request for reading Extended Display Identification
>>>>> Data (EDID) structure which allows better configuration of the
>>>>> display connectors over the configuration set in XenStore.
>>>>> With this change connectors may have multiple resolutions defined
>>>>> with respect to detailed timing definitions and additional properties
>>>>> normally provided by displays.
>>>>>
>>>>> If this request is not supported by the backend then visible area
>>>>> is defined by the relevant XenStore's "resolution" property.
>>>>>
>>>>> If backend provides extended display identification data (EDID) with
>>>>> XENDISPL_OP_GET_EDID request then EDID values must take precedence
>>>>> over the resolutions defined in XenStore.
>>>>>
>>>>> 4. Bump protocol version to 2.
>>>>>
>>>>> Signed-off-by: Oleksandr Andrushchenko 
>>>>> <oleksandr_andrushchenko@epam.com>
>>>>> ---
>>>>>    xen/include/public/io/displif.h | 83 
>>>>> +++++++++++++++++++++++++++++++--
>>>>>    1 file changed, 80 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/xen/include/public/io/displif.h 
>>>>> b/xen/include/public/io/displif.h
>>>>> index cc5de9cb1f35..4d43ba5078c8 100644
>>>>> --- a/xen/include/public/io/displif.h
>>>>> +++ b/xen/include/public/io/displif.h
>>>>> @@ -38,7 +38,7 @@
>>>>>     *                           Protocol version
>>>>> ****************************************************************************** 
>>>>>
>>>>>     */
>>>>> -#define XENDISPL_PROTOCOL_VERSION     "1"
>>>>> +#define XENDISPL_PROTOCOL_VERSION     2
>>>>
>>>> This is not very nice regarding compatibility.
>>>>
>>>> Can't you add a new macro for the integer value?
>>>
>>> We can leave it as is, e.g. define XENDISPL_PROTOCOL_VERSION as "2",
>>>
>>> so we do not break the existing users. Then if every user of the 
>>> header has
>>>
>>> its local copy (this is what we now use in the display backend), then 
>>> that
>>> local copy can be changed in a way suitable for the concrete user, 
>>> e.g. "2"
>>>
>>> redefined to 2. This cannot be done (?) for the Linux Kernel though.
>>>
>>> Or we can have
>>>
>>> #define XENDISPL_PROTOCOL_VERSION     "2"
>>>
>>> #define XENDISPL_PROTOCOL_VERSION_INT  2
>>>
>>> Jurgen, what's your preference here?
>>
>> I would prefer the latter, as it avoids the need to modify the header
>> when copying it to a local project.
>>
> Ok, I'll have 2 definitions then
> 
> Anything else (beside the comments on new functionality) I can add/change
> 
> before sending v2 of the patch?

I would have said so. :-)


Juergen


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] xen/displif: Protocol version 2
  2020-06-30  7:30           ` Jürgen Groß
@ 2020-06-30  7:39             ` Oleksandr Andrushchenko
  2020-06-30  7:57               ` Jürgen Groß
  0 siblings, 1 reply; 13+ messages in thread
From: Oleksandr Andrushchenko @ 2020-06-30  7:39 UTC (permalink / raw)
  To: Jürgen Groß,
	Oleksandr Andrushchenko, xen-devel, ian.jackson, wl, konrad.wilk

On 6/30/20 10:30 AM, Jürgen Groß wrote:
> On 30.06.20 09:09, Oleksandr Andrushchenko wrote:
>> On 6/30/20 10:03 AM, Jürgen Groß wrote:
>>> On 30.06.20 08:13, Oleksandr Andrushchenko wrote:
>>>> On 6/29/20 10:02 AM, Jürgen Groß wrote:
>>>>> On 20.05.20 11:04, Oleksandr Andrushchenko wrote:
>>>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>>>>
>>>>>> 1. Change protocol version from string to integer
>>>>>>
>>>>>> Version string, which is in fact an integer, is hard to handle in the
>>>>>> code that supports different protocol versions. To simplify that
>>>>>> make the version an integer.
>>>>>>
>>>>>> 2. Pass buffer offset with XENDISPL_OP_DBUF_CREATE
>>>>>>
>>>>>> There are cases when display data buffer is created with non-zero
>>>>>> offset to the data start. Handle such cases and provide that offset
>>>>>> while creating a display buffer.
>>>>>>
>>>>>> 3. Add XENDISPL_OP_GET_EDID command
>>>>>>
>>>>>> Add an optional request for reading Extended Display Identification
>>>>>> Data (EDID) structure which allows better configuration of the
>>>>>> display connectors over the configuration set in XenStore.
>>>>>> With this change connectors may have multiple resolutions defined
>>>>>> with respect to detailed timing definitions and additional properties
>>>>>> normally provided by displays.
>>>>>>
>>>>>> If this request is not supported by the backend then visible area
>>>>>> is defined by the relevant XenStore's "resolution" property.
>>>>>>
>>>>>> If backend provides extended display identification data (EDID) with
>>>>>> XENDISPL_OP_GET_EDID request then EDID values must take precedence
>>>>>> over the resolutions defined in XenStore.
>>>>>>
>>>>>> 4. Bump protocol version to 2.
>>>>>>
>>>>>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>>>> ---
>>>>>>    xen/include/public/io/displif.h | 83 +++++++++++++++++++++++++++++++--
>>>>>>    1 file changed, 80 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h
>>>>>> index cc5de9cb1f35..4d43ba5078c8 100644
>>>>>> --- a/xen/include/public/io/displif.h
>>>>>> +++ b/xen/include/public/io/displif.h
>>>>>> @@ -38,7 +38,7 @@
>>>>>>     *                           Protocol version
>>>>>> ******************************************************************************
>>>>>>     */
>>>>>> -#define XENDISPL_PROTOCOL_VERSION     "1"
>>>>>> +#define XENDISPL_PROTOCOL_VERSION     2
>>>>>
>>>>> This is not very nice regarding compatibility.
>>>>>
>>>>> Can't you add a new macro for the integer value?
>>>>
>>>> We can leave it as is, e.g. define XENDISPL_PROTOCOL_VERSION as "2",
>>>>
>>>> so we do not break the existing users. Then if every user of the header has
>>>>
>>>> its local copy (this is what we now use in the display backend), then that
>>>> local copy can be changed in a way suitable for the concrete user, e.g. "2"
>>>>
>>>> redefined to 2. This cannot be done (?) for the Linux Kernel though.
>>>>
>>>> Or we can have
>>>>
>>>> #define XENDISPL_PROTOCOL_VERSION     "2"
>>>>
>>>> #define XENDISPL_PROTOCOL_VERSION_INT  2
>>>>
>>>> Jurgen, what's your preference here?
>>>
>>> I would prefer the latter, as it avoids the need to modify the header
>>> when copying it to a local project.
>>>
>> Ok, I'll have 2 definitions then
>>
>> Anything else (beside the comments on new functionality) I can add/change
>>
>> before sending v2 of the patch?
>
> I would have said so. :-)

Thank you,

the series also has a patch for libgnttab. Do you want me to send the protocol patch

separately or should we wait for Ian and Wei to review? These changes are related,

thus I sent then togheter

>
>
> Juergen

Thank you,

Oleksandr

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] xen/displif: Protocol version 2
  2020-06-30  7:39             ` Oleksandr Andrushchenko
@ 2020-06-30  7:57               ` Jürgen Groß
  0 siblings, 0 replies; 13+ messages in thread
From: Jürgen Groß @ 2020-06-30  7:57 UTC (permalink / raw)
  To: Oleksandr Andrushchenko, Oleksandr Andrushchenko, xen-devel,
	ian.jackson, wl, konrad.wilk

On 30.06.20 09:39, Oleksandr Andrushchenko wrote:
> On 6/30/20 10:30 AM, Jürgen Groß wrote:
>> On 30.06.20 09:09, Oleksandr Andrushchenko wrote:
>>> On 6/30/20 10:03 AM, Jürgen Groß wrote:
>>>> On 30.06.20 08:13, Oleksandr Andrushchenko wrote:
>>>>> On 6/29/20 10:02 AM, Jürgen Groß wrote:
>>>>>> On 20.05.20 11:04, Oleksandr Andrushchenko wrote:
>>>>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>>>>>
>>>>>>> 1. Change protocol version from string to integer
>>>>>>>
>>>>>>> Version string, which is in fact an integer, is hard to handle in the
>>>>>>> code that supports different protocol versions. To simplify that
>>>>>>> make the version an integer.
>>>>>>>
>>>>>>> 2. Pass buffer offset with XENDISPL_OP_DBUF_CREATE
>>>>>>>
>>>>>>> There are cases when display data buffer is created with non-zero
>>>>>>> offset to the data start. Handle such cases and provide that offset
>>>>>>> while creating a display buffer.
>>>>>>>
>>>>>>> 3. Add XENDISPL_OP_GET_EDID command
>>>>>>>
>>>>>>> Add an optional request for reading Extended Display Identification
>>>>>>> Data (EDID) structure which allows better configuration of the
>>>>>>> display connectors over the configuration set in XenStore.
>>>>>>> With this change connectors may have multiple resolutions defined
>>>>>>> with respect to detailed timing definitions and additional properties
>>>>>>> normally provided by displays.
>>>>>>>
>>>>>>> If this request is not supported by the backend then visible area
>>>>>>> is defined by the relevant XenStore's "resolution" property.
>>>>>>>
>>>>>>> If backend provides extended display identification data (EDID) with
>>>>>>> XENDISPL_OP_GET_EDID request then EDID values must take precedence
>>>>>>> over the resolutions defined in XenStore.
>>>>>>>
>>>>>>> 4. Bump protocol version to 2.
>>>>>>>
>>>>>>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>>>>>> ---
>>>>>>>     xen/include/public/io/displif.h | 83 +++++++++++++++++++++++++++++++--
>>>>>>>     1 file changed, 80 insertions(+), 3 deletions(-)
>>>>>>>
>>>>>>> diff --git a/xen/include/public/io/displif.h b/xen/include/public/io/displif.h
>>>>>>> index cc5de9cb1f35..4d43ba5078c8 100644
>>>>>>> --- a/xen/include/public/io/displif.h
>>>>>>> +++ b/xen/include/public/io/displif.h
>>>>>>> @@ -38,7 +38,7 @@
>>>>>>>      *                           Protocol version
>>>>>>> ******************************************************************************
>>>>>>>      */
>>>>>>> -#define XENDISPL_PROTOCOL_VERSION     "1"
>>>>>>> +#define XENDISPL_PROTOCOL_VERSION     2
>>>>>>
>>>>>> This is not very nice regarding compatibility.
>>>>>>
>>>>>> Can't you add a new macro for the integer value?
>>>>>
>>>>> We can leave it as is, e.g. define XENDISPL_PROTOCOL_VERSION as "2",
>>>>>
>>>>> so we do not break the existing users. Then if every user of the header has
>>>>>
>>>>> its local copy (this is what we now use in the display backend), then that
>>>>> local copy can be changed in a way suitable for the concrete user, e.g. "2"
>>>>>
>>>>> redefined to 2. This cannot be done (?) for the Linux Kernel though.
>>>>>
>>>>> Or we can have
>>>>>
>>>>> #define XENDISPL_PROTOCOL_VERSION     "2"
>>>>>
>>>>> #define XENDISPL_PROTOCOL_VERSION_INT  2
>>>>>
>>>>> Jurgen, what's your preference here?
>>>>
>>>> I would prefer the latter, as it avoids the need to modify the header
>>>> when copying it to a local project.
>>>>
>>> Ok, I'll have 2 definitions then
>>>
>>> Anything else (beside the comments on new functionality) I can add/change
>>>
>>> before sending v2 of the patch?
>>
>> I would have said so. :-)
> 
> Thank you,
> 
> the series also has a patch for libgnttab. Do you want me to send the protocol patch
> 
> separately or should we wait for Ian and Wei to review? These changes are related,
> 
> thus I sent then togheter

This is something to ask the tools maintainers IMO.


Juergen



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] libgnttab: Add support for Linux dma-buf offset
  2020-05-20  9:04 ` [PATCH 2/2] libgnttab: Add support for Linux dma-buf offset Oleksandr Andrushchenko
@ 2020-06-30  9:42   ` Oleksandr Andrushchenko
  2020-07-31 10:53   ` Oleksandr Andrushchenko
  1 sibling, 0 replies; 13+ messages in thread
From: Oleksandr Andrushchenko @ 2020-06-30  9:42 UTC (permalink / raw)
  To: Oleksandr Andrushchenko, ian.jackson, wl; +Cc: jgross, xen-devel

Ian, Wei, would you mind looking at the below please?

Thank you in advance,

Oleksandr

On 5/20/20 12:04 PM, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>
> Add version 2 of the dma-buf ioctls which adds data_ofs parameter.
>
> dma-buf is backed by a scatter-gather table and has offset parameter
> which tells where the actual data starts. Relevant ioctls are extended
> to support that offset:
>    - when dma-buf is created (exported) from grant references then
>      data_ofs is used to set the offset field in the scatter list
>      of the new dma-buf
>    - when dma-buf is imported and grant references provided then
>      data_ofs is used to report that offset to user-space
>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> ---
>   tools/include/xen-sys/Linux/gntdev.h  | 53 +++++++++++++++++
>   tools/libs/gnttab/Makefile            |  2 +-
>   tools/libs/gnttab/freebsd.c           | 15 +++++
>   tools/libs/gnttab/gnttab_core.c       | 17 ++++++
>   tools/libs/gnttab/include/xengnttab.h | 13 ++++
>   tools/libs/gnttab/libxengnttab.map    |  6 ++
>   tools/libs/gnttab/linux.c             | 86 +++++++++++++++++++++++++++
>   tools/libs/gnttab/minios.c            | 15 +++++
>   tools/libs/gnttab/private.h           |  9 +++
>   9 files changed, 215 insertions(+), 1 deletion(-)
>
> diff --git a/tools/include/xen-sys/Linux/gntdev.h b/tools/include/xen-sys/Linux/gntdev.h
> index d16076044c71..0c43393cbee5 100644
> --- a/tools/include/xen-sys/Linux/gntdev.h
> +++ b/tools/include/xen-sys/Linux/gntdev.h
> @@ -274,4 +274,57 @@ struct ioctl_gntdev_dmabuf_imp_release {
>       uint32_t reserved;
>   };
>   
> +/*
> + * Version 2 of the ioctls adds @data_ofs parameter.
> + *
> + * dma-buf is backed by a scatter-gather table and has offset
> + * parameter which tells where the actual data starts.
> + * Relevant ioctls are extended to support that offset:
> + *   - when dma-buf is created (exported) from grant references then
> + *     @data_ofs is used to set the offset field in the scatter list
> + *     of the new dma-buf
> + *   - when dma-buf is imported and grant references are provided then
> + *     @data_ofs is used to report that offset to user-space
> + */
> +#define IOCTL_GNTDEV_DMABUF_EXP_FROM_REFS_V2 \
> +    _IOC(_IOC_NONE, 'G', 13, \
> +         sizeof(struct ioctl_gntdev_dmabuf_exp_from_refs_v2))
> +struct ioctl_gntdev_dmabuf_exp_from_refs_v2 {
> +    /* IN parameters. */
> +    /* Specific options for this dma-buf: see GNTDEV_DMA_FLAG_XXX. */
> +    uint32_t flags;
> +    /* Number of grant references in @refs array. */
> +    uint32_t count;
> +    /* Offset of the data in the dma-buf. */
> +    uint32_t data_ofs;
> +    /* OUT parameters. */
> +    /* File descriptor of the dma-buf. */
> +    uint32_t fd;
> +    /* The domain ID of the grant references to be mapped. */
> +    uint32_t domid;
> +    /* Variable IN parameter. */
> +    /* Array of grant references of size @count. */
> +    uint32_t refs[1];
> +};
> +
> +#define IOCTL_GNTDEV_DMABUF_IMP_TO_REFS_V2 \
> +    _IOC(_IOC_NONE, 'G', 14, \
> +         sizeof(struct ioctl_gntdev_dmabuf_imp_to_refs_v2))
> +struct ioctl_gntdev_dmabuf_imp_to_refs_v2 {
> +    /* IN parameters. */
> +    /* File descriptor of the dma-buf. */
> +    uint32_t fd;
> +    /* Number of grant references in @refs array. */
> +    uint32_t count;
> +    /* The domain ID for which references to be granted. */
> +    uint32_t domid;
> +    /* Reserved - must be zero. */
> +    uint32_t reserved;
> +    /* OUT parameters. */
> +    /* Offset of the data in the dma-buf. */
> +    uint32_t data_ofs;
> +    /* Array of grant references of size @count. */
> +    uint32_t refs[1];
> +};
> +
>   #endif /* __LINUX_PUBLIC_GNTDEV_H__ */
> diff --git a/tools/libs/gnttab/Makefile b/tools/libs/gnttab/Makefile
> index 2da8fbbb7f6f..5ee2d965214f 100644
> --- a/tools/libs/gnttab/Makefile
> +++ b/tools/libs/gnttab/Makefile
> @@ -2,7 +2,7 @@ XEN_ROOT = $(CURDIR)/../../..
>   include $(XEN_ROOT)/tools/Rules.mk
>   
>   MAJOR    = 1
> -MINOR    = 2
> +MINOR    = 3
>   LIBNAME  := gnttab
>   USELIBS  := toollog toolcore
>   
> diff --git a/tools/libs/gnttab/freebsd.c b/tools/libs/gnttab/freebsd.c
> index 886b588303a0..baf0f60aa4d3 100644
> --- a/tools/libs/gnttab/freebsd.c
> +++ b/tools/libs/gnttab/freebsd.c
> @@ -319,6 +319,14 @@ int osdep_gnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
>       abort();
>   }
>   
> +int osdep_gnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                         uint32_t flags, uint32_t count,
> +                                         const uint32_t *refs,
> +                                         uint32_t *dmabuf_fd, uint32_t data_ofs)
> +{
> +    abort();
> +}
> +
>   int osdep_gnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt,
>                                             uint32_t fd, uint32_t wait_to_ms)
>   {
> @@ -331,6 +339,13 @@ int osdep_gnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
>       abort();
>   }
>   
> +int osdep_gnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                       uint32_t fd, uint32_t count,
> +                                       uint32_t *refs, uint32_t *data_ofs)
> +{
> +    abort();
> +}
> +
>   int osdep_gnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd)
>   {
>       abort();
> diff --git a/tools/libs/gnttab/gnttab_core.c b/tools/libs/gnttab/gnttab_core.c
> index 92e7228a2671..3af3cec80045 100644
> --- a/tools/libs/gnttab/gnttab_core.c
> +++ b/tools/libs/gnttab/gnttab_core.c
> @@ -144,6 +144,15 @@ int xengnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
>                                                refs, fd);
>   }
>   
> +int xengnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                      uint32_t flags, uint32_t count,
> +                                      const uint32_t *refs, uint32_t *fd,
> +                                      uint32_t data_ofs)
> +{
> +    return osdep_gnttab_dmabuf_exp_from_refs_v2(xgt, domid, flags, count,
> +                                                refs, fd, data_ofs);
> +}
> +
>   int xengnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt, uint32_t fd,
>                                          uint32_t wait_to_ms)
>   {
> @@ -156,6 +165,14 @@ int xengnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
>       return osdep_gnttab_dmabuf_imp_to_refs(xgt, domid, fd, count, refs);
>   }
>   
> +int xengnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                    uint32_t fd, uint32_t count, uint32_t *refs,
> +                                    uint32_t *data_ofs)
> +{
> +    return osdep_gnttab_dmabuf_imp_to_refs_v2(xgt, domid, fd, count, refs,
> +                                              data_ofs);
> +}
> +
>   int xengnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd)
>   {
>       return osdep_gnttab_dmabuf_imp_release(xgt, fd);
> diff --git a/tools/libs/gnttab/include/xengnttab.h b/tools/libs/gnttab/include/xengnttab.h
> index 111fc88caeb3..0956bd91e0df 100644
> --- a/tools/libs/gnttab/include/xengnttab.h
> +++ b/tools/libs/gnttab/include/xengnttab.h
> @@ -322,12 +322,19 @@ int xengnttab_grant_copy(xengnttab_handle *xgt,
>    * Returns 0 if dma-buf was successfully created and the corresponding
>    * dma-buf's file descriptor is returned in @fd.
>    *
> + * Version 2 also accepts @data_ofs offset of the data in the buffer.
> + *
>    * [1] https://urldefense.com/v3/__https://elixir.bootlin.com/linux/latest/source/Documentation/driver-api/dma-buf.rst__;!!GF_29dbcQIUBPA!nBABkPpEyQW1_5nPE9nbyCbEaCvRjXQxOBKRpRSIGUgAdqcc0VCm4jL-9cCabcWNDS4bc_DR6Q$
>    */
>   int xengnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
>                                      uint32_t flags, uint32_t count,
>                                      const uint32_t *refs, uint32_t *fd);
>   
> +int xengnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                      uint32_t flags, uint32_t count,
> +                                      const uint32_t *refs, uint32_t *fd,
> +                                      uint32_t data_ofs);
> +
>   /*
>    * This will block until the dma-buf with the file descriptor @fd is
>    * released. This is only valid for buffers created with
> @@ -345,10 +352,16 @@ int xengnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt, uint32_t fd,
>   /*
>    * Import a dma-buf with file descriptor @fd and export granted references
>    * to the pages of that dma-buf into array @refs of size @count.
> + *
> + * Version 2 also provides @data_ofs offset of the data in the buffer.
>    */
>   int xengnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
>                                    uint32_t fd, uint32_t count, uint32_t *refs);
>   
> +int xengnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                    uint32_t fd, uint32_t count, uint32_t *refs,
> +                                    uint32_t *data_ofs);
> +
>   /*
>    * This will close all references to an imported buffer, so it can be
>    * released by the owner. This is only valid for buffers created with
> diff --git a/tools/libs/gnttab/libxengnttab.map b/tools/libs/gnttab/libxengnttab.map
> index d2a9b7e18bea..ddf77e064b08 100644
> --- a/tools/libs/gnttab/libxengnttab.map
> +++ b/tools/libs/gnttab/libxengnttab.map
> @@ -36,3 +36,9 @@ VERS_1.2 {
>   		xengnttab_dmabuf_imp_to_refs;
>   		xengnttab_dmabuf_imp_release;
>   } VERS_1.1;
> +
> +VERS_1.3 {
> +    global:
> +		xengnttab_dmabuf_exp_from_refs_v2;
> +		xengnttab_dmabuf_imp_to_refs_v2;
> +} VERS_1.2;
> diff --git a/tools/libs/gnttab/linux.c b/tools/libs/gnttab/linux.c
> index a01bb6c698c6..75e249fb3202 100644
> --- a/tools/libs/gnttab/linux.c
> +++ b/tools/libs/gnttab/linux.c
> @@ -352,6 +352,51 @@ out:
>       return rc;
>   }
>   
> +int osdep_gnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                         uint32_t flags, uint32_t count,
> +                                         const uint32_t *refs,
> +                                         uint32_t *dmabuf_fd,
> +                                         uint32_t data_ofs)
> +{
> +    struct ioctl_gntdev_dmabuf_exp_from_refs_v2 *from_refs_v2 = NULL;
> +    int rc = -1;
> +
> +    if ( !count )
> +    {
> +        errno = EINVAL;
> +        goto out;
> +    }
> +
> +    from_refs_v2 = malloc(sizeof(*from_refs_v2) +
> +                          (count - 1) * sizeof(from_refs_v2->refs[0]));
> +    if ( !from_refs_v2 )
> +    {
> +        errno = ENOMEM;
> +        goto out;
> +    }
> +
> +    from_refs_v2->flags = flags;
> +    from_refs_v2->count = count;
> +    from_refs_v2->domid = domid;
> +    from_refs_v2->data_ofs = data_ofs;
> +
> +    memcpy(from_refs_v2->refs, refs, count * sizeof(from_refs_v2->refs[0]));
> +
> +    if ( (rc = ioctl(xgt->fd, IOCTL_GNTDEV_DMABUF_EXP_FROM_REFS_V2,
> +                     from_refs_v2)) )
> +    {
> +        GTERROR(xgt->logger, "ioctl DMABUF_EXP_FROM_REFS_V2 failed");
> +        goto out;
> +    }
> +
> +    *dmabuf_fd = from_refs_v2->fd;
> +    rc = 0;
> +
> +out:
> +    free(from_refs_v2);
> +    return rc;
> +}
> +
>   int osdep_gnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt,
>                                             uint32_t fd, uint32_t wait_to_ms)
>   {
> @@ -413,6 +458,47 @@ out:
>       return rc;
>   }
>   
> +int osdep_gnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                       uint32_t fd, uint32_t count,
> +                                       uint32_t *refs,
> +                                       uint32_t *data_ofs)
> +{
> +    struct ioctl_gntdev_dmabuf_imp_to_refs_v2 *to_refs_v2 = NULL;
> +    int rc = -1;
> +
> +    if ( !count )
> +    {
> +        errno = EINVAL;
> +        goto out;
> +    }
> +
> +    to_refs_v2 = malloc(sizeof(*to_refs_v2) +
> +                        (count - 1) * sizeof(to_refs_v2->refs[0]));
> +    if ( !to_refs_v2 )
> +    {
> +        errno = ENOMEM;
> +        goto out;
> +    }
> +
> +    to_refs_v2->fd = fd;
> +    to_refs_v2->count = count;
> +    to_refs_v2->domid = domid;
> +
> +    if ( (rc = ioctl(xgt->fd, IOCTL_GNTDEV_DMABUF_IMP_TO_REFS_V2, to_refs_v2)) )
> +    {
> +        GTERROR(xgt->logger, "ioctl DMABUF_IMP_TO_REFS_V2 failed");
> +        goto out;
> +    }
> +
> +    memcpy(refs, to_refs_v2->refs, count * sizeof(*refs));
> +    *data_ofs = to_refs_v2->data_ofs;
> +    rc = 0;
> +
> +out:
> +    free(to_refs_v2);
> +    return rc;
> +}
> +
>   int osdep_gnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd)
>   {
>       struct ioctl_gntdev_dmabuf_imp_release release;
> diff --git a/tools/libs/gnttab/minios.c b/tools/libs/gnttab/minios.c
> index f78caadd3043..298416b2a98d 100644
> --- a/tools/libs/gnttab/minios.c
> +++ b/tools/libs/gnttab/minios.c
> @@ -120,6 +120,14 @@ int osdep_gnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
>       return -1;
>   }
>   
> +int osdep_gnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                         uint32_t flags, uint32_t count,
> +                                         const uint32_t *refs, uint32_t *fd,
> +                                         uint32_t data_ofs)
> +{
> +    return -1;
> +}
> +
>   int osdep_gnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt,
>                                             uint32_t fd, uint32_t wait_to_ms)
>   {
> @@ -133,6 +141,13 @@ int osdep_gnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
>       return -1;
>   }
>   
> +int osdep_gnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                       uint32_t fd, uint32_t count,
> +                                       uint32_t *refs, uint32_t *data_ofs)
> +{
> +    return -1;
> +}
> +
>   int osdep_gnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd)
>   {
>       return -1;
> diff --git a/tools/libs/gnttab/private.h b/tools/libs/gnttab/private.h
> index c5e23639b141..07271637f609 100644
> --- a/tools/libs/gnttab/private.h
> +++ b/tools/libs/gnttab/private.h
> @@ -39,6 +39,11 @@ int osdep_gnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
>                                         uint32_t flags, uint32_t count,
>                                         const uint32_t *refs, uint32_t *fd);
>   
> +int osdep_gnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                         uint32_t flags, uint32_t count,
> +                                         const uint32_t *refs, uint32_t *fd,
> +                                         uint32_t data_ofs);
> +
>   int osdep_gnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt,
>                                             uint32_t fd, uint32_t wait_to_ms);
>   
> @@ -46,6 +51,10 @@ int osdep_gnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
>                                       uint32_t fd, uint32_t count,
>                                       uint32_t *refs);
>   
> +int osdep_gnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                       uint32_t fd, uint32_t count,
> +                                       uint32_t *refs, uint32_t *data_ofs);
> +
>   int osdep_gnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd);
>   
>   int osdep_gntshr_open(xengntshr_handle *xgs);

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] libgnttab: Add support for Linux dma-buf offset
  2020-05-20  9:04 ` [PATCH 2/2] libgnttab: Add support for Linux dma-buf offset Oleksandr Andrushchenko
  2020-06-30  9:42   ` Oleksandr Andrushchenko
@ 2020-07-31 10:53   ` Oleksandr Andrushchenko
  1 sibling, 0 replies; 13+ messages in thread
From: Oleksandr Andrushchenko @ 2020-07-31 10:53 UTC (permalink / raw)
  To: xen-devel, ian.jackson, wl; +Cc: Oleksandr Andrushchenko, Artem Mygaiev

Hello, Ian, Wei!

Initially I have sent this patch as a part of the series for the display protocol changes,

but later decided to split. At the moment the protocol changes are already

accepted, but this part is still missing feedback and is still wanted.

I would really appreciate if you could have a look at the change below.

Thank you in advance,

Oleksandr Andrushchenko

On 5/20/20 12:04 PM, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>
> Add version 2 of the dma-buf ioctls which adds data_ofs parameter.
>
> dma-buf is backed by a scatter-gather table and has offset parameter
> which tells where the actual data starts. Relevant ioctls are extended
> to support that offset:
>    - when dma-buf is created (exported) from grant references then
>      data_ofs is used to set the offset field in the scatter list
>      of the new dma-buf
>    - when dma-buf is imported and grant references provided then
>      data_ofs is used to report that offset to user-space
>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> ---
>   tools/include/xen-sys/Linux/gntdev.h  | 53 +++++++++++++++++
>   tools/libs/gnttab/Makefile            |  2 +-
>   tools/libs/gnttab/freebsd.c           | 15 +++++
>   tools/libs/gnttab/gnttab_core.c       | 17 ++++++
>   tools/libs/gnttab/include/xengnttab.h | 13 ++++
>   tools/libs/gnttab/libxengnttab.map    |  6 ++
>   tools/libs/gnttab/linux.c             | 86 +++++++++++++++++++++++++++
>   tools/libs/gnttab/minios.c            | 15 +++++
>   tools/libs/gnttab/private.h           |  9 +++
>   9 files changed, 215 insertions(+), 1 deletion(-)
>
> diff --git a/tools/include/xen-sys/Linux/gntdev.h b/tools/include/xen-sys/Linux/gntdev.h
> index d16076044c71..0c43393cbee5 100644
> --- a/tools/include/xen-sys/Linux/gntdev.h
> +++ b/tools/include/xen-sys/Linux/gntdev.h
> @@ -274,4 +274,57 @@ struct ioctl_gntdev_dmabuf_imp_release {
>       uint32_t reserved;
>   };
>   
> +/*
> + * Version 2 of the ioctls adds @data_ofs parameter.
> + *
> + * dma-buf is backed by a scatter-gather table and has offset
> + * parameter which tells where the actual data starts.
> + * Relevant ioctls are extended to support that offset:
> + *   - when dma-buf is created (exported) from grant references then
> + *     @data_ofs is used to set the offset field in the scatter list
> + *     of the new dma-buf
> + *   - when dma-buf is imported and grant references are provided then
> + *     @data_ofs is used to report that offset to user-space
> + */
> +#define IOCTL_GNTDEV_DMABUF_EXP_FROM_REFS_V2 \
> +    _IOC(_IOC_NONE, 'G', 13, \
> +         sizeof(struct ioctl_gntdev_dmabuf_exp_from_refs_v2))
> +struct ioctl_gntdev_dmabuf_exp_from_refs_v2 {
> +    /* IN parameters. */
> +    /* Specific options for this dma-buf: see GNTDEV_DMA_FLAG_XXX. */
> +    uint32_t flags;
> +    /* Number of grant references in @refs array. */
> +    uint32_t count;
> +    /* Offset of the data in the dma-buf. */
> +    uint32_t data_ofs;
> +    /* OUT parameters. */
> +    /* File descriptor of the dma-buf. */
> +    uint32_t fd;
> +    /* The domain ID of the grant references to be mapped. */
> +    uint32_t domid;
> +    /* Variable IN parameter. */
> +    /* Array of grant references of size @count. */
> +    uint32_t refs[1];
> +};
> +
> +#define IOCTL_GNTDEV_DMABUF_IMP_TO_REFS_V2 \
> +    _IOC(_IOC_NONE, 'G', 14, \
> +         sizeof(struct ioctl_gntdev_dmabuf_imp_to_refs_v2))
> +struct ioctl_gntdev_dmabuf_imp_to_refs_v2 {
> +    /* IN parameters. */
> +    /* File descriptor of the dma-buf. */
> +    uint32_t fd;
> +    /* Number of grant references in @refs array. */
> +    uint32_t count;
> +    /* The domain ID for which references to be granted. */
> +    uint32_t domid;
> +    /* Reserved - must be zero. */
> +    uint32_t reserved;
> +    /* OUT parameters. */
> +    /* Offset of the data in the dma-buf. */
> +    uint32_t data_ofs;
> +    /* Array of grant references of size @count. */
> +    uint32_t refs[1];
> +};
> +
>   #endif /* __LINUX_PUBLIC_GNTDEV_H__ */
> diff --git a/tools/libs/gnttab/Makefile b/tools/libs/gnttab/Makefile
> index 2da8fbbb7f6f..5ee2d965214f 100644
> --- a/tools/libs/gnttab/Makefile
> +++ b/tools/libs/gnttab/Makefile
> @@ -2,7 +2,7 @@ XEN_ROOT = $(CURDIR)/../../..
>   include $(XEN_ROOT)/tools/Rules.mk
>   
>   MAJOR    = 1
> -MINOR    = 2
> +MINOR    = 3
>   LIBNAME  := gnttab
>   USELIBS  := toollog toolcore
>   
> diff --git a/tools/libs/gnttab/freebsd.c b/tools/libs/gnttab/freebsd.c
> index 886b588303a0..baf0f60aa4d3 100644
> --- a/tools/libs/gnttab/freebsd.c
> +++ b/tools/libs/gnttab/freebsd.c
> @@ -319,6 +319,14 @@ int osdep_gnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
>       abort();
>   }
>   
> +int osdep_gnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                         uint32_t flags, uint32_t count,
> +                                         const uint32_t *refs,
> +                                         uint32_t *dmabuf_fd, uint32_t data_ofs)
> +{
> +    abort();
> +}
> +
>   int osdep_gnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt,
>                                             uint32_t fd, uint32_t wait_to_ms)
>   {
> @@ -331,6 +339,13 @@ int osdep_gnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
>       abort();
>   }
>   
> +int osdep_gnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                       uint32_t fd, uint32_t count,
> +                                       uint32_t *refs, uint32_t *data_ofs)
> +{
> +    abort();
> +}
> +
>   int osdep_gnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd)
>   {
>       abort();
> diff --git a/tools/libs/gnttab/gnttab_core.c b/tools/libs/gnttab/gnttab_core.c
> index 92e7228a2671..3af3cec80045 100644
> --- a/tools/libs/gnttab/gnttab_core.c
> +++ b/tools/libs/gnttab/gnttab_core.c
> @@ -144,6 +144,15 @@ int xengnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
>                                                refs, fd);
>   }
>   
> +int xengnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                      uint32_t flags, uint32_t count,
> +                                      const uint32_t *refs, uint32_t *fd,
> +                                      uint32_t data_ofs)
> +{
> +    return osdep_gnttab_dmabuf_exp_from_refs_v2(xgt, domid, flags, count,
> +                                                refs, fd, data_ofs);
> +}
> +
>   int xengnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt, uint32_t fd,
>                                          uint32_t wait_to_ms)
>   {
> @@ -156,6 +165,14 @@ int xengnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
>       return osdep_gnttab_dmabuf_imp_to_refs(xgt, domid, fd, count, refs);
>   }
>   
> +int xengnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                    uint32_t fd, uint32_t count, uint32_t *refs,
> +                                    uint32_t *data_ofs)
> +{
> +    return osdep_gnttab_dmabuf_imp_to_refs_v2(xgt, domid, fd, count, refs,
> +                                              data_ofs);
> +}
> +
>   int xengnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd)
>   {
>       return osdep_gnttab_dmabuf_imp_release(xgt, fd);
> diff --git a/tools/libs/gnttab/include/xengnttab.h b/tools/libs/gnttab/include/xengnttab.h
> index 111fc88caeb3..0956bd91e0df 100644
> --- a/tools/libs/gnttab/include/xengnttab.h
> +++ b/tools/libs/gnttab/include/xengnttab.h
> @@ -322,12 +322,19 @@ int xengnttab_grant_copy(xengnttab_handle *xgt,
>    * Returns 0 if dma-buf was successfully created and the corresponding
>    * dma-buf's file descriptor is returned in @fd.
>    *
> + * Version 2 also accepts @data_ofs offset of the data in the buffer.
> + *
>    * [1] https://elixir.bootlin.com/linux/latest/source/Documentation/driver-api/dma-buf.rst
>    */
>   int xengnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
>                                      uint32_t flags, uint32_t count,
>                                      const uint32_t *refs, uint32_t *fd);
>   
> +int xengnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                      uint32_t flags, uint32_t count,
> +                                      const uint32_t *refs, uint32_t *fd,
> +                                      uint32_t data_ofs);
> +
>   /*
>    * This will block until the dma-buf with the file descriptor @fd is
>    * released. This is only valid for buffers created with
> @@ -345,10 +352,16 @@ int xengnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt, uint32_t fd,
>   /*
>    * Import a dma-buf with file descriptor @fd and export granted references
>    * to the pages of that dma-buf into array @refs of size @count.
> + *
> + * Version 2 also provides @data_ofs offset of the data in the buffer.
>    */
>   int xengnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
>                                    uint32_t fd, uint32_t count, uint32_t *refs);
>   
> +int xengnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                    uint32_t fd, uint32_t count, uint32_t *refs,
> +                                    uint32_t *data_ofs);
> +
>   /*
>    * This will close all references to an imported buffer, so it can be
>    * released by the owner. This is only valid for buffers created with
> diff --git a/tools/libs/gnttab/libxengnttab.map b/tools/libs/gnttab/libxengnttab.map
> index d2a9b7e18bea..ddf77e064b08 100644
> --- a/tools/libs/gnttab/libxengnttab.map
> +++ b/tools/libs/gnttab/libxengnttab.map
> @@ -36,3 +36,9 @@ VERS_1.2 {
>   		xengnttab_dmabuf_imp_to_refs;
>   		xengnttab_dmabuf_imp_release;
>   } VERS_1.1;
> +
> +VERS_1.3 {
> +    global:
> +		xengnttab_dmabuf_exp_from_refs_v2;
> +		xengnttab_dmabuf_imp_to_refs_v2;
> +} VERS_1.2;
> diff --git a/tools/libs/gnttab/linux.c b/tools/libs/gnttab/linux.c
> index a01bb6c698c6..75e249fb3202 100644
> --- a/tools/libs/gnttab/linux.c
> +++ b/tools/libs/gnttab/linux.c
> @@ -352,6 +352,51 @@ out:
>       return rc;
>   }
>   
> +int osdep_gnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                         uint32_t flags, uint32_t count,
> +                                         const uint32_t *refs,
> +                                         uint32_t *dmabuf_fd,
> +                                         uint32_t data_ofs)
> +{
> +    struct ioctl_gntdev_dmabuf_exp_from_refs_v2 *from_refs_v2 = NULL;
> +    int rc = -1;
> +
> +    if ( !count )
> +    {
> +        errno = EINVAL;
> +        goto out;
> +    }
> +
> +    from_refs_v2 = malloc(sizeof(*from_refs_v2) +
> +                          (count - 1) * sizeof(from_refs_v2->refs[0]));
> +    if ( !from_refs_v2 )
> +    {
> +        errno = ENOMEM;
> +        goto out;
> +    }
> +
> +    from_refs_v2->flags = flags;
> +    from_refs_v2->count = count;
> +    from_refs_v2->domid = domid;
> +    from_refs_v2->data_ofs = data_ofs;
> +
> +    memcpy(from_refs_v2->refs, refs, count * sizeof(from_refs_v2->refs[0]));
> +
> +    if ( (rc = ioctl(xgt->fd, IOCTL_GNTDEV_DMABUF_EXP_FROM_REFS_V2,
> +                     from_refs_v2)) )
> +    {
> +        GTERROR(xgt->logger, "ioctl DMABUF_EXP_FROM_REFS_V2 failed");
> +        goto out;
> +    }
> +
> +    *dmabuf_fd = from_refs_v2->fd;
> +    rc = 0;
> +
> +out:
> +    free(from_refs_v2);
> +    return rc;
> +}
> +
>   int osdep_gnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt,
>                                             uint32_t fd, uint32_t wait_to_ms)
>   {
> @@ -413,6 +458,47 @@ out:
>       return rc;
>   }
>   
> +int osdep_gnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                       uint32_t fd, uint32_t count,
> +                                       uint32_t *refs,
> +                                       uint32_t *data_ofs)
> +{
> +    struct ioctl_gntdev_dmabuf_imp_to_refs_v2 *to_refs_v2 = NULL;
> +    int rc = -1;
> +
> +    if ( !count )
> +    {
> +        errno = EINVAL;
> +        goto out;
> +    }
> +
> +    to_refs_v2 = malloc(sizeof(*to_refs_v2) +
> +                        (count - 1) * sizeof(to_refs_v2->refs[0]));
> +    if ( !to_refs_v2 )
> +    {
> +        errno = ENOMEM;
> +        goto out;
> +    }
> +
> +    to_refs_v2->fd = fd;
> +    to_refs_v2->count = count;
> +    to_refs_v2->domid = domid;
> +
> +    if ( (rc = ioctl(xgt->fd, IOCTL_GNTDEV_DMABUF_IMP_TO_REFS_V2, to_refs_v2)) )
> +    {
> +        GTERROR(xgt->logger, "ioctl DMABUF_IMP_TO_REFS_V2 failed");
> +        goto out;
> +    }
> +
> +    memcpy(refs, to_refs_v2->refs, count * sizeof(*refs));
> +    *data_ofs = to_refs_v2->data_ofs;
> +    rc = 0;
> +
> +out:
> +    free(to_refs_v2);
> +    return rc;
> +}
> +
>   int osdep_gnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd)
>   {
>       struct ioctl_gntdev_dmabuf_imp_release release;
> diff --git a/tools/libs/gnttab/minios.c b/tools/libs/gnttab/minios.c
> index f78caadd3043..298416b2a98d 100644
> --- a/tools/libs/gnttab/minios.c
> +++ b/tools/libs/gnttab/minios.c
> @@ -120,6 +120,14 @@ int osdep_gnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
>       return -1;
>   }
>   
> +int osdep_gnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                         uint32_t flags, uint32_t count,
> +                                         const uint32_t *refs, uint32_t *fd,
> +                                         uint32_t data_ofs)
> +{
> +    return -1;
> +}
> +
>   int osdep_gnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt,
>                                             uint32_t fd, uint32_t wait_to_ms)
>   {
> @@ -133,6 +141,13 @@ int osdep_gnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
>       return -1;
>   }
>   
> +int osdep_gnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                       uint32_t fd, uint32_t count,
> +                                       uint32_t *refs, uint32_t *data_ofs)
> +{
> +    return -1;
> +}
> +
>   int osdep_gnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd)
>   {
>       return -1;
> diff --git a/tools/libs/gnttab/private.h b/tools/libs/gnttab/private.h
> index c5e23639b141..07271637f609 100644
> --- a/tools/libs/gnttab/private.h
> +++ b/tools/libs/gnttab/private.h
> @@ -39,6 +39,11 @@ int osdep_gnttab_dmabuf_exp_from_refs(xengnttab_handle *xgt, uint32_t domid,
>                                         uint32_t flags, uint32_t count,
>                                         const uint32_t *refs, uint32_t *fd);
>   
> +int osdep_gnttab_dmabuf_exp_from_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                         uint32_t flags, uint32_t count,
> +                                         const uint32_t *refs, uint32_t *fd,
> +                                         uint32_t data_ofs);
> +
>   int osdep_gnttab_dmabuf_exp_wait_released(xengnttab_handle *xgt,
>                                             uint32_t fd, uint32_t wait_to_ms);
>   
> @@ -46,6 +51,10 @@ int osdep_gnttab_dmabuf_imp_to_refs(xengnttab_handle *xgt, uint32_t domid,
>                                       uint32_t fd, uint32_t count,
>                                       uint32_t *refs);
>   
> +int osdep_gnttab_dmabuf_imp_to_refs_v2(xengnttab_handle *xgt, uint32_t domid,
> +                                       uint32_t fd, uint32_t count,
> +                                       uint32_t *refs, uint32_t *data_ofs);
> +
>   int osdep_gnttab_dmabuf_imp_release(xengnttab_handle *xgt, uint32_t fd);
>   
>   int osdep_gntshr_open(xengntshr_handle *xgs);

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, back to index

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-20  9:04 [PATCH 0/2] displif: Protocol version 2 Oleksandr Andrushchenko
2020-05-20  9:04 ` [PATCH 1/2] xen/displif: " Oleksandr Andrushchenko
2020-06-29  7:02   ` Jürgen Groß
2020-06-30  6:13     ` Oleksandr Andrushchenko
2020-06-30  7:03       ` Jürgen Groß
2020-06-30  7:09         ` Oleksandr Andrushchenko
2020-06-30  7:30           ` Jürgen Groß
2020-06-30  7:39             ` Oleksandr Andrushchenko
2020-06-30  7:57               ` Jürgen Groß
2020-05-20  9:04 ` [PATCH 2/2] libgnttab: Add support for Linux dma-buf offset Oleksandr Andrushchenko
2020-06-30  9:42   ` Oleksandr Andrushchenko
2020-07-31 10:53   ` Oleksandr Andrushchenko
2020-06-01 16:01 ` [PATCH 0/2] displif: Protocol version 2 Oleksandr Andrushchenko

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git