Re: [PATCH] PCI Arbiter

[Joan Lledó]

Hi!

2017-10-20 15:18 GMT+02:00 Justus Winter <teythoon@avior.uberspace.de>:
> Hi Joan :)
>
> let me be the first to say: Awesome :)
>
> Joan Lledó <joanlluislledo@gmail.com> writes:
>
>> Attached are two patches. The first one is to be applied to the Hurd
>> source tree and contains a new mig pci interface, the pci server and a
>> new library which includes the mig client stubs.
>
> So I'd say the libpciclient library is a bit superfluous, that could be
> done in the Hurd-specific libpcibackend.

What do you mean with libpcibackend? Adding that code to libpciaccess
and pciutils? If so, I was talking to Samuel about that (irc channel
at 2017-09-28) and he suggested me to create this library to avoid
adding mig stuff to pciutils and libpciaccess.

>> The second one is to be applied to the debian libpciaccess repository[1] and 
>> adds a new module for the Hurd. There's no patch for pciutils for now.
>
> I didn't get that one.

Yes, don't know what happened. I've uploaded it to darnassus[1].

>> I've tested them and they seem to be working fine with netdde in my box.
>
> Sweet.
>
>> I'd like to have some comments, no need for a deep review, but I had to make 
>> some design decisions and wouldn't like to keep working on a wrong approach.
>
> Ok, I don't have access to my box right now, but I can do a quick review
> of the Hurdish bits.
>
>> ------------------
>> [1] https://anonscm.debian.org/cgit/pkg-xorg/lib/libpciaccess.git/
>> ---
>>  Makefile                  |   2 +
>>  hurd/hurd_types.defs      |  19 ++-
>>  hurd/hurd_types.h         |   1 +
>>  hurd/paths.h              |   1 +
>>  hurd/pci.defs             |  50 +++++++
>>  hurd/subsystems           |   1 +
>>  libpciclient/Makefile     |  29 ++++
>>  libpciclient/pciclient.c  | 103 +++++++++++++++
>>  libpciclient/pciclient.h  |  42 ++++++
>>  pci_arbiter/Makefile      |  40 ++++++
>
> I'd suggest pci-arbiter.
>
>>  pci_arbiter/config.h      |   5 +
>>  pci_arbiter/main.c        | 116 ++++++++++++++++
>>  pci_arbiter/mig-mutate.h  |  28 ++++
>>  pci_arbiter/pci-ops.c     |  89 +++++++++++++
>>  pci_arbiter/pci_access.c  |  51 +++++++
>>  pci_arbiter/pci_access.h  |  78 +++++++++++
>>  pci_arbiter/pci_arbiter.h |  46 +++++++
>>  pci_arbiter/x86_pci.c     | 331 
>> ++++++++++++++++++++++++++++++++++++++++++++++
>>  pci_arbiter/x86_pci.h     |  42 ++++++
>>  19 files changed, 1073 insertions(+), 1 deletion(-)
>>  create mode 100644 hurd/pci.defs
>>  create mode 100644 libpciclient/Makefile
>>  create mode 100644 libpciclient/pciclient.c
>>  create mode 100644 libpciclient/pciclient.h
>>  create mode 100644 pci_arbiter/Makefile
>>  create mode 100644 pci_arbiter/config.h
>>  create mode 100644 pci_arbiter/main.c
>>  create mode 100644 pci_arbiter/mig-mutate.h
>>  create mode 100644 pci_arbiter/pci-ops.c
>>  create mode 100644 pci_arbiter/pci_access.c
>>  create mode 100644 pci_arbiter/pci_access.h
>>  create mode 100644 pci_arbiter/pci_arbiter.h
>>  create mode 100644 pci_arbiter/x86_pci.c
>>  create mode 100644 pci_arbiter/x86_pci.h
>>
>> diff --git a/Makefile b/Makefile
>> index 119f130b..d3d93b87 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -31,6 +31,7 @@ lib-subdirs = libshouldbeinlibc libihash libiohelp 
>> libports libthreads \
>>             libnetfs libpipe libstore libhurdbugaddr libftpconn libcons \
>>             libhurd-slab \
>>             libbpf \
>> +           libpciclient
>>
>>  # Hurd programs
>>  prog-subdirs = auth proc exec term \
>> @@ -45,6 +46,7 @@ prog-subdirs = auth proc exec term \
>>              init \
>>              devnode \
>>              eth-multiplexer \
>> +            pci_arbiter
>>
>>  ifeq ($(HAVE_SUN_RPC),yes)
>>  prog-subdirs += nfs nfsd
>> diff --git a/hurd/hurd_types.defs b/hurd/hurd_types.defs
>> index 4d7013c8..0e9b990e 100644
>> --- a/hurd/hurd_types.defs
>> +++ b/hurd/hurd_types.defs
>> @@ -1,5 +1,5 @@
>>  /* MiG type declarations for Hurd interfaces         -*- C -*-
>> -   Copyright (C) 1993,94,95,96,98,2001,02 Free Software Foundation, Inc.
>> +   Copyright (C) 1993,94,95,96,98,2001,02,17 Free Software Foundation, Inc.
>>
>>  This file is part of the GNU Hurd.
>>
>> @@ -296,6 +296,23 @@ destructor: INTERRUPT_DESTRUCTOR
>>  #endif
>>  ;
>>
>> +/* PCI arbiter */
>> +type pci_t = mach_port_copy_send_t
>> +#ifdef PCI_INTRAN
>> +intran: PCI_INTRAN
>> +intranpayload: PCI_INTRAN_PAYLOAD
>> +#else
>> +#ifdef HURD_DEFAULT_PAYLOAD_TO_PORT
>> +intranpayload: pci_t HURD_DEFAULT_PAYLOAD_TO_PORT
>> +#endif
>> +#endif
>> +#ifdef PCI_OUTTRAN
>> +outtran: PCI_OUTTRAN
>> +#endif
>> +#ifdef PCI_DESTRUCTOR
>> +destructor: PCI_DESTRUCTOR
>> +#endif
>> +;
>>
>>  type proccoll_t = mach_port_copy_send_t;
>>
>> diff --git a/hurd/hurd_types.h b/hurd/hurd_types.h
>> index 2960a294..8b1092a5 100644
>> --- a/hurd/hurd_types.h
>> +++ b/hurd/hurd_types.h
>> @@ -50,6 +50,7 @@ typedef mach_port_t exec_startup_t;
>>  typedef mach_port_t interrupt_t;
>>  typedef mach_port_t proccoll_t;
>>  typedef mach_port_t ctty_t;
>> +typedef mach_port_t pci_t;
>>
>>  #include <errno.h>           /* Defines `error_t'.  */
>>
>> diff --git a/hurd/paths.h b/hurd/paths.h
>> index e1b00e90..1484b43e 100644
>> --- a/hurd/paths.h
>> +++ b/hurd/paths.h
>> @@ -30,6 +30,7 @@ the Free Software Foundation, 675 Mass Ave, Cambridge, MA 
>> 02139, USA.  */
>>  #define _SERVERS_PROC                _SERVERS "proc"
>>  #define _SERVERS_PASSWORD    _SERVERS "password"
>>  #define _SERVERS_DEFPAGER    _SERVERS "default-pager"
>> +#define _SERVERS_PCI         _SERVERS "pci"
>>
>>  /* Directory containing naming points for socket servers.
>>     Entries are named by the string representing the domain number
>> diff --git a/hurd/pci.defs b/hurd/pci.defs
>> new file mode 100644
>> index 00000000..d051bd69
>> --- /dev/null
>> +++ b/hurd/pci.defs
>> @@ -0,0 +1,50 @@
>> +/* Definitions for pci-specific calls
>> +   Copyright (C) 2017 Free Software Foundation, Inc.
>> +
>> +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; see the file COPYING.  If not, write to
>> +the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.  */
>> +
>> +subsystem pci 39000;
>> +
>> +#include <hurd/hurd_types.defs>
>> +
>> +#ifdef PCI_IMPORTS
>> +PCI_IMPORTS
>> +#endif
>> +
>> +INTR_INTERFACE
>
> Leftover here.
>
>> +
>> +/* Read 'amount' bytes from B/D/F + 'reg' and store it in 'data' */
>> +routine pci_read (
>> +     master: pci_t;
>> +     bus: int;
>> +     dev: int;
>> +     func: int;
>> +     reg: int;
>> +     out data: data_t, dealloc;
>> +     amount: vm_size_t
>> +);
>> +
>> +/* Write 'amount' bytes from 'data' to B/D/F + 'reg' */
>> +routine pci_write(
>> +     master: pci_t;
>> +     bus: int;
>> +     dev: int;
>> +     func: int;
>> +     reg: int;
>> +     data: data_t;
>> +     out amount: vm_size_t
>> +);
>> diff --git a/hurd/subsystems b/hurd/subsystems
>> index c05895c2..0677bb1e 100644
>> --- a/hurd/subsystems
>> +++ b/hurd/subsystems
>> @@ -36,6 +36,7 @@ tape                35000   Special control operations for 
>> magtapes
>>  login                36000   Database of logged-in users
>>  pfinet               37000   Internet configuration calls
>>  password     38000   Password checker
>> +pci          39000   PCI arbiter
>>  <ioctl space>  100000-       First subsystem of ioctl class 'f' (lowest 
>> class)
>>  tioctl              156000   Ioctl class 't' (terminals)
>>  tioctl              156200     (continued)
>
> Looks good otherwise.  I can't judge the interface, but that is easy
> enough to extend later.
>
>> diff --git a/libpciclient/Makefile b/libpciclient/Makefile
>> new file mode 100644
>> index 00000000..d550115d
>> --- /dev/null
>> +++ b/libpciclient/Makefile
>> @@ -0,0 +1,29 @@
>> +#   Copyright (C) 2017 Free Software Foundation, Inc.
>> +#
>> +#   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/>.
>> +
>> +dir := libpciclient
>> +makemode := library
>> +
>> +libname = libpciclient
>> +installhdrs = pciclient.h
>> +
>> +SRCS = pciclient.c
>> +
>> +MIGSTUBS = pciUser.o
>> +OBJS = $(sort $(SRCS:.c=.o)) $(MIGSTUBS)
>> +
>> +include ../Makeconf
>> diff --git a/libpciclient/pciclient.c b/libpciclient/pciclient.c
>> new file mode 100644
>> index 00000000..9f03dc30
>> --- /dev/null
>> +++ b/libpciclient/pciclient.c
>> @@ -0,0 +1,103 @@
>> +/*
>> +   Copyright (C) 2017 Free Software Foundation, Inc.
>> +
>> +   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/>.
>> +*/
>> +
>> +/* PCI Client implementation */
>> +
>> +#include <pciclient.h>
>> +
>> +#include <hurd.h>
>> +#include <hurd/paths.h>
>> +#include <error.h>
>> +#include <sys/types.h>
>> +#include <errno.h>
>> +#include <string.h>
>> +
>> +#include <pci_U.h>
>> +
>> +/* Get the server port */
>> +static void
>> +init_client ()
>> +{
>> +  pci_server_port = file_name_lookup (_SERVERS_PCI, 0, 0);
>> +
>> +  if (pci_server_port == MACH_PORT_NULL)
>> +    error (-1, errno, "Could not open file `%s'", _SERVERS_PCI);
>> +
>> +  return;
>> +}
>> +
>> +/*
>> + * Read 'size' bytes from B/D/F + reg and store them in 'data'.
>> + *
>> + * It's assumed that 'size' bytes are allocated in 'data'
>> + */
>> +int
>> +pciclient_cfg_read (int bus, int dev, int func, int reg, char *buf,
>> +                 size_t * nbytes)
>> +{
>> +  error_t err;
>> +  size_t nread;
>> +  char *data;
>> +
>> +  if (pci_server_port == MACH_PORT_NULL)
>> +    init_client ();
>> +
>> +  data = buf;
>> +  nread = *nbytes;
>> +  err =
>> +    pci_read (pci_server_port, bus, dev, func, reg, &data, &nread, *nbytes);
>> +  if (err)
>> +    return err;
>> +
>> +  if (data != buf)
>> +    {
>> +      if (nread > *nbytes)   /* Sanity check for bogus server.  */
>> +     {
>> +       vm_deallocate (mach_task_self (), (vm_address_t) data, nread);
>> +       return EGRATUITOUS;
>> +     }
>> +
>> +      memcpy (buf, data, nread);
>> +      vm_deallocate (mach_task_self (), (vm_address_t) data, nread);
>> +    }
>> +
>> +  *nbytes = nread;
>> +
>> +  return 0;
>> +}
>> +
>> +/* Write 'size' bytes from 'data' to B/D/F + reg */
>> +int
>> +pciclient_cfg_write (int bus, int dev, int func, int reg, char *buf,
>> +                  size_t * nbytes)
>> +{
>> +  error_t err;
>> +  size_t nwrote;
>> +
>> +  if (pci_server_port == MACH_PORT_NULL)
>> +    init_client ();
>> +
>> +  err =
>> +    pci_write (pci_server_port, bus, dev, func, reg, buf, *nbytes, &nwrote);
>> +
>> +  if (!err)
>> +    *nbytes = nwrote;
>> +
>> +  return err;
>> +}
>
> I don't see these functions adding anything over just doing that by
> foot in libpciaccess.

I'm not sure to understand what you mean, it's the same as before,
right? you're suggesting to move this code to libpciaccess. If so, as
I said above, the only purpose of this library is to avoid including
the mig stuff in libpciaccess and pciutils (or that's what I
understood :P).

>> diff --git a/libpciclient/pciclient.h b/libpciclient/pciclient.h
>> new file mode 100644
>> index 00000000..5148ba25
>> --- /dev/null
>> +++ b/libpciclient/pciclient.h
>> @@ -0,0 +1,42 @@
>> +/*
>> +   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/>.
>> +*/
>> +
>> +/* PCI client header */
>> +
>> +#ifndef PCI_CLIENT_H
>> +#define PCI_CLIENT_H
>> +
>> +#include <mach/port.h>
>> +#include <sys/types.h>
>> +
>> +/* Server port */
>> +mach_port_t pci_server_port = MACH_PORT_NULL;
>> +
>> +/* Public function declarations */
>> +int
>> +pciclient_cfg_read (int bus, int dev, int func, int reg, char *buf,
>> +                 size_t * nbytes);
>> +
>> +int
>> +pciclient_cfg_write (int bus, int dev, int func, int reg, char *buf,
>> +                  size_t * nbytes);
>> +
>> +#endif /* PCI_CLIENT_H */
>> diff --git a/pci_arbiter/Makefile b/pci_arbiter/Makefile
>> new file mode 100644
>> index 00000000..e3a3b732
>> --- /dev/null
>> +++ b/pci_arbiter/Makefile
>> @@ -0,0 +1,40 @@
>> +#   Copyright (C) 2017 Free Software Foundation, Inc.
>> +#
>> +#   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/>.
>> +
>> +dir          = pci_arbiter
>> +makemode     = server
>> +
>> +PORTDIR = $(srcdir)/port
>> +
>> +SRCS         = main.c pci-ops.c pci_access.c x86_pci.c
>> +MIGSRCS              = pciServer.c
>> +OBJS         = $(patsubst %.S,%.o,$(patsubst %.c,%.o, $(SRCS) $(MIGSRCS)))
>> +
>> +HURDLIBS= trivfs fshelp ports shouldbeinlibc
>> +
>> +target = pci_arbiter
>> +
>> +include ../Makeconf
>> +
>> +CFLAGS += -I$(PORTDIR)/include
>> +
>> +CPPFLAGS += -imacros $(srcdir)/config.h
>> +pci-MIGSFLAGS = -imacros $(srcdir)/mig-mutate.h
>> +
>> +# cpp doesn't automatically make dependencies for -imacros dependencies. 
>> argh.
>> +pci_S.h pciServer.c: mig-mutate.h
>> +$(OBJS): config.h
>> diff --git a/pci_arbiter/config.h b/pci_arbiter/config.h
>> new file mode 100644
>> index 00000000..b0d5196d
>> --- /dev/null
>> +++ b/pci_arbiter/config.h
>> @@ -0,0 +1,5 @@
>> +#define __KERNEL__   1
>> +#undef       __SMP__
>> +
>> +#define _HURD_               1
>> +
>> diff --git a/pci_arbiter/main.c b/pci_arbiter/main.c
>> new file mode 100644
>> index 00000000..5475382a
>> --- /dev/null
>> +++ b/pci_arbiter/main.c
>> @@ -0,0 +1,116 @@
>> +/*
>> +   Copyright (C) 2017 Free Software Foundation, Inc.
>> +
>> +   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/>.
>> +*/
>> +
>> +/* Translator initialization and demuxing */
>> +
>> +#include <pci_arbiter.h>
>> +
>> +#include <error.h>
>> +#include <fcntl.h>
>> +#include <hurd/trivfs.h>
>> +
>> +#include <pci_access.h>
>> +#include <pci_S.h>
>> +
>> +int trivfs_fstype = FSTYPE_MISC;
>> +int trivfs_fsid = 0;
>> +int trivfs_support_read = 0;
>> +int trivfs_support_write = 0;
>> +int trivfs_support_exec = 0;
>> +int trivfs_allow_open = O_READ | O_WRITE;
>> +
>> +void
>> +trivfs_modify_stat (struct trivfs_protid *cred, io_statbuf_t * st)
>> +{
>> +}
>> +
>> +error_t
>> +trivfs_goaway (struct trivfs_control *fsys, int flags)
>> +{
>> +  exit (0);
>> +}
>> +
>> +int
>> +pci_demuxer (mach_msg_header_t * inp, mach_msg_header_t * outp)
>> +{
>> +  mig_routine_t routine;
>> +  if ((routine = pci_server_routine (inp)) ||
>> +      (routine = NULL, trivfs_demuxer (inp, outp)))
>> +    {
>> +      if (routine)
>> +     (*routine) (inp, outp);
>> +      return TRUE;
>> +    }
>> +  else
>> +    return FALSE;
>> +}
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  error_t err;
>> +  struct stat st;
>> +  mach_port_t bootstrap;
>> +
>> +  pci_bucket = ports_create_bucket ();
>> +
>> +  mach_port_allocate (mach_task_self (),
>> +                   MACH_PORT_RIGHT_RECEIVE, &fsys_identity);
>> +
>> +  task_get_bootstrap_port (mach_task_self (), &bootstrap);
>> +  if (bootstrap == MACH_PORT_NULL)
>> +    error (-1, 0, "Must be started as a translator");
>> +
>> +  err = trivfs_add_protid_port_class (&pci_protid_portclass);
>> +  if (err)
>> +    error (1, err, "Error creating protid port class");
>> +
>> +  err = trivfs_add_control_port_class (&pci_cntl_portclass);
>> +  if (err)
>> +    error (1, err, "Error creating control port class");
>> +
>> +  /* Reply to our parent */
>> +  err = trivfs_startup (bootstrap, 0,
>> +                     pci_cntl_portclass,
>> +                     pci_bucket,
>> +                     pci_protid_portclass, pci_bucket, &pcicntl);
>> +  mach_port_deallocate (mach_task_self (), bootstrap);
>> +  if (err)
>> +    {
>> +      return (-1);
>> +    }
>> +
>> +  /* Initialize status from underlying node.  */
>> +  pci_owner = pci_group = 0;
>> +  err = io_stat (pcicntl->underlying, &st);
>> +  if (!err)
>> +    {
>> +      pci_owner = st.st_uid;
>> +      pci_group = st.st_gid;
>> +    }
>> +
>> +  /* Start the PCI system */
>> +  err = pci_system_init ();
>> +  if (err)
>> +    error (1, err, "Error starting the PCI system");
>> +
>> +  ports_manage_port_operations_one_thread (pci_bucket, pci_demuxer, 0);
>
> Maybe add a comment here that we rely on the one threaded nature of the
> server for synchronization.
>
>> +
>> +  return 0;
>> +}
>> diff --git a/pci_arbiter/mig-mutate.h b/pci_arbiter/mig-mutate.h
>> new file mode 100644
>> index 00000000..df68fb05
>> --- /dev/null
>> +++ b/pci_arbiter/mig-mutate.h
>> @@ -0,0 +1,28 @@
>> +/*
>> +   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/>.
>> +*/
>> +
>> +/* Only CPP macro definitions should go in this file. */
>> +
>> +#define PCI_IMPORTS                          \
>> +  import "../libtrivfs/mig-decls.h";         \
>> +
>> +#define PCI_INTRAN trivfs_protid_t trivfs_begin_using_protid (pci_t)
>> +#define PCI_INTRAN_PAYLOAD trivfs_protid_t trivfs_begin_using_protid_payload
>> +#define PCI_DESTRUCTOR trivfs_end_using_protid (trivfs_protid_t)
>> diff --git a/pci_arbiter/pci-ops.c b/pci_arbiter/pci-ops.c
>> new file mode 100644
>> index 00000000..f29b58d0
>> --- /dev/null
>> +++ b/pci_arbiter/pci-ops.c
>> @@ -0,0 +1,89 @@
>> +/*
>> +   Copyright (C) 2017 Free Software Foundation, Inc.
>> +
>> +   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/>.
>> +*/
>> +
>> +/* Implementation of PCI operations */
>> +
>> +#include <pci_S.h>
>> +
>> +#include <hurd/fshelp.h>
>> +
>> +#include <pci_arbiter.h>
>> +#include <pci_access.h>
>> +
>> +error_t
>> +S_pci_read (struct trivfs_protid *master, int bus, int dev, int func,
>> +         int reg, char **data, size_t * datalen,
>> +         mach_msg_type_number_t amount)
>> +{
>> +  error_t err;
>> +
>> +  if (!master)
>> +    return EOPNOTSUPP;
>> +
>> +  if (!master->isroot)
>> +    {
>> +      struct stat st;
>> +
>> +      st.st_uid = pci_owner;
>> +      st.st_gid = pci_group;
>> +
>> +      err = fshelp_isowner (&st, master->user);
>> +      if (err)
>> +     return EPERM;
>> +    }
>> +
>> +  if (amount > *datalen)
>> +    amount = *datalen;
>> +
>> +  err = pci_ifc->read (bus, dev, func, reg, *data, amount);
>> +
>> +  if(!err)
>> +    *datalen = amount;
>> +
>> +  return err;
>> +}
>> +
>> +error_t
>> +S_pci_write (struct trivfs_protid * master, int bus, int dev, int func,
>> +          int reg, char *data, size_t datalen,
>> +          mach_msg_type_number_t * amount)
>> +{
>> +  error_t err;
>> +
>> +  if (!master)
>> +    return EOPNOTSUPP;
>> +
>> +  if (!master->isroot)
>> +    {
>> +      struct stat st;
>> +
>> +      st.st_uid = pci_owner;
>> +      st.st_gid = pci_group;
>> +
>> +      err = fshelp_isowner (&st, master->user);
>> +      if (err)
>> +     return EPERM;
>> +    }
>> +
>> +  err = pci_ifc->write (bus, dev, func, reg, data, datalen);
>> +
>> +  *amount = datalen;
>> +
>> +  return err;
>> +}
>
> The server code looks good.
>
>
> Cheers :)
> Justus

OK, I'll add your comments to my todo. Thanks!

-------------------------------------------------
[1] 
http://darnassus.sceen.net/~jlledom/patches/0001-New-module-for-the-Hurd.patch