Re: [PATCH v8 0/2 hurd] Add irqhelp and clean up ddekit

2024-03-08 Thread Samuel Thibault
Damien Zammit, le ven. 08 mars 2024 06:57:47 +, a ecrit:
> I think we need to allow the irq handler to be called one more time,
> because we don't know if there is a pending interrupt until we need
> to handle one.
> Once we enable the irq one more time, it is necessary
> to handle the last interrupt before quitting the handler.

I believe we still don't need to call the application handler. The fact
that the application calls irqhelp_remove_interrupt_handler means that
it's done with the hardware, and doesn't care about getting notified
of more interrupts. Worse, we should really *not* call the handler one
more time after irqhelp_remove_interrupt_handler returns, since the
application could very well want to deinitialize some data and whatnot
that the handler could try to access.

Leaving the application handler pointer recorded somewhere without
actually letting the application control when it's actually finished
being used is asking for trouble :) And I really think the application
doesn't need any more notification after calling
irqhelp_remove_interrupt_handler anyway.

Samuel



Re: [PATCH hurd] rumpdisk: do not open device if block size is 0

2024-03-08 Thread Samuel Thibault
Flávio Cruz, le jeu. 29 févr. 2024 22:01:41 -0500, a ecrit:
> I could be wrong but if you look at this build log 
> [3]https://buildd.debian.org
> /status/fetch.php?pkg=perl&arch=hurd-i386&ver=5.38.2-3&stamp=1705087520&raw=0
> it fails on t/op/stat:
> 
> 
> t/op/stat  # Failed 
> test 36 - ls and -b agreeing on /dev (0 310) at op/stat.t line 322
> #  got "0"
> # expected "310"
> # = before
> # total 1912
> 
> I did run the suite myself and it did seem to get stuck at listing /dev when 
> it
> attempted to open /dev/cd0.

And I have just seen on the buildd that the vim hang is indeed during

buildd   26884 26883 24413 24413 1 So0:00.00 find /dev -type c 
-maxdepth 2

Samuel



Re: [PATCH v2 hurd] MAKEDEV: when creating devices, ensure the underlying files are either block/char devices or directories

2024-03-08 Thread Samuel Thibault
Applied, thanks a lot for fixing the perl testsuite! :)

Flavio Cruz, le mer. 06 mars 2024 00:32:44 -0500, a ecrit:
> The perl test suite has a test where it reads all the block or char devices
> under /dev without following the translators. Then it compares it against a
> list of devices that read the translated nodes stat info.
> 
> The patch changes how the the device files are created initially so that the 
> stat information
> is identical and makes the Hurd environment appear more similar to other 
> operating
> systems.
> ---
>  sutils/MAKEDEV.sh | 64 +--
>  1 file changed, 39 insertions(+), 25 deletions(-)
> 
> diff --git a/sutils/MAKEDEV.sh b/sutils/MAKEDEV.sh
> index 0702663..c3d7d11 100644
> --- a/sutils/MAKEDEV.sh
> +++ b/sutils/MAKEDEV.sh
> @@ -77,10 +77,24 @@ st() {
>local NODE="$1"
>local OWNER="$2"
>local PERM="$3"
> -  shift 3
> +  local NODE_TYPE="$4"
> +  shift 4
>if [ "$KEEP" ] && showtrans "$NODE" > /dev/null 2>&1 ; then
>  return;
>fi
> +  if [ ! -e "$NODE" ]; then
> +case "$NODE_TYPE" in
> +  b|c)
> +cmd mknod "$NODE" "$NODE_TYPE" 0 0
> +;;
> +  d)
> +cmd mkdir "$NODE"
> +;;
> +  *)
> +lose "Unknown node type $NODE_TYPE for $NODE"
> +;;
> +esac
> +  fi
>if cmd settrans $STFLAGS -c "$NODE"; then
>  cmd chown "$OWNER" "$NODE"
>  cmd chmod "$PERM" "$NODE"
> @@ -109,47 +123,47 @@ mkdev() {
>   mkdev console tty random urandom null zero full fd time mem klog shm
>   ;;
>console|com[0-9])
> - st $I root 600 /hurd/term ${DEVDIR}/$I device $I;;
> + st $I root 600 c /hurd/term ${DEVDIR}/$I device $I;;
>vcs)
> -st $I root 600 /hurd/console;;
> +st $I root 600 d /hurd/console;;
>tty[1-9][0-9]|tty[1-9])
> -st $I root 600 /hurd/term ${DEVDIR}/$I hurdio \
> +st $I root 600 c /hurd/term ${DEVDIR}/$I hurdio \
>  ${DEVDIR}/vcs/`echo $I | sed -e s/tty//`/console;;
>lpr[0-9])
> -st $I root 660 /hurd/streamio "$I";;
> +st $I root 660 c /hurd/streamio "$I";;
>random)
> - st $I root 644 /hurd/random --seed-file /var/lib/random-seed;;
> + st $I root 644 c /hurd/random --seed-file /var/lib/random-seed;;
>urandom)
>   # Our /dev/random is both secure and non-blocking.  Create a
>   # link for compatibility with Linux.
>   cmd ln -f -s random $I;;
>null)
> - st $I root 666 /hurd/null;;
> + st $I root 666 c /hurd/null;;
>full)
> - st $I root 666 /hurd/null --full;;
> + st $I root 666 c /hurd/null --full;;
>zero)
> - st $I root 666 /bin/nullauth -- /hurd/storeio -Tzero;;
> + st $I root 666 c /bin/nullauth -- /hurd/storeio -Tzero;;
>tty)
> - st $I root 666 /hurd/magic tty;;
> + st $I root 666 c /hurd/magic tty;;
>fd)
> - st $I root 666 /hurd/magic --directory fd
> + st $I root 666 d /hurd/magic --directory fd
>   cmd ln -f -s fd/0 stdin
>   cmd ln -f -s fd/1 stdout
>   cmd ln -f -s fd/2 stderr
>   ;;
>'time')
> - st $I root 644 /hurd/storeio --no-cache time ;;
> + st $I root 644 c /hurd/storeio --no-cache time ;;
>mem)
> - st $I root 660 /hurd/storeio --no-cache mem ;;
> + st $I root 660 c /hurd/storeio --no-cache mem ;;
>klog)
> -st $I root 660 /hurd/streamio kmsg;;
> +st $I root 660 c /hurd/streamio kmsg;;
># ptys
>[pt]ty[pqrstuvwxyzPQRS]?)
>   # Make one pty, both the master and slave halves.
>   local id="${I#???}"
> - st pty$id root 666 /hurd/term ${DEVDIR}/pty$id \
> + st pty$id root 666 c /hurd/term ${DEVDIR}/pty$id \
> pty-master ${DEVDIR}/tty$id
> - st tty$id root 666 /hurd/term ${DEVDIR}/tty$id \
> + st tty$id root 666 c /hurd/term ${DEVDIR}/tty$id \
> pty-slave ${DEVDIR}/pty$id
>   ;;
>[pt]ty[pqrstuvwxyzPQRS])
> @@ -162,11 +176,11 @@ mkdev() {
>   ;;
>  
>fd*|mt*)
> - st $I root 640 /hurd/storeio $I
> + st $I root 640 b /hurd/storeio $I
>   ;;
>  
>rumpdisk)
> - st $I root 660 /hurd/rumpdisk
> + st $I root 660 c /hurd/rumpdisk
>   cmd ln -f -s rumpdisk disk
>   ;;
>[hrscw]d*)
> @@ -214,18 +228,18 @@ mkdev() {
>   # The device name passed all syntax checks, so finally use it!
>   if [ "$USE_PARTSTORE" ] && [ -z "$rest" ] && [ "$sliceno" ]; then
> local dev=${I%s[0-9]*}
> -   st $I root 640 /hurd/storeio -T typed part:$sliceno:device:$MASTER$dev
> +   st $I root 640 b /hurd/storeio -T typed 
> part:$sliceno:device:$MASTER$dev
>   else
> -   st $I root 640 /hurd/storeio $MASTER$I
> +   st $I root 640 b /hurd/storeio $MASTER$I
>   fi
>   ;;
>  
>netdde)
> - st $I root 660 /hurd/netdde
> + st $I root 660 c /hurd/netdde
>   cmd ln -f -s

[PATCH 1/5] console-client: use constant value instead of magic number

2024-03-08 Thread Etienne Brateau
---
 console-client/pc-kbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/console-client/pc-kbd.c b/console-client/pc-kbd.c
index afd3411e..aa5f1103 100644
--- a/console-client/pc-kbd.c
+++ b/console-client/pc-kbd.c
@@ -713,7 +713,7 @@ read_keycode (void)
   int release = 0;
 
   /* The keypress generated two keycodes.  */
-  if (sc == 0xE0)
+  if (sc == SC_EXTENDED1)
 {
   sc = input_next ();
 
-- 
2.44.0




Re: [PATCH 1/5] console-client: use constant value instead of magic number

2024-03-08 Thread Samuel Thibault
Applied, thanks :)

Etienne Brateau, le sam. 09 mars 2024 01:38:40 +0100, a ecrit:
> ---
>  console-client/pc-kbd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/console-client/pc-kbd.c b/console-client/pc-kbd.c
> index afd3411e..aa5f1103 100644
> --- a/console-client/pc-kbd.c
> +++ b/console-client/pc-kbd.c
> @@ -713,7 +713,7 @@ read_keycode (void)
>int release = 0;
>  
>/* The keypress generated two keycodes.  */
> -  if (sc == 0xE0)
> +  if (sc == SC_EXTENDED1)
>  {
>sc = input_next ();
>  
> -- 
> 2.44.0
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.



[PATCH v9 0/2 hurd] Add irqhelp and clean up ddekit

2024-03-08 Thread Damien Zammit
Hi,

So having discussed this, we allow the irq thread to be called one more time,
but not actually call the user irq handler if the shutdown flag is set.
We acknowledge the interrupt has been handled, however.

I tested this behaviour and it works on netdde.static to ifup and ifdown,
and send a large file over ssh.

Damien






[PATCH v9 2/2 hurd] ddekit: Use libirqhelp for interrupt registration

2024-03-08 Thread Damien Zammit
Use the new irqhelp library in ddekit and clean up.

---
 libddekit/Makefile|   2 +-
 libddekit/interrupt.c | 208 ++
 2 files changed, 28 insertions(+), 182 deletions(-)

diff --git a/libddekit/Makefile b/libddekit/Makefile
index 88a0c8909..c74ec1128 100644
--- a/libddekit/Makefile
+++ b/libddekit/Makefile
@@ -39,7 +39,7 @@ LCLHDRS = $(installhdrs)  \
 
 OBJS = $(sort $(SRCS:.c=.o))
 
-HURDLIBS = ports shouldbeinlibc hurd-slab
+HURDLIBS = ports shouldbeinlibc hurd-slab irqhelp
 LDLIBS += -lpthread
 
 MIGCOMSFLAGS = -prefix dde_
diff --git a/libddekit/interrupt.c b/libddekit/interrupt.c
index 35f95a68c..fa6631f83 100644
--- a/libddekit/interrupt.c
+++ b/libddekit/interrupt.c
@@ -4,8 +4,6 @@
  * \author  Christian Helmuth 
  * \date2007-01-22
  *
- * FIXME could intloop_param freed after startup?
- * FIXME use consume flag to indicate IRQ was handled
  */
 
 #include 
@@ -13,153 +11,25 @@
 #include 
 #include 
 
-#include 
-#include 
+#include "libirqhelp/irqhelp.h"
 
-#include "ddekit/interrupt.h"
 #include "ddekit/semaphore.h"
+#include "ddekit/thread.h"
+#include "ddekit/interrupt.h"
 #include "ddekit/printf.h"
-#include "ddekit/memory.h"
-#include "ddekit/condvar.h"
-
-#define DEBUG_INTERRUPTS  0
 
 #define MAX_INTERRUPTS   32
 
-#define BLOCK_IRQ 0
-
-/*
- * Internal type for interrupt loop parameters
- */
-struct intloop_params
-{
-   unsigned  irq;   /* irq number */
-   int   shared;/* irq sharing supported? */
-   void(*thread_init)(void *);  /* thread initialization */
-   void(*handler)(void *);  /* IRQ handler function */
-   void *priv;  /* private token */ 
-   mach_port_t   irqport;   /* delivery port for notifications */
-   ddekit_sem_t *started;
-
-   int   start_err;
-};
-
 static struct
 {
int  handle_irq; /* nested irq disable count */
ddekit_lock_tirqlock;
-   struct ddekit_condvar   *cond;
-   ddekit_thread_t  *irq_thread; /* thread ID for detaching from IRQ later 
on */
-   boolean_tthread_exit;
thread_t mach_thread;
+   ddekit_thread_t  *irq_thread;
+   ddekit_sem_t *started;
+   void *irqhelp;   /* irqhelp instance for detaching from IRQ 
*/
 } ddekit_irq_ctrl[MAX_INTERRUPTS];
 
-static mach_port_t master_host;
-static mach_port_t master_device;
-static device_t irq_dev;
-
-/**
- * Interrupt service loop
- *
- */
-static void intloop(void *arg)
-{
-   kern_return_t ret;
-   struct intloop_params *params = arg;
-   mach_port_t delivery_port;
-   mach_port_t pset, psetcntl;
-   int my_index;
-
-   ret = mach_port_allocate (mach_task_self (), MACH_PORT_RIGHT_RECEIVE,
- &delivery_port);
-   if (ret)
- error (2, ret, "mach_port_allocate");
-
-   my_index = params->irq;
-   params->irqport = delivery_port;
-   ddekit_irq_ctrl[my_index].mach_thread = mach_thread_self ();
-   ret = thread_get_assignment (mach_thread_self (), &pset);
-   if (ret)
-   error (0, ret, "thread_get_assignment");
-   ret = host_processor_set_priv (master_host, pset, &psetcntl);
-   if (ret)
-   error (0, ret, "host_processor_set_priv");
-   thread_max_priority (mach_thread_self (), psetcntl, 0);
-   ret = thread_priority (mach_thread_self (), DDEKIT_IRQ_PRIO, 0);
-   if (ret)
-   error (0, ret, "thread_priority");
-
-   // TODO the flags for shared irq should be indicated by params->shared.
-   // Flags needs to be 0 for new irq interface for now.
-   // Otherwise, the interrupt handler cannot be installed in the kernel.
-   ret = device_intr_register (irq_dev, my_index,
- 0, delivery_port,
- MACH_MSG_TYPE_MAKE_SEND);
-   ddekit_printf ("device_intr_register returns %d\n", ret);
-   if (ret) {
-   /* inform thread creator of error */
-   /* XXX does omega0 error code have any meaning to DDEKit users? 
*/
-   params->start_err = ret;
-   ddekit_sem_up(params->started);
-   ddekit_printf ("cannot install irq %d\n", my_index);
-   return;
-   }
-
-#if 0
-   /* 
-* Setup an exit fn. This will make sure that we clean up everything,
-* before shutting down an IRQ thread.
-*/
-   if (l4thread_on_exit(&exit_fn, (void *)my_index) < 0)
-   ddekit_panic("Could not set exit handler for IRQ fn.");
-#endif
-
-   /* after successful initialization call thread_init() before doing 
anything
-* else here */
-   if (params->thread_init) params->thread_init(params->priv);
-
-   /* save handle + inform thread creator of success */
-   params->start_err = 0;
-   ddekit_sem_up(params->started);
-
-   int irq_

[PATCH v9 1/2 hurd] libirqhelp: Add library

2024-03-08 Thread Damien Zammit
Add a helper library for attaching interrupt handlers in userspace.

---
 Makefile |   1 +
 libirqhelp/Makefile  |  28 
 libirqhelp/irqhelp.c | 360 +++
 libirqhelp/irqhelp.h |  49 ++
 4 files changed, 438 insertions(+)
 create mode 100644 libirqhelp/Makefile
 create mode 100644 libirqhelp/irqhelp.c
 create mode 100644 libirqhelp/irqhelp.h

diff --git a/Makefile b/Makefile
index 874349c06..4d8482219 100644
--- a/Makefile
+++ b/Makefile
@@ -32,6 +32,7 @@ lib-subdirs = libshouldbeinlibc libihash libiohelp libports \
  libhurd-slab \
  libbpf \
  libmachdev \
+ libirqhelp \
 
 # Hurd programs
 prog-subdirs = auth proc exec term \
diff --git a/libirqhelp/Makefile b/libirqhelp/Makefile
new file mode 100644
index 0..c32632abe
--- /dev/null
+++ b/libirqhelp/Makefile
@@ -0,0 +1,28 @@
+# Copyright (C) 2022 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.
+
+dir := libirqhelp
+makemode := library
+
+SRCS = irqhelp.c acpiUser.c
+
+OBJS = $(SRCS:.c=.o)
+HURDLIBS =
+LDLIBS += -lpthread
+libname = libirqhelp
+installhdrs = irqhelp.h
+
+include ../Makeconf
diff --git a/libirqhelp/irqhelp.c b/libirqhelp/irqhelp.c
new file mode 100644
index 0..2a9f6c5e3
--- /dev/null
+++ b/libirqhelp/irqhelp.c
@@ -0,0 +1,360 @@
+/* Library providing helper functions for userspace irq handling.
+   Copyright (C) 2022 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. */
+
+#include "irqhelp.h"
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "acpi_U.h"
+#include 
+#include 
+
+#define IRQ_THREAD_PRIORITY2
+#define log_error(fmt...)  fprintf(stderr, ## fmt)
+
+struct irq {
+  void (*handler)(void *);
+  void *context;
+  int gsi;
+  mach_port_t port;
+  bool enabled;
+  bool shutdown;
+  pthread_mutex_t irqlock;
+  pthread_cond_t irqcond;
+};
+
+static mach_port_t irqdev = MACH_PORT_NULL;
+static mach_port_t acpidev = MACH_PORT_NULL;
+
+static error_t
+get_acpi(void)
+{
+  error_t err = 0;
+  mach_port_t tryacpi, device_master;
+
+  err = get_privileged_ports (0, &device_master);
+  if (!err)
+{
+  err = device_open (device_master, D_READ, "acpi", &tryacpi);
+  mach_port_deallocate (mach_task_self (), device_master);
+  if (!err)
+   {
+ acpidev = tryacpi;
+ return 0;
+   }
+}
+
+  tryacpi = file_name_lookup (_SERVERS_ACPI, O_RDONLY, 0);
+  if (tryacpi == MACH_PORT_NULL)
+return ENODEV;
+
+  acpidev = tryacpi;
+  return 0;
+}
+
+static error_t
+get_irq(void)
+{
+  error_t err = 0;
+  mach_port_t tryirq, device_master;
+
+  err = get_privileged_ports (0, &device_master);
+  if (err)
+return err;
+
+  err = device_open (device_master, D_READ|D_WRITE, "irq", &tryirq);
+  mach_port_deallocate (mach_task_self (), device_master);
+  if (err)
+return err;
+
+  irqdev = tryirq;
+  return err;
+}
+
+static void
+toggle_irq(struct irq *irq, bool on)
+{
+  pthread_mutex_lock (&irq->irqlock);
+  irq->enabled = on;
+  pthread_mutex_unlock (&irq->irqlock);
+
+  if (on)
+pthread_cond_signal (&irq->irqcond);
+}
+
+error_t
+irqhelp_init(void)
+{
+  static bool inited = false;
+  error_t err;
+
+  if (inited)
+{
+  log_error("already inited\n");
+  return 0;
+}
+
+  err = get_irq();
+  if (err)
+{
+  log_error("cannot grab irq device\n");
+  return err;
+}
+
+  err = get_acpi();
+  if (err)
+{
+  log_error("cannot grab acpi device\n");
+  return err;
+}
+
+  inited = true;
+  return 0;
+}
+
+void
+irqhelp