[PATCH 0/2] mini-os: remove device specific struct file members

[PATCH 0/2] mini-os: remove device specific struct file members

[Juergen Gross]

This small series is a followup to the series sent recently:

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

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.

Juergen Gross (2):
  mini-os: remove event channel specific struct file member
  mini-os: remove gnttab specific member from struct file

 include/lib.h | 13 -------------
 1 file changed, 13 deletions(-)

-- 
2.26.2

[PATCH 1/2] mini-os: remove event channel specific struct file member

[Juergen Gross]

The event channel specific union member in struct file is no longer
needed, so remove it together with the associated structure
definitions.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 include/lib.h | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/include/lib.h b/include/lib.h
index 7a0546b..cbfeea3 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -173,15 +173,6 @@ enum fd_type {
     FTYPE_TPM_TIS,
 };
 
-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 {
     enum fd_type type;
     bool read;	/* maybe available for read */
@@ -189,9 +180,6 @@ struct file {
     union {
         int fd; /* Any fd from an upper layer. */
         void *dev;
-	struct {
-	    struct evtchn_port_list ports;
-	} evtchn;
 	struct gntmap gntmap;
     };
 };
-- 
2.26.2

[PATCH 2/2] mini-os: remove gnttab specific member from struct file

[Juergen Gross]

The event channel specific union member in struct file is no longer
needed, so remove it

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 include/lib.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/lib.h b/include/lib.h
index cbfeea3..b82e285 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -180,7 +180,6 @@ struct file {
     union {
         int fd; /* Any fd from an upper layer. */
         void *dev;
-	struct gntmap gntmap;
     };
 };
 
-- 
2.26.2

Re: [PATCH 0/2] mini-os: remove device specific struct file members

[Andrew Cooper]

On 07/01/2022 10:47, Juergen Gross wrote:
> This small series is a followup to the series sent recently:
>
> https://lists.xen.org/archives/html/xen-devel/2022-01/threads.html#00110
>
> 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.
>
> Juergen Gross (2):
>   mini-os: remove event channel specific struct file member
>   mini-os: remove gnttab specific member from struct file
>
>  include/lib.h | 13 -------------
>  1 file changed, 13 deletions(-)
>

This all looks fine.  Even if we don't want to hide struct file,
shouldn't we at least make the files[] array private to the MiniOS
core?  It's another implementation detail which external users mustn't
rely on.

~Andrew

Re: [PATCH 0/2] mini-os: remove device specific struct file members

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 1012 bytes --]

On 07.01.22 12:30, Andrew Cooper wrote:
> On 07/01/2022 10:47, Juergen Gross wrote:
>> This small series is a followup to the series sent recently:
>>
>> https://lists.xen.org/archives/html/xen-devel/2022-01/threads.html#00110
>>
>> 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.
>>
>> Juergen Gross (2):
>>    mini-os: remove event channel specific struct file member
>>    mini-os: remove gnttab specific member from struct file
>>
>>   include/lib.h | 13 -------------
>>   1 file changed, 13 deletions(-)
>>
> 
> This all looks fine.  Even if we don't want to hide struct file,
> shouldn't we at least make the files[] array private to the MiniOS
> core?  It's another implementation detail which external users mustn't
> rely on.

In theory yes.

This will need some more work either by splitting headers or by
using get_file_from_fd() everywhere in Mini-OS.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH 1/2] mini-os: remove event channel specific struct file member

[Samuel Thibault]

Juergen Gross, le ven. 07 janv. 2022 11:47:05 +0100, a ecrit:
> The event channel specific union member in struct file is no longer
> needed,

Doesn't xen/tools/libs/evtchn/minios.c use it?

> so remove it together with the associated structure
> definitions.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  include/lib.h | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/include/lib.h b/include/lib.h
> index 7a0546b..cbfeea3 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -173,15 +173,6 @@ enum fd_type {
>      FTYPE_TPM_TIS,
>  };
>  
> -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 {
>      enum fd_type type;
>      bool read;	/* maybe available for read */
> @@ -189,9 +180,6 @@ struct file {
>      union {
>          int fd; /* Any fd from an upper layer. */
>          void *dev;
> -	struct {
> -	    struct evtchn_port_list ports;
> -	} evtchn;
>  	struct gntmap gntmap;
>      };
>  };
> -- 
> 2.26.2

Re: [PATCH 0/2] mini-os: remove device specific struct file members

[Samuel Thibault]

Juergen Gross, le ven. 07 janv. 2022 11:47:04 +0100, a ecrit:
> can only be applied after the Xen libraries have stopped using the
> related union members.

Ah, ok :) Was that submitted somewhere?

Samuel

Re: [PATCH 0/2] mini-os: remove device specific struct file members

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 714 bytes --]

On 09.01.22 02:16, Samuel Thibault wrote:
> Juergen Gross, le ven. 07 janv. 2022 11:47:04 +0100, a ecrit:
>> can only be applied after the Xen libraries have stopped using the
>> related union members.
> 
> Ah, ok :) Was that submitted somewhere?

I've sent the patches, but they can be applied only after the first
Mini-OS series has been applied and the Xen build is using the
updated Mini-OS tree.

Meanwhile I've found a way to decouple Mini-OS and the Xen libs even
further. So this series and the Xen one will be updated.

In the end the libs will use dynamic hooks and Mini-OS will no longer
need to have a config knob for supporting the libs (other than having
libc enabled).


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH 1/2] mini-os: remove event channel specific struct file member

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 307 bytes --]

On 09.01.22 02:15, Samuel Thibault wrote:
> Juergen Gross, le ven. 07 janv. 2022 11:47:05 +0100, a ecrit:
>> The event channel specific union member in struct file is no longer
>> needed,
> 
> Doesn't xen/tools/libs/evtchn/minios.c use it?

Not after the Xen patches have been applied.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH 0/2] mini-os: remove device specific struct file members

[Andrew Cooper]

On 09/01/2022 01:16, Samuel Thibault wrote:
> Juergen Gross, le ven. 07 janv. 2022 11:47:04 +0100, a ecrit:
>> can only be applied after the Xen libraries have stopped using the
>> related union members.
> Ah, ok :) Was that submitted somewhere?

https://lore.kernel.org/xen-devel/20220107103544.9271-1-jgross@suse.com/T/#u

~Andrew

Re: [PATCH 0/2] mini-os: remove device specific struct file members

[Samuel Thibault]

Andrew Cooper, le lun. 10 janv. 2022 11:35:14 +0000, a ecrit:
> On 09/01/2022 01:16, Samuel Thibault wrote:
> > Juergen Gross, le ven. 07 janv. 2022 11:47:04 +0100, a ecrit:
> >> can only be applied after the Xen libraries have stopped using the
> >> related union members.
> > Ah, ok :) Was that submitted somewhere?
> 
> https://lore.kernel.org/xen-devel/20220107103544.9271-1-jgross@suse.com/T/#u

Ok!

Samuel