2021-06-23 23:40:21

by Maciej W. Rozycki

[permalink] [raw]
Subject: [PATCH RFC 0/2] x86/PCI: SiS PIRQ router updates

Hi,

Nikolai has observed the trigger mode not being fixed up once it has been
incorrectly set by the BIOS for PCI devices, causing all kinds of usual
issues. As it turns out we don't have a PIRQ router defined for the
SiS85C497 southbridge, which Nikolai's system uses, and which is different
from the SiS85C503 southbridge we have support for.

As we use the generic `sis' infix (capitalised or not) for the SiS85C503
southbridge I have prepared this small patch series to first make the
existing SiS program entities use a more specific `sis503' infix, and then
provide a suitable PIRQ router for the SiS85C497 device.

Posted as an RFC at this stage as it still has to be verified.

Nikolai, can you please give it a hit with the extra debug patch as
requested in my other message?

Maciej


2021-06-23 23:40:46

by Maciej W. Rozycki

[permalink] [raw]
Subject: [PATCH RFC 1/2] x86/PCI: Disambiguate SiS85C503 PIRQ router code entities

In preparation to adding support for the SiS85C497 PIRQ router add `503'
to the names of SiS85C503 PIRQ router code entities so that they clearly
indicate which device they refer to.

Also restructure `sis_router_probe' such that new device IDs will be
just new switch cases.

No functional change.

Signed-off-by: Maciej W. Rozycki <[email protected]>
---
arch/x86/pci/irq.c | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)

linux-x86-pirq-router-sis85c503.diff
Index: linux-macro-ide/arch/x86/pci/irq.c
===================================================================
--- linux-macro-ide.orig/arch/x86/pci/irq.c
+++ linux-macro-ide/arch/x86/pci/irq.c
@@ -389,11 +389,12 @@ static int pirq_cyrix_set(struct pci_dev
* bit 6-4 are probably unused, not like 5595
*/

-#define PIRQ_SIS_IRQ_MASK 0x0f
-#define PIRQ_SIS_IRQ_DISABLE 0x80
-#define PIRQ_SIS_USB_ENABLE 0x40
+#define PIRQ_SIS503_IRQ_MASK 0x0f
+#define PIRQ_SIS503_IRQ_DISABLE 0x80
+#define PIRQ_SIS503_USB_ENABLE 0x40

-static int pirq_sis_get(struct pci_dev *router, struct pci_dev *dev, int pirq)
+static int pirq_sis503_get(struct pci_dev *router, struct pci_dev *dev,
+ int pirq)
{
u8 x;
int reg;
@@ -402,10 +403,11 @@ static int pirq_sis_get(struct pci_dev *
if (reg >= 0x01 && reg <= 0x04)
reg += 0x40;
pci_read_config_byte(router, reg, &x);
- return (x & PIRQ_SIS_IRQ_DISABLE) ? 0 : (x & PIRQ_SIS_IRQ_MASK);
+ return (x & PIRQ_SIS503_IRQ_DISABLE) ? 0 : (x & PIRQ_SIS503_IRQ_MASK);
}

-static int pirq_sis_set(struct pci_dev *router, struct pci_dev *dev, int pirq, int irq)
+static int pirq_sis503_set(struct pci_dev *router, struct pci_dev *dev,
+ int pirq, int irq)
{
u8 x;
int reg;
@@ -414,8 +416,8 @@ static int pirq_sis_set(struct pci_dev *
if (reg >= 0x01 && reg <= 0x04)
reg += 0x40;
pci_read_config_byte(router, reg, &x);
- x &= ~(PIRQ_SIS_IRQ_MASK | PIRQ_SIS_IRQ_DISABLE);
- x |= irq ? irq: PIRQ_SIS_IRQ_DISABLE;
+ x &= ~(PIRQ_SIS503_IRQ_MASK | PIRQ_SIS503_IRQ_DISABLE);
+ x |= irq ? irq : PIRQ_SIS503_IRQ_DISABLE;
pci_write_config_byte(router, reg, x);
return 1;
}
@@ -697,13 +699,14 @@ static __init int serverworks_router_pro

static __init int sis_router_probe(struct irq_router *r, struct pci_dev *router, u16 device)
{
- if (device != PCI_DEVICE_ID_SI_503)
- return 0;
-
- r->name = "SIS";
- r->get = pirq_sis_get;
- r->set = pirq_sis_set;
- return 1;
+ switch (device) {
+ case PCI_DEVICE_ID_SI_503:
+ r->name = "SiS85C503";
+ r->get = pirq_sis503_get;
+ r->set = pirq_sis503_set;
+ return 1;
+ }
+ return 0;
}

static __init int cyrix_router_probe(struct irq_router *r, struct pci_dev *router, u16 device)

2021-06-23 23:41:21

by Maciej W. Rozycki

[permalink] [raw]
Subject: [PATCH RFC 2/2] x86/PCI: Add support for the SiS85C497 PIRQ router

The SiS 85C496/497 486 Green PC VESA/ISA/PCI Chipset has support for PCI
steering and the ELCR register implemented. These features are handled
by the SiS85C497 AT Bus Controller & Megacell (ATM) ISA bridge, however
the device is wired as a peer bridge directly to the host bus and has
its PCI configuration registers decoded at addresses 0x80-0xff by the
accompanying SiS85C496 PCI & CPU Memory Controller (PCM) host bridge[1].
Therefore we need to match on the host bridge's vendor and device ID.

Like with the SiS85C503 PIRQ router handle link value ranges of 1-4 and
0xc0-0xc3, corresponding respectively to PIRQ line numbers counted from
1 and link register PCI configuration space addresses.

References:

[1] "486 Green PC VESA/ISA/PCI Chipset, SiS 85C496/497", Rev 3.0,
Silicon Integrated Systems Corp., July 1995, Part IV, Section 3.
"PCI Configuration Space Registers (00h ~ FFh)", p. 114

Signed-off-by: Maciej W. Rozycki <[email protected]>
---
arch/x86/pci/irq.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 80 insertions(+)

linux-x86-pirq-router-sis85c497.diff
Index: linux-macro-ide/arch/x86/pci/irq.c
===================================================================
--- linux-macro-ide.orig/arch/x86/pci/irq.c
+++ linux-macro-ide/arch/x86/pci/irq.c
@@ -328,6 +328,81 @@ static int pirq_cyrix_set(struct pci_dev
return 1;
}

+
+/*
+ * PIRQ routing for the SiS85C497 AT Bus Controller & Megacell (ATM)
+ * ISA bridge used with the SiS 85C496/497 486 Green PC VESA/ISA/PCI
+ * Chipset.
+ *
+ * There are four PCI INTx#-to-IRQ Link registers provided in the
+ * SiS85C497 part of the peculiar combined 85C496/497 configuration
+ * space decoded by the SiS85C496 PCI & CPU Memory Controller (PCM)
+ * host bridge, at 0xc0/0xc1/0xc2/0xc3 respectively for the PCI INT
+ * A/B/C/D lines. Bit 7 enables the respective link if set and bits
+ * 3:0 select the 8259A IRQ line as follows:
+ *
+ * 0000 : Reserved
+ * 0001 : Reserved
+ * 0010 : Reserved
+ * 0011 : IRQ3
+ * 0100 : IRQ4
+ * 0101 : IRQ5
+ * 0110 : IRQ6
+ * 0111 : IRQ7
+ * 1000 : Reserved
+ * 1001 : IRQ9
+ * 1010 : IRQ10
+ * 1011 : IRQ11
+ * 1100 : IRQ12
+ * 1101 : Reserved
+ * 1110 : IRQ14
+ * 1111 : IRQ15
+ *
+ * We avoid using a reserved value for disabled links, hence the
+ * choice of IRQ15 for that case.
+ *
+ * References:
+ *
+ * "486 Green PC VESA/ISA/PCI Chipset, SiS 85C496/497", Rev 3.0,
+ * Silicon Integrated Systems Corp., July 1995
+ */
+
+#define PCI_SIS497_INTA_TO_IRQ_LINK 0xc0u
+
+#define PIRQ_SIS497_IRQ_MASK 0x0fu
+#define PIRQ_SIS497_IRQ_ENABLE 0x80u
+
+static int pirq_sis497_get(struct pci_dev *router, struct pci_dev *dev,
+ int pirq)
+{
+ int reg;
+ u8 x;
+
+ reg = pirq;
+ if (reg >= 1 && reg <= 4)
+ reg += PCI_SIS497_INTA_TO_IRQ_LINK - 1;
+
+ pci_read_config_byte(router, reg, &x);
+ return (x & PIRQ_SIS497_IRQ_ENABLE) ? (x & PIRQ_SIS497_IRQ_MASK) : 0;
+}
+
+static int pirq_sis497_set(struct pci_dev *router, struct pci_dev *dev,
+ int pirq, int irq)
+{
+ int reg;
+ u8 x;
+
+ reg = pirq;
+ if (reg >= 1 && reg <= 4)
+ reg += PCI_SIS497_INTA_TO_IRQ_LINK - 1;
+
+ pci_read_config_byte(router, reg, &x);
+ x &= ~(PIRQ_SIS497_IRQ_MASK | PIRQ_SIS497_IRQ_ENABLE);
+ x |= irq ? (PIRQ_SIS497_IRQ_ENABLE | irq) : PIRQ_SIS497_IRQ_MASK;
+ pci_write_config_byte(router, reg, x);
+ return 1;
+}
+
/*
* PIRQ routing for SiS 85C503 router used in several SiS chipsets.
* We have to deal with the following issues here:
@@ -700,6 +775,11 @@ static __init int serverworks_router_pro
static __init int sis_router_probe(struct irq_router *r, struct pci_dev *router, u16 device)
{
switch (device) {
+ case PCI_DEVICE_ID_SI_496:
+ r->name = "SiS85C497";
+ r->get = pirq_sis497_get;
+ r->set = pirq_sis497_set;
+ return 1;
case PCI_DEVICE_ID_SI_503:
r->name = "SiS85C503";
r->get = pirq_sis503_get;

2021-07-02 17:17:26

by Nikolai Zhubr

[permalink] [raw]
Subject: Re: [PATCH RFC 0/2] x86/PCI: SiS PIRQ router updates

Hello Maciej,

24.06.2021 2:38, Maciej W. Rozycki:
> As we use the generic `sis' infix (capitalised or not) for the SiS85C503
> southbridge I have prepared this small patch series to first make the
> existing SiS program entities use a more specific `sis503' infix, and then
> provide a suitable PIRQ router for the SiS85C497 device.
>
> Posted as an RFC at this stage as it still has to be verified.
>
> Nikolai, can you please give it a hit with the extra debug patch as
> requested in my other message?

With
linux-x86-pirq-router-sis85c503.diff applied
linux-x86-pirq-router-sis85c497.diff applied
and DEBUG 1 in arch/x86/include/asm/pci_x86.h
here is new log:

https://pastebin.com/n3udQgcq

My feeling is that something went a bit wrong because:

8139too 0000:00:0d.0: can't route interrupt

and

# 8259A.pl
irq 0: 00, edge
irq 1: 00, edge
irq 2: 00, edge
irq 3: 00, edge
irq 4: 00, edge
irq 5: 00, edge
irq 6: 00, edge
irq 7: 00, edge
irq 8: 00, edge
irq 9: 00, edge
irq 10: 00, edge
irq 11: 00, edge
irq 12: 00, edge
irq 13: 00, edge
irq 14: 00, edge
irq 15: 00, edge

Note: I still used 4.14 kernel for this test but your patches applied
cleanly with no fuzz so I suppose it should be ok.


Thank you,

Regards,
Nikolai


>
> Maciej
>

2021-07-02 23:10:32

by Maciej W. Rozycki

[permalink] [raw]
Subject: Re: [PATCH RFC 0/2] x86/PCI: SiS PIRQ router updates

Hello Nikolai,

> > Nikolai, can you please give it a hit with the extra debug patch as
> > requested in my other message?
>
> With
> linux-x86-pirq-router-sis85c503.diff applied
> linux-x86-pirq-router-sis85c497.diff applied
> and DEBUG 1 in arch/x86/include/asm/pci_x86.h
> here is new log:
>
> https://pastebin.com/n3udQgcq
>
> My feeling is that something went a bit wrong because:
>
> 8139too 0000:00:0d.0: can't route interrupt

More important is actually the previous line:

PCI: Attempting to find IRQ router for [0000:0000]

meaning that the PIRQ structure defined by the BIOS has not specified the
vendor:device ID pair for the router [1039:0496] which we match against.

I don't know where exactly the definition our `struct irq_routing_table'
corresponds to comes from (I'll appreciate pointers!); it has not been
given in the PCI BIOS 2.1 specification, only individual IRQ routing
entries have. It may well be that [0000:0000] is a legitimate blanket
entry requesting the OS to figure out itself which device is the hardware
router.

Anyway it is something we can handle in a reasonably staightforward
manner, so I'll spend some time to implement such wildcard matching. Most
importantly your system does have the router structure, so barring this
minor snag it can be handled properly without horrible hacks. We're very
close now.

> Note: I still used 4.14 kernel for this test but your patches applied cleanly
> with no fuzz so I suppose it should be ok.

Indeed, that does not matter. This code is very old, dating back to
1990s.

Maciej

2021-07-05 10:57:02

by Maciej W. Rozycki

[permalink] [raw]
Subject: Re: [PATCH RFC 0/2] x86/PCI: SiS PIRQ router updates

On Sat, 3 Jul 2021, Maciej W. Rozycki wrote:

> > > Nikolai, can you please give it a hit with the extra debug patch as
> > > requested in my other message?
> >
> > With
> > linux-x86-pirq-router-sis85c503.diff applied
> > linux-x86-pirq-router-sis85c497.diff applied
> > and DEBUG 1 in arch/x86/include/asm/pci_x86.h
> > here is new log:
> >
> > https://pastebin.com/n3udQgcq
> >
> > My feeling is that something went a bit wrong because:
> >
> > 8139too 0000:00:0d.0: can't route interrupt
>
> More important is actually the previous line:
>
> PCI: Attempting to find IRQ router for [0000:0000]
>
> meaning that the PIRQ structure defined by the BIOS has not specified the
> vendor:device ID pair for the router [1039:0496] which we match against.

So it's actually both the vendor:device ID pair and the bus address that
matter here. I've now posted a patch I am fairly sure about that combined
with the earlier changes it will fix your issue. I have verified it in a
simulated environment where the PIRQ routing table has an invalid vendor
ID given, with correct results produced:

PCI: Attempting to find IRQ router for [0000:0000]
PCI: Trying IRQ router for [8086:1250]
PCI: Trying IRQ router for [8086:7000]
pci 0000:00:07.0: SIO/PIIX/ICH IRQ router [8086:7000]

Please let me know of the outcome.

Maciej

2021-07-08 20:50:38

by Maciej W. Rozycki

[permalink] [raw]
Subject: Re: [PATCH RFC 0/2] x86/PCI: SiS PIRQ router updates

On Thu, 24 Jun 2021, Maciej W. Rozycki wrote:

> Posted as an RFC at this stage as it still has to be verified.

So this has passed verification as it is, although with an extra change,
not directly related. Shall I repost the series unchanged with the "RFC"
annotation removed then, or can it go in as posted?

Maciej