On Sun, 3 Dec 2017 10:37:35 +1100 David Gibson <da...@gibson.dropbear.id.au> wrote:
> On Sat, Dec 02, 2017 at 08:30:11PM +0100, Greg Kurz wrote:
> > PAPR 2.7 C.6.9.1.2 describes the "#interrupt-cells" property of the
> > PowerPC External Interrupt Source Controller node as follows:
> >
> > “#interrupt-cells”
> >
> > Standard property name to define the number of cells in an interrupt-
> > specifier within an interrupt domain.
> >
> > prop-encoded-array: An integer, encoded as with encode-int, that denotes
> > the number of cells required to represent an interrupt specifier in its
> > child nodes.
> >
> > The value of this property for the PowerPC External Interrupt option shall
> > be 2. Thus all interrupt specifiers (as used in the standard “interrupts”
> > property) shall consist of two cells, each containing an integer encoded
> > as with encode-int. The first integer represents the interrupt number the
> > second integer is the trigger code: 0 for edge triggered, 1 for level
> > triggered.
> >
> > This patch adds a second cell to the interrupt specifier stored in the
> > "interrupts" property of PCI device nodes. This property only exists if
> > the Interrupt Pin register is set, ie, the interrupt is level, the extra
> > cell is hence set to 1.
>
> Nack. This format of interrupt specifier is only needed for things
> wired directly to the xics. The PCI INTx interrupts aren't - they go
> through the interrupt nexus in the PHB. The interrupt-map is intended
> to remap the simple 1,2,3,4 for INTA..INTD to xics interrupt specifiers.
>
Indeed you're right... and the “#interrupt-cells” property of the PHB
is set to 1, ie, the interrupt specifier format in the child PCI isn't
expected to have an extra cell... This should have rung a bell :)
> > This also fixes the interrupt specifiers in the "interrupt-map" property
> > of the PHB node, that were setting the second cell to 8 (confusion with
> > IRQ_TYPE_LEVEL_LOW ?) instead of 1.
>
> Fixing that is correct, though I think. As might be the changes in
> other places I'll have to check.
>
> > While here, let's introduce defines for the interrupt specifier trigger
> > code, and patch other users in spapr.
> >
> > Signed-off-by: Greg Kurz <gr...@kaod.org>
>
>
> > ---
> >
> > This fixes /proc/interrupts in linux guests where LSIs appear as
> > Edge instead of Level.
> > ---
> > hw/ppc/spapr_events.c | 2 +-
> > hw/ppc/spapr_pci.c | 4 +++-
> > hw/ppc/spapr_vio.c | 3 ++-
> > include/hw/ppc/spapr.h | 3 +++
> > 4 files changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> > index e377fc7ddea2..4bcb98f948ea 100644
> > --- a/hw/ppc/spapr_events.c
> > +++ b/hw/ppc/spapr_events.c
> > @@ -283,7 +283,7 @@ void spapr_dt_events(sPAPRMachineState *spapr, void
> > *fdt)
> > }
> >
> > interrupts[0] = cpu_to_be32(source->irq);
> > - interrupts[1] = 0;
> > + interrupts[1] = SPAPR_DT_INTERRUPT_IDENTIFIER_EDGE;
> >
> > _FDT(node_offset = fdt_add_subnode(fdt, event_sources,
> > source_name));
> > _FDT(fdt_setprop(fdt, node_offset, "interrupts", interrupts,
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 5a3122a9f9f9..91fedbf0929c 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -1231,6 +1231,8 @@ static void spapr_populate_pci_child_dt(PCIDevice
> > *dev, void *fdt, int offset,
> > if (pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)) {
> > _FDT(fdt_setprop_cell(fdt, offset, "interrupts",
> > pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)));
> > + _FDT(fdt_appendprop_cell(fdt, offset, "interrupts",
> > + SPAPR_DT_INTERRUPT_IDENTIFIER_LEVEL));
> > }
> >
> > if (!is_bridge) {
> > @@ -2122,7 +2124,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
> > irqmap[3] = cpu_to_be32(j+1);
> > irqmap[4] = cpu_to_be32(xics_phandle);
> > irqmap[5] = cpu_to_be32(phb->lsi_table[lsi_num].irq);
> > - irqmap[6] = cpu_to_be32(0x8);
> > + irqmap[6] = cpu_to_be32(SPAPR_DT_INTERRUPT_IDENTIFIER_LEVEL);
> > }
> > }
> > /* Write interrupt map */
> > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> > index ea3bc8bd9e21..29a17651a17c 100644
> > --- a/hw/ppc/spapr_vio.c
> > +++ b/hw/ppc/spapr_vio.c
> > @@ -126,7 +126,8 @@ static int vio_make_devnode(VIOsPAPRDevice *dev,
> > }
> >
> > if (dev->irq) {
> > - uint32_t ints_prop[] = {cpu_to_be32(dev->irq), 0};
> > + uint32_t ints_prop[] = { cpu_to_be32(dev->irq),
> > + SPAPR_DT_INTERRUPT_IDENTIFIER_EDGE };
> >
> > ret = fdt_setprop(fdt, node_off, "interrupts", ints_prop,
> > sizeof(ints_prop));
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index 9d21ca9bde3a..8f6298bde59b 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -590,6 +590,9 @@ void spapr_load_rtas(sPAPRMachineState *spapr, void
> > *fdt, hwaddr addr);
> >
> > #define RTAS_EVENT_SCAN_RATE 1
> >
> > +#define SPAPR_DT_INTERRUPT_IDENTIFIER_EDGE 0
> > +#define SPAPR_DT_INTERRUPT_IDENTIFIER_LEVEL 1
> > +
> > typedef struct sPAPRTCETable sPAPRTCETable;
> >
> > #define TYPE_SPAPR_TCE_TABLE "spapr-tce-table"
> >
>
pgpXfm4thzdjs.pgp
Description: OpenPGP digital signature
