2021-07-20 03:31:48

by Maciej W. Rozycki

[permalink] [raw]
Subject: [PATCH 0/6] x86: PIRQ/ELCR-related fixes and updates

Hi,

In the course of adding PIRQ routing support for Nikolai's FinALi system
I realised we need to have some infrastructure for the indirectly accessed
configuration space implemented by some chipsets as well as Cyrix CPUs and
also included with the Intel MP spec for the IMCR register via port I/O
space locations 0x22/0x23. With that in place I implemented PIRQ support
for the Intel PCEB/ESC combined EISA southbridge using the same scheme to
access the relevant registers and for the final remaining Intel chipset of
the era, that is the i420EX.

While at it I chose to rewrite ELCR register accesses to avoid using
magic numbers scattered across our code and use proper macros like with
the remaining PIC registers, and while at it again I noticed and fixed a
number of typos: s/ECLR/ELCR/.

Since there are mechanical dependencies between the patches (except for
typo fixes) I chose to send them as a series rather than individually,
though 3/6 depends on: <https://lore.kernel.org/patchwork/patch/1452772/>
necessarily as well, the fate of which is currently unclear to me.

See individual change descriptions for details.

Nikolai: for your system only 1/6 and 2/6 are required, though you are
free to experiment with all the patches. Mind that 3/6 mechanically
depends on the earlier change for the SIO PIRQ router referred above. In
any case please use the debug patch for PCI code as well as the earlier
patches for your other system and send the resulting bootstrap log for
confirmation.

Ideally this would be verified with PCI interrupt sharing, but for that
you'd have to track down one or more multifunction option cards (USB 2.0
interfaces with legacy 1.1 functions or serial/parallel multi-I/O cards
are good candidates, but of course there are more) or option devices with
PCI-to-PCI bridges, and then actually use some of these devices as well.
Any interrupt sharing will be reported, e.g.:

pci 0000:00:07.0: SIO/PIIX/ICH IRQ router [8086:7000]
pci 0000:00:11.0: PCI INT A -> PIRQ 63, mask deb8, excl 0c20
pci 0000:00:11.0: PCI INT A -> newirq 0
PCI: setting IRQ 11 as level-triggered
pci 0000:00:11.0: found PCI INT A -> IRQ 11
pci 0000:00:11.0: sharing IRQ 11 with 0000:00:07.2
pci 0000:02:00.0: using bridge 0000:00:11.0 INT A to get INT A
pci 0000:00:11.0: sharing IRQ 11 with 0000:02:00.0
pci 0000:02:01.0: using bridge 0000:00:11.0 INT B to get INT A
pci 0000:02:02.0: using bridge 0000:00:11.0 INT C to get INT A
pci 0000:03:00.0: using bridge 0000:00:11.0 INT A to get INT A
pci 0000:00:11.0: sharing IRQ 11 with 0000:03:00.0
pci 0000:04:00.0: using bridge 0000:00:11.0 INT B to get INT A
pci 0000:04:00.3: using bridge 0000:00:11.0 INT A to get INT D
pci 0000:00:11.0: sharing IRQ 11 with 0000:04:00.3
pci 0000:06:05.0: using bridge 0000:00:11.0 INT D to get INT A
pci 0000:06:08.0: using bridge 0000:00:11.0 INT C to get INT A
pci 0000:06:08.1: using bridge 0000:00:11.0 INT D to get INT B
pci 0000:06:08.2: using bridge 0000:00:11.0 INT A to get INT C
pci 0000:00:11.0: sharing IRQ 11 with 0000:06:08.2

-- a lot of sharing and swizzling here. :) You'd most definitely need:
<https://lore.kernel.org/patchwork/patch/1454747/> for that though, as I
can't imagine PCI BIOS 2.1 PIRQ routers to commonly enumerate devices
behind PCI-to-PCI bridges, given that they fail to cope with more complex
bus topologies created by option devices in the first place.

Maciej


2021-07-20 03:32:20

by Maciej W. Rozycki

[permalink] [raw]
Subject: [PATCH 5/6] x86: Avoid magic number with ELCR register accesses

Define PIC_ELCR1 and PIC_ELCR2 macros for accesses to the ELCR registers
implemented by many chipsets in their embedded 8259A PIC cores, avoiding
magic numbers that are difficult to handle, and complementing the macros
we already have for registers originally defined with discrete 8259A PIC
implementations. No functional change.

Signed-off-by: Maciej W. Rozycki <[email protected]>
---
This deliberately doesn't touch KVM, which refrains from using macros for
any PIC accesses.
---
arch/x86/include/asm/i8259.h | 2 ++
arch/x86/kernel/acpi/boot.c | 6 +++---
arch/x86/kernel/apic/io_apic.c | 2 +-
arch/x86/kernel/apic/vector.c | 2 +-
arch/x86/kernel/i8259.c | 8 ++++----
arch/x86/kernel/mpparse.c | 3 ++-
arch/x86/pci/irq.c | 3 ++-
7 files changed, 15 insertions(+), 11 deletions(-)

linux-x86-pic-elcr.diff
Index: linux-macro-pirq/arch/x86/include/asm/i8259.h
===================================================================
--- linux-macro-pirq.orig/arch/x86/include/asm/i8259.h
+++ linux-macro-pirq/arch/x86/include/asm/i8259.h
@@ -19,6 +19,8 @@ extern unsigned int cached_irq_mask;
#define PIC_MASTER_OCW3 PIC_MASTER_ISR
#define PIC_SLAVE_CMD 0xa0
#define PIC_SLAVE_IMR 0xa1
+#define PIC_ELCR1 0x4d0
+#define PIC_ELCR2 0x4d1

/* i8259A PIC related value */
#define PIC_CASCADE_IR 2
Index: linux-macro-pirq/arch/x86/kernel/acpi/boot.c
===================================================================
--- linux-macro-pirq.orig/arch/x86/kernel/acpi/boot.c
+++ linux-macro-pirq/arch/x86/kernel/acpi/boot.c
@@ -570,7 +570,7 @@ void __init acpi_pic_sci_set_trigger(uns
unsigned int old, new;

/* Real old ELCR mask */
- old = inb(0x4d0) | (inb(0x4d1) << 8);
+ old = inb(PIC_ELCR1) | (inb(PIC_ELCR2) << 8);

/*
* If we use ACPI to set PCI IRQs, then we should clear ELCR
@@ -596,8 +596,8 @@ void __init acpi_pic_sci_set_trigger(uns
return;

pr_warn("setting ELCR to %04x (from %04x)\n", new, old);
- outb(new, 0x4d0);
- outb(new >> 8, 0x4d1);
+ outb(new, PIC_ELCR1);
+ outb(new >> 8, PIC_ELCR2);
}

int acpi_gsi_to_irq(u32 gsi, unsigned int *irqp)
Index: linux-macro-pirq/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-macro-pirq.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-macro-pirq/arch/x86/kernel/apic/io_apic.c
@@ -764,7 +764,7 @@ static bool irq_active_low(int idx)
static bool EISA_ELCR(unsigned int irq)
{
if (irq < nr_legacy_irqs()) {
- unsigned int port = 0x4d0 + (irq >> 3);
+ unsigned int port = PIC_ELCR1 + (irq >> 3);
return (inb(port) >> (irq & 7)) & 1;
}
apic_printk(APIC_VERBOSE, KERN_INFO
Index: linux-macro-pirq/arch/x86/kernel/apic/vector.c
===================================================================
--- linux-macro-pirq.orig/arch/x86/kernel/apic/vector.c
+++ linux-macro-pirq/arch/x86/kernel/apic/vector.c
@@ -1299,7 +1299,7 @@ static void __init print_PIC(void)

pr_debug("... PIC ISR: %04x\n", v);

- v = inb(0x4d1) << 8 | inb(0x4d0);
+ v = inb(PIC_ELCR2) << 8 | inb(PIC_ELCR1);
pr_debug("... PIC ELCR: %04x\n", v);
}

Index: linux-macro-pirq/arch/x86/kernel/i8259.c
===================================================================
--- linux-macro-pirq.orig/arch/x86/kernel/i8259.c
+++ linux-macro-pirq/arch/x86/kernel/i8259.c
@@ -235,15 +235,15 @@ static char irq_trigger[2];
*/
static void restore_ELCR(char *trigger)
{
- outb(trigger[0], 0x4d0);
- outb(trigger[1], 0x4d1);
+ outb(trigger[0], PIC_ELCR1);
+ outb(trigger[1], PIC_ELCR2);
}

static void save_ELCR(char *trigger)
{
/* IRQ 0,1,2,8,13 are marked as reserved */
- trigger[0] = inb(0x4d0) & 0xF8;
- trigger[1] = inb(0x4d1) & 0xDE;
+ trigger[0] = inb(PIC_ELCR1) & 0xF8;
+ trigger[1] = inb(PIC_ELCR2) & 0xDE;
}

static void i8259A_resume(void)
Index: linux-macro-pirq/arch/x86/kernel/mpparse.c
===================================================================
--- linux-macro-pirq.orig/arch/x86/kernel/mpparse.c
+++ linux-macro-pirq/arch/x86/kernel/mpparse.c
@@ -19,6 +19,7 @@
#include <linux/smp.h>
#include <linux/pci.h>

+#include <asm/i8259.h>
#include <asm/io_apic.h>
#include <asm/acpi.h>
#include <asm/irqdomain.h>
@@ -251,7 +252,7 @@ static int __init ELCR_trigger(unsigned
{
unsigned int port;

- port = 0x4d0 + (irq >> 3);
+ port = PIC_ELCR1 + (irq >> 3);
return (inb(port) >> (irq & 7)) & 1;
}

Index: linux-macro-pirq/arch/x86/pci/irq.c
===================================================================
--- linux-macro-pirq.orig/arch/x86/pci/irq.c
+++ linux-macro-pirq/arch/x86/pci/irq.c
@@ -18,6 +18,7 @@
#include <linux/irq.h>
#include <linux/acpi.h>

+#include <asm/i8259.h>
#include <asm/pc-conf-reg.h>
#include <asm/pci_x86.h>

@@ -159,7 +160,7 @@ static void __init pirq_peer_trick(void)
void elcr_set_level_irq(unsigned int irq)
{
unsigned char mask = 1 << (irq & 7);
- unsigned int port = 0x4d0 + (irq >> 3);
+ unsigned int port = PIC_ELCR1 + (irq >> 3);
unsigned char val;
static u16 elcr_irq_mask;

2021-07-20 03:33:22

by Maciej W. Rozycki

[permalink] [raw]
Subject: [PATCH 6/6] x86: Fix typo s/ECLR/ELCR/ for the PIC register

The proper spelling for the acronym referring to the Edge/Level Control
Register is ELCR rather than ECLR. Adjust references accordingly. No
functional change.

Signed-off-by: Maciej W. Rozycki <[email protected]>
---
arch/x86/kernel/acpi/boot.c | 6 +++---
arch/x86/kvm/i8259.c | 20 ++++++++++----------
arch/x86/kvm/irq.h | 2 +-
3 files changed, 14 insertions(+), 14 deletions(-)

linux-x86-pic-elcr-typo.diff
Index: linux-macro-pirq/arch/x86/kernel/acpi/boot.c
===================================================================
--- linux-macro-pirq.orig/arch/x86/kernel/acpi/boot.c
+++ linux-macro-pirq/arch/x86/kernel/acpi/boot.c
@@ -558,10 +558,10 @@ acpi_parse_nmi_src(union acpi_subtable_h
* If a PIC-mode SCI is not recognized or gives spurious IRQ7's
* it may require Edge Trigger -- use "acpi_sci=edge"
*
- * Port 0x4d0-4d1 are ECLR1 and ECLR2, the Edge/Level Control Registers
+ * Port 0x4d0-4d1 are ELCR1 and ELCR2, the Edge/Level Control Registers
* for the 8259 PIC. bit[n] = 1 means irq[n] is Level, otherwise Edge.
- * ECLR1 is IRQs 0-7 (IRQ 0, 1, 2 must be 0)
- * ECLR2 is IRQs 8-15 (IRQ 8, 13 must be 0)
+ * ELCR1 is IRQs 0-7 (IRQ 0, 1, 2 must be 0)
+ * ELCR2 is IRQs 8-15 (IRQ 8, 13 must be 0)
*/

void __init acpi_pic_sci_set_trigger(unsigned int irq, u16 trigger)
Index: linux-macro-pirq/arch/x86/kvm/i8259.c
===================================================================
--- linux-macro-pirq.orig/arch/x86/kvm/i8259.c
+++ linux-macro-pirq/arch/x86/kvm/i8259.c
@@ -541,17 +541,17 @@ static int picdev_slave_read(struct kvm_
addr, len, val);
}

-static int picdev_eclr_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
+static int picdev_elcr_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
gpa_t addr, int len, const void *val)
{
- return picdev_write(container_of(dev, struct kvm_pic, dev_eclr),
+ return picdev_write(container_of(dev, struct kvm_pic, dev_elcr),
addr, len, val);
}

-static int picdev_eclr_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
+static int picdev_elcr_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
gpa_t addr, int len, void *val)
{
- return picdev_read(container_of(dev, struct kvm_pic, dev_eclr),
+ return picdev_read(container_of(dev, struct kvm_pic, dev_elcr),
addr, len, val);
}

@@ -577,9 +577,9 @@ static const struct kvm_io_device_ops pi
.write = picdev_slave_write,
};

-static const struct kvm_io_device_ops picdev_eclr_ops = {
- .read = picdev_eclr_read,
- .write = picdev_eclr_write,
+static const struct kvm_io_device_ops picdev_elcr_ops = {
+ .read = picdev_elcr_read,
+ .write = picdev_elcr_write,
};

int kvm_pic_init(struct kvm *kvm)
@@ -602,7 +602,7 @@ int kvm_pic_init(struct kvm *kvm)
*/
kvm_iodevice_init(&s->dev_master, &picdev_master_ops);
kvm_iodevice_init(&s->dev_slave, &picdev_slave_ops);
- kvm_iodevice_init(&s->dev_eclr, &picdev_eclr_ops);
+ kvm_iodevice_init(&s->dev_elcr, &picdev_elcr_ops);
mutex_lock(&kvm->slots_lock);
ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, 0x20, 2,
&s->dev_master);
@@ -613,7 +613,7 @@ int kvm_pic_init(struct kvm *kvm)
if (ret < 0)
goto fail_unreg_2;

- ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, 0x4d0, 2, &s->dev_eclr);
+ ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, 0x4d0, 2, &s->dev_elcr);
if (ret < 0)
goto fail_unreg_1;

@@ -647,7 +647,7 @@ void kvm_pic_destroy(struct kvm *kvm)
mutex_lock(&kvm->slots_lock);
kvm_io_bus_unregister_dev(vpic->kvm, KVM_PIO_BUS, &vpic->dev_master);
kvm_io_bus_unregister_dev(vpic->kvm, KVM_PIO_BUS, &vpic->dev_slave);
- kvm_io_bus_unregister_dev(vpic->kvm, KVM_PIO_BUS, &vpic->dev_eclr);
+ kvm_io_bus_unregister_dev(vpic->kvm, KVM_PIO_BUS, &vpic->dev_elcr);
mutex_unlock(&kvm->slots_lock);

kvm->arch.vpic = NULL;
Index: linux-macro-pirq/arch/x86/kvm/irq.h
===================================================================
--- linux-macro-pirq.orig/arch/x86/kvm/irq.h
+++ linux-macro-pirq/arch/x86/kvm/irq.h
@@ -55,7 +55,7 @@ struct kvm_pic {
int output; /* intr from master PIC */
struct kvm_io_device dev_master;
struct kvm_io_device dev_slave;
- struct kvm_io_device dev_eclr;
+ struct kvm_io_device dev_elcr;
void (*ack_notifier)(void *opaque, int irq);
unsigned long irq_states[PIC_NUM_PINS];
};

2021-07-21 00:14:16

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 0/6] x86: PIRQ/ELCR-related fixes and updates

On Tue, Jul 20, 2021 at 05:27:43AM +0200, Maciej W. Rozycki wrote:
> Hi,
>
> In the course of adding PIRQ routing support for Nikolai's FinALi system
> I realised we need to have some infrastructure for the indirectly accessed
> configuration space implemented by some chipsets as well as Cyrix CPUs and
> also included with the Intel MP spec for the IMCR register via port I/O
> space locations 0x22/0x23. With that in place I implemented PIRQ support
> for the Intel PCEB/ESC combined EISA southbridge using the same scheme to
> access the relevant registers and for the final remaining Intel chipset of
> the era, that is the i420EX.
>
> While at it I chose to rewrite ELCR register accesses to avoid using
> magic numbers scattered across our code and use proper macros like with
> the remaining PIC registers, and while at it again I noticed and fixed a
> number of typos: s/ECLR/ELCR/.
>
> Since there are mechanical dependencies between the patches (except for
> typo fixes) I chose to send them as a series rather than individually,
> though 3/6 depends on: <https://lore.kernel.org/patchwork/patch/1452772/>
> necessarily as well, the fate of which is currently unclear to me.
>
> See individual change descriptions for details.
>
> Nikolai: for your system only 1/6 and 2/6 are required, though you are
> free to experiment with all the patches. Mind that 3/6 mechanically
> depends on the earlier change for the SIO PIRQ router referred above. In
> any case please use the debug patch for PCI code as well as the earlier
> patches for your other system and send the resulting bootstrap log for
> confirmation.
>
> Ideally this would be verified with PCI interrupt sharing, but for that
> you'd have to track down one or more multifunction option cards (USB 2.0
> interfaces with legacy 1.1 functions or serial/parallel multi-I/O cards
> are good candidates, but of course there are more) or option devices with
> PCI-to-PCI bridges, and then actually use some of these devices as well.
> Any interrupt sharing will be reported, e.g.:
>
> pci 0000:00:07.0: SIO/PIIX/ICH IRQ router [8086:7000]
> pci 0000:00:11.0: PCI INT A -> PIRQ 63, mask deb8, excl 0c20
> pci 0000:00:11.0: PCI INT A -> newirq 0
> PCI: setting IRQ 11 as level-triggered
> pci 0000:00:11.0: found PCI INT A -> IRQ 11
> pci 0000:00:11.0: sharing IRQ 11 with 0000:00:07.2
> pci 0000:02:00.0: using bridge 0000:00:11.0 INT A to get INT A
> pci 0000:00:11.0: sharing IRQ 11 with 0000:02:00.0
> pci 0000:02:01.0: using bridge 0000:00:11.0 INT B to get INT A
> pci 0000:02:02.0: using bridge 0000:00:11.0 INT C to get INT A
> pci 0000:03:00.0: using bridge 0000:00:11.0 INT A to get INT A
> pci 0000:00:11.0: sharing IRQ 11 with 0000:03:00.0
> pci 0000:04:00.0: using bridge 0000:00:11.0 INT B to get INT A
> pci 0000:04:00.3: using bridge 0000:00:11.0 INT A to get INT D
> pci 0000:00:11.0: sharing IRQ 11 with 0000:04:00.3
> pci 0000:06:05.0: using bridge 0000:00:11.0 INT D to get INT A
> pci 0000:06:08.0: using bridge 0000:00:11.0 INT C to get INT A
> pci 0000:06:08.1: using bridge 0000:00:11.0 INT D to get INT B
> pci 0000:06:08.2: using bridge 0000:00:11.0 INT A to get INT C
> pci 0000:00:11.0: sharing IRQ 11 with 0000:06:08.2
>
> -- a lot of sharing and swizzling here. :) You'd most definitely need:
> <https://lore.kernel.org/patchwork/patch/1454747/> for that though, as I
> can't imagine PCI BIOS 2.1 PIRQ routers to commonly enumerate devices
> behind PCI-to-PCI bridges, given that they fail to cope with more complex
> bus topologies created by option devices in the first place.

Looks nicely done but I have no ability to review or test, so I assume
the x86 folks will take care of this.

Bjorn

2021-07-21 20:54:34

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 0/6] x86: PIRQ/ELCR-related fixes and updates

On Tue, Jul 20 2021 at 19:12, Bjorn Helgaas wrote:
> On Tue, Jul 20, 2021 at 05:27:43AM +0200, Maciej W. Rozycki wrote:
>> -- a lot of sharing and swizzling here. :) You'd most definitely need:
>> <https://lore.kernel.org/patchwork/patch/1454747/> for that though, as I
>> can't imagine PCI BIOS 2.1 PIRQ routers to commonly enumerate devices
>> behind PCI-to-PCI bridges, given that they fail to cope with more complex
>> bus topologies created by option devices in the first place.
>
> Looks nicely done but I have no ability to review or test, so I assume
> the x86 folks will take care of this.

I can review it and pick it up, but for testing I have to rely on the
reporter/submitters.

Thanks,

tglx

Subject: [tip: x86/irq] x86: Avoid magic number with ELCR register accesses

The following commit has been merged into the x86/irq branch of tip:

Commit-ID: d25316616842b593de6f89ce2101f1af62f4d559
Gitweb: https://git.kernel.org/tip/d25316616842b593de6f89ce2101f1af62f4d559
Author: Maciej W. Rozycki <[email protected]>
AuthorDate: Tue, 20 Jul 2021 05:28:09 +02:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Tue, 10 Aug 2021 23:31:43 +02:00

x86: Avoid magic number with ELCR register accesses

Define PIC_ELCR1 and PIC_ELCR2 macros for accesses to the ELCR registers
implemented by many chipsets in their embedded 8259A PIC cores, avoiding
magic numbers that are difficult to handle, and complementing the macros
we already have for registers originally defined with discrete 8259A PIC
implementations. No functional change.

Signed-off-by: Maciej W. Rozycki <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

---
arch/x86/include/asm/i8259.h | 2 ++
arch/x86/kernel/acpi/boot.c | 6 +++---
arch/x86/kernel/apic/io_apic.c | 2 +-
arch/x86/kernel/apic/vector.c | 2 +-
arch/x86/kernel/i8259.c | 8 ++++----
arch/x86/kernel/mpparse.c | 3 ++-
arch/x86/pci/irq.c | 3 ++-
7 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/i8259.h b/arch/x86/include/asm/i8259.h
index 89789e8..637fa1d 100644
--- a/arch/x86/include/asm/i8259.h
+++ b/arch/x86/include/asm/i8259.h
@@ -19,6 +19,8 @@ extern unsigned int cached_irq_mask;
#define PIC_MASTER_OCW3 PIC_MASTER_ISR
#define PIC_SLAVE_CMD 0xa0
#define PIC_SLAVE_IMR 0xa1
+#define PIC_ELCR1 0x4d0
+#define PIC_ELCR2 0x4d1

/* i8259A PIC related value */
#define PIC_CASCADE_IR 2
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index e55e0c1..7f59f83 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -570,7 +570,7 @@ void __init acpi_pic_sci_set_trigger(unsigned int irq, u16 trigger)
unsigned int old, new;

/* Real old ELCR mask */
- old = inb(0x4d0) | (inb(0x4d1) << 8);
+ old = inb(PIC_ELCR1) | (inb(PIC_ELCR2) << 8);

/*
* If we use ACPI to set PCI IRQs, then we should clear ELCR
@@ -596,8 +596,8 @@ void __init acpi_pic_sci_set_trigger(unsigned int irq, u16 trigger)
return;

pr_warn("setting ELCR to %04x (from %04x)\n", new, old);
- outb(new, 0x4d0);
- outb(new >> 8, 0x4d1);
+ outb(new, PIC_ELCR1);
+ outb(new >> 8, PIC_ELCR2);
}

int acpi_gsi_to_irq(u32 gsi, unsigned int *irqp)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index d5c691a..7846499 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -764,7 +764,7 @@ static bool irq_active_low(int idx)
static bool EISA_ELCR(unsigned int irq)
{
if (irq < nr_legacy_irqs()) {
- unsigned int port = 0x4d0 + (irq >> 3);
+ unsigned int port = PIC_ELCR1 + (irq >> 3);
return (inb(port) >> (irq & 7)) & 1;
}
apic_printk(APIC_VERBOSE, KERN_INFO
diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index fb67ed5..c132daa 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -1299,7 +1299,7 @@ static void __init print_PIC(void)

pr_debug("... PIC ISR: %04x\n", v);

- v = inb(0x4d1) << 8 | inb(0x4d0);
+ v = inb(PIC_ELCR2) << 8 | inb(PIC_ELCR1);
pr_debug("... PIC ELCR: %04x\n", v);
}

diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c
index 282b4ee..15aefa3 100644
--- a/arch/x86/kernel/i8259.c
+++ b/arch/x86/kernel/i8259.c
@@ -235,15 +235,15 @@ static char irq_trigger[2];
*/
static void restore_ELCR(char *trigger)
{
- outb(trigger[0], 0x4d0);
- outb(trigger[1], 0x4d1);
+ outb(trigger[0], PIC_ELCR1);
+ outb(trigger[1], PIC_ELCR2);
}

static void save_ELCR(char *trigger)
{
/* IRQ 0,1,2,8,13 are marked as reserved */
- trigger[0] = inb(0x4d0) & 0xF8;
- trigger[1] = inb(0x4d1) & 0xDE;
+ trigger[0] = inb(PIC_ELCR1) & 0xF8;
+ trigger[1] = inb(PIC_ELCR2) & 0xDE;
}

static void i8259A_resume(void)
diff --git a/arch/x86/kernel/mpparse.c b/arch/x86/kernel/mpparse.c
index 8f06449..fed721f 100644
--- a/arch/x86/kernel/mpparse.c
+++ b/arch/x86/kernel/mpparse.c
@@ -19,6 +19,7 @@
#include <linux/smp.h>
#include <linux/pci.h>

+#include <asm/i8259.h>
#include <asm/io_apic.h>
#include <asm/acpi.h>
#include <asm/irqdomain.h>
@@ -251,7 +252,7 @@ static int __init ELCR_trigger(unsigned int irq)
{
unsigned int port;

- port = 0x4d0 + (irq >> 3);
+ port = PIC_ELCR1 + (irq >> 3);
return (inb(port) >> (irq & 7)) & 1;
}

diff --git a/arch/x86/pci/irq.c b/arch/x86/pci/irq.c
index b937c96..97b63e3 100644
--- a/arch/x86/pci/irq.c
+++ b/arch/x86/pci/irq.c
@@ -18,6 +18,7 @@
#include <linux/irq.h>
#include <linux/acpi.h>

+#include <asm/i8259.h>
#include <asm/pc-conf-reg.h>
#include <asm/pci_x86.h>

@@ -158,7 +159,7 @@ static void __init pirq_peer_trick(void)
void elcr_set_level_irq(unsigned int irq)
{
unsigned char mask = 1 << (irq & 7);
- unsigned int port = 0x4d0 + (irq >> 3);
+ unsigned int port = PIC_ELCR1 + (irq >> 3);
unsigned char val;
static u16 elcr_irq_mask;

Subject: [tip: x86/irq] x86: Fix typo s/ECLR/ELCR/ for the PIC register

The following commit has been merged into the x86/irq branch of tip:

Commit-ID: 34739a2809e1e5d54d41d93cfc6b074e8d781ee2
Gitweb: https://git.kernel.org/tip/34739a2809e1e5d54d41d93cfc6b074e8d781ee2
Author: Maciej W. Rozycki <[email protected]>
AuthorDate: Tue, 20 Jul 2021 05:28:15 +02:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Tue, 10 Aug 2021 23:31:44 +02:00

x86: Fix typo s/ECLR/ELCR/ for the PIC register

The proper spelling for the acronym referring to the Edge/Level Control
Register is ELCR rather than ECLR. Adjust references accordingly. No
functional change.

Signed-off-by: Maciej W. Rozycki <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

---
arch/x86/kernel/acpi/boot.c | 6 +++---
arch/x86/kvm/i8259.c | 20 ++++++++++----------
arch/x86/kvm/irq.h | 2 +-
3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 7f59f83..14bcd59 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -558,10 +558,10 @@ acpi_parse_nmi_src(union acpi_subtable_headers * header, const unsigned long end
* If a PIC-mode SCI is not recognized or gives spurious IRQ7's
* it may require Edge Trigger -- use "acpi_sci=edge"
*
- * Port 0x4d0-4d1 are ECLR1 and ECLR2, the Edge/Level Control Registers
+ * Port 0x4d0-4d1 are ELCR1 and ELCR2, the Edge/Level Control Registers
* for the 8259 PIC. bit[n] = 1 means irq[n] is Level, otherwise Edge.
- * ECLR1 is IRQs 0-7 (IRQ 0, 1, 2 must be 0)
- * ECLR2 is IRQs 8-15 (IRQ 8, 13 must be 0)
+ * ELCR1 is IRQs 0-7 (IRQ 0, 1, 2 must be 0)
+ * ELCR2 is IRQs 8-15 (IRQ 8, 13 must be 0)
*/

void __init acpi_pic_sci_set_trigger(unsigned int irq, u16 trigger)
diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index 629a09c..0b80263 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -541,17 +541,17 @@ static int picdev_slave_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
addr, len, val);
}

-static int picdev_eclr_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
+static int picdev_elcr_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
gpa_t addr, int len, const void *val)
{
- return picdev_write(container_of(dev, struct kvm_pic, dev_eclr),
+ return picdev_write(container_of(dev, struct kvm_pic, dev_elcr),
addr, len, val);
}

-static int picdev_eclr_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
+static int picdev_elcr_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
gpa_t addr, int len, void *val)
{
- return picdev_read(container_of(dev, struct kvm_pic, dev_eclr),
+ return picdev_read(container_of(dev, struct kvm_pic, dev_elcr),
addr, len, val);
}

@@ -577,9 +577,9 @@ static const struct kvm_io_device_ops picdev_slave_ops = {
.write = picdev_slave_write,
};

-static const struct kvm_io_device_ops picdev_eclr_ops = {
- .read = picdev_eclr_read,
- .write = picdev_eclr_write,
+static const struct kvm_io_device_ops picdev_elcr_ops = {
+ .read = picdev_elcr_read,
+ .write = picdev_elcr_write,
};

int kvm_pic_init(struct kvm *kvm)
@@ -602,7 +602,7 @@ int kvm_pic_init(struct kvm *kvm)
*/
kvm_iodevice_init(&s->dev_master, &picdev_master_ops);
kvm_iodevice_init(&s->dev_slave, &picdev_slave_ops);
- kvm_iodevice_init(&s->dev_eclr, &picdev_eclr_ops);
+ kvm_iodevice_init(&s->dev_elcr, &picdev_elcr_ops);
mutex_lock(&kvm->slots_lock);
ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, 0x20, 2,
&s->dev_master);
@@ -613,7 +613,7 @@ int kvm_pic_init(struct kvm *kvm)
if (ret < 0)
goto fail_unreg_2;

- ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, 0x4d0, 2, &s->dev_eclr);
+ ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, 0x4d0, 2, &s->dev_elcr);
if (ret < 0)
goto fail_unreg_1;

@@ -647,7 +647,7 @@ void kvm_pic_destroy(struct kvm *kvm)
mutex_lock(&kvm->slots_lock);
kvm_io_bus_unregister_dev(vpic->kvm, KVM_PIO_BUS, &vpic->dev_master);
kvm_io_bus_unregister_dev(vpic->kvm, KVM_PIO_BUS, &vpic->dev_slave);
- kvm_io_bus_unregister_dev(vpic->kvm, KVM_PIO_BUS, &vpic->dev_eclr);
+ kvm_io_bus_unregister_dev(vpic->kvm, KVM_PIO_BUS, &vpic->dev_elcr);
mutex_unlock(&kvm->slots_lock);

kvm->arch.vpic = NULL;
diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index 9b64abf..650642b 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -55,7 +55,7 @@ struct kvm_pic {
int output; /* intr from master PIC */
struct kvm_io_device dev_master;
struct kvm_io_device dev_slave;
- struct kvm_io_device dev_eclr;
+ struct kvm_io_device dev_elcr;
void (*ack_notifier)(void *opaque, int irq);
unsigned long irq_states[PIC_NUM_PINS];
};

2021-08-15 22:12:54

by Nikolai Zhubr

[permalink] [raw]
Subject: Re: [PATCH 0/6] x86: PIRQ/ELCR-related fixes and updates

Hello Maciej,

20.07.2021 6:27, Maciej W. Rozycki:
[...]
> Nikolai: for your system only 1/6 and 2/6 are required, though you are
> free to experiment with all the patches. Mind that 3/6 mechanically
> depends on the earlier change for the SIO PIRQ router referred above. In
> any case please use the debug patch for PCI code as well as the earlier
> patches for your other system and send the resulting bootstrap log for
> confirmation.

Here is a new log with 1/6 and 2/6 applied:

https://pastebin.com/0MgXAGtG

It looks like something went a bit unexpected ("runtime IRQ mapping not
provided by arch").


Thank you,

Regards,
Nikolai

2021-08-16 22:31:58

by Maciej W. Rozycki

[permalink] [raw]
Subject: Re: [PATCH 0/6] x86: PIRQ/ELCR-related fixes and updates

Hi Nikolai,

> > Nikolai: for your system only 1/6 and 2/6 are required, though you are
> > free to experiment with all the patches. Mind that 3/6 mechanically
> > depends on the earlier change for the SIO PIRQ router referred above. In
> > any case please use the debug patch for PCI code as well as the earlier
> > patches for your other system and send the resulting bootstrap log for
> > confirmation.
>
> Here is a new log with 1/6 and 2/6 applied:
>
> https://pastebin.com/0MgXAGtG
>
> It looks like something went a bit unexpected ("runtime IRQ mapping not
> provided by arch").

Offhand it looks like your system does not supply a PIRQ table, not at
least at the usual locations we look through. The presence of the table
is reported like:

PCI: IRQ init
PCI: Interrupt Routing Table found at 0xfde10
[...]
PCI: IRQ fixup

while your system says:

PCI: IRQ init
PCI: IRQ fixup

If you have a look through /dev/mem and see if there's a "$PIR" signature
somewhere (though not a Linux kernel area of course), then we may know for
sure.

I'm a little busy at the moment with other stuff and may not be able to
look into it properly right now. There may be no solution, not at least
an easy one. A DMI quirk is not possible, because:

DMI not present or invalid.

There is a PCI BIOS:

PCI: PCI BIOS revision 2.10 entry at 0xf6f41, last bus=0

however, so CONFIG_PCI_BIOS just might work. Please try that too, by
choosing CONFIG_PCI_GOANY or CONFIG_PCI_GOBIOS (it may break things
horribly though I imagine).

Maciej