* [MINIOS PATCH v3 01/12] remove event channel specific struct file definitions
2022-01-16 8:33 [MINIOS PATCH v3 00/12] remove device specific struct file members Juergen Gross
@ 2022-01-16 8:33 ` Juergen Gross
2022-01-16 20:53 ` Samuel Thibault
2022-01-16 8:33 ` [MINIOS PATCH v3 02/12] remove gnttab specific member from struct file Juergen Gross
` (11 subsequent siblings)
12 siblings, 1 reply; 29+ messages in thread
From: Juergen Gross @ 2022-01-16 8:33 UTC (permalink / raw)
To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross
The event channel specific union member in struct file is no longer
needed, so remove it together with the associated structure
definitions.
The event channel file type and its associated handling can be removed,
too, as libxenevtchn is now supplying a struct file_ops via a call of
alloc_file_type().
This removes all contents of CONFIG_LIBXENEVTCHN guarded sections, so
this config option can be removed.
Add an extern declaration for event_queue as it is used by
libxenevtchn.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- add extern declaration for event_queue (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 | 17 +++--------------
lib/sys.c | 7 -------
6 files changed, 3 insertions(+), 25 deletions(-)
diff --git a/Config.mk b/Config.mk
index 03023033..52eb26d8 100644
--- a/Config.mk
+++ b/Config.mk
@@ -200,7 +200,6 @@ CONFIG-n += CONFIG_PARAVIRT
endif
# Support legacy CONFIG_XC value
CONFIG_XC ?= $(libc)
-CONFIG-$(CONFIG_XC) += CONFIG_LIBXENEVTCHN
CONFIG-$(CONFIG_XC) += CONFIG_LIBXENGNTTAB
CONFIG-$(lwip) += CONFIG_LWIP
diff --git a/arch/x86/testbuild/all-no b/arch/x86/testbuild/all-no
index 46f974de..15c954ff 100644
--- a/arch/x86/testbuild/all-no
+++ b/arch/x86/testbuild/all-no
@@ -14,7 +14,6 @@ CONFIG_KBDFRONT = n
CONFIG_CONSFRONT = n
CONFIG_XENBUS = n
CONFIG_LIBXS = n
-CONFIG_LIBXENEVTCHN = n
CONFIG_LIBXENGNTTAB = n
CONFIG_LWIP = n
CONFIG_BALLOON = n
diff --git a/arch/x86/testbuild/all-yes b/arch/x86/testbuild/all-yes
index 3ead12f1..9964d42f 100644
--- a/arch/x86/testbuild/all-yes
+++ b/arch/x86/testbuild/all-yes
@@ -17,6 +17,5 @@ CONFIG_LIBXS = y
CONFIG_BALLOON = y
CONFIG_USE_XEN_CONSOLE = y
# The following are special: they need support from outside
-CONFIG_LIBXENEVTCHN = n
CONFIG_LIBXENGNTTAB = n
CONFIG_LWIP = n
diff --git a/arch/x86/testbuild/newxen-yes b/arch/x86/testbuild/newxen-yes
index 5c0b3c80..c2519938 100644
--- a/arch/x86/testbuild/newxen-yes
+++ b/arch/x86/testbuild/newxen-yes
@@ -18,6 +18,5 @@ 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_LIBXENEVTCHN = n
CONFIG_LIBXENGNTTAB = n
CONFIG_LWIP = n
diff --git a/include/lib.h b/include/lib.h
index 44696806..5f24be5d 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -155,6 +155,8 @@ do { \
void sanity_check(void);
#ifdef HAVE_LIBC
+extern struct wait_queue_head event_queue;
+
#define FTYPE_NONE 0
#define FTYPE_CONSOLE 1
#define FTYPE_FILE 2
@@ -169,19 +171,9 @@ void sanity_check(void);
#define FTYPE_TPM_TIS 11
#define FTYPE_XENBUS 12
#define FTYPE_GNTMAP 13
-#define FTYPE_EVTCHN 14
-#define FTYPE_N 15
+#define FTYPE_N 14
#define FTYPE_SPARE 16
-LIST_HEAD(evtchn_port_list, evtchn_port_info);
-
-struct evtchn_port_info {
- LIST_ENTRY(evtchn_port_info) list;
- evtchn_port_t port;
- unsigned long pending;
- int bound;
-};
-
struct file {
unsigned int type;
bool read; /* maybe available for read */
@@ -189,9 +181,6 @@ struct file {
union {
int fd; /* Any fd from an upper layer. */
void *dev;
- struct {
- struct evtchn_port_list ports;
- } evtchn;
struct gntmap gntmap;
};
};
diff --git a/lib/sys.c b/lib/sys.c
index e0ac5099..34f0193d 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -533,11 +533,6 @@ int close(int fd)
res = lwip_close(files[fd].fd);
break;
#endif
-#ifdef CONFIG_LIBXENEVTCHN
- case FTYPE_EVTCHN:
- minios_evtchn_close_fd(fd);
- break;
-#endif
#ifdef CONFIG_LIBXENGNTTAB
case FTYPE_GNTMAP:
minios_gnttab_close_fd(fd);
@@ -770,7 +765,6 @@ 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",
@@ -970,7 +964,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
FD_CLR(i, exceptfds);
break;
#endif
- case FTYPE_EVTCHN:
case FTYPE_TAP:
case FTYPE_BLK:
case FTYPE_KBD:
--
2.26.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 01/12] remove event channel specific struct file definitions
2022-01-16 8:33 ` [MINIOS PATCH v3 01/12] remove event channel specific struct file definitions Juergen Gross
@ 2022-01-16 20:53 ` Samuel Thibault
0 siblings, 0 replies; 29+ messages in thread
From: Samuel Thibault @ 2022-01-16 20:53 UTC (permalink / raw)
To: Juergen Gross; +Cc: minios-devel, xen-devel, wl
Juergen Gross, le dim. 16 janv. 2022 09:33:17 +0100, a ecrit:
> The event channel specific union member in struct file is no longer
> needed, so remove it together with the associated structure
> definitions.
>
> The event channel file type and its associated handling can be removed,
> too, as libxenevtchn is now supplying a struct file_ops via a call of
> alloc_file_type().
>
> This removes all contents of CONFIG_LIBXENEVTCHN guarded sections, so
> this config option can be removed.
>
> Add an extern declaration for event_queue as it is used by
> libxenevtchn.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> V3:
> - add extern declaration for event_queue (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 | 17 +++--------------
> lib/sys.c | 7 -------
> 6 files changed, 3 insertions(+), 25 deletions(-)
>
> diff --git a/Config.mk b/Config.mk
> index 03023033..52eb26d8 100644
> --- a/Config.mk
> +++ b/Config.mk
> @@ -200,7 +200,6 @@ CONFIG-n += CONFIG_PARAVIRT
> endif
> # Support legacy CONFIG_XC value
> CONFIG_XC ?= $(libc)
> -CONFIG-$(CONFIG_XC) += CONFIG_LIBXENEVTCHN
> CONFIG-$(CONFIG_XC) += CONFIG_LIBXENGNTTAB
>
> CONFIG-$(lwip) += CONFIG_LWIP
> diff --git a/arch/x86/testbuild/all-no b/arch/x86/testbuild/all-no
> index 46f974de..15c954ff 100644
> --- a/arch/x86/testbuild/all-no
> +++ b/arch/x86/testbuild/all-no
> @@ -14,7 +14,6 @@ CONFIG_KBDFRONT = n
> CONFIG_CONSFRONT = n
> CONFIG_XENBUS = n
> CONFIG_LIBXS = n
> -CONFIG_LIBXENEVTCHN = n
> CONFIG_LIBXENGNTTAB = n
> CONFIG_LWIP = n
> CONFIG_BALLOON = n
> diff --git a/arch/x86/testbuild/all-yes b/arch/x86/testbuild/all-yes
> index 3ead12f1..9964d42f 100644
> --- a/arch/x86/testbuild/all-yes
> +++ b/arch/x86/testbuild/all-yes
> @@ -17,6 +17,5 @@ CONFIG_LIBXS = y
> CONFIG_BALLOON = y
> CONFIG_USE_XEN_CONSOLE = y
> # The following are special: they need support from outside
> -CONFIG_LIBXENEVTCHN = n
> CONFIG_LIBXENGNTTAB = n
> CONFIG_LWIP = n
> diff --git a/arch/x86/testbuild/newxen-yes b/arch/x86/testbuild/newxen-yes
> index 5c0b3c80..c2519938 100644
> --- a/arch/x86/testbuild/newxen-yes
> +++ b/arch/x86/testbuild/newxen-yes
> @@ -18,6 +18,5 @@ 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_LIBXENEVTCHN = n
> CONFIG_LIBXENGNTTAB = n
> CONFIG_LWIP = n
> diff --git a/include/lib.h b/include/lib.h
> index 44696806..5f24be5d 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -155,6 +155,8 @@ do { \
> void sanity_check(void);
>
> #ifdef HAVE_LIBC
> +extern struct wait_queue_head event_queue;
> +
> #define FTYPE_NONE 0
> #define FTYPE_CONSOLE 1
> #define FTYPE_FILE 2
> @@ -169,19 +171,9 @@ void sanity_check(void);
> #define FTYPE_TPM_TIS 11
> #define FTYPE_XENBUS 12
> #define FTYPE_GNTMAP 13
> -#define FTYPE_EVTCHN 14
> -#define FTYPE_N 15
> +#define FTYPE_N 14
> #define FTYPE_SPARE 16
>
> -LIST_HEAD(evtchn_port_list, evtchn_port_info);
> -
> -struct evtchn_port_info {
> - LIST_ENTRY(evtchn_port_info) list;
> - evtchn_port_t port;
> - unsigned long pending;
> - int bound;
> -};
> -
> struct file {
> unsigned int type;
> bool read; /* maybe available for read */
> @@ -189,9 +181,6 @@ struct file {
> union {
> int fd; /* Any fd from an upper layer. */
> void *dev;
> - struct {
> - struct evtchn_port_list ports;
> - } evtchn;
> struct gntmap gntmap;
> };
> };
> diff --git a/lib/sys.c b/lib/sys.c
> index e0ac5099..34f0193d 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -533,11 +533,6 @@ int close(int fd)
> res = lwip_close(files[fd].fd);
> break;
> #endif
> -#ifdef CONFIG_LIBXENEVTCHN
> - case FTYPE_EVTCHN:
> - minios_evtchn_close_fd(fd);
> - break;
> -#endif
> #ifdef CONFIG_LIBXENGNTTAB
> case FTYPE_GNTMAP:
> minios_gnttab_close_fd(fd);
> @@ -770,7 +765,6 @@ 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",
> @@ -970,7 +964,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
> FD_CLR(i, exceptfds);
> break;
> #endif
> - case FTYPE_EVTCHN:
> case FTYPE_TAP:
> case FTYPE_BLK:
> case FTYPE_KBD:
> --
> 2.26.2
>
--
Samuel
tohi.cybercable.fr (212.198.0.3) si une personne se reconnait derriere
cette adresse que ce soit un pirate ou une victime qu'il se manifeste,
cette personne pourrait bien etre un petit malin
-+- Fred in NPC : Mamaaaaan, y a le routeur qui veut me hacker -+-
^ permalink raw reply [flat|nested] 29+ messages in thread
* [MINIOS PATCH v3 02/12] remove gnttab specific member from struct file
2022-01-16 8:33 [MINIOS PATCH v3 00/12] remove device specific struct file members Juergen Gross
2022-01-16 8:33 ` [MINIOS PATCH v3 01/12] remove event channel specific struct file definitions Juergen Gross
@ 2022-01-16 8:33 ` Juergen Gross
2022-01-16 8:33 ` [MINIOS PATCH v3 03/12] use alloc_file_type() and get_file_from_fd() in xs Juergen Gross
` (10 subsequent siblings)
12 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-01-16 8:33 UTC (permalink / raw)
To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross
The gnttab specific union member in struct file is no longer
needed, so remove it.
The gnttab file type and its associated handling can be removed, too,
as libxengnttab is now supplying a struct file_ops via a call of
alloc_file_type().
This removes all contents of CONFIG_LIBXENGNTTAB guarded sections, so
this config option can be removed.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
Config.mk | 1 -
arch/x86/testbuild/all-no | 1 -
arch/x86/testbuild/all-yes | 1 -
arch/x86/testbuild/newxen-yes | 1 -
gntmap.c | 2 +-
include/lib.h | 4 +---
lib/sys.c | 5 -----
7 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/Config.mk b/Config.mk
index 52eb26d8..1a24b30e 100644
--- a/Config.mk
+++ b/Config.mk
@@ -200,7 +200,6 @@ CONFIG-n += CONFIG_PARAVIRT
endif
# Support legacy CONFIG_XC value
CONFIG_XC ?= $(libc)
-CONFIG-$(CONFIG_XC) += CONFIG_LIBXENGNTTAB
CONFIG-$(lwip) += CONFIG_LWIP
diff --git a/arch/x86/testbuild/all-no b/arch/x86/testbuild/all-no
index 15c954ff..f79a1012 100644
--- a/arch/x86/testbuild/all-no
+++ b/arch/x86/testbuild/all-no
@@ -14,7 +14,6 @@ CONFIG_KBDFRONT = n
CONFIG_CONSFRONT = n
CONFIG_XENBUS = n
CONFIG_LIBXS = n
-CONFIG_LIBXENGNTTAB = n
CONFIG_LWIP = n
CONFIG_BALLOON = n
CONFIG_USE_XEN_CONSOLE = n
diff --git a/arch/x86/testbuild/all-yes b/arch/x86/testbuild/all-yes
index 9964d42f..faa3af32 100644
--- a/arch/x86/testbuild/all-yes
+++ b/arch/x86/testbuild/all-yes
@@ -17,5 +17,4 @@ CONFIG_LIBXS = y
CONFIG_BALLOON = y
CONFIG_USE_XEN_CONSOLE = y
# The following are special: they need support from outside
-CONFIG_LIBXENGNTTAB = n
CONFIG_LWIP = n
diff --git a/arch/x86/testbuild/newxen-yes b/arch/x86/testbuild/newxen-yes
index c2519938..dc83e670 100644
--- a/arch/x86/testbuild/newxen-yes
+++ b/arch/x86/testbuild/newxen-yes
@@ -18,5 +18,4 @@ 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_LIBXENGNTTAB = n
CONFIG_LWIP = n
diff --git a/gntmap.c b/gntmap.c
index 6fa1dac1..3422ab23 100644
--- a/gntmap.c
+++ b/gntmap.c
@@ -3,7 +3,7 @@
*
* Diego Ongaro <diego.ongaro@citrix.com>, July 2008
*
- * Files of type FTYPE_GNTMAP contain a gntmap, which is an array of
+ * Files of libxengnttab contain a gntmap, which is an array of
* (host address, grant handle) pairs. Grant handles come from a hypervisor map
* operation and are needed for the corresponding unmap.
*
diff --git a/include/lib.h b/include/lib.h
index 5f24be5d..5838b0d4 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -170,8 +170,7 @@ extern struct wait_queue_head event_queue;
#define FTYPE_TPMFRONT 10
#define FTYPE_TPM_TIS 11
#define FTYPE_XENBUS 12
-#define FTYPE_GNTMAP 13
-#define FTYPE_N 14
+#define FTYPE_N 13
#define FTYPE_SPARE 16
struct file {
@@ -181,7 +180,6 @@ struct file {
union {
int fd; /* Any fd from an upper layer. */
void *dev;
- struct gntmap gntmap;
};
};
diff --git a/lib/sys.c b/lib/sys.c
index 34f0193d..6ba26294 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -533,11 +533,6 @@ int close(int fd)
res = lwip_close(files[fd].fd);
break;
#endif
-#ifdef CONFIG_LIBXENGNTTAB
- case FTYPE_GNTMAP:
- minios_gnttab_close_fd(fd);
- break;
-#endif
#ifdef CONFIG_NETFRONT
case FTYPE_TAP:
shutdown_netfront(files[fd].dev);
--
2.26.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* [MINIOS PATCH v3 03/12] use alloc_file_type() and get_file_from_fd() in xs
2022-01-16 8:33 [MINIOS PATCH v3 00/12] remove device specific struct file members Juergen Gross
2022-01-16 8:33 ` [MINIOS PATCH v3 01/12] remove event channel specific struct file definitions Juergen Gross
2022-01-16 8:33 ` [MINIOS PATCH v3 02/12] remove gnttab specific member from struct file Juergen Gross
@ 2022-01-16 8:33 ` Juergen Gross
2022-01-16 20:54 ` Samuel Thibault
2022-01-18 14:29 ` Andrew Cooper
2022-01-16 8:33 ` [MINIOS PATCH v3 04/12] use alloc_file_type() and get_file_from_fd() in tpm_tis Juergen Gross
` (9 subsequent siblings)
12 siblings, 2 replies; 29+ messages in thread
From: Juergen Gross @ 2022-01-16 8:33 UTC (permalink / raw)
To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross
Allocate the needed file type via alloc_file_type().
Instead of directly accessing the files[] array use get_file_from_fd().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- switch to struct file * parameter for callbacks
- use __attribute__((constructor))
---
include/lib.h | 3 +--
lib/sys.c | 18 --------------
lib/xs.c | 65 +++++++++++++++++++++++++++++++++++++------------
xenbus/xenbus.c | 1 +
4 files changed, 52 insertions(+), 35 deletions(-)
diff --git a/include/lib.h b/include/lib.h
index 5838b0d4..c5e47786 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -169,8 +169,7 @@ extern struct wait_queue_head event_queue;
#define FTYPE_BLK 9
#define FTYPE_TPMFRONT 10
#define FTYPE_TPM_TIS 11
-#define FTYPE_XENBUS 12
-#define FTYPE_N 13
+#define FTYPE_N 12
#define FTYPE_SPARE 16
struct file {
diff --git a/lib/sys.c b/lib/sys.c
index 6ba26294..53870aa4 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -523,11 +523,6 @@ int close(int fd)
switch (file->type) {
default:
break;
-#ifdef CONFIG_LIBXS
- case FTYPE_XENBUS:
- xs_daemon_close((void*)(intptr_t) fd);
- break;
-#endif
#ifdef HAVE_LWIP
case FTYPE_SOCKET:
res = lwip_close(files[fd].fd);
@@ -759,7 +754,6 @@ int closedir(DIR *dir)
static const char *const file_types[] = {
[FTYPE_NONE] = "none",
[FTYPE_CONSOLE] = "console",
- [FTYPE_XENBUS] = "xenbus",
[FTYPE_SOCKET] = "socket",
[FTYPE_TAP] = "net",
[FTYPE_BLK] = "blk",
@@ -947,18 +941,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
n++;
FD_CLR(i, exceptfds);
break;
-#ifdef CONFIG_LIBXS
- case FTYPE_XENBUS:
- if (FD_ISSET(i, readfds)) {
- if (files[i].dev)
- n++;
- else
- FD_CLR(i, readfds);
- }
- FD_CLR(i, writefds);
- FD_CLR(i, exceptfds);
- break;
-#endif
case FTYPE_TAP:
case FTYPE_BLK:
case FTYPE_KBD:
diff --git a/lib/xs.c b/lib/xs.c
index 4af0f960..c12341aa 100644
--- a/lib/xs.c
+++ b/lib/xs.c
@@ -18,23 +18,56 @@ static inline int _xs_fileno(struct xs_handle *h) {
return (intptr_t) h;
}
+static int xs_close_fd(struct file *file)
+{
+ struct xenbus_event *event, *next;
+
+ for (event = file->dev; event; event = next)
+ {
+ next = event->next;
+ free(event);
+ }
+
+ return 0;
+}
+
+static bool xs_can_read(struct file *file)
+{
+ return file && file->dev;
+}
+
+static const struct file_ops xenbus_ops = {
+ .name = "xenbus",
+ .close = xs_close_fd,
+ .select_rd = xs_can_read,
+};
+
+static unsigned int ftype_xenbus;
+
+__attribute__((constructor))
+static void xs_initialize(void)
+{
+ ftype_xenbus = alloc_file_type(&xenbus_ops);
+}
+
struct xs_handle *xs_daemon_open()
{
- int fd = alloc_fd(FTYPE_XENBUS);
- files[fd].dev = NULL;
- printk("xs_daemon_open -> %d, %p\n", fd, &files[fd].dev);
+ int fd;
+ struct file *file;
+
+ fd = alloc_fd(ftype_xenbus);
+ file = get_file_from_fd(fd);
+ if ( !file )
+ return NULL;
+
+ file->dev = NULL;
+ printk("xs_daemon_open -> %d, %p\n", fd, &file->dev);
return (void*)(intptr_t) fd;
}
void xs_daemon_close(struct xs_handle *h)
{
- int fd = _xs_fileno(h);
- struct xenbus_event *event, *next;
- for (event = files[fd].dev; event; event = next)
- {
- next = event->next;
- free(event);
- }
+ close(_xs_fileno(h));
}
int xs_fileno(struct xs_handle *h)
@@ -169,18 +202,20 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t,
bool xs_watch(struct xs_handle *h, const char *path, const char *token)
{
- int fd = _xs_fileno(h);
+ struct file *file = get_file_from_fd(_xs_fileno(h));
+
printk("xs_watch(%s, %s)\n", path, token);
return xs_bool(xenbus_watch_path_token(XBT_NULL, path, token,
- (xenbus_event_queue *)&files[fd].dev));
+ (xenbus_event_queue *)&file->dev));
}
char **xs_read_watch(struct xs_handle *h, unsigned int *num)
{
- int fd = _xs_fileno(h);
struct xenbus_event *event;
- event = files[fd].dev;
- files[fd].dev = event->next;
+ struct file *file = get_file_from_fd(_xs_fileno(h));
+
+ event = file->dev;
+ file->dev = event->next;
printk("xs_read_watch() -> %s %s\n", event->path, event->token);
*num = 2;
return (char **) &event->path;
diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
index b687678f..785389fb 100644
--- a/xenbus/xenbus.c
+++ b/xenbus/xenbus.c
@@ -393,6 +393,7 @@ static int allocate_xenbus_id(void)
void init_xenbus(void)
{
int err;
+
DEBUG("init_xenbus called.\n");
create_thread("xenstore", xenbus_thread_func, NULL);
DEBUG("buf at %p.\n", xenstore_buf);
--
2.26.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 03/12] use alloc_file_type() and get_file_from_fd() in xs
2022-01-16 8:33 ` [MINIOS PATCH v3 03/12] use alloc_file_type() and get_file_from_fd() in xs Juergen Gross
@ 2022-01-16 20:54 ` Samuel Thibault
2022-01-18 14:29 ` Andrew Cooper
1 sibling, 0 replies; 29+ messages in thread
From: Samuel Thibault @ 2022-01-16 20:54 UTC (permalink / raw)
To: Juergen Gross; +Cc: minios-devel, xen-devel, wl
Juergen Gross, le dim. 16 janv. 2022 09:33:19 +0100, a ecrit:
> Allocate the needed file type via alloc_file_type().
>
> Instead of directly accessing the files[] array use get_file_from_fd().
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> V3:
> - switch to struct file * parameter for callbacks
> - use __attribute__((constructor))
> ---
> include/lib.h | 3 +--
> lib/sys.c | 18 --------------
> lib/xs.c | 65 +++++++++++++++++++++++++++++++++++++------------
> xenbus/xenbus.c | 1 +
> 4 files changed, 52 insertions(+), 35 deletions(-)
>
> diff --git a/include/lib.h b/include/lib.h
> index 5838b0d4..c5e47786 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -169,8 +169,7 @@ extern struct wait_queue_head event_queue;
> #define FTYPE_BLK 9
> #define FTYPE_TPMFRONT 10
> #define FTYPE_TPM_TIS 11
> -#define FTYPE_XENBUS 12
> -#define FTYPE_N 13
> +#define FTYPE_N 12
> #define FTYPE_SPARE 16
>
> struct file {
> diff --git a/lib/sys.c b/lib/sys.c
> index 6ba26294..53870aa4 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -523,11 +523,6 @@ int close(int fd)
> switch (file->type) {
> default:
> break;
> -#ifdef CONFIG_LIBXS
> - case FTYPE_XENBUS:
> - xs_daemon_close((void*)(intptr_t) fd);
> - break;
> -#endif
> #ifdef HAVE_LWIP
> case FTYPE_SOCKET:
> res = lwip_close(files[fd].fd);
> @@ -759,7 +754,6 @@ int closedir(DIR *dir)
> static const char *const file_types[] = {
> [FTYPE_NONE] = "none",
> [FTYPE_CONSOLE] = "console",
> - [FTYPE_XENBUS] = "xenbus",
> [FTYPE_SOCKET] = "socket",
> [FTYPE_TAP] = "net",
> [FTYPE_BLK] = "blk",
> @@ -947,18 +941,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
> n++;
> FD_CLR(i, exceptfds);
> break;
> -#ifdef CONFIG_LIBXS
> - case FTYPE_XENBUS:
> - if (FD_ISSET(i, readfds)) {
> - if (files[i].dev)
> - n++;
> - else
> - FD_CLR(i, readfds);
> - }
> - FD_CLR(i, writefds);
> - FD_CLR(i, exceptfds);
> - break;
> -#endif
> case FTYPE_TAP:
> case FTYPE_BLK:
> case FTYPE_KBD:
> diff --git a/lib/xs.c b/lib/xs.c
> index 4af0f960..c12341aa 100644
> --- a/lib/xs.c
> +++ b/lib/xs.c
> @@ -18,23 +18,56 @@ static inline int _xs_fileno(struct xs_handle *h) {
> return (intptr_t) h;
> }
>
> +static int xs_close_fd(struct file *file)
> +{
> + struct xenbus_event *event, *next;
> +
> + for (event = file->dev; event; event = next)
> + {
> + next = event->next;
> + free(event);
> + }
> +
> + return 0;
> +}
> +
> +static bool xs_can_read(struct file *file)
> +{
> + return file && file->dev;
> +}
> +
> +static const struct file_ops xenbus_ops = {
> + .name = "xenbus",
> + .close = xs_close_fd,
> + .select_rd = xs_can_read,
> +};
> +
> +static unsigned int ftype_xenbus;
> +
> +__attribute__((constructor))
> +static void xs_initialize(void)
> +{
> + ftype_xenbus = alloc_file_type(&xenbus_ops);
> +}
> +
> struct xs_handle *xs_daemon_open()
> {
> - int fd = alloc_fd(FTYPE_XENBUS);
> - files[fd].dev = NULL;
> - printk("xs_daemon_open -> %d, %p\n", fd, &files[fd].dev);
> + int fd;
> + struct file *file;
> +
> + fd = alloc_fd(ftype_xenbus);
> + file = get_file_from_fd(fd);
> + if ( !file )
> + return NULL;
> +
> + file->dev = NULL;
> + printk("xs_daemon_open -> %d, %p\n", fd, &file->dev);
> return (void*)(intptr_t) fd;
> }
>
> void xs_daemon_close(struct xs_handle *h)
> {
> - int fd = _xs_fileno(h);
> - struct xenbus_event *event, *next;
> - for (event = files[fd].dev; event; event = next)
> - {
> - next = event->next;
> - free(event);
> - }
> + close(_xs_fileno(h));
> }
>
> int xs_fileno(struct xs_handle *h)
> @@ -169,18 +202,20 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t,
>
> bool xs_watch(struct xs_handle *h, const char *path, const char *token)
> {
> - int fd = _xs_fileno(h);
> + struct file *file = get_file_from_fd(_xs_fileno(h));
> +
> printk("xs_watch(%s, %s)\n", path, token);
> return xs_bool(xenbus_watch_path_token(XBT_NULL, path, token,
> - (xenbus_event_queue *)&files[fd].dev));
> + (xenbus_event_queue *)&file->dev));
> }
>
> char **xs_read_watch(struct xs_handle *h, unsigned int *num)
> {
> - int fd = _xs_fileno(h);
> struct xenbus_event *event;
> - event = files[fd].dev;
> - files[fd].dev = event->next;
> + struct file *file = get_file_from_fd(_xs_fileno(h));
> +
> + event = file->dev;
> + file->dev = event->next;
> printk("xs_read_watch() -> %s %s\n", event->path, event->token);
> *num = 2;
> return (char **) &event->path;
> diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
> index b687678f..785389fb 100644
> --- a/xenbus/xenbus.c
> +++ b/xenbus/xenbus.c
> @@ -393,6 +393,7 @@ static int allocate_xenbus_id(void)
> void init_xenbus(void)
> {
> int err;
> +
> DEBUG("init_xenbus called.\n");
> create_thread("xenstore", xenbus_thread_func, NULL);
> DEBUG("buf at %p.\n", xenstore_buf);
> --
> 2.26.2
>
--
Samuel
RR> Ce que je cherche à démontrer, c'est qu'il est injuste de faire
RR> l'amalgame entre du bulk mail et du courrier non-solicité très ciblé
un suppositoire non reclamé, meme tres bien ciblé, reste un suppositoire.
-+-OS in : Guide du Neuneu d'Usenet - Plein le cul de la pub à neuneu -+-
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 03/12] use alloc_file_type() and get_file_from_fd() in xs
2022-01-16 8:33 ` [MINIOS PATCH v3 03/12] use alloc_file_type() and get_file_from_fd() in xs Juergen Gross
2022-01-16 20:54 ` Samuel Thibault
@ 2022-01-18 14:29 ` Andrew Cooper
2022-01-18 14:53 ` Juergen Gross
1 sibling, 1 reply; 29+ messages in thread
From: Andrew Cooper @ 2022-01-18 14:29 UTC (permalink / raw)
To: Juergen Gross, minios-devel, xen-devel; +Cc: samuel.thibault, wl
On 16/01/2022 08:33, Juergen Gross wrote:
> diff --git a/lib/xs.c b/lib/xs.c
> index 4af0f960..c12341aa 100644
> --- a/lib/xs.c
> +++ b/lib/xs.c
> @@ -18,23 +18,56 @@ static inline int _xs_fileno(struct xs_handle *h) {
> return (intptr_t) h;
> }
>
> +static int xs_close_fd(struct file *file)
> +{
> + struct xenbus_event *event, *next;
> +
> + for (event = file->dev; event; event = next)
> + {
> + next = event->next;
> + free(event);
> + }
> +
> + return 0;
> +}
> +
> +static bool xs_can_read(struct file *file)
> +{
> + return file && file->dev;
Just 'return file->dev;' ?
> @@ -169,18 +202,20 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t,
>
> bool xs_watch(struct xs_handle *h, const char *path, const char *token)
> {
> - int fd = _xs_fileno(h);
> + struct file *file = get_file_from_fd(_xs_fileno(h));
> +
> printk("xs_watch(%s, %s)\n", path, token);
> return xs_bool(xenbus_watch_path_token(XBT_NULL, path, token,
> - (xenbus_event_queue *)&files[fd].dev));
> + (xenbus_event_queue *)&file->dev));
This is utterly mad. In particular, close() looks to be very racy with
new watches arriving.
However, can the indentation at least be fixed here as the line is
changing. That's a parameter to xenbus_watch_path_token(), not xs_bool().
> diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
> index b687678f..785389fb 100644
> --- a/xenbus/xenbus.c
> +++ b/xenbus/xenbus.c
> @@ -393,6 +393,7 @@ static int allocate_xenbus_id(void)
> void init_xenbus(void)
> {
> int err;
> +
Spurious hunk?
~Andrew
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 03/12] use alloc_file_type() and get_file_from_fd() in xs
2022-01-18 14:29 ` Andrew Cooper
@ 2022-01-18 14:53 ` Juergen Gross
0 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-01-18 14:53 UTC (permalink / raw)
To: Andrew Cooper, minios-devel, xen-devel; +Cc: samuel.thibault, wl
[-- Attachment #1.1.1: Type: text/plain, Size: 2097 bytes --]
On 18.01.22 15:29, Andrew Cooper wrote:
> On 16/01/2022 08:33, Juergen Gross wrote:
>> diff --git a/lib/xs.c b/lib/xs.c
>> index 4af0f960..c12341aa 100644
>> --- a/lib/xs.c
>> +++ b/lib/xs.c
>> @@ -18,23 +18,56 @@ static inline int _xs_fileno(struct xs_handle *h) {
>> return (intptr_t) h;
>> }
>>
>> +static int xs_close_fd(struct file *file)
>> +{
>> + struct xenbus_event *event, *next;
>> +
>> + for (event = file->dev; event; event = next)
>> + {
>> + next = event->next;
>> + free(event);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static bool xs_can_read(struct file *file)
>> +{
>> + return file && file->dev;
>
> Just 'return file->dev;' ?
Yes.
>
>> @@ -169,18 +202,20 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t,
>>
>> bool xs_watch(struct xs_handle *h, const char *path, const char *token)
>> {
>> - int fd = _xs_fileno(h);
>> + struct file *file = get_file_from_fd(_xs_fileno(h));
>> +
>> printk("xs_watch(%s, %s)\n", path, token);
>> return xs_bool(xenbus_watch_path_token(XBT_NULL, path, token,
>> - (xenbus_event_queue *)&files[fd].dev));
>> + (xenbus_event_queue *)&file->dev));
>
> This is utterly mad. In particular, close() looks to be very racy with
> new watches arriving.
In practice it should be no problem, though (closing the file in one
thread while the other one is adding a watch would be rather strange).
Additionally close() for xenbus in Mini-OS is called only when stopping
the domain today.
> However, can the indentation at least be fixed here as the line is
> changing. That's a parameter to xenbus_watch_path_token(), not xs_bool().
Yes, that should be done.
>
>> diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
>> index b687678f..785389fb 100644
>> --- a/xenbus/xenbus.c
>> +++ b/xenbus/xenbus.c
>> @@ -393,6 +393,7 @@ static int allocate_xenbus_id(void)
>> void init_xenbus(void)
>> {
>> int err;
>> +
>
> Spurious hunk?
Yes.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* [MINIOS PATCH v3 04/12] use alloc_file_type() and get_file_from_fd() in tpm_tis
2022-01-16 8:33 [MINIOS PATCH v3 00/12] remove device specific struct file members Juergen Gross
` (2 preceding siblings ...)
2022-01-16 8:33 ` [MINIOS PATCH v3 03/12] use alloc_file_type() and get_file_from_fd() in xs Juergen Gross
@ 2022-01-16 8:33 ` Juergen Gross
2022-01-16 20:56 ` Samuel Thibault
2022-01-18 14:32 ` Andrew Cooper
2022-01-16 8:33 ` [MINIOS PATCH v3 05/12] use alloc_file_type() and get_file_from_fd() in tpmfront Juergen Gross
` (8 subsequent siblings)
12 siblings, 2 replies; 29+ messages in thread
From: Juergen Gross @ 2022-01-16 8:33 UTC (permalink / raw)
To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross
Allocate a file type dynamically via alloc_file_type().
Instead of directly accessing the files[] array use get_file_from_fd().
Make some now local functions static and modify their prototypes to
match the file_ops requirements.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- switch to struct file * parameter for callbacks
- use __attribute__((constructor))
---
include/lib.h | 3 +-
include/tpm_tis.h | 6 ---
lib/sys.c | 23 ---------
tpm_tis.c | 121 +++++++++++++++++++++++++++++-----------------
4 files changed, 78 insertions(+), 75 deletions(-)
diff --git a/include/lib.h b/include/lib.h
index c5e47786..9dfb4689 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -168,8 +168,7 @@ extern struct wait_queue_head event_queue;
#define FTYPE_TAP 8
#define FTYPE_BLK 9
#define FTYPE_TPMFRONT 10
-#define FTYPE_TPM_TIS 11
-#define FTYPE_N 12
+#define FTYPE_N 11
#define FTYPE_SPARE 16
struct file {
diff --git a/include/tpm_tis.h b/include/tpm_tis.h
index 86e83f13..2af974db 100644
--- a/include/tpm_tis.h
+++ b/include/tpm_tis.h
@@ -37,14 +37,11 @@ struct tpm_chip;
struct tpm_chip* init_tpm_tis(unsigned long baseaddr, int localities, unsigned int irq);
struct tpm_chip* init_tpm2_tis(unsigned long baseaddr, int localities, unsigned int irq);
-void shutdown_tpm_tis(struct tpm_chip* tpm);
int tpm_tis_request_locality(struct tpm_chip* tpm, int locality);
int tpm_tis_cmd(struct tpm_chip* tpm, uint8_t* req, size_t reqlen, uint8_t** resp, size_t* resplen);
#ifdef HAVE_LIBC
-#include <sys/stat.h>
-#include <fcntl.h>
/* POSIX IO functions:
* use tpm_tis_open() to get a file descriptor to the tpm device
* use write() on the fd to send a command to the backend. You must
@@ -53,9 +50,6 @@ int tpm_tis_cmd(struct tpm_chip* tpm, uint8_t* req, size_t reqlen, uint8_t** res
* fstat() to get the size of the response and lseek() to seek on it.
*/
int tpm_tis_open(struct tpm_chip* tpm);
-int tpm_tis_posix_read(int fd, uint8_t* buf, size_t count);
-int tpm_tis_posix_write(int fd, const uint8_t* buf, size_t count);
-int tpm_tis_posix_fstat(int fd, struct stat* buf);
#endif
#endif
diff --git a/lib/sys.c b/lib/sys.c
index 53870aa4..2fc81f20 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -29,7 +29,6 @@
#include <blkfront.h>
#include <fbfront.h>
#include <tpmfront.h>
-#include <tpm_tis.h>
#include <xenbus.h>
#include <xenstore.h>
#include <poll.h>
@@ -355,11 +354,6 @@ int read(int fd, void *buf, size_t nbytes)
case FTYPE_TPMFRONT: {
return tpmfront_posix_read(fd, buf, nbytes);
}
-#endif
-#ifdef CONFIG_TPM_TIS
- case FTYPE_TPM_TIS: {
- return tpm_tis_posix_read(fd, buf, nbytes);
- }
#endif
default:
break;
@@ -412,10 +406,6 @@ int write(int fd, const void *buf, size_t nbytes)
#ifdef CONFIG_TPMFRONT
case FTYPE_TPMFRONT:
return tpmfront_posix_write(fd, buf, nbytes);
-#endif
-#ifdef CONFIG_TPM_TIS
- case FTYPE_TPM_TIS:
- return tpm_tis_posix_write(fd, buf, nbytes);
#endif
default:
break;
@@ -482,10 +472,6 @@ off_t lseek(int fd, off_t offset, int whence)
#ifdef CONFIG_TPMFRONT
case FTYPE_TPMFRONT:
break;
-#endif
-#ifdef CONFIG_TPM_TIS
- case FTYPE_TPM_TIS:
- break;
#endif
case FTYPE_FILE:
break;
@@ -543,11 +529,6 @@ int close(int fd)
shutdown_tpmfront(files[fd].dev);
break;
#endif
-#ifdef CONFIG_TPM_TIS
- case FTYPE_TPM_TIS:
- shutdown_tpm_tis(files[fd].dev);
- break;
-#endif
#ifdef CONFIG_KBDFRONT
case FTYPE_KBD:
shutdown_kbdfront(files[fd].dev);
@@ -636,10 +617,6 @@ int fstat(int fd, struct stat *buf)
#ifdef CONFIG_TPMFRONT
case FTYPE_TPMFRONT:
return tpmfront_posix_fstat(fd, buf);
-#endif
-#ifdef CONFIG_TPM_TIS
- case FTYPE_TPM_TIS:
- return tpm_tis_posix_fstat(fd, buf);
#endif
default:
break;
diff --git a/tpm_tis.c b/tpm_tis.c
index 477f5550..b88ec874 100644
--- a/tpm_tis.c
+++ b/tpm_tis.c
@@ -792,6 +792,9 @@ int tpm_tis_send(struct tpm_chip* tpm, uint8_t* buf, size_t len) {
int status, burstcnt = 0;
int count = 0;
uint32_t ordinal;
+#ifdef HAVE_LIBC
+ struct file *file = get_file_from_fd(tpm->fd);
+#endif
if(tpm_tis_request_locality(tpm, tpm->locality) < 0) {
return -EBUSY;
@@ -844,9 +847,10 @@ int tpm_tis_send(struct tpm_chip* tpm, uint8_t* buf, size_t len) {
}
}
#ifdef HAVE_LIBC
- if(tpm->fd >= 0) {
- files[tpm->fd].read = false;
- files[tpm->fd].offset = 0;
+ if ( file )
+ {
+ file->read = false;
+ file->offset = 0;
}
#endif
return len;
@@ -1093,6 +1097,26 @@ ssize_t tpm_getcap(struct tpm_chip *chip, uint32_t subcap_id, cap_t *cap,
return rc;
}
+static void shutdown_tpm_tis(struct tpm_chip *tpm)
+{
+ int i;
+
+ printk("Shutting down tpm_tis device\n");
+
+ iowrite32(TPM_INT_ENABLE(tpm, tpm->locality), ~TPM_GLOBAL_INT_ENABLE);
+
+ /* Unmap all of the mmio pages */
+ for ( i = 0; i < 5; ++i )
+ {
+ if ( tpm->pages[i] != NULL )
+ {
+ iounmap(tpm->pages[i], PAGE_SIZE);
+ tpm->pages[i] = NULL;
+ }
+ }
+ free(tpm);
+ return;
+}
struct tpm_chip* init_tpm_tis(unsigned long baseaddr, int localities, unsigned int irq)
{
@@ -1242,25 +1266,6 @@ abort_egress:
return NULL;
}
-void shutdown_tpm_tis(struct tpm_chip* tpm){
- int i;
-
- printk("Shutting down tpm_tis device\n");
-
- iowrite32(TPM_INT_ENABLE(tpm, tpm->locality), ~TPM_GLOBAL_INT_ENABLE);
-
- /*Unmap all of the mmio pages */
- for(i = 0; i < 5; ++i) {
- if(tpm->pages[i] != NULL) {
- iounmap(tpm->pages[i], PAGE_SIZE);
- tpm->pages[i] = NULL;
- }
- }
- free(tpm);
- return;
-}
-
-
int tpm_tis_cmd(struct tpm_chip* tpm, uint8_t* req, size_t reqlen, uint8_t** resp, size_t* resplen)
{
if(tpm->locality < 0) {
@@ -1279,23 +1284,19 @@ int tpm_tis_cmd(struct tpm_chip* tpm, uint8_t* req, size_t reqlen, uint8_t** res
}
#ifdef HAVE_LIBC
-int tpm_tis_open(struct tpm_chip* tpm)
+#include <sys/stat.h>
+#include <fcntl.h>
+
+static int tpm_tis_close(struct file *file)
{
- /* Silently prevent multiple opens */
- if(tpm->fd != -1) {
- return tpm->fd;
- }
+ shutdown_tpm_tis(file->dev);
- tpm->fd = alloc_fd(FTYPE_TPM_TIS);
- printk("tpm_tis_open() -> %d\n", tpm->fd);
- files[tpm->fd].dev = tpm;
- return tpm->fd;
+ return 0;
}
-int tpm_tis_posix_write(int fd, const uint8_t* buf, size_t count)
+static int tpm_tis_posix_write(struct file *file, const void *buf, size_t count)
{
- struct tpm_chip* tpm;
- tpm = files[fd].dev;
+ struct tpm_chip *tpm = file->dev;
if(tpm->locality < 0) {
printk("tpm_tis_posix_write() failed! locality not set!\n");
@@ -1319,11 +1320,10 @@ int tpm_tis_posix_write(int fd, const uint8_t* buf, size_t count)
return count;
}
-int tpm_tis_posix_read(int fd, uint8_t* buf, size_t count)
+static int tpm_tis_posix_read(struct file *file, void *buf, size_t count)
{
int rc;
- struct tpm_chip* tpm;
- tpm = files[fd].dev;
+ struct tpm_chip *tpm = file->dev;
if(count == 0) {
return 0;
@@ -1337,20 +1337,21 @@ int tpm_tis_posix_read(int fd, uint8_t* buf, size_t count)
/* Handle EOF case */
- if(files[fd].offset >= tpm->data_len) {
+ if ( file->offset >= tpm->data_len )
+ {
rc = 0;
} else {
- rc = min(tpm->data_len - files[fd].offset, count);
- memcpy(buf, tpm->data_buffer + files[fd].offset, rc);
+ rc = min(tpm->data_len - file->offset, count);
+ memcpy(buf, tpm->data_buffer + file->offset, rc);
}
- files[fd].offset += rc;
+ file->offset += rc;
/* Reset the data pending flag */
return rc;
}
-int tpm_tis_posix_fstat(int fd, struct stat* buf)
+
+static int tpm_tis_posix_fstat(struct file *file, struct stat *buf)
{
- struct tpm_chip* tpm;
- tpm = files[fd].dev;
+ struct tpm_chip *tpm = file->dev;
buf->st_mode = O_RDWR;
buf->st_uid = 0;
@@ -1360,6 +1361,38 @@ int tpm_tis_posix_fstat(int fd, struct stat* buf)
return 0;
}
+static const struct file_ops tpm_tis_ops = {
+ .name = "tpm_tis",
+ .read = tpm_tis_posix_read,
+ .write = tpm_tis_posix_write,
+ .lseek = lseek_default,
+ .close = tpm_tis_close,
+ .fstat = tpm_tis_posix_fstat,
+};
+
+static unsigned int ftype_tis;
+
+__attribute__((constructor))
+static void tpm_tis_initialize(void)
+{
+ ftype_tis = alloc_file_type(&tpm_tis_ops);
+}
+
+int tpm_tis_open(struct tpm_chip *tpm)
+{
+ struct file *file;
+
+ /* Silently prevent multiple opens */
+ if ( tpm->fd != -1 )
+ return tpm->fd;
+
+ tpm->fd = alloc_fd(ftype_tis);
+ printk("tpm_tis_open() -> %d\n", tpm->fd);
+ file = get_file_from_fd(tpm->fd);
+ file->dev = tpm;
+ return tpm->fd;
+}
+
/* TPM 2.0 */
/*TPM2.0 Selftest*/
--
2.26.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 04/12] use alloc_file_type() and get_file_from_fd() in tpm_tis
2022-01-16 8:33 ` [MINIOS PATCH v3 04/12] use alloc_file_type() and get_file_from_fd() in tpm_tis Juergen Gross
@ 2022-01-16 20:56 ` Samuel Thibault
2022-01-18 14:32 ` Andrew Cooper
1 sibling, 0 replies; 29+ messages in thread
From: Samuel Thibault @ 2022-01-16 20:56 UTC (permalink / raw)
To: Juergen Gross; +Cc: minios-devel, xen-devel, wl
Juergen Gross, le dim. 16 janv. 2022 09:33:20 +0100, a ecrit:
> Allocate a file type dynamically via alloc_file_type().
>
> Instead of directly accessing the files[] array use get_file_from_fd().
>
> Make some now local functions static and modify their prototypes to
> match the file_ops requirements.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> V3:
> - switch to struct file * parameter for callbacks
> - use __attribute__((constructor))
> ---
> include/lib.h | 3 +-
> include/tpm_tis.h | 6 ---
> lib/sys.c | 23 ---------
> tpm_tis.c | 121 +++++++++++++++++++++++++++++-----------------
> 4 files changed, 78 insertions(+), 75 deletions(-)
>
> diff --git a/include/lib.h b/include/lib.h
> index c5e47786..9dfb4689 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -168,8 +168,7 @@ extern struct wait_queue_head event_queue;
> #define FTYPE_TAP 8
> #define FTYPE_BLK 9
> #define FTYPE_TPMFRONT 10
> -#define FTYPE_TPM_TIS 11
> -#define FTYPE_N 12
> +#define FTYPE_N 11
> #define FTYPE_SPARE 16
>
> struct file {
> diff --git a/include/tpm_tis.h b/include/tpm_tis.h
> index 86e83f13..2af974db 100644
> --- a/include/tpm_tis.h
> +++ b/include/tpm_tis.h
> @@ -37,14 +37,11 @@ struct tpm_chip;
>
> struct tpm_chip* init_tpm_tis(unsigned long baseaddr, int localities, unsigned int irq);
> struct tpm_chip* init_tpm2_tis(unsigned long baseaddr, int localities, unsigned int irq);
> -void shutdown_tpm_tis(struct tpm_chip* tpm);
>
> int tpm_tis_request_locality(struct tpm_chip* tpm, int locality);
> int tpm_tis_cmd(struct tpm_chip* tpm, uint8_t* req, size_t reqlen, uint8_t** resp, size_t* resplen);
>
> #ifdef HAVE_LIBC
> -#include <sys/stat.h>
> -#include <fcntl.h>
> /* POSIX IO functions:
> * use tpm_tis_open() to get a file descriptor to the tpm device
> * use write() on the fd to send a command to the backend. You must
> @@ -53,9 +50,6 @@ int tpm_tis_cmd(struct tpm_chip* tpm, uint8_t* req, size_t reqlen, uint8_t** res
> * fstat() to get the size of the response and lseek() to seek on it.
> */
> int tpm_tis_open(struct tpm_chip* tpm);
> -int tpm_tis_posix_read(int fd, uint8_t* buf, size_t count);
> -int tpm_tis_posix_write(int fd, const uint8_t* buf, size_t count);
> -int tpm_tis_posix_fstat(int fd, struct stat* buf);
> #endif
>
> #endif
> diff --git a/lib/sys.c b/lib/sys.c
> index 53870aa4..2fc81f20 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -29,7 +29,6 @@
> #include <blkfront.h>
> #include <fbfront.h>
> #include <tpmfront.h>
> -#include <tpm_tis.h>
> #include <xenbus.h>
> #include <xenstore.h>
> #include <poll.h>
> @@ -355,11 +354,6 @@ int read(int fd, void *buf, size_t nbytes)
> case FTYPE_TPMFRONT: {
> return tpmfront_posix_read(fd, buf, nbytes);
> }
> -#endif
> -#ifdef CONFIG_TPM_TIS
> - case FTYPE_TPM_TIS: {
> - return tpm_tis_posix_read(fd, buf, nbytes);
> - }
> #endif
> default:
> break;
> @@ -412,10 +406,6 @@ int write(int fd, const void *buf, size_t nbytes)
> #ifdef CONFIG_TPMFRONT
> case FTYPE_TPMFRONT:
> return tpmfront_posix_write(fd, buf, nbytes);
> -#endif
> -#ifdef CONFIG_TPM_TIS
> - case FTYPE_TPM_TIS:
> - return tpm_tis_posix_write(fd, buf, nbytes);
> #endif
> default:
> break;
> @@ -482,10 +472,6 @@ off_t lseek(int fd, off_t offset, int whence)
> #ifdef CONFIG_TPMFRONT
> case FTYPE_TPMFRONT:
> break;
> -#endif
> -#ifdef CONFIG_TPM_TIS
> - case FTYPE_TPM_TIS:
> - break;
> #endif
> case FTYPE_FILE:
> break;
> @@ -543,11 +529,6 @@ int close(int fd)
> shutdown_tpmfront(files[fd].dev);
> break;
> #endif
> -#ifdef CONFIG_TPM_TIS
> - case FTYPE_TPM_TIS:
> - shutdown_tpm_tis(files[fd].dev);
> - break;
> -#endif
> #ifdef CONFIG_KBDFRONT
> case FTYPE_KBD:
> shutdown_kbdfront(files[fd].dev);
> @@ -636,10 +617,6 @@ int fstat(int fd, struct stat *buf)
> #ifdef CONFIG_TPMFRONT
> case FTYPE_TPMFRONT:
> return tpmfront_posix_fstat(fd, buf);
> -#endif
> -#ifdef CONFIG_TPM_TIS
> - case FTYPE_TPM_TIS:
> - return tpm_tis_posix_fstat(fd, buf);
> #endif
> default:
> break;
> diff --git a/tpm_tis.c b/tpm_tis.c
> index 477f5550..b88ec874 100644
> --- a/tpm_tis.c
> +++ b/tpm_tis.c
> @@ -792,6 +792,9 @@ int tpm_tis_send(struct tpm_chip* tpm, uint8_t* buf, size_t len) {
> int status, burstcnt = 0;
> int count = 0;
> uint32_t ordinal;
> +#ifdef HAVE_LIBC
> + struct file *file = get_file_from_fd(tpm->fd);
> +#endif
>
> if(tpm_tis_request_locality(tpm, tpm->locality) < 0) {
> return -EBUSY;
> @@ -844,9 +847,10 @@ int tpm_tis_send(struct tpm_chip* tpm, uint8_t* buf, size_t len) {
> }
> }
> #ifdef HAVE_LIBC
> - if(tpm->fd >= 0) {
> - files[tpm->fd].read = false;
> - files[tpm->fd].offset = 0;
> + if ( file )
> + {
> + file->read = false;
> + file->offset = 0;
> }
> #endif
> return len;
> @@ -1093,6 +1097,26 @@ ssize_t tpm_getcap(struct tpm_chip *chip, uint32_t subcap_id, cap_t *cap,
> return rc;
> }
>
> +static void shutdown_tpm_tis(struct tpm_chip *tpm)
> +{
> + int i;
> +
> + printk("Shutting down tpm_tis device\n");
> +
> + iowrite32(TPM_INT_ENABLE(tpm, tpm->locality), ~TPM_GLOBAL_INT_ENABLE);
> +
> + /* Unmap all of the mmio pages */
> + for ( i = 0; i < 5; ++i )
> + {
> + if ( tpm->pages[i] != NULL )
> + {
> + iounmap(tpm->pages[i], PAGE_SIZE);
> + tpm->pages[i] = NULL;
> + }
> + }
> + free(tpm);
> + return;
> +}
>
> struct tpm_chip* init_tpm_tis(unsigned long baseaddr, int localities, unsigned int irq)
> {
> @@ -1242,25 +1266,6 @@ abort_egress:
> return NULL;
> }
>
> -void shutdown_tpm_tis(struct tpm_chip* tpm){
> - int i;
> -
> - printk("Shutting down tpm_tis device\n");
> -
> - iowrite32(TPM_INT_ENABLE(tpm, tpm->locality), ~TPM_GLOBAL_INT_ENABLE);
> -
> - /*Unmap all of the mmio pages */
> - for(i = 0; i < 5; ++i) {
> - if(tpm->pages[i] != NULL) {
> - iounmap(tpm->pages[i], PAGE_SIZE);
> - tpm->pages[i] = NULL;
> - }
> - }
> - free(tpm);
> - return;
> -}
> -
> -
> int tpm_tis_cmd(struct tpm_chip* tpm, uint8_t* req, size_t reqlen, uint8_t** resp, size_t* resplen)
> {
> if(tpm->locality < 0) {
> @@ -1279,23 +1284,19 @@ int tpm_tis_cmd(struct tpm_chip* tpm, uint8_t* req, size_t reqlen, uint8_t** res
> }
>
> #ifdef HAVE_LIBC
> -int tpm_tis_open(struct tpm_chip* tpm)
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +
> +static int tpm_tis_close(struct file *file)
> {
> - /* Silently prevent multiple opens */
> - if(tpm->fd != -1) {
> - return tpm->fd;
> - }
> + shutdown_tpm_tis(file->dev);
>
> - tpm->fd = alloc_fd(FTYPE_TPM_TIS);
> - printk("tpm_tis_open() -> %d\n", tpm->fd);
> - files[tpm->fd].dev = tpm;
> - return tpm->fd;
> + return 0;
> }
>
> -int tpm_tis_posix_write(int fd, const uint8_t* buf, size_t count)
> +static int tpm_tis_posix_write(struct file *file, const void *buf, size_t count)
> {
> - struct tpm_chip* tpm;
> - tpm = files[fd].dev;
> + struct tpm_chip *tpm = file->dev;
>
> if(tpm->locality < 0) {
> printk("tpm_tis_posix_write() failed! locality not set!\n");
> @@ -1319,11 +1320,10 @@ int tpm_tis_posix_write(int fd, const uint8_t* buf, size_t count)
> return count;
> }
>
> -int tpm_tis_posix_read(int fd, uint8_t* buf, size_t count)
> +static int tpm_tis_posix_read(struct file *file, void *buf, size_t count)
> {
> int rc;
> - struct tpm_chip* tpm;
> - tpm = files[fd].dev;
> + struct tpm_chip *tpm = file->dev;
>
> if(count == 0) {
> return 0;
> @@ -1337,20 +1337,21 @@ int tpm_tis_posix_read(int fd, uint8_t* buf, size_t count)
>
>
> /* Handle EOF case */
> - if(files[fd].offset >= tpm->data_len) {
> + if ( file->offset >= tpm->data_len )
> + {
> rc = 0;
> } else {
> - rc = min(tpm->data_len - files[fd].offset, count);
> - memcpy(buf, tpm->data_buffer + files[fd].offset, rc);
> + rc = min(tpm->data_len - file->offset, count);
> + memcpy(buf, tpm->data_buffer + file->offset, rc);
> }
> - files[fd].offset += rc;
> + file->offset += rc;
> /* Reset the data pending flag */
> return rc;
> }
> -int tpm_tis_posix_fstat(int fd, struct stat* buf)
> +
> +static int tpm_tis_posix_fstat(struct file *file, struct stat *buf)
> {
> - struct tpm_chip* tpm;
> - tpm = files[fd].dev;
> + struct tpm_chip *tpm = file->dev;
>
> buf->st_mode = O_RDWR;
> buf->st_uid = 0;
> @@ -1360,6 +1361,38 @@ int tpm_tis_posix_fstat(int fd, struct stat* buf)
> return 0;
> }
>
> +static const struct file_ops tpm_tis_ops = {
> + .name = "tpm_tis",
> + .read = tpm_tis_posix_read,
> + .write = tpm_tis_posix_write,
> + .lseek = lseek_default,
> + .close = tpm_tis_close,
> + .fstat = tpm_tis_posix_fstat,
> +};
> +
> +static unsigned int ftype_tis;
> +
> +__attribute__((constructor))
> +static void tpm_tis_initialize(void)
> +{
> + ftype_tis = alloc_file_type(&tpm_tis_ops);
> +}
> +
> +int tpm_tis_open(struct tpm_chip *tpm)
> +{
> + struct file *file;
> +
> + /* Silently prevent multiple opens */
> + if ( tpm->fd != -1 )
> + return tpm->fd;
> +
> + tpm->fd = alloc_fd(ftype_tis);
> + printk("tpm_tis_open() -> %d\n", tpm->fd);
> + file = get_file_from_fd(tpm->fd);
> + file->dev = tpm;
> + return tpm->fd;
> +}
> +
> /* TPM 2.0 */
>
> /*TPM2.0 Selftest*/
> --
> 2.26.2
>
--
Samuel
> et sinon, quand on s'interesse a un media que l'on ne maitrise pas,
> on essaye de le comprendre d'abord.
(Suivi par l'intégralité du message initial de 45 lignes.)
-+-BM in : GNU - La maîtrise est un long apprentissage petit scarabé -+-
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 04/12] use alloc_file_type() and get_file_from_fd() in tpm_tis
2022-01-16 8:33 ` [MINIOS PATCH v3 04/12] use alloc_file_type() and get_file_from_fd() in tpm_tis Juergen Gross
2022-01-16 20:56 ` Samuel Thibault
@ 2022-01-18 14:32 ` Andrew Cooper
2022-01-18 14:54 ` Juergen Gross
1 sibling, 1 reply; 29+ messages in thread
From: Andrew Cooper @ 2022-01-18 14:32 UTC (permalink / raw)
To: Juergen Gross, minios-devel, xen-devel; +Cc: samuel.thibault, wl
On 16/01/2022 08:33, Juergen Gross wrote:
> diff --git a/tpm_tis.c b/tpm_tis.c
> index 477f5550..b88ec874 100644
> --- a/tpm_tis.c
> +++ b/tpm_tis.c
> @@ -1093,6 +1097,26 @@ ssize_t tpm_getcap(struct tpm_chip *chip, uint32_t subcap_id, cap_t *cap,
> return rc;
> }
>
> +static void shutdown_tpm_tis(struct tpm_chip *tpm)
> +{
> + int i;
> +
> + printk("Shutting down tpm_tis device\n");
> +
> + iowrite32(TPM_INT_ENABLE(tpm, tpm->locality), ~TPM_GLOBAL_INT_ENABLE);
> +
> + /* Unmap all of the mmio pages */
> + for ( i = 0; i < 5; ++i )
> + {
> + if ( tpm->pages[i] != NULL )
> + {
> + iounmap(tpm->pages[i], PAGE_SIZE);
> + tpm->pages[i] = NULL;
> + }
> + }
> + free(tpm);
> + return;
Seeing as the function is moving anyway, this spurious return can go too.
~Andrew
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 04/12] use alloc_file_type() and get_file_from_fd() in tpm_tis
2022-01-18 14:32 ` Andrew Cooper
@ 2022-01-18 14:54 ` Juergen Gross
0 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-01-18 14:54 UTC (permalink / raw)
To: Andrew Cooper, minios-devel, xen-devel; +Cc: samuel.thibault, wl
[-- Attachment #1.1.1: Type: text/plain, Size: 974 bytes --]
On 18.01.22 15:32, Andrew Cooper wrote:
> On 16/01/2022 08:33, Juergen Gross wrote:
>> diff --git a/tpm_tis.c b/tpm_tis.c
>> index 477f5550..b88ec874 100644
>> --- a/tpm_tis.c
>> +++ b/tpm_tis.c
>> @@ -1093,6 +1097,26 @@ ssize_t tpm_getcap(struct tpm_chip *chip, uint32_t subcap_id, cap_t *cap,
>> return rc;
>> }
>>
>> +static void shutdown_tpm_tis(struct tpm_chip *tpm)
>> +{
>> + int i;
>> +
>> + printk("Shutting down tpm_tis device\n");
>> +
>> + iowrite32(TPM_INT_ENABLE(tpm, tpm->locality), ~TPM_GLOBAL_INT_ENABLE);
>> +
>> + /* Unmap all of the mmio pages */
>> + for ( i = 0; i < 5; ++i )
>> + {
>> + if ( tpm->pages[i] != NULL )
>> + {
>> + iounmap(tpm->pages[i], PAGE_SIZE);
>> + tpm->pages[i] = NULL;
>> + }
>> + }
>> + free(tpm);
>> + return;
>
> Seeing as the function is moving anyway, this spurious return can go too.
Yes.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* [MINIOS PATCH v3 05/12] use alloc_file_type() and get_file_from_fd() in tpmfront
2022-01-16 8:33 [MINIOS PATCH v3 00/12] remove device specific struct file members Juergen Gross
` (3 preceding siblings ...)
2022-01-16 8:33 ` [MINIOS PATCH v3 04/12] use alloc_file_type() and get_file_from_fd() in tpm_tis Juergen Gross
@ 2022-01-16 8:33 ` Juergen Gross
2022-01-16 20:57 ` Samuel Thibault
2022-01-16 8:33 ` [MINIOS PATCH v3 06/12] use alloc_file_type() and get_file_from_fd() in blkfront Juergen Gross
` (7 subsequent siblings)
12 siblings, 1 reply; 29+ messages in thread
From: Juergen Gross @ 2022-01-16 8:33 UTC (permalink / raw)
To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross
Allocate a file type dynamically via alloc_file_type().
Instead of directly accessing the files[] array use get_file_from_fd().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- switch to struct file * parameter for callbacks
- use __attribute__((constructor))
---
include/lib.h | 3 +-
include/tpmfront.h | 5 ---
lib/sys.c | 23 -----------
tpmfront.c | 100 ++++++++++++++++++++++++++++++---------------
4 files changed, 69 insertions(+), 62 deletions(-)
diff --git a/include/lib.h b/include/lib.h
index 9dfb4689..9b7eaae7 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -167,8 +167,7 @@ extern struct wait_queue_head event_queue;
#define FTYPE_KBD 7
#define FTYPE_TAP 8
#define FTYPE_BLK 9
-#define FTYPE_TPMFRONT 10
-#define FTYPE_N 11
+#define FTYPE_N 10
#define FTYPE_SPARE 16
struct file {
diff --git a/include/tpmfront.h b/include/tpmfront.h
index b7da50e3..a527371b 100644
--- a/include/tpmfront.h
+++ b/include/tpmfront.h
@@ -81,7 +81,6 @@ int tpmfront_cmd(struct tpmfront_dev* dev, uint8_t* req, size_t reqlen, uint8_t*
int tpmfront_set_locality(struct tpmfront_dev* dev, int locality);
#ifdef HAVE_LIBC
-#include <sys/stat.h>
/* POSIX IO functions:
* use tpmfront_open() to get a file descriptor to the tpm device
* use write() on the fd to send a command to the backend. You must
@@ -90,10 +89,6 @@ int tpmfront_set_locality(struct tpmfront_dev* dev, int locality);
* fstat() to get the size of the response and lseek() to seek on it.
*/
int tpmfront_open(struct tpmfront_dev* dev);
-int tpmfront_posix_read(int fd, uint8_t* buf, size_t count);
-int tpmfront_posix_write(int fd, const uint8_t* buf, size_t count);
-int tpmfront_posix_fstat(int fd, struct stat* buf);
#endif
-
#endif
diff --git a/lib/sys.c b/lib/sys.c
index 2fc81f20..39f51980 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -28,7 +28,6 @@
#include <netfront.h>
#include <blkfront.h>
#include <fbfront.h>
-#include <tpmfront.h>
#include <xenbus.h>
#include <xenstore.h>
#include <poll.h>
@@ -349,11 +348,6 @@ int read(int fd, void *buf, size_t nbytes)
case FTYPE_BLK: {
return blkfront_posix_read(fd, buf, nbytes);
}
-#endif
-#ifdef CONFIG_TPMFRONT
- case FTYPE_TPMFRONT: {
- return tpmfront_posix_read(fd, buf, nbytes);
- }
#endif
default:
break;
@@ -402,10 +396,6 @@ int write(int fd, const void *buf, size_t nbytes)
#ifdef CONFIG_BLKFRONT
case FTYPE_BLK:
return blkfront_posix_write(fd, buf, nbytes);
-#endif
-#ifdef CONFIG_TPMFRONT
- case FTYPE_TPMFRONT:
- return tpmfront_posix_write(fd, buf, nbytes);
#endif
default:
break;
@@ -468,10 +458,6 @@ off_t lseek(int fd, off_t offset, int whence)
#ifdef CONFIG_BLKFRONT
case FTYPE_BLK:
break;
-#endif
-#ifdef CONFIG_TPMFRONT
- case FTYPE_TPMFRONT:
- break;
#endif
case FTYPE_FILE:
break;
@@ -524,11 +510,6 @@ int close(int fd)
shutdown_blkfront(files[fd].dev);
break;
#endif
-#ifdef CONFIG_TPMFRONT
- case FTYPE_TPMFRONT:
- shutdown_tpmfront(files[fd].dev);
- break;
-#endif
#ifdef CONFIG_KBDFRONT
case FTYPE_KBD:
shutdown_kbdfront(files[fd].dev);
@@ -613,10 +594,6 @@ int fstat(int fd, struct stat *buf)
#ifdef CONFIG_BLKFRONT
case FTYPE_BLK:
return blkfront_posix_fstat(fd, buf);
-#endif
-#ifdef CONFIG_TPMFRONT
- case FTYPE_TPMFRONT:
- return tpmfront_posix_fstat(fd, buf);
#endif
default:
break;
diff --git a/tpmfront.c b/tpmfront.c
index 0a2fefc5..13163ad6 100644
--- a/tpmfront.c
+++ b/tpmfront.c
@@ -49,6 +49,10 @@
void tpmfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data) {
struct tpmfront_dev* dev = (struct tpmfront_dev*) data;
tpmif_shared_page_t *shr = dev->page;
+#ifdef HAVE_LIBC
+ struct file *file = get_file_from_fd(dev->fd);
+#endif
+
/*If we get a response when we didnt make a request, just ignore it */
if(!dev->waiting) {
return;
@@ -65,8 +69,9 @@ void tpmfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data) {
dev->waiting = 0;
#ifdef HAVE_LIBC
- if(dev->fd >= 0) {
- files[dev->fd].read = true;
+ if ( file )
+ {
+ file->read = true;
}
#endif
wake_up(&dev->waitq);
@@ -405,6 +410,10 @@ int tpmfront_send(struct tpmfront_dev* dev, const uint8_t* msg, size_t length)
#ifdef TPMFRONT_PRINT_DEBUG
int i;
#endif
+#ifdef HAVE_LIBC
+ struct file *file = dev ? get_file_from_fd(dev->fd) : NULL;
+#endif
+
/* Error Checking */
if(dev == NULL || dev->state != XenbusStateConnected) {
TPMFRONT_ERR("Tried to send message through disconnected frontend\n");
@@ -437,9 +446,10 @@ int tpmfront_send(struct tpmfront_dev* dev, const uint8_t* msg, size_t length)
dev->waiting = 1;
dev->resplen = 0;
#ifdef HAVE_LIBC
- if(dev->fd >= 0) {
- files[dev->fd].read = false;
- files[dev->fd].offset = 0;
+ if ( file )
+ {
+ file->read = false;
+ file->offset = 0;
dev->respgot = false;
}
#endif
@@ -529,25 +539,11 @@ int tpmfront_set_locality(struct tpmfront_dev* dev, int locality)
#ifdef HAVE_LIBC
#include <errno.h>
-int tpmfront_open(struct tpmfront_dev* dev)
-{
- /* Silently prevent multiple opens */
- if(dev->fd != -1) {
- return dev->fd;
- }
- dev->fd = alloc_fd(FTYPE_TPMFRONT);
- printk("tpmfront_open(%s) -> %d\n", dev->nodename, dev->fd);
- files[dev->fd].dev = dev;
- dev->respgot = false;
- return dev->fd;
-}
-
-int tpmfront_posix_write(int fd, const uint8_t* buf, size_t count)
+static int tpmfront_posix_write(struct file *file, const void *buf, size_t count)
{
int rc;
- struct tpmfront_dev* dev;
- dev = files[fd].dev;
+ struct tpmfront_dev *dev = file->dev;
if(count == 0) {
return 0;
@@ -566,14 +562,12 @@ int tpmfront_posix_write(int fd, const uint8_t* buf, size_t count)
return count;
}
-int tpmfront_posix_read(int fd, uint8_t* buf, size_t count)
+static int tpmfront_posix_read(struct file *file, void *buf, size_t count)
{
int rc;
uint8_t* dummybuf;
size_t dummysz;
- struct tpmfront_dev* dev;
-
- dev = files[fd].dev;
+ struct tpmfront_dev *dev = file->dev;
if(count == 0) {
return 0;
@@ -588,29 +582,32 @@ int tpmfront_posix_read(int fd, uint8_t* buf, size_t count)
}
/* handle EOF case */
- if(files[dev->fd].offset >= dev->resplen) {
+ if ( file->offset >= dev->resplen )
+ {
return 0;
}
/* Compute the number of bytes and do the copy operation */
- if((rc = min(count, dev->resplen - files[dev->fd].offset)) != 0) {
- memcpy(buf, dev->respbuf + files[dev->fd].offset, rc);
- files[dev->fd].offset += rc;
+ if ( (rc = min(count, dev->resplen - file->offset)) != 0 )
+ {
+ memcpy(buf, dev->respbuf + file->offset, rc);
+ file->offset += rc;
}
return rc;
}
-int tpmfront_posix_fstat(int fd, struct stat* buf)
+static int tpmfront_posix_fstat(struct file *file, struct stat *buf)
{
uint8_t* dummybuf;
size_t dummysz;
int rc;
- struct tpmfront_dev* dev = files[fd].dev;
+ struct tpmfront_dev *dev = file->dev;
/* If we have a response waiting, then read it now from the backend
* so we can get its length*/
- if(dev->waiting || (files[dev->fd].read && !dev->respgot)) {
+ if ( dev->waiting || (file->read && !dev->respgot) )
+ {
if ((rc = tpmfront_recv(dev, &dummybuf, &dummysz)) != 0) {
errno = EIO;
return -1;
@@ -626,5 +623,44 @@ int tpmfront_posix_fstat(int fd, struct stat* buf)
return 0;
}
+static int tpmfront_close_fd(struct file *file)
+{
+ shutdown_tpmfront(file->dev);
+
+ return 0;
+}
+
+static const struct file_ops tpmfront_ops = {
+ .name = "tpmfront",
+ .read = tpmfront_posix_read,
+ .write = tpmfront_posix_write,
+ .lseek = lseek_default,
+ .close = tpmfront_close_fd,
+ .fstat = tpmfront_posix_fstat,
+};
+
+static unsigned int ftype_tpmfront;
+
+__attribute__((constructor))
+static void tpmfront_initialize(void)
+{
+ ftype_tpmfront = alloc_file_type(&tpmfront_ops);
+}
+
+int tpmfront_open(struct tpmfront_dev *dev)
+{
+ struct file *file;
+
+ /* Silently prevent multiple opens */
+ if ( dev->fd != -1 )
+ return dev->fd;
+
+ dev->fd = alloc_fd(ftype_tpmfront);
+ printk("tpmfront_open(%s) -> %d\n", dev->nodename, dev->fd);
+ file = get_file_from_fd(dev->fd);
+ file->dev = dev;
+ dev->respgot = false;
+ return dev->fd;
+}
#endif
--
2.26.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 05/12] use alloc_file_type() and get_file_from_fd() in tpmfront
2022-01-16 8:33 ` [MINIOS PATCH v3 05/12] use alloc_file_type() and get_file_from_fd() in tpmfront Juergen Gross
@ 2022-01-16 20:57 ` Samuel Thibault
0 siblings, 0 replies; 29+ messages in thread
From: Samuel Thibault @ 2022-01-16 20:57 UTC (permalink / raw)
To: Juergen Gross; +Cc: minios-devel, xen-devel, wl
Juergen Gross, le dim. 16 janv. 2022 09:33:21 +0100, a ecrit:
> Allocate a file type dynamically via alloc_file_type().
>
> Instead of directly accessing the files[] array use get_file_from_fd().
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> V3:
> - switch to struct file * parameter for callbacks
> - use __attribute__((constructor))
> ---
> include/lib.h | 3 +-
> include/tpmfront.h | 5 ---
> lib/sys.c | 23 -----------
> tpmfront.c | 100 ++++++++++++++++++++++++++++++---------------
> 4 files changed, 69 insertions(+), 62 deletions(-)
>
> diff --git a/include/lib.h b/include/lib.h
> index 9dfb4689..9b7eaae7 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -167,8 +167,7 @@ extern struct wait_queue_head event_queue;
> #define FTYPE_KBD 7
> #define FTYPE_TAP 8
> #define FTYPE_BLK 9
> -#define FTYPE_TPMFRONT 10
> -#define FTYPE_N 11
> +#define FTYPE_N 10
> #define FTYPE_SPARE 16
>
> struct file {
> diff --git a/include/tpmfront.h b/include/tpmfront.h
> index b7da50e3..a527371b 100644
> --- a/include/tpmfront.h
> +++ b/include/tpmfront.h
> @@ -81,7 +81,6 @@ int tpmfront_cmd(struct tpmfront_dev* dev, uint8_t* req, size_t reqlen, uint8_t*
> int tpmfront_set_locality(struct tpmfront_dev* dev, int locality);
>
> #ifdef HAVE_LIBC
> -#include <sys/stat.h>
> /* POSIX IO functions:
> * use tpmfront_open() to get a file descriptor to the tpm device
> * use write() on the fd to send a command to the backend. You must
> @@ -90,10 +89,6 @@ int tpmfront_set_locality(struct tpmfront_dev* dev, int locality);
> * fstat() to get the size of the response and lseek() to seek on it.
> */
> int tpmfront_open(struct tpmfront_dev* dev);
> -int tpmfront_posix_read(int fd, uint8_t* buf, size_t count);
> -int tpmfront_posix_write(int fd, const uint8_t* buf, size_t count);
> -int tpmfront_posix_fstat(int fd, struct stat* buf);
> #endif
>
> -
> #endif
> diff --git a/lib/sys.c b/lib/sys.c
> index 2fc81f20..39f51980 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -28,7 +28,6 @@
> #include <netfront.h>
> #include <blkfront.h>
> #include <fbfront.h>
> -#include <tpmfront.h>
> #include <xenbus.h>
> #include <xenstore.h>
> #include <poll.h>
> @@ -349,11 +348,6 @@ int read(int fd, void *buf, size_t nbytes)
> case FTYPE_BLK: {
> return blkfront_posix_read(fd, buf, nbytes);
> }
> -#endif
> -#ifdef CONFIG_TPMFRONT
> - case FTYPE_TPMFRONT: {
> - return tpmfront_posix_read(fd, buf, nbytes);
> - }
> #endif
> default:
> break;
> @@ -402,10 +396,6 @@ int write(int fd, const void *buf, size_t nbytes)
> #ifdef CONFIG_BLKFRONT
> case FTYPE_BLK:
> return blkfront_posix_write(fd, buf, nbytes);
> -#endif
> -#ifdef CONFIG_TPMFRONT
> - case FTYPE_TPMFRONT:
> - return tpmfront_posix_write(fd, buf, nbytes);
> #endif
> default:
> break;
> @@ -468,10 +458,6 @@ off_t lseek(int fd, off_t offset, int whence)
> #ifdef CONFIG_BLKFRONT
> case FTYPE_BLK:
> break;
> -#endif
> -#ifdef CONFIG_TPMFRONT
> - case FTYPE_TPMFRONT:
> - break;
> #endif
> case FTYPE_FILE:
> break;
> @@ -524,11 +510,6 @@ int close(int fd)
> shutdown_blkfront(files[fd].dev);
> break;
> #endif
> -#ifdef CONFIG_TPMFRONT
> - case FTYPE_TPMFRONT:
> - shutdown_tpmfront(files[fd].dev);
> - break;
> -#endif
> #ifdef CONFIG_KBDFRONT
> case FTYPE_KBD:
> shutdown_kbdfront(files[fd].dev);
> @@ -613,10 +594,6 @@ int fstat(int fd, struct stat *buf)
> #ifdef CONFIG_BLKFRONT
> case FTYPE_BLK:
> return blkfront_posix_fstat(fd, buf);
> -#endif
> -#ifdef CONFIG_TPMFRONT
> - case FTYPE_TPMFRONT:
> - return tpmfront_posix_fstat(fd, buf);
> #endif
> default:
> break;
> diff --git a/tpmfront.c b/tpmfront.c
> index 0a2fefc5..13163ad6 100644
> --- a/tpmfront.c
> +++ b/tpmfront.c
> @@ -49,6 +49,10 @@
> void tpmfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data) {
> struct tpmfront_dev* dev = (struct tpmfront_dev*) data;
> tpmif_shared_page_t *shr = dev->page;
> +#ifdef HAVE_LIBC
> + struct file *file = get_file_from_fd(dev->fd);
> +#endif
> +
> /*If we get a response when we didnt make a request, just ignore it */
> if(!dev->waiting) {
> return;
> @@ -65,8 +69,9 @@ void tpmfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data) {
>
> dev->waiting = 0;
> #ifdef HAVE_LIBC
> - if(dev->fd >= 0) {
> - files[dev->fd].read = true;
> + if ( file )
> + {
> + file->read = true;
> }
> #endif
> wake_up(&dev->waitq);
> @@ -405,6 +410,10 @@ int tpmfront_send(struct tpmfront_dev* dev, const uint8_t* msg, size_t length)
> #ifdef TPMFRONT_PRINT_DEBUG
> int i;
> #endif
> +#ifdef HAVE_LIBC
> + struct file *file = dev ? get_file_from_fd(dev->fd) : NULL;
> +#endif
> +
> /* Error Checking */
> if(dev == NULL || dev->state != XenbusStateConnected) {
> TPMFRONT_ERR("Tried to send message through disconnected frontend\n");
> @@ -437,9 +446,10 @@ int tpmfront_send(struct tpmfront_dev* dev, const uint8_t* msg, size_t length)
> dev->waiting = 1;
> dev->resplen = 0;
> #ifdef HAVE_LIBC
> - if(dev->fd >= 0) {
> - files[dev->fd].read = false;
> - files[dev->fd].offset = 0;
> + if ( file )
> + {
> + file->read = false;
> + file->offset = 0;
> dev->respgot = false;
> }
> #endif
> @@ -529,25 +539,11 @@ int tpmfront_set_locality(struct tpmfront_dev* dev, int locality)
>
> #ifdef HAVE_LIBC
> #include <errno.h>
> -int tpmfront_open(struct tpmfront_dev* dev)
> -{
> - /* Silently prevent multiple opens */
> - if(dev->fd != -1) {
> - return dev->fd;
> - }
>
> - dev->fd = alloc_fd(FTYPE_TPMFRONT);
> - printk("tpmfront_open(%s) -> %d\n", dev->nodename, dev->fd);
> - files[dev->fd].dev = dev;
> - dev->respgot = false;
> - return dev->fd;
> -}
> -
> -int tpmfront_posix_write(int fd, const uint8_t* buf, size_t count)
> +static int tpmfront_posix_write(struct file *file, const void *buf, size_t count)
> {
> int rc;
> - struct tpmfront_dev* dev;
> - dev = files[fd].dev;
> + struct tpmfront_dev *dev = file->dev;
>
> if(count == 0) {
> return 0;
> @@ -566,14 +562,12 @@ int tpmfront_posix_write(int fd, const uint8_t* buf, size_t count)
> return count;
> }
>
> -int tpmfront_posix_read(int fd, uint8_t* buf, size_t count)
> +static int tpmfront_posix_read(struct file *file, void *buf, size_t count)
> {
> int rc;
> uint8_t* dummybuf;
> size_t dummysz;
> - struct tpmfront_dev* dev;
> -
> - dev = files[fd].dev;
> + struct tpmfront_dev *dev = file->dev;
>
> if(count == 0) {
> return 0;
> @@ -588,29 +582,32 @@ int tpmfront_posix_read(int fd, uint8_t* buf, size_t count)
> }
>
> /* handle EOF case */
> - if(files[dev->fd].offset >= dev->resplen) {
> + if ( file->offset >= dev->resplen )
> + {
> return 0;
> }
>
> /* Compute the number of bytes and do the copy operation */
> - if((rc = min(count, dev->resplen - files[dev->fd].offset)) != 0) {
> - memcpy(buf, dev->respbuf + files[dev->fd].offset, rc);
> - files[dev->fd].offset += rc;
> + if ( (rc = min(count, dev->resplen - file->offset)) != 0 )
> + {
> + memcpy(buf, dev->respbuf + file->offset, rc);
> + file->offset += rc;
> }
>
> return rc;
> }
>
> -int tpmfront_posix_fstat(int fd, struct stat* buf)
> +static int tpmfront_posix_fstat(struct file *file, struct stat *buf)
> {
> uint8_t* dummybuf;
> size_t dummysz;
> int rc;
> - struct tpmfront_dev* dev = files[fd].dev;
> + struct tpmfront_dev *dev = file->dev;
>
> /* If we have a response waiting, then read it now from the backend
> * so we can get its length*/
> - if(dev->waiting || (files[dev->fd].read && !dev->respgot)) {
> + if ( dev->waiting || (file->read && !dev->respgot) )
> + {
> if ((rc = tpmfront_recv(dev, &dummybuf, &dummysz)) != 0) {
> errno = EIO;
> return -1;
> @@ -626,5 +623,44 @@ int tpmfront_posix_fstat(int fd, struct stat* buf)
> return 0;
> }
>
> +static int tpmfront_close_fd(struct file *file)
> +{
> + shutdown_tpmfront(file->dev);
> +
> + return 0;
> +}
> +
> +static const struct file_ops tpmfront_ops = {
> + .name = "tpmfront",
> + .read = tpmfront_posix_read,
> + .write = tpmfront_posix_write,
> + .lseek = lseek_default,
> + .close = tpmfront_close_fd,
> + .fstat = tpmfront_posix_fstat,
> +};
> +
> +static unsigned int ftype_tpmfront;
> +
> +__attribute__((constructor))
> +static void tpmfront_initialize(void)
> +{
> + ftype_tpmfront = alloc_file_type(&tpmfront_ops);
> +}
> +
> +int tpmfront_open(struct tpmfront_dev *dev)
> +{
> + struct file *file;
> +
> + /* Silently prevent multiple opens */
> + if ( dev->fd != -1 )
> + return dev->fd;
> +
> + dev->fd = alloc_fd(ftype_tpmfront);
> + printk("tpmfront_open(%s) -> %d\n", dev->nodename, dev->fd);
> + file = get_file_from_fd(dev->fd);
> + file->dev = dev;
> + dev->respgot = false;
> + return dev->fd;
> +}
>
> #endif
> --
> 2.26.2
>
--
Samuel
requests.</FONT></SPAN></TD></TR></TBODY></TABLE></DIV></BODY></HTML>agnjo
gj a po mi
shnthdrdcvallus hsx mvgduwolgfwtq
uzuy
s
p
h
-+- spams forever ... -+-
^ permalink raw reply [flat|nested] 29+ messages in thread
* [MINIOS PATCH v3 06/12] use alloc_file_type() and get_file_from_fd() in blkfront
2022-01-16 8:33 [MINIOS PATCH v3 00/12] remove device specific struct file members Juergen Gross
` (4 preceding siblings ...)
2022-01-16 8:33 ` [MINIOS PATCH v3 05/12] use alloc_file_type() and get_file_from_fd() in tpmfront Juergen Gross
@ 2022-01-16 8:33 ` Juergen Gross
2022-01-16 20:58 ` Samuel Thibault
2022-01-18 14:41 ` Andrew Cooper
2022-01-16 8:33 ` [MINIOS PATCH v3 07/12] use alloc_file_type() and get_file_from_fd() in netfront Juergen Gross
` (6 subsequent siblings)
12 siblings, 2 replies; 29+ messages in thread
From: Juergen Gross @ 2022-01-16 8:33 UTC (permalink / raw)
To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross
Allocate the file type dynamically via alloc_file_type().
Instead of directly accessing the files[] array use get_file_from_fd().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- switch to struct file * parameter for callbacks
- use __attribute__((constructor))
---
blkfront.c | 92 ++++++++++++++++++++++++++++++++++------------
include/blkfront.h | 5 ---
include/lib.h | 3 +-
lib/sys.c | 24 ------------
4 files changed, 69 insertions(+), 55 deletions(-)
diff --git a/blkfront.c b/blkfront.c
index e3f42bef..ed902702 100644
--- a/blkfront.c
+++ b/blkfront.c
@@ -59,10 +59,10 @@ void blkfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
{
#ifdef HAVE_LIBC
struct blkfront_dev *dev = data;
- int fd = dev->fd;
+ struct file *file = get_file_from_fd(dev->fd);
- if (fd != -1)
- files[fd].read = true;
+ if ( file )
+ file->read = true;
#endif
wake_up(&blkfront_queue);
}
@@ -483,9 +483,13 @@ int blkfront_aio_poll(struct blkfront_dev *dev)
moretodo:
#ifdef HAVE_LIBC
- if (dev->fd != -1) {
- files[dev->fd].read = false;
- mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */
+ {
+ struct file *file = get_file_from_fd(dev->fd);
+
+ if ( file ) {
+ file->read = false;
+ mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */
+ }
}
#endif
@@ -554,22 +558,11 @@ moretodo:
}
#ifdef HAVE_LIBC
-int blkfront_open(struct blkfront_dev *dev)
+static int blkfront_posix_rwop(struct file *file, uint8_t *buf, size_t count,
+ bool write)
{
- /* Silently prevent multiple opens */
- if(dev->fd != -1) {
- return dev->fd;
- }
- dev->fd = alloc_fd(FTYPE_BLK);
- printk("blk_open(%s) -> %d\n", dev->nodename, dev->fd);
- files[dev->fd].dev = dev;
- return dev->fd;
-}
-
-int blkfront_posix_rwop(int fd, uint8_t* buf, size_t count, int write)
-{
- struct blkfront_dev* dev = files[fd].dev;
- off_t offset = files[fd].offset;
+ struct blkfront_dev *dev = file->dev;
+ off_t offset = file->offset;
struct blkfront_aiocb aiocb;
unsigned long long disksize = dev->info.sectors * dev->info.sector_size;
unsigned int blocksize = dev->info.sector_size;
@@ -711,14 +704,24 @@ int blkfront_posix_rwop(int fd, uint8_t* buf, size_t count, int write)
}
free(copybuf);
- files[fd].offset += rc;
+ file->offset += rc;
return rc;
}
-int blkfront_posix_fstat(int fd, struct stat* buf)
+static int blkfront_posix_read(struct file *file, void *buf, size_t nbytes)
{
- struct blkfront_dev* dev = files[fd].dev;
+ return blkfront_posix_rwop(file, buf, nbytes, false);
+}
+
+static int blkfront_posix_write(struct file *file, const void *buf, size_t nbytes)
+{
+ return blkfront_posix_rwop(file, (void *)buf, nbytes, true);
+}
+
+static int blkfront_posix_fstat(struct file *file, struct stat *buf)
+{
+ struct blkfront_dev *dev = file->dev;
buf->st_mode = dev->info.mode;
buf->st_uid = 0;
@@ -728,4 +731,45 @@ int blkfront_posix_fstat(int fd, struct stat* buf)
return 0;
}
+
+static int blkfront_close_fd(struct file *file)
+{
+ shutdown_blkfront(file->dev);
+
+ return 0;
+}
+
+static const struct file_ops blk_ops = {
+ .name = "blk",
+ .read = blkfront_posix_read,
+ .write = blkfront_posix_write,
+ .lseek = lseek_default,
+ .close = blkfront_close_fd,
+ .fstat = blkfront_posix_fstat,
+ .select_rd = select_read_flag,
+};
+
+static unsigned int ftype_blk;
+
+__attribute__((constructor))
+static void blkfron_initialize(void)
+{
+ ftype_blk = alloc_file_type(&blk_ops);
+}
+
+int blkfront_open(struct blkfront_dev *dev)
+{
+ struct file *file;
+
+ /* Silently prevent multiple opens */
+ if ( dev->fd != -1 )
+ return dev->fd;
+
+ dev->fd = alloc_fd(ftype_blk);
+ printk("blk_open(%s) -> %d\n", dev->nodename, dev->fd);
+ file = get_file_from_fd(dev->fd);
+ file->dev = dev;
+
+ return dev->fd;
+}
#endif
diff --git a/include/blkfront.h b/include/blkfront.h
index 3528af9d..7f84a0a2 100644
--- a/include/blkfront.h
+++ b/include/blkfront.h
@@ -28,17 +28,12 @@ struct blkfront_info
};
struct blkfront_dev *init_blkfront(char *nodename, struct blkfront_info *info);
#ifdef HAVE_LIBC
-#include <sys/stat.h>
/* POSIX IO functions:
* use blkfront_open() to get a file descriptor to the block device
* Don't use the other blkfront posix functions here directly, instead use
* read(), write(), lseek() and fstat() on the file descriptor
*/
int blkfront_open(struct blkfront_dev *dev);
-int blkfront_posix_rwop(int fd, uint8_t* buf, size_t count, int write);
-#define blkfront_posix_write(fd, buf, count) blkfront_posix_rwop(fd, (uint8_t*)buf, count, 1)
-#define blkfront_posix_read(fd, buf, count) blkfront_posix_rwop(fd, (uint8_t*)buf, count, 0)
-int blkfront_posix_fstat(int fd, struct stat* buf);
#endif
void blkfront_aio(struct blkfront_aiocb *aiocbp, int write);
#define blkfront_aio_read(aiocbp) blkfront_aio(aiocbp, 0)
diff --git a/include/lib.h b/include/lib.h
index 9b7eaae7..2f472b00 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -166,8 +166,7 @@ extern struct wait_queue_head event_queue;
#define FTYPE_FB 6
#define FTYPE_KBD 7
#define FTYPE_TAP 8
-#define FTYPE_BLK 9
-#define FTYPE_N 10
+#define FTYPE_N 9
#define FTYPE_SPARE 16
struct file {
diff --git a/lib/sys.c b/lib/sys.c
index 39f51980..90684335 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -343,11 +343,6 @@ int read(int fd, void *buf, size_t nbytes)
}
return ret * sizeof(union xenfb_in_event);
}
-#endif
-#ifdef CONFIG_BLKFRONT
- case FTYPE_BLK: {
- return blkfront_posix_read(fd, buf, nbytes);
- }
#endif
default:
break;
@@ -392,10 +387,6 @@ int write(int fd, const void *buf, size_t nbytes)
case FTYPE_TAP:
netfront_xmit(files[fd].dev, (void*) buf, nbytes);
return nbytes;
-#endif
-#ifdef CONFIG_BLKFRONT
- case FTYPE_BLK:
- return blkfront_posix_write(fd, buf, nbytes);
#endif
default:
break;
@@ -455,10 +446,6 @@ off_t lseek(int fd, off_t offset, int whence)
return ops->lseek(file, offset, whence);
switch(file->type) {
-#ifdef CONFIG_BLKFRONT
- case FTYPE_BLK:
- break;
-#endif
case FTYPE_FILE:
break;
default:
@@ -505,11 +492,6 @@ int close(int fd)
shutdown_netfront(files[fd].dev);
break;
#endif
-#ifdef CONFIG_BLKFRONT
- case FTYPE_BLK:
- shutdown_blkfront(files[fd].dev);
- break;
-#endif
#ifdef CONFIG_KBDFRONT
case FTYPE_KBD:
shutdown_kbdfront(files[fd].dev);
@@ -591,10 +573,6 @@ int fstat(int fd, struct stat *buf)
buf->st_ctime = time(NULL);
return 0;
}
-#ifdef CONFIG_BLKFRONT
- case FTYPE_BLK:
- return blkfront_posix_fstat(fd, buf);
-#endif
default:
break;
}
@@ -710,7 +688,6 @@ static const char *const file_types[] = {
[FTYPE_CONSOLE] = "console",
[FTYPE_SOCKET] = "socket",
[FTYPE_TAP] = "net",
- [FTYPE_BLK] = "blk",
[FTYPE_KBD] = "kbd",
[FTYPE_FB] = "fb",
};
@@ -896,7 +873,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
FD_CLR(i, exceptfds);
break;
case FTYPE_TAP:
- case FTYPE_BLK:
case FTYPE_KBD:
case FTYPE_FB:
if (FD_ISSET(i, readfds)) {
--
2.26.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 06/12] use alloc_file_type() and get_file_from_fd() in blkfront
2022-01-16 8:33 ` [MINIOS PATCH v3 06/12] use alloc_file_type() and get_file_from_fd() in blkfront Juergen Gross
@ 2022-01-16 20:58 ` Samuel Thibault
2022-01-18 14:41 ` Andrew Cooper
1 sibling, 0 replies; 29+ messages in thread
From: Samuel Thibault @ 2022-01-16 20:58 UTC (permalink / raw)
To: Juergen Gross; +Cc: minios-devel, xen-devel, wl
Juergen Gross, le dim. 16 janv. 2022 09:33:22 +0100, a ecrit:
> Allocate the file type dynamically via alloc_file_type().
>
> Instead of directly accessing the files[] array use get_file_from_fd().
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> V3:
> - switch to struct file * parameter for callbacks
> - use __attribute__((constructor))
> ---
> blkfront.c | 92 ++++++++++++++++++++++++++++++++++------------
> include/blkfront.h | 5 ---
> include/lib.h | 3 +-
> lib/sys.c | 24 ------------
> 4 files changed, 69 insertions(+), 55 deletions(-)
>
> diff --git a/blkfront.c b/blkfront.c
> index e3f42bef..ed902702 100644
> --- a/blkfront.c
> +++ b/blkfront.c
> @@ -59,10 +59,10 @@ void blkfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
> {
> #ifdef HAVE_LIBC
> struct blkfront_dev *dev = data;
> - int fd = dev->fd;
> + struct file *file = get_file_from_fd(dev->fd);
>
> - if (fd != -1)
> - files[fd].read = true;
> + if ( file )
> + file->read = true;
> #endif
> wake_up(&blkfront_queue);
> }
> @@ -483,9 +483,13 @@ int blkfront_aio_poll(struct blkfront_dev *dev)
>
> moretodo:
> #ifdef HAVE_LIBC
> - if (dev->fd != -1) {
> - files[dev->fd].read = false;
> - mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */
> + {
> + struct file *file = get_file_from_fd(dev->fd);
> +
> + if ( file ) {
> + file->read = false;
> + mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */
> + }
> }
> #endif
>
> @@ -554,22 +558,11 @@ moretodo:
> }
>
> #ifdef HAVE_LIBC
> -int blkfront_open(struct blkfront_dev *dev)
> +static int blkfront_posix_rwop(struct file *file, uint8_t *buf, size_t count,
> + bool write)
> {
> - /* Silently prevent multiple opens */
> - if(dev->fd != -1) {
> - return dev->fd;
> - }
> - dev->fd = alloc_fd(FTYPE_BLK);
> - printk("blk_open(%s) -> %d\n", dev->nodename, dev->fd);
> - files[dev->fd].dev = dev;
> - return dev->fd;
> -}
> -
> -int blkfront_posix_rwop(int fd, uint8_t* buf, size_t count, int write)
> -{
> - struct blkfront_dev* dev = files[fd].dev;
> - off_t offset = files[fd].offset;
> + struct blkfront_dev *dev = file->dev;
> + off_t offset = file->offset;
> struct blkfront_aiocb aiocb;
> unsigned long long disksize = dev->info.sectors * dev->info.sector_size;
> unsigned int blocksize = dev->info.sector_size;
> @@ -711,14 +704,24 @@ int blkfront_posix_rwop(int fd, uint8_t* buf, size_t count, int write)
> }
>
> free(copybuf);
> - files[fd].offset += rc;
> + file->offset += rc;
> return rc;
>
> }
>
> -int blkfront_posix_fstat(int fd, struct stat* buf)
> +static int blkfront_posix_read(struct file *file, void *buf, size_t nbytes)
> {
> - struct blkfront_dev* dev = files[fd].dev;
> + return blkfront_posix_rwop(file, buf, nbytes, false);
> +}
> +
> +static int blkfront_posix_write(struct file *file, const void *buf, size_t nbytes)
> +{
> + return blkfront_posix_rwop(file, (void *)buf, nbytes, true);
> +}
> +
> +static int blkfront_posix_fstat(struct file *file, struct stat *buf)
> +{
> + struct blkfront_dev *dev = file->dev;
>
> buf->st_mode = dev->info.mode;
> buf->st_uid = 0;
> @@ -728,4 +731,45 @@ int blkfront_posix_fstat(int fd, struct stat* buf)
>
> return 0;
> }
> +
> +static int blkfront_close_fd(struct file *file)
> +{
> + shutdown_blkfront(file->dev);
> +
> + return 0;
> +}
> +
> +static const struct file_ops blk_ops = {
> + .name = "blk",
> + .read = blkfront_posix_read,
> + .write = blkfront_posix_write,
> + .lseek = lseek_default,
> + .close = blkfront_close_fd,
> + .fstat = blkfront_posix_fstat,
> + .select_rd = select_read_flag,
> +};
> +
> +static unsigned int ftype_blk;
> +
> +__attribute__((constructor))
> +static void blkfron_initialize(void)
> +{
> + ftype_blk = alloc_file_type(&blk_ops);
> +}
> +
> +int blkfront_open(struct blkfront_dev *dev)
> +{
> + struct file *file;
> +
> + /* Silently prevent multiple opens */
> + if ( dev->fd != -1 )
> + return dev->fd;
> +
> + dev->fd = alloc_fd(ftype_blk);
> + printk("blk_open(%s) -> %d\n", dev->nodename, dev->fd);
> + file = get_file_from_fd(dev->fd);
> + file->dev = dev;
> +
> + return dev->fd;
> +}
> #endif
> diff --git a/include/blkfront.h b/include/blkfront.h
> index 3528af9d..7f84a0a2 100644
> --- a/include/blkfront.h
> +++ b/include/blkfront.h
> @@ -28,17 +28,12 @@ struct blkfront_info
> };
> struct blkfront_dev *init_blkfront(char *nodename, struct blkfront_info *info);
> #ifdef HAVE_LIBC
> -#include <sys/stat.h>
> /* POSIX IO functions:
> * use blkfront_open() to get a file descriptor to the block device
> * Don't use the other blkfront posix functions here directly, instead use
> * read(), write(), lseek() and fstat() on the file descriptor
> */
> int blkfront_open(struct blkfront_dev *dev);
> -int blkfront_posix_rwop(int fd, uint8_t* buf, size_t count, int write);
> -#define blkfront_posix_write(fd, buf, count) blkfront_posix_rwop(fd, (uint8_t*)buf, count, 1)
> -#define blkfront_posix_read(fd, buf, count) blkfront_posix_rwop(fd, (uint8_t*)buf, count, 0)
> -int blkfront_posix_fstat(int fd, struct stat* buf);
> #endif
> void blkfront_aio(struct blkfront_aiocb *aiocbp, int write);
> #define blkfront_aio_read(aiocbp) blkfront_aio(aiocbp, 0)
> diff --git a/include/lib.h b/include/lib.h
> index 9b7eaae7..2f472b00 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -166,8 +166,7 @@ extern struct wait_queue_head event_queue;
> #define FTYPE_FB 6
> #define FTYPE_KBD 7
> #define FTYPE_TAP 8
> -#define FTYPE_BLK 9
> -#define FTYPE_N 10
> +#define FTYPE_N 9
> #define FTYPE_SPARE 16
>
> struct file {
> diff --git a/lib/sys.c b/lib/sys.c
> index 39f51980..90684335 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -343,11 +343,6 @@ int read(int fd, void *buf, size_t nbytes)
> }
> return ret * sizeof(union xenfb_in_event);
> }
> -#endif
> -#ifdef CONFIG_BLKFRONT
> - case FTYPE_BLK: {
> - return blkfront_posix_read(fd, buf, nbytes);
> - }
> #endif
> default:
> break;
> @@ -392,10 +387,6 @@ int write(int fd, const void *buf, size_t nbytes)
> case FTYPE_TAP:
> netfront_xmit(files[fd].dev, (void*) buf, nbytes);
> return nbytes;
> -#endif
> -#ifdef CONFIG_BLKFRONT
> - case FTYPE_BLK:
> - return blkfront_posix_write(fd, buf, nbytes);
> #endif
> default:
> break;
> @@ -455,10 +446,6 @@ off_t lseek(int fd, off_t offset, int whence)
> return ops->lseek(file, offset, whence);
>
> switch(file->type) {
> -#ifdef CONFIG_BLKFRONT
> - case FTYPE_BLK:
> - break;
> -#endif
> case FTYPE_FILE:
> break;
> default:
> @@ -505,11 +492,6 @@ int close(int fd)
> shutdown_netfront(files[fd].dev);
> break;
> #endif
> -#ifdef CONFIG_BLKFRONT
> - case FTYPE_BLK:
> - shutdown_blkfront(files[fd].dev);
> - break;
> -#endif
> #ifdef CONFIG_KBDFRONT
> case FTYPE_KBD:
> shutdown_kbdfront(files[fd].dev);
> @@ -591,10 +573,6 @@ int fstat(int fd, struct stat *buf)
> buf->st_ctime = time(NULL);
> return 0;
> }
> -#ifdef CONFIG_BLKFRONT
> - case FTYPE_BLK:
> - return blkfront_posix_fstat(fd, buf);
> -#endif
> default:
> break;
> }
> @@ -710,7 +688,6 @@ static const char *const file_types[] = {
> [FTYPE_CONSOLE] = "console",
> [FTYPE_SOCKET] = "socket",
> [FTYPE_TAP] = "net",
> - [FTYPE_BLK] = "blk",
> [FTYPE_KBD] = "kbd",
> [FTYPE_FB] = "fb",
> };
> @@ -896,7 +873,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
> FD_CLR(i, exceptfds);
> break;
> case FTYPE_TAP:
> - case FTYPE_BLK:
> case FTYPE_KBD:
> case FTYPE_FB:
> if (FD_ISSET(i, readfds)) {
> --
> 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.)
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 06/12] use alloc_file_type() and get_file_from_fd() in blkfront
2022-01-16 8:33 ` [MINIOS PATCH v3 06/12] use alloc_file_type() and get_file_from_fd() in blkfront Juergen Gross
2022-01-16 20:58 ` Samuel Thibault
@ 2022-01-18 14:41 ` Andrew Cooper
2022-01-18 14:55 ` Juergen Gross
1 sibling, 1 reply; 29+ messages in thread
From: Andrew Cooper @ 2022-01-18 14:41 UTC (permalink / raw)
To: Juergen Gross, minios-devel, xen-devel; +Cc: samuel.thibault, wl
On 16/01/2022 08:33, Juergen Gross wrote:
> diff --git a/blkfront.c b/blkfront.c
> index e3f42bef..ed902702 100644
> --- a/blkfront.c
> +++ b/blkfront.c
> @@ -483,9 +483,13 @@ int blkfront_aio_poll(struct blkfront_dev *dev)
>
> moretodo:
> #ifdef HAVE_LIBC
> - if (dev->fd != -1) {
> - files[dev->fd].read = false;
> - mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */
> + {
> + struct file *file = get_file_from_fd(dev->fd);
> +
> + if ( file ) {
Style, as this is is being indented.
> + file->read = false;
> + mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */
MiniOS is in desperate need of being dragged over to LKMM, and to stop
using inappropriate fences. Things will go much faster when these have
all been corrected to smp barriers.
~Andrew
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 06/12] use alloc_file_type() and get_file_from_fd() in blkfront
2022-01-18 14:41 ` Andrew Cooper
@ 2022-01-18 14:55 ` Juergen Gross
0 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-01-18 14:55 UTC (permalink / raw)
To: Andrew Cooper, minios-devel, xen-devel; +Cc: samuel.thibault, wl
[-- Attachment #1.1.1: Type: text/plain, Size: 1044 bytes --]
On 18.01.22 15:41, Andrew Cooper wrote:
> On 16/01/2022 08:33, Juergen Gross wrote:
>> diff --git a/blkfront.c b/blkfront.c
>> index e3f42bef..ed902702 100644
>> --- a/blkfront.c
>> +++ b/blkfront.c
>> @@ -483,9 +483,13 @@ int blkfront_aio_poll(struct blkfront_dev *dev)
>>
>> moretodo:
>> #ifdef HAVE_LIBC
>> - if (dev->fd != -1) {
>> - files[dev->fd].read = false;
>> - mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */
>> + {
>> + struct file *file = get_file_from_fd(dev->fd);
>> +
>> + if ( file ) {
>
> Style, as this is is being indented.
Oh, missed that one. :-(
>
>> + file->read = false;
>> + mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */
>
> MiniOS is in desperate need of being dragged over to LKMM, and to stop
> using inappropriate fences. Things will go much faster when these have
> all been corrected to smp barriers.
Indeed.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* [MINIOS PATCH v3 07/12] use alloc_file_type() and get_file_from_fd() in netfront
2022-01-16 8:33 [MINIOS PATCH v3 00/12] remove device specific struct file members Juergen Gross
` (5 preceding siblings ...)
2022-01-16 8:33 ` [MINIOS PATCH v3 06/12] use alloc_file_type() and get_file_from_fd() in blkfront Juergen Gross
@ 2022-01-16 8:33 ` Juergen Gross
2022-01-16 20:59 ` Samuel Thibault
2022-01-16 8:33 ` [MINIOS PATCH v3 08/12] use alloc_file_type() and get_file_from_fd() in fbfront Juergen Gross
` (5 subsequent siblings)
12 siblings, 1 reply; 29+ messages in thread
From: Juergen Gross @ 2022-01-16 8:33 UTC (permalink / raw)
To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross
Allocate the file type dynamically via alloc_file_type().
Instead of directly accessing the files[] array use get_file_from_fd().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- switch to struct file * parameter for callbacks
- use __attribute__((constructor))
---
include/lib.h | 3 +--
include/netfront.h | 3 ++-
lib/sys.c | 23 ----------------
netfront.c | 65 +++++++++++++++++++++++++++++++++++++++-------
4 files changed, 59 insertions(+), 35 deletions(-)
diff --git a/include/lib.h b/include/lib.h
index 2f472b00..b0d08807 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -165,8 +165,7 @@ extern struct wait_queue_head event_queue;
#define FTYPE_SAVEFILE 5
#define FTYPE_FB 6
#define FTYPE_KBD 7
-#define FTYPE_TAP 8
-#define FTYPE_N 9
+#define FTYPE_N 8
#define FTYPE_SPARE 16
struct file {
diff --git a/include/netfront.h b/include/netfront.h
index ec641c80..75080c26 100644
--- a/include/netfront.h
+++ b/include/netfront.h
@@ -10,7 +10,8 @@ struct netfront_dev *init_netfront(char *nodename,
char **ip);
char *netfront_get_netmask(struct netfront_dev *dev);
char *netfront_get_gateway(struct netfront_dev *dev);
-void netfront_xmit(struct netfront_dev *dev, unsigned char* data,int len);
+void netfront_xmit(struct netfront_dev *dev, const unsigned char *data,
+ int len);
void shutdown_netfront(struct netfront_dev *dev);
void suspend_netfront(void);
void resume_netfront(void);
diff --git a/lib/sys.c b/lib/sys.c
index 90684335..44fe848c 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -309,17 +309,6 @@ int read(int fd, void *buf, size_t nbytes)
case FTYPE_SOCKET:
return lwip_read(files[fd].fd, buf, nbytes);
#endif
-#ifdef CONFIG_NETFRONT
- case FTYPE_TAP: {
- ssize_t ret;
- ret = netfront_receive(files[fd].dev, buf, nbytes);
- if (ret <= 0) {
- errno = EAGAIN;
- return -1;
- }
- return ret;
- }
-#endif
#ifdef CONFIG_KBDFRONT
case FTYPE_KBD: {
int ret, n;
@@ -382,11 +371,6 @@ int write(int fd, const void *buf, size_t nbytes)
#ifdef HAVE_LWIP
case FTYPE_SOCKET:
return lwip_write(files[fd].fd, (void*) buf, nbytes);
-#endif
-#ifdef CONFIG_NETFRONT
- case FTYPE_TAP:
- netfront_xmit(files[fd].dev, (void*) buf, nbytes);
- return nbytes;
#endif
default:
break;
@@ -487,11 +471,6 @@ int close(int fd)
res = lwip_close(files[fd].fd);
break;
#endif
-#ifdef CONFIG_NETFRONT
- case FTYPE_TAP:
- shutdown_netfront(files[fd].dev);
- break;
-#endif
#ifdef CONFIG_KBDFRONT
case FTYPE_KBD:
shutdown_kbdfront(files[fd].dev);
@@ -687,7 +666,6 @@ static const char *const file_types[] = {
[FTYPE_NONE] = "none",
[FTYPE_CONSOLE] = "console",
[FTYPE_SOCKET] = "socket",
- [FTYPE_TAP] = "net",
[FTYPE_KBD] = "kbd",
[FTYPE_FB] = "fb",
};
@@ -872,7 +850,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
n++;
FD_CLR(i, exceptfds);
break;
- case FTYPE_TAP:
case FTYPE_KBD:
case FTYPE_FB:
if (FD_ISSET(i, readfds)) {
diff --git a/netfront.c b/netfront.c
index 76964514..164fdc66 100644
--- a/netfront.c
+++ b/netfront.c
@@ -248,14 +248,14 @@ void netfront_select_handler(evtchn_port_t port, struct pt_regs *regs, void *dat
{
int flags;
struct netfront_dev *dev = data;
- int fd = dev->fd;
+ struct file *file = get_file_from_fd(dev->fd);
local_irq_save(flags);
network_tx_buf_gc(dev);
local_irq_restore(flags);
- if (fd != -1)
- files[fd].read = true;
+ if ( file )
+ file->read = true;
wake_up(&netfront_queue);
}
#endif
@@ -565,8 +565,53 @@ error:
}
#ifdef HAVE_LIBC
+static int netfront_read(struct file *file, void *buf, size_t nbytes)
+{
+ ssize_t ret;
+
+ ret = netfront_receive(file->dev, buf, nbytes);
+ if ( ret <= 0 )
+ {
+ errno = EAGAIN;
+ return -1;
+ }
+
+ return ret;
+}
+
+static int netfront_write(struct file *file, const void *buf, size_t nbytes)
+{
+ netfront_xmit(file->dev, buf, nbytes);
+
+ return nbytes;
+}
+
+static int netfront_close_fd(struct file *file)
+{
+ shutdown_netfront(file->dev);
+
+ return 0;
+}
+
+static const struct file_ops netfront_ops = {
+ .name = "net",
+ .read = netfront_read,
+ .write = netfront_write,
+ .close = netfront_close_fd,
+ .select_rd = select_read_flag,
+};
+
+static unsigned int ftype_netfront;
+
+__attribute__((constructor))
+static void netfront_initialize(void)
+{
+ ftype_netfront = alloc_file_type(&netfront_ops);
+}
+
int netfront_tap_open(char *nodename) {
struct netfront_dev *dev;
+ struct file *file;
dev = init_netfront(nodename, NETIF_SELECT_RX, NULL, NULL);
if (!dev) {
@@ -574,9 +619,10 @@ int netfront_tap_open(char *nodename) {
errno = EIO;
return -1;
}
- dev->fd = alloc_fd(FTYPE_TAP);
+ dev->fd = alloc_fd(ftype_netfront);
printk("tap_open(%s) -> %d\n", nodename, dev->fd);
- files[dev->fd].dev = dev;
+ file = get_file_from_fd(dev->fd);
+ file->dev = dev;
return dev->fd;
}
#endif
@@ -720,7 +766,7 @@ void init_rx_buffers(struct netfront_dev *dev)
}
-void netfront_xmit(struct netfront_dev *dev, unsigned char* data,int len)
+void netfront_xmit(struct netfront_dev *dev, const unsigned char *data, int len)
{
int flags;
struct netif_tx_request *tx;
@@ -772,7 +818,8 @@ void netfront_xmit(struct netfront_dev *dev, unsigned char* data,int len)
ssize_t netfront_receive(struct netfront_dev *dev, unsigned char *data, size_t len)
{
unsigned long flags;
- int fd = dev->fd;
+ struct file *file = get_file_from_fd(dev->fd);
+
ASSERT(current == main_thread);
dev->rlen = 0;
@@ -781,9 +828,9 @@ ssize_t netfront_receive(struct netfront_dev *dev, unsigned char *data, size_t l
local_irq_save(flags);
network_rx(dev);
- if (!dev->rlen && fd != -1)
+ if ( !dev->rlen && file )
/* No data for us, make select stop returning */
- files[fd].read = false;
+ file->read = false;
/* Before re-enabling the interrupts, in case a packet just arrived in the
* meanwhile. */
local_irq_restore(flags);
--
2.26.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 07/12] use alloc_file_type() and get_file_from_fd() in netfront
2022-01-16 8:33 ` [MINIOS PATCH v3 07/12] use alloc_file_type() and get_file_from_fd() in netfront Juergen Gross
@ 2022-01-16 20:59 ` Samuel Thibault
0 siblings, 0 replies; 29+ messages in thread
From: Samuel Thibault @ 2022-01-16 20:59 UTC (permalink / raw)
To: Juergen Gross; +Cc: minios-devel, xen-devel, wl
Juergen Gross, le dim. 16 janv. 2022 09:33:23 +0100, a ecrit:
> Allocate the file type dynamically via alloc_file_type().
>
> Instead of directly accessing the files[] array use get_file_from_fd().
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> V3:
> - switch to struct file * parameter for callbacks
> - use __attribute__((constructor))
> ---
> include/lib.h | 3 +--
> include/netfront.h | 3 ++-
> lib/sys.c | 23 ----------------
> netfront.c | 65 +++++++++++++++++++++++++++++++++++++++-------
> 4 files changed, 59 insertions(+), 35 deletions(-)
>
> diff --git a/include/lib.h b/include/lib.h
> index 2f472b00..b0d08807 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -165,8 +165,7 @@ extern struct wait_queue_head event_queue;
> #define FTYPE_SAVEFILE 5
> #define FTYPE_FB 6
> #define FTYPE_KBD 7
> -#define FTYPE_TAP 8
> -#define FTYPE_N 9
> +#define FTYPE_N 8
> #define FTYPE_SPARE 16
>
> struct file {
> diff --git a/include/netfront.h b/include/netfront.h
> index ec641c80..75080c26 100644
> --- a/include/netfront.h
> +++ b/include/netfront.h
> @@ -10,7 +10,8 @@ struct netfront_dev *init_netfront(char *nodename,
> char **ip);
> char *netfront_get_netmask(struct netfront_dev *dev);
> char *netfront_get_gateway(struct netfront_dev *dev);
> -void netfront_xmit(struct netfront_dev *dev, unsigned char* data,int len);
> +void netfront_xmit(struct netfront_dev *dev, const unsigned char *data,
> + int len);
> void shutdown_netfront(struct netfront_dev *dev);
> void suspend_netfront(void);
> void resume_netfront(void);
> diff --git a/lib/sys.c b/lib/sys.c
> index 90684335..44fe848c 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -309,17 +309,6 @@ int read(int fd, void *buf, size_t nbytes)
> case FTYPE_SOCKET:
> return lwip_read(files[fd].fd, buf, nbytes);
> #endif
> -#ifdef CONFIG_NETFRONT
> - case FTYPE_TAP: {
> - ssize_t ret;
> - ret = netfront_receive(files[fd].dev, buf, nbytes);
> - if (ret <= 0) {
> - errno = EAGAIN;
> - return -1;
> - }
> - return ret;
> - }
> -#endif
> #ifdef CONFIG_KBDFRONT
> case FTYPE_KBD: {
> int ret, n;
> @@ -382,11 +371,6 @@ int write(int fd, const void *buf, size_t nbytes)
> #ifdef HAVE_LWIP
> case FTYPE_SOCKET:
> return lwip_write(files[fd].fd, (void*) buf, nbytes);
> -#endif
> -#ifdef CONFIG_NETFRONT
> - case FTYPE_TAP:
> - netfront_xmit(files[fd].dev, (void*) buf, nbytes);
> - return nbytes;
> #endif
> default:
> break;
> @@ -487,11 +471,6 @@ int close(int fd)
> res = lwip_close(files[fd].fd);
> break;
> #endif
> -#ifdef CONFIG_NETFRONT
> - case FTYPE_TAP:
> - shutdown_netfront(files[fd].dev);
> - break;
> -#endif
> #ifdef CONFIG_KBDFRONT
> case FTYPE_KBD:
> shutdown_kbdfront(files[fd].dev);
> @@ -687,7 +666,6 @@ static const char *const file_types[] = {
> [FTYPE_NONE] = "none",
> [FTYPE_CONSOLE] = "console",
> [FTYPE_SOCKET] = "socket",
> - [FTYPE_TAP] = "net",
> [FTYPE_KBD] = "kbd",
> [FTYPE_FB] = "fb",
> };
> @@ -872,7 +850,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
> n++;
> FD_CLR(i, exceptfds);
> break;
> - case FTYPE_TAP:
> case FTYPE_KBD:
> case FTYPE_FB:
> if (FD_ISSET(i, readfds)) {
> diff --git a/netfront.c b/netfront.c
> index 76964514..164fdc66 100644
> --- a/netfront.c
> +++ b/netfront.c
> @@ -248,14 +248,14 @@ void netfront_select_handler(evtchn_port_t port, struct pt_regs *regs, void *dat
> {
> int flags;
> struct netfront_dev *dev = data;
> - int fd = dev->fd;
> + struct file *file = get_file_from_fd(dev->fd);
>
> local_irq_save(flags);
> network_tx_buf_gc(dev);
> local_irq_restore(flags);
>
> - if (fd != -1)
> - files[fd].read = true;
> + if ( file )
> + file->read = true;
> wake_up(&netfront_queue);
> }
> #endif
> @@ -565,8 +565,53 @@ error:
> }
>
> #ifdef HAVE_LIBC
> +static int netfront_read(struct file *file, void *buf, size_t nbytes)
> +{
> + ssize_t ret;
> +
> + ret = netfront_receive(file->dev, buf, nbytes);
> + if ( ret <= 0 )
> + {
> + errno = EAGAIN;
> + return -1;
> + }
> +
> + return ret;
> +}
> +
> +static int netfront_write(struct file *file, const void *buf, size_t nbytes)
> +{
> + netfront_xmit(file->dev, buf, nbytes);
> +
> + return nbytes;
> +}
> +
> +static int netfront_close_fd(struct file *file)
> +{
> + shutdown_netfront(file->dev);
> +
> + return 0;
> +}
> +
> +static const struct file_ops netfront_ops = {
> + .name = "net",
> + .read = netfront_read,
> + .write = netfront_write,
> + .close = netfront_close_fd,
> + .select_rd = select_read_flag,
> +};
> +
> +static unsigned int ftype_netfront;
> +
> +__attribute__((constructor))
> +static void netfront_initialize(void)
> +{
> + ftype_netfront = alloc_file_type(&netfront_ops);
> +}
> +
> int netfront_tap_open(char *nodename) {
> struct netfront_dev *dev;
> + struct file *file;
>
> dev = init_netfront(nodename, NETIF_SELECT_RX, NULL, NULL);
> if (!dev) {
> @@ -574,9 +619,10 @@ int netfront_tap_open(char *nodename) {
> errno = EIO;
> return -1;
> }
> - dev->fd = alloc_fd(FTYPE_TAP);
> + dev->fd = alloc_fd(ftype_netfront);
> printk("tap_open(%s) -> %d\n", nodename, dev->fd);
> - files[dev->fd].dev = dev;
> + file = get_file_from_fd(dev->fd);
> + file->dev = dev;
> return dev->fd;
> }
> #endif
> @@ -720,7 +766,7 @@ void init_rx_buffers(struct netfront_dev *dev)
> }
>
>
> -void netfront_xmit(struct netfront_dev *dev, unsigned char* data,int len)
> +void netfront_xmit(struct netfront_dev *dev, const unsigned char *data, int len)
> {
> int flags;
> struct netif_tx_request *tx;
> @@ -772,7 +818,8 @@ void netfront_xmit(struct netfront_dev *dev, unsigned char* data,int len)
> ssize_t netfront_receive(struct netfront_dev *dev, unsigned char *data, size_t len)
> {
> unsigned long flags;
> - int fd = dev->fd;
> + struct file *file = get_file_from_fd(dev->fd);
> +
> ASSERT(current == main_thread);
>
> dev->rlen = 0;
> @@ -781,9 +828,9 @@ ssize_t netfront_receive(struct netfront_dev *dev, unsigned char *data, size_t l
>
> local_irq_save(flags);
> network_rx(dev);
> - if (!dev->rlen && fd != -1)
> + if ( !dev->rlen && file )
> /* No data for us, make select stop returning */
> - files[fd].read = false;
> + file->read = false;
> /* Before re-enabling the interrupts, in case a packet just arrived in the
> * meanwhile. */
> local_irq_restore(flags);
> --
> 2.26.2
>
--
Samuel
<O> Ça peut être une madeleine à sous munitions (avec des composants,
par exemple)
-+- #runtime -+-
^ permalink raw reply [flat|nested] 29+ messages in thread
* [MINIOS PATCH v3 08/12] use alloc_file_type() and get_file_from_fd() in fbfront
2022-01-16 8:33 [MINIOS PATCH v3 00/12] remove device specific struct file members Juergen Gross
` (6 preceding siblings ...)
2022-01-16 8:33 ` [MINIOS PATCH v3 07/12] use alloc_file_type() and get_file_from_fd() in netfront Juergen Gross
@ 2022-01-16 8:33 ` Juergen Gross
2022-01-16 21:00 ` Samuel Thibault
2022-01-16 8:33 ` [MINIOS PATCH v3 09/12] use file_ops and get_file_from_fd() for console Juergen Gross
` (4 subsequent siblings)
12 siblings, 1 reply; 29+ messages in thread
From: Juergen Gross @ 2022-01-16 8:33 UTC (permalink / raw)
To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross
Allocate file types dynamically via alloc_file_type().
Instead of directly accessing the files[] array use get_file_from_fd().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- switch to struct file * parameter for callbacks
- use __attribute__((constructor))
---
fbfront.c | 127 ++++++++++++++++++++++++++++++++++++++++++--------
include/lib.h | 4 +-
lib/sys.c | 47 -------------------
3 files changed, 108 insertions(+), 70 deletions(-)
diff --git a/fbfront.c b/fbfront.c
index 1e055fb0..360a9cf6 100644
--- a/fbfront.c
+++ b/fbfront.c
@@ -14,6 +14,9 @@
#include <mini-os/xmalloc.h>
#include <mini-os/fbfront.h>
#include <mini-os/lib.h>
+#ifdef HAVE_LIBC
+#include <errno.h>
+#endif
DECLARE_WAIT_QUEUE_HEAD(kbdfront_queue);
@@ -42,10 +45,10 @@ void kbdfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
{
#ifdef HAVE_LIBC
struct kbdfront_dev *dev = data;
- int fd = dev->fd;
+ struct file *file = get_file_from_fd(dev->fd);
- if (fd != -1)
- files[fd].read = true;
+ if ( file )
+ file->read = true;
#endif
wake_up(&kbdfront_queue);
}
@@ -204,10 +207,12 @@ int kbdfront_receive(struct kbdfront_dev *dev, union xenkbd_in_event *buf, int n
struct xenkbd_page *page = dev->page;
uint32_t prod, cons;
int i;
-
#ifdef HAVE_LIBC
- if (dev->fd != -1) {
- files[dev->fd].read = false;
+ struct file *file = get_file_from_fd(dev->fd);
+
+ if ( file )
+ {
+ file->read = false;
mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */
}
#endif
@@ -227,9 +232,9 @@ int kbdfront_receive(struct kbdfront_dev *dev, union xenkbd_in_event *buf, int n
notify_remote_via_evtchn(dev->evtchn);
#ifdef HAVE_LIBC
- if (cons != prod && dev->fd != -1)
+ if ( cons != prod && file )
/* still some events to read */
- files[dev->fd].read = true;
+ file->read = true;
#endif
return i;
@@ -298,11 +303,51 @@ close_kbdfront:
}
#ifdef HAVE_LIBC
+static int kbd_read(struct file *file, void *buf, size_t nbytes)
+{
+ int ret, n;
+
+ n = nbytes / sizeof(union xenkbd_in_event);
+ ret = kbdfront_receive(file->dev, buf, n);
+ if ( ret <= 0 )
+ {
+ errno = EAGAIN;
+ return -1;
+ }
+
+ return ret * sizeof(union xenkbd_in_event);
+}
+
+static int kbd_close_fd(struct file *file)
+{
+ shutdown_kbdfront(file->dev);
+
+ return 0;
+}
+
+static struct file_ops kbd_ops = {
+ .name = "kbd",
+ .read = kbd_read,
+ .close = kbd_close_fd,
+ .select_rd = select_read_flag,
+};
+
+static unsigned int ftype_kbd;
+
+__attribute__((constructor))
+static void kbdfront_initialize(void)
+{
+ ftype_kbd = alloc_file_type(&kbd_ops);
+}
+
int kbdfront_open(struct kbdfront_dev *dev)
{
- dev->fd = alloc_fd(FTYPE_KBD);
+ struct file *file;
+
+ dev->fd = alloc_fd(ftype_kbd);
printk("kbd_open(%s) -> %d\n", dev->nodename, dev->fd);
- files[dev->fd].dev = dev;
+ file = get_file_from_fd(dev->fd);
+ file->dev = dev;
return dev->fd;
}
#endif
@@ -346,10 +391,10 @@ void fbfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
{
#ifdef HAVE_LIBC
struct fbfront_dev *dev = data;
- int fd = dev->fd;
+ struct file *file = get_file_from_fd(dev->fd);
- if (fd != -1)
- files[fd].read = true;
+ if ( file )
+ file->read = true;
#endif
wake_up(&fbfront_queue);
}
@@ -373,10 +418,12 @@ int fbfront_receive(struct fbfront_dev *dev, union xenfb_in_event *buf, int n)
struct xenfb_page *page = dev->page;
uint32_t prod, cons;
int i;
-
#ifdef HAVE_LIBC
- if (dev->fd != -1) {
- files[dev->fd].read = false;
+ struct file *file = get_file_from_fd(dev->fd);
+
+ if ( file )
+ {
+ file->read = false;
mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */
}
#endif
@@ -396,9 +443,9 @@ int fbfront_receive(struct fbfront_dev *dev, union xenfb_in_event *buf, int n)
notify_remote_via_evtchn(dev->evtchn);
#ifdef HAVE_LIBC
- if (cons != prod && dev->fd != -1)
+ if ( cons != prod && file )
/* still some events to read */
- files[dev->fd].read = true;
+ file->read = true;
#endif
return i;
@@ -699,11 +746,51 @@ close_fbfront:
}
#ifdef HAVE_LIBC
+static int fbfront_read(struct file *file, void *buf, size_t nbytes)
+{
+ int ret, n;
+
+ n = nbytes / sizeof(union xenfb_in_event);
+ ret = fbfront_receive(file->dev, buf, n);
+ if ( ret <= 0 )
+ {
+ errno = EAGAIN;
+ return -1;
+ }
+
+ return ret * sizeof(union xenfb_in_event);
+}
+
+static int fbfront_close_fd(struct file *file)
+{
+ shutdown_fbfront(file->dev);
+
+ return 0;
+}
+
+static const struct file_ops fb_ops = {
+ .name = "fb",
+ .read = fbfront_read,
+ .close = fbfront_close_fd,
+ .select_rd = select_read_flag,
+};
+
+static unsigned int ftype_fb;
+
+__attribute__((constructor))
+static void fbfront_initialize(void)
+{
+ ftype_fb = alloc_file_type(&fb_ops);
+}
+
int fbfront_open(struct fbfront_dev *dev)
{
- dev->fd = alloc_fd(FTYPE_FB);
+ struct file *file;
+
+ dev->fd = alloc_fd(ftype_fb);
printk("fb_open(%s) -> %d\n", dev->nodename, dev->fd);
- files[dev->fd].dev = dev;
+ file = get_file_from_fd(dev->fd);
+ file->dev = dev;
return dev->fd;
}
#endif
diff --git a/include/lib.h b/include/lib.h
index b0d08807..b1beeed3 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -163,9 +163,7 @@ extern struct wait_queue_head event_queue;
#define FTYPE_SOCKET 3
#define FTYPE_MEM 4
#define FTYPE_SAVEFILE 5
-#define FTYPE_FB 6
-#define FTYPE_KBD 7
-#define FTYPE_N 8
+#define FTYPE_N 6
#define FTYPE_SPARE 16
struct file {
diff --git a/lib/sys.c b/lib/sys.c
index 44fe848c..db26fd2d 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -308,30 +308,6 @@ int read(int fd, void *buf, size_t nbytes)
#ifdef HAVE_LWIP
case FTYPE_SOCKET:
return lwip_read(files[fd].fd, buf, nbytes);
-#endif
-#ifdef CONFIG_KBDFRONT
- case FTYPE_KBD: {
- int ret, n;
- n = nbytes / sizeof(union xenkbd_in_event);
- ret = kbdfront_receive(files[fd].dev, buf, n);
- if (ret <= 0) {
- errno = EAGAIN;
- return -1;
- }
- return ret * sizeof(union xenkbd_in_event);
- }
-#endif
-#ifdef CONFIG_FBFRONT
- case FTYPE_FB: {
- int ret, n;
- n = nbytes / sizeof(union xenfb_in_event);
- ret = fbfront_receive(files[fd].dev, buf, n);
- if (ret <= 0) {
- errno = EAGAIN;
- return -1;
- }
- return ret * sizeof(union xenfb_in_event);
- }
#endif
default:
break;
@@ -471,16 +447,6 @@ int close(int fd)
res = lwip_close(files[fd].fd);
break;
#endif
-#ifdef CONFIG_KBDFRONT
- case FTYPE_KBD:
- shutdown_kbdfront(files[fd].dev);
- break;
-#endif
-#ifdef CONFIG_FBFRONT
- case FTYPE_FB:
- shutdown_fbfront(files[fd].dev);
- break;
-#endif
#ifdef CONFIG_CONSFRONT
case FTYPE_SAVEFILE:
case FTYPE_CONSOLE:
@@ -666,8 +632,6 @@ static const char *const file_types[] = {
[FTYPE_NONE] = "none",
[FTYPE_CONSOLE] = "console",
[FTYPE_SOCKET] = "socket",
- [FTYPE_KBD] = "kbd",
- [FTYPE_FB] = "fb",
};
static const char *get_type_name(unsigned int type)
@@ -850,17 +814,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
n++;
FD_CLR(i, exceptfds);
break;
- case FTYPE_KBD:
- case FTYPE_FB:
- if (FD_ISSET(i, readfds)) {
- if (files[i].read)
- n++;
- else
- FD_CLR(i, readfds);
- }
- FD_CLR(i, writefds);
- FD_CLR(i, exceptfds);
- break;
#ifdef HAVE_LWIP
case FTYPE_SOCKET:
if (FD_ISSET(i, readfds)) {
--
2.26.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 08/12] use alloc_file_type() and get_file_from_fd() in fbfront
2022-01-16 8:33 ` [MINIOS PATCH v3 08/12] use alloc_file_type() and get_file_from_fd() in fbfront Juergen Gross
@ 2022-01-16 21:00 ` Samuel Thibault
0 siblings, 0 replies; 29+ messages in thread
From: Samuel Thibault @ 2022-01-16 21:00 UTC (permalink / raw)
To: Juergen Gross; +Cc: minios-devel, xen-devel, wl
Juergen Gross, le dim. 16 janv. 2022 09:33:24 +0100, a ecrit:
> Allocate file types dynamically via alloc_file_type().
>
> Instead of directly accessing the files[] array use get_file_from_fd().
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> V3:
> - switch to struct file * parameter for callbacks
> - use __attribute__((constructor))
> ---
> fbfront.c | 127 ++++++++++++++++++++++++++++++++++++++++++--------
> include/lib.h | 4 +-
> lib/sys.c | 47 -------------------
> 3 files changed, 108 insertions(+), 70 deletions(-)
>
> diff --git a/fbfront.c b/fbfront.c
> index 1e055fb0..360a9cf6 100644
> --- a/fbfront.c
> +++ b/fbfront.c
> @@ -14,6 +14,9 @@
> #include <mini-os/xmalloc.h>
> #include <mini-os/fbfront.h>
> #include <mini-os/lib.h>
> +#ifdef HAVE_LIBC
> +#include <errno.h>
> +#endif
>
> DECLARE_WAIT_QUEUE_HEAD(kbdfront_queue);
>
> @@ -42,10 +45,10 @@ void kbdfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
> {
> #ifdef HAVE_LIBC
> struct kbdfront_dev *dev = data;
> - int fd = dev->fd;
> + struct file *file = get_file_from_fd(dev->fd);
>
> - if (fd != -1)
> - files[fd].read = true;
> + if ( file )
> + file->read = true;
> #endif
> wake_up(&kbdfront_queue);
> }
> @@ -204,10 +207,12 @@ int kbdfront_receive(struct kbdfront_dev *dev, union xenkbd_in_event *buf, int n
> struct xenkbd_page *page = dev->page;
> uint32_t prod, cons;
> int i;
> -
> #ifdef HAVE_LIBC
> - if (dev->fd != -1) {
> - files[dev->fd].read = false;
> + struct file *file = get_file_from_fd(dev->fd);
> +
> + if ( file )
> + {
> + file->read = false;
> mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */
> }
> #endif
> @@ -227,9 +232,9 @@ int kbdfront_receive(struct kbdfront_dev *dev, union xenkbd_in_event *buf, int n
> notify_remote_via_evtchn(dev->evtchn);
>
> #ifdef HAVE_LIBC
> - if (cons != prod && dev->fd != -1)
> + if ( cons != prod && file )
> /* still some events to read */
> - files[dev->fd].read = true;
> + file->read = true;
> #endif
>
> return i;
> @@ -298,11 +303,51 @@ close_kbdfront:
> }
>
> #ifdef HAVE_LIBC
> +static int kbd_read(struct file *file, void *buf, size_t nbytes)
> +{
> + int ret, n;
> +
> + n = nbytes / sizeof(union xenkbd_in_event);
> + ret = kbdfront_receive(file->dev, buf, n);
> + if ( ret <= 0 )
> + {
> + errno = EAGAIN;
> + return -1;
> + }
> +
> + return ret * sizeof(union xenkbd_in_event);
> +}
> +
> +static int kbd_close_fd(struct file *file)
> +{
> + shutdown_kbdfront(file->dev);
> +
> + return 0;
> +}
> +
> +static struct file_ops kbd_ops = {
> + .name = "kbd",
> + .read = kbd_read,
> + .close = kbd_close_fd,
> + .select_rd = select_read_flag,
> +};
> +
> +static unsigned int ftype_kbd;
> +
> +__attribute__((constructor))
> +static void kbdfront_initialize(void)
> +{
> + ftype_kbd = alloc_file_type(&kbd_ops);
> +}
> +
> int kbdfront_open(struct kbdfront_dev *dev)
> {
> - dev->fd = alloc_fd(FTYPE_KBD);
> + struct file *file;
> +
> + dev->fd = alloc_fd(ftype_kbd);
> printk("kbd_open(%s) -> %d\n", dev->nodename, dev->fd);
> - files[dev->fd].dev = dev;
> + file = get_file_from_fd(dev->fd);
> + file->dev = dev;
> return dev->fd;
> }
> #endif
> @@ -346,10 +391,10 @@ void fbfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
> {
> #ifdef HAVE_LIBC
> struct fbfront_dev *dev = data;
> - int fd = dev->fd;
> + struct file *file = get_file_from_fd(dev->fd);
>
> - if (fd != -1)
> - files[fd].read = true;
> + if ( file )
> + file->read = true;
> #endif
> wake_up(&fbfront_queue);
> }
> @@ -373,10 +418,12 @@ int fbfront_receive(struct fbfront_dev *dev, union xenfb_in_event *buf, int n)
> struct xenfb_page *page = dev->page;
> uint32_t prod, cons;
> int i;
> -
> #ifdef HAVE_LIBC
> - if (dev->fd != -1) {
> - files[dev->fd].read = false;
> + struct file *file = get_file_from_fd(dev->fd);
> +
> + if ( file )
> + {
> + file->read = false;
> mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */
> }
> #endif
> @@ -396,9 +443,9 @@ int fbfront_receive(struct fbfront_dev *dev, union xenfb_in_event *buf, int n)
> notify_remote_via_evtchn(dev->evtchn);
>
> #ifdef HAVE_LIBC
> - if (cons != prod && dev->fd != -1)
> + if ( cons != prod && file )
> /* still some events to read */
> - files[dev->fd].read = true;
> + file->read = true;
> #endif
>
> return i;
> @@ -699,11 +746,51 @@ close_fbfront:
> }
>
> #ifdef HAVE_LIBC
> +static int fbfront_read(struct file *file, void *buf, size_t nbytes)
> +{
> + int ret, n;
> +
> + n = nbytes / sizeof(union xenfb_in_event);
> + ret = fbfront_receive(file->dev, buf, n);
> + if ( ret <= 0 )
> + {
> + errno = EAGAIN;
> + return -1;
> + }
> +
> + return ret * sizeof(union xenfb_in_event);
> +}
> +
> +static int fbfront_close_fd(struct file *file)
> +{
> + shutdown_fbfront(file->dev);
> +
> + return 0;
> +}
> +
> +static const struct file_ops fb_ops = {
> + .name = "fb",
> + .read = fbfront_read,
> + .close = fbfront_close_fd,
> + .select_rd = select_read_flag,
> +};
> +
> +static unsigned int ftype_fb;
> +
> +__attribute__((constructor))
> +static void fbfront_initialize(void)
> +{
> + ftype_fb = alloc_file_type(&fb_ops);
> +}
> +
> int fbfront_open(struct fbfront_dev *dev)
> {
> - dev->fd = alloc_fd(FTYPE_FB);
> + struct file *file;
> +
> + dev->fd = alloc_fd(ftype_fb);
> printk("fb_open(%s) -> %d\n", dev->nodename, dev->fd);
> - files[dev->fd].dev = dev;
> + file = get_file_from_fd(dev->fd);
> + file->dev = dev;
> return dev->fd;
> }
> #endif
> diff --git a/include/lib.h b/include/lib.h
> index b0d08807..b1beeed3 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -163,9 +163,7 @@ extern struct wait_queue_head event_queue;
> #define FTYPE_SOCKET 3
> #define FTYPE_MEM 4
> #define FTYPE_SAVEFILE 5
> -#define FTYPE_FB 6
> -#define FTYPE_KBD 7
> -#define FTYPE_N 8
> +#define FTYPE_N 6
> #define FTYPE_SPARE 16
>
> struct file {
> diff --git a/lib/sys.c b/lib/sys.c
> index 44fe848c..db26fd2d 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -308,30 +308,6 @@ int read(int fd, void *buf, size_t nbytes)
> #ifdef HAVE_LWIP
> case FTYPE_SOCKET:
> return lwip_read(files[fd].fd, buf, nbytes);
> -#endif
> -#ifdef CONFIG_KBDFRONT
> - case FTYPE_KBD: {
> - int ret, n;
> - n = nbytes / sizeof(union xenkbd_in_event);
> - ret = kbdfront_receive(files[fd].dev, buf, n);
> - if (ret <= 0) {
> - errno = EAGAIN;
> - return -1;
> - }
> - return ret * sizeof(union xenkbd_in_event);
> - }
> -#endif
> -#ifdef CONFIG_FBFRONT
> - case FTYPE_FB: {
> - int ret, n;
> - n = nbytes / sizeof(union xenfb_in_event);
> - ret = fbfront_receive(files[fd].dev, buf, n);
> - if (ret <= 0) {
> - errno = EAGAIN;
> - return -1;
> - }
> - return ret * sizeof(union xenfb_in_event);
> - }
> #endif
> default:
> break;
> @@ -471,16 +447,6 @@ int close(int fd)
> res = lwip_close(files[fd].fd);
> break;
> #endif
> -#ifdef CONFIG_KBDFRONT
> - case FTYPE_KBD:
> - shutdown_kbdfront(files[fd].dev);
> - break;
> -#endif
> -#ifdef CONFIG_FBFRONT
> - case FTYPE_FB:
> - shutdown_fbfront(files[fd].dev);
> - break;
> -#endif
> #ifdef CONFIG_CONSFRONT
> case FTYPE_SAVEFILE:
> case FTYPE_CONSOLE:
> @@ -666,8 +632,6 @@ static const char *const file_types[] = {
> [FTYPE_NONE] = "none",
> [FTYPE_CONSOLE] = "console",
> [FTYPE_SOCKET] = "socket",
> - [FTYPE_KBD] = "kbd",
> - [FTYPE_FB] = "fb",
> };
>
> static const char *get_type_name(unsigned int type)
> @@ -850,17 +814,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
> n++;
> FD_CLR(i, exceptfds);
> break;
> - case FTYPE_KBD:
> - case FTYPE_FB:
> - if (FD_ISSET(i, readfds)) {
> - if (files[i].read)
> - n++;
> - else
> - FD_CLR(i, readfds);
> - }
> - FD_CLR(i, writefds);
> - FD_CLR(i, exceptfds);
> - break;
> #ifdef HAVE_LWIP
> case FTYPE_SOCKET:
> if (FD_ISSET(i, readfds)) {
> --
> 2.26.2
>
--
Samuel
<B> l'alim je sais où elle est, elle est juste à côté de la dame qui dort
<g> B: clairement faut revoir les priorités dans la vie
<g> B: une dame ça se retrouve, un uptime...
^ permalink raw reply [flat|nested] 29+ messages in thread
* [MINIOS PATCH v3 09/12] use file_ops and get_file_from_fd() for console
2022-01-16 8:33 [MINIOS PATCH v3 00/12] remove device specific struct file members Juergen Gross
` (7 preceding siblings ...)
2022-01-16 8:33 ` [MINIOS PATCH v3 08/12] use alloc_file_type() and get_file_from_fd() in fbfront Juergen Gross
@ 2022-01-16 8:33 ` Juergen Gross
2022-01-16 21:01 ` Samuel Thibault
2022-01-16 8:33 ` [MINIOS PATCH v3 10/12] add struct file_ops for file type socket Juergen Gross
` (3 subsequent siblings)
12 siblings, 1 reply; 29+ messages in thread
From: Juergen Gross @ 2022-01-16 8:33 UTC (permalink / raw)
To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross
Add struct file_ops for the console related file types (FTYPE_CONSOLE
and savefile). FTYPE_CONSOLE remains statically defined, as it is used
to statically init stdin, stdout and stderr.
Instead of directly accessing the files[] array use get_file_from_fd().
With CONSOLE now handled via file_ops the bogus file descriptor case in
select_poll() now needs to be handled more explicit instead of dropping
into console handling, assuming that this case was basically meant to
cover SAVEFILE.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- switch to struct file * parameter for callbacks
- use __attribute__((constructor))
- make console_print() take a const pointer (Andrew Cooper)
---
console/console.c | 2 +-
console/xenbus.c | 112 +++++++++++++++++++++++++++++++++++++++++
console/xencons_ring.c | 6 +--
include/console.h | 7 ++-
include/lib.h | 3 +-
lib/sys.c | 87 ++++++++------------------------
6 files changed, 144 insertions(+), 73 deletions(-)
diff --git a/console/console.c b/console/console.c
index af1a6dd3..68c8435e 100644
--- a/console/console.c
+++ b/console/console.c
@@ -76,7 +76,7 @@ void xencons_tx(void)
#endif
-void console_print(struct consfront_dev *dev, char *data, int length)
+void console_print(struct consfront_dev *dev, const char *data, int length)
{
char *curr_char, saved_char;
char copied_str[length+1];
diff --git a/console/xenbus.c b/console/xenbus.c
index 05fc31c0..d8950454 100644
--- a/console/xenbus.c
+++ b/console/xenbus.c
@@ -192,3 +192,115 @@ void fini_consfront(struct consfront_dev *dev)
{
if (dev) free_consfront(dev);
}
+
+#ifdef HAVE_LIBC
+static int consfront_read(struct file *file, void *buf, size_t nbytes)
+{
+ int ret;
+ DEFINE_WAIT(w);
+
+ while ( 1 )
+ {
+ add_waiter(w, console_queue);
+ ret = xencons_ring_recv(file->dev, buf, nbytes);
+ if ( ret )
+ break;
+ schedule();
+ }
+
+ remove_waiter(w, console_queue);
+
+ return ret;
+}
+
+static int savefile_write(struct file *file, const void *buf, size_t nbytes)
+{
+ int ret = 0, tot = nbytes;
+
+ while ( nbytes > 0 )
+ {
+ ret = xencons_ring_send(file->dev, buf, nbytes);
+ nbytes -= ret;
+ buf = (char *)buf + ret;
+ }
+
+ return tot - nbytes;
+}
+
+static int console_write(struct file *file, const void *buf, size_t nbytes)
+{
+ console_print(file->dev, buf, nbytes);
+
+ return nbytes;
+}
+
+static int consfront_close_fd(struct file *file)
+{
+ fini_consfront(file->dev);
+
+ return 0;
+}
+
+static int consfront_fstat(struct file *file, struct stat *buf)
+{
+ buf->st_mode = S_IRUSR | S_IWUSR;
+ buf->st_mode |= (file->type == FTYPE_CONSOLE) ? S_IFCHR : S_IFREG;
+ buf->st_atime = buf->st_mtime = buf->st_ctime = time(NULL);
+
+ return 0;
+}
+
+static bool consfront_select_rd(struct file *file)
+{
+ return xencons_ring_avail(file->dev);
+}
+
+static const struct file_ops savefile_ops = {
+ .name = "savefile",
+ .read = consfront_read,
+ .write = savefile_write,
+ .close = consfront_close_fd,
+ .fstat = consfront_fstat,
+ .select_rd = consfront_select_rd,
+ .select_wr = select_yes,
+};
+
+const struct file_ops console_ops = {
+ .name = "console",
+ .read = consfront_read,
+ .write = console_write,
+ .close = consfront_close_fd,
+ .fstat = consfront_fstat,
+ .select_rd = consfront_select_rd,
+ .select_wr = select_yes,
+};
+
+static unsigned int ftype_savefile;
+
+__attribute__((constructor))
+static void consfront_initialize(void)
+{
+ ftype_savefile = alloc_file_type(&savefile_ops);
+}
+
+int open_consfront(char *nodename)
+{
+ struct consfront_dev *dev;
+ struct file *file;
+
+ dev = init_consfront(nodename);
+ if ( !dev )
+ return -1;
+
+ dev->fd = alloc_fd(nodename ? ftype_savefile : FTYPE_CONSOLE);
+ file = get_file_from_fd(dev->fd);
+ if ( !file )
+ {
+ fini_consfront(dev);
+ return -1;
+ }
+ file->dev = dev;
+
+ return dev->fd;
+}
+#endif
diff --git a/console/xencons_ring.c b/console/xencons_ring.c
index c348f3ce..efedf46b 100644
--- a/console/xencons_ring.c
+++ b/console/xencons_ring.c
@@ -99,10 +99,10 @@ void console_handle_input(evtchn_port_t port, struct pt_regs *regs, void *data)
{
struct consfront_dev *dev = (struct consfront_dev *) data;
#ifdef HAVE_LIBC
- int fd = dev ? dev->fd : -1;
+ struct file *file = dev ? get_file_from_fd(dev->fd) : NULL;
- if (fd != -1)
- files[fd].read = true;
+ if ( file )
+ file->read = true;
wake_up(&console_queue);
#else
diff --git a/include/console.h b/include/console.h
index 0d7bf077..e76e4234 100644
--- a/include/console.h
+++ b/include/console.h
@@ -39,6 +39,7 @@
#include <mini-os/os.h>
#include <mini-os/traps.h>
#include <mini-os/types.h>
+#include <mini-os/lib.h>
#include <xen/grant_table.h>
#include <xenbus.h>
#include <xen/io/console.h>
@@ -77,7 +78,7 @@ void xencons_tx(void);
void get_console(void *p);
void init_console(void);
-void console_print(struct consfront_dev *dev, char *data, int length);
+void console_print(struct consfront_dev *dev, const char *data, int length);
void fini_consfront(struct consfront_dev *dev);
void suspend_console(void);
void resume_console(void);
@@ -93,5 +94,9 @@ int xencons_ring_send_no_notify(struct consfront_dev *dev, const char *data, uns
int xencons_ring_avail(struct consfront_dev *dev);
int xencons_ring_recv(struct consfront_dev *dev, char *data, unsigned len);
void free_consfront(struct consfront_dev *dev);
+#ifdef HAVE_LIBC
+extern const struct file_ops console_ops;
+int open_consfront(char *nodename);
+#endif
#endif /* _LIB_CONSOLE_H_ */
diff --git a/include/lib.h b/include/lib.h
index b1beeed3..e815e0a3 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -162,8 +162,7 @@ extern struct wait_queue_head event_queue;
#define FTYPE_FILE 2
#define FTYPE_SOCKET 3
#define FTYPE_MEM 4
-#define FTYPE_SAVEFILE 5
-#define FTYPE_N 6
+#define FTYPE_N 5
#define FTYPE_SPARE 16
struct file {
diff --git a/lib/sys.c b/lib/sys.c
index db26fd2d..74c82b2a 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -101,6 +101,9 @@ static const struct file_ops file_ops_none = {
static const struct file_ops *file_ops[FTYPE_N + FTYPE_SPARE] = {
[FTYPE_NONE] = &file_ops_none,
+#ifdef CONFIG_CONSFRONT
+ [FTYPE_CONSOLE] = &console_ops,
+#endif
};
unsigned int alloc_file_type(const struct file_ops *ops)
@@ -212,31 +215,26 @@ int mkdir(const char *pathname, mode_t mode)
#ifdef CONFIG_CONSFRONT
int posix_openpt(int flags)
{
- struct consfront_dev *dev;
+ int fd;
/* Ignore flags */
+ fd = open_consfront(NULL);
+ printk("fd(%d) = posix_openpt\n", fd);
- dev = init_consfront(NULL);
- dev->fd = alloc_fd(FTYPE_CONSOLE);
- files[dev->fd].dev = dev;
-
- printk("fd(%d) = posix_openpt\n", dev->fd);
- return(dev->fd);
+ return fd;
}
int open_savefile(const char *path, int save)
{
- struct consfront_dev *dev;
+ int fd;
char nodename[64];
snprintf(nodename, sizeof(nodename), "device/console/%d", save ? SAVE_CONSOLE : RESTORE_CONSOLE);
- dev = init_consfront(nodename);
- dev->fd = alloc_fd(FTYPE_SAVEFILE);
- files[dev->fd].dev = dev;
+ fd = open_consfront(nodename);
+ printk("fd(%d) = open_savefile\n", fd);
- printk("fd(%d) = open_savefile\n", dev->fd);
- return(dev->fd);
+ return fd;
}
#else
int posix_openpt(int flags)
@@ -291,20 +289,6 @@ int read(int fd, void *buf, size_t nbytes)
return ops->read(file, buf, nbytes);
switch (file->type) {
- case FTYPE_SAVEFILE:
- case FTYPE_CONSOLE: {
- int ret;
- DEFINE_WAIT(w);
- while(1) {
- add_waiter(w, console_queue);
- ret = xencons_ring_recv(files[fd].dev, buf, nbytes);
- if (ret)
- break;
- schedule();
- }
- remove_waiter(w, console_queue);
- return ret;
- }
#ifdef HAVE_LWIP
case FTYPE_SOCKET:
return lwip_read(files[fd].fd, buf, nbytes);
@@ -332,18 +316,6 @@ int write(int fd, const void *buf, size_t nbytes)
return ops->write(file, buf, nbytes);
switch (file->type) {
- case FTYPE_SAVEFILE: {
- int ret = 0, tot = nbytes;
- while (nbytes > 0) {
- ret = xencons_ring_send(files[fd].dev, (char *)buf, nbytes);
- nbytes -= ret;
- buf = (char *)buf + ret;
- }
- return tot - nbytes;
- }
- case FTYPE_CONSOLE:
- console_print(files[fd].dev, (char *)buf, nbytes);
- return nbytes;
#ifdef HAVE_LWIP
case FTYPE_SOCKET:
return lwip_write(files[fd].fd, (void*) buf, nbytes);
@@ -446,12 +418,6 @@ int close(int fd)
case FTYPE_SOCKET:
res = lwip_close(files[fd].fd);
break;
-#endif
-#ifdef CONFIG_CONSFRONT
- case FTYPE_SAVEFILE:
- case FTYPE_CONSOLE:
- fini_consfront(files[fd].dev);
- break;
#endif
case FTYPE_NONE:
goto error;
@@ -501,15 +467,8 @@ int fstat(int fd, struct stat *buf)
return ops->fstat(file, buf);
switch (file->type) {
- case FTYPE_SAVEFILE:
- case FTYPE_CONSOLE:
case FTYPE_SOCKET: {
- if (files[fd].type == FTYPE_CONSOLE)
- buf->st_mode = S_IFCHR|S_IRUSR|S_IWUSR;
- else if (files[fd].type == FTYPE_SOCKET)
- buf->st_mode = S_IFSOCK|S_IRUSR|S_IWUSR;
- else if (files[fd].type == FTYPE_SAVEFILE)
- buf->st_mode = S_IFREG|S_IRUSR|S_IWUSR;
+ buf->st_mode = S_IFSOCK|S_IRUSR|S_IWUSR;
buf->st_uid = 0;
buf->st_gid = 0;
buf->st_size = 0;
@@ -630,7 +589,6 @@ int closedir(DIR *dir)
#if defined(LIBC_DEBUG) || defined(LIBC_VERBOSE)
static const char *const file_types[] = {
[FTYPE_NONE] = "none",
- [FTYPE_CONSOLE] = "console",
[FTYPE_SOCKET] = "socket",
};
@@ -799,21 +757,18 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
}
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))
- n++;
- else
- FD_CLR(i, readfds);
+ if ( FD_ISSET(i, readfds) )
+ FD_CLR(i, readfds);
+ if ( FD_ISSET(i, writefds) )
+ FD_CLR(i, writefds);
+ if ( FD_ISSET(i, exceptfds) )
+ FD_CLR(i, exceptfds);
}
- if (FD_ISSET(i, writefds))
- n++;
- FD_CLR(i, exceptfds);
break;
+ }
+
#ifdef HAVE_LWIP
case FTYPE_SOCKET:
if (FD_ISSET(i, readfds)) {
--
2.26.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 09/12] use file_ops and get_file_from_fd() for console
2022-01-16 8:33 ` [MINIOS PATCH v3 09/12] use file_ops and get_file_from_fd() for console Juergen Gross
@ 2022-01-16 21:01 ` Samuel Thibault
0 siblings, 0 replies; 29+ messages in thread
From: Samuel Thibault @ 2022-01-16 21:01 UTC (permalink / raw)
To: Juergen Gross; +Cc: minios-devel, xen-devel, wl
Juergen Gross, le dim. 16 janv. 2022 09:33:25 +0100, a ecrit:
> Add struct file_ops for the console related file types (FTYPE_CONSOLE
> and savefile). FTYPE_CONSOLE remains statically defined, as it is used
> to statically init stdin, stdout and stderr.
>
> Instead of directly accessing the files[] array use get_file_from_fd().
>
> With CONSOLE now handled via file_ops the bogus file descriptor case in
> select_poll() now needs to be handled more explicit instead of dropping
> into console handling, assuming that this case was basically meant to
> cover SAVEFILE.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> V3:
> - switch to struct file * parameter for callbacks
> - use __attribute__((constructor))
> - make console_print() take a const pointer (Andrew Cooper)
> ---
> console/console.c | 2 +-
> console/xenbus.c | 112 +++++++++++++++++++++++++++++++++++++++++
> console/xencons_ring.c | 6 +--
> include/console.h | 7 ++-
> include/lib.h | 3 +-
> lib/sys.c | 87 ++++++++------------------------
> 6 files changed, 144 insertions(+), 73 deletions(-)
>
> diff --git a/console/console.c b/console/console.c
> index af1a6dd3..68c8435e 100644
> --- a/console/console.c
> +++ b/console/console.c
> @@ -76,7 +76,7 @@ void xencons_tx(void)
> #endif
>
>
> -void console_print(struct consfront_dev *dev, char *data, int length)
> +void console_print(struct consfront_dev *dev, const char *data, int length)
> {
> char *curr_char, saved_char;
> char copied_str[length+1];
> diff --git a/console/xenbus.c b/console/xenbus.c
> index 05fc31c0..d8950454 100644
> --- a/console/xenbus.c
> +++ b/console/xenbus.c
> @@ -192,3 +192,115 @@ void fini_consfront(struct consfront_dev *dev)
> {
> if (dev) free_consfront(dev);
> }
> +
> +#ifdef HAVE_LIBC
> +static int consfront_read(struct file *file, void *buf, size_t nbytes)
> +{
> + int ret;
> + DEFINE_WAIT(w);
> +
> + while ( 1 )
> + {
> + add_waiter(w, console_queue);
> + ret = xencons_ring_recv(file->dev, buf, nbytes);
> + if ( ret )
> + break;
> + schedule();
> + }
> +
> + remove_waiter(w, console_queue);
> +
> + return ret;
> +}
> +
> +static int savefile_write(struct file *file, const void *buf, size_t nbytes)
> +{
> + int ret = 0, tot = nbytes;
> +
> + while ( nbytes > 0 )
> + {
> + ret = xencons_ring_send(file->dev, buf, nbytes);
> + nbytes -= ret;
> + buf = (char *)buf + ret;
> + }
> +
> + return tot - nbytes;
> +}
> +
> +static int console_write(struct file *file, const void *buf, size_t nbytes)
> +{
> + console_print(file->dev, buf, nbytes);
> +
> + return nbytes;
> +}
> +
> +static int consfront_close_fd(struct file *file)
> +{
> + fini_consfront(file->dev);
> +
> + return 0;
> +}
> +
> +static int consfront_fstat(struct file *file, struct stat *buf)
> +{
> + buf->st_mode = S_IRUSR | S_IWUSR;
> + buf->st_mode |= (file->type == FTYPE_CONSOLE) ? S_IFCHR : S_IFREG;
> + buf->st_atime = buf->st_mtime = buf->st_ctime = time(NULL);
> +
> + return 0;
> +}
> +
> +static bool consfront_select_rd(struct file *file)
> +{
> + return xencons_ring_avail(file->dev);
> +}
> +
> +static const struct file_ops savefile_ops = {
> + .name = "savefile",
> + .read = consfront_read,
> + .write = savefile_write,
> + .close = consfront_close_fd,
> + .fstat = consfront_fstat,
> + .select_rd = consfront_select_rd,
> + .select_wr = select_yes,
> +};
> +
> +const struct file_ops console_ops = {
> + .name = "console",
> + .read = consfront_read,
> + .write = console_write,
> + .close = consfront_close_fd,
> + .fstat = consfront_fstat,
> + .select_rd = consfront_select_rd,
> + .select_wr = select_yes,
> +};
> +
> +static unsigned int ftype_savefile;
> +
> +__attribute__((constructor))
> +static void consfront_initialize(void)
> +{
> + ftype_savefile = alloc_file_type(&savefile_ops);
> +}
> +
> +int open_consfront(char *nodename)
> +{
> + struct consfront_dev *dev;
> + struct file *file;
> +
> + dev = init_consfront(nodename);
> + if ( !dev )
> + return -1;
> +
> + dev->fd = alloc_fd(nodename ? ftype_savefile : FTYPE_CONSOLE);
> + file = get_file_from_fd(dev->fd);
> + if ( !file )
> + {
> + fini_consfront(dev);
> + return -1;
> + }
> + file->dev = dev;
> +
> + return dev->fd;
> +}
> +#endif
> diff --git a/console/xencons_ring.c b/console/xencons_ring.c
> index c348f3ce..efedf46b 100644
> --- a/console/xencons_ring.c
> +++ b/console/xencons_ring.c
> @@ -99,10 +99,10 @@ void console_handle_input(evtchn_port_t port, struct pt_regs *regs, void *data)
> {
> struct consfront_dev *dev = (struct consfront_dev *) data;
> #ifdef HAVE_LIBC
> - int fd = dev ? dev->fd : -1;
> + struct file *file = dev ? get_file_from_fd(dev->fd) : NULL;
>
> - if (fd != -1)
> - files[fd].read = true;
> + if ( file )
> + file->read = true;
>
> wake_up(&console_queue);
> #else
> diff --git a/include/console.h b/include/console.h
> index 0d7bf077..e76e4234 100644
> --- a/include/console.h
> +++ b/include/console.h
> @@ -39,6 +39,7 @@
> #include <mini-os/os.h>
> #include <mini-os/traps.h>
> #include <mini-os/types.h>
> +#include <mini-os/lib.h>
> #include <xen/grant_table.h>
> #include <xenbus.h>
> #include <xen/io/console.h>
> @@ -77,7 +78,7 @@ void xencons_tx(void);
>
> void get_console(void *p);
> void init_console(void);
> -void console_print(struct consfront_dev *dev, char *data, int length);
> +void console_print(struct consfront_dev *dev, const char *data, int length);
> void fini_consfront(struct consfront_dev *dev);
> void suspend_console(void);
> void resume_console(void);
> @@ -93,5 +94,9 @@ int xencons_ring_send_no_notify(struct consfront_dev *dev, const char *data, uns
> int xencons_ring_avail(struct consfront_dev *dev);
> int xencons_ring_recv(struct consfront_dev *dev, char *data, unsigned len);
> void free_consfront(struct consfront_dev *dev);
> +#ifdef HAVE_LIBC
> +extern const struct file_ops console_ops;
> +int open_consfront(char *nodename);
> +#endif
>
> #endif /* _LIB_CONSOLE_H_ */
> diff --git a/include/lib.h b/include/lib.h
> index b1beeed3..e815e0a3 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -162,8 +162,7 @@ extern struct wait_queue_head event_queue;
> #define FTYPE_FILE 2
> #define FTYPE_SOCKET 3
> #define FTYPE_MEM 4
> -#define FTYPE_SAVEFILE 5
> -#define FTYPE_N 6
> +#define FTYPE_N 5
> #define FTYPE_SPARE 16
>
> struct file {
> diff --git a/lib/sys.c b/lib/sys.c
> index db26fd2d..74c82b2a 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -101,6 +101,9 @@ static const struct file_ops file_ops_none = {
>
> static const struct file_ops *file_ops[FTYPE_N + FTYPE_SPARE] = {
> [FTYPE_NONE] = &file_ops_none,
> +#ifdef CONFIG_CONSFRONT
> + [FTYPE_CONSOLE] = &console_ops,
> +#endif
> };
>
> unsigned int alloc_file_type(const struct file_ops *ops)
> @@ -212,31 +215,26 @@ int mkdir(const char *pathname, mode_t mode)
> #ifdef CONFIG_CONSFRONT
> int posix_openpt(int flags)
> {
> - struct consfront_dev *dev;
> + int fd;
>
> /* Ignore flags */
> + fd = open_consfront(NULL);
> + printk("fd(%d) = posix_openpt\n", fd);
>
> - dev = init_consfront(NULL);
> - dev->fd = alloc_fd(FTYPE_CONSOLE);
> - files[dev->fd].dev = dev;
> -
> - printk("fd(%d) = posix_openpt\n", dev->fd);
> - return(dev->fd);
> + return fd;
> }
>
> int open_savefile(const char *path, int save)
> {
> - struct consfront_dev *dev;
> + int fd;
> char nodename[64];
>
> snprintf(nodename, sizeof(nodename), "device/console/%d", save ? SAVE_CONSOLE : RESTORE_CONSOLE);
>
> - dev = init_consfront(nodename);
> - dev->fd = alloc_fd(FTYPE_SAVEFILE);
> - files[dev->fd].dev = dev;
> + fd = open_consfront(nodename);
> + printk("fd(%d) = open_savefile\n", fd);
>
> - printk("fd(%d) = open_savefile\n", dev->fd);
> - return(dev->fd);
> + return fd;
> }
> #else
> int posix_openpt(int flags)
> @@ -291,20 +289,6 @@ int read(int fd, void *buf, size_t nbytes)
> return ops->read(file, buf, nbytes);
>
> switch (file->type) {
> - case FTYPE_SAVEFILE:
> - case FTYPE_CONSOLE: {
> - int ret;
> - DEFINE_WAIT(w);
> - while(1) {
> - add_waiter(w, console_queue);
> - ret = xencons_ring_recv(files[fd].dev, buf, nbytes);
> - if (ret)
> - break;
> - schedule();
> - }
> - remove_waiter(w, console_queue);
> - return ret;
> - }
> #ifdef HAVE_LWIP
> case FTYPE_SOCKET:
> return lwip_read(files[fd].fd, buf, nbytes);
> @@ -332,18 +316,6 @@ int write(int fd, const void *buf, size_t nbytes)
> return ops->write(file, buf, nbytes);
>
> switch (file->type) {
> - case FTYPE_SAVEFILE: {
> - int ret = 0, tot = nbytes;
> - while (nbytes > 0) {
> - ret = xencons_ring_send(files[fd].dev, (char *)buf, nbytes);
> - nbytes -= ret;
> - buf = (char *)buf + ret;
> - }
> - return tot - nbytes;
> - }
> - case FTYPE_CONSOLE:
> - console_print(files[fd].dev, (char *)buf, nbytes);
> - return nbytes;
> #ifdef HAVE_LWIP
> case FTYPE_SOCKET:
> return lwip_write(files[fd].fd, (void*) buf, nbytes);
> @@ -446,12 +418,6 @@ int close(int fd)
> case FTYPE_SOCKET:
> res = lwip_close(files[fd].fd);
> break;
> -#endif
> -#ifdef CONFIG_CONSFRONT
> - case FTYPE_SAVEFILE:
> - case FTYPE_CONSOLE:
> - fini_consfront(files[fd].dev);
> - break;
> #endif
> case FTYPE_NONE:
> goto error;
> @@ -501,15 +467,8 @@ int fstat(int fd, struct stat *buf)
> return ops->fstat(file, buf);
>
> switch (file->type) {
> - case FTYPE_SAVEFILE:
> - case FTYPE_CONSOLE:
> case FTYPE_SOCKET: {
> - if (files[fd].type == FTYPE_CONSOLE)
> - buf->st_mode = S_IFCHR|S_IRUSR|S_IWUSR;
> - else if (files[fd].type == FTYPE_SOCKET)
> - buf->st_mode = S_IFSOCK|S_IRUSR|S_IWUSR;
> - else if (files[fd].type == FTYPE_SAVEFILE)
> - buf->st_mode = S_IFREG|S_IRUSR|S_IWUSR;
> + buf->st_mode = S_IFSOCK|S_IRUSR|S_IWUSR;
> buf->st_uid = 0;
> buf->st_gid = 0;
> buf->st_size = 0;
> @@ -630,7 +589,6 @@ int closedir(DIR *dir)
> #if defined(LIBC_DEBUG) || defined(LIBC_VERBOSE)
> static const char *const file_types[] = {
> [FTYPE_NONE] = "none",
> - [FTYPE_CONSOLE] = "console",
> [FTYPE_SOCKET] = "socket",
> };
>
> @@ -799,21 +757,18 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
> }
>
> 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))
> - n++;
> - else
> - FD_CLR(i, readfds);
> + if ( FD_ISSET(i, readfds) )
> + FD_CLR(i, readfds);
> + if ( FD_ISSET(i, writefds) )
> + FD_CLR(i, writefds);
> + if ( FD_ISSET(i, exceptfds) )
> + FD_CLR(i, exceptfds);
> }
> - if (FD_ISSET(i, writefds))
> - n++;
> - FD_CLR(i, exceptfds);
> break;
> + }
> +
> #ifdef HAVE_LWIP
> case FTYPE_SOCKET:
> if (FD_ISSET(i, readfds)) {
> --
> 2.26.2
>
--
Samuel
<N> (* If you have a precise idea of the intended use of the following code, please
<N> write to Eduardo.Gimenez@inria.fr and ask for the prize :-)
<N> -- Eduardo (11/8/97) *)
-+- N sur #ens-mim - et c'était un des développeurs -+-
^ permalink raw reply [flat|nested] 29+ messages in thread
* [MINIOS PATCH v3 10/12] add struct file_ops for file type socket
2022-01-16 8:33 [MINIOS PATCH v3 00/12] remove device specific struct file members Juergen Gross
` (8 preceding siblings ...)
2022-01-16 8:33 ` [MINIOS PATCH v3 09/12] use file_ops and get_file_from_fd() for console Juergen Gross
@ 2022-01-16 8:33 ` Juergen Gross
2022-01-16 21:02 ` Samuel Thibault
2022-01-16 8:33 ` [MINIOS PATCH v3 11/12] add struct file_ops for FTYPE_FILE Juergen Gross
` (2 subsequent siblings)
12 siblings, 1 reply; 29+ messages in thread
From: Juergen Gross @ 2022-01-16 8:33 UTC (permalink / raw)
To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross
Even with some special handling needed in select_poll(), add a struct
file_ops for FTYPE_SOCKET. Due to the need of the special handling it
isn't possible to use a dynamically allocated file type.
Most functions calling the file_ops methods can be simplified a lot now
that no file type specific handling is left. Same applies to the file
type name printing in debug/verbose mode.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- switch to struct file * parameter for callbacks
---
lib/sys.c | 148 +++++++++++++++++++++++-------------------------------
1 file changed, 64 insertions(+), 84 deletions(-)
diff --git a/lib/sys.c b/lib/sys.c
index 74c82b2a..538d6e7b 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -99,11 +99,67 @@ static const struct file_ops file_ops_none = {
.name = "none",
};
+#ifdef HAVE_LWIP
+static int socket_read(struct file *file, void *buf, size_t nbytes)
+{
+ return lwip_read(file->fd, buf, nbytes);
+}
+
+static int socket_write(struct file *file, const void *buf, size_t nbytes)
+{
+ return lwip_write(file->fd, buf, nbytes);
+}
+
+static int close_socket_fd(struct file *file)
+{
+ return lwip_close(file->fd);
+}
+
+static int socket_fstat(struct file *file, struct stat *buf)
+{
+ buf->st_mode = S_IFSOCK | S_IRUSR | S_IWUSR;
+ buf->st_atime = buf->st_mtime = buf->st_ctime = time(NULL);
+
+ return 0;
+}
+
+static int socket_fcntl(struct file *file, int cmd, va_list args)
+{
+ long arg;
+
+ arg = va_arg(args, long);
+
+ if ( cmd == F_SETFL && !(arg & ~O_NONBLOCK) )
+ {
+ /* Only flag supported: non-blocking mode */
+ uint32_t nblock = !!(arg & O_NONBLOCK);
+
+ return lwip_ioctl(file->fd, FIONBIO, &nblock);
+ }
+
+ printk("socket fcntl(fd, %d, %lx/%lo)\n", cmd, arg, arg);
+ errno = ENOSYS;
+ return -1;
+}
+
+static const struct file_ops socket_ops = {
+ .name = "socket",
+ .read = socket_read,
+ .write = socket_write,
+ .close = close_socket_fd,
+ .fstat = socket_fstat,
+ .fcntl = socket_fcntl,
+};
+#endif
+
static const struct file_ops *file_ops[FTYPE_N + FTYPE_SPARE] = {
[FTYPE_NONE] = &file_ops_none,
#ifdef CONFIG_CONSFRONT
[FTYPE_CONSOLE] = &console_ops,
#endif
+#ifdef HAVE_LWIP
+ [FTYPE_SOCKET] = &socket_ops,
+#endif
};
unsigned int alloc_file_type(const struct file_ops *ops)
@@ -288,15 +344,6 @@ int read(int fd, void *buf, size_t nbytes)
if ( ops->read )
return ops->read(file, buf, nbytes);
- switch (file->type) {
-#ifdef HAVE_LWIP
- case FTYPE_SOCKET:
- return lwip_read(files[fd].fd, buf, nbytes);
-#endif
- default:
- break;
- }
-
error:
printk("read(%d): Bad descriptor\n", fd);
errno = EBADF;
@@ -315,15 +362,6 @@ int write(int fd, const void *buf, size_t nbytes)
if ( ops->write )
return ops->write(file, buf, nbytes);
- switch (file->type) {
-#ifdef HAVE_LWIP
- case FTYPE_SOCKET:
- return lwip_write(files[fd].fd, (void*) buf, nbytes);
-#endif
- default:
- break;
- }
-
error:
printk("write(%d): Bad descriptor\n", fd);
errno = EBADF;
@@ -406,24 +444,10 @@ int close(int fd)
ops = get_file_ops(file->type);
printk("close(%d)\n", fd);
if ( ops->close )
- {
res = ops->close(file);
- goto out;
- }
-
- switch (file->type) {
- default:
- break;
-#ifdef HAVE_LWIP
- case FTYPE_SOCKET:
- res = lwip_close(files[fd].fd);
- break;
-#endif
- case FTYPE_NONE:
- goto error;
- }
+ else if ( file->type == FTYPE_NONE )
+ goto error;
- out:
memset(files + fd, 0, sizeof(struct file));
BUILD_BUG_ON(FTYPE_NONE != 0);
@@ -466,21 +490,6 @@ int fstat(int fd, struct stat *buf)
if ( ops->fstat )
return ops->fstat(file, buf);
- switch (file->type) {
- case FTYPE_SOCKET: {
- buf->st_mode = S_IFSOCK|S_IRUSR|S_IWUSR;
- buf->st_uid = 0;
- buf->st_gid = 0;
- buf->st_size = 0;
- buf->st_atime =
- buf->st_mtime =
- buf->st_ctime = time(NULL);
- return 0;
- }
- default:
- break;
- }
-
error:
printk("statf(%d): Bad descriptor\n", fd);
errno = EBADF;
@@ -538,21 +547,9 @@ int fcntl(int fd, int cmd, ...)
arg = va_arg(ap, long);
va_end(ap);
- switch (cmd) {
-#ifdef HAVE_LWIP
- case F_SETFL:
- if (files[fd].type == FTYPE_SOCKET && !(arg & ~O_NONBLOCK)) {
- /* Only flag supported: non-blocking mode */
- uint32_t nblock = !!(arg & O_NONBLOCK);
- return lwip_ioctl(files[fd].fd, FIONBIO, &nblock);
- }
- /* Fallthrough */
-#endif
- default:
- printk("fcntl(%d, %d, %lx/%lo)\n", fd, cmd, arg, arg);
- errno = ENOSYS;
- return -1;
- }
+ printk("fcntl(%d, %d, %lx/%lo)\n", fd, cmd, arg, arg);
+ errno = ENOSYS;
+ return -1;
}
DIR *opendir(const char *name)
@@ -586,23 +583,6 @@ int closedir(DIR *dir)
/* We assume that only the main thread calls select(). */
-#if defined(LIBC_DEBUG) || defined(LIBC_VERBOSE)
-static const char *const file_types[] = {
- [FTYPE_NONE] = "none",
- [FTYPE_SOCKET] = "socket",
-};
-
-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)
{
@@ -613,7 +593,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(%s)", i, get_type_name(files[i].type)); \
+ printk("%d(%s)", i, get_file_ops(files[i].type)->name); \
comma = 1; \
} \
} \
@@ -647,7 +627,7 @@ static void dump_pollfds(struct pollfd *pfd, int nfds, int timeout)
fd = pfd[i].fd;
if (comma)
printk(", ");
- printk("%d(%s)/%02x", fd, get_type_name(files[fd].type),
+ printk("%d(%s)/%02x", fd, get_file_ops(files[fd].type)->name,
pfd[i].events);
comma = 1;
}
@@ -809,7 +789,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, get_type_name(files[i].type));
+ printk(" %d(%c):", i, get_file_ops(files[i].type)->name);
if (nbread[i])
printk(" %dR", nbread[i]);
if (nbwrite[i])
--
2.26.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 10/12] add struct file_ops for file type socket
2022-01-16 8:33 ` [MINIOS PATCH v3 10/12] add struct file_ops for file type socket Juergen Gross
@ 2022-01-16 21:02 ` Samuel Thibault
0 siblings, 0 replies; 29+ messages in thread
From: Samuel Thibault @ 2022-01-16 21:02 UTC (permalink / raw)
To: Juergen Gross; +Cc: minios-devel, xen-devel, wl
Juergen Gross, le dim. 16 janv. 2022 09:33:26 +0100, a ecrit:
> Even with some special handling needed in select_poll(), add a struct
> file_ops for FTYPE_SOCKET. Due to the need of the special handling it
> isn't possible to use a dynamically allocated file type.
>
> Most functions calling the file_ops methods can be simplified a lot now
> that no file type specific handling is left. Same applies to the file
> type name printing in debug/verbose mode.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> V3:
> - switch to struct file * parameter for callbacks
> ---
> lib/sys.c | 148 +++++++++++++++++++++++-------------------------------
> 1 file changed, 64 insertions(+), 84 deletions(-)
>
> diff --git a/lib/sys.c b/lib/sys.c
> index 74c82b2a..538d6e7b 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -99,11 +99,67 @@ static const struct file_ops file_ops_none = {
> .name = "none",
> };
>
> +#ifdef HAVE_LWIP
> +static int socket_read(struct file *file, void *buf, size_t nbytes)
> +{
> + return lwip_read(file->fd, buf, nbytes);
> +}
> +
> +static int socket_write(struct file *file, const void *buf, size_t nbytes)
> +{
> + return lwip_write(file->fd, buf, nbytes);
> +}
> +
> +static int close_socket_fd(struct file *file)
> +{
> + return lwip_close(file->fd);
> +}
> +
> +static int socket_fstat(struct file *file, struct stat *buf)
> +{
> + buf->st_mode = S_IFSOCK | S_IRUSR | S_IWUSR;
> + buf->st_atime = buf->st_mtime = buf->st_ctime = time(NULL);
> +
> + return 0;
> +}
> +
> +static int socket_fcntl(struct file *file, int cmd, va_list args)
> +{
> + long arg;
> +
> + arg = va_arg(args, long);
> +
> + if ( cmd == F_SETFL && !(arg & ~O_NONBLOCK) )
> + {
> + /* Only flag supported: non-blocking mode */
> + uint32_t nblock = !!(arg & O_NONBLOCK);
> +
> + return lwip_ioctl(file->fd, FIONBIO, &nblock);
> + }
> +
> + printk("socket fcntl(fd, %d, %lx/%lo)\n", cmd, arg, arg);
> + errno = ENOSYS;
> + return -1;
> +}
> +
> +static const struct file_ops socket_ops = {
> + .name = "socket",
> + .read = socket_read,
> + .write = socket_write,
> + .close = close_socket_fd,
> + .fstat = socket_fstat,
> + .fcntl = socket_fcntl,
> +};
> +#endif
> +
> static const struct file_ops *file_ops[FTYPE_N + FTYPE_SPARE] = {
> [FTYPE_NONE] = &file_ops_none,
> #ifdef CONFIG_CONSFRONT
> [FTYPE_CONSOLE] = &console_ops,
> #endif
> +#ifdef HAVE_LWIP
> + [FTYPE_SOCKET] = &socket_ops,
> +#endif
> };
>
> unsigned int alloc_file_type(const struct file_ops *ops)
> @@ -288,15 +344,6 @@ int read(int fd, void *buf, size_t nbytes)
> if ( ops->read )
> return ops->read(file, buf, nbytes);
>
> - switch (file->type) {
> -#ifdef HAVE_LWIP
> - case FTYPE_SOCKET:
> - return lwip_read(files[fd].fd, buf, nbytes);
> -#endif
> - default:
> - break;
> - }
> -
> error:
> printk("read(%d): Bad descriptor\n", fd);
> errno = EBADF;
> @@ -315,15 +362,6 @@ int write(int fd, const void *buf, size_t nbytes)
> if ( ops->write )
> return ops->write(file, buf, nbytes);
>
> - switch (file->type) {
> -#ifdef HAVE_LWIP
> - case FTYPE_SOCKET:
> - return lwip_write(files[fd].fd, (void*) buf, nbytes);
> -#endif
> - default:
> - break;
> - }
> -
> error:
> printk("write(%d): Bad descriptor\n", fd);
> errno = EBADF;
> @@ -406,24 +444,10 @@ int close(int fd)
> ops = get_file_ops(file->type);
> printk("close(%d)\n", fd);
> if ( ops->close )
> - {
> res = ops->close(file);
> - goto out;
> - }
> -
> - switch (file->type) {
> - default:
> - break;
> -#ifdef HAVE_LWIP
> - case FTYPE_SOCKET:
> - res = lwip_close(files[fd].fd);
> - break;
> -#endif
> - case FTYPE_NONE:
> - goto error;
> - }
> + else if ( file->type == FTYPE_NONE )
> + goto error;
>
> - out:
> memset(files + fd, 0, sizeof(struct file));
> BUILD_BUG_ON(FTYPE_NONE != 0);
>
> @@ -466,21 +490,6 @@ int fstat(int fd, struct stat *buf)
> if ( ops->fstat )
> return ops->fstat(file, buf);
>
> - switch (file->type) {
> - case FTYPE_SOCKET: {
> - buf->st_mode = S_IFSOCK|S_IRUSR|S_IWUSR;
> - buf->st_uid = 0;
> - buf->st_gid = 0;
> - buf->st_size = 0;
> - buf->st_atime =
> - buf->st_mtime =
> - buf->st_ctime = time(NULL);
> - return 0;
> - }
> - default:
> - break;
> - }
> -
> error:
> printk("statf(%d): Bad descriptor\n", fd);
> errno = EBADF;
> @@ -538,21 +547,9 @@ int fcntl(int fd, int cmd, ...)
> arg = va_arg(ap, long);
> va_end(ap);
>
> - switch (cmd) {
> -#ifdef HAVE_LWIP
> - case F_SETFL:
> - if (files[fd].type == FTYPE_SOCKET && !(arg & ~O_NONBLOCK)) {
> - /* Only flag supported: non-blocking mode */
> - uint32_t nblock = !!(arg & O_NONBLOCK);
> - return lwip_ioctl(files[fd].fd, FIONBIO, &nblock);
> - }
> - /* Fallthrough */
> -#endif
> - default:
> - printk("fcntl(%d, %d, %lx/%lo)\n", fd, cmd, arg, arg);
> - errno = ENOSYS;
> - return -1;
> - }
> + printk("fcntl(%d, %d, %lx/%lo)\n", fd, cmd, arg, arg);
> + errno = ENOSYS;
> + return -1;
> }
>
> DIR *opendir(const char *name)
> @@ -586,23 +583,6 @@ int closedir(DIR *dir)
>
> /* We assume that only the main thread calls select(). */
>
> -#if defined(LIBC_DEBUG) || defined(LIBC_VERBOSE)
> -static const char *const file_types[] = {
> - [FTYPE_NONE] = "none",
> - [FTYPE_SOCKET] = "socket",
> -};
> -
> -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)
> {
> @@ -613,7 +593,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(%s)", i, get_type_name(files[i].type)); \
> + printk("%d(%s)", i, get_file_ops(files[i].type)->name); \
> comma = 1; \
> } \
> } \
> @@ -647,7 +627,7 @@ static void dump_pollfds(struct pollfd *pfd, int nfds, int timeout)
> fd = pfd[i].fd;
> if (comma)
> printk(", ");
> - printk("%d(%s)/%02x", fd, get_type_name(files[fd].type),
> + printk("%d(%s)/%02x", fd, get_file_ops(files[fd].type)->name,
> pfd[i].events);
> comma = 1;
> }
> @@ -809,7 +789,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, get_type_name(files[i].type));
> + printk(" %d(%c):", i, get_file_ops(files[i].type)->name);
> if (nbread[i])
> printk(" %dR", nbread[i]);
> if (nbwrite[i])
> --
> 2.26.2
>
--
Samuel
*** s has joined channel #ens-mim
<N> re
<s> pfff
<s> mare de la pfp.
<s> pas commencer et j'en ai deja marre.
<s> bon ct juste un cou de gueule ++
*** s has left channel #ens-mim (s)
-+- #ens-mim et la peufeupeu -+-
^ permalink raw reply [flat|nested] 29+ messages in thread
* [MINIOS PATCH v3 11/12] add struct file_ops for FTYPE_FILE
2022-01-16 8:33 [MINIOS PATCH v3 00/12] remove device specific struct file members Juergen Gross
` (9 preceding siblings ...)
2022-01-16 8:33 ` [MINIOS PATCH v3 10/12] add struct file_ops for file type socket Juergen Gross
@ 2022-01-16 8:33 ` Juergen Gross
2022-01-16 8:33 ` [MINIOS PATCH v3 12/12] make files array private to sys.c Juergen Gross
2022-01-16 21:03 ` [MINIOS PATCH v3 00/12] remove device specific struct file members Samuel Thibault
12 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-01-16 8:33 UTC (permalink / raw)
To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross
FTYPE_FILE is the last relevant file type without a struct file_ops.
Add it.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
lib/sys.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/lib/sys.c b/lib/sys.c
index 538d6e7b..7f3dc4e4 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -99,6 +99,11 @@ static const struct file_ops file_ops_none = {
.name = "none",
};
+static const struct file_ops file_file_ops = {
+ .name = "file",
+ .lseek = lseek_default,
+};
+
#ifdef HAVE_LWIP
static int socket_read(struct file *file, void *buf, size_t nbytes)
{
@@ -157,6 +162,7 @@ static const struct file_ops *file_ops[FTYPE_N + FTYPE_SPARE] = {
#ifdef CONFIG_CONSFRONT
[FTYPE_CONSOLE] = &console_ops,
#endif
+ [FTYPE_FILE] = &file_file_ops,
#ifdef HAVE_LWIP
[FTYPE_SOCKET] = &socket_ops,
#endif
@@ -415,16 +421,9 @@ off_t lseek(int fd, off_t offset, int whence)
if ( ops->lseek )
return ops->lseek(file, offset, whence);
- switch(file->type) {
- case FTYPE_FILE:
- break;
- default:
- /* Not implemented for this filetype */
- errno = ESPIPE;
- return (off_t) -1;
- }
-
- return lseek_default(file, offset, whence);
+ /* Not implemented for this filetype */
+ errno = ESPIPE;
+ return (off_t) -1;
}
int fsync(int fd) {
--
2.26.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* [MINIOS PATCH v3 12/12] make files array private to sys.c
2022-01-16 8:33 [MINIOS PATCH v3 00/12] remove device specific struct file members Juergen Gross
` (10 preceding siblings ...)
2022-01-16 8:33 ` [MINIOS PATCH v3 11/12] add struct file_ops for FTYPE_FILE Juergen Gross
@ 2022-01-16 8:33 ` Juergen Gross
2022-01-16 21:03 ` [MINIOS PATCH v3 00/12] remove device specific struct file members Samuel Thibault
12 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-01-16 8:33 UTC (permalink / raw)
To: minios-devel, xen-devel; +Cc: samuel.thibault, wl, Juergen Gross
There is no user of the files[] array outside of lib/sys.c left, so
it can be made static.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
include/lib.h | 2 --
lib/sys.c | 2 +-
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/include/lib.h b/include/lib.h
index e815e0a3..bec99646 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -175,8 +175,6 @@ struct file {
};
};
-extern struct file files[];
-
struct file_ops {
const char *name;
int (*read)(struct file *file, void *buf, size_t nbytes);
diff --git a/lib/sys.c b/lib/sys.c
index 7f3dc4e4..8f8a3de2 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -89,7 +89,7 @@ extern void minios_evtchn_close_fd(int fd);
extern void minios_gnttab_close_fd(int fd);
pthread_mutex_t fd_lock = PTHREAD_MUTEX_INITIALIZER;
-struct file files[NOFILE] = {
+static struct file files[NOFILE] = {
{ .type = FTYPE_CONSOLE }, /* stdin */
{ .type = FTYPE_CONSOLE }, /* stdout */
{ .type = FTYPE_CONSOLE }, /* stderr */
--
2.26.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [MINIOS PATCH v3 00/12] remove device specific struct file members
2022-01-16 8:33 [MINIOS PATCH v3 00/12] remove device specific struct file members Juergen Gross
` (11 preceding siblings ...)
2022-01-16 8:33 ` [MINIOS PATCH v3 12/12] make files array private to sys.c Juergen Gross
@ 2022-01-16 21:03 ` Samuel Thibault
12 siblings, 0 replies; 29+ messages in thread
From: Samuel Thibault @ 2022-01-16 21:03 UTC (permalink / raw)
To: Juergen Gross; +Cc: minios-devel, xen-devel, wl
Juergen Gross, le dim. 16 janv. 2022 09:33:16 +0100, a ecrit:
> This series is a followup to the series sent recently:
>
> https://lists.xen.org/archives/html/xen-devel/2022-01/threads.html#00635
>
> It contains the last cleanups related to struct file and can only be
> applied after the Xen libraries have stopped using the related union
> members:
>
> https://lists.xen.org/archives/html/xen-devel/2022-01/threads.html#00643
>
> The three series applied have been tested to work with:
>
> - xenstorepvh-stubdom
> - pv-grub
> - ioemu-stubdom
Thanks!
Samuel
^ permalink raw reply [flat|nested] 29+ messages in thread