[PATCH v3 0/3] tools/libs: decouple more from mini-os

[PATCH v3 0/3] tools/libs: decouple more from mini-os

[Juergen Gross]

This small series removes some hard coupling of the Xen build with some
Mini-OS internals, especially the struct file layout and the internal
organization of the file handling.

This series depends on the Mini-OS series posted recently:

https://lists.xen.org/archives/html/xen-devel/2022-01/threads.html#00635

The main idea is to no longer have Xen library specific structures
inside struct file, or to let struct file layout depend on the
configuration of Mini-OS.

All Xen libraries needing a hook in struct file should use the now
available generic dev pointer and allocate the needed data dynamically.

Additionally Xen libraries should get the pointer of struct file via
the new get_file_from_fd() function instead of accessing directly the
files[] array, which might go away in future (e.g. in order to support
dynamic allocation of struct file as needed).

Via using alloc_file_type() the libs provide a function vector in
order to enable Mini-OS to remove the dedicated callbacks into the
libs via known function names and replacing them to use the new
vector.

Changes in V3:
- comments addressed

Changes in V2:
- comments addressed
- add using alloc_file_type()
- new patch 3

Juergen Gross (3):
  tools/libs/evtchn: decouple more from mini-os
  tools/libs/gnttab: decouple more from mini-os
  tools/libs/ctrl: remove file related handling

 tools/libs/ctrl/xc_minios.c |   9 ---
 tools/libs/evtchn/minios.c  | 143 ++++++++++++++++++++++++------------
 tools/libs/gnttab/minios.c  |  67 +++++++++++++----
 3 files changed, 148 insertions(+), 71 deletions(-)

-- 
2.26.2

[PATCH v3 1/3] tools/libs/evtchn: decouple more from mini-os

[Juergen Gross]

Mini-OS and libevtchn are using implementation details of each other.
Change that by letting libevtchn use the new alloc_file_type() and
get_file_from_fd() function and the generic dev pointer of struct file
from Mini-OS.

By using private struct declarations Mini-OS will be able to drop the
libevtchn specific definitions of struct evtchn_port_info and
evtchn_port_list in future. While at it use bool for "pending" and
"bound".

Switch to use xce as function parameter instead of fd where possible.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- use xce instead of fd as parameter internally (Andrew Cooper)
- add alloc_file_type() support
V3:
- switch callback to use struct file * as a parameter (Andrew Cooper)
- use __attribute__((constructor)) (Andrew Cooper)
---
 tools/libs/evtchn/minios.c | 143 +++++++++++++++++++++++++------------
 1 file changed, 96 insertions(+), 47 deletions(-)

diff --git a/tools/libs/evtchn/minios.c b/tools/libs/evtchn/minios.c
index 8f84048b11..ee3da9f8fe 100644
--- a/tools/libs/evtchn/minios.c
+++ b/tools/libs/evtchn/minios.c
@@ -38,29 +38,40 @@
 
 #include "private.h"
 
-extern void minios_evtchn_close_fd(int fd);
+LIST_HEAD(port_list, port_info);
+
+struct port_info {
+    LIST_ENTRY(port_info) list;
+    evtchn_port_t port;
+    bool pending;
+    bool bound;
+};
 
 extern struct wait_queue_head event_queue;
 
+void minios_evtchn_close_fd(int fd);
+
 /* XXX Note: This is not threadsafe */
-static struct evtchn_port_info *port_alloc(int fd)
+static struct port_info *port_alloc(xenevtchn_handle *xce)
 {
-    struct evtchn_port_info *port_info;
+    struct port_info *port_info;
+    struct file *file = get_file_from_fd(xce->fd);
+    struct port_list *port_list = file->dev;
 
-    port_info = malloc(sizeof(struct evtchn_port_info));
+    port_info = malloc(sizeof(struct port_info));
     if ( port_info == NULL )
         return NULL;
 
-    port_info->pending = 0;
+    port_info->pending = false;
     port_info->port = -1;
-    port_info->bound = 0;
+    port_info->bound = false;
 
-    LIST_INSERT_HEAD(&files[fd].evtchn.ports, port_info, list);
+    LIST_INSERT_HEAD(port_list, port_info, list);
 
     return port_info;
 }
 
-static void port_dealloc(struct evtchn_port_info *port_info)
+static void port_dealloc(struct port_info *port_info)
 {
     if ( port_info->bound )
         unbind_evtchn(port_info->port);
@@ -69,18 +80,57 @@ static void port_dealloc(struct evtchn_port_info *port_info)
     free(port_info);
 }
 
+static int evtchn_close_fd(struct file *file)
+{
+    struct port_info *port_info, *tmp;
+    struct port_list *port_list = file->dev;
+
+    LIST_FOREACH_SAFE(port_info, port_list, list, tmp)
+        port_dealloc(port_info);
+    free(port_list);
+
+    return 0;
+}
+
+static const struct file_ops evtchn_ops = {
+    .name = "evtchn",
+    .close = evtchn_close_fd,
+    .select_rd = select_read_flag,
+};
+
+static unsigned int ftype_evtchn;
+
+__attribute__((constructor))
+static void evtchn_initialize(void)
+{
+    ftype_evtchn = alloc_file_type(&evtchn_ops);
+}
+
 /*
  * XENEVTCHN_NO_CLOEXEC is being ignored, as there is no exec() call supported
  * in Mini-OS.
  */
 int osdep_evtchn_open(xenevtchn_handle *xce, unsigned int flags)
 {
-    int fd = alloc_fd(FTYPE_EVTCHN);
+    int fd;
+    struct file *file;
+    struct port_list *list;
 
-    if ( fd == -1 )
+    list = malloc(sizeof(*list));
+    if ( !list )
         return -1;
 
-    LIST_INIT(&files[fd].evtchn.ports);
+    fd = alloc_fd(ftype_evtchn);
+    file = get_file_from_fd(fd);
+
+    if ( !file )
+    {
+        free(list);
+        return -1;
+    }
+
+    file->dev = list;
+    LIST_INIT(list);
     xce->fd = fd;
     printf("evtchn_open() -> %d\n", fd);
 
@@ -104,12 +154,9 @@ int osdep_evtchn_restrict(xenevtchn_handle *xce, domid_t domid)
 
 void minios_evtchn_close_fd(int fd)
 {
-    struct evtchn_port_info *port_info, *tmp;
+    struct file *file = get_file_from_fd(fd);
 
-    LIST_FOREACH_SAFE(port_info, &files[fd].evtchn.ports, list, tmp)
-        port_dealloc(port_info);
-
-    files[fd].type = FTYPE_NONE;
+    evtchn_close_fd(file);
 }
 
 int xenevtchn_notify(xenevtchn_handle *xce, evtchn_port_t port)
@@ -129,42 +176,43 @@ int xenevtchn_notify(xenevtchn_handle *xce, evtchn_port_t port)
 
 static void evtchn_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
 {
-    int fd = (int)(intptr_t)data;
-    struct evtchn_port_info *port_info;
+    xenevtchn_handle *xce = data;
+    struct file *file = get_file_from_fd(xce->fd);
+    struct port_info *port_info;
+    struct port_list *port_list;
 
-    assert(files[fd].type == FTYPE_EVTCHN);
+    assert(file);
+    port_list = file->dev;
     mask_evtchn(port);
-    LIST_FOREACH(port_info, &files[fd].evtchn.ports, list)
+    LIST_FOREACH(port_info, port_list, list)
     {
         if ( port_info->port == port )
             goto found;
     }
 
-    printk("Unknown port for handle %d\n", fd);
+    printk("Unknown port %d for handle %d\n", port, xce->fd);
     return;
 
  found:
-    port_info->pending = 1;
-    files[fd].read = 1;
+    port_info->pending = true;
+    file->read = true;
     wake_up(&event_queue);
 }
 
 xenevtchn_port_or_error_t xenevtchn_bind_unbound_port(xenevtchn_handle *xce,
                                                       uint32_t domid)
 {
-    int fd = xce->fd;
-    struct evtchn_port_info *port_info;
+    struct port_info *port_info;
     int ret;
     evtchn_port_t port;
 
     assert(get_current() == main_thread);
-    port_info = port_alloc(fd);
+    port_info = port_alloc(xce);
     if ( port_info == NULL )
         return -1;
 
     printf("xenevtchn_bind_unbound_port(%d)", domid);
-    ret = evtchn_alloc_unbound(domid, evtchn_handler,
-                               (void *)(intptr_t)fd, &port);
+    ret = evtchn_alloc_unbound(domid, evtchn_handler, xce, &port);
     printf(" = %d\n", ret);
 
     if ( ret < 0 )
@@ -174,7 +222,7 @@ xenevtchn_port_or_error_t xenevtchn_bind_unbound_port(xenevtchn_handle *xce,
         return -1;
     }
 
-    port_info->bound = 1;
+    port_info->bound = true;
     port_info->port = port;
     unmask_evtchn(port);
 
@@ -185,19 +233,18 @@ xenevtchn_port_or_error_t xenevtchn_bind_interdomain(xenevtchn_handle *xce,
                                                      uint32_t domid,
                                                      evtchn_port_t remote_port)
 {
-    int fd = xce->fd;
-    struct evtchn_port_info *port_info;
+    struct port_info *port_info;
     evtchn_port_t local_port;
     int ret;
 
     assert(get_current() == main_thread);
-    port_info = port_alloc(fd);
+    port_info = port_alloc(xce);
     if ( port_info == NULL )
         return -1;
 
     printf("xenevtchn_bind_interdomain(%d, %"PRId32")", domid, remote_port);
     ret = evtchn_bind_interdomain(domid, remote_port, evtchn_handler,
-                                  (void *)(intptr_t)fd, &local_port);
+                                  xce, &local_port);
     printf(" = %d\n", ret);
 
     if ( ret < 0 )
@@ -207,7 +254,7 @@ xenevtchn_port_or_error_t xenevtchn_bind_interdomain(xenevtchn_handle *xce,
         return -1;
     }
 
-    port_info->bound = 1;
+    port_info->bound = true;
     port_info->port = local_port;
     unmask_evtchn(local_port);
 
@@ -217,9 +264,11 @@ xenevtchn_port_or_error_t xenevtchn_bind_interdomain(xenevtchn_handle *xce,
 int xenevtchn_unbind(xenevtchn_handle *xce, evtchn_port_t port)
 {
     int fd = xce->fd;
-    struct evtchn_port_info *port_info;
+    struct file *file = get_file_from_fd(fd);
+    struct port_info *port_info;
+    struct port_list *port_list = file->dev;
 
-    LIST_FOREACH(port_info, &files[fd].evtchn.ports, list)
+    LIST_FOREACH(port_info, port_list, list)
     {
         if ( port_info->port == port )
         {
@@ -238,17 +287,16 @@ int xenevtchn_unbind(xenevtchn_handle *xce, evtchn_port_t port)
 xenevtchn_port_or_error_t xenevtchn_bind_virq(xenevtchn_handle *xce,
                                               unsigned int virq)
 {
-    int fd = xce->fd;
-    struct evtchn_port_info *port_info;
+    struct port_info *port_info;
     evtchn_port_t port;
 
     assert(get_current() == main_thread);
-    port_info = port_alloc(fd);
+    port_info = port_alloc(xce);
     if ( port_info == NULL )
         return -1;
 
     printf("xenevtchn_bind_virq(%d)", virq);
-    port = bind_virq(virq, evtchn_handler, (void *)(intptr_t)fd);
+    port = bind_virq(virq, evtchn_handler, xce);
     printf(" = %d\n", port);
 
     if ( port < 0 )
@@ -258,7 +306,7 @@ xenevtchn_port_or_error_t xenevtchn_bind_virq(xenevtchn_handle *xce,
         return -1;
     }
 
-    port_info->bound = 1;
+    port_info->bound = true;
     port_info->port = port;
     unmask_evtchn(port);
 
@@ -267,27 +315,28 @@ xenevtchn_port_or_error_t xenevtchn_bind_virq(xenevtchn_handle *xce,
 
 xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
 {
-    int fd = xce->fd;
-    struct evtchn_port_info *port_info;
+    struct file *file = get_file_from_fd(xce->fd);
+    struct port_info *port_info;
+    struct port_list *port_list = file->dev;
     unsigned long flags;
     evtchn_port_t ret = -1;
 
     local_irq_save(flags);
 
-    files[fd].read = 0;
+    file->read = false;
 
-    LIST_FOREACH(port_info, &files[fd].evtchn.ports, list)
+    LIST_FOREACH(port_info, port_list, list)
     {
         if ( port_info->port != -1 && port_info->pending )
         {
             if ( ret == -1 )
             {
                 ret = port_info->port;
-                port_info->pending = 0;
+                port_info->pending = false;
             }
             else
             {
-                files[fd].read = 1;
+                file->read = true;
                 break;
             }
         }
-- 
2.26.2

[PATCH v3 2/3] tools/libs/gnttab: decouple more from mini-os

[Juergen Gross]

libgnttab is using implementation details of Mini-OS. Change that by
letting libgnttab use the new alloc_file_type() and get_file_from_fd()
functions and the generic dev pointer of struct file from Mini-OS.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- add alloc_file_type() support
V3:
- switch callback to use struct file * as a parameter (Andrew Cooper)
- use __attribute__((constructor)) (Andrew Cooper)
---
 tools/libs/gnttab/minios.c | 67 +++++++++++++++++++++++++++++---------
 1 file changed, 52 insertions(+), 15 deletions(-)

diff --git a/tools/libs/gnttab/minios.c b/tools/libs/gnttab/minios.c
index f78caadd30..f59fad3577 100644
--- a/tools/libs/gnttab/minios.c
+++ b/tools/libs/gnttab/minios.c
@@ -28,18 +28,55 @@
 #include <sys/mman.h>
 
 #include <errno.h>
+#include <malloc.h>
 #include <unistd.h>
 
 #include "private.h"
 
 void minios_gnttab_close_fd(int fd);
 
+static int gnttab_close_fd(struct file *file)
+{
+    gntmap_fini(file->dev);
+    free(file->dev);
+
+    return 0;
+}
+
+static const struct file_ops gnttab_ops = {
+    .name = "gnttab",
+    .close = gnttab_close_fd,
+};
+
+static unsigned int ftype_gnttab;
+
+__attribute__((constructor))
+static void gnttab_initialize(void)
+{
+    ftype_gnttab = alloc_file_type(&gnttab_ops);
+}
+
 int osdep_gnttab_open(xengnttab_handle *xgt)
 {
-    int fd = alloc_fd(FTYPE_GNTMAP);
-    if ( fd == -1 )
+    int fd;
+    struct file *file;
+    struct gntmap *gntmap;
+
+    gntmap = malloc(sizeof(*gntmap));
+    if ( !gntmap )
         return -1;
-    gntmap_init(&files[fd].gntmap);
+
+    fd = alloc_fd(ftype_gnttab);
+    file = get_file_from_fd(fd);
+
+    if ( !file )
+    {
+        free(gntmap);
+        return -1;
+    }
+
+    file->dev = gntmap;
+    gntmap_init(gntmap);
     xgt->fd = fd;
     return 0;
 }
@@ -54,8 +91,9 @@ int osdep_gnttab_close(xengnttab_handle *xgt)
 
 void minios_gnttab_close_fd(int fd)
 {
-    gntmap_fini(&files[fd].gntmap);
-    files[fd].type = FTYPE_NONE;
+    struct file *file = get_file_from_fd(fd);
+
+    gnttab_close_fd(file);
 }
 
 void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
@@ -64,16 +102,16 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
                              uint32_t notify_offset,
                              evtchn_port_t notify_port)
 {
-    int fd = xgt->fd;
+    struct file *file = get_file_from_fd(xgt->fd);
     int stride = 1;
+
     if (flags & XENGNTTAB_GRANT_MAP_SINGLE_DOMAIN)
         stride = 0;
     if (notify_offset != -1 || notify_port != -1) {
         errno = ENOSYS;
         return NULL;
     }
-    return gntmap_map_grant_refs(&files[fd].gntmap,
-                                 count, domids, stride,
+    return gntmap_map_grant_refs(file->dev, count, domids, stride,
                                  refs, prot & PROT_WRITE);
 }
 
@@ -81,11 +119,10 @@ int osdep_gnttab_unmap(xengnttab_handle *xgt,
                        void *start_address,
                        uint32_t count)
 {
-    int fd = xgt->fd;
+    struct file *file = get_file_from_fd(xgt->fd);
     int ret;
-    ret = gntmap_munmap(&files[fd].gntmap,
-                        (unsigned long) start_address,
-                        count);
+
+    ret = gntmap_munmap(file->dev, (unsigned long) start_address, count);
     if (ret < 0) {
         errno = -ret;
         return -1;
@@ -95,10 +132,10 @@ int osdep_gnttab_unmap(xengnttab_handle *xgt,
 
 int osdep_gnttab_set_max_grants(xengnttab_handle *xgt, uint32_t count)
 {
-    int fd = xgt->fd;
+    struct file *file = get_file_from_fd(xgt->fd);
     int ret;
-    ret = gntmap_set_max_grants(&files[fd].gntmap,
-                                count);
+
+    ret = gntmap_set_max_grants(file->dev, count);
     if (ret < 0) {
         errno = -ret;
         return -1;
-- 
2.26.2

[PATCH v3 3/3] tools/libs/ctrl: remove file related handling

[Juergen Gross]

There is no special file handling related to libxenctrl in Mini-OS
any longer, so the close hook can be removed.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
V2:
- new patch
---
 tools/libs/ctrl/xc_minios.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/tools/libs/ctrl/xc_minios.c b/tools/libs/ctrl/xc_minios.c
index 1799daafdc..3dea7a78a5 100644
--- a/tools/libs/ctrl/xc_minios.c
+++ b/tools/libs/ctrl/xc_minios.c
@@ -35,15 +35,6 @@
 
 #include "xc_private.h"
 
-void minios_interface_close_fd(int fd);
-
-extern void minios_interface_close_fd(int fd);
-
-void minios_interface_close_fd(int fd)
-{
-    files[fd].type = FTYPE_NONE;
-}
-
 /* Optionally flush file to disk and discard page cache */
 void discard_file_cache(xc_interface *xch, int fd, int flush)
 {
-- 
2.26.2

Re: [PATCH v3 1/3] tools/libs/evtchn: decouple more from mini-os

[Samuel Thibault]

Juergen Gross, le dim. 16 janv. 2022 09:23:44 +0100, a ecrit:
> Mini-OS and libevtchn are using implementation details of each other.
> Change that by letting libevtchn use the new alloc_file_type() and
> get_file_from_fd() function and the generic dev pointer of struct file
> from Mini-OS.
> 
> By using private struct declarations Mini-OS will be able to drop the
> libevtchn specific definitions of struct evtchn_port_info and
> evtchn_port_list in future. While at it use bool for "pending" and
> "bound".
> 
> Switch to use xce as function parameter instead of fd where possible.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
> V2:
> - use xce instead of fd as parameter internally (Andrew Cooper)
> - add alloc_file_type() support
> V3:
> - switch callback to use struct file * as a parameter (Andrew Cooper)
> - use __attribute__((constructor)) (Andrew Cooper)
> ---
>  tools/libs/evtchn/minios.c | 143 +++++++++++++++++++++++++------------
>  1 file changed, 96 insertions(+), 47 deletions(-)
> 
> diff --git a/tools/libs/evtchn/minios.c b/tools/libs/evtchn/minios.c
> index 8f84048b11..ee3da9f8fe 100644
> --- a/tools/libs/evtchn/minios.c
> +++ b/tools/libs/evtchn/minios.c
> @@ -38,29 +38,40 @@
>  
>  #include "private.h"
>  
> -extern void minios_evtchn_close_fd(int fd);
> +LIST_HEAD(port_list, port_info);
> +
> +struct port_info {
> +    LIST_ENTRY(port_info) list;
> +    evtchn_port_t port;
> +    bool pending;
> +    bool bound;
> +};
>  
>  extern struct wait_queue_head event_queue;
>  
> +void minios_evtchn_close_fd(int fd);
> +
>  /* XXX Note: This is not threadsafe */
> -static struct evtchn_port_info *port_alloc(int fd)
> +static struct port_info *port_alloc(xenevtchn_handle *xce)
>  {
> -    struct evtchn_port_info *port_info;
> +    struct port_info *port_info;
> +    struct file *file = get_file_from_fd(xce->fd);
> +    struct port_list *port_list = file->dev;
>  
> -    port_info = malloc(sizeof(struct evtchn_port_info));
> +    port_info = malloc(sizeof(struct port_info));
>      if ( port_info == NULL )
>          return NULL;
>  
> -    port_info->pending = 0;
> +    port_info->pending = false;
>      port_info->port = -1;
> -    port_info->bound = 0;
> +    port_info->bound = false;
>  
> -    LIST_INSERT_HEAD(&files[fd].evtchn.ports, port_info, list);
> +    LIST_INSERT_HEAD(port_list, port_info, list);
>  
>      return port_info;
>  }
>  
> -static void port_dealloc(struct evtchn_port_info *port_info)
> +static void port_dealloc(struct port_info *port_info)
>  {
>      if ( port_info->bound )
>          unbind_evtchn(port_info->port);
> @@ -69,18 +80,57 @@ static void port_dealloc(struct evtchn_port_info *port_info)
>      free(port_info);
>  }
>  
> +static int evtchn_close_fd(struct file *file)
> +{
> +    struct port_info *port_info, *tmp;
> +    struct port_list *port_list = file->dev;
> +
> +    LIST_FOREACH_SAFE(port_info, port_list, list, tmp)
> +        port_dealloc(port_info);
> +    free(port_list);
> +
> +    return 0;
> +}
> +
> +static const struct file_ops evtchn_ops = {
> +    .name = "evtchn",
> +    .close = evtchn_close_fd,
> +    .select_rd = select_read_flag,
> +};
> +
> +static unsigned int ftype_evtchn;
> +
> +__attribute__((constructor))
> +static void evtchn_initialize(void)
> +{
> +    ftype_evtchn = alloc_file_type(&evtchn_ops);
> +}
> +
>  /*
>   * XENEVTCHN_NO_CLOEXEC is being ignored, as there is no exec() call supported
>   * in Mini-OS.
>   */
>  int osdep_evtchn_open(xenevtchn_handle *xce, unsigned int flags)
>  {
> -    int fd = alloc_fd(FTYPE_EVTCHN);
> +    int fd;
> +    struct file *file;
> +    struct port_list *list;
>  
> -    if ( fd == -1 )
> +    list = malloc(sizeof(*list));
> +    if ( !list )
>          return -1;
>  
> -    LIST_INIT(&files[fd].evtchn.ports);
> +    fd = alloc_fd(ftype_evtchn);
> +    file = get_file_from_fd(fd);
> +
> +    if ( !file )
> +    {
> +        free(list);
> +        return -1;
> +    }
> +
> +    file->dev = list;
> +    LIST_INIT(list);
>      xce->fd = fd;
>      printf("evtchn_open() -> %d\n", fd);
>  
> @@ -104,12 +154,9 @@ int osdep_evtchn_restrict(xenevtchn_handle *xce, domid_t domid)
>  
>  void minios_evtchn_close_fd(int fd)
>  {
> -    struct evtchn_port_info *port_info, *tmp;
> +    struct file *file = get_file_from_fd(fd);
>  
> -    LIST_FOREACH_SAFE(port_info, &files[fd].evtchn.ports, list, tmp)
> -        port_dealloc(port_info);
> -
> -    files[fd].type = FTYPE_NONE;
> +    evtchn_close_fd(file);
>  }
>  
>  int xenevtchn_notify(xenevtchn_handle *xce, evtchn_port_t port)
> @@ -129,42 +176,43 @@ int xenevtchn_notify(xenevtchn_handle *xce, evtchn_port_t port)
>  
>  static void evtchn_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
>  {
> -    int fd = (int)(intptr_t)data;
> -    struct evtchn_port_info *port_info;
> +    xenevtchn_handle *xce = data;
> +    struct file *file = get_file_from_fd(xce->fd);
> +    struct port_info *port_info;
> +    struct port_list *port_list;
>  
> -    assert(files[fd].type == FTYPE_EVTCHN);
> +    assert(file);
> +    port_list = file->dev;
>      mask_evtchn(port);
> -    LIST_FOREACH(port_info, &files[fd].evtchn.ports, list)
> +    LIST_FOREACH(port_info, port_list, list)
>      {
>          if ( port_info->port == port )
>              goto found;
>      }
>  
> -    printk("Unknown port for handle %d\n", fd);
> +    printk("Unknown port %d for handle %d\n", port, xce->fd);
>      return;
>  
>   found:
> -    port_info->pending = 1;
> -    files[fd].read = 1;
> +    port_info->pending = true;
> +    file->read = true;
>      wake_up(&event_queue);
>  }
>  
>  xenevtchn_port_or_error_t xenevtchn_bind_unbound_port(xenevtchn_handle *xce,
>                                                        uint32_t domid)
>  {
> -    int fd = xce->fd;
> -    struct evtchn_port_info *port_info;
> +    struct port_info *port_info;
>      int ret;
>      evtchn_port_t port;
>  
>      assert(get_current() == main_thread);
> -    port_info = port_alloc(fd);
> +    port_info = port_alloc(xce);
>      if ( port_info == NULL )
>          return -1;
>  
>      printf("xenevtchn_bind_unbound_port(%d)", domid);
> -    ret = evtchn_alloc_unbound(domid, evtchn_handler,
> -                               (void *)(intptr_t)fd, &port);
> +    ret = evtchn_alloc_unbound(domid, evtchn_handler, xce, &port);
>      printf(" = %d\n", ret);
>  
>      if ( ret < 0 )
> @@ -174,7 +222,7 @@ xenevtchn_port_or_error_t xenevtchn_bind_unbound_port(xenevtchn_handle *xce,
>          return -1;
>      }
>  
> -    port_info->bound = 1;
> +    port_info->bound = true;
>      port_info->port = port;
>      unmask_evtchn(port);
>  
> @@ -185,19 +233,18 @@ xenevtchn_port_or_error_t xenevtchn_bind_interdomain(xenevtchn_handle *xce,
>                                                       uint32_t domid,
>                                                       evtchn_port_t remote_port)
>  {
> -    int fd = xce->fd;
> -    struct evtchn_port_info *port_info;
> +    struct port_info *port_info;
>      evtchn_port_t local_port;
>      int ret;
>  
>      assert(get_current() == main_thread);
> -    port_info = port_alloc(fd);
> +    port_info = port_alloc(xce);
>      if ( port_info == NULL )
>          return -1;
>  
>      printf("xenevtchn_bind_interdomain(%d, %"PRId32")", domid, remote_port);
>      ret = evtchn_bind_interdomain(domid, remote_port, evtchn_handler,
> -                                  (void *)(intptr_t)fd, &local_port);
> +                                  xce, &local_port);
>      printf(" = %d\n", ret);
>  
>      if ( ret < 0 )
> @@ -207,7 +254,7 @@ xenevtchn_port_or_error_t xenevtchn_bind_interdomain(xenevtchn_handle *xce,
>          return -1;
>      }
>  
> -    port_info->bound = 1;
> +    port_info->bound = true;
>      port_info->port = local_port;
>      unmask_evtchn(local_port);
>  
> @@ -217,9 +264,11 @@ xenevtchn_port_or_error_t xenevtchn_bind_interdomain(xenevtchn_handle *xce,
>  int xenevtchn_unbind(xenevtchn_handle *xce, evtchn_port_t port)
>  {
>      int fd = xce->fd;
> -    struct evtchn_port_info *port_info;
> +    struct file *file = get_file_from_fd(fd);
> +    struct port_info *port_info;
> +    struct port_list *port_list = file->dev;
>  
> -    LIST_FOREACH(port_info, &files[fd].evtchn.ports, list)
> +    LIST_FOREACH(port_info, port_list, list)
>      {
>          if ( port_info->port == port )
>          {
> @@ -238,17 +287,16 @@ int xenevtchn_unbind(xenevtchn_handle *xce, evtchn_port_t port)
>  xenevtchn_port_or_error_t xenevtchn_bind_virq(xenevtchn_handle *xce,
>                                                unsigned int virq)
>  {
> -    int fd = xce->fd;
> -    struct evtchn_port_info *port_info;
> +    struct port_info *port_info;
>      evtchn_port_t port;
>  
>      assert(get_current() == main_thread);
> -    port_info = port_alloc(fd);
> +    port_info = port_alloc(xce);
>      if ( port_info == NULL )
>          return -1;
>  
>      printf("xenevtchn_bind_virq(%d)", virq);
> -    port = bind_virq(virq, evtchn_handler, (void *)(intptr_t)fd);
> +    port = bind_virq(virq, evtchn_handler, xce);
>      printf(" = %d\n", port);
>  
>      if ( port < 0 )
> @@ -258,7 +306,7 @@ xenevtchn_port_or_error_t xenevtchn_bind_virq(xenevtchn_handle *xce,
>          return -1;
>      }
>  
> -    port_info->bound = 1;
> +    port_info->bound = true;
>      port_info->port = port;
>      unmask_evtchn(port);
>  
> @@ -267,27 +315,28 @@ xenevtchn_port_or_error_t xenevtchn_bind_virq(xenevtchn_handle *xce,
>  
>  xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
>  {
> -    int fd = xce->fd;
> -    struct evtchn_port_info *port_info;
> +    struct file *file = get_file_from_fd(xce->fd);
> +    struct port_info *port_info;
> +    struct port_list *port_list = file->dev;
>      unsigned long flags;
>      evtchn_port_t ret = -1;
>  
>      local_irq_save(flags);
>  
> -    files[fd].read = 0;
> +    file->read = false;
>  
> -    LIST_FOREACH(port_info, &files[fd].evtchn.ports, list)
> +    LIST_FOREACH(port_info, port_list, list)
>      {
>          if ( port_info->port != -1 && port_info->pending )
>          {
>              if ( ret == -1 )
>              {
>                  ret = port_info->port;
> -                port_info->pending = 0;
> +                port_info->pending = false;
>              }
>              else
>              {
> -                files[fd].read = 1;
> +                file->read = true;
>                  break;
>              }
>          }
> -- 
> 2.26.2
> 

-- 
Samuel
> Allez, soyez sympa ... traduisez-lui "linux"
Linux, c'est comme le miel : c'est vachement bon mais ça attire les
mouches. En plus, ça colle aux doigts et on a du mal à s'en défaire.
-+- TP in: Guide du linuxien pervers - "Barrez vous les mouches !"

Re: [PATCH v3 2/3] tools/libs/gnttab: decouple more from mini-os

[Samuel Thibault]

Juergen Gross, le dim. 16 janv. 2022 09:23:45 +0100, a ecrit:
> libgnttab is using implementation details of Mini-OS. Change that by
> letting libgnttab use the new alloc_file_type() and get_file_from_fd()
> functions and the generic dev pointer of struct file from Mini-OS.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
> V2:
> - add alloc_file_type() support
> V3:
> - switch callback to use struct file * as a parameter (Andrew Cooper)
> - use __attribute__((constructor)) (Andrew Cooper)
> ---
>  tools/libs/gnttab/minios.c | 67 +++++++++++++++++++++++++++++---------
>  1 file changed, 52 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/libs/gnttab/minios.c b/tools/libs/gnttab/minios.c
> index f78caadd30..f59fad3577 100644
> --- a/tools/libs/gnttab/minios.c
> +++ b/tools/libs/gnttab/minios.c
> @@ -28,18 +28,55 @@
>  #include <sys/mman.h>
>  
>  #include <errno.h>
> +#include <malloc.h>
>  #include <unistd.h>
>  
>  #include "private.h"
>  
>  void minios_gnttab_close_fd(int fd);
>  
> +static int gnttab_close_fd(struct file *file)
> +{
> +    gntmap_fini(file->dev);
> +    free(file->dev);
> +
> +    return 0;
> +}
> +
> +static const struct file_ops gnttab_ops = {
> +    .name = "gnttab",
> +    .close = gnttab_close_fd,
> +};
> +
> +static unsigned int ftype_gnttab;
> +
> +__attribute__((constructor))
> +static void gnttab_initialize(void)
> +{
> +    ftype_gnttab = alloc_file_type(&gnttab_ops);
> +}
> +
>  int osdep_gnttab_open(xengnttab_handle *xgt)
>  {
> -    int fd = alloc_fd(FTYPE_GNTMAP);
> -    if ( fd == -1 )
> +    int fd;
> +    struct file *file;
> +    struct gntmap *gntmap;
> +
> +    gntmap = malloc(sizeof(*gntmap));
> +    if ( !gntmap )
>          return -1;
> -    gntmap_init(&files[fd].gntmap);
> +
> +    fd = alloc_fd(ftype_gnttab);
> +    file = get_file_from_fd(fd);
> +
> +    if ( !file )
> +    {
> +        free(gntmap);
> +        return -1;
> +    }
> +
> +    file->dev = gntmap;
> +    gntmap_init(gntmap);
>      xgt->fd = fd;
>      return 0;
>  }
> @@ -54,8 +91,9 @@ int osdep_gnttab_close(xengnttab_handle *xgt)
>  
>  void minios_gnttab_close_fd(int fd)
>  {
> -    gntmap_fini(&files[fd].gntmap);
> -    files[fd].type = FTYPE_NONE;
> +    struct file *file = get_file_from_fd(fd);
> +
> +    gnttab_close_fd(file);
>  }
>  
>  void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
> @@ -64,16 +102,16 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
>                               uint32_t notify_offset,
>                               evtchn_port_t notify_port)
>  {
> -    int fd = xgt->fd;
> +    struct file *file = get_file_from_fd(xgt->fd);
>      int stride = 1;
> +
>      if (flags & XENGNTTAB_GRANT_MAP_SINGLE_DOMAIN)
>          stride = 0;
>      if (notify_offset != -1 || notify_port != -1) {
>          errno = ENOSYS;
>          return NULL;
>      }
> -    return gntmap_map_grant_refs(&files[fd].gntmap,
> -                                 count, domids, stride,
> +    return gntmap_map_grant_refs(file->dev, count, domids, stride,
>                                   refs, prot & PROT_WRITE);
>  }
>  
> @@ -81,11 +119,10 @@ int osdep_gnttab_unmap(xengnttab_handle *xgt,
>                         void *start_address,
>                         uint32_t count)
>  {
> -    int fd = xgt->fd;
> +    struct file *file = get_file_from_fd(xgt->fd);
>      int ret;
> -    ret = gntmap_munmap(&files[fd].gntmap,
> -                        (unsigned long) start_address,
> -                        count);
> +
> +    ret = gntmap_munmap(file->dev, (unsigned long) start_address, count);
>      if (ret < 0) {
>          errno = -ret;
>          return -1;
> @@ -95,10 +132,10 @@ int osdep_gnttab_unmap(xengnttab_handle *xgt,
>  
>  int osdep_gnttab_set_max_grants(xengnttab_handle *xgt, uint32_t count)
>  {
> -    int fd = xgt->fd;
> +    struct file *file = get_file_from_fd(xgt->fd);
>      int ret;
> -    ret = gntmap_set_max_grants(&files[fd].gntmap,
> -                                count);
> +
> +    ret = gntmap_set_max_grants(file->dev, count);
>      if (ret < 0) {
>          errno = -ret;
>          return -1;
> -- 
> 2.26.2
> 

-- 
Samuel
"c'est pas nous qui sommes à la rue, c'est la rue qui est à nous"

Re: [PATCH v3 3/3] tools/libs/ctrl: remove file related handling

[Samuel Thibault]

Juergen Gross, le dim. 16 janv. 2022 09:23:46 +0100, a ecrit:
> There is no special file handling related to libxenctrl in Mini-OS
> any longer, so the close hook can be removed.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

> ---
> V2:
> - new patch
> ---
>  tools/libs/ctrl/xc_minios.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/tools/libs/ctrl/xc_minios.c b/tools/libs/ctrl/xc_minios.c
> index 1799daafdc..3dea7a78a5 100644
> --- a/tools/libs/ctrl/xc_minios.c
> +++ b/tools/libs/ctrl/xc_minios.c
> @@ -35,15 +35,6 @@
>  
>  #include "xc_private.h"
>  
> -void minios_interface_close_fd(int fd);
> -
> -extern void minios_interface_close_fd(int fd);
> -
> -void minios_interface_close_fd(int fd)
> -{
> -    files[fd].type = FTYPE_NONE;
> -}
> -
>  /* Optionally flush file to disk and discard page cache */
>  void discard_file_cache(xc_interface *xch, int fd, int flush)
>  {
> -- 
> 2.26.2
> 

-- 
Samuel
After watching my newly-retired dad spend two weeks learning how to make a new
folder, it became obvious that "intuitive" mostly means "what the writer or
speaker of intuitive likes".
(Bruce Ediger, bediger@teal.csn.org, in comp.os.linux.misc, on X the
intuitiveness of a Mac interface.)

Re: [PATCH v3 0/3] tools/libs: decouple more from mini-os

[Samuel Thibault]

Juergen Gross, le dim. 16 janv. 2022 09:23:43 +0100, a ecrit:
> This small series removes some hard coupling of the Xen build with some
> Mini-OS internals, especially the struct file layout and the internal
> organization of the file handling.
> 
> This series depends on the Mini-OS series posted recently:
> 
> https://lists.xen.org/archives/html/xen-devel/2022-01/threads.html#00635
> 
> The main idea is to no longer have Xen library specific structures
> inside struct file, or to let struct file layout depend on the
> configuration of Mini-OS.
> 
> All Xen libraries needing a hook in struct file should use the now
> available generic dev pointer and allocate the needed data dynamically.
> 
> Additionally Xen libraries should get the pointer of struct file via
> the new get_file_from_fd() function instead of accessing directly the
> files[] array, which might go away in future (e.g. in order to support
> dynamic allocation of struct file as needed).
> 
> Via using alloc_file_type() the libs provide a function vector in
> order to enable Mini-OS to remove the dedicated callbacks into the
> libs via known function names and replacing them to use the new
> vector.

Thanks!