Re: [PATCH] rumpkernel dependencies

2020-04-11 Thread Samuel Thibault
Hello,

Damien Zammit, le ven. 10 avril 2020 14:37:52 +1000, a ecrit:
> I can't contribute patches upstream to rump currently because netbsd
> are planning to merge buildrump.sh into netbsd src/ tree and the "upstream"
> rump repos will possibly be deprecated/merged into netbsd.

Oh!

> I believe we need to maintain our own rumpkernel debian tree for now until
> they merge everything into src, then we can update rump properly.

Yep, sure.

> Can we keep the netbsd ioccom.h header for now in hurd?

Indeed!

I have applied and fixed your patch, thanks!

Samuel



Re: [PATCH] rumpkernel dependencies

2020-04-09 Thread Damien Zammit
On 4/4/20 9:06 am, Samuel Thibault wrote:
> Well, rump does have it but doesn't install it (as well as dkio.h), but
> it should, shouldn't it?  Otherwise rump_sys_ioctl is basically unusable
> without it. It looks like makekernelheaders could be installing this?
> Probably we don't want all of them? Probably discussion can happen with
> upstream to know what they think about it? (I guess not being able to
> use ioctl is a good argument).

I can't contribute patches upstream to rump currently because netbsd
are planning to merge buildrump.sh into netbsd src/ tree and the "upstream"
rump repos will possibly be deprecated/merged into netbsd.
I believe we need to maintain our own rumpkernel debian tree for now until
they merge everything into src, then we can update rump properly.
I have been working on fixing UBSan errors in rump within netbsd upstream,
they're almost all fixed!

Can we keep the netbsd ioccom.h header for now in hurd?

+ifeq ($(HAVE_LIBRUMP),yes)

Fixed

> Please make it automatically optional as I mentioned: either rump is
> detected and it is used, or it is not detected, and then anything that
> needs it is disabled.

Fixed

>> +++ b/rumpdisk/Makefile
> 
>> +rumpdisk.static: main.o block-rump.o
> 
> I don't think this is needed? SRCS already provides the list of files to
> include.

Fixed

>> +++ b/rumpdisk/block-rump.c
> Make the length a parameter, to avoid the magic assumption that it's
> DISK_NAME_LEN.

Fixed

>> +/* FIXME:
>> + * Call rump_sys_aio_read/write and return MIG_NO_REPLY from
> 
> Do mention only that long-term plan. Also mention rump_sys_pread/pwrite
> plan for multiple thread support. That's what we'll probably use short-
> and middle-term wise.

Fixed

Damien
>From 995085eca1f178d9d2db6de04abb9cb5dea17e9b Mon Sep 17 00:00:00 2001
From: Damien Zammit 
Date: Sun, 29 Mar 2020 22:37:23 +1100
Subject: [PATCH] rumpdisk: Add userspace disk support via librump

---
 Makefile   |   4 +
 configure.ac   |   4 +
 rumpdisk/Makefile  |  33 
 rumpdisk/block-rump.c  | 350 +
 rumpdisk/block-rump.h  |  23 +++
 rumpdisk/ioccom-rump.h |  82 ++
 rumpdisk/main.c|  39 +
 7 files changed, 535 insertions(+)
 create mode 100644 rumpdisk/Makefile
 create mode 100644 rumpdisk/block-rump.c
 create mode 100644 rumpdisk/block-rump.h
 create mode 100644 rumpdisk/ioccom-rump.h
 create mode 100644 rumpdisk/main.c

diff --git a/Makefile b/Makefile
index 6b1e8066..fa80398d 100644
--- a/Makefile
+++ b/Makefile
@@ -49,6 +49,10 @@ prog-subdirs = auth proc exec term \
 	   acpi \
 	   shutdown
 
+ifeq ($(HAVE_LIBRUMP),yes)
+prog-subdirs += rumpdisk
+endif
+
 ifeq ($(HAVE_SUN_RPC),yes)
 prog-subdirs += nfs nfsd
 endif
diff --git a/configure.ac b/configure.ac
index 897a9146..2d7ff5ca 100644
--- a/configure.ac
+++ b/configure.ac
@@ -243,6 +243,10 @@ AS_IF([test "x$with_libz" != xno], [
 ])
 AC_SUBST([HAVE_LIBZ])
 
+AC_CHECK_HEADER([rump/rump.h], [HAVE_LIBRUMP=yes], [HAVE_LIBRUMP=no])
+AC_CHECK_LIB(rump, rump_init, [HAVE_LIBRUMP=yes], [HAVE_LIBRUMP=no])
+AC_SUBST([HAVE_LIBRUMP])
+
 AC_ARG_ENABLE(boot-store-types,
 [  --enable-boot-store-types=TYPES...
 			  list of store types included in statically
diff --git a/rumpdisk/Makefile b/rumpdisk/Makefile
new file mode 100644
index ..496e62d1
--- /dev/null
+++ b/rumpdisk/Makefile
@@ -0,0 +1,33 @@
+#
+#   Copyright (C) 2019 Free Software Foundation, Inc.
+#
+#   This program 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.
+#
+#   This program 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 this program; if not, write to the Free Software
+#   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+RUMPPATH=/usr/lib
+RUMPLIBS=rump rumpuser rumpdev rumpdev_disk rumpdev_pci rumpvfs rumpdev_ahcisata
+RUMPEXTRA=rumpdev_scsipi
+
+dir := rumpdisk
+makemode := server
+
+SRCS = main.c block-rump.c
+LCLHDRS = block-rump.h ioccom-rump.h
+target = rumpdisk
+OBJS = $(SRCS:.c=.o)
+HURDLIBS = machdev ports trivfs shouldbeinlibc iohelp ihash fshelp 
+LDLIBS += -lpthread -lpciaccess -ldl
+LDLIBS += -Wl,--whole-archive $(RUMPLIBS:%=$(RUMPPATH)/lib%_pic.a) -Wl,--no-whole-archive $(RUMPEXTRA:%=$(RUMPPATH)/lib%_pic.a)
+
+include ../Makeconf
diff --git a/rumpdisk/block-rump.c b/rumpdisk/block-rump.c
new file mode 100644
index ..bc5f2d8a
--- /dev/null
+++ b/rumpdisk/block-rump.c
@@ -0,0 +1,350 @@
+/*
+ * Rump block driver support
+ *
+ * Copyright (C) 2019 Free Software Foundation
+ *
+ * This program is free software; you can 

Re: [PATCH] rumpkernel dependencies

2020-04-03 Thread Samuel Thibault
Damien Zammit, le ven. 03 avril 2020 21:48:15 +1100, a ecrit:
> On 3/4/20 6:45 pm, Samuel Thibault wrote:
> > Isn't that ./buildrump.sh/src/sys/sys/ioccom.h ?
> 
> Oh yea, I have added that one instead to our tree (rump doesn't have it).

Well, rump does have it but doesn't install it (as well as dkio.h), but
it should, shouldn't it?  Otherwise rump_sys_ioctl is basically unusable
without it. It looks like makekernelheaders could be installing this?
Probably we don't want all of them? Probably discussion can happen with
upstream to know what they think about it? (I guess not being able to
use ioctl is a good argument).

> +ifeq ($(HAVE_LIBRUMP),1)
> +prog-subdirs += rumpdisk
> +endif

One usually use "yes" for these, not "1", see just below :)

>  AC_SUBST([HAVE_LIBZ])
>  
> +AC_ARG_WITH([librump],
> +  [AS_HELP_STRING([--without-librump], [disable librump])], , 
> [with_librump=yes])
> +
> +AC_DEFUN([LIBRUMP_FAIL], [
> +  AC_MSG_FAILURE([Please install required libraries or use 
> --without-librump.])
> +])
> +AS_IF([test "x$with_librump" != xno], [
> +  AC_CHECK_HEADER([rump/rump.h],
> +[AC_DEFINE(HAVE_RUMP_RUMP_H)],
> +[LIBRUMP_FAIL])
> +  AC_CHECK_LIB(rump, rump_init, [HAVE_LIBRUMP=1], [LIBRUMP_FAIL])
> +])
> +AC_SUBST([HAVE_LIBRUMP])

Please make it automatically optionnal as I mentioned: either rump is
detected and it is used, or it is not detected, and then anything that
needs it is disabled.

> +++ b/rumpdisk/Makefile

> +rumpdisk.static: main.o block-rump.o

I don't think this is needed? SRCS already provides the list of files to
include.

> +++ b/rumpdisk/block-rump.c

> +/* BSD name of whole disk device is /dev/wdXd
> + * but we will receive /dev/wdX as the name */
> +static void
> +translate_name (char *name, char *output)
> +{
> +  snprintf (output, DISK_NAME_LEN, "%sd", name);
> +}

Make the length a parameter, to avoid the magic assumption that it's
DISK_NAME_LEN.

> +/* FIXME:
> + * Call rump_sys_aio_read/write and return MIG_NO_REPLY from

Do mention only that long-term plan. Also mention rump_sys_pread/pwrite
plan for multiple thread support. That's what we'll probably use short-
and middle-term wise.

Samuel



Re: [PATCH] rumpkernel dependencies

2020-04-03 Thread Damien Zammit
Hi, see attached patch.

On 3/4/20 6:45 pm, Samuel Thibault wrote:
> I don't see the reason why for most of them: libpthread.a,
> libpciaccess.a, libdl.a are not _pic.a variants for instance.
> 
> For a start, try to use non-pic variants for libtrivfs,
> libshouldbeinlibc, libports, libiohelp, libihash, libfshelp. Then try
> also for libmachdev. And my guess is that only the rump ones seemingly
> need it, and possibly it just means that the rump package builds them in
> an odd way.

I fixed this, except rump.

> Isn't that ./buildrump.sh/src/sys/sys/ioccom.h ?

Oh yea, I have added that one instead to our tree (rump doesn't have it).

>>> There are a couple mach_print debugging calls left.

Oops, yeah done.

Damien
>From fd76bb26a629dbe42840ff8807047fe531494bc0 Mon Sep 17 00:00:00 2001
From: Damien Zammit 
Date: Sun, 29 Mar 2020 22:37:23 +1100
Subject: [PATCH] rumpdisk: Add userspace disk support via librump

---
 Makefile  |   4 +
 configure.ac  |  14 ++
 rumpdisk/Makefile |  35 +
 rumpdisk/block-rump.c | 341 ++
 rumpdisk/block-rump.h |  23 +++
 rumpdisk/ioc-rump.h   |  32 
 rumpdisk/main.c   |  39 +
 7 files changed, 488 insertions(+)
 create mode 100644 rumpdisk/Makefile
 create mode 100644 rumpdisk/block-rump.c
 create mode 100644 rumpdisk/block-rump.h
 create mode 100644 rumpdisk/ioc-rump.h
 create mode 100644 rumpdisk/main.c

diff --git a/Makefile b/Makefile
index 6b1e8066..caf99921 100644
--- a/Makefile
+++ b/Makefile
@@ -49,6 +49,10 @@ prog-subdirs = auth proc exec term \
 	   acpi \
 	   shutdown
 
+ifeq ($(HAVE_LIBRUMP),1)
+prog-subdirs += rumpdisk
+endif
+
 ifeq ($(HAVE_SUN_RPC),yes)
 prog-subdirs += nfs nfsd
 endif
diff --git a/configure.ac b/configure.ac
index 897a9146..76c10df3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -243,6 +243,20 @@ AS_IF([test "x$with_libz" != xno], [
 ])
 AC_SUBST([HAVE_LIBZ])
 
+AC_ARG_WITH([librump],
+  [AS_HELP_STRING([--without-librump], [disable librump])], , [with_librump=yes])
+
+AC_DEFUN([LIBRUMP_FAIL], [
+  AC_MSG_FAILURE([Please install required libraries or use --without-librump.])
+])
+AS_IF([test "x$with_librump" != xno], [
+  AC_CHECK_HEADER([rump/rump.h],
+[AC_DEFINE(HAVE_RUMP_RUMP_H)],
+[LIBRUMP_FAIL])
+  AC_CHECK_LIB(rump, rump_init, [HAVE_LIBRUMP=1], [LIBRUMP_FAIL])
+])
+AC_SUBST([HAVE_LIBRUMP])
+
 AC_ARG_ENABLE(boot-store-types,
 [  --enable-boot-store-types=TYPES...
 			  list of store types included in statically
diff --git a/rumpdisk/Makefile b/rumpdisk/Makefile
new file mode 100644
index ..ab4c3ef7
--- /dev/null
+++ b/rumpdisk/Makefile
@@ -0,0 +1,35 @@
+#
+#   Copyright (C) 2019 Free Software Foundation, Inc.
+#
+#   This program 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.
+#
+#   This program 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 this program; if not, write to the Free Software
+#   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+RUMPPATH=/usr/lib
+RUMPLIBS=rump rumpuser rumpdev rumpdev_disk rumpdev_pci rumpvfs rumpdev_ahcisata
+RUMPEXTRA=rumpdev_scsipi
+
+dir := rumpdisk
+makemode := server
+
+SRCS = main.c block-rump.c
+LCLHDRS = block-rump.h ioc-rump.h
+target = rumpdisk
+OBJS = $(SRCS:.c=.o)
+HURDLIBS = machdev ports trivfs shouldbeinlibc iohelp ihash fshelp 
+LDLIBS += -lpthread -lpciaccess -ldl
+LDLIBS += -Wl,--whole-archive $(RUMPLIBS:%=$(RUMPPATH)/lib%_pic.a) -Wl,--no-whole-archive $(RUMPEXTRA:%=$(RUMPPATH)/lib%_pic.a)
+
+include ../Makeconf
+
+rumpdisk.static: main.o block-rump.o
diff --git a/rumpdisk/block-rump.c b/rumpdisk/block-rump.c
new file mode 100644
index ..6ab66eeb
--- /dev/null
+++ b/rumpdisk/block-rump.c
@@ -0,0 +1,341 @@
+/*
+ * Rump block driver support
+ *
+ * Copyright (C) 2019 Free Software Foundation
+ *
+ * This program 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.
+ *
+ * This program 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 this program; if not, write to the Free Software
+ * Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include 
+#include 
+#include 
+#include 

Re: [PATCH] rumpkernel dependencies

2020-04-03 Thread Samuel Thibault
Damien Zammit, le ven. 03 avril 2020 12:23:56 +1100, a ecrit:
> On 3/4/20 9:28 am, Samuel Thibault wrote:
> > Concerning the link line, I don't understand why hardcoding everything?
> > 
> > - For a start, are the _pic.a versions really needed? The .a versions
> >   should just work.
> 
> I think the _pic.a versions are required.  I could not get rump to work with 
> .a
> That is why I made rumpkernel package install them.

I don't see the reason why for most of them: libpthread.a,
libpciaccess.a, libdl.a are not _pic.a variants for instance.

For a start, try to use non-pic variants for libtrivfs,
libshouldbeinlibc, libports, libiohelp, libihash, libfshelp. Then try
also for libmachdev. And my guess is that only the rump ones seemingly
need it, and possibly it just means that the rump package builds them in
an odd way.

> > About IOC macros in block-rump.c, is rump not providing a header that
> > defines them, instead of hardcoding them here?
> 
> I can put them in my own header, I could not find them exposed in rump.

Isn't that ./buildrump.sh/src/sys/sys/ioccom.h ?

> > There are a couple mach_print debugging calls left.
> 
> I think they should remain, they only occur in severe error.

Except for the ENODEV error case, which can be due to a common user
typo.

Samuel



Re: [PATCH] rumpkernel dependencies

2020-04-02 Thread Damien Zammit
On 3/4/20 9:28 am, Samuel Thibault wrote:
> Concerning the link line, I don't understand why hardcoding everything?
> 
> - For a start, are the _pic.a versions really needed? The .a versions
>   should just work.

I think the _pic.a versions are required.  I could not get rump to work with .a
That is why I made rumpkernel package install them.

> About IOC macros in block-rump.c, is rump not providing a header that
> defines them, instead of hardcoding them here?

I can put them in my own header, I could not find them exposed in rump.

> Concerning translate_name, instead of malloc+snprintf, you can use
> asprintf.  That being said, since it doesn't actually need to be kept
> allocated, better just make this a function that takes the output
> buffer, which you can allocate on the stack in device_open.

OK I will fix this.

> Take care of indentation, the GNU Coding Style is...

OK I will fix this.

> There are a couple mach_print debugging calls left.

I think they should remain, they only occur in severe error.

> Please add a /* FIXME: make multithreaded */ in device_write/read.  The
> way it is currently shaped is completely synchronous: device_read will
> be stuck inside rump_sys_read() until the data is retrieved from the
> disk.  To fix this, we need to add a machdev_multithread_server function
> in libmachdev, which calls ports_manage_port_operations_multithread
> instead of ports_manage_port_operations_one_thread, and use that here
> instead of machdev_server. That way there will be one thread per
> device_read/write request, not blocking each other.  That will however
> pose a problem with the lseek-then-read/write way currently used in
> device_read/write: two concurrent device_read calls with call lseek
> concurrently before calling read concurrently. You can already fix this
> now (even before moving to ports_manage_port_operations_multithread) by
> calling rump_sys_pread/pwrite instead.

ACK

> That being said, on even longer run, we may probably not want
> to keep one thread per device_read/write request. We'll want to
> call rump_sys_aio_read/write instead and return MIG_NO_REPLY from
> device_read/write, and send the mig reply once the aio request has
> completed. That way, only the aio request will be kept in rumpdisk
> memory instead of a whole thread structure. Again, please add this as a
> FIXME note, but for much later. Our current linux glue disk driver does
> not even do that currently :)

ACK

> On rump_sys_read error, you need to unmap buf.

ACK

Damien



Re: [PATCH] rumpkernel dependencies

2020-04-02 Thread Samuel Thibault
Damien Zammit, le lun. 30 mars 2020 22:22:12 +1100, a ecrit:
> I cleaned up rumpdisk and used your new api.  See patch below.

I'm amazed how very straighforward this implementation looks, rump rocks
:)



Concerning the link line, I don't understand why hardcoding everything?

- For a start, are the _pic.a versions really needed? The .a versions
  should just work.

- Then, you don't need to define a link rule by hand, you can put the
  names of the hurd libraries to be linked in in HURDLIBS, and the -l
  flags for the system libraries to be linked in in LDLIBS. See for
  instance ext2fs/Makefile.

- For the rump+machdev piece with "whole-archive", you can stuff that in
  LDLIBS.

That way, rumpdisk will benefit from all the common make bits.


About IOC macros in block-rump.c, is rump not providing a header that
defines them, instead of hardcoding them here?

Concerning translate_name, instead of malloc+snprintf, you can use
asprintf.  That being said, since it doesn't actually need to be kept
allocated, better just make this a function that takes the output
buffer, which you can allocate on the stack in device_open.

Take care of indentation, the GNU Coding Style is not

if (foo)
{
  bla();
}

but

if (foo)
  {
bla();
  }


There are a couple mach_print debugging calls left.


Please add a /* FIXME: make multithreaded */ in device_write/read.  The
way it is currently shaped is completely synchronous: device_read will
be stuck inside rump_sys_read() until the data is retrieved from the
disk.  To fix this, we need to add a machdev_multithread_server function
in libmachdev, which calls ports_manage_port_operations_multithread
instead of ports_manage_port_operations_one_thread, and use that here
instead of machdev_server. That way there will be one thread per
device_read/write request, not blocking each other.  That will however
pose a problem with the lseek-then-read/write way currently used in
device_read/write: two concurrent device_read calls with call lseek
concurrently before calling read concurrently. You can already fix this
now (even before moving to ports_manage_port_operations_multithread) by
calling rump_sys_pread/pwrite instead.


That being said, on even longer run, we may probably not want
to keep one thread per device_read/write request. We'll want to
call rump_sys_aio_read/write instead and return MIG_NO_REPLY from
device_read/write, and send the mig reply once the aio request has
completed. That way, only the aio request will be kept in rumpdisk
memory instead of a whole thread structure. Again, please add this as a
FIXME note, but for much later. Our current linux glue disk driver does
not even do that currently :)


On rump_sys_read error, you need to unmap buf.

Samuel



Re: [PATCH] rumpkernel dependencies

2020-04-02 Thread Samuel Thibault
Damien Zammit, le mer. 01 avril 2020 16:04:25 +1100, a ecrit:
> Thanks.  See patch for configure.ac/Makefile changes to make rumpdisk 
> optional.

AIUI you made it so that one has to pass --without-librump to disable
rump support?  That is not how it is usually done.  Usually configure.ac
would just detect whether the library is there, if so use it, and
otherwise disable the pieces that need it.

Samuel



Re: [PATCH] rumpkernel dependencies

2020-04-02 Thread Samuel Thibault
Damien Zammit, le mer. 01 avril 2020 16:04:25 +1100, a ecrit:
> For some reason --without-librump still creates the Makefile for rumpdisk,

That's expected with autotools, they always generate files from .in
files.

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-31 Thread Damien Zammit
On 30/3/20 10:38 pm, Samuel Thibault wrote:
> Damien Zammit, le lun. 30 mars 2020 22:22:12 +1100, a ecrit:
>> I don't think block-rump.c needs a separate library, what do you think?
> 
> Indeed, the way you did seems well-contained.

Thanks.  See patch for configure.ac/Makefile changes to make rumpdisk optional.

For some reason --without-librump still creates the Makefile for rumpdisk,
but the test for rump does not occur, ie I still see:

...
config.status: creating rumpdisk/Makefile
...

Damien
>From 7b7907511fe5d5b32ac3fe3a48eb0a031c8effc8 Mon Sep 17 00:00:00 2001
From: Damien Zammit 
Date: Wed, 1 Apr 2020 15:44:22 +1100
Subject: [PATCH] rumpdisk: Make optional based on configure --with-librump

---
 Makefile |  5 -
 configure.ac | 14 ++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 0e23c5b2..fa80398d 100644
--- a/Makefile
+++ b/Makefile
@@ -47,9 +47,12 @@ prog-subdirs = auth proc exec term \
 	   devnode \
 	   eth-multiplexer \
 	   acpi \
-	   rumpdisk \
 	   shutdown
 
+ifeq ($(HAVE_LIBRUMP),yes)
+prog-subdirs += rumpdisk
+endif
+
 ifeq ($(HAVE_SUN_RPC),yes)
 prog-subdirs += nfs nfsd
 endif
diff --git a/configure.ac b/configure.ac
index 897a9146..3b275e26 100644
--- a/configure.ac
+++ b/configure.ac
@@ -243,6 +243,20 @@ AS_IF([test "x$with_libz" != xno], [
 ])
 AC_SUBST([HAVE_LIBZ])
 
+AC_ARG_WITH([librump],
+  [AS_HELP_STRING([--without-librump], [disable librump])], , [with_librump=yes])
+
+AC_DEFUN([LIBRUMP_FAIL], [
+  AC_MSG_FAILURE([Please install required libraries or use --without-librump.])
+])
+AS_IF([test "x$with_librump" != xno], [
+  AC_CHECK_HEADER([rump/rump.h],
+[AC_DEFINE(HAVE_RUMP_RUMP_H)],
+[LIBRUMP_FAIL])
+  AC_CHECK_LIB(rump, rump_init, [HAVE_LIBRUMP=yes], [LIBRUMP_FAIL])
+])
+AC_SUBST([HAVE_LIBRUMP])
+
 AC_ARG_ENABLE(boot-store-types,
 [  --enable-boot-store-types=TYPES...
 			  list of store types included in statically
-- 
2.25.1



Re: [PATCH] rumpkernel dependencies

2020-03-30 Thread Samuel Thibault
Damien Zammit, le lun. 30 mars 2020 22:22:12 +1100, a ecrit:
> I don't think block-rump.c needs a separate library, what do you think?

Indeed, the way you did seems well-contained.

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-30 Thread Damien Zammit
On 30/3/20 9:12 am, Samuel Thibault wrote:
> I cleaned that quite heavily, to have something that actually looks like
> a library :)
> Along the way I renamed various things, basically to add the machdev
> prefix, see machdev.h and machdev-device_emul.h

Fantastic!  I cleaned up rumpdisk and used your new api.  See patch below.
I don't think block-rump.c needs a separate library, what do you think?

Thanks,
Damien
>From 45ac091794bd7b96e7b0cb01852f3bee72c29168 Mon Sep 17 00:00:00 2001
From: Damien Zammit 
Date: Sun, 29 Mar 2020 22:37:23 +1100
Subject: [PATCH] rumpdisk: Add userspace disk support via librump

---
 Makefile  |   1 +
 rumpdisk/Makefile |  36 +
 rumpdisk/block-rump.c | 351 ++
 rumpdisk/block-rump.h |   6 +
 rumpdisk/main.c   |  20 +++
 5 files changed, 414 insertions(+)
 create mode 100644 rumpdisk/Makefile
 create mode 100644 rumpdisk/block-rump.c
 create mode 100644 rumpdisk/block-rump.h
 create mode 100644 rumpdisk/main.c

diff --git a/Makefile b/Makefile
index 6b1e8066..0e23c5b2 100644
--- a/Makefile
+++ b/Makefile
@@ -47,6 +47,7 @@ prog-subdirs = auth proc exec term \
 	   devnode \
 	   eth-multiplexer \
 	   acpi \
+	   rumpdisk \
 	   shutdown
 
 ifeq ($(HAVE_SUN_RPC),yes)
diff --git a/rumpdisk/Makefile b/rumpdisk/Makefile
new file mode 100644
index ..227f6187
--- /dev/null
+++ b/rumpdisk/Makefile
@@ -0,0 +1,36 @@
+#
+#   Copyright (C) 2019 Free Software Foundation, Inc.
+#
+#   This program 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.
+#
+#   This program 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 this program; if not, write to the Free Software
+#   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+RUMPPATH=/usr/lib
+RUMPLIBS=rump rumpuser rumpdev rumpdev_disk rumpdev_pci rumpvfs rumpdev_ahcisata
+RUMPEXTRA=rumpdev_scsipi
+
+dir := rumpdisk
+makemode := server
+
+SRCS = main.c block-rump.c
+LCLHDRS = block-rump.h
+target = rumpdisk
+OBJS = $(SRCS:.c=.o)
+HURDLIBS = machdev ports
+LDLIBS += -lpthread
+LDLIBS += $(RUMPLIBS:%=-l%) $(RUMPEXTRA:%=-l%)
+
+include ../Makeconf
+
+rumpdisk.static: main.o block-rump.o
+	$(CC) -static main.o block-rump.o -Wl,--whole-archive $(RUMPLIBS:%=$(RUMPPATH)/lib%_pic.a) ../libmachdev/libmachdev_pic.a -Wl,--no-whole-archive $(RUMPEXTRA:%=$(RUMPPATH)/lib%_pic.a) ../libtrivfs/libtrivfs_pic.a ../libshouldbeinlibc/libshouldbeinlibc_pic.a ../libports/libports_pic.a ../libiohelp/libiohelp_pic.a ../libihash/libihash_pic.a ../libfshelp/libfshelp_pic.a /usr/lib/i386-gnu/libpthread.a /usr/lib/i386-gnu/libpciaccess.a /usr/lib/i386-gnu/libdl.a -o $@
diff --git a/rumpdisk/block-rump.c b/rumpdisk/block-rump.c
new file mode 100644
index ..424fc4ef
--- /dev/null
+++ b/rumpdisk/block-rump.c
@@ -0,0 +1,351 @@
+/*
+ * Rump block driver support
+ *
+ * Copyright (C) 2019 Free Software Foundation
+ *
+ * This program 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.
+ *
+ * This program 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 this program; if not, write to the Free Software
+ * Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "mach_U.h"
+
+#include 
+#include 
+#include 
+
+#define MACH_INCLUDE
+
+#include "libmachdev/machdev.h"
+#include "device_reply_U.h"
+
+#include 
+#include 
+#include 
+
+/* rump ioctl stuff */
+#define IOCPARM_MASK0x1fff
+#define IOCPARM_SHIFT   16
+#define IOCGROUP_SHIFT  8
+#define _IOC(inout, group, num, len) \
+((inout) | (((len) & IOCPARM_MASK) << IOCPARM_SHIFT) | \
+((group) << IOCGROUP_SHIFT) | (num))
+#define IOC_OUT (unsigned long)0x4000
+#define _IOR(g,n,t) _IOC(IOC_OUT,   (g), (n), sizeof(t))
+#define  DIOCGMEDIASIZE  _IOR('d', 132, off_t)
+#define  DIOCGSECTORSIZE _IOR('d', 133, unsigned int)
+
+#define DISK_NAME_LEN 32
+
+/* One of these is associated with each open instance of a device.  */
+struct block_data
+{
+  struct port_info port;	/* device port */
+  struct machdev_emul_device device; /* generic device 

Re: [PATCH] rumpkernel dependencies

2020-03-29 Thread Samuel Thibault
Damien Zammit, le lun. 30 mars 2020 10:16:29 +1100, a ecrit:
> I don't know if I can do that, I committed an upstream branch,
> then merged it into master and added patches.

Ok.

Ah, there is a upstream/0_20191130 tag, I can build the orig tarball
from that.

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-29 Thread Damien Zammit
On 30/3/20 7:06 am, Samuel Thibault wrote:
> Damien Zammit, le sam. 28 mars 2020 16:50:41 +1100, a ecrit:
>> Then you'll need me to push latest rumpkernel tree so you can build rump libs
> 
> Could you commit the upstream tarballs to the repo as well so I have the
> correct version? You can do it by running this inside the debian repo:
> 
> pristine-tar commit rumpkernel_0~20191130-1.orig.tar.bz2
> 
> (or any other tarball extension)
> 
> and push the pristine-tar branch.
> 
> Samuel
> 

I don't know if I can do that, I committed an upstream branch,
then merged it into master and added patches.

I built the package without a tarball, I just used:

$ git checkout master
$ quilt push -a
$ dpkg-buildpackage -nc -us -uc

Do your instructions still hold?

Damien



Re: [PATCH] rumpkernel dependencies

2020-03-29 Thread Samuel Thibault
Hello,

Damien Zammit, le dim. 29 mars 2020 23:19:28 +1100, a ecrit:
> >> - put the files common to dde-based and rump-based libmachdev to
> >>   libmachdev/.

I cleaned that quite heavily, to have something that actually looks like
a library :)
Along the way I renamed various things, basically to add the machdev
prefix, see machdev.h and machdev-device_emul.h

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-29 Thread Samuel Thibault
Damien Zammit, le sam. 28 mars 2020 16:50:41 +1100, a ecrit:
> Then you'll need me to push latest rumpkernel tree so you can build rump libs

Could you commit the upstream tarballs to the repo as well so I have the
correct version? You can do it by running this inside the debian repo:

pristine-tar commit rumpkernel_0~20191130-1.orig.tar.bz2

(or any other tarball extension)

and push the pristine-tar branch.

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-29 Thread Samuel Thibault
Damien Zammit, le dim. 29 mars 2020 23:19:28 +1100, a ecrit:
> I created a ds_pre_server() hook in libmachdev and implemented it for 
> libmachdevdde,
> and left an empty stub for libmachdevrump. 

I don't see why making it a hook:

+void ds_pre_server(void)
+{
+  /* empty */
+}
+
+int main()
+{
+  register_block ();
+  mach_device_init ();
+  trivfs_init ();
+  cthread_detach (cthread_fork (ds_server, NULL));
+  trivfs_server ();
+  return 0;
+}

could be made into

+void rump_ds_server(void)
+{
+  ds_server();
+}
+
+int main()
+{
+  register_block ();
+  mach_device_init ();
+  trivfs_init ();
+  cthread_detach (cthread_fork (rump_ds_server, NULL));
+  trivfs_server ();
+  return 0;
+}

and similarly for DDE, provide a dde_ds_server, that dde translators can
use instead of ds_server.


For libmachdevrump, we want to enable its build only when librump is
available, that should be tested in configure.ac, just like we do for
libpciaccess and we enable pci-arbiter accordingly.

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-29 Thread Samuel Thibault
Damien Zammit, le dim. 29 mars 2020 23:19:28 +1100, a ecrit:
> On 29/3/20 4:16 pm, Damien Zammit wrote:
> > Hurd TODO for Damien:
> > =
> > 
> > - Fix libstore patch
> Done

Applied, thanks!

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-29 Thread Damien Zammit
On 29/3/20 4:16 pm, Damien Zammit wrote:
> Hurd TODO for Damien:
> =
> 
> - Fix libstore patch
Done

> - Remove useless debugging eg:

Done

> 
> - Remove most of this header:
>>> +++ b/libmachdevrump/dev_hdr.h
>> - put the files common to dde-based and rump-based libmachdev to
>>   libmachdev/.  That includes basically all your files except disk.c,
>>   machdevrump.c and Makefile. The only real discrepancy I see is in
>>   ds_routines.c' call to l4dde26_process_from_ddekit, but we can make
>>   libmachdevdde provide a function that does that call then call
>>   ds_server(). AIUI the call to is_master_device() should indeed be made
>>   like in the incubator?  I don't think we should deallocate it at the
>>   very least, was it really needed in your tests?

I compiled rumpdisk.static with the attached patches and it probed a real disk,
but I still need to test that the disk fully mounts.

I created a ds_pre_server() hook in libmachdev and implemented it for 
libmachdevdde,
and left an empty stub for libmachdevrump. 

>> - move the dde-based files (for network) to a libmachdevdde/

Done (have not tested dde with this)

>> - put your rump-based disk.c to a libmachdevrump/

Done

>> and the rumpdisk daemon will just need to link in both libmachdev and
>> libmachdevrump.

Done
>From ef2122c9c028e0607ca54bc715d97f1ac934019a Mon Sep 17 00:00:00 2001
From: Damien Zammit 
Date: Sun, 29 Mar 2020 18:51:00 +1100
Subject: [PATCH 1/4] libstore: Add ability to pass custom master device with
 format @master:/dev/device

---
 libstore/device.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/libstore/device.c b/libstore/device.c
index 1d8f57fd..b8961a52 100644
--- a/libstore/device.c
+++ b/libstore/device.c
@@ -95,15 +95,22 @@ dopen (const char *name, device_t *device, int *mod_flags)
 {
   device_t dev_master;
   error_t err = ENODEV;
+  char *pos;
+  char *master;
+  char *rest;
 
-  if (name[0] == '/')
+  /* Parse @master:/dev/hello */
+  if ( (name[0] == '@') && (pos = strchr (name, ':')) )
 {
+  master = strndup (name+1, pos-(name+1));
+  rest = strdup (pos+1);
+
   if (*mod_flags & STORE_HARD_READONLY)
 	{
-	  dev_master = file_name_lookup (name, O_READ, 0);
+	  dev_master = file_name_lookup (master, O_READ, 0);
 	  if (dev_master != MACH_PORT_NULL)
 	{
-	  err = device_open (dev_master, D_READ, "disk", device);
+	  err = device_open (dev_master, D_READ, rest, device);
 	  if (err)
 		err = ENODEV;
 
@@ -114,13 +121,13 @@ dopen (const char *name, device_t *device, int *mod_flags)
 	}
   else
 	{
-	  dev_master = file_name_lookup (name, O_READ | O_WRITE, 0);
+	  dev_master = file_name_lookup (master, O_READ | O_WRITE, 0);
 	  if (dev_master != MACH_PORT_NULL)
 	{
-	  err = device_open (dev_master, D_READ | D_WRITE, "disk", device);
+	  err = device_open (dev_master, D_READ | D_WRITE, rest, device);
 	  if (err == ED_READ_ONLY)
 		{
-		  err = device_open (dev_master, D_READ, "disk", device);
+		  err = device_open (dev_master, D_READ, rest, device);
 		  if (! err)
 		*mod_flags |= STORE_HARD_READONLY;
 		  else
-- 
2.25.1

>From 65acd70fcc369ab075594796330f00d4e0439a04 Mon Sep 17 00:00:00 2001
From: Damien Zammit 
Date: Sun, 29 Mar 2020 22:32:44 +1100
Subject: [PATCH 2/4] libmachdev: Add common machdev

---
 Makefile   |   1 +
 libmachdev/Makefile|  34 
 libmachdev/dev_hdr.h   | 133 +++
 libmachdev/device_emul.h   |  65 +++
 libmachdev/ds_routines.c   | 335 +
 libmachdev/ds_routines.h   |  57 +++
 libmachdev/io_req.h| 135 +++
 libmachdev/machdev.h   |  34 
 libmachdev/mig-decls.h |  50 ++
 libmachdev/mig-mutate.h|  36 
 libmachdev/trivfs_server.c | 169 +++
 libmachdev/vm_param.h  |   7 +
 12 files changed, 1056 insertions(+)
 create mode 100644 libmachdev/Makefile
 create mode 100644 libmachdev/dev_hdr.h
 create mode 100644 libmachdev/device_emul.h
 create mode 100644 libmachdev/ds_routines.c
 create mode 100644 libmachdev/ds_routines.h
 create mode 100644 libmachdev/io_req.h
 create mode 100644 libmachdev/machdev.h
 create mode 100644 libmachdev/mig-decls.h
 create mode 100644 libmachdev/mig-mutate.h
 create mode 100644 libmachdev/trivfs_server.c
 create mode 100644 libmachdev/vm_param.h

diff --git a/Makefile b/Makefile
index 09662f87..6b1e8066 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 \
+	  libmachdev \
 
 # Hurd programs
 prog-subdirs = auth proc exec term \
diff --git a/libmachdev/Makefile b/libmachdev/Makefile
new file mode 100644
index ..78f0756f
--- /dev/null
+++ b/libmachdev/Makefile
@@ -0,0 +1,34 @@
+# Copyright (C) 2009 Free Software 

Re: [PATCH] rumpkernel dependencies

2020-03-29 Thread Samuel Thibault
Damien Zammit, le dim. 29 mars 2020 16:01:30 +1100, a ecrit:
> On 29/3/20 11:24 am, Samuel Thibault wrote:
> > Damien Zammit, le sam. 28 mars 2020 16:50:41 +1100, a ecrit:
> >> Apply these first to libpciaccess:
> >> upstreaming/libpciaccess/99-fix-pciconf-calls
> >> upstreaming/libpciaccess/99-region-probe
> >> upstreaming/libpciaccess/99-common-reuse-mapping
> > 
> > Concerning 99-common-reuse-mapping, as mentioned I'd rather avoid the
> > reuse. Please discuss with upstream about the "just-drop-the-whole-loop"
> > solution, and I can probably upload a patched pciaccess package in
> > "unreleased".
> 
> Feel free to merge these except 99-common-reuse-mapping.

Please also submit them upstream of course :)

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-29 Thread Samuel Thibault
Damien Zammit, le dim. 29 mars 2020 15:55:42 +1100, a ecrit:
> I fixed rump instead.  It now unmaps pci devices correctly and I tested
> it without the above patch at all, it works.

Ok, that's better.

> > I don't really know why libpciaccess maintainers thought it should
> > exclude mapping the same base Maybe you need to confront the rump
> > usage case with upstream libpciaccess, so they perhaps just drop that
> > check entirely, and thus allow *several* mappings of the region?
> 
> I will close the conversation upstream, unless you think it's still worth 
> pursuing?

I don't think it's worth pursuing.

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-28 Thread Damien Zammit
Hurd TODO for Damien:
=

- Fix libstore patch

- Remove useless debugging eg:

>> +  mach_print("device open\n");

- Remove most of this header:

>> +++ b/libmachdevrump/dev_hdr.h
 
- Complete the following:

> I see that most of the patch is actually coming from the incubator's
> dde-based libmachdev. I'd rather avoid having two copies of that code in
> different Hurd repos :)
> 
> I'd say what we want to do is:
> 
> - put the files common to dde-based and rump-based libmachdev to
>   libmachdev/.  That includes basically all your files except disk.c,
>   machdevrump.c and Makefile. The only real discrepancy I see is in
>   ds_routines.c' call to l4dde26_process_from_ddekit, but we can make
>   libmachdevdde provide a function that does that call then call
>   ds_server(). AIUI the call to is_master_device() should indeed be made
>   like in the incubator?  I don't think we should deallocate it at the
>   very least, was it really needed in your tests?
> 
> - move the dde-based files (for network) to a libmachdevdde/
> 
> - put your rump-based disk.c to a libmachdevrump/
> 
> and the rumpdisk daemon will just need to link in both libmachdev and
> libmachdevrump.

Thanks Samuel for guidance.

Damien



Re: [PATCH] rumpkernel dependencies

2020-03-28 Thread Damien Zammit
On 29/3/20 11:31 am, Samuel Thibault wrote:
> Damien Zammit, le sam. 28 mars 2020 16:50:41 +1100, a ecrit:
>> I could then push out my latest debian rumpkernel tree.
> 
> I'd say yes, you can go ahead.
> 
> I don't think you need to wait for libpciaccess patches? Sure all
> the pieces are needed for the whole thing to work, but compile-wise,
> libpciaccess fixes can come after building the rumpkernel libs? It's the
> hurd package will glue everything altogether anyway.

I modified rump and it tested ok without the 99-common-reuse-mapping patch, so 
I will push it now, thanks.

Damien



Re: [PATCH] rumpkernel dependencies

2020-03-28 Thread Damien Zammit
On 29/3/20 11:24 am, Samuel Thibault wrote:
> Damien Zammit, le sam. 28 mars 2020 16:50:41 +1100, a ecrit:
>> Apply these first to libpciaccess:
>> upstreaming/libpciaccess/99-fix-pciconf-calls
>> upstreaming/libpciaccess/99-region-probe
>> upstreaming/libpciaccess/99-common-reuse-mapping
> 
> Concerning 99-common-reuse-mapping, as mentioned I'd rather avoid the
> reuse. Please discuss with upstream about the "just-drop-the-whole-loop"
> solution, and I can probably upload a patched pciaccess package in
> "unreleased".

Feel free to merge these except 99-common-reuse-mapping.
(As it is no longer required for rump).

Damien



Re: [PATCH] rumpkernel dependencies

2020-03-28 Thread Damien Zammit
Hi Samuel,

On 29/3/20 11:19 am, Samuel Thibault wrote:
> --- a/src/common_interface.c
> +++ b/src/common_interface.c
> @@ -290,11 +290,13 @@ pci_device_map_range(struct pci_device *dev, pciaddr_t 
> base,
> 
>  /* Make sure that there isn't already a mapping with the same base and
>   * size.
> + * If there is, use it.
>   */
>  for (i = 0; i < devp->num_mappings; i++) {
>  if ((devp->mappings[i].base == base)
>  && (devp->mappings[i].size == size)) {
> -return EINVAL;
> +*addr = devp->mappings[i].memory;
> +return 0;
>  }
>  }
> 
> 
> I don't think we can just return it. Think of a process that uses
> libpciaccess in several components, which happen to use the same
> mapping. One will call pci_device_map_range(), the other will call
> pci_device_map_range(), at some point the first will call
> pci_device_unmap_memory_range() and then oops the second has unexpected
> lost access to the mapping.

I fixed rump instead.  It now unmaps pci devices correctly and I tested
it without the above patch at all, it works.

> I don't really know why libpciaccess maintainers thought it should
> exclude mapping the same base Maybe you need to confront the rump
> usage case with upstream libpciaccess, so they perhaps just drop that
> check entirely, and thus allow *several* mappings of the region?

I will close the conversation upstream, unless you think it's still worth 
pursuing?

Damien



Re: [PATCH] rumpkernel dependencies

2020-03-28 Thread Samuel Thibault
> diff --git a/libmachdevrump/block.c b/libmachdevrump/block.c
> new file mode 100644
> index 0..e4b519bb8
> --- /dev/null
> +++ b/libmachdevrump/block.c
> +static io_return_t
> +device_open (mach_port_t reply_port, mach_msg_type_name_t reply_port_type,
> +  dev_mode_t mode, char *name, device_t *devp,
> +  mach_msg_type_name_t *devicePoly)
> +{
> +  io_return_t err = D_SUCCESS;
> +  struct block_data *bd = NULL;
> +  char *dev_name;
> +  off_t media_size;
> +  uint32_t block_size;
> +
> +  mach_print("device open\n");
> +  dev_name = translate_name (name);

Are these mach_print calls still needed for your debugging?  I'm fine
with committing some of them for now, but perhaps they are now useless
already?

> diff --git a/libmachdevrump/dev_hdr.h b/libmachdevrump/dev_hdr.h
> new file mode 100644
> index 0..79edc43a3
> --- /dev/null
> +++ b/libmachdevrump/dev_hdr.h

Most of this doesn't seem to be used? Only this:

+/* This structure is associated with each open device port.
+ * The port representing the device points to this structure.  */
+struct emul_device
+{
+struct device_emulation_ops *emul_ops;
+void *emul_data;
+};
+
+typedef struct emul_device *emul_device_t;
+
+#define DEVICE_NULL ((device_t) 0)
+
+/*
+ * Generic device header.  May be allocated with the device,
+ * or built when the device is opened.
+ */
+struct mach_device {
+   struct port_info port;
+   struct emul_device  dev;/* the real device structure */
+};
+typedefstruct mach_device *mach_device_t;
+#defineMACH_DEVICE_NULL ((mach_device_t)0)




I see that most of the patch is actually coming from the incubator's
dde-based libmachdev. I'd rather avoid having two copies of that code in
different Hurd repos :)

I'd say what we want to do is:

- put the files common to dde-based and rump-based libmachdev to
  libmachdev/.  That includes basically all your files except disk.c,
  machdevrump.c and Makefile. The only real discrepancy I see is in
  ds_routines.c' call to l4dde26_process_from_ddekit, but we can make
  libmachdevdde provide a function that does that call then call
  ds_server(). AIUI the call to is_master_device() should indeed be made
  like in the incubator?  I don't think we should deallocate it at the
  very least, was it really needed in your tests?

- move the dde-based files (for network) to a libmachdevdde/

- put your rump-based disk.c to a libmachdevrump/

and the rumpdisk daemon will just need to link in both libmachdev and
libmachdevrump.

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-28 Thread Samuel Thibault
Damien Zammit, le sam. 28 mars 2020 16:50:41 +1100, a ecrit:
> I could then push out my latest debian rumpkernel tree.

I'd say yes, you can go ahead.

I don't think you need to wait for libpciaccess patches? Sure all
the pieces are needed for the whole thing to work, but compile-wise,
libpciaccess fixes can come after building the rumpkernel libs? It's the
hurd package will glue everything altogether anyway.

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-28 Thread Samuel Thibault
Damien Zammit, le sam. 28 mars 2020 16:50:41 +1100, a ecrit:
> I'm not sure how to solve this exactly, I'm guessing you don't want 
> rumpkernel to be a build-dep for hurd.

I don't think this is a problem?

I mean, we have cross-deps in various places: we have to build hurd
before building glibc and other libs (e.g. libpciaccess) before building
the hurd with support using these libs. Not being able to build the
rumpdisk in the initial bootstrap steps will not be a problem for the
bootstrap.

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-28 Thread Samuel Thibault
Damien Zammit, le sam. 28 mars 2020 16:50:41 +1100, a ecrit:
> Apply these first to libpciaccess:
> upstreaming/libpciaccess/99-fix-pciconf-calls
> upstreaming/libpciaccess/99-region-probe
> upstreaming/libpciaccess/99-common-reuse-mapping

Concerning 99-common-reuse-mapping, as mentioned I'd rather avoid the
reuse. Please discuss with upstream about the "just-drop-the-whole-loop"
solution, and I can probably upload a patched pciaccess package in
"unreleased".

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-28 Thread Samuel Thibault
diff --git a/src/hurd_pci.c b/src/hurd_pci.c
index 26505f3..ada7af8 100644
--- a/src/hurd_pci.c
+++ b/src/hurd_pci.c
@@ -162,7 +162,7 @@ pciclient_cfg_read(mach_port_t device_port, int reg, char 
*buf,
 
 data = buf;
 nread = *nbytes;
-err = pci_conf_read(device_port, reg, , , *nbytes);
+err = __pci_conf_read(device_port, reg, , , *nbytes);
 if (err)
 return err;
 
@@ -189,7 +189,7 @@ pciclient_cfg_write(mach_port_t device_port, int reg, char 
*buf,
 int err;
 size_t nwrote;
 
-err = pci_conf_write(device_port, reg, buf, *nbytes, );
+err = __pci_conf_write(device_port, reg, buf, *nbytes, );
 
 if (!err)
 *nbytes = nwrote;

IIRC this is because rump has its own pci_conf_read symbol?  This is
unfortunate, but yes, we can use the __ version to make sure to access
the RPC.


+pci_device_hurd_probe((struct pci_device *)d);

I don't know the details, but I trust you do know :)

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-28 Thread Samuel Thibault
--- a/src/common_interface.c
+++ b/src/common_interface.c
@@ -290,11 +290,13 @@ pci_device_map_range(struct pci_device *dev, pciaddr_t 
base,

 /* Make sure that there isn't already a mapping with the same base and
  * size.
+ * If there is, use it.
  */
 for (i = 0; i < devp->num_mappings; i++) {
 if ((devp->mappings[i].base == base)
 && (devp->mappings[i].size == size)) {
-return EINVAL;
+*addr = devp->mappings[i].memory;
+return 0;
 }
 }


I don't think we can just return it. Think of a process that uses
libpciaccess in several components, which happen to use the same
mapping. One will call pci_device_map_range(), the other will call
pci_device_map_range(), at some point the first will call
pci_device_unmap_memory_range() and then oops the second has unexpected
lost access to the mapping.

I don't really know why libpciaccess maintainers thought it should
exclude mapping the same base Maybe you need to confront the rump
usage case with upstream libpciaccess, so they perhaps just drop that
check entirely, and thus allow *several* mappings of the region?

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-28 Thread Samuel Thibault
Damien Zammit, le sam. 28 mars 2020 16:50:41 +1100, a ecrit:
> upstreaming/hurd/pciarbiter-short-rw.patch

Applied, thanks!

Samuel



Re: [PATCH] rumpkernel dependencies

2020-03-28 Thread Samuel Thibault
Hello,

Damien Zammit, le sam. 28 mars 2020 16:50:41 +1100, a ecrit:
> I am sending this primarily because I don't want to lose these patches.

Ok, I already have comments on them :)


+  /* Parse @master:/dev/hello */
+  master = strdup (name);
+  copy = strdup (name);
+  pos = strchr (copy, ':');
+  *pos = '\0';
+  sprintf(master, "%s", [1]);

There is no need to make a copy, strdup the whole string and then use
sprintf. Better do something like:

pos = strchr(name, ':')
master = strndup(name+1, pos-(name+1));

Samuel