Hi Joan :) let me be the first to say: Awesome :)
Joan Lledó <joanlluisll...@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. > 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. > 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. > 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
signature.asc
Description: PGP signature
