Skip to contentSkip to conversation

x86/of: Add support for interrupt mode config for x86 OF systems

PATCH Superseded Rahul Tanwar 3 participants 9 messages Nov 23, 2022 – Nov 24, 2022

Subsystems (3)

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v4 cover x86/of: Add support for interrupt mode config for x86 OF systems

[RESEND due a missing Cc in previous send]

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

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                  | 62 ++++++++++++++++++
 .../intel,ce4100-lapic.yaml                   | 63 +++++++++++++++++++
 arch/x86/kernel/devicetree.c                  | 13 +++-
 4 files changed, 135 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
Signature · 1 line
2.17.1
#1 lore raw

PATCH v4 1/4 dt-bindings: x86: apic: Convert Intel's APIC bindings to YAML schema

Intel's APIC family of interrupt controllers support local APIC (lapic) & I/O APIC (ioapic). Convert existing bindings for lapic & ioapic from text to YAML schema. Separate lapic & ioapic schemas. Addditionally, add description which was missing in text file and add few more required standard properties which were also missing in text file.

---
 .../intel,ce4100-ioapic.txt                   | 26 --------
 .../intel,ce4100-ioapic.yaml                  | 62 +++++++++++++++++++
 .../intel,ce4100-lapic.yaml                   | 49 +++++++++++++++
 3 files changed, 111 insertions(+), 26 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
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt deleted +0 -26
deleted file mode 100644
index 7d19f494f19a..000000000000
@@ -1,26 +0,0 @@
-Interrupt chips
----------------
-
-* Intel I/O Advanced Programmable Interrupt Controller (IO APIC)
-
-  Required properties:
-  --------------------
-     compatible = "intel,ce4100-ioapic";
-     #interrupt-cells = <2>;
-
-  Device's interrupt property:
-
-     interrupts = <P S>;
-
-  The first number (P) represents the interrupt pin which is wired to the
-  IO APIC. The second number (S) represents the sense of interrupt which
-  should be configured and can be one of:
-    0 - Edge Rising
-    1 - Level Low
-    2 - Level High
-    3 - Edge Falling
-
-* Local APIC
-  Required property:
-
-     compatible = "intel,ce4100-lapic";
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml new file +62 -0

62 lines not on this page · raw message

Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml new file +49 -0

49 lines not on this page · raw message

Signature · 1 line
2.17.1
maintainer
#2 lore raw

Re: [PATCH v4 1/4] dt-bindings: x86: apic: Convert Intel's APIC bindings to YAML schema

Quoted 9 lines · Rahul Tanwar

Intel's APIC family of interrupt controllers support local APIC (lapic) & I/O APIC (ioapic). Convert existing bindings for lapic & ioapic from text to YAML schema. Separate lapic & ioapic schemas. Addditionally, add description which was missing in text file and add few more required standard properties which were also missing in text file.

Quoted 9 lines
---
 .../intel,ce4100-ioapic.txt                   | 26 --------
 .../intel,ce4100-ioapic.yaml                  | 62 +++++++++++++++++++
 .../intel,ce4100-lapic.yaml                   | 49 +++++++++++++++
 3 files changed, 111 insertions(+), 26 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

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.example.dtb: interrupt-controller@fec00000: '#address-cells' does not match any of the regexes: 'pinctrl-[0-9]+'
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command.

maintainer
#3 lore raw

Re: [PATCH v4 1/4] dt-bindings: x86: apic: Convert Intel's APIC bindings to YAML schema

In reply to Rahul Tanwar
Quoted 58 lines · Rahul Tanwar

Intel's APIC family of interrupt controllers support local APIC (lapic) & I/O APIC (ioapic). Convert existing bindings for lapic & ioapic from text to YAML schema. Separate lapic & ioapic schemas. Addditionally, add description which was missing in text file and add few more required standard properties which were also missing in text file.

---
 .../intel,ce4100-ioapic.txt                   | 26 --------
 .../intel,ce4100-ioapic.yaml                  | 62 +++++++++++++++++++
 .../intel,ce4100-lapic.yaml                   | 49 +++++++++++++++
 3 files changed, 111 insertions(+), 26 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
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt deleted +0 -26
deleted file mode 100644
index 7d19f494f19a..000000000000
@@ -1,26 +0,0 @@
-Interrupt chips
----------------
-
-* Intel I/O Advanced Programmable Interrupt Controller (IO APIC)
-
-  Required properties:
-  --------------------
-     compatible = "intel,ce4100-ioapic";
-     #interrupt-cells = <2>;
-
-  Device's interrupt property:
-
-     interrupts = <P S>;
-
-  The first number (P) represents the interrupt pin which is wired to the
-  IO APIC. The second number (S) represents the sense of interrupt which
-  should be configured and can be one of:
-    0 - Edge Rising
-    1 - Level Low
-    2 - Level High
-    3 - Edge Falling
-
-* Local APIC
-  Required property:
-
-     compatible = "intel,ce4100-lapic";
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml new file +2 -0
new file mode 100644
index 000000000000..25d549220c2a
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2

Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml line 12

Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml new file +10 -0
+---
+$id: "http://devicetree.org/schemas/interrupt-controller/intel,ce4100-ioapic.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Intel I/O Advanced Programmable Interrupt Controller (IO APIC)
+
+maintainers:
+  - Rahul Tanwar <[email protected]>
+
+

One blank line.

Quoted 94 lines
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml new file +50 -0
+description: |
+  Intel's Advanced Programmable Interrupt Controller (APIC) is a
+  family of interrupt controllers. The APIC is a split
+  architecture design, with a local component (LAPIC) integrated
+  into the processor itself and an external I/O APIC. Local APIC
+  (lapic) receives interrupts from the processor's interrupt pins,
+  from internal sources and from an external I/O APIC (ioapic).
+  And it sends these to the processor core for handling.
+  See [1] Chapter 8 for more details.
+
+  Many of the Intel's generic devices like hpet, ioapic, lapic have
+  the ce4100 name in their compatible property names because they
+  first appeared in CE4100 SoC.
+
+  This schema defines bindings for I/O APIC interrupt controller.
+
+  [1] https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf
+
+properties:
+  compatible:
+    const: intel,ce4100-ioapic
+
+  reg:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - '#interrupt-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    ioapic1: interrupt-controller@fec00000 {
+        compatible = "intel,ce4100-ioapic";
+        reg = <0xfec00000 0x1000>;
+        #interrupt-cells = <2>;
+        #address-cells = <0>;
+        interrupt-controller;
+    };
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml new file +38 -0
new file mode 100644
index 000000000000..88f320ef4616
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/interrupt-controller/intel,ce4100-lapic.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Intel Local Advanced Programmable Interrupt Controller (LAPIC)
+
+maintainers:
+  - Rahul Tanwar <[email protected]>
+
+
+description: |
+  Intel's Advanced Programmable Interrupt Controller (APIC) is a
+  family of interrupt controllers. The APIC is a split
+  architecture design, with a local component (LAPIC) integrated
+  into the processor itself and an external I/O APIC. Local APIC
+  (lapic) receives interrupts from the processor's interrupt pins,
+  from internal sources and from an external I/O APIC (ioapic).
+  And it sends these to the processor core for handling.
+  See [1] Chapter 8 for more details.
+
+  Many of the Intel's generic devices like hpet, ioapic, lapic have
+  the ce4100 name in their compatible property names because they
+  first appeared in CE4100 SoC.
+
+  This schema defines bindings for local APIC interrupt controller.
+
+  [1] https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf
+
+properties:
+  compatible:
+    const: intel,ce4100-lapic
+
+  reg:
+    maxItems: 1
+
+required:

Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml line 48

Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml new file +10 -0
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    lapic0: interrupt-controller@fee00000 {
+        compatible = "intel,ce4100-lapic";
+        reg = <0xfee00000 0x1000>;

What about interrupt-controller and #interrupt-cells properties?

Quoted 5 lines
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml new file +1 -0
+    };
Signature · 1 line
2.17.1
#4 lore raw

Re: [PATCH v4 1/4] dt-bindings: x86: apic: Convert Intel's APIC bindings to YAML schema

Quoted 73 lines · Rob Herring

This email was sent from outside of MaxLinear.

Quoted 7 lines · Rahul Tanwar

Intel's APIC family of interrupt controllers support local APIC (lapic) & I/O APIC (ioapic). Convert existing bindings for lapic & ioapic from text to YAML schema. Separate lapic & ioapic schemas. Addditionally, add description which was missing in text file and add few more required standard properties which were also missing in text file.

Quoted 8 lines

---
.../intel,ce4100-ioapic.txt | 26 -------- .../intel,ce4100-ioapic.yaml | 62 +++++++++++++++++++ .../intel,ce4100-lapic.yaml | 49 +++++++++++++++

3 files changed, 111 insertions(+), 26 deletions(-)

delete mode 100644

Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt

Quoted 1 line

create mode 100644

Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml

Quoted 1 line

create mode 100644

Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml

Review

Quoted 2 lines
(quoted diff) +0 -0

a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt

Quoted 3 lines

deleted file mode 100644
index 7d19f494f19a..000000000000
---

a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt

Review

Quoted 29 lines

+++ /dev/null

Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.txt deleted +0 -26
@@ -1,26 +0,0 @@
-Interrupt chips
----------------
-
-* Intel I/O Advanced Programmable Interrupt Controller (IO APIC)
-
- Required properties:
- --------------------
- compatible = "intel,ce4100-ioapic";
- #interrupt-cells = <2>;
-
- Device's interrupt property:
-
- interrupts = <P S>;
-
- The first number (P) represents the interrupt pin which is wired to the
- IO APIC. The second number (S) represents the sense of interrupt which
- should be configured and can be one of:
- 0 - Edge Rising
- 1 - Level Low
- 2 - Level High
- 3 - Edge Falling
-
-* Local APIC
- Required property:
-
- compatible = "intel,ce4100-lapic";
(quoted diff) +0 -0

a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml

Quoted 4 lines

new file mode 100644
index 000000000000..25d549220c2a
--- /dev/null
+++

b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml

Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml line 4

Quoted 5 lines
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml new file +4 -0
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: 
"http://devicetree.org/schemas/interrupt-controller/intel,ce4100-ioapic.yaml# <http://devicetree.org/schemas/interrupt-controller/intel,ce4100-ioapic.yaml#>"

Review line 5

Quoted 1 line
(quoted diff) +1 -0
+$schema: "http://devicetree.org/meta-schemas/core.yaml# 

<http://devicetree.org/meta-schemas/core.yaml#>"

Quoted 9 lines

Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml line 12

Quoted 7 lines
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml new file +7 -0
+
+title: Intel I/O Advanced Programmable Interrupt Controller (IO APIC)
+
+maintainers:
+ - Rahul Tanwar <[email protected]>
+
+

One blank line.

Well noted.

Quoted 105 lines
Quoted 8 lines
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml new file +7 -0
+description: |
+ Intel's Advanced Programmable Interrupt Controller (APIC) is a
+ family of interrupt controllers. The APIC is a split
+ architecture design, with a local component (LAPIC) integrated
+ into the processor itself and an external I/O APIC. Local APIC
+ (lapic) receives interrupts from the processor's interrupt pins,
+ from internal sources and from an external I/O APIC (ioapic).

Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml line 29

Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml new file +10 -0
+ And it sends these to the processor core for handling.
+ See [1] Chapter 8 for more details.
+
+ Many of the Intel's generic devices like hpet, ioapic, lapic have
+ the ce4100 name in their compatible property names because they
+ first appeared in CE4100 SoC.
+
+ This schema defines bindings for I/O APIC interrupt controller.
+
+ [1] https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf 

<https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf>

Review

Quoted 34 lines
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml new file +33 -0
+
+properties:
+ compatible:
+ const: intel,ce4100-ioapic
+
+ reg:
+ maxItems: 1
+
+ interrupt-controller: true
+
+ '#interrupt-cells':
+ const: 2
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupt-controller
+ - '#interrupt-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ ioapic1: interrupt-controller@fec00000 {
+ compatible = "intel,ce4100-ioapic";
+ reg = <0xfec00000 0x1000>;
+ #interrupt-cells = <2>;
+ #address-cells = <0>;
+ interrupt-controller;
+ };
(quoted diff) +0 -0

a/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml

Quoted 4 lines

new file mode 100644
index 000000000000..88f320ef4616
--- /dev/null
+++

b/Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml

Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml line 4

Quoted 5 lines
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-ioapic.yaml new file +4 -0
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: 
"http://devicetree.org/schemas/interrupt-controller/intel,ce4100-lapic.yaml# <http://devicetree.org/schemas/interrupt-controller/intel,ce4100-lapic.yaml#>"

Review line 5

Quoted 1 line
(quoted diff) +1 -0
+$schema: "http://devicetree.org/meta-schemas/core.yaml# 

<http://devicetree.org/meta-schemas/core.yaml#>"

Quoted 14 lines
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml new file +14 -0
+
+title: Intel Local Advanced Programmable Interrupt Controller (LAPIC)
+
+maintainers:
+ - Rahul Tanwar <[email protected]>
+
+
+description: |
+ Intel's Advanced Programmable Interrupt Controller (APIC) is a
+ family of interrupt controllers. The APIC is a split
+ architecture design, with a local component (LAPIC) integrated
+ into the processor itself and an external I/O APIC. Local APIC
+ (lapic) receives interrupts from the processor's interrupt pins,
+ from internal sources and from an external I/O APIC (ioapic).

Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml line 29

Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml new file +10 -0
+ And it sends these to the processor core for handling.
+ See [1] Chapter 8 for more details.
+
+ Many of the Intel's generic devices like hpet, ioapic, lapic have
+ the ce4100 name in their compatible property names because they
+ first appeared in CE4100 SoC.
+
+ This schema defines bindings for local APIC interrupt controller.
+
+ [1] https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf 

<https://pdos.csail.mit.edu/6.828/2008/readings/ia32/IA32-3A.pdf>

Quoted 13 lines
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml new file +13 -0
+
+properties:
+ compatible:
+ const: intel,ce4100-lapic
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
Quoted 8 lines

Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml line 48

Quoted 6 lines
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml new file +6 -0
+
+examples:
+ - |
+ lapic0: interrupt-controller@fee00000 {
+ compatible = "intel,ce4100-lapic";
+ reg = <0xfee00000 0x1000>;

What about interrupt-controller and #interrupt-cells properties?

Thanks for pointing it out. Yes, interrupt-controller & #interrupt-cells properties rightfully belong here. Will update in v5. Thanks.

Regards,
Rahul

Quoted 6 lines
Quoted 6 lines
(quoted diff) +1 -1
+ };
--

2.17.1

#5 lore raw

PATCH v4 2/4 dt-bindings: x86: apic: Introduce new optional bool property for lapic

In reply to Rahul Tanwar

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.

---
 .../interrupt-controller/intel,ce4100-lapic.yaml   | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
Documentation/devicetree/bindings/interrupt-controller/intel,ce4100-lapic.yaml +14 -0
index 88f320ef4616..ef47cb657335 100644
@@ -35,6 +35,19 @@ properties:
   reg:
     maxItems: 1
 
+  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
@@ -46,4 +59,5 @@ examples:
     lapic0: interrupt-controller@fee00000 {
         compatible = "intel,ce4100-lapic";
         reg = <0xfee00000 0x1000>;
+        intel,virtual-wire-mode;
     };
Signature · 1 line
2.17.1
#6 lore raw

PATCH v4 3/4 x86/of: Replace printk(KERN_LVL) with pr_lvl()

In reply to Rahul Tanwar

Use latest available pr_lvl() instead of older printk(KERN_LVL) Just a upgrade of print utilities usage no functional changes.

---
 arch/x86/kernel/devicetree.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kernel/devicetree.c +2 -2
index 5cd51f25f446..fcc6f1b7818f 100644
@@ -248,7 +248,7 @@ static void __init dtb_add_ioapic(struct device_node *dn)
 
 	ret = of_address_to_resource(dn, 0, &r);
 	if (ret) {
-		printk(KERN_ERR "Can't obtain address from device node %pOF.\n", dn);
+		pr_err("Can't obtain address from device node %pOF.\n", dn);
 		return;
 	}
 	mp_register_ioapic(++ioapic_id, r.start, gsi_top, &cfg);
@@ -265,7 +265,7 @@ static void __init dtb_ioapic_setup(void)
 		of_ioapic = 1;
 		return;
 	}
-	printk(KERN_ERR "Error: No information about IO-APIC in OF.\n");
+	pr_err("Error: No information about IO-APIC in OF.\n");
 }
 #else
 static void __init dtb_ioapic_setup(void) {}
Signature · 1 line
2.17.1
#7 lore raw

PATCH v4 4/4 x86/of: Add support for boot time interrupt delivery mode configuration

In reply to Rahul Tanwar

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.

---
 arch/x86/kernel/devicetree.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
arch/x86/kernel/devicetree.c +8 -1
index fcc6f1b7818f..458e43490414 100644
@@ -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);
 }
 
Signature · 1 line
2.17.1
Reviewed-by on v4 4/4
#8 lore raw

Re: [PATCH v4 4/4] x86/of: Add support for boot time interrupt delivery mode configuration

Quoted 6 lines · Rahul Tanwar

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.

Quoted 8 lines

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.

From code perspective looks good to me, but you need to have a blessing by DT people for first two patches.

With whatever property name agreed on,

Quoted 28 lines
---
 arch/x86/kernel/devicetree.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
arch/x86/kernel/devicetree.c +8 -1
index fcc6f1b7818f..458e43490414 100644
@@ -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);
 }
 
Signature · 1 line
2.17.1
Signature · 2 lines
With Best Regards,
Andy Shevchenko