[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH-1] A new irq device interface
|
From: |
Junling Ma |
|
Subject: |
[PATCH-1] A new irq device interface |
|
Date: |
Sat, 1 Aug 2020 18:59:58 -0700 |
Hi all,
When I am playing with the current irq device and user interrupt handling, I
see that each device must implement the device_intr_register and
device_intr_ack calls. However, they are only meaningful for the irq device. So
when writing device translators, this seems a little confusing to me. Starting
from this email, I am sending 3 patches that propose a new interface for user
space irq handling:
1. A program requests for irq by opening one of the irq devices irq0 - irq15.
2. The program performs a read (of any size), which blocks until an interrupt
happens on the requested line. The read returns with 0-byte data.
3. The interrupt is acked by writing to the device (of any sized data).
4. Go to step 2 to wait for the next irq.
PATCH 1 is included in this email: it prepares the stage by
1. Change devicer_user_intr to be a linux interrupt handler.
2. Make user_intr_t represent a specific interrupt line, so that notifications
queue on each line.
3. Make struct irqdev store a vector user_intr_t, indexed by interrupt line.
4. The n_unacked counter only increases when successfully delivered an
interrupt.
Any comments?
Best,
Junling Ma
---
device/device_init.c | 1 +
device/ds_routines.c | 6 +-
device/intr.c | 375 +++++++++++++++----------------
device/intr.h | 28 ++-
i386/i386/irq.c | 15 +-
linux/dev/arch/i386/kernel/irq.c | 68 +-----
6 files changed, 203 insertions(+), 290 deletions(-)
diff --git a/device/device_init.c b/device/device_init.c
index 794186ee..ee3e6dab 100644
--- a/device/device_init.c
+++ b/device/device_init.c
@@ -60,6 +60,7 @@ device_service_create(void)
net_io_init();
device_pager_init();
chario_init();
+ irq_init();
(void) kernel_thread(kernel_task, io_done_thread, 0);
(void) kernel_thread(kernel_task, net_thread, 0);
diff --git a/device/ds_routines.c b/device/ds_routines.c
index e94e5ca8..91787950 100644
--- a/device/ds_routines.c
+++ b/device/ds_routines.c
@@ -343,13 +343,9 @@ ds_device_intr_register (device_t dev, int id,
if (! name_equal(mdev->dev_ops->d_name, 3, "irq"))
return D_INVALID_OPERATION;
- user_intr_t *e = insert_intr_entry (&irqtab, id, receive_port);
- if (!e)
- return D_NO_MEMORY;
-
// TODO detect when the port get destroyed because the driver crashes and
// restart, to replace it when the same device driver calls it again.
- err = install_user_intr_handler (&irqtab, id, flags, e);
+ err = install_user_intr_handler (id, receive_port);
if (err == D_SUCCESS)
{
/* If the port is installed successfully, increase its reference by 1.
diff --git a/device/intr.c b/device/intr.c
index fbb9f495..682b12d7 100644
--- a/device/intr.c
+++ b/device/intr.c
@@ -20,219 +20,164 @@
#include <machine/spl.h>
#include <machine/irq.h>
#include <ipc/ipc_space.h>
+#include "io_req.h"
+#include "ds_routines.h"
#ifndef MACH_XEN
-queue_head_t main_intr_queue;
-static boolean_t deliver_intr (int id, ipc_port_t dst_port);
+#define SA_SHIRQ 0x04000000
+#define SA_INTERRUPT 0x20000000
-static user_intr_t *
-search_intr (struct irqdev *dev, ipc_port_t dst_port)
-{
- user_intr_t *e;
- queue_iterate (dev->intr_queue, e, user_intr_t *, chain)
- {
- if (e->dst_port == dst_port)
- return e;
- }
- return NULL;
+struct irqdev irqtab;
+static boolean_t deliver_intr (int id, ipc_port_t dst_port);
+struct pt_regs;
+extern int request_irq (unsigned int irq, void (*handler) (int, void *, struct
pt_regs *),
+ unsigned long flags, const char *device, void *dev_id);
+extern void free_irq (unsigned int irq, void *dev_id);
+
+#define SPLHIGH(critical_section) \
+{ \
+ spl_t s = splhigh(); \
+ critical_section; \
+ splx(s); \
}
-kern_return_t
-irq_acknowledge (ipc_port_t receive_port)
-{
- user_intr_t *e;
- kern_return_t ret = 0;
-
- spl_t s = splhigh ();
- e = search_intr (&irqtab, receive_port);
-
- if (!e)
- printf("didn't find user intr for interrupt !?\n");
- else
- {
- if (!e->n_unacked)
- ret = D_INVALID_OPERATION;
- else
- e->n_unacked--;
- }
- splx (s);
-
- if (ret)
- return ret;
-
- if (irqtab.irqdev_ack)
- (*(irqtab.irqdev_ack)) (&irqtab, e->id);
-
- __enable_irq (irqtab.irq[e->id]);
-
- return D_SUCCESS;
+#define PROTECT(lock, critical_section) \
+{ \
+ simple_lock(&(lock)); \
+ critical_section; \
+ simple_unlock(&(lock)); \
}
-/* This function can only be used in the interrupt handler. */
-static void
-queue_intr (struct irqdev *dev, int id, user_intr_t *e)
+static user_intr_notification_t
+search_notification (user_intr_t *handler, ipc_port_t dst_port)
{
- /* Until userland has handled the IRQ in the driver, we have to keep it
- * disabled. Level-triggered interrupts would keep raising otherwise. */
- __disable_irq (dev->irq[id]);
-
- spl_t s = splhigh ();
- e->n_unacked++;
- e->interrupts++;
- dev->tot_num_intr++;
- splx (s);
-
- thread_wakeup ((event_t) &intr_thread);
+ user_intr_notification_t n = NULL, p;
+ PROTECT(handler->lock,
+ {
+ queue_iterate (&handler->notification_queue, p,
user_intr_notification_t, chain)
+ {
+ if (p->dst_port == dst_port) {
+ n = p;
+ break;
+ }
+ }
+ });
+ return n;
}
-int
-deliver_user_intr (struct irqdev *dev, int id, user_intr_t *e)
+kern_return_t
+irq_acknowledge (ipc_port_t receive_port)
{
- /* The reference of the port was increased
- * when the port was installed.
- * If the reference is 1, it means the port should
- * have been destroyed and I destroy it now. */
- if (e->dst_port
- && e->dst_port->ip_references == 1)
- {
- printf ("irq handler [%d]: release a dead delivery port %p entry %p\n",
id, e->dst_port, e);
- ipc_port_release (e->dst_port);
- e->dst_port = MACH_PORT_NULL;
- thread_wakeup ((event_t) &intr_thread);
- return 0;
- }
- else
- {
- queue_intr (dev, id, e);
- return 1;
- }
+ user_intr_t *e;
+ unsigned id;
+ for (id = 0; id < NINTR; ++id)
+ {
+ e = irqtab.handler[id];
+ if (e && search_notification (e, receive_port))
+ break;
+ }
+ if (id == NINTR)
+ return D_INVALID_OPERATION;
+
+ kern_return_t ret = D_SUCCESS;
+ PROTECT(e->lock,
+ {
+ if (!e->n_unacked)
+ ret = D_INVALID_OPERATION;
+ else {
+ e->n_unacked--;
+ if (e->n_unacked == 0)
+ __enable_irq(id);
+ }
+ });
+ return ret;
}
-/* insert an interrupt entry in the queue.
- * This entry exists in the queue until
- * the corresponding interrupt port is removed.*/
-user_intr_t *
-insert_intr_entry (struct irqdev *dev, int id, ipc_port_t dst_port)
+static void
+deliver_user_intr (int id, void *dev_id, struct pt_regs *regs)
{
- user_intr_t *e, *new, *ret;
- int free = 0;
-
- new = (user_intr_t *) kalloc (sizeof (*new));
- if (new == NULL)
- return NULL;
-
- /* check whether the intr entry has been in the queue. */
- spl_t s = splhigh ();
- e = search_intr (dev, dst_port);
- if (e)
- {
- printf ("the interrupt entry for irq[%d] and port %p has already been
inserted\n", id, dst_port);
- free = 1;
- ret = NULL;
- goto out;
- }
- printf("irq handler [%d]: new delivery port %p entry %p\n", id, dst_port,
new);
- ret = new;
- new->id = id;
- new->dst_port = dst_port;
- new->interrupts = 0;
- new->n_unacked = 0;
-
- queue_enter (dev->intr_queue, new, user_intr_t *, chain);
-out:
- splx (s);
- if (free)
- kfree ((vm_offset_t) new, sizeof (*new));
- return ret;
+ user_intr_t *e;
+ SPLHIGH({
+ e = irqtab.handler[id];
+ if (e && !queue_empty(&e->notification_queue)) {
+ ++e->interrupts;
+ __disable_irq(id);
+ }
+ });
+ if (!e) return;
+ if (queue_empty(&e->notification_queue))
+ {
+ irqtab.handler[id] = NULL;
+ if (e->n_unacked)
+ __enable_irq (id);
+ free_irq(id, &irqtab);
+ e = NULL;
+ }
+ else if (e->interrupts)
+ thread_wakeup ((event_t) &intr_thread);
}
+/* main service thread for firing irqs */
void
intr_thread (void)
{
- user_intr_t *e;
- int id;
- ipc_port_t dst_port;
- queue_init (&main_intr_queue);
-
+ unsigned total_interrupts;
for (;;)
{
- assert_wait ((event_t) &intr_thread, FALSE);
- /* Make sure we wake up from times to times to check for aborted
processes */
+ assert_wait ((event_t) &intr_thread, FALSE);
+ /* Make sure we wake up from times to times to check for aborted
processes */
thread_set_timeout (hz);
- spl_t s = splhigh ();
-
- /* Check for aborted processes */
- queue_iterate (&main_intr_queue, e, user_intr_t *, chain)
- {
- if ((!e->dst_port || e->dst_port->ip_references == 1) && e->n_unacked)
- {
- printf ("irq handler [%d]: release dead delivery %d unacked irqs
port %p entry %p\n", e->id, e->n_unacked, e->dst_port, e);
- /* The reference of the port was increased
- * when the port was installed.
- * If the reference is 1, it means the port should
- * have been destroyed and I clear unacked irqs now, so the Linux
- * handling can trigger, and we will cleanup later after the Linux
- * handler is cleared. */
- /* TODO: rather immediately remove from Linux handler */
- while (e->n_unacked)
- {
- __enable_irq (irqtab.irq[e->id]);
- e->n_unacked--;
- }
- }
- }
-
- /* Now check for interrupts */
- while (irqtab.tot_num_intr)
- {
- int del = 0;
-
- queue_iterate (&main_intr_queue, e, user_intr_t *, chain)
- {
- /* if an entry doesn't have dest port,
- * we should remove it. */
- if (e->dst_port == MACH_PORT_NULL)
- {
- clear_wait (current_thread (), 0, 0);
- del = 1;
- break;
- }
-
- if (e->interrupts)
- {
- clear_wait (current_thread (), 0, 0);
- id = e->id;
- dst_port = e->dst_port;
- e->interrupts--;
- irqtab.tot_num_intr--;
-
- splx (s);
- deliver_intr (id, dst_port);
- s = splhigh ();
- }
- }
-
- /* remove the entry without dest port from the queue and free it. */
- if (del)
- {
- assert (!queue_empty (&main_intr_queue));
- queue_remove (&main_intr_queue, e, user_intr_t *, chain);
- if (e->n_unacked)
- printf("irq handler [%d]: still %d unacked irqs in entry %p\n",
e->id, e->n_unacked, e);
- while (e->n_unacked)
- {
- __enable_irq (irqtab.irq[e->id]);
- e->n_unacked--;
- }
- printf("irq handler [%d]: removed entry %p\n", e->id, e);
- splx (s);
- kfree ((vm_offset_t) e, sizeof (*e));
- s = splhigh ();
- }
- }
- splx (s);
- thread_block (NULL);
+ thread_block(NULL);
+ do
+ {
+ total_interrupts = 0;
+ for (unsigned id = 0; id < NINTR; ++id)
+ {
+ user_intr_t *e =
irqtab.handler[id];
+ if (!e) continue;
+ clear_wait
(current_thread (), 0, 0);
+
user_intr_notification_t next;
+
user_intr_notification_t n;
+ /* go through each
notification to fire or remove dead ones */
+ PROTECT(e->lock,
+ {
+
queue_iterate (&e->notification_queue, n, user_intr_notification_t, chain)
+
{
+
/* is the notification port dead? */
+
if (n->dst_port->ip_references == 1)
+
{
+
/* dead, move it to the dead queue */
+
next = (user_intr_notification_t)queue_next(&n->chain);
+
queue_remove(&e->notification_queue, n,
user_intr_notification_t, chain);
+
ipc_port_release (n->dst_port);
+
kfree((vm_offset_t)n, sizeof(*n));
+
--e->n_unacked;
+
if (e->n_unacked == 0)
+
__enable_irq(id);
+
printf("dead notification port %p released \n",
n->dst_port);
+
if (queue_empty(&e->notification_queue))
+
{
+
printf("irq %d has no registered
notifications. remove\n", id);
+
kfree((vm_offset_t)e, sizeof(*e));
+
irqtab.handler[id] = NULL;
+
break;
+
}
+
n = next;
+
}
+
else if (e->interrupts)
+
{
+
SPLHIGH(e->interrupts--);
+
total_interrupts += e->interrupts;
+
if (deliver_intr(id, n->dst_port))
+
/* n_unacked is increased when firing. Without
firing, there is no ack */
+
e->n_unacked++;
+
}
+
} /* end of queue_iterate */
+ }); /* end of
PROTECT */
+ }
+ }
+ while (total_interrupts);
}
}
@@ -280,4 +225,54 @@ deliver_intr (int id, ipc_port_t dst_port)
return TRUE;
}
+int
+install_user_intr_handler (int id, ipc_port_t dst_port)
+{
+ if (id > NINTR || dst_port == NULL)
+ return D_INVALID_OPERATION;
+ user_intr_t *e = irqtab.handler[id];
+ if (e == NULL) {
+ e = (user_intr_t *) kalloc (sizeof (*e));
+ if (e == NULL)
+ return D_NO_MEMORY;
+ queue_init(&e->notification_queue);
+ e->interrupts = 0;
+ e->n_unacked = 0;
+ simple_lock_init(&e->lock);
+ irqtab.handler[id] = e;
+ /* install the new handler */
+ kern_return_t r = request_irq (id, deliver_user_intr, SA_SHIRQ,
NULL, &irqtab);
+ if (r) {
+ printf("could not register irq handler: %d(%08x)\n", r,
r);
+ irqtab.handler[id] = NULL;
+ kfree((vm_size_t)e, sizeof(*e));
+ return r;
+ }
+ }
+
+ /* check whether the intr entry has been in the queue. */
+ user_intr_notification_t n = search_notification (e, dst_port);
+ if (n)
+ {
+ printf ("the interrupt entry for irq %d and port %p has already been
inserted\n", id, dst_port);
+ return D_ALREADY_OPEN;
+ }
+
+ n = (user_intr_notification_t) kalloc (sizeof (*n));
+ if (n == NULL)
+ return D_NO_MEMORY;
+
+ n->dst_port = dst_port;
+ PROTECT(e->lock, queue_enter (&e->notification_queue, n,
user_intr_notification_t, chain));
+ printf("irq handler [%d]: new delivery port %p\n", id, dst_port);
+ return D_SUCCESS;
+}
+
+void irq_init()
+{
+ irqtab.name = "irq";
+ for (int i = 0; i < NINTR; ++i)
+ irqtab.handler[i] = NULL;
+}
+
#endif /* MACH_XEN */
diff --git a/device/intr.h b/device/intr.h
index cd3e0bce..90bc6f4c 100644
--- a/device/intr.h
+++ b/device/intr.h
@@ -30,29 +30,27 @@
struct irqdev;
#include <machine/irq.h>
-typedef struct {
+/* a struct to hold notifications */
+struct user_intr_notification {
queue_chain_t chain;
- int interrupts; /* Number of interrupts occurred since last run of
intr_thread */
- int n_unacked; /* Number of times irqs were disabled for this */
ipc_port_t dst_port; /* Notification port */
- int id; /* Mapping to machine dependent irq_t array elem */
+};
+typedef struct user_intr_notification * user_intr_notification_t;
+
+typedef struct {
+ queue_head_t notification_queue;
+ unsigned interrupts; /* Number of interrupts occurred since last run of
intr_thread */
+ unsigned n_unacked; /* Number of times irqs were disabled for this */
+ decl_simple_lock_data(, lock); /* a lock to protect the queues */
} user_intr_t;
struct irqdev {
char *name;
- void (*irqdev_ack)(struct irqdev *dev, int id);
-
- queue_head_t *intr_queue;
- int tot_num_intr; /* Total number of unprocessed interrupts */
-
- /* Machine dependent */
- irq_t irq[NINTR];
+ user_intr_t *handler[NINTR]; /* irq handlers for device_open */
};
-extern queue_head_t main_intr_queue;
-extern int install_user_intr_handler (struct irqdev *dev, int id, unsigned
long flags, user_intr_t *e);
-extern int deliver_user_intr (struct irqdev *dev, int id, user_intr_t *e);
-extern user_intr_t *insert_intr_entry (struct irqdev *dev, int id, ipc_port_t
receive_port);
+extern int install_user_intr_handler (int id, ipc_port_t dst_port);
+extern void irq_init();
void intr_thread (void);
kern_return_t irq_acknowledge (ipc_port_t receive_port);
diff --git a/i386/i386/irq.c b/i386/i386/irq.c
index 35681191..c13138a8 100644
--- a/i386/i386/irq.c
+++ b/i386/i386/irq.c
@@ -24,15 +24,7 @@
#include <kern/assert.h>
#include <machine/machspl.h>
-extern queue_head_t main_intr_queue;
-
-static void
-irq_eoi (struct irqdev *dev, int id)
-{
- /* TODO EOI(dev->irq[id]) */
-}
-
-static unsigned int ndisabled_irq[NINTR];
+static unsigned int ndisabled_irq[NINTR] = {0};
void
__disable_irq (irq_t irq_nr)
@@ -60,8 +52,3 @@ __enable_irq (irq_t irq_nr)
splx(s);
}
-struct irqdev irqtab = {
- "irq", irq_eoi, &main_intr_queue, 0,
- {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15},
-};
-
diff --git a/linux/dev/arch/i386/kernel/irq.c b/linux/dev/arch/i386/kernel/irq.c
index 1e911f33..6f433acb 100644
--- a/linux/dev/arch/i386/kernel/irq.c
+++ b/linux/dev/arch/i386/kernel/irq.c
@@ -78,7 +78,6 @@ struct linux_action
void *dev_id;
struct linux_action *next;
unsigned long flags;
- user_intr_t *user_intr;
};
static struct linux_action *irq_action[16] =
@@ -98,7 +97,6 @@ linux_intr (int irq)
{
struct pt_regs regs;
struct linux_action *action = *(irq_action + irq);
- struct linux_action **prev = &irq_action[irq];
unsigned long flags;
kstat.interrupts[irq]++;
@@ -110,21 +108,8 @@ linux_intr (int irq)
while (action)
{
- // TODO I might need to check whether the interrupt belongs to
- // the current device. But I don't do it for now.
- if (action->user_intr)
- {
- if (!deliver_user_intr(&irqtab, irq, action->user_intr))
- {
- *prev = action->next;
- linux_kfree(action);
- action = *prev;
- continue;
- }
- }
- else if (action->handler)
- action->handler (irq, action->dev_id, ®s);
- prev = &action->next;
+ if (action->handler)
+ action->handler (irq, action->dev_id, ®s);
action = action->next;
}
@@ -194,7 +179,6 @@ setup_x86_irq (int irq, struct linux_action *new)
/* Can't share interrupts unless both are same type */
if ((old->flags ^ new->flags) & SA_INTERRUPT)
return (-EBUSY);
-
/* add new interrupt at end of irq queue */
do
{
@@ -219,53 +203,6 @@ setup_x86_irq (int irq, struct linux_action *new)
return 0;
}
-int
-install_user_intr_handler (struct irqdev *dev, int id, unsigned long flags,
- user_intr_t *user_intr)
-{
- struct linux_action *action;
- struct linux_action *old;
- int retval;
-
- unsigned int irq = dev->irq[id];
-
- assert (irq < 16);
-
- /* Test whether the irq handler has been set */
- // TODO I need to protect the array when iterating it.
- old = irq_action[irq];
- while (old)
- {
- if (old->user_intr && old->user_intr->dst_port == user_intr->dst_port)
- {
- printk ("The interrupt handler has already been installed on line
%d", irq);
- return linux_to_mach_error (-EAGAIN);
- }
- old = old->next;
- }
-
- /*
- * Hmm... Should I use `kalloc()' ?
- * By OKUJI Yoshinori.
- */
- action = (struct linux_action *)
- linux_kmalloc (sizeof (struct linux_action), GFP_KERNEL);
- if (action == NULL)
- return linux_to_mach_error (-ENOMEM);
-
- action->handler = NULL;
- action->next = NULL;
- action->dev_id = NULL;
- action->flags = SA_SHIRQ;
- action->user_intr = user_intr;
-
- retval = setup_x86_irq (irq, action);
- if (retval)
- linux_kfree (action);
-
- return linux_to_mach_error (retval);
-}
-
/*
* Attach a handler to an IRQ.
*/
@@ -294,7 +231,6 @@ request_irq (unsigned int irq, void (*handler) (int, void
*, struct pt_regs *),
action->next = NULL;
action->dev_id = dev_id;
action->flags = flags;
- action->user_intr = NULL;
retval = setup_x86_irq (irq, action);
if (retval)
--
2.28.0.rc1
- [PATCH-1] A new irq device interface,
Junling Ma <=
- Re: [PATCH-1] A new irq device interface, Samuel Thibault, 2020/08/02
- Re: [PATCH-1] A new irq device interface, Junling Ma, 2020/08/02
- Re: [PATCH-1] A new irq device interface, Samuel Thibault, 2020/08/02
- Re: [PATCH-1] A new irq device interface, Junling Ma, 2020/08/02
- Re: [PATCH-1] A new irq device interface, Samuel Thibault, 2020/08/02
- Re: [PATCH-1] A new irq device interface, Samuel Thibault, 2020/08/02
- Re: [PATCH-1] A new irq device interface, Junling Ma, 2020/08/02
- Re: [PATCH-1] A new irq device interface, Samuel Thibault, 2020/08/02
- Re: [PATCH-1] A new irq device interface, Junling Ma, 2020/08/02