Hi All,
This patch series mainly adds a boot time interrupt delivery mode
configuration option for OF based x86 platforms. Presently,
boot time interrupt delivery mode is hardcoded to legacy PIC mode
with no option to configure it to virtual wire mode. This patch
series aims to extend it by introducing a new optional boolean
property for lapic devicetree node which can be used to configure
it to virtual wire mode where applicable. Please find below detailed
rationale behind it.
Rationale:
References [1], [2] & [6]
For SMP systems, Intel defines three (logically four) interrupt modes
during boot/init time while BIOS/bootloader boots & switches to linux
kernel.
1. PIC mode - Legacy 8259 PIC interrupt controller.
2. Virtual wire mode via Local APIC - uses local APIC as virtual wire
3. Virtual wire mode via I/O APIC - uses I/O APIC as virtual wire
4. Symmetric I/O mode - final one used by linux for SMP systems.
BIOS/bootloaders are supposed to boot in either #1 or #2 or #3 and then
switch to #4 in linux for SMP systems.
For our platform, we use #2.
Detection of which interrupt mode the system is booting in is made by using
below global variable in apic.c
int pic_mode __ro_after_init;
Here pic_mode = 1 means #1 (PIC mode) above.
And pic_mode = 0 means #2 or #3 (basically virtual wire mode via apic).
And apic.c while doing setup_local_APIC() uses below code [3]:
value = apic_read(APIC_LVT0) & APIC_LVT_MASKED;
if (!cpu && (pic_mode || !value || skip_ioapic_setup)) {
value = APIC_DM_EXTINT;
apic_printk(APIC_VERBOSE, "enabled ExtINT on CPU#%d\n", cpu);
} else {
value = APIC_DM_EXTINT | APIC_LVT_MASKED;
apic_printk(APIC_VERBOSE, "masked ExtINT on CPU#%d\n", cpu);
}
apic_write(APIC_LVT0, value);
What i understand from above is that if at this point of time, as long as
it is cpu0 & pic_mode=1, it will set delivery mode to ExtINT (causes the
processor to respond to the interrupt as if the interrupt originated in an
externally connected (8259A-compatible) interrupt controller) and enables/
unmask the interrupts. This causes kernel boot crash for platforms which
does not support 8259 compatible external PIC.
pic_mode is presently set/populated/initialized at only two places:
1. In mpparse.c [4]
2. In devicetree.c [7]
For #1 MPPARSE Kconfig definition is as below:
config X86_MPPARSE
bool "Enable MPS table" if ACPI
default y
depends on X86_LOCAL_APIC
help
For old smp systems that do not have proper acpi support. Newer systems
(esp with 64bit cpus) with acpi support, MADT and DSDT will override it
As seen above, if ACPI is not enabled, then mpparse by default is always
enabled. Presently, there is no way to disable MPPARSE (if ACPI is not
enabled). This to me appears to be a bug which needs fixing. As per
theory, MPPARSE was to support MPS spec [1] as a temporary solution to
support SMP systems until a final ACPI standard was added. But now if ACPI
is not enabled, it will rely on MPPARSE driver to read MP floating pointer
structure's IMCRP Bit 7 of MP feature info byte 2 [5] to figure out if it
supports PIC mode or virtual wire mode and initialize pic_mode variable
accordingly. If ACPI is enabled, the ACPI code overrides it by using the
MADT table spec'ed in ACPI spec [2].
For #2 devicetree.c presently hardcodes pic_mode = 1 (PIC Mode). There is
no support to configure virtual wire mode via devicetree path for OF based
systems.
Now we have a platform which is OF based & does not use legacy 8259 PIC
interrupt controller. Non ACPI compliant as well as non MPPARSE compliant.
For such platforms, it appears to me that hardcoding pic_mode = 1 (PIC Mode)
and giving no other choice to choose virtual wire mode is a lacking feature.
Just like mpparse relies on IMCRP bit 7 of MP feature info byte2 [5] to
select pic_mode to PIC mode or virtual wire mode. arch/x86/kernel/devicetree.c
should also provide some similar configurability to choose interrupt
delivery mode & not hardcode it to PIC mode.
This patch is to add above mentioned interrupt mode configurability in x86/of
controlled via a new optional bool property.
Please let me know if you find any mistake in above understanding or if you
have a alternative better suggestion to solve it or if you find anything odd
here in our platform/system. TIA.
The patch is baselined on below git tree (linux-v6.1.0-rc6):
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
[1] https://pdos.csail.mit.edu/6.828/2008/readings/ia32/MPspec.pdf
[2] https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf
[3] https://elixir.bootlin.com/linux/v6.1-rc5/source/arch/x86/kernel/apic/apic.c#L1691
[4] https://elixir.bootlin.com/linux/v6.1-rc5/source/arch/x86/kernel/mpparse.c#L517
[5] https://www.manualslib.com/manual/77733/Intel-Multiprocessor.html?page=40#manual
[6] https://www.intel.com/content/www/us/en/developer/articles/technical/intel-sdm.html
[7] https://elixir.bootlin.com/linux/v6.1-rc5/source/arch/x86/kernel/devicetree.c#L170
v5:
- Fix make dt_binding_check error.
- Address review concerns from Rob Herring
* Add interrupt-controller & #interrupt-cells properties in lapic binding schema.
* Fix some indentation issues.
- Add Reviewed-by tag from Andy for patch 4/4.
v4:
- Address review concerns from Andy Shevchenko
* Update maintainers in binding files.
* Place URL in YAML schema properly as reference.
* Remove some unnecessary comments from YAML description.
* Remove fixes tag & not treat it as a bug. Treat it as new feature addition instead.
* Use proper prefixes for bindings file (dt-bindings: x86: ioapic:)
* Add Reviewed-by tag from Andy for patch 3/4.
v3:
- Address review concerns from Andy Shevchenko
* Reshuffle patch series changes to make it more logical.
* Patch 1 just converts existing intel,ce4100-ioapic.txt into
YAML schema and separates out ioapic & lapic.
* Patch 2 adds new optional property for lapic.
* Patch 3 replaces older printk(KERN_LVL) to newer pr_lvl()
* Patch 4 adds code changes in devicetree.c to support newly
added property.
- Fix 'make DT_CHECKER_FLAGS=-m dt_binding_check' errors reported
by Rob Herring's bot.
v2:
- Address review concern from Andy - rename property name to make
it a bit more positive & self explanatory.
- Found that the bindings document for these HW's (APIC) are a bit
off/obsolete and still in text format. Created new YAML schemas
one for each - lapic & ioapic. Updated these schemas with latest
info and add in new optional property details in the updated
schema for lapic. Delete/let go of the text binding doc.
- CC [email protected] as these changes appear to be
mainly targeted for devicetree maintainers review & approval.
- Increase CCed list to include all possible people who touched
and were involved this part of code/feature addition.
v1:
- Initial draft
Rahul Tanwar (4):
dt-bindings: x86: apic: Convert Intel's APIC bindings to YAML schema
dt-bindings: x86: apic: Introduce new optional bool property for lapic
x86/of: Replace printk(KERN_LVL) with pr_lvl()
x86/of: Add support for boot time interrupt delivery mode
configuration
.../intel,ce4100-ioapic.txt | 26 -------
.../intel,ce4100-ioapic.yaml | 60 ++++++++++++++++
.../intel,ce4100-lapic.yaml | 71 +++++++++++++++++++
arch/x86/kernel/devicetree.c | 13 +++-
4 files changed, 141 insertions(+), 29 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
--
2.17.1
Presently, init/boot time interrupt delivery mode is enumerated
only for ACPI enabled systems by parsing MADT table or for older
systems by parsing MP table. But for OF based x86 systems, it is
assumed & hardcoded to legacy PIC mode. This causes boot time crash
for platforms which do not use 8259 compliant legacy PIC.
Add support for configuration of init time interrupt delivery mode
for x86 OF based systems by introducing a new optional boolean
property 'intel,virtual-wire-mode' for interrupt-controller node
of local APIC. This property emulates IMCRP Bit 7 of MP feature
info byte 2 of MP floating pointer structure.
Defaults to legacy PIC mode if absent. Configures it to virtual
wire compatibility mode if present.
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Rahul Tanwar <[email protected]>
---
arch/x86/kernel/devicetree.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index fcc6f1b7818f..458e43490414 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -167,7 +167,14 @@ static void __init dtb_lapic_setup(void)
return;
}
smp_found_config = 1;
- pic_mode = 1;
+ if (of_property_read_bool(dn, "intel,virtual-wire-mode")) {
+ pr_info("Virtual Wire compatibility mode.\n");
+ pic_mode = 0;
+ } else {
+ pr_info("IMCR and PIC compatibility mode.\n");
+ pic_mode = 1;
+ }
+
register_lapic_address(lapic_addr);
}
--
2.17.1
Intel defines a few possible interrupt delivery modes. With respect
to boot/init time, mainly two interrupt delivery modes are possible.
PIC Mode - Legacy external 8259 compliant PIC interrupt controller.
Virtual Wire Mode - use lapic as virtual wire interrupt delivery mode.
For ACPI or MPS spec compliant systems, it is figured out by some read
only bit field/s available in their respective defined data structures.
But for OF based systems, it is by default set to PIC mode. Presently,
it is hardcoded to legacy PIC mode for OF based x86 systems with no
option to choose the configuration between PIC mode & virtual wire mode.
For this purpose, introduce a new boolean property for interrupt
controller node of lapic which can allow it to be configured to virtual
wire mode as well.
Property name: 'intel,virtual-wire-mode'
Type: Boolean
If not present/not defined, interrupt delivery mode defaults to legacy PIC
mode. If present/defined, interrupt delivery mode is set to virtual wire
mode.
Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: Rahul Tanwar <[email protected]>
---
.../interrupt-controller/intel,ce4100-lapic.yaml | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
index 55184cb49432..d2d0145cb889 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
@@ -39,6 +39,19 @@ properties:
'#interrupt-cells':
const: 2
+ intel,virtual-wire-mode:
+ description: Intel defines a few possible interrupt delivery
+ modes. With respect to boot/init time, mainly two interrupt
+ delivery modes are possible.
+ PIC Mode - Legacy external 8259 compliant PIC interrupt controller.
+ Virtual Wire Mode - use lapic as virtual wire interrupt delivery mode.
+ For ACPI or MPS spec compliant systems, it is figured out by some read
+ only bit field/s available in their respective defined data structures.
+ For OF based systems, it is by default set to PIC mode.
+ But if this optional boolean property is set, then the interrupt delivery
+ mode is configured to virtual wire compatibility mode.
+ type: boolean
+
required:
- compatible
- reg
@@ -54,4 +67,5 @@ examples:
reg = <0xfee00000 0x1000>;
interrupt-controller;
#interrupt-cells = <2>;
+ intel,virtual-wire-mode;
};
--
2.17.1
On Thu, 24 Nov 2022 16:41:41 +0800, Rahul Tanwar wrote:
> Intel defines a few possible interrupt delivery modes. With respect
> to boot/init time, mainly two interrupt delivery modes are possible.
> PIC Mode - Legacy external 8259 compliant PIC interrupt controller.
> Virtual Wire Mode - use lapic as virtual wire interrupt delivery mode.
>
> For ACPI or MPS spec compliant systems, it is figured out by some read
> only bit field/s available in their respective defined data structures.
> But for OF based systems, it is by default set to PIC mode. Presently,
> it is hardcoded to legacy PIC mode for OF based x86 systems with no
> option to choose the configuration between PIC mode & virtual wire mode.
>
> For this purpose, introduce a new boolean property for interrupt
> controller node of lapic which can allow it to be configured to virtual
> wire mode as well.
>
> Property name: 'intel,virtual-wire-mode'
> Type: Boolean
>
> If not present/not defined, interrupt delivery mode defaults to legacy PIC
> mode. If present/defined, interrupt delivery mode is set to virtual wire
> mode.
>
> Suggested-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Rahul Tanwar <[email protected]>
> ---
> .../interrupt-controller/intel,ce4100-lapic.yaml | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
Reviewed-by: Rob Herring <[email protected]>
The following commit has been merged into the x86/apic branch of tip:
Commit-ID: 2833275568755eb937a52c358bf8bfa7125a463e
Gitweb: https://git.kernel.org/tip/2833275568755eb937a52c358bf8bfa7125a463e
Author: Rahul Tanwar <[email protected]>
AuthorDate: Thu, 24 Nov 2022 16:41:43 +08:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Fri, 02 Dec 2022 14:57:14 +01:00
x86/of: Add support for boot time interrupt delivery mode configuration
Presently, init/boot time interrupt delivery mode is enumerated only for
ACPI enabled systems by parsing MADT table or for older systems by parsing
MP table. But for OF based x86 systems, it is assumed & hardcoded to be
legacy PIC mode. This causes a boot time crash for platforms which do not
provide a 8259 compliant legacy PIC.
Add support for configuration of init time interrupt delivery mode for x86
OF based systems by introducing a new optional boolean property
'intel,virtual-wire-mode' for the local APIC interrupt-controller
node. This property emulates IMCRP Bit 7 of MP feature info byte 2 of MP
floating pointer structure.
Defaults to legacy PIC mode if absent. Configures it to virtual wire
compatibility mode if present.
Signed-off-by: Rahul Tanwar <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/devicetree.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 484783f..28da5dd 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -162,7 +162,14 @@ static void __init dtb_lapic_setup(void)
return;
}
smp_found_config = 1;
- pic_mode = 1;
+ if (of_property_read_bool(dn, "intel,virtual-wire-mode")) {
+ pr_info("Virtual Wire compatibility mode.\n");
+ pic_mode = 0;
+ } else {
+ pr_info("IMCR and PIC compatibility mode.\n");
+ pic_mode = 1;
+ }
+
register_lapic_address(lapic_addr);
}
The following commit has been merged into the x86/apic branch of tip:
Commit-ID: b3a9801cccefda304263b4e84e9dfe49057f3c29
Gitweb: https://git.kernel.org/tip/b3a9801cccefda304263b4e84e9dfe49057f3c29
Author: Rahul Tanwar <[email protected]>
AuthorDate: Thu, 24 Nov 2022 16:41:41 +08:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Fri, 02 Dec 2022 14:57:14 +01:00
dt-bindings: x86: apic: Introduce new optional bool property for lapic
X86 defines a few possible interrupt delivery modes. With respect to
boot/init time, mainly two interrupt delivery modes are possible.
- PIC Mode: Legacy external 8259 compliant PIC interrupt controller
- Virtual Wire Mode: Use lapic as virtual wire interrupt delivery mode
ACPI and MPS spec compliant systems provide this information, but for OF
based systems, it is by default set to PIC mode.
In fact it is hardcoded to legacy PIC mode for OF based x86 systems with no
option to choose the configuration between PIC mode & virtual wire mode.
For this purpose, introduce a new boolean property for the lapic interrupt
controller node which allows to configure it for virtual wire mode as well.
Property name: 'intel,virtual-wire-mode'
Type: Boolean
If not present/not defined, interrupt delivery mode defaults to legacy PIC
mode. If present/defined, interrupt delivery mode is set to virtual wire
mode.
Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: Rahul Tanwar <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
index 55184cb..d2d0145 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml
@@ -39,6 +39,19 @@ properties:
'#interrupt-cells':
const: 2
+ intel,virtual-wire-mode:
+ description: Intel defines a few possible interrupt delivery
+ modes. With respect to boot/init time, mainly two interrupt
+ delivery modes are possible.
+ PIC Mode - Legacy external 8259 compliant PIC interrupt controller.
+ Virtual Wire Mode - use lapic as virtual wire interrupt delivery mode.
+ For ACPI or MPS spec compliant systems, it is figured out by some read
+ only bit field/s available in their respective defined data structures.
+ For OF based systems, it is by default set to PIC mode.
+ But if this optional boolean property is set, then the interrupt delivery
+ mode is configured to virtual wire compatibility mode.
+ type: boolean
+
required:
- compatible
- reg
@@ -54,4 +67,5 @@ examples:
reg = <0xfee00000 0x1000>;
interrupt-controller;
#interrupt-cells = <2>;
+ intel,virtual-wire-mode;
};