Re: [PATCH V2 2/2] fix pci device can't alloc irq from fdt

[lixianglai]

Hi Bibo Mao :
On 2025/11/12 上午11:26, Xianglai Li wrote:
When we use the -kernel parameter to start an elf format kernel 
relying on
fdt, we get the following error:
pcieport 0000:00:01.0: of_irq_parse_pci: failed with rc=-22
pcieport 0000:00:01.0: enabling device (0000 -> 0003)
pcieport 0000:00:01.0: PME: Signaling with IRQ 19
pcieport 0000:00:01.0: AER: enabled with IRQ 19
pcieport 0000:00:01.1: of_irq_parse_pci: failed with rc=-22
pcieport 0000:00:01.1: enabling device (0000 -> 0003)
pcieport 0000:00:01.1: PME: Signaling with IRQ 20
pcieport 0000:00:01.1: AER: enabled with IRQ 20
pcieport 0000:00:01.2: of_irq_parse_pci: failed with rc=-22
pcieport 0000:00:01.2: enabling device (0000 -> 0003)
pcieport 0000:00:01.2: PME: Signaling with IRQ 21
pcieport 0000:00:01.2: AER: enabled with IRQ 21
pcieport 0000:00:01.3: of_irq_parse_pci: failed with rc=-22
pcieport 0000:00:01.3: enabling device (0000 -> 0003)
pcieport 0000:00:01.3: PME: Signaling with IRQ 22
pcieport 0000:00:01.3: AER: enabled with IRQ 22
pcieport 0000:00:01.4: of_irq_parse_pci: failed with rc=-22

This is because  the description of interrupt-cell is missing in the 
pcie
irq map.  And there is a lack of a description of the interrupt trigger
type.  Now it is corrected and the correct interrupt-cell is added in 
the
pcie irq map.

Refer to the implementation in arm and add some comments.

Signed-off-by: Xianglai Li <lixiang...@loongson.cn>
---
Cc: Bibo Mao <maob...@loongson.cn>
Cc: Jiaxun Yang <jiaxun.y...@flygoat.com>
Cc: Song Gao <gaos...@loongson.cn>

  hw/loongarch/virt-fdt-build.c | 43 +++++++++++++++++++++++------------
  1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/hw/loongarch/virt-fdt-build.c 
b/hw/loongarch/virt-fdt-build.c
index 7333019cf7..73ac28bcba 100644
--- a/hw/loongarch/virt-fdt-build.c
+++ b/hw/loongarch/virt-fdt-build.c
@@ -317,10 +317,13 @@ static void fdt_add_pcie_irq_map_node(const 
LoongArchVirtMachineState *lvms,
                                        uint32_t *pch_pic_phandle)
  {
      int pin, dev;
-    uint32_t irq_map_stride = 0;
+    int irq_map_stride = 0;
This change may be unnecessary.
      uint32_t full_irq_map[PCI_NUM_PINS * PCI_NUM_PINS * 10] = {};
      uint32_t *irq_map = full_irq_map;
      const MachineState *ms = MACHINE(lvms);
+    uint32_t pch_pic_handle = *pch_pic_phandle;
This change may be unnecessary also, let us keep smallest modification.
+    uint32_t pin_mask;
+    uint32_t devfn_mask;
        /*
       * This code creates a standard swizzle of interrupts such that
@@ -339,31 +342,39 @@ static void fdt_add_pcie_irq_map_node(const 
LoongArchVirtMachineState *lvms,
              int irq_nr = 16 + ((pin + PCI_SLOT(devfn)) % 
PCI_NUM_PINS);
              int i = 0;
  -            /* Fill PCI address cells */
-            irq_map[i] = cpu_to_be32(devfn << 8);
-            i += 3;
-
-            /* Fill PCI Interrupt cells */
-            irq_map[i] = cpu_to_be32(pin + 1);
-            i += 1;
-
-            /* Fill interrupt controller phandle and cells */
-            irq_map[i++] = cpu_to_be32(*pch_pic_phandle);
-            irq_map[i++] = cpu_to_be32(irq_nr);
+            uint32_t map[] = {
+                devfn << 8, 0, 0,             /* devfn */
Is there any specification for two consecutive 0, 0 with 
interrupt-map? Although many of code is copied from other places, we 
had better know howto since the problem comes out already -:)

In the fdt specification, it is stipulated that address-cells require 
three cells in the interrupt-map, among which the first cell stores devfn.
The other two cells default to 0, which corresponds to the laddr 
variable in the linux kernel function of_irq_parse_pci.

By searching the Internet, the other two cells correspond respectively 
to the high and low bits of the pci device configuration space address.
Since interrupt information is described here and no memory address is 
needed, it is set to 0 by default.

Thanks!
Xianglai.

Regards
Bibo Mao
+                pin + 1,                      /* PCI pin */
+                pch_pic_handle,              /* interrupt controller 
handle */
+                irq_nr,                       /* irq number */
+                FDT_IRQ_TYPE_LEVEL_HIGH };    /* irq trigger level */
                if (!irq_map_stride) {
-                irq_map_stride = i;
+                irq_map_stride = sizeof(map) / sizeof(uint32_t);
              }
+
+            /* Convert map to big endian */
+            for (i = 0; i < irq_map_stride; i++) {
+                irq_map[i] = cpu_to_be32(map[i]);
+            }
+
              irq_map += irq_map_stride;
          }
      }
  -
      qemu_fdt_setprop(ms->fdt, nodename, "interrupt-map", full_irq_map,
                       PCI_NUM_PINS * PCI_NUM_PINS *
                       irq_map_stride * sizeof(uint32_t));
+
+    /* The pci slot only needs to specify the matching of the lower 
bit */
+    devfn_mask = cpu_to_be16(PCI_DEVFN((PCI_NUM_PINS - 1), 0));
+    /* The pci pin only needs to match the specified low bit */
+    pin_mask = (1 << ((PCI_NUM_PINS - 1))) - 1;
+
      qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupt-map-mask",
-                     0x1800, 0, 0, 0x7);
+                           devfn_mask,
+                           0, 0,
+                           pin_mask);
  }
    static void fdt_add_pcie_node(const LoongArchVirtMachineState *lvms,
@@ -400,6 +411,8 @@ static void fdt_add_pcie_node(const 
LoongArchVirtMachineState *lvms,
                                   2, base_mmio, 2, size_mmio);
      qemu_fdt_setprop_cells(ms->fdt, nodename, "msi-map",
                             0, *pch_msi_phandle, 0, 0x10000);
+
+    qemu_fdt_setprop_cell(ms->fdt, nodename, "#interrupt-cells", 1);
      fdt_add_pcie_irq_map_node(lvms, nodename, pch_pic_phandle);
      g_free(nodename);
  }