[PATCH v4 0/4] ACPI / boot: Don't handle SCI on HW reduced platforms

[PATCH v4 0/4] ACPI / boot: Don't handle SCI on HW reduced platforms

[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

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

-- 
2.15.1

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

[Andy Shevchenko]

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 <andriy.shevchenko@linux.intel.com>
---
 arch/x86/kernel/acpi/boot.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 3b33662c072a..bd556b0a48f3 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -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
-- 
2.15.1

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

[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 <jiang.liu@linux.intel.com>
Cc: Chen Yu <yu.c.chen@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/kernel/acpi/boot.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index bd556b0a48f3..c6a6b82882b2 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -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;
 	}
-- 
2.15.1

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

[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 <andriy.shevchenko@linux.intel.com>
---
 arch/x86/include/asm/acpi.h | 2 +-
 arch/x86/kernel/acpi/boot.c | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index 8d0ec9df1cbe..44f5d79d5105 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
@@ -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;
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index c6a6b82882b2..6302a931fe9b 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -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);
 
-- 
2.15.1

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

[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 <andriy.shevchenko@linux.intel.com>
---
 arch/x86/kernel/acpi/boot.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 6302a931fe9b..290b6f5a2d50 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -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);
 
-- 
2.15.1

Re: [PATCH v4 0/4] ACPI / boot: Don't handle SCI on HW reduced platforms

[Rafael J. Wysocki]

On Friday, December 15, 2017 10:25:07 PM CET Andy Shevchenko wrote:
> 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(-)
> 
> 

Queued up for 4.16, thanks!