Skip to contentSkip to conversation

ACPI / boot: Don't handle SCI on HW reduced platforms

PATCH Applied Andy Shevchenko 2 participants 6 messages Dec 15, 2017 – Jan 3, 2018

Subsystems (2)
x86 ACPI
lore
Share & tools
lore

Andy Shevchenko

author
#0 lore raw

v4 cover ACPI / boot: Don't handle SCI on HW reduced platforms

The series improves handling of invalid IRQs in ACPI glue layer along with preventing setup SCI on HW reduced platforms as it prescribed by the spec.

Patch 1 is just convenient to add to the series.

Patches 2 and 3 make consistent value and variable types for SCI and GSI in ACPI code.

Patch 4 prevents setting up SCI on HW reduced platforms.

The series has been tested on T100TA (HW reduced platform) and on Intel Kabylake.

In v4:
- drop upstreamed patches
- drop patch which belongs to other (future) series
- rework approach to address Rafael's comment

Andy Shevchenko (4):
ACPI / boot: Swap variables in condition in acpi_register_gsi_ioapic() ACPI / boot: Get rid of ACPI_INVALID_GSI
ACPI / boot: Use INVALID_ACPI_IRQ instead of 0 for
acpi_sci_override_gsi
ACPI / boot: Don't setup SCI on HW reduced platforms

 arch/x86/include/asm/acpi.h |  2 +-
 arch/x86/kernel/acpi/boot.c | 15 +++++++--------
 2 files changed, 8 insertions(+), 9 deletions(-)
Signature · 1 line
2.15.1
author
#1 lore raw

PATCH v4 1/4 ACPI / boot: Swap variables in condition in acpi_register_gsi_ioapic()

For better readability compare input to something considered settled down. Additionally move it to one line (while it's slightly longer 80 characters it makes readability better).

No functional change intended.

Signed-off-by Andy Shevchenko <[email protected]>
---
 arch/x86/kernel/acpi/boot.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
arch/x86/kernel/acpi/boot.c +1 -2
index 3b33662c072a..bd556b0a48f3 100644
@@ -679,8 +679,7 @@ static int acpi_register_gsi_ioapic(struct device *dev, u32 gsi,
 	mutex_lock(&acpi_ioapic_lock);
 	irq = mp_map_gsi_to_irq(gsi, IOAPIC_MAP_ALLOC, &info);
 	/* Don't set up the ACPI SCI because it's already set up */
-	if (irq >= 0 && enable_update_mptable &&
-	    acpi_gbl_FADT.sci_interrupt != gsi)
+	if (irq >= 0 && enable_update_mptable && gsi != acpi_gbl_FADT.sci_interrupt)
 		mp_config_acpi_gsi(dev, gsi, trigger, polarity);
 	mutex_unlock(&acpi_ioapic_lock);
 #endif
Signature · 1 line
2.15.1
author
#2 lore raw

PATCH v4 2/4 ACPI / boot: Get rid of ACPI_INVALID_GSI

In reply to Andy Shevchenko

The commit 49e4b84333f3
("ACPI: Use correct IRQ when uninstalling ACPI interrupt handler") brings a new definition for invalid ACPI IRQ, i.e. INVALID_ACPI_IRQ, which is defined to 0xffffffff (or -1 for unsigned value).

Get rid of former one, which was brought by the commit 2c0a6894df19 ("x86, ACPI, irq: Enhance error handling in function acpi_register_gsi()"), in favour of latter.

To clarify the rationale of changing from INT_MIN to ((unsigned)-1) definition consider the following:
- IRQ 0 is valid one in hardware, so, better not to use it everywhere (Linux uses 0 as NO IRQ, though it's another story)
- INT_MIN splits the range into two, while 0xffffffff reserves only the last item
- when type casting is done in most cases 0xff, 0xffff is naturally used as a marker of invalid HW IRQ: for example PCI INT line 0xff means no IRQ assigned by BIOS

Cc Jiang Liu <[email protected]>
Cc Chen Yu <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>
---
 arch/x86/kernel/acpi/boot.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
arch/x86/kernel/acpi/boot.c +2 -4
index bd556b0a48f3..c6a6b82882b2 100644
@@ -115,8 +115,6 @@ static u32 isa_irq_to_gsi[NR_IRQS_LEGACY] __read_mostly = {
 	0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15
 };
 
-#define	ACPI_INVALID_GSI		INT_MIN
-
 /*
  * This is just a simple wrapper around early_memremap(),
  * with sanity checks for phys == 0 and size == 0.

@@ -375,7 +373,7 @@ static void __init mp_override_legacy_irq(u8 bus_irq, u8 polarity, u8 trigger,
 	 * and acpi_isa_irq_to_gsi() may give wrong result.
 	 */
 	if (gsi < nr_legacy_irqs() && isa_irq_to_gsi[gsi] == gsi)
-		isa_irq_to_gsi[gsi] = ACPI_INVALID_GSI;
+		isa_irq_to_gsi[gsi] = INVALID_ACPI_IRQ;
 	isa_irq_to_gsi[bus_irq] = gsi;
 }
 

@@ -640,7 +638,7 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
 int acpi_isa_irq_to_gsi(unsigned isa_irq, u32 *gsi)
 {
 	if (isa_irq < nr_legacy_irqs() &&
-	    isa_irq_to_gsi[isa_irq] != ACPI_INVALID_GSI) {
+	    isa_irq_to_gsi[isa_irq] != INVALID_ACPI_IRQ) {
 		*gsi = isa_irq_to_gsi[isa_irq];
 		return 0;
 	}
Signature · 1 line
2.15.1
author
#3 lore raw

PATCH v4 3/4 ACPI / boot: Use INVALID_ACPI_IRQ instead of 0 for acpi_sci_override_gsi

In reply to Andy Shevchenko

0 is valid hardware interrupt which might be in some cases overridden. Due to this, switch to INVALID_ACPI_IRQ to mark SCI override not set.

While here, change type of the variable from int to u32 to be in align with GSI type used in the rest of the code.

Signed-off-by Andy Shevchenko <[email protected]>
---
 arch/x86/include/asm/acpi.h | 2 +-
 arch/x86/kernel/acpi/boot.c | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)
arch/x86/include/asm/acpi.h +1 -1
index 8d0ec9df1cbe..44f5d79d5105 100644
@@ -49,7 +49,7 @@ extern int acpi_fix_pin2_polarity;
 extern int acpi_disable_cmcff;
 
 extern u8 acpi_sci_flags;
-extern int acpi_sci_override_gsi;
+extern u32 acpi_sci_override_gsi;
 void acpi_pic_sci_set_trigger(unsigned int, u16);
 
 struct device;
arch/x86/kernel/acpi/boot.c +3 -2
index c6a6b82882b2..6302a931fe9b 100644
@@ -71,8 +71,9 @@ int acpi_ioapic;
 int acpi_strict;
 int acpi_disable_cmcff;
 
+/* ACPI SCI override configuration */
 u8 acpi_sci_flags __initdata;
-int acpi_sci_override_gsi __initdata;
+u32 acpi_sci_override_gsi __initdata = INVALID_ACPI_IRQ;
 int acpi_skip_timer_override __initdata;
 int acpi_use_timer_override __initdata;
 int acpi_fix_pin2_polarity __initdata;

@@ -1212,7 +1213,7 @@ static int __init acpi_parse_madt_ioapic_entries(void)
 	 * If BIOS did not supply an INT_SRC_OVR for the SCI
 	 * pretend we got one so we can set the SCI flags.
 	 */
-	if (!acpi_sci_override_gsi)
+	if (acpi_sci_override_gsi == INVALID_ACPI_IRQ)
 		acpi_sci_ioapic_setup(acpi_gbl_FADT.sci_interrupt, 0, 0,
 				      acpi_gbl_FADT.sci_interrupt);
Signature · 1 line
2.15.1
author
#4 lore raw

PATCH v4 4/4 ACPI / boot: Don't setup SCI on HW reduced platforms

In reply to Andy Shevchenko

As per note in 5.2.9 Fixed ACPI Description Table (FADT) chapter of ACPI specification OSPM will ignore fields related to the ACPI HW register interface, one of which is SCI_INT.

Follow the spec and ignore any configuration done for interrupt line defined by SCI_INT if FADT specifies HW reduced platform.

HW reduced platform would be still able to use SCI in case it provides an override record in MADT table.

Signed-off-by Andy Shevchenko <[email protected]>
---
 arch/x86/kernel/acpi/boot.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/x86/kernel/acpi/boot.c +2 -1
index 6302a931fe9b..290b6f5a2d50 100644
@@ -1212,8 +1212,9 @@ static int __init acpi_parse_madt_ioapic_entries(void)
 	/*
 	 * If BIOS did not supply an INT_SRC_OVR for the SCI
 	 * pretend we got one so we can set the SCI flags.
+	 * But ignore setting up SCI on hardware reduced platforms.
 	 */
-	if (acpi_sci_override_gsi == INVALID_ACPI_IRQ)
+	if (acpi_sci_override_gsi == INVALID_ACPI_IRQ && !acpi_gbl_reduced_hardware)
 		acpi_sci_ioapic_setup(acpi_gbl_FADT.sci_interrupt, 0, 0,
 				      acpi_gbl_FADT.sci_interrupt);
Signature · 1 line
2.15.1
Applied
#5 lore raw
In reply to Andy Shevchenko
Quoted 25 lines · CET Andy Shevchenko

The series improves handling of invalid IRQs in ACPI glue layer along with preventing setup SCI on HW reduced platforms as it prescribed by the spec.

Patch 1 is just convenient to add to the series.

Patches 2 and 3 make consistent value and variable types for SCI and GSI in ACPI code.

Patch 4 prevents setting up SCI on HW reduced platforms.

The series has been tested on T100TA (HW reduced platform) and on Intel Kabylake.

In v4:
- drop upstreamed patches
- drop patch which belongs to other (future) series
- rework approach to address Rafael's comment

Andy Shevchenko (4):
ACPI / boot: Swap variables in condition in acpi_register_gsi_ioapic() ACPI / boot: Get rid of ACPI_INVALID_GSI
ACPI / boot: Use INVALID_ACPI_IRQ instead of 0 for
acpi_sci_override_gsi
ACPI / boot: Don't setup SCI on HW reduced platforms

Quoted 5 lines
 arch/x86/include/asm/acpi.h |  2 +-
 arch/x86/kernel/acpi/boot.c | 15 +++++++--------
 2 files changed, 8 insertions(+), 9 deletions(-)

Queued up for 4.16, thanks!