Re: [PATCH v3] machdev,rump,pci: Rework bootstrap

[Samuel Thibault]

Applied, thanks and congrats!

Samuel

Damien Zammit, le mar. 16 mars 2021 16:47:15 +1100, a ecrit:
> * libmachdev/trivfs_server.c
> Rework the bootstrapping of machdev
> * pci-arbiter/Makefile
> Remove startup.c
> * pci-arbiter/startup.{c,h}
> Delete files
> * pci-arbiter/main.c
> (main): Rework bootstrap, remove duplicated notification
> * rumpdisk/main.c
> (main): Don't pretend to be the arbiter anymore
> 
> ---
>  libmachdev/trivfs_server.c | 101 ++++++++++++++++++++++++++++---------
>  pci-arbiter/Makefile       |   2 +-
>  pci-arbiter/main.c         |  29 +++--------
>  pci-arbiter/startup.c      |  60 ----------------------
>  pci-arbiter/startup.h      |  31 ------------
>  rumpdisk/main.c            |   7 +--
>  6 files changed, 86 insertions(+), 144 deletions(-)
>  delete mode 100644 pci-arbiter/startup.c
>  delete mode 100644 pci-arbiter/startup.h
> 
> diff --git a/libmachdev/trivfs_server.c b/libmachdev/trivfs_server.c
> index e3e4045d..607166f9 100644
> --- a/libmachdev/trivfs_server.c
> +++ b/libmachdev/trivfs_server.c
> @@ -63,7 +63,7 @@ struct port_class *trivfs_protid_class;
>  struct trivfs_control *control;
>  
>  /* Are we providing bootstrap translator? */
> -static boolean_t bootstrapped;
> +static boolean_t bootstrapping;
>  
>  /* Our underlying node in the FS for bootstrap */
>  static mach_port_t underlying;
> @@ -239,12 +239,37 @@ trivfs_S_fsys_startup (mach_port_t bootport,
>                         mach_port_t *realnode,
>                         mach_msg_type_name_t *realnodetype)
>  {
> +  mach_port_t mybootport;
> +
>    control_port = cntl;
>    *realnode = MACH_PORT_NULL;
> -  *realnodetype = MACH_MSG_TYPE_MOVE_SEND;
> +  *realnodetype = MACH_MSG_TYPE_COPY_SEND;
> +
> +  task_get_bootstrap_port (mach_task_self (), &mybootport);
> +  if (mybootport)
> +    fsys_startup (mybootport, flags, control_port, MACH_MSG_TYPE_COPY_SEND, 
> realnode);
>    return 0;
>  }
>  
> +static void
> +essential_task (void)
> +{
> +  mach_port_t host, startup;
> +
> +  get_privileged_ports (&host, 0);
> +  startup = file_name_lookup (_SERVERS_STARTUP, 0, 0);
> +  if (startup == MACH_PORT_NULL)
> +    {
> +      mach_print ("WARNING: Cannot register as essential task\n");
> +      mach_port_deallocate (mach_task_self (), host);
> +      return;
> +    }
> +  startup_essential_task (startup, mach_task_self (), MACH_PORT_NULL,
> +                          program_invocation_short_name, host);
> +  mach_port_deallocate (mach_task_self (), startup);
> +  mach_port_deallocate (mach_task_self (), host);
> +}
> +
>  kern_return_t
>  trivfs_S_fsys_init (struct trivfs_control *fsys,
>                      mach_port_t reply, mach_msg_type_name_t replytype,
> @@ -255,11 +280,19 @@ trivfs_S_fsys_init (struct trivfs_control *fsys,
>    mach_port_t *portarray;
>    unsigned int i;
>    uid_t idlist[] = {0, 0, 0};
> -  mach_port_t root;
> +  mach_port_t root, bootstrap;
>    retry_type retry;
>    string_t retry_name;
>    mach_port_t right = MACH_PORT_NULL;
> +  process_t proc;
>  
> +  /* Traverse to the bootstrapping server first */
> +  task_get_bootstrap_port (mach_task_self (), &bootstrap);
> +  if (bootstrap)
> +    {
> +      err = fsys_init (bootstrap, procserver, MACH_MSG_TYPE_COPY_SEND, 
> authhandle);
> +      assert_perror_backtrace (err);
> +    }
>    err = fsys_getroot (control_port, MACH_PORT_NULL, MACH_MSG_TYPE_COPY_SEND,
>                        idlist, 3, idlist, 3, 0,
>                        &retry, retry_name, &root);
> @@ -278,16 +311,32 @@ trivfs_S_fsys_init (struct trivfs_control *fsys,
>    portarray[INIT_PORT_CWDIR] = root;
>    _hurd_init (0, NULL, portarray, INIT_PORT_MAX, NULL, 0);
>  
> -  arrange_shutdown_notification ();
> +  /* Mark us as important.  */
> +  proc = getproc ();
> +  assert_backtrace (proc);
> +  err = proc_mark_important (proc);
> +  assert_perror_backtrace (err);
> +  err = proc_mark_exec (proc);
> +  assert_perror_backtrace (err);
> +  proc_set_exe (proc, program_invocation_short_name);
> +  mach_port_deallocate (mach_task_self (), proc);
>  
> -  /* Install the bootstrap port on /dev/something so users
> -   * can still access the bootstrapped device */
> -  if (bootstrapped && devnode)
> +  if (bootstrapping)
>      {
> -      right = ports_get_send_right (&control->pi);
> -      install_as_translator (right);
> -      control->underlying = underlying;
> +      if (devnode)
> +        {
> +          /* Install the bootstrap port on /dev/something so users
> +           * can still access the bootstrapped device */
> +          right = ports_get_send_right (&control->pi);
> +          install_as_translator (right);
> +          control->underlying = underlying;
> +        }
> +      /* Mark us as essential if bootstrapping */
> +      essential_task ();
>      }
> +
> +  arrange_shutdown_notification ();
> +
>    return 0;
>  }
>  
> @@ -296,12 +345,8 @@ arrange_shutdown_notification (void)
>  {
>    error_t err;
>    mach_port_t initport, notify;
> -  process_t proc;
>    struct port_info *pi;
>  
> -  proc = getproc ();
> -  assert_backtrace (proc);
> -
>    machdev_shutdown_notify_class = ports_create_class (0, 0);
>  
>    /* Arrange to get notified when the system goes down */
> @@ -310,10 +355,6 @@ arrange_shutdown_notification (void)
>    if (err)
>      return;
>  
> -  /* Mark us as important.  */
> -  err = proc_mark_important (proc);
> -  mach_port_deallocate (mach_task_self (), proc);
> -
>    initport = file_name_lookup (_SERVERS_STARTUP, 0, 0);
>    if (initport == MACH_PORT_NULL)
>      {
> @@ -380,37 +421,47 @@ resume_bootstrap_server(mach_port_t server_task, const 
> char *server_name)
>    stdout = stderr = mach_open_devstream (cons, "w");
>    mach_port_deallocate (mach_task_self (), cons);
>  
> -  printf ("Hurd bootstrap %s ", server_name);
> +  printf ("%s ", server_name);
>    fflush (stdout);
>  }
>  
>  int
> -machdev_trivfs_init(mach_port_t bootstrap_resume_task, const char *name, 
> const char *path, mach_port_t *bootstrap)
> +machdev_trivfs_init(mach_port_t bootstrap_resume_task, const char *name, 
> const char *path,
> +                    mach_port_t *bootstrap)
>  {
> +  mach_port_t mybootstrap = MACH_PORT_NULL;
> +  task_t parent_task;
>    port_bucket = ports_create_bucket ();
>    trivfs_cntl_class = ports_create_class (trivfs_clean_cntl, 0);
>    trivfs_protid_class = ports_create_class (trivfs_clean_protid, 0);
>    trivfs_create_control (MACH_PORT_NULL, trivfs_cntl_class, port_bucket,
>                           trivfs_protid_class, 0, &control);
>  
> +  *bootstrap = MACH_PORT_NULL;
> +
> +  task_get_bootstrap_port (mach_task_self (), &mybootstrap);
> +  if (mybootstrap)
> +    {
> +      *bootstrap = mybootstrap;
> +      fsys_getpriv (*bootstrap, &_hurd_host_priv, &_hurd_device_master, 
> &parent_task);
> +    }
> +
>    if (bootstrap_resume_task != MACH_PORT_NULL)
>      {
>        if (path)
>       devnode = strdup(path);
>        resume_bootstrap_server(bootstrap_resume_task, name);
> -      *bootstrap = ports_get_send_right (&control->pi);
>  
>        /* We need to install as a translator later */
> -      bootstrapped = TRUE;
> +      bootstrapping = TRUE;
>      }
>    else
>      {
> -      task_get_bootstrap_port (mach_task_self (), bootstrap);
>        if (*bootstrap == MACH_PORT_NULL)
>          error (1, 0, "must be started as a translator");
>  
>        /* We do not need to install as a translator later */
> -      bootstrapped = FALSE;
> +      bootstrapping = FALSE;
>      }
>  
>    return 0;
> @@ -502,7 +553,7 @@ machdev_trivfs_server(mach_port_t bootstrap)
>    int err;
>    pthread_t t;
>  
> -  if (bootstrapped == FALSE)
> +  if (bootstrapping == FALSE)
>      {
>        /* This path is executed when a parent exists */
>        err = trivfs_startup (bootstrap, 0,
> diff --git a/pci-arbiter/Makefile b/pci-arbiter/Makefile
> index b32bc579..d3d205ec 100644
> --- a/pci-arbiter/Makefile
> +++ b/pci-arbiter/Makefile
> @@ -21,7 +21,7 @@ makemode    = server
>  PORTDIR = $(srcdir)/port
>  
>  SRCS         = main.c pci-ops.c netfs_impl.c \
> -               pcifs.c ncache.c options.c func_files.c startup.c \
> +               pcifs.c ncache.c options.c func_files.c \
>                 pciServer.c startup_notifyServer.c
>  OBJS         = $(SRCS:.c=.o) $(MIGSTUBS)
>  
> diff --git a/pci-arbiter/main.c b/pci-arbiter/main.c
> index efb9f65c..1815994a 100644
> --- a/pci-arbiter/main.c
> +++ b/pci-arbiter/main.c
> @@ -43,7 +43,6 @@
>  #include <pciaccess.h>
>  #include <pthread.h>
>  #include "pcifs.h"
> -#include "startup.h"
>  
>  struct pcifs *fs;
>  volatile struct mapped_time_value *pcifs_maptime;
> @@ -106,17 +105,9 @@ pci_device_close (void *d)
>  static void
>  pci_device_shutdown (mach_port_t dosync_handle)
>  {
> -  struct port_info *inpi = ports_lookup_port (netfs_port_bucket, 
> dosync_handle,
> -                                           pci_shutdown_notify_class);
> -
> -  if (!inpi)
> -    return;
> -
>    // Free all libpciaccess resources
>    pci_system_cleanup ();
>  
> -  ports_port_deref (inpi);
> -
>    ports_destroy_right (&pci_control_port);
>  
>    netfs_shutdown (FSYS_GOAWAY_FORCE);
> @@ -192,15 +183,17 @@ pcifs_startup(mach_port_t bootstrap, int flags)
>  
>    err = ports_create_port (netfs_control_class, netfs_port_bucket,
>                            sizeof (struct port_info), &newpi);
> -  if (!err)
> +  if (err)
> +    error (11, err, "Translator startup failure: pcifs_startup");
> +
> +  pci_control_port = ports_get_send_right (newpi);
> +
> +  if (bootstrap != MACH_PORT_NULL)
>      {
> -      pci_control_port = ports_get_send_right (newpi);
>        err = fsys_startup (bootstrap, flags, pci_control_port, 
> MACH_MSG_TYPE_COPY_SEND,
> -                         &realnode);
> +                          &realnode);
>        assert_perror_backtrace (err);
>      }
> -  if (err)
> -    error (11, err, "Translator startup failure: fsys_startup");
>  
>    return realnode;
>  }
> @@ -221,8 +214,8 @@ main (int argc, char **argv)
>    if (disk_server_task != MACH_PORT_NULL)
>      {
>        machdev_register (&pci_arbiter_emulation_ops);
> -      machdev_device_init ();
>        machdev_trivfs_init (disk_server_task, "pci", "/servers/bus/pci", 
> &bootstrap);
> +      machdev_device_init ();
>        err = pthread_create (&t, NULL, machdev_server, NULL);
>        if (err)
>          error (1, err, "Creating machdev thread");
> @@ -278,12 +271,6 @@ main (int argc, char **argv)
>      error (1, err, "Creating netfs loop thread");
>    pthread_detach (nt);
>  
> -  /*
> -   * Ask init to tell us when the system is going down,
> -   * so we can try to be friendly to our correspondents on the network.
> -   */
> -  arrange_shutdown_notification ();
> -
>    /* Let the other threads do their job */
>    pthread_exit(NULL);
>    /* Never reached */
> diff --git a/pci-arbiter/startup.c b/pci-arbiter/startup.c
> deleted file mode 100644
> index 421c9e24..00000000
> --- a/pci-arbiter/startup.c
> +++ /dev/null
> @@ -1,60 +0,0 @@
> -/*
> -   Copyright (C) 2017 Free Software Foundation, Inc.
> -   Written by Michael I. Bushnell, p/BSG.
> -
> -   This file is part of the GNU Hurd.
> -
> -   The GNU Hurd is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU General Public License as
> -   published by the Free Software Foundation; either version 2, or (at
> -   your option) any later version.
> -
> -   The GNU Hurd is distributed in the hope that it will be useful, but
> -   WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   General Public License for more details.
> -
> -   You should have received a copy of the GNU General Public License
> -   along with the GNU Hurd.  If not, see <http://www.gnu.org/licenses/>.
> -*/
> -
> -/* Startup and shutdown notifications management */
> -
> -#include "startup.h"
> -
> -#include <unistd.h>
> -#include <hurd/paths.h>
> -#include <hurd/startup.h>
> -#include <hurd/netfs.h>
> -
> -struct port_class *pci_shutdown_notify_class;
> -
> -void
> -arrange_shutdown_notification ()
> -{
> -  error_t err;
> -  mach_port_t initport, notify;
> -  struct port_info *pi;
> -
> -  pci_shutdown_notify_class = ports_create_class (0, 0);
> -
> -  /* Arrange to get notified when the system goes down,
> -     but if we fail for some reason, just silently give up.  No big deal. */
> -
> -  err = ports_create_port (pci_shutdown_notify_class, netfs_port_bucket,
> -                        sizeof (struct port_info), &pi);
> -  if (err)
> -    return;
> -
> -  initport = file_name_lookup (_SERVERS_STARTUP, 0, 0);
> -  if (initport == MACH_PORT_NULL)
> -    return;
> -
> -  notify = ports_get_send_right (pi);
> -  ports_port_deref (pi);
> -  startup_request_notification (initport, notify,
> -                             MACH_MSG_TYPE_MAKE_SEND,
> -                             program_invocation_short_name);
> -  mach_port_deallocate (mach_task_self (), notify);
> -  mach_port_deallocate (mach_task_self (), initport);
> -}
> diff --git a/pci-arbiter/startup.h b/pci-arbiter/startup.h
> deleted file mode 100644
> index 416b84d6..00000000
> --- a/pci-arbiter/startup.h
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -/*
> -   Copyright (C) 2017 Free Software Foundation, Inc.
> -   Written by Michael I. Bushnell, p/BSG.
> -
> -   This file is part of the GNU Hurd.
> -
> -   The GNU Hurd is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU General Public License as
> -   published by the Free Software Foundation; either version 2, or (at
> -   your option) any later version.
> -
> -   The GNU Hurd is distributed in the hope that it will be useful, but
> -   WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   General Public License for more details.
> -
> -   You should have received a copy of the GNU General Public License
> -   along with the GNU Hurd.  If not, see <http://www.gnu.org/licenses/>.
> -*/
> -
> -#ifndef STARTUP_H
> -#define STARTUP_H
> -
> -/* Startup and shutdown notifications management */
> -
> -/* Port class for startup requests */
> -extern struct port_class *pci_shutdown_notify_class;
> -
> -void arrange_shutdown_notification (void);
> -
> -#endif /* STARTUP_H */
> diff --git a/rumpdisk/main.c b/rumpdisk/main.c
> index a26e17cb..87bc5573 100644
> --- a/rumpdisk/main.c
> +++ b/rumpdisk/main.c
> @@ -31,11 +31,6 @@
>  #include <pthread.h>
>  #include <mach.h>
>  
> -/* TODO: Add api to pciaccess to allow selecting backend.
> - * For now we pretend to be the arbiter and claim x86 method.
> - */
> -char *netfs_server_name = "pci-arbiter";
> -
>  mach_port_t bootstrap_resume_task = MACH_PORT_NULL;
>  
>  static const struct argp_option options[] = {
> @@ -117,8 +112,8 @@ main (int argc, char **argv)
>      }
>  
>    rump_register_block ();
> -  machdev_device_init ();
>    machdev_trivfs_init (bootstrap_resume_task, "rumpdisk", "/dev/rumpdisk", 
> &bootstrap);
> +  machdev_device_init ();
>    err = pthread_create (&t, NULL, machdev_server, NULL);
>    if (err)
>      return err;
> -- 
> 2.30.1
> 
> 

-- 
Samuel
Pour un père, autant mourir que de faire plein de calculs et pas s'occuper
de son fils
 -+- y sur #ens-mim - sombres histoires de zombies -+-