Hi,
Forwarding this to the list so we don't lose your patches.
Thanks,
Damien
-------- Forwarded Message --------
Subject: Re: [RFC] IRQ deduction with subordinate PCI bus
Date: Thu, 23 Oct 2025 06:29:21 +0100
From: Michael Kelly <[email protected]>
Reply-To: Michael Kelly <[email protected]>
To: Damien Zammit <[email protected]>
Hi Damien,
I've attached the proposal that I mailed to the list as a patch. This
has the same alterations but is based on what I believe is the latest
acpi-init.c. The patch should be applied after all the patches are
applied in the debian/patches/series.
I've no problem at all if you want to make any alterations to this code
or even use it as a model for your own implementation. As mentioned in
the mailing-list, I have only tested this code in my 2 use cases.
I've also attached another patch that I made within the hurd sources. It
adds an API to libirqhelp to lookup the GSI from the PCI parameters
without installing an interrupt handler. That was necessary as libddekit
installs the handler elsewhere to the IRQ lookup. The same patch also
shows the place where the GSI conversion is called from in libddekit.
The final patch I'm less confident about although it seems to work for
my network adapter. I've only included it to illustrate how some drivers
need some extra work. The addition of a vendor/id mapping for the driver
is probably OK but the change to use pci_enable_device rather than
pci_enable_device_mem is a 'hack'. Hurd implements an architecture
specific version of the Linux function pci_enable_device (in
libdde-linux26/lib/src/arch/l4/pci.c) to interface into libddekit. I'd
guess that one or more additional functions might need to be added there
to make all drivers work without any changes to the drivers themselves.
Some of the drivers might be OK and I can't remember for sure but I
think the e1000 driver worked with APIC.
All the best,
Mike.
On 23/10/2025 03:31, Damien Zammit wrote:
> Thanks for this.
>
> What change did you make to netdde to make it work with ioapic? We should add
> that too (?)
>
> I might reformat your latest patch for libacpica and send it in again so we
> can upstream it - unless you can use quilt and fix it yourself.
>
> Damien
>
> Sent from Proton Mail Android
>
>
> -------- Original Message --------
> On 18/10/25 7:22 am, Michael Kelly <[email protected]> wrote:
>
>> Hi All,
>>
>> I spent some time attempting to get the network adapter working in my
>> test hardware machine (advent). It is an Atheros device that is
>> supported by netdde. It worked successfully when using PIC but not when
>> using IOAPIC which implied that the interrupt detection was at fault. I
>> found 2 separate but related issues:
>>
>> 1) The ACPI tables within advent have additional _PRT maps for the
>> subordinate PCI bridges which were not being examined by the acpi
>> server. The _PRT table for the subordinate bridge should be examined
>> first before moving onto the root PCI bridge _PRT table if necessary. It
>> is quite a convoluted process to discover these tables but in essence an
>> ACPI walk of all devices needs to be made and each device tested to see
>> if it is a root bridge or a PCI-PCI bridge. I decided to just make a
>> single walk at acpi startup and cache the relevant information for later
>> use within irq lookup.
>>
>> 2) In the event that there are no additional _PRT maps for subordinate
>> buses (as is the case on my Qemu virtual machines) the root bridge _PRT
>> map still applies. Additional translation of the INTx pins needs however
>> to occur first.
>>
>> The general rule appears to be that interrupt pins are translated from
>> the down stream to the up stream sides of each bridge. So downstream dev
>> 0 INTA maps to upstream dev 0 INTA. Downstream dev 1 INTA maps to
>> upstream dev1 INTB. Downstream dev 2 INTA maps to upstream dev 2 INTC
>> and so on. In summary for a downstream 'int-pin', the upstream
>> equivalent is ((int-pin + dev) %4). This translation applies across each
>> intermediate PCI-PCI bridge until the root bridge is reached. Note that
>> this means that device 0 will always match the same INTx pin wherever it
>> is on the bus hierarchy.
>>
>> Implementing the above I was able to correctly determine the IOAPIC GSI
>> required for advent (using pic or IOAPIC) and also Qemu on both
>> interrupt controller schemes.
>>
>> It seems on my advent machine that if the IOAPIC is in use then the
>> legacy IRQ numbers don't work at all (that was the case when I was
>> previously finding the fix for the AHCI controller on this machine).
>> Netdde doesn't actually perform a GSI translation.
>> libdde-linux26/...../arch/l4/pci.c has replaced the Linux function
>> pci_irq_enable() and calls libddekit::ddekit_pci_irq_enable() to offer
>> an opportunity to replace the legacy IRQ with the GSI. Doing so allowed
>> advent's network adapter to work with IOAPIC.
>>
>> I'm aware that the Netdde is not the future for Hurd network drivers.
>> It's also I think the case that using IOAPIC with the Netdde drivers is
>> a bit hit and miss (mine didn't work with IOAPIC without a minor change
>> to the driver itself). I think it's true though that rumpnet still needs
>> to make the same GSI translation and I think that the same issues
>> described here will apply.
>>
>> I did look at making use of the alc NetBSD driver but that I believe
>> requires some work for rumpkernel to produce an alc driver module to
>> link with rumpnet. Does anyone have a rough idea of how much work that
>> would be? I'm willing to have a go at this but it would obviously be
>> faster if someone can steer me in the right direction.
>>
>> The bulk of my changes apply to libacpica/acpi_init.c. There's little
>> value in including the other minor changes which might not be
>> appropriate for merging at all anyway. I've attached the entire
>> acpi_init.c file rather than a patch as it is simpler to see what I'm
>> describing. There's an element of roughness to the my changes at the
>> moment, in particular use of some unnamed numeric constants for PCI
>> value limits and so on. It also isn't based on the absolute latest
>> source (with the minor revision to using irq_status.h). It's very
>> premature to present this as an actual patch. Anyway the main
>> alterations are:
>>
>> 1) acpi_load_pci_bridges()
>>
>> This does the ACPI walk and populates a list of bridge mappings. Each
>> mapping has the up and down stream bus numbers and the ACPI handle that
>> would be used to load the _PRT table. It doesn't check if there is such
>> a table; that occurs during the IRQ lookup.
>>
>> 2) acpi_get_irq_number()
>>
>> This bulk of this was factored out into a new acpi_get_irq_from_bus()
>> (possibly a bad function name). Now lookup of the IRQ number tries each
>> bridge specific _PRT (if any) in turn traversing the hierarchy to the
>> root bridge. Finally it tries the root bridge _PRT having performed the
>> necessary INTx translation(s).
>>
>> 3) Note that I had to remove the translation of legacy IRQs by not
>> adding the PCI_IRQ_START. advent uses legacy IRQ5 for the network
>> adapter and without removing this I ended up with the IRQ 21 when usin
>> PIC and not IOAPIC. I didn't understand what this translation was for so
>> what I'm doing here might be very wrong.
>>
>> I've tried this concept on all combinations of PIC/IOAPIC, 32 bit and 64
>> bit, advent/Qemu and they all work with 1 exception. 64 bit on advent
>> with PIC (not IOAPIC) does not work. I've not studied this yet but it
>> perhaps relates to point 3) above as I see intnull(2) in the log.
>>
>> I think that about summarises the main points and I'd be very interested
>> in feedback: good or bad.
>>
>> Regards,
>>
>> Mike.
>>
>>
--- linux/drivers/net/atl1c/atl1c_main.c.orig 2025-10-16 15:39:31.000000000 +0100
+++ linux/drivers/net/atl1c/atl1c_main.c 2025-10-16 15:42:08.000000000 +0100
@@ -26,6 +26,7 @@
char atl1c_driver_version[] = ATL1C_DRV_VERSION;
#define PCI_DEVICE_ID_ATTANSIC_L2C 0x1062
#define PCI_DEVICE_ID_ATTANSIC_L1C 0x1063
+#define PCI_DEVICE_ID_ATTANSIC_L1C_MK 0x1083
/*
* atl1c_pci_tbl - PCI Device ID Table
*
@@ -37,6 +38,7 @@
*/
static struct pci_device_id atl1c_pci_tbl[] = {
{PCI_DEVICE(PCI_VENDOR_ID_ATTANSIC, PCI_DEVICE_ID_ATTANSIC_L1C)},
+ {PCI_DEVICE(PCI_VENDOR_ID_ATTANSIC, PCI_DEVICE_ID_ATTANSIC_L1C_MK)},
{PCI_DEVICE(PCI_VENDOR_ID_ATTANSIC, PCI_DEVICE_ID_ATTANSIC_L2C)},
/* required last entry */
{ 0 }
@@ -595,6 +597,7 @@
break;
case PCI_DEVICE_ID_ATTANSIC_L1C:
+ case PCI_DEVICE_ID_ATTANSIC_L1C_MK:
hw->nic_type = athr_l1c;
break;
@@ -2487,7 +2490,7 @@
int err = 0;
/* enable device (incl. PCI PM wakeup and hotplug setup) */
- err = pci_enable_device_mem(pdev);
+ err = pci_enable_device(pdev);
if (err) {
dev_err(&pdev->dev, "cannot enable PCI device\n");
return err;
diff -ur hurd.orig/libddekit/pci.c hurd-0.9.git20250801/libddekit/pci.c
--- hurd.orig/libddekit/pci.c 2025-10-23 05:36:08.000000000 +0100
+++ hurd-0.9.git20250801/libddekit/pci.c 2025-10-23 05:38:19.000000000 +0100
@@ -399,7 +399,21 @@
//TODO l4io_pci_set_master(dev->l4dev.handle);
}
+#include "libirqhelp/irqhelp.h"
+#include <hurd.h>
+
int ddekit_pci_irq_enable(int bus, int slot, int func, int pin, int *irq)
{
- return 0;
+ int gsi = -1;
+
+ if (irqhelp_gsi_from_acpi(bus, slot, func, &gsi))
+ {
+ ddekit_printf("Cannot find gsi for: %d:%d:%d\n", bus, slot, func);
+ }
+ else if (gsi >= 0)
+ {
+ *irq = gsi;
+ }
+
+ return 0;
}
diff -ur hurd.orig/libirqhelp/irqhelp.c hurd-0.9.git20250801/libirqhelp/irqhelp.c
--- hurd.orig/libirqhelp/irqhelp.c 2025-10-23 05:36:53.000000000 +0100
+++ hurd-0.9.git20250801/libirqhelp/irqhelp.c 2025-10-23 05:37:19.000000000 +0100
@@ -355,6 +355,18 @@
}
error_t
+irqhelp_gsi_from_acpi(int bus, int dev, int fun, int* gsi)
+{
+ if (acpi_missing)
+ return EIO;
+
+ if ((bus < 0) || (dev < 0) || (fun < 0))
+ return EINVAL;
+
+ return acpi_get_pci_irq (acpidev, bus, dev, fun, gsi);
+}
+
+error_t
irqhelp_remove_interrupt_handler(struct irq *irq)
{
if (!irq)
diff -ur hurd.orig/libirqhelp/irqhelp.h hurd-0.9.git20250801/libirqhelp/irqhelp.h
--- hurd.orig/libirqhelp/irqhelp.h 2025-10-23 05:37:01.000000000 +0100
+++ hurd-0.9.git20250801/libirqhelp/irqhelp.h 2025-10-23 05:37:19.000000000 +0100
@@ -34,6 +34,11 @@
Returns a pointer to be used with other functions. */
struct irq * irqhelp_install_interrupt_handler(int gsi, int bus, int dev, int fun,
void (*handler)(void *), void *context);
+
+/* Lookup the required GSI for the device via ACPI */
+error_t
+irqhelp_gsi_from_acpi(int bus, int dev, int fun, int* gsi);
+
/* Install as pthread */
void * irqhelp_server_loop(void *arg);
--- acpi_init.c.orig 2025-10-18 12:40:48.000000000 +0100
+++ acpi_init.c 2025-10-18 13:13:03.000000000 +0100
@@ -31,11 +31,14 @@
#define PCI_CFG1_END 0xcff
#define LEGACY_ISA_IRQS 8
-#define PCI_IRQ_START 16
#define PCI_INTERRUPT_PIN 0x3d
#define BAD_PIC_MODE(m) ((m) < ACPI_PICMODE_PIC || (m) > ACPI_PICMODE_SAPIC)
+#define PCI_HDR_TYPE_REG 0x0e
+#define PCI_HDR_TYPE_PPB 1
+#define PCI_PPB_SECBUS_REG 0x19
+
extern acpi_status acpi_hw_legacy_sleep(u8 sleep_state);
// Lets keep the ACPI tables in this module
@@ -45,6 +48,16 @@
static int numdevs = -1;
static int pic_mode = -1;
+typedef struct acpi_pci_bridge
+{
+ acpi_handle handle;
+ uint8_t up_bus;
+ uint8_t down_bus;
+ struct acpi_pci_bridge* next;
+} acpi_pci_bridge_t;
+
+static acpi_pci_bridge_t* acpi_pci_bridges;
+
struct slots {
uint8_t bus;
uint16_t dev;
@@ -108,6 +121,17 @@
return NULL;
}
+static void
+acpi_unload_pci_bridges(void)
+{
+ while (acpi_pci_bridges != NULL)
+ {
+ acpi_pci_bridge_t* bridge = acpi_pci_bridges;
+ acpi_pci_bridges = bridge->next;
+ free(bridge);
+ }
+}
+
void
acpi_ds_dump_method_stack(acpi_status status, ...)
// struct acpi_walk_state *walk_state, union acpi_parse_object *op)
@@ -328,6 +352,7 @@
acpi_os_terminate(void)
{
free(pci_devices);
+ acpi_unload_pci_bridges();
acpi_os_printf("Bye!\n");
return AE_OK;
}
@@ -505,29 +530,22 @@
: (int)pci_pin);
}
-int
-acpi_get_irq_number(uint16_t bus, uint16_t dev, uint16_t func)
+static int
+acpi_get_irq_from_bus(acpi_handle handle,
+ uint16_t dev, uint16_t func, int pci_pin)
{
struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
struct acpi_buffer srs_buffer = { ACPI_ALLOCATE_BUFFER, NULL };
struct acpi_pci_routing_table *entry;
struct acpi_resource *res;
- uint8_t *buf;
int srs_count;
- int pci_pin;
- acpi_handle handle = ACPI_ROOT_OBJECT;
- acpi_handle parent_handle = NULL;
acpi_handle prt = NULL;
acpi_handle lnk = NULL;
acpi_status err = AE_OK;
u16 prt_dev, prt_func;
- err = acpi_get_handle(handle, "\\_SB.PCI0", &parent_handle);
- if (ACPI_FAILURE(err))
- return -EIO;
-
- err = acpi_get_handle(parent_handle, METHOD_NAME__PRT, &prt);
+ err = acpi_get_handle(handle, METHOD_NAME__PRT, &prt);
if (ACPI_FAILURE(err))
return -EIO;
@@ -535,7 +553,7 @@
if (ACPI_FAILURE(err))
return -EIO;
- err = acpi_get_irq_routing_table(parent_handle, &buffer);
+ err = acpi_get_irq_routing_table(handle, &buffer);
if (ACPI_FAILURE(err))
return -EIO;
@@ -543,11 +561,6 @@
if (BAD_PIC_MODE(pic_mode))
return -EIO;
- pci_pin = acpi_pci_get_device_pin(bus, dev, func);
- /* Fail if the device pin used is unknown (0) */
- if (pci_pin <= 0)
- return -EIO;
-
entry = ACPI_CAST_PTR(struct acpi_pci_routing_table, ACPI_CAST_PTR(u8, buffer.pointer));
while (entry && (entry->length > 0))
{
@@ -616,7 +629,6 @@
acpi_os_printf("Error setting _SRS\n");
return -EIO;
}
- irq += (irq > LEGACY_ISA_IRQS) ? 0 : PCI_IRQ_START;
acpi_os_printf("Final irq %d\n", irq);
return irq;
}
@@ -678,7 +690,6 @@
return -EIO;
}
free(res);
- irq += (irq > LEGACY_ISA_IRQS) ? 0 : PCI_IRQ_START;
acpi_os_printf("Final irq %d\n", irq);
return irq;
}
@@ -705,6 +716,55 @@
return -EIO;
}
+static acpi_pci_bridge_t*
+acpi_find_pci_bridge(int bus)
+{
+ if (bus == 0)
+ return NULL;
+
+ acpi_pci_bridge_t* bridge = acpi_pci_bridges;
+
+ while (bridge != NULL)
+ {
+ if (bridge->down_bus == bus)
+ return bridge;
+
+ bridge = bridge->next;
+ }
+
+ return NULL;
+}
+
+int
+acpi_get_irq_number(uint16_t bus, uint16_t dev, uint16_t func)
+{
+ int pci_pin = acpi_pci_get_device_pin(bus, dev, func);
+ /* Fail if the device pin used is unknown (0) */
+ if (pci_pin <= 0)
+ return -EIO;
+
+ acpi_pci_bridge_t* bridge = acpi_find_pci_bridge(bus);
+
+ while (bridge != NULL)
+ {
+ int irq = acpi_get_irq_from_bus(bridge->handle, dev, func, pci_pin);
+ if (irq >= 0)
+ return irq;
+
+ /* Translate the downstream INTx to the upstream one */
+ pci_pin = ((pci_pin + dev) % 4);
+ bus = bridge->up_bus;
+ bridge = acpi_find_pci_bridge(bus);
+ }
+
+ acpi_handle handle;
+ acpi_status err = acpi_get_handle(ACPI_ROOT_OBJECT, "\\_SB.PCI0", &handle);
+ if (ACPI_FAILURE(err))
+ return -EIO;
+
+ return acpi_get_irq_from_bus(handle, dev, func, pci_pin);
+}
+
/* Fetch the active interrupt mode from GNU mach */
static error_t
acpi_required_pic_mode (void)
@@ -789,6 +849,145 @@
return err;
}
+static acpi_status
+acpi_pci_bridge_add(acpi_handle handle, uint8_t up_bus, uint8_t down_bus)
+{
+ acpi_pci_bridge_t* bridge =
+ (acpi_pci_bridge_t*)malloc(sizeof(acpi_pci_bridge_t));
+
+ if (bridge == NULL)
+ return AE_NO_MEMORY;
+
+ bridge->handle = handle;
+ bridge->up_bus = up_bus;
+ bridge->down_bus = down_bus;
+ bridge->next = acpi_pci_bridges;
+ acpi_pci_bridges = bridge;
+
+ return AE_OK;
+}
+
+struct acpi_find_bus_context
+{
+ int is_bridge; /* Is this level a bridge or not */
+ uint8_t down_bus; /* The downstream bus num if a bridge */
+ struct acpi_find_bus_context* parent;
+};
+
+static acpi_status
+acpi_find_busses_down(acpi_handle handle, uint32_t level,
+ void *context_arg, void **status)
+{
+ struct acpi_find_bus_context** context =
+ (struct acpi_find_bus_context**)context_arg;
+
+ struct acpi_device_info *devinfo;
+ acpi_status rv = acpi_get_object_info(handle, &devinfo);
+
+ if (ACPI_FAILURE(rv))
+ return rv;
+
+ struct acpi_find_bus_context* device_context =
+ (struct acpi_find_bus_context*)malloc(sizeof(struct acpi_find_bus_context));
+
+ if (device_context == NULL)
+ return AE_NO_MEMORY;
+
+ device_context->is_bridge = 0;
+ device_context->down_bus = 0;
+ device_context->parent = *context;
+ *context = device_context;
+
+ if (devinfo->flags & ACPI_PCI_ROOT_BRIDGE)
+ {
+ device_context->is_bridge = 1;
+ }
+ else if (device_context->parent->is_bridge && (devinfo->valid & ACPI_VALID_ADR))
+ {
+ uint8_t bus = device_context->parent->down_bus;
+ uint32_t devfunc = ACPI_LODWORD(devinfo->address);
+ uint16_t dev = ACPI_HIWORD(devfunc);
+ uint16_t func = ACPI_LOWORD(devfunc);
+
+ if (func <= 0x7 && dev <= 0x1f)
+ {
+ struct acpi_pci_id pci_id =
+ {
+ .segment = 0, .bus = bus, .device = dev, .function = func
+ };
+ u64 val;
+
+ /* Some ACPI entries don't refer to actual devices */
+ if (acpi_os_read_pci_configuration(&pci_id, PCI_HDR_TYPE_REG, &val, 8))
+ goto exit;
+
+ if ((val & 0x7f) != PCI_HDR_TYPE_PPB)
+ goto exit;
+
+ if (acpi_os_read_pci_configuration(&pci_id, PCI_PPB_SECBUS_REG, &val, 8))
+ {
+ acpi_os_printf("Failed to find secondary bus for: %x:%x:%x\n", bus, dev, func);
+ rv = AE_IO_ERROR;
+ goto exit;
+ }
+
+ /* Only 8 bits was read from the config. space so cast to uint8_t is safe */
+ device_context->down_bus = (uint8_t)val;
+ device_context->is_bridge = 1;
+
+ rv = acpi_pci_bridge_add(handle, bus, device_context->down_bus);
+ }
+ }
+
+ exit:
+ ACPI_FREE(devinfo);
+
+ return rv;
+}
+
+static acpi_status
+acpi_find_busses_up(acpi_handle handle, uint32_t level,
+ void *context_arg, void **status)
+{
+ struct acpi_find_bus_context** context =
+ (struct acpi_find_bus_context**)context_arg;
+
+ struct acpi_find_bus_context* entry = *context;
+ *context = entry->parent;
+
+ free(entry);
+
+ return AE_OK;
+}
+
+static acpi_status
+acpi_load_pci_bridges(void)
+{
+ struct acpi_find_bus_context head_context =
+ {
+ .is_bridge = 0,
+ .down_bus = 0,
+ .parent = NULL
+ };
+
+ struct acpi_find_bus_context* context = &head_context;
+
+ acpi_status rv = acpi_walk_namespace
+ (ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, UINT_MAX,
+ acpi_find_busses_down, acpi_find_busses_up, &context, NULL);
+
+ /* There should only be anything to free here in error cases */
+ while (context != &head_context)
+ {
+ struct acpi_find_bus_context* entry = context;
+ context = context->parent;
+
+ free(entry);
+ }
+
+ return rv;
+}
+
void acpi_init(void)
{
acpi_status err;
@@ -812,6 +1011,10 @@
if (ACPI_FAILURE (err))
goto die;
+ err = acpi_load_pci_bridges();
+ if (ACPI_FAILURE (err))
+ goto die;
+
/* Ignore return value for old gnumach without irq status */
acpi_configure_pic_mode();
