All of lore.kernel.org
 help / color / mirror / Atom feed
* [MINIOS PATCH v3 0/5] mini-os: remove struct file dependency on config
@ 2022-01-16  6:45 Juergen Gross
  2022-01-16  6:45 ` [MINIOS PATCH v3 1/5] introduce get_file_from_fd() Juergen Gross
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Juergen Gross @ 2022-01-16  6:45 UTC (permalink / raw)
  To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross

Today the layout of struct file is depending on the Mini-OS
configuration. This is especially bad as the layout is exported to
external users like the Xen libraries built for Mini-OS, and those
are being built only once for multiple stubdom configurations.

Today there is no direct problem resulting from this, as the main
difference between struct file layouts is a large union containing all
the device specific data for the different file types. The largest
union member is not configuration dependant, so the build is currently
not broken.

In order to avoid any future problems this patch series is eliminating
the configuration dependency by replacing most of the device specific
union members by a single pointer.

The two union members used by Xen libraries can't be replaced yet, as
those need to be switched to use the generic pointer first.

In order to hide the Mini-OS internal implementation of the files
array, patches 1, 2, and  4 are introducing a common framework to
access a struct file via its file descriptor, and to allocate new file
types dynamically instead of having them all pre-defined. The file type
specific operations are supplied via a function vector in order to
remove the dependency of lib/sys.c on all the various file types.

Patch 5 is preparing a possible future support of libxenstore instead
of using the Mini-OS internal variant located in lib/xs.c.

Changes in v3:
- first 14 patches already applied
- added patch 5

Changes in V2:
- added 3 more patches

Juergen Gross (5):
  introduce get_file_from_fd()
  reset file type in close() in one place only
  remove file type FTYPE_XC
  use function vectors instead of switch for file operations
  add CONFIG_LIBXS item

 Config.mk                     |  11 +-
 Makefile                      |   2 +-
 arch/x86/testbuild/all-no     |   2 +-
 arch/x86/testbuild/all-yes    |   2 +-
 arch/x86/testbuild/newxen-yes |   2 +-
 include/lib.h                 |  59 ++++--
 lib/sys.c                     | 345 ++++++++++++++++++++++++++--------
 lib/xs.c                      |   1 -
 8 files changed, 313 insertions(+), 111 deletions(-)

-- 
2.26.2



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

* [MINIOS PATCH v3 1/5] introduce get_file_from_fd()
  2022-01-16  6:45 [MINIOS PATCH v3 0/5] mini-os: remove struct file dependency on config Juergen Gross
@ 2022-01-16  6:45 ` Juergen Gross
  2022-01-16  6:45 ` [MINIOS PATCH v3 2/5] reset file type in close() in one place only Juergen Gross
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Juergen Gross @ 2022-01-16  6:45 UTC (permalink / raw)
  To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross

Exporting the files[] array especially for components outside the
mini-os source tree is limiting the ability to change any file handling
in mini-os.

Introduce a new function get_file_from_fd() to return the struct file
pointer (or NULL) for a given file descriptor.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
V3:
- use ARRAY_SIZE() for boundary check (Andrew Cooper)
---
 include/lib.h | 1 +
 lib/sys.c     | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/include/lib.h b/include/lib.h
index 91364ba7..7a0546bd 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -198,6 +198,7 @@ struct file {
 
 extern struct file files[];
 
+struct file *get_file_from_fd(int fd);
 int alloc_fd(enum fd_type type);
 void close_all_files(void);
 extern struct thread *main_thread;
diff --git a/lib/sys.c b/lib/sys.c
index 6f2b026d..9a03875c 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -98,6 +98,14 @@ struct file files[NOFILE] = {
     { .type = FTYPE_CONSOLE }, /* stderr */
 };
 
+struct file *get_file_from_fd(int fd)
+{
+    if ( fd < 0 || fd >= ARRAY_SIZE(files) )
+        return NULL;
+
+    return (files[fd].type == FTYPE_NONE) ? NULL : files + fd;
+}
+
 DECLARE_WAIT_QUEUE_HEAD(event_queue);
 
 int alloc_fd(enum fd_type type)
-- 
2.26.2



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

* [MINIOS PATCH v3 2/5] reset file type in close() in one place only
  2022-01-16  6:45 [MINIOS PATCH v3 0/5] mini-os: remove struct file dependency on config Juergen Gross
  2022-01-16  6:45 ` [MINIOS PATCH v3 1/5] introduce get_file_from_fd() Juergen Gross
@ 2022-01-16  6:45 ` Juergen Gross
  2022-01-16 20:33   ` Samuel Thibault
  2022-01-16  6:45 ` [MINIOS PATCH v3 3/5] remove file type FTYPE_XC Juergen Gross
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Juergen Gross @ 2022-01-16  6:45 UTC (permalink / raw)
  To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross

Today the file type in struct file is set to FTYPE_NONE for each
file type individually. Do that at the end of close() handling for
all types.

While at it wipe the complete struct file, too, in order to avoid
old data creeping into a new allocated struct file.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch
V3:
- add error handling (Andrew Cooper)
- use BUILD_BUG_ON() (Andrew Cooper)
---
 lib/sys.c | 56 ++++++++++++++++++++++++++++---------------------------
 lib/xs.c  |  1 -
 2 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/lib/sys.c b/lib/sys.c
index 9a03875c..7df77cc6 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -265,7 +265,7 @@ int read(int fd, void *buf, size_t nbytes)
             return ret;
         }
 #ifdef HAVE_LWIP
-	case FTYPE_SOCKET:
+        case FTYPE_SOCKET:
 	    return lwip_read(files[fd].fd, buf, nbytes);
 #endif
 #ifdef CONFIG_NETFRONT
@@ -424,84 +424,86 @@ int fsync(int fd) {
 
 int close(int fd)
 {
+    int res = 0;
+
+    if ( fd < 0 || fd >= ARRAY_SIZE(files) )
+        goto error;
+
     printk("close(%d)\n", fd);
     switch (files[fd].type) {
         default:
-	    files[fd].type = FTYPE_NONE;
-	    return 0;
+            break;
 #ifdef CONFIG_XENBUS
 	case FTYPE_XENBUS:
             xs_daemon_close((void*)(intptr_t) fd);
-            return 0;
+            break;
 #endif
 #ifdef HAVE_LWIP
-	case FTYPE_SOCKET: {
-	    int res = lwip_close(files[fd].fd);
-	    files[fd].type = FTYPE_NONE;
-	    return res;
-	}
+	case FTYPE_SOCKET:
+            res = lwip_close(files[fd].fd);
+            break;
 #endif
 #ifdef CONFIG_LIBXENCTRL
 	case FTYPE_XC:
 	    minios_interface_close_fd(fd);
-	    return 0;
+            break;
 #endif
 #ifdef CONFIG_LIBXENEVTCHN
 	case FTYPE_EVTCHN:
 	    minios_evtchn_close_fd(fd);
-            return 0;
+            break;
 #endif
 #ifdef CONFIG_LIBXENGNTTAB
 	case FTYPE_GNTMAP:
 	    minios_gnttab_close_fd(fd);
-	    return 0;
+            break;
 #endif
 #ifdef CONFIG_NETFRONT
 	case FTYPE_TAP:
 	    shutdown_netfront(files[fd].dev);
-	    files[fd].type = FTYPE_NONE;
-	    return 0;
+            break;
 #endif
 #ifdef CONFIG_BLKFRONT
 	case FTYPE_BLK:
             shutdown_blkfront(files[fd].dev);
-	    files[fd].type = FTYPE_NONE;
-	    return 0;
+            break;
 #endif
 #ifdef CONFIG_TPMFRONT
 	case FTYPE_TPMFRONT:
             shutdown_tpmfront(files[fd].dev);
-	    files[fd].type = FTYPE_NONE;
-	    return 0;
+            break;
 #endif
 #ifdef CONFIG_TPM_TIS
 	case FTYPE_TPM_TIS:
             shutdown_tpm_tis(files[fd].dev);
-	    files[fd].type = FTYPE_NONE;
-	    return 0;
+            break;
 #endif
 #ifdef CONFIG_KBDFRONT
 	case FTYPE_KBD:
             shutdown_kbdfront(files[fd].dev);
-            files[fd].type = FTYPE_NONE;
-            return 0;
+            break;
 #endif
 #ifdef CONFIG_FBFRONT
 	case FTYPE_FB:
             shutdown_fbfront(files[fd].dev);
-            files[fd].type = FTYPE_NONE;
-            return 0;
+            break;
 #endif
 #ifdef CONFIG_CONSFRONT
         case FTYPE_SAVEFILE:
         case FTYPE_CONSOLE:
             fini_consfront(files[fd].dev);
-            files[fd].type = FTYPE_NONE;
-            return 0;
+            break;
 #endif
 	case FTYPE_NONE:
-	    break;
+            goto error;
     }
+
+    memset(files + fd, 0, sizeof(struct file));
+    BUILD_BUG_ON(FTYPE_NONE != 0);
+
+    return res;
+
+ error:
     printk("close(%d): Bad descriptor\n", fd);
     errno = EBADF;
     return -1;
diff --git a/lib/xs.c b/lib/xs.c
index 0459f52f..4af0f960 100644
--- a/lib/xs.c
+++ b/lib/xs.c
@@ -35,7 +35,6 @@ void xs_daemon_close(struct xs_handle *h)
         next = event->next;
         free(event);
     }
-    files[fd].type = FTYPE_NONE;
 }
 
 int xs_fileno(struct xs_handle *h)
-- 
2.26.2



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

* [MINIOS PATCH v3 3/5] remove file type FTYPE_XC
  2022-01-16  6:45 [MINIOS PATCH v3 0/5] mini-os: remove struct file dependency on config Juergen Gross
  2022-01-16  6:45 ` [MINIOS PATCH v3 1/5] introduce get_file_from_fd() Juergen Gross
  2022-01-16  6:45 ` [MINIOS PATCH v3 2/5] reset file type in close() in one place only Juergen Gross
@ 2022-01-16  6:45 ` Juergen Gross
  2022-01-16 20:34   ` Samuel Thibault
  2022-01-16  6:45 ` [MINIOS PATCH v3 4/5] use function vectors instead of switch for file operations Juergen Gross
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Juergen Gross @ 2022-01-16  6:45 UTC (permalink / raw)
  To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross

The only reason for the file type FTYPE_XC has been gone long time
ago: it was needed for xc_map_foreign_bulk(), which has been switched
to use libxenforeignmemory and doesn't need this special file any
more.

So remove everything related to FTYPE_XC.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch
V3:
- moved patch in series (Andrew Cooper)
---
 Config.mk                     |  1 -
 arch/x86/testbuild/all-no     |  1 -
 arch/x86/testbuild/all-yes    |  1 -
 arch/x86/testbuild/newxen-yes |  1 -
 include/lib.h                 |  1 -
 lib/sys.c                     | 13 -------------
 6 files changed, 18 deletions(-)

diff --git a/Config.mk b/Config.mk
index 5e660891..1e083881 100644
--- a/Config.mk
+++ b/Config.mk
@@ -197,7 +197,6 @@ CONFIG-n += CONFIG_PARAVIRT
 endif
 # Support legacy CONFIG_XC value
 CONFIG_XC ?= $(libc)
-CONFIG-$(CONFIG_XC) += CONFIG_LIBXENCTRL
 CONFIG-$(CONFIG_XC) += CONFIG_LIBXENEVTCHN
 CONFIG-$(CONFIG_XC) += CONFIG_LIBXENGNTTAB
 
diff --git a/arch/x86/testbuild/all-no b/arch/x86/testbuild/all-no
index 7972ecd5..d6fc2608 100644
--- a/arch/x86/testbuild/all-no
+++ b/arch/x86/testbuild/all-no
@@ -13,7 +13,6 @@ CONFIG_FBFRONT = n
 CONFIG_KBDFRONT = n
 CONFIG_CONSFRONT = n
 CONFIG_XENBUS = n
-CONFIG_LIBXENCTRL = n
 CONFIG_LIBXENEVTCHN = n
 CONFIG_LIBXENGNTTAB = n
 CONFIG_LWIP = n
diff --git a/arch/x86/testbuild/all-yes b/arch/x86/testbuild/all-yes
index bc8eea57..98bbfebf 100644
--- a/arch/x86/testbuild/all-yes
+++ b/arch/x86/testbuild/all-yes
@@ -16,7 +16,6 @@ CONFIG_XENBUS = y
 CONFIG_BALLOON = y
 CONFIG_USE_XEN_CONSOLE = y
 # The following are special: they need support from outside
-CONFIG_LIBXENCTRL = n
 CONFIG_LIBXENEVTCHN = n
 CONFIG_LIBXENGNTTAB = n
 CONFIG_LWIP = n
diff --git a/arch/x86/testbuild/newxen-yes b/arch/x86/testbuild/newxen-yes
index f72123b5..06032004 100644
--- a/arch/x86/testbuild/newxen-yes
+++ b/arch/x86/testbuild/newxen-yes
@@ -17,7 +17,6 @@ CONFIG_BALLOON = y
 CONFIG_USE_XEN_CONSOLE = y
 XEN_INTERFACE_VERSION=__XEN_LATEST_INTERFACE_VERSION__
 # The following are special: they need support from outside
-CONFIG_LIBXENCTRL = n
 CONFIG_LIBXENEVTCHN = n
 CONFIG_LIBXENGNTTAB = n
 CONFIG_LWIP = n
diff --git a/include/lib.h b/include/lib.h
index 7a0546bd..7ca90768 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -159,7 +159,6 @@ enum fd_type {
     FTYPE_CONSOLE,
     FTYPE_FILE,
     FTYPE_XENBUS,
-    FTYPE_XC,
     FTYPE_EVTCHN,
     FTYPE_GNTMAP,
     FTYPE_SOCKET,
diff --git a/lib/sys.c b/lib/sys.c
index 7df77cc6..a24687b7 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -87,7 +87,6 @@
     }
 
 #define NOFILE 32
-extern void minios_interface_close_fd(int fd);
 extern void minios_evtchn_close_fd(int fd);
 extern void minios_gnttab_close_fd(int fd);
 
@@ -443,11 +442,6 @@ int close(int fd)
             res = lwip_close(files[fd].fd);
             break;
 #endif
-#ifdef CONFIG_LIBXENCTRL
-	case FTYPE_XC:
-	    minios_interface_close_fd(fd);
-            break;
-#endif
 #ifdef CONFIG_LIBXENEVTCHN
 	case FTYPE_EVTCHN:
 	    minios_evtchn_close_fd(fd);
@@ -651,7 +645,6 @@ static const char file_types[] = {
     [FTYPE_NONE]	= 'N',
     [FTYPE_CONSOLE]	= 'C',
     [FTYPE_XENBUS]	= 'S',
-    [FTYPE_XC]		= 'X',
     [FTYPE_EVTCHN]	= 'E',
     [FTYPE_SOCKET]	= 's',
     [FTYPE_TAP]		= 'T',
@@ -1383,12 +1376,6 @@ void *mmap(void *start, size_t length, int prot, int flags, int fd, off_t offset
 
     if (fd == -1)
         return map_zero(n, 1);
-#ifdef CONFIG_LIBXENCTRL
-    else if (files[fd].type == FTYPE_XC) {
-        unsigned long zero = 0;
-        return map_frames_ex(&zero, n, 0, 0, 1, DOMID_SELF, NULL, 0);
-    }
-#endif
     else if (files[fd].type == FTYPE_MEM) {
         unsigned long first_mfn = offset >> PAGE_SHIFT;
         return map_frames_ex(&first_mfn, n, 0, 1, 1, DOMID_IO, NULL, _PAGE_PRESENT|_PAGE_RW);
-- 
2.26.2



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

* [MINIOS PATCH v3 4/5] use function vectors instead of switch for file operations
  2022-01-16  6:45 [MINIOS PATCH v3 0/5] mini-os: remove struct file dependency on config Juergen Gross
                   ` (2 preceding siblings ...)
  2022-01-16  6:45 ` [MINIOS PATCH v3 3/5] remove file type FTYPE_XC Juergen Gross
@ 2022-01-16  6:45 ` Juergen Gross
  2022-01-16 20:39   ` Samuel Thibault
  2022-01-16  6:45 ` [MINIOS PATCH v3 5/5] add CONFIG_LIBXS item Juergen Gross
  2022-01-16 20:41 ` [MINIOS PATCH v3 0/5] mini-os: remove struct file dependency on config Samuel Thibault
  5 siblings, 1 reply; 11+ messages in thread
From: Juergen Gross @ 2022-01-16  6:45 UTC (permalink / raw)
  To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross

Introduce file type specific function vectors for all the needed file
operations which are file type specific in order to prepare replacing
the large switch statements in each generic file function.

Add a function to allocate a new file type dynamically in order to
prepare removing direct dependencies to external components in the
future. For this reason switch the file type from an enum to an
unsigned int. Prepare removal of some statically defines file types
by putting them at the end of the defined list.

Change the debug output for the file type from a single character to
a string in order to support a future removal of the file_types[]
array.

Provide some functions useful for file_ops in future patches.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch
V3:
- drop typedefs for callbacks (Andrew Cooper)
- const (Andrew Cooper)
- drop loop from alloc_file_type() (Andrew Cooper)
- switch to struct file * as first parameter of callbacks (Andrew Cooper)
---
 include/lib.h |  57 +++++++----
 lib/sys.c     | 268 ++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 263 insertions(+), 62 deletions(-)

diff --git a/include/lib.h b/include/lib.h
index 7ca90768..44696806 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -66,6 +66,7 @@
 
 #ifdef HAVE_LIBC
 #include <sys/queue.h>
+#include <sys/stat.h>
 #include <stdio.h>
 #include <string.h>
 #else
@@ -154,23 +155,23 @@ do {                                                           \
 void sanity_check(void);
 
 #ifdef HAVE_LIBC
-enum fd_type {
-    FTYPE_NONE = 0,
-    FTYPE_CONSOLE,
-    FTYPE_FILE,
-    FTYPE_XENBUS,
-    FTYPE_EVTCHN,
-    FTYPE_GNTMAP,
-    FTYPE_SOCKET,
-    FTYPE_TAP,
-    FTYPE_BLK,
-    FTYPE_KBD,
-    FTYPE_FB,
-    FTYPE_MEM,
-    FTYPE_SAVEFILE,
-    FTYPE_TPMFRONT,
-    FTYPE_TPM_TIS,
-};
+#define FTYPE_NONE       0
+#define FTYPE_CONSOLE    1
+#define FTYPE_FILE       2
+#define FTYPE_SOCKET     3
+#define FTYPE_MEM        4
+#define FTYPE_SAVEFILE   5
+#define FTYPE_FB         6
+#define FTYPE_KBD        7
+#define FTYPE_TAP        8
+#define FTYPE_BLK        9
+#define FTYPE_TPMFRONT  10
+#define FTYPE_TPM_TIS   11
+#define FTYPE_XENBUS    12
+#define FTYPE_GNTMAP    13
+#define FTYPE_EVTCHN    14
+#define FTYPE_N         15
+#define FTYPE_SPARE     16
 
 LIST_HEAD(evtchn_port_list, evtchn_port_info);
 
@@ -182,7 +183,7 @@ struct evtchn_port_info {
 };
 
 struct file {
-    enum fd_type type;
+    unsigned int type;
     bool read;	/* maybe available for read */
     off_t offset;
     union {
@@ -197,8 +198,26 @@ struct file {
 
 extern struct file files[];
 
+struct file_ops {
+    const char *name;
+    int (*read)(struct file *file, void *buf, size_t nbytes);
+    int (*write)(struct file *file, const void *buf, size_t nbytes);
+    off_t (*lseek)(struct file *file, off_t offset, int whence);
+    int (*close)(struct file *file);
+    int (*fstat)(struct file *file, struct stat *buf);
+    int (*fcntl)(struct file *file, int cmd, va_list args);
+    bool (*select_rd)(struct file *file);
+    bool (*select_wr)(struct file *file);
+};
+
+unsigned int alloc_file_type(const struct file_ops *ops);
+
+off_t lseek_default(struct file *file, off_t offset, int whence);
+bool select_yes(struct file *file);
+bool select_read_flag(struct file *file);
+
 struct file *get_file_from_fd(int fd);
-int alloc_fd(enum fd_type type);
+int alloc_fd(unsigned int type);
 void close_all_files(void);
 extern struct thread *main_thread;
 void sparse(unsigned long data, size_t size);
diff --git a/lib/sys.c b/lib/sys.c
index a24687b7..7be01fd3 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -97,6 +97,40 @@ struct file files[NOFILE] = {
     { .type = FTYPE_CONSOLE }, /* stderr */
 };
 
+static const struct file_ops file_ops_none = {
+    .name = "none",
+};
+
+static const struct file_ops *file_ops[FTYPE_N + FTYPE_SPARE] = {
+    [FTYPE_NONE] = &file_ops_none,
+};
+
+unsigned int alloc_file_type(const struct file_ops *ops)
+{
+    static unsigned int i = FTYPE_N;
+    unsigned int ret;
+
+    pthread_mutex_lock(&fd_lock);
+
+    BUG_ON(i == ARRAY_SIZE(file_ops));
+    ret = i++;
+    file_ops[ret] = ops;
+
+    pthread_mutex_unlock(&fd_lock);
+
+    printk("New file type \"%s\"(%u) allocated\n", ops->name, ret);
+
+    return ret;
+}
+
+static const struct file_ops *get_file_ops(unsigned int type)
+{
+    if ( type >= ARRAY_SIZE(file_ops) || !file_ops[type] )
+        return &file_ops_none;
+
+    return file_ops[type];
+}
+
 struct file *get_file_from_fd(int fd)
 {
     if ( fd < 0 || fd >= ARRAY_SIZE(files) )
@@ -107,7 +141,7 @@ struct file *get_file_from_fd(int fd)
 
 DECLARE_WAIT_QUEUE_HEAD(event_queue);
 
-int alloc_fd(enum fd_type type)
+int alloc_fd(unsigned int type)
 {
     int i;
     pthread_mutex_lock(&fd_lock);
@@ -248,7 +282,17 @@ int isatty(int fd)
 
 int read(int fd, void *buf, size_t nbytes)
 {
-    switch (files[fd].type) {
+    struct file *file = get_file_from_fd(fd);
+    const struct file_ops *ops;
+
+    if ( !file )
+        goto error;
+
+    ops = get_file_ops(file->type);
+    if ( ops->read )
+        return ops->read(file, buf, nbytes);
+
+    switch (file->type) {
         case FTYPE_SAVEFILE:
 	case FTYPE_CONSOLE: {
 	    int ret;
@@ -320,6 +364,8 @@ int read(int fd, void *buf, size_t nbytes)
 	default:
 	    break;
     }
+
+ error:
     printk("read(%d): Bad descriptor\n", fd);
     errno = EBADF;
     return -1;
@@ -327,7 +373,17 @@ int read(int fd, void *buf, size_t nbytes)
 
 int write(int fd, const void *buf, size_t nbytes)
 {
-    switch (files[fd].type) {
+    struct file *file = get_file_from_fd(fd);
+    const struct file_ops *ops;
+
+    if ( !file )
+        goto error;
+
+    ops = get_file_ops(file->type);
+    if ( ops->write )
+        return ops->write(file, buf, nbytes);
+
+    switch (file->type) {
         case FTYPE_SAVEFILE: {
                 int ret = 0, tot = nbytes;
                 while (nbytes > 0) {
@@ -364,14 +420,61 @@ int write(int fd, const void *buf, size_t nbytes)
 	default:
 	    break;
     }
+
+ error:
     printk("write(%d): Bad descriptor\n", fd);
     errno = EBADF;
     return -1;
 }
 
+off_t lseek_default(struct file *file, off_t offset, int whence)
+{
+    switch ( whence )
+    {
+    case SEEK_SET:
+        file->offset = offset;
+        break;
+
+    case SEEK_CUR:
+        file->offset += offset;
+        break;
+
+    case SEEK_END:
+    {
+        struct stat st;
+        int ret;
+
+        ret = fstat(file - files, &st);
+        if ( ret )
+            return -1;
+        file->offset = st.st_size + offset;
+        break;
+    }
+
+    default:
+        errno = EINVAL;
+        return -1;
+    }
+
+    return file->offset;
+}
+
 off_t lseek(int fd, off_t offset, int whence)
 {
-    switch(files[fd].type) {
+    struct file *file = get_file_from_fd(fd);
+    const struct file_ops *ops;
+
+    if ( !file )
+    {
+        errno = EBADF;
+        return (off_t)-1;
+    }
+
+    ops = get_file_ops(file->type);
+    if ( ops->lseek )
+        return ops->lseek(file, offset, whence);
+
+    switch(file->type) {
 #ifdef CONFIG_BLKFRONT
        case FTYPE_BLK:
           break;
@@ -392,28 +495,7 @@ off_t lseek(int fd, off_t offset, int whence)
           return (off_t) -1;
     }
 
-    switch (whence) {
-       case SEEK_SET:
-          files[fd].offset = offset;
-          break;
-       case SEEK_CUR:
-          files[fd].offset += offset;
-          break;
-       case SEEK_END:
-          {
-             struct stat st;
-             int ret;
-             ret = fstat(fd, &st);
-             if (ret)
-                return -1;
-             files[fd].offset = st.st_size + offset;
-             break;
-          }
-       default:
-          errno = EINVAL;
-          return -1;
-    }
-    return files[fd].offset;
+    return lseek_default(file, offset, whence);
 }
 
 int fsync(int fd) {
@@ -424,12 +506,21 @@ int fsync(int fd) {
 int close(int fd)
 {
     int res = 0;
+    struct file *file = get_file_from_fd(fd);
+    const struct file_ops *ops;
 
-    if ( fd < 0 || fd >= ARRAY_SIZE(files) )
+    if ( !file )
         goto error;
 
+    ops = get_file_ops(file->type);
     printk("close(%d)\n", fd);
-    switch (files[fd].type) {
+    if ( ops->close )
+    {
+        res = ops->close(file);
+        goto out;
+    }
+
+    switch (file->type) {
         default:
             break;
 #ifdef CONFIG_XENBUS
@@ -492,6 +583,7 @@ int close(int fd)
             goto error;
     }
 
+ out:
     memset(files + fd, 0, sizeof(struct file));
     BUILD_BUG_ON(FTYPE_NONE != 0);
 
@@ -522,8 +614,19 @@ int stat(const char *path, struct stat *buf)
 
 int fstat(int fd, struct stat *buf)
 {
+    struct file *file = get_file_from_fd(fd);
+    const struct file_ops *ops;
+
+    if ( !file )
+        goto error;
+
     init_stat(buf);
-    switch (files[fd].type) {
+
+    ops = get_file_ops(file->type);
+    if ( ops->fstat )
+        return ops->fstat(file, buf);
+
+    switch (file->type) {
 	case FTYPE_SAVEFILE:
 	case FTYPE_CONSOLE:
 	case FTYPE_SOCKET: {
@@ -557,6 +660,7 @@ int fstat(int fd, struct stat *buf)
 	    break;
     }
 
+ error:
     printk("statf(%d): Bad descriptor\n", fd);
     errno = EBADF;
     return -1;
@@ -588,6 +692,27 @@ int fcntl(int fd, int cmd, ...)
 {
     long arg;
     va_list ap;
+    int res;
+    struct file *file = get_file_from_fd(fd);
+    const struct file_ops *ops;
+
+    if ( !file )
+    {
+        errno = EBADF;
+        return -1;
+    }
+
+    ops = get_file_ops(files[fd].type);
+
+    if ( ops->fcntl )
+    {
+        va_start(ap, cmd);
+        res = ops->fcntl(file, cmd, ap);
+        va_end(ap);
+
+        return res;
+    }
+
     va_start(ap, cmd);
     arg = va_arg(ap, long);
     va_end(ap);
@@ -641,17 +766,28 @@ int closedir(DIR *dir)
 /* We assume that only the main thread calls select(). */
 
 #if defined(LIBC_DEBUG) || defined(LIBC_VERBOSE)
-static const char file_types[] = {
-    [FTYPE_NONE]	= 'N',
-    [FTYPE_CONSOLE]	= 'C',
-    [FTYPE_XENBUS]	= 'S',
-    [FTYPE_EVTCHN]	= 'E',
-    [FTYPE_SOCKET]	= 's',
-    [FTYPE_TAP]		= 'T',
-    [FTYPE_BLK]		= 'B',
-    [FTYPE_KBD]		= 'K',
-    [FTYPE_FB]		= 'G',
+static const char *const file_types[] = {
+    [FTYPE_NONE]    = "none",
+    [FTYPE_CONSOLE] = "console",
+    [FTYPE_XENBUS]  = "xenbus",
+    [FTYPE_EVTCHN]  = "evtchn",
+    [FTYPE_SOCKET]  = "socket",
+    [FTYPE_TAP]     = "net",
+    [FTYPE_BLK]     = "blk",
+    [FTYPE_KBD]     = "kbd",
+    [FTYPE_FB]      = "fb",
 };
+
+static const char *get_type_name(unsigned int type)
+{
+    if ( type < ARRAY_SIZE(file_ops) && file_ops[type] )
+        return file_ops[type]->name;
+
+    if ( type < ARRAY_SIZE(file_types) && file_types[type] )
+        return file_types[type];
+
+    return "none";
+}
 #endif
 #ifdef LIBC_DEBUG
 static void dump_set(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, struct timeval *timeout)
@@ -663,7 +799,7 @@ static void dump_set(int nfds, fd_set *readfds, fd_set *writefds, fd_set *except
 	if (FD_ISSET(i, set)) { \
 	    if (comma) \
 		printk(", "); \
-	    printk("%d(%c)", i, file_types[files[i].type]); \
+            printk("%d(%s)", i, get_type_name(files[i].type)); \
 	    comma = 1; \
 	} \
     } \
@@ -697,7 +833,7 @@ static void dump_pollfds(struct pollfd *pfd, int nfds, int timeout)
         fd = pfd[i].fd;
         if (comma)
             printk(", ");
-        printk("%d(%c)/%02x", fd, file_types[files[fd].type],
+        printk("%d(%s)/%02x", fd, get_type_name(files[fd].type),
             pfd[i].events);
             comma = 1;
     }
@@ -709,6 +845,16 @@ static void dump_pollfds(struct pollfd *pfd, int nfds, int timeout)
 #define dump_pollfds(pfds, nfds, timeout)
 #endif
 
+bool select_yes(struct file *file)
+{
+    return true;
+}
+
+bool select_read_flag(struct file *file)
+{
+    return file->read;
+}
+
 /* Just poll without blocking */
 static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds)
 {
@@ -760,11 +906,47 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
 
     /* Then see others as well. */
     for (i = 0; i < nfds; i++) {
-	switch(files[i].type) {
+        struct file *file = get_file_from_fd(i);
+
+        if ( !file )
+        {
+            FD_CLR(i, readfds);
+            FD_CLR(i, writefds);
+            FD_CLR(i, exceptfds);
+            continue;
+        }
+
+        switch(file->type) {
 	default:
+        {
+            const struct file_ops *ops = file_ops[file->type];
+
+            if ( ops )
+            {
+                if ( FD_ISSET(i, readfds) )
+                {
+                    if ( ops->select_rd && ops->select_rd(file) )
+                        n++;
+                    else
+                        FD_CLR(i, readfds);
+                }
+                if ( FD_ISSET(i, writefds) )
+                {
+                    if ( ops->select_wr && ops->select_wr(file) )
+                        n++;
+                    else
+                        FD_CLR(i, writefds);
+                }
+                FD_CLR(i, exceptfds);
+
+                break;
+            }
+
 	    if (FD_ISSET(i, readfds) || FD_ISSET(i, writefds) || FD_ISSET(i, exceptfds))
 		printk("bogus fd %d in select\n", i);
 	    /* Fallthrough.  */
+        }
+
 	case FTYPE_CONSOLE:
 	    if (FD_ISSET(i, readfds)) {
                 if (xencons_ring_avail(files[i].dev))
@@ -842,7 +1024,7 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
 	printk("%d(%d): ", nb, sock_n);
 	for (i = 0; i < nfds; i++) {
 	    if (nbread[i] || nbwrite[i] || nbexcept[i])
-		printk(" %d(%c):", i, file_types[files[i].type]);
+                printk(" %d(%c):", i, get_type_name(files[i].type));
 	    if (nbread[i])
 	    	printk(" %dR", nbread[i]);
 	    if (nbwrite[i])
-- 
2.26.2



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

* [MINIOS PATCH v3 5/5] add CONFIG_LIBXS item
  2022-01-16  6:45 [MINIOS PATCH v3 0/5] mini-os: remove struct file dependency on config Juergen Gross
                   ` (3 preceding siblings ...)
  2022-01-16  6:45 ` [MINIOS PATCH v3 4/5] use function vectors instead of switch for file operations Juergen Gross
@ 2022-01-16  6:45 ` Juergen Gross
  2022-01-16 20:40   ` Samuel Thibault
  2022-01-16 20:41 ` [MINIOS PATCH v3 0/5] mini-os: remove struct file dependency on config Samuel Thibault
  5 siblings, 1 reply; 11+ messages in thread
From: Juergen Gross @ 2022-01-16  6:45 UTC (permalink / raw)
  To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross

Mini-OS contains a stripped down version of libxenstore in lib/xs.c.
Today it is being built always if CONFIG_XENBUS is set and libc
support is enabled.

In order to allow a Mini-OS specific build of libxenstore to be used
instead add a new CONFIG_LIBXS item which per default will have the
same setting as CONFIG_XENBUS. A user wanting to replace lib/xs.c
with libxenstore would just need to set CONFIG_XENBUS=y and
CONFIG_LIBXS=n.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- new patch
---
 Config.mk                     | 10 ++++++++--
 Makefile                      |  2 +-
 arch/x86/testbuild/all-no     |  1 +
 arch/x86/testbuild/all-yes    |  1 +
 arch/x86/testbuild/newxen-yes |  1 +
 lib/sys.c                     |  4 ++--
 6 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/Config.mk b/Config.mk
index 1e083881..03023033 100644
--- a/Config.mk
+++ b/Config.mk
@@ -171,7 +171,10 @@ endif
 # arch/*/testbuild/*-yes and arch/*/testbuild/*-no should set ALL possible
 # CONFIG_ variables.
 
-# Configuration defaults
+# Configuration defaults:
+# CONFIG-y contains all items defaulting to "y"
+# CONFIG-n contains all items defaulting to "n"
+# CONFIG-x contains all items being calculated if not set explicitly
 CONFIG-y += CONFIG_START_NETWORK
 CONFIG-y += CONFIG_SPARSE_BSS
 CONFIG-y += CONFIG_BLKFRONT
@@ -205,7 +208,10 @@ CONFIG-$(lwip) += CONFIG_LWIP
 $(foreach i,$(CONFIG-y),$(eval $(i) ?= y))
 $(foreach i,$(CONFIG-n),$(eval $(i) ?= n))
 
-CONFIG-all := $(CONFIG-y) $(CONFIG-n)
+CONFIG-x += CONFIG_LIBXS
+CONFIG_LIBXS ?= $(CONFIG_XENBUS)
+
+CONFIG-all := $(CONFIG-y) $(CONFIG-n) $(CONFIG-x)
 
 # Export config items as compiler directives
 $(foreach i,$(CONFIG-all),$(eval DEFINES-$($(i)) += -D$(i)))
diff --git a/Makefile b/Makefile
index 06b60fc7..9f95d197 100644
--- a/Makefile
+++ b/Makefile
@@ -65,7 +65,7 @@ src-y += lib/stack_chk_fail.c
 src-y += lib/string.c
 src-y += lib/sys.c
 src-y += lib/xmalloc.c
-src-$(CONFIG_XENBUS) += lib/xs.c
+src-$(CONFIG_LIBXS) += lib/xs.c
 
 src-$(CONFIG_XENBUS) += xenbus/xenbus.c
 
diff --git a/arch/x86/testbuild/all-no b/arch/x86/testbuild/all-no
index d6fc2608..46f974de 100644
--- a/arch/x86/testbuild/all-no
+++ b/arch/x86/testbuild/all-no
@@ -13,6 +13,7 @@ CONFIG_FBFRONT = n
 CONFIG_KBDFRONT = n
 CONFIG_CONSFRONT = n
 CONFIG_XENBUS = n
+CONFIG_LIBXS = n
 CONFIG_LIBXENEVTCHN = n
 CONFIG_LIBXENGNTTAB = n
 CONFIG_LWIP = n
diff --git a/arch/x86/testbuild/all-yes b/arch/x86/testbuild/all-yes
index 98bbfebf..3ead12f1 100644
--- a/arch/x86/testbuild/all-yes
+++ b/arch/x86/testbuild/all-yes
@@ -13,6 +13,7 @@ CONFIG_FBFRONT = y
 CONFIG_KBDFRONT = y
 CONFIG_CONSFRONT = y
 CONFIG_XENBUS = y
+CONFIG_LIBXS = y
 CONFIG_BALLOON = y
 CONFIG_USE_XEN_CONSOLE = y
 # The following are special: they need support from outside
diff --git a/arch/x86/testbuild/newxen-yes b/arch/x86/testbuild/newxen-yes
index 06032004..5c0b3c80 100644
--- a/arch/x86/testbuild/newxen-yes
+++ b/arch/x86/testbuild/newxen-yes
@@ -13,6 +13,7 @@ CONFIG_FBFRONT = y
 CONFIG_KBDFRONT = y
 CONFIG_CONSFRONT = y
 CONFIG_XENBUS = y
+CONFIG_LIBXS = y
 CONFIG_BALLOON = y
 CONFIG_USE_XEN_CONSOLE = y
 XEN_INTERFACE_VERSION=__XEN_LATEST_INTERFACE_VERSION__
diff --git a/lib/sys.c b/lib/sys.c
index 7be01fd3..e0ac5099 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -523,7 +523,7 @@ int close(int fd)
     switch (file->type) {
         default:
             break;
-#ifdef CONFIG_XENBUS
+#ifdef CONFIG_LIBXS
 	case FTYPE_XENBUS:
             xs_daemon_close((void*)(intptr_t) fd);
             break;
@@ -958,7 +958,7 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
                 n++;
 	    FD_CLR(i, exceptfds);
 	    break;
-#ifdef CONFIG_XENBUS
+#ifdef CONFIG_LIBXS
 	case FTYPE_XENBUS:
 	    if (FD_ISSET(i, readfds)) {
                 if (files[i].dev)
-- 
2.26.2



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

* Re: [MINIOS PATCH v3 2/5] reset file type in close() in one place only
  2022-01-16  6:45 ` [MINIOS PATCH v3 2/5] reset file type in close() in one place only Juergen Gross
@ 2022-01-16 20:33   ` Samuel Thibault
  0 siblings, 0 replies; 11+ messages in thread
From: Samuel Thibault @ 2022-01-16 20:33 UTC (permalink / raw)
  To: Juergen Gross; +Cc: minios-devel, xen-devel, wl

Juergen Gross, le dim. 16 janv. 2022 07:45:24 +0100, a ecrit:
> Today the file type in struct file is set to FTYPE_NONE for each
> file type individually. Do that at the end of close() handling for
> all types.
> 
> While at it wipe the complete struct file, too, in order to avoid
> old data creeping into a new allocated struct file.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

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

> ---
> V2:
> - new patch
> V3:
> - add error handling (Andrew Cooper)
> - use BUILD_BUG_ON() (Andrew Cooper)
> ---
>  lib/sys.c | 56 ++++++++++++++++++++++++++++---------------------------
>  lib/xs.c  |  1 -
>  2 files changed, 29 insertions(+), 28 deletions(-)
> 
> diff --git a/lib/sys.c b/lib/sys.c
> index 9a03875c..7df77cc6 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -265,7 +265,7 @@ int read(int fd, void *buf, size_t nbytes)
>              return ret;
>          }
>  #ifdef HAVE_LWIP
> -	case FTYPE_SOCKET:
> +        case FTYPE_SOCKET:
>  	    return lwip_read(files[fd].fd, buf, nbytes);
>  #endif
>  #ifdef CONFIG_NETFRONT
> @@ -424,84 +424,86 @@ int fsync(int fd) {
>  
>  int close(int fd)
>  {
> +    int res = 0;
> +
> +    if ( fd < 0 || fd >= ARRAY_SIZE(files) )
> +        goto error;
> +
>      printk("close(%d)\n", fd);
>      switch (files[fd].type) {
>          default:
> -	    files[fd].type = FTYPE_NONE;
> -	    return 0;
> +            break;
>  #ifdef CONFIG_XENBUS
>  	case FTYPE_XENBUS:
>              xs_daemon_close((void*)(intptr_t) fd);
> -            return 0;
> +            break;
>  #endif
>  #ifdef HAVE_LWIP
> -	case FTYPE_SOCKET: {
> -	    int res = lwip_close(files[fd].fd);
> -	    files[fd].type = FTYPE_NONE;
> -	    return res;
> -	}
> +	case FTYPE_SOCKET:
> +            res = lwip_close(files[fd].fd);
> +            break;
>  #endif
>  #ifdef CONFIG_LIBXENCTRL
>  	case FTYPE_XC:
>  	    minios_interface_close_fd(fd);
> -	    return 0;
> +            break;
>  #endif
>  #ifdef CONFIG_LIBXENEVTCHN
>  	case FTYPE_EVTCHN:
>  	    minios_evtchn_close_fd(fd);
> -            return 0;
> +            break;
>  #endif
>  #ifdef CONFIG_LIBXENGNTTAB
>  	case FTYPE_GNTMAP:
>  	    minios_gnttab_close_fd(fd);
> -	    return 0;
> +            break;
>  #endif
>  #ifdef CONFIG_NETFRONT
>  	case FTYPE_TAP:
>  	    shutdown_netfront(files[fd].dev);
> -	    files[fd].type = FTYPE_NONE;
> -	    return 0;
> +            break;
>  #endif
>  #ifdef CONFIG_BLKFRONT
>  	case FTYPE_BLK:
>              shutdown_blkfront(files[fd].dev);
> -	    files[fd].type = FTYPE_NONE;
> -	    return 0;
> +            break;
>  #endif
>  #ifdef CONFIG_TPMFRONT
>  	case FTYPE_TPMFRONT:
>              shutdown_tpmfront(files[fd].dev);
> -	    files[fd].type = FTYPE_NONE;
> -	    return 0;
> +            break;
>  #endif
>  #ifdef CONFIG_TPM_TIS
>  	case FTYPE_TPM_TIS:
>              shutdown_tpm_tis(files[fd].dev);
> -	    files[fd].type = FTYPE_NONE;
> -	    return 0;
> +            break;
>  #endif
>  #ifdef CONFIG_KBDFRONT
>  	case FTYPE_KBD:
>              shutdown_kbdfront(files[fd].dev);
> -            files[fd].type = FTYPE_NONE;
> -            return 0;
> +            break;
>  #endif
>  #ifdef CONFIG_FBFRONT
>  	case FTYPE_FB:
>              shutdown_fbfront(files[fd].dev);
> -            files[fd].type = FTYPE_NONE;
> -            return 0;
> +            break;
>  #endif
>  #ifdef CONFIG_CONSFRONT
>          case FTYPE_SAVEFILE:
>          case FTYPE_CONSOLE:
>              fini_consfront(files[fd].dev);
> -            files[fd].type = FTYPE_NONE;
> -            return 0;
> +            break;
>  #endif
>  	case FTYPE_NONE:
> -	    break;
> +            goto error;
>      }
> +
> +    memset(files + fd, 0, sizeof(struct file));
> +    BUILD_BUG_ON(FTYPE_NONE != 0);
> +
> +    return res;
> +
> + error:
>      printk("close(%d): Bad descriptor\n", fd);
>      errno = EBADF;
>      return -1;
> diff --git a/lib/xs.c b/lib/xs.c
> index 0459f52f..4af0f960 100644
> --- a/lib/xs.c
> +++ b/lib/xs.c
> @@ -35,7 +35,6 @@ void xs_daemon_close(struct xs_handle *h)
>          next = event->next;
>          free(event);
>      }
> -    files[fd].type = FTYPE_NONE;
>  }
>  
>  int xs_fileno(struct xs_handle *h)
> -- 
> 2.26.2
> 

-- 
Samuel
/*
 * [...] Note that 120 sec is defined in the protocol as the maximum
 * possible RTT.  I guess we'll have to use something other than TCP
 * to talk to the University of Mars.
 * PAWS allows us longer timeouts and large windows, so once implemented
 * ftp to mars will work nicely.
 */
(from /usr/src/linux/net/inet/tcp.c, concerning RTT [retransmission timeout])


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

* Re: [MINIOS PATCH v3 3/5] remove file type FTYPE_XC
  2022-01-16  6:45 ` [MINIOS PATCH v3 3/5] remove file type FTYPE_XC Juergen Gross
@ 2022-01-16 20:34   ` Samuel Thibault
  0 siblings, 0 replies; 11+ messages in thread
From: Samuel Thibault @ 2022-01-16 20:34 UTC (permalink / raw)
  To: Juergen Gross; +Cc: minios-devel, xen-devel, wl

Juergen Gross, le dim. 16 janv. 2022 07:45:25 +0100, a ecrit:
> The only reason for the file type FTYPE_XC has been gone long time
> ago: it was needed for xc_map_foreign_bulk(), which has been switched
> to use libxenforeignmemory and doesn't need this special file any
> more.
> 
> So remove everything related to FTYPE_XC.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

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

> ---
> V2:
> - new patch
> V3:
> - moved patch in series (Andrew Cooper)
> ---
>  Config.mk                     |  1 -
>  arch/x86/testbuild/all-no     |  1 -
>  arch/x86/testbuild/all-yes    |  1 -
>  arch/x86/testbuild/newxen-yes |  1 -
>  include/lib.h                 |  1 -
>  lib/sys.c                     | 13 -------------
>  6 files changed, 18 deletions(-)
> 
> diff --git a/Config.mk b/Config.mk
> index 5e660891..1e083881 100644
> --- a/Config.mk
> +++ b/Config.mk
> @@ -197,7 +197,6 @@ CONFIG-n += CONFIG_PARAVIRT
>  endif
>  # Support legacy CONFIG_XC value
>  CONFIG_XC ?= $(libc)
> -CONFIG-$(CONFIG_XC) += CONFIG_LIBXENCTRL
>  CONFIG-$(CONFIG_XC) += CONFIG_LIBXENEVTCHN
>  CONFIG-$(CONFIG_XC) += CONFIG_LIBXENGNTTAB
>  
> diff --git a/arch/x86/testbuild/all-no b/arch/x86/testbuild/all-no
> index 7972ecd5..d6fc2608 100644
> --- a/arch/x86/testbuild/all-no
> +++ b/arch/x86/testbuild/all-no
> @@ -13,7 +13,6 @@ CONFIG_FBFRONT = n
>  CONFIG_KBDFRONT = n
>  CONFIG_CONSFRONT = n
>  CONFIG_XENBUS = n
> -CONFIG_LIBXENCTRL = n
>  CONFIG_LIBXENEVTCHN = n
>  CONFIG_LIBXENGNTTAB = n
>  CONFIG_LWIP = n
> diff --git a/arch/x86/testbuild/all-yes b/arch/x86/testbuild/all-yes
> index bc8eea57..98bbfebf 100644
> --- a/arch/x86/testbuild/all-yes
> +++ b/arch/x86/testbuild/all-yes
> @@ -16,7 +16,6 @@ CONFIG_XENBUS = y
>  CONFIG_BALLOON = y
>  CONFIG_USE_XEN_CONSOLE = y
>  # The following are special: they need support from outside
> -CONFIG_LIBXENCTRL = n
>  CONFIG_LIBXENEVTCHN = n
>  CONFIG_LIBXENGNTTAB = n
>  CONFIG_LWIP = n
> diff --git a/arch/x86/testbuild/newxen-yes b/arch/x86/testbuild/newxen-yes
> index f72123b5..06032004 100644
> --- a/arch/x86/testbuild/newxen-yes
> +++ b/arch/x86/testbuild/newxen-yes
> @@ -17,7 +17,6 @@ CONFIG_BALLOON = y
>  CONFIG_USE_XEN_CONSOLE = y
>  XEN_INTERFACE_VERSION=__XEN_LATEST_INTERFACE_VERSION__
>  # The following are special: they need support from outside
> -CONFIG_LIBXENCTRL = n
>  CONFIG_LIBXENEVTCHN = n
>  CONFIG_LIBXENGNTTAB = n
>  CONFIG_LWIP = n
> diff --git a/include/lib.h b/include/lib.h
> index 7a0546bd..7ca90768 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -159,7 +159,6 @@ enum fd_type {
>      FTYPE_CONSOLE,
>      FTYPE_FILE,
>      FTYPE_XENBUS,
> -    FTYPE_XC,
>      FTYPE_EVTCHN,
>      FTYPE_GNTMAP,
>      FTYPE_SOCKET,
> diff --git a/lib/sys.c b/lib/sys.c
> index 7df77cc6..a24687b7 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -87,7 +87,6 @@
>      }
>  
>  #define NOFILE 32
> -extern void minios_interface_close_fd(int fd);
>  extern void minios_evtchn_close_fd(int fd);
>  extern void minios_gnttab_close_fd(int fd);
>  
> @@ -443,11 +442,6 @@ int close(int fd)
>              res = lwip_close(files[fd].fd);
>              break;
>  #endif
> -#ifdef CONFIG_LIBXENCTRL
> -	case FTYPE_XC:
> -	    minios_interface_close_fd(fd);
> -            break;
> -#endif
>  #ifdef CONFIG_LIBXENEVTCHN
>  	case FTYPE_EVTCHN:
>  	    minios_evtchn_close_fd(fd);
> @@ -651,7 +645,6 @@ static const char file_types[] = {
>      [FTYPE_NONE]	= 'N',
>      [FTYPE_CONSOLE]	= 'C',
>      [FTYPE_XENBUS]	= 'S',
> -    [FTYPE_XC]		= 'X',
>      [FTYPE_EVTCHN]	= 'E',
>      [FTYPE_SOCKET]	= 's',
>      [FTYPE_TAP]		= 'T',
> @@ -1383,12 +1376,6 @@ void *mmap(void *start, size_t length, int prot, int flags, int fd, off_t offset
>  
>      if (fd == -1)
>          return map_zero(n, 1);
> -#ifdef CONFIG_LIBXENCTRL
> -    else if (files[fd].type == FTYPE_XC) {
> -        unsigned long zero = 0;
> -        return map_frames_ex(&zero, n, 0, 0, 1, DOMID_SELF, NULL, 0);
> -    }
> -#endif
>      else if (files[fd].type == FTYPE_MEM) {
>          unsigned long first_mfn = offset >> PAGE_SHIFT;
>          return map_frames_ex(&first_mfn, n, 0, 1, 1, DOMID_IO, NULL, _PAGE_PRESENT|_PAGE_RW);
> -- 
> 2.26.2
> 

-- 
Samuel
$ du temp.iso 
2,0T    temp.iso
$ ls temp.iso -l
-r-xr-xr-x    1 samy     thibault      16E 2003-03-22 14:44 temp.iso*
 -+- je vous dirai pas la marque de mon disque dur, na :p -+- 


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

* Re: [MINIOS PATCH v3 4/5] use function vectors instead of switch for file operations
  2022-01-16  6:45 ` [MINIOS PATCH v3 4/5] use function vectors instead of switch for file operations Juergen Gross
@ 2022-01-16 20:39   ` Samuel Thibault
  0 siblings, 0 replies; 11+ messages in thread
From: Samuel Thibault @ 2022-01-16 20:39 UTC (permalink / raw)
  To: Juergen Gross; +Cc: minios-devel, xen-devel, wl

Juergen Gross, le dim. 16 janv. 2022 07:45:26 +0100, a ecrit:
> Introduce file type specific function vectors for all the needed file
> operations which are file type specific in order to prepare replacing
> the large switch statements in each generic file function.
> 
> Add a function to allocate a new file type dynamically in order to
> prepare removing direct dependencies to external components in the
> future. For this reason switch the file type from an enum to an
> unsigned int. Prepare removal of some statically defines file types
> by putting them at the end of the defined list.
> 
> Change the debug output for the file type from a single character to
> a string in order to support a future removal of the file_types[]
> array.
> 
> Provide some functions useful for file_ops in future patches.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

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

> ---
> V2:
> - new patch
> V3:
> - drop typedefs for callbacks (Andrew Cooper)
> - const (Andrew Cooper)
> - drop loop from alloc_file_type() (Andrew Cooper)
> - switch to struct file * as first parameter of callbacks (Andrew Cooper)
> ---
>  include/lib.h |  57 +++++++----
>  lib/sys.c     | 268 ++++++++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 263 insertions(+), 62 deletions(-)
> 
> diff --git a/include/lib.h b/include/lib.h
> index 7ca90768..44696806 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -66,6 +66,7 @@
>  
>  #ifdef HAVE_LIBC
>  #include <sys/queue.h>
> +#include <sys/stat.h>
>  #include <stdio.h>
>  #include <string.h>
>  #else
> @@ -154,23 +155,23 @@ do {                                                           \
>  void sanity_check(void);
>  
>  #ifdef HAVE_LIBC
> -enum fd_type {
> -    FTYPE_NONE = 0,
> -    FTYPE_CONSOLE,
> -    FTYPE_FILE,
> -    FTYPE_XENBUS,
> -    FTYPE_EVTCHN,
> -    FTYPE_GNTMAP,
> -    FTYPE_SOCKET,
> -    FTYPE_TAP,
> -    FTYPE_BLK,
> -    FTYPE_KBD,
> -    FTYPE_FB,
> -    FTYPE_MEM,
> -    FTYPE_SAVEFILE,
> -    FTYPE_TPMFRONT,
> -    FTYPE_TPM_TIS,
> -};
> +#define FTYPE_NONE       0
> +#define FTYPE_CONSOLE    1
> +#define FTYPE_FILE       2
> +#define FTYPE_SOCKET     3
> +#define FTYPE_MEM        4
> +#define FTYPE_SAVEFILE   5
> +#define FTYPE_FB         6
> +#define FTYPE_KBD        7
> +#define FTYPE_TAP        8
> +#define FTYPE_BLK        9
> +#define FTYPE_TPMFRONT  10
> +#define FTYPE_TPM_TIS   11
> +#define FTYPE_XENBUS    12
> +#define FTYPE_GNTMAP    13
> +#define FTYPE_EVTCHN    14
> +#define FTYPE_N         15
> +#define FTYPE_SPARE     16
>  
>  LIST_HEAD(evtchn_port_list, evtchn_port_info);
>  
> @@ -182,7 +183,7 @@ struct evtchn_port_info {
>  };
>  
>  struct file {
> -    enum fd_type type;
> +    unsigned int type;
>      bool read;	/* maybe available for read */
>      off_t offset;
>      union {
> @@ -197,8 +198,26 @@ struct file {
>  
>  extern struct file files[];
>  
> +struct file_ops {
> +    const char *name;
> +    int (*read)(struct file *file, void *buf, size_t nbytes);
> +    int (*write)(struct file *file, const void *buf, size_t nbytes);
> +    off_t (*lseek)(struct file *file, off_t offset, int whence);
> +    int (*close)(struct file *file);
> +    int (*fstat)(struct file *file, struct stat *buf);
> +    int (*fcntl)(struct file *file, int cmd, va_list args);
> +    bool (*select_rd)(struct file *file);
> +    bool (*select_wr)(struct file *file);
> +};
> +
> +unsigned int alloc_file_type(const struct file_ops *ops);
> +
> +off_t lseek_default(struct file *file, off_t offset, int whence);
> +bool select_yes(struct file *file);
> +bool select_read_flag(struct file *file);
> +
>  struct file *get_file_from_fd(int fd);
> -int alloc_fd(enum fd_type type);
> +int alloc_fd(unsigned int type);
>  void close_all_files(void);
>  extern struct thread *main_thread;
>  void sparse(unsigned long data, size_t size);
> diff --git a/lib/sys.c b/lib/sys.c
> index a24687b7..7be01fd3 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -97,6 +97,40 @@ struct file files[NOFILE] = {
>      { .type = FTYPE_CONSOLE }, /* stderr */
>  };
>  
> +static const struct file_ops file_ops_none = {
> +    .name = "none",
> +};
> +
> +static const struct file_ops *file_ops[FTYPE_N + FTYPE_SPARE] = {
> +    [FTYPE_NONE] = &file_ops_none,
> +};
> +
> +unsigned int alloc_file_type(const struct file_ops *ops)
> +{
> +    static unsigned int i = FTYPE_N;
> +    unsigned int ret;
> +
> +    pthread_mutex_lock(&fd_lock);
> +
> +    BUG_ON(i == ARRAY_SIZE(file_ops));
> +    ret = i++;
> +    file_ops[ret] = ops;
> +
> +    pthread_mutex_unlock(&fd_lock);
> +
> +    printk("New file type \"%s\"(%u) allocated\n", ops->name, ret);
> +
> +    return ret;
> +}
> +
> +static const struct file_ops *get_file_ops(unsigned int type)
> +{
> +    if ( type >= ARRAY_SIZE(file_ops) || !file_ops[type] )
> +        return &file_ops_none;
> +
> +    return file_ops[type];
> +}
> +
>  struct file *get_file_from_fd(int fd)
>  {
>      if ( fd < 0 || fd >= ARRAY_SIZE(files) )
> @@ -107,7 +141,7 @@ struct file *get_file_from_fd(int fd)
>  
>  DECLARE_WAIT_QUEUE_HEAD(event_queue);
>  
> -int alloc_fd(enum fd_type type)
> +int alloc_fd(unsigned int type)
>  {
>      int i;
>      pthread_mutex_lock(&fd_lock);
> @@ -248,7 +282,17 @@ int isatty(int fd)
>  
>  int read(int fd, void *buf, size_t nbytes)
>  {
> -    switch (files[fd].type) {
> +    struct file *file = get_file_from_fd(fd);
> +    const struct file_ops *ops;
> +
> +    if ( !file )
> +        goto error;
> +
> +    ops = get_file_ops(file->type);
> +    if ( ops->read )
> +        return ops->read(file, buf, nbytes);
> +
> +    switch (file->type) {
>          case FTYPE_SAVEFILE:
>  	case FTYPE_CONSOLE: {
>  	    int ret;
> @@ -320,6 +364,8 @@ int read(int fd, void *buf, size_t nbytes)
>  	default:
>  	    break;
>      }
> +
> + error:
>      printk("read(%d): Bad descriptor\n", fd);
>      errno = EBADF;
>      return -1;
> @@ -327,7 +373,17 @@ int read(int fd, void *buf, size_t nbytes)
>  
>  int write(int fd, const void *buf, size_t nbytes)
>  {
> -    switch (files[fd].type) {
> +    struct file *file = get_file_from_fd(fd);
> +    const struct file_ops *ops;
> +
> +    if ( !file )
> +        goto error;
> +
> +    ops = get_file_ops(file->type);
> +    if ( ops->write )
> +        return ops->write(file, buf, nbytes);
> +
> +    switch (file->type) {
>          case FTYPE_SAVEFILE: {
>                  int ret = 0, tot = nbytes;
>                  while (nbytes > 0) {
> @@ -364,14 +420,61 @@ int write(int fd, const void *buf, size_t nbytes)
>  	default:
>  	    break;
>      }
> +
> + error:
>      printk("write(%d): Bad descriptor\n", fd);
>      errno = EBADF;
>      return -1;
>  }
>  
> +off_t lseek_default(struct file *file, off_t offset, int whence)
> +{
> +    switch ( whence )
> +    {
> +    case SEEK_SET:
> +        file->offset = offset;
> +        break;
> +
> +    case SEEK_CUR:
> +        file->offset += offset;
> +        break;
> +
> +    case SEEK_END:
> +    {
> +        struct stat st;
> +        int ret;
> +
> +        ret = fstat(file - files, &st);
> +        if ( ret )
> +            return -1;
> +        file->offset = st.st_size + offset;
> +        break;
> +    }
> +
> +    default:
> +        errno = EINVAL;
> +        return -1;
> +    }
> +
> +    return file->offset;
> +}
> +
>  off_t lseek(int fd, off_t offset, int whence)
>  {
> -    switch(files[fd].type) {
> +    struct file *file = get_file_from_fd(fd);
> +    const struct file_ops *ops;
> +
> +    if ( !file )
> +    {
> +        errno = EBADF;
> +        return (off_t)-1;
> +    }
> +
> +    ops = get_file_ops(file->type);
> +    if ( ops->lseek )
> +        return ops->lseek(file, offset, whence);
> +
> +    switch(file->type) {
>  #ifdef CONFIG_BLKFRONT
>         case FTYPE_BLK:
>            break;
> @@ -392,28 +495,7 @@ off_t lseek(int fd, off_t offset, int whence)
>            return (off_t) -1;
>      }
>  
> -    switch (whence) {
> -       case SEEK_SET:
> -          files[fd].offset = offset;
> -          break;
> -       case SEEK_CUR:
> -          files[fd].offset += offset;
> -          break;
> -       case SEEK_END:
> -          {
> -             struct stat st;
> -             int ret;
> -             ret = fstat(fd, &st);
> -             if (ret)
> -                return -1;
> -             files[fd].offset = st.st_size + offset;
> -             break;
> -          }
> -       default:
> -          errno = EINVAL;
> -          return -1;
> -    }
> -    return files[fd].offset;
> +    return lseek_default(file, offset, whence);
>  }
>  
>  int fsync(int fd) {
> @@ -424,12 +506,21 @@ int fsync(int fd) {
>  int close(int fd)
>  {
>      int res = 0;
> +    struct file *file = get_file_from_fd(fd);
> +    const struct file_ops *ops;
>  
> -    if ( fd < 0 || fd >= ARRAY_SIZE(files) )
> +    if ( !file )
>          goto error;
>  
> +    ops = get_file_ops(file->type);
>      printk("close(%d)\n", fd);
> -    switch (files[fd].type) {
> +    if ( ops->close )
> +    {
> +        res = ops->close(file);
> +        goto out;
> +    }
> +
> +    switch (file->type) {
>          default:
>              break;
>  #ifdef CONFIG_XENBUS
> @@ -492,6 +583,7 @@ int close(int fd)
>              goto error;
>      }
>  
> + out:
>      memset(files + fd, 0, sizeof(struct file));
>      BUILD_BUG_ON(FTYPE_NONE != 0);
>  
> @@ -522,8 +614,19 @@ int stat(const char *path, struct stat *buf)
>  
>  int fstat(int fd, struct stat *buf)
>  {
> +    struct file *file = get_file_from_fd(fd);
> +    const struct file_ops *ops;
> +
> +    if ( !file )
> +        goto error;
> +
>      init_stat(buf);
> -    switch (files[fd].type) {
> +
> +    ops = get_file_ops(file->type);
> +    if ( ops->fstat )
> +        return ops->fstat(file, buf);
> +
> +    switch (file->type) {
>  	case FTYPE_SAVEFILE:
>  	case FTYPE_CONSOLE:
>  	case FTYPE_SOCKET: {
> @@ -557,6 +660,7 @@ int fstat(int fd, struct stat *buf)
>  	    break;
>      }
>  
> + error:
>      printk("statf(%d): Bad descriptor\n", fd);
>      errno = EBADF;
>      return -1;
> @@ -588,6 +692,27 @@ int fcntl(int fd, int cmd, ...)
>  {
>      long arg;
>      va_list ap;
> +    int res;
> +    struct file *file = get_file_from_fd(fd);
> +    const struct file_ops *ops;
> +
> +    if ( !file )
> +    {
> +        errno = EBADF;
> +        return -1;
> +    }
> +
> +    ops = get_file_ops(files[fd].type);
> +
> +    if ( ops->fcntl )
> +    {
> +        va_start(ap, cmd);
> +        res = ops->fcntl(file, cmd, ap);
> +        va_end(ap);
> +
> +        return res;
> +    }
> +
>      va_start(ap, cmd);
>      arg = va_arg(ap, long);
>      va_end(ap);
> @@ -641,17 +766,28 @@ int closedir(DIR *dir)
>  /* We assume that only the main thread calls select(). */
>  
>  #if defined(LIBC_DEBUG) || defined(LIBC_VERBOSE)
> -static const char file_types[] = {
> -    [FTYPE_NONE]	= 'N',
> -    [FTYPE_CONSOLE]	= 'C',
> -    [FTYPE_XENBUS]	= 'S',
> -    [FTYPE_EVTCHN]	= 'E',
> -    [FTYPE_SOCKET]	= 's',
> -    [FTYPE_TAP]		= 'T',
> -    [FTYPE_BLK]		= 'B',
> -    [FTYPE_KBD]		= 'K',
> -    [FTYPE_FB]		= 'G',
> +static const char *const file_types[] = {
> +    [FTYPE_NONE]    = "none",
> +    [FTYPE_CONSOLE] = "console",
> +    [FTYPE_XENBUS]  = "xenbus",
> +    [FTYPE_EVTCHN]  = "evtchn",
> +    [FTYPE_SOCKET]  = "socket",
> +    [FTYPE_TAP]     = "net",
> +    [FTYPE_BLK]     = "blk",
> +    [FTYPE_KBD]     = "kbd",
> +    [FTYPE_FB]      = "fb",
>  };
> +
> +static const char *get_type_name(unsigned int type)
> +{
> +    if ( type < ARRAY_SIZE(file_ops) && file_ops[type] )
> +        return file_ops[type]->name;
> +
> +    if ( type < ARRAY_SIZE(file_types) && file_types[type] )
> +        return file_types[type];
> +
> +    return "none";
> +}
>  #endif
>  #ifdef LIBC_DEBUG
>  static void dump_set(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, struct timeval *timeout)
> @@ -663,7 +799,7 @@ static void dump_set(int nfds, fd_set *readfds, fd_set *writefds, fd_set *except
>  	if (FD_ISSET(i, set)) { \
>  	    if (comma) \
>  		printk(", "); \
> -	    printk("%d(%c)", i, file_types[files[i].type]); \
> +            printk("%d(%s)", i, get_type_name(files[i].type)); \
>  	    comma = 1; \
>  	} \
>      } \
> @@ -697,7 +833,7 @@ static void dump_pollfds(struct pollfd *pfd, int nfds, int timeout)
>          fd = pfd[i].fd;
>          if (comma)
>              printk(", ");
> -        printk("%d(%c)/%02x", fd, file_types[files[fd].type],
> +        printk("%d(%s)/%02x", fd, get_type_name(files[fd].type),
>              pfd[i].events);
>              comma = 1;
>      }
> @@ -709,6 +845,16 @@ static void dump_pollfds(struct pollfd *pfd, int nfds, int timeout)
>  #define dump_pollfds(pfds, nfds, timeout)
>  #endif
>  
> +bool select_yes(struct file *file)
> +{
> +    return true;
> +}
> +
> +bool select_read_flag(struct file *file)
> +{
> +    return file->read;
> +}
> +
>  /* Just poll without blocking */
>  static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds)
>  {
> @@ -760,11 +906,47 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
>  
>      /* Then see others as well. */
>      for (i = 0; i < nfds; i++) {
> -	switch(files[i].type) {
> +        struct file *file = get_file_from_fd(i);
> +
> +        if ( !file )
> +        {
> +            FD_CLR(i, readfds);
> +            FD_CLR(i, writefds);
> +            FD_CLR(i, exceptfds);
> +            continue;
> +        }
> +
> +        switch(file->type) {
>  	default:
> +        {
> +            const struct file_ops *ops = file_ops[file->type];
> +
> +            if ( ops )
> +            {
> +                if ( FD_ISSET(i, readfds) )
> +                {
> +                    if ( ops->select_rd && ops->select_rd(file) )
> +                        n++;
> +                    else
> +                        FD_CLR(i, readfds);
> +                }
> +                if ( FD_ISSET(i, writefds) )
> +                {
> +                    if ( ops->select_wr && ops->select_wr(file) )
> +                        n++;
> +                    else
> +                        FD_CLR(i, writefds);
> +                }
> +                FD_CLR(i, exceptfds);
> +
> +                break;
> +            }
> +
>  	    if (FD_ISSET(i, readfds) || FD_ISSET(i, writefds) || FD_ISSET(i, exceptfds))
>  		printk("bogus fd %d in select\n", i);
>  	    /* Fallthrough.  */
> +        }
> +
>  	case FTYPE_CONSOLE:
>  	    if (FD_ISSET(i, readfds)) {
>                  if (xencons_ring_avail(files[i].dev))
> @@ -842,7 +1024,7 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
>  	printk("%d(%d): ", nb, sock_n);
>  	for (i = 0; i < nfds; i++) {
>  	    if (nbread[i] || nbwrite[i] || nbexcept[i])
> -		printk(" %d(%c):", i, file_types[files[i].type]);
> +                printk(" %d(%c):", i, get_type_name(files[i].type));
>  	    if (nbread[i])
>  	    	printk(" %dR", nbread[i]);
>  	    if (nbwrite[i])
> -- 
> 2.26.2
> 

-- 
Samuel
* D a decide de peter un cable dans son rapport de pfp
<c> et il a bien raison ;-)
<c> tu vas dire quoi ?
<D> j'ai mis les paroles de "le coq est mort" en en-tete
 -+- #ens-mim et la peufeupeu -+-


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

* Re: [MINIOS PATCH v3 5/5] add CONFIG_LIBXS item
  2022-01-16  6:45 ` [MINIOS PATCH v3 5/5] add CONFIG_LIBXS item Juergen Gross
@ 2022-01-16 20:40   ` Samuel Thibault
  0 siblings, 0 replies; 11+ messages in thread
From: Samuel Thibault @ 2022-01-16 20:40 UTC (permalink / raw)
  To: Juergen Gross; +Cc: minios-devel, xen-devel, wl

Juergen Gross, le dim. 16 janv. 2022 07:45:27 +0100, a ecrit:
> Mini-OS contains a stripped down version of libxenstore in lib/xs.c.
> Today it is being built always if CONFIG_XENBUS is set and libc
> support is enabled.
> 
> In order to allow a Mini-OS specific build of libxenstore to be used
> instead add a new CONFIG_LIBXS item which per default will have the
> same setting as CONFIG_XENBUS. A user wanting to replace lib/xs.c
> with libxenstore would just need to set CONFIG_XENBUS=y and
> CONFIG_LIBXS=n.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

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

> ---
> V3:
> - new patch
> ---
>  Config.mk                     | 10 ++++++++--
>  Makefile                      |  2 +-
>  arch/x86/testbuild/all-no     |  1 +
>  arch/x86/testbuild/all-yes    |  1 +
>  arch/x86/testbuild/newxen-yes |  1 +
>  lib/sys.c                     |  4 ++--
>  6 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/Config.mk b/Config.mk
> index 1e083881..03023033 100644
> --- a/Config.mk
> +++ b/Config.mk
> @@ -171,7 +171,10 @@ endif
>  # arch/*/testbuild/*-yes and arch/*/testbuild/*-no should set ALL possible
>  # CONFIG_ variables.
>  
> -# Configuration defaults
> +# Configuration defaults:
> +# CONFIG-y contains all items defaulting to "y"
> +# CONFIG-n contains all items defaulting to "n"
> +# CONFIG-x contains all items being calculated if not set explicitly
>  CONFIG-y += CONFIG_START_NETWORK
>  CONFIG-y += CONFIG_SPARSE_BSS
>  CONFIG-y += CONFIG_BLKFRONT
> @@ -205,7 +208,10 @@ CONFIG-$(lwip) += CONFIG_LWIP
>  $(foreach i,$(CONFIG-y),$(eval $(i) ?= y))
>  $(foreach i,$(CONFIG-n),$(eval $(i) ?= n))
>  
> -CONFIG-all := $(CONFIG-y) $(CONFIG-n)
> +CONFIG-x += CONFIG_LIBXS
> +CONFIG_LIBXS ?= $(CONFIG_XENBUS)
> +
> +CONFIG-all := $(CONFIG-y) $(CONFIG-n) $(CONFIG-x)
>  
>  # Export config items as compiler directives
>  $(foreach i,$(CONFIG-all),$(eval DEFINES-$($(i)) += -D$(i)))
> diff --git a/Makefile b/Makefile
> index 06b60fc7..9f95d197 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -65,7 +65,7 @@ src-y += lib/stack_chk_fail.c
>  src-y += lib/string.c
>  src-y += lib/sys.c
>  src-y += lib/xmalloc.c
> -src-$(CONFIG_XENBUS) += lib/xs.c
> +src-$(CONFIG_LIBXS) += lib/xs.c
>  
>  src-$(CONFIG_XENBUS) += xenbus/xenbus.c
>  
> diff --git a/arch/x86/testbuild/all-no b/arch/x86/testbuild/all-no
> index d6fc2608..46f974de 100644
> --- a/arch/x86/testbuild/all-no
> +++ b/arch/x86/testbuild/all-no
> @@ -13,6 +13,7 @@ CONFIG_FBFRONT = n
>  CONFIG_KBDFRONT = n
>  CONFIG_CONSFRONT = n
>  CONFIG_XENBUS = n
> +CONFIG_LIBXS = n
>  CONFIG_LIBXENEVTCHN = n
>  CONFIG_LIBXENGNTTAB = n
>  CONFIG_LWIP = n
> diff --git a/arch/x86/testbuild/all-yes b/arch/x86/testbuild/all-yes
> index 98bbfebf..3ead12f1 100644
> --- a/arch/x86/testbuild/all-yes
> +++ b/arch/x86/testbuild/all-yes
> @@ -13,6 +13,7 @@ CONFIG_FBFRONT = y
>  CONFIG_KBDFRONT = y
>  CONFIG_CONSFRONT = y
>  CONFIG_XENBUS = y
> +CONFIG_LIBXS = y
>  CONFIG_BALLOON = y
>  CONFIG_USE_XEN_CONSOLE = y
>  # The following are special: they need support from outside
> diff --git a/arch/x86/testbuild/newxen-yes b/arch/x86/testbuild/newxen-yes
> index 06032004..5c0b3c80 100644
> --- a/arch/x86/testbuild/newxen-yes
> +++ b/arch/x86/testbuild/newxen-yes
> @@ -13,6 +13,7 @@ CONFIG_FBFRONT = y
>  CONFIG_KBDFRONT = y
>  CONFIG_CONSFRONT = y
>  CONFIG_XENBUS = y
> +CONFIG_LIBXS = y
>  CONFIG_BALLOON = y
>  CONFIG_USE_XEN_CONSOLE = y
>  XEN_INTERFACE_VERSION=__XEN_LATEST_INTERFACE_VERSION__
> diff --git a/lib/sys.c b/lib/sys.c
> index 7be01fd3..e0ac5099 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -523,7 +523,7 @@ int close(int fd)
>      switch (file->type) {
>          default:
>              break;
> -#ifdef CONFIG_XENBUS
> +#ifdef CONFIG_LIBXS
>  	case FTYPE_XENBUS:
>              xs_daemon_close((void*)(intptr_t) fd);
>              break;
> @@ -958,7 +958,7 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
>                  n++;
>  	    FD_CLR(i, exceptfds);
>  	    break;
> -#ifdef CONFIG_XENBUS
> +#ifdef CONFIG_LIBXS
>  	case FTYPE_XENBUS:
>  	    if (FD_ISSET(i, readfds)) {
>                  if (files[i].dev)
> -- 
> 2.26.2
> 

-- 
Samuel
<s> bah, j'aime bien les feux d'artifices, mais j'ai peur de me prendre un boeing sur le coin de la gueule si je vais sur le pont de brooklyn
 -+- #ens-mim - 11 septembre forever -+-


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

* Re: [MINIOS PATCH v3 0/5] mini-os: remove struct file dependency on config
  2022-01-16  6:45 [MINIOS PATCH v3 0/5] mini-os: remove struct file dependency on config Juergen Gross
                   ` (4 preceding siblings ...)
  2022-01-16  6:45 ` [MINIOS PATCH v3 5/5] add CONFIG_LIBXS item Juergen Gross
@ 2022-01-16 20:41 ` Samuel Thibault
  5 siblings, 0 replies; 11+ messages in thread
From: Samuel Thibault @ 2022-01-16 20:41 UTC (permalink / raw)
  To: Juergen Gross; +Cc: minios-devel, xen-devel, wl

Juergen Gross, le dim. 16 janv. 2022 07:45:22 +0100, a ecrit:
> Today the layout of struct file is depending on the Mini-OS
> configuration. This is especially bad as the layout is exported to
> external users like the Xen libraries built for Mini-OS, and those
> are being built only once for multiple stubdom configurations.
> 
> Today there is no direct problem resulting from this, as the main
> difference between struct file layouts is a large union containing all
> the device specific data for the different file types. The largest
> union member is not configuration dependant, so the build is currently
> not broken.
> 
> In order to avoid any future problems this patch series is eliminating
> the configuration dependency by replacing most of the device specific
> union members by a single pointer.
> 
> The two union members used by Xen libraries can't be replaced yet, as
> those need to be switched to use the generic pointer first.
> 
> In order to hide the Mini-OS internal implementation of the files
> array, patches 1, 2, and  4 are introducing a common framework to
> access a struct file via its file descriptor, and to allocate new file
> types dynamically instead of having them all pre-defined. The file type
> specific operations are supplied via a function vector in order to
> remove the dependency of lib/sys.c on all the various file types.
> 
> Patch 5 is preparing a possible future support of libxenstore instead
> of using the Mini-OS internal variant located in lib/xs.c.

Thanks!

Samuel


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

end of thread, other threads:[~2022-01-16 20:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-16  6:45 [MINIOS PATCH v3 0/5] mini-os: remove struct file dependency on config Juergen Gross
2022-01-16  6:45 ` [MINIOS PATCH v3 1/5] introduce get_file_from_fd() Juergen Gross
2022-01-16  6:45 ` [MINIOS PATCH v3 2/5] reset file type in close() in one place only Juergen Gross
2022-01-16 20:33   ` Samuel Thibault
2022-01-16  6:45 ` [MINIOS PATCH v3 3/5] remove file type FTYPE_XC Juergen Gross
2022-01-16 20:34   ` Samuel Thibault
2022-01-16  6:45 ` [MINIOS PATCH v3 4/5] use function vectors instead of switch for file operations Juergen Gross
2022-01-16 20:39   ` Samuel Thibault
2022-01-16  6:45 ` [MINIOS PATCH v3 5/5] add CONFIG_LIBXS item Juergen Gross
2022-01-16 20:40   ` Samuel Thibault
2022-01-16 20:41 ` [MINIOS PATCH v3 0/5] mini-os: remove struct file dependency on config Samuel Thibault

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.