x86/devicetree: Re-enable x86-specific implementation
Subsystems (2)
v4 cover x86/devicetree: Re-enable x86-specific implementation
Fixing x86-specific DT implementation in the kernel allows reusing most of firmware code for SoC that have ARM core replaced with x86, e.g. SC9853i.
Changes since v3:
* Using fdt_totalsize() to get DTB size before remapping instead of setting initial_boot_params and calling of_get_flat_dt_size() before early_init_dt_verify();
* Adding new intel,apic-id property to the documentation.
Changes since v2:
* WARN_ON_ONCE instead of WARN_ON to aviod multiple warnings when APIC ID is missing in CPU device tree nodes
* Switched to Mutt because of white space issues:
"Preformatted" paragraph style does not work in Evolution 3.18.5.2
Changes since first version:
* Splitting a single patch into three parts
Ivan Gorinov (4):
x86/devicetree: Initialize device tree before using it x86/devicetree: Fix device IRQ settings in DT
of: Documentation: Add x86 local APIC ID property x86/devicetree: Enable multiprocessing in DT
Documentation/devicetree/bindings/x86/ce4100.txt | 6 +++ arch/x86/kernel/devicetree.c | 48 +++++++++++++++++++----- 2 files changed, 44 insertions(+), 10 deletions(-)
Signature · 1 line
2.7.4
PATCH v4 1/4 x86/devicetree: Initialize device tree before using it
Commit 08d53aa58cb162e6 ("of/fdt: export fdt blob as /sys/firmware/fdt") adds CRC32 calculation in early_init_dt_verify() and checking in late initcall of_fdt_raw_init(), making early_init_dt_verify() mandatory. Required call to early_init_dt_verify() was not added to the x86-specific implementation, causing failure to create sysfs entry in of_fdt_raw_init().
--- arch/x86/kernel/devicetree.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
arch/x86/kernel/devicetree.c +5 -3
@@ -11,6 +11,7 @@#include <linux/of_address.h> #include <linux/of_platform.h> #include <linux/of_irq.h> +#include <linux/libfdt.h> #include <linux/slab.h> #include <linux/pci.h> #include <linux/of_pci.h>
@@ -270,14 +271,15 @@ static void __init x86_flattree_get_config(void) map_len =max (PAGE_SIZE - (initial_dtb & ~PAGE_MASK ), (u64 )128 ); - initial_boot_params = dt =early_memremap (initial_dtb, map_len); - size =of_get_flat_dt_size (); + dt =early_memremap (initial_dtb, map_len); + size =fdt_totalsize (dt);if (map_len < size) {early_memunmap (dt, map_len); - initial_boot_params = dt =early_memremap (initial_dtb, size); + dt =early_memremap (initial_dtb, size); map_len = size; } +early_init_dt_verify (dt);unflatten_and_copy_device_tree ();early_memunmap (dt, map_len); }
Signature · 1 line
2.7.4
tip-bot for Ivan Gorinov
merge bot[tip:x86/platform] x86/devicetree: Initialize device tree before using it
x86/devicetree: Initialize device tree before using it
Commit-ID: 628df9dc5ad886b0a9b33c75a7b09710eb859ca1 Gitweb: https://git.kernel.org/tip/628df9dc5ad886b0a9b33c75a7b09710eb859ca1 Author: Ivan Gorinov <[email protected]> AuthorDate: Wed, 7 Mar 2018 11:46:29 -0800 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 8 Mar 2018 09:59:53 +0100
x86/devicetree: Initialize device tree before using it
Commit 08d53aa58cb1 added CRC32 calculation in early_init_dt_verify() and checking in late initcall of_fdt_raw_init(), making early_init_dt_verify() mandatory.
The required call to early_init_dt_verify() was not added to the x86-specific implementation, causing failure to create the sysfs entry in of_fdt_raw_init().
--- arch/x86/kernel/devicetree.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
arch/x86/kernel/devicetree.c +5 -3
@@ -11,6 +11,7 @@#include <linux/of_address.h> #include <linux/of_platform.h> #include <linux/of_irq.h> +#include <linux/libfdt.h> #include <linux/slab.h> #include <linux/pci.h> #include <linux/of_pci.h>
@@ -270,14 +271,15 @@ static void __init x86_flattree_get_config(void) map_len =max (PAGE_SIZE - (initial_dtb & ~PAGE_MASK ), (u64 )128 ); - initial_boot_params = dt =early_memremap (initial_dtb, map_len); - size =of_get_flat_dt_size (); + dt =early_memremap (initial_dtb, map_len); + size =fdt_totalsize (dt);if (map_len < size) {early_memunmap (dt, map_len); - initial_boot_params = dt =early_memremap (initial_dtb, size); + dt =early_memremap (initial_dtb, size); map_len = size; } +early_init_dt_verify (dt);unflatten_and_copy_device_tree ();early_memunmap (dt, map_len); }
PATCH v4 2/4 x86/devicetree: Fix device IRQ settings in DT
In reply to Ivan GorinovIRQ parameters for the SoC devices connected directly to I/O APIC lines (without PCI IRQ routing) may be specified in the Device Tree. Called from DT IRQ parser, irq_create_fwspec_mapping() calls irq_domain_alloc_irqs() with a pointer to irq_fwspec structure as @arg. But x86-specific DT IRQ allocation code casts @arg to of_phandle_args structure pointer and crashes trying to read the IRQ parameters.
--- arch/x86/kernel/devicetree.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
arch/x86/kernel/devicetree.c +8 -5
@@ -195,19 +195,22 @@ static struct of_ioapic_type of_ioapic_type[] =static int dt_irqdomain_alloc (struct irq_domain *domain,unsigned int virq,unsigned int nr_irqs,void *arg) { -struct of_phandle_args *irq_data = (void *)arg; +struct irq_fwspec *fwspec = (struct irq_fwspec *)arg;struct of_ioapic_type *it;struct irq_alloc_info tmp; +int type_index; -if (WARN_ON (irq_data->args_count <2 )) +if (WARN_ON (fwspec->param_count <2 ))return -EINVAL ; -if (irq_data->args[1 ] >=ARRAY_SIZE (of_ioapic_type)) + + type_index = fwspec->param[1 ]; +if (type_index >=ARRAY_SIZE (of_ioapic_type))return -EINVAL ; - it = &of_ioapic_type[irq_data->args[1 ]]; + it = &of_ioapic_type[type_index];ioapic_set_alloc_attr (&tmp,NUMA_NO_NODE , it->trigger, it->polarity); tmp.ioapic_id =mpc_ioapic_id (mp_irqdomain_ioapic_idx (domain)); - tmp.ioapic_pin = irq_data->args[0 ]; + tmp.ioapic_pin = fwspec->param[0 ];return mp_irqdomain_alloc (domain, virq, nr_irqs, &tmp); }
Signature · 1 line
2.7.4
tip-bot for Ivan Gorinov
merge bot[tip:x86/platform] x86/devicetree: Fix device IRQ settings in DT
x86/devicetree: Fix device IRQ settings in DT
Commit-ID: 0a5169add90e43ab45ab1ba34223b8583fcaf675 Gitweb: https://git.kernel.org/tip/0a5169add90e43ab45ab1ba34223b8583fcaf675 Author: Ivan Gorinov <[email protected]> AuthorDate: Wed, 7 Mar 2018 11:46:53 -0800 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 8 Mar 2018 09:59:53 +0100
x86/devicetree: Fix device IRQ settings in DT
IRQ parameters for the SoC devices connected directly to I/O APIC lines (without PCI IRQ routing) may be specified in the Device Tree.
Called from DT IRQ parser, irq_create_fwspec_mapping() calls irq_domain_alloc_irqs() with a pointer to irq_fwspec structure as @arg.
But x86-specific DT IRQ allocation code casts @arg to of_phandle_args structure pointer and crashes trying to read the IRQ parameters. The function was not converted when the mapping descriptor was changed to irq_fwspec in the generic irqdomain code.
--- arch/x86/kernel/devicetree.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
arch/x86/kernel/devicetree.c +8 -5
@@ -195,19 +195,22 @@ static struct of_ioapic_type of_ioapic_type[] =static int dt_irqdomain_alloc (struct irq_domain *domain,unsigned int virq,unsigned int nr_irqs,void *arg) { -struct of_phandle_args *irq_data = (void *)arg; +struct irq_fwspec *fwspec = (struct irq_fwspec *)arg;struct of_ioapic_type *it;struct irq_alloc_info tmp; +int type_index; -if (WARN_ON (irq_data->args_count <2 )) +if (WARN_ON (fwspec->param_count <2 ))return -EINVAL ; -if (irq_data->args[1 ] >=ARRAY_SIZE (of_ioapic_type)) + + type_index = fwspec->param[1 ]; +if (type_index >=ARRAY_SIZE (of_ioapic_type))return -EINVAL ; - it = &of_ioapic_type[irq_data->args[1 ]]; + it = &of_ioapic_type[type_index];ioapic_set_alloc_attr (&tmp,NUMA_NO_NODE , it->trigger, it->polarity); tmp.ioapic_id =mpc_ioapic_id (mp_irqdomain_ioapic_idx (domain)); - tmp.ioapic_pin = irq_data->args[0 ]; + tmp.ioapic_pin = fwspec->param[0 ];return mp_irqdomain_alloc (domain, virq, nr_irqs, &tmp); }
PATCH v4 3/4 of: Documentation: Add x86 local APIC ID property
In reply to Ivan GorinovCurrent x86 Device Tree implementation does not support multiprocessing. Add new "intel,apic-id" property to allow using CPU descriptions in Device Tree data provided by the U-Boot loader.
Address specified in 'reg' to be used as default local APIC ID to avoid breaking existing systems with DTB provided by firmware.
--- Documentation/devicetree/bindings/x86/ce4100.txt | 6 ++++++ 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/x86/ce4100.txt +6 -0
@@ -14,11 +14,17 @@ The CPU node compatible = "intel,ce4100"; reg = <0>; lapic = <&lapic0>; + intel,apic-id = <0>; }; The reg property describes the CPU number. The lapic property points to the local APIC timer. +Optional properties: + +- intel,apic-id: local APIC ID. + The address specified in "reg" is used as default local APIC ID. + The SoC node ------------
Signature · 1 line
2.7.4
Re: [PATCH v4 3/4] of: Documentation: Add x86 local APIC ID property
Quoted 5 lines · Ivan Gorinov
Current x86 Device Tree implementation does not support multiprocessing. Add new "intel,apic-id" property to allow using CPU descriptions in Device Tree data provided by the U-Boot loader.
Address specified in 'reg' to be used as default local APIC ID to avoid breaking existing systems with DTB provided by firmware.
Is there some reason to not always use reg? For when the numbering of cpus and timers is different?
Of course, we do have the situation on ARM with the GIC that the GIC CPU IDs may be
Quoted 11 lines
Signed-off-by Ivan Gorinov <[email protected]>--- Documentation/devicetree/bindings/x86/ce4100.txt | 6 ++++++ 1 file changed, 6 insertions(+)Documentation/devicetree/bindings/x86/ce4100.txt +0
-0
Documentation/devicetree/bindings/x86/ce4100.txt The CPU node · line 16
Quoted 3 lines
Documentation/devicetree/bindings/x86/ce4100.txt +0
-0@@ -14,11 +14,17 @@ The CPU node compatible = "intel,ce4100"; reg = <0>; lapic = <&lapic0>;
Isn't this enough? I can't tell because whatever this points to has no binding documentation.
You could perhaps extend it and add a cell with the id value.
Quoted 17 lines
Documentation/devicetree/bindings/x86/ce4100.txt +6
-1+ intel,apic-id = <0>; }; The reg property describes the CPU number. The lapic property points to the local APIC timer. +Optional properties: + +- intel,apic-id: local APIC ID. + The address specified in "reg" is used as default local APIC ID. + The SoC node ------------ --2.7.4
Re: [PATCH v4 3/4] of: Documentation: Add x86 local APIC ID property
Quoted 6 lines · Rob Herring
Quoted 4 lines
Add new "intel,apic-id" property to allow using CPU descriptions in Device Tree data provided by the U-Boot loader. Address specified in 'reg' to be used as default local APIC ID to avoid breaking existing systems with DTB provided by firmware.
Is there some reason to not always use reg? For when the numbering of cpus and timers is different?
Yes, local APIC ID may differ from CPU number.
For example, in Atom E38xx (u-boot/arch/x86/dts/minnowmax.dts):
cpus {
#address-cells = <1>;
#size-cells = <0>;
cpu@0 {
device_type = "cpu";
compatible = "intel,baytrail-cpu";
reg = <0>;
intel,apic-id = <0>;
};
cpu@1 {
device_type = "cpu";
compatible = "intel,baytrail-cpu";
reg = <1>;
intel,apic-id = <4>;
};
};Quoted 9 lines
Of course, we do have the situation on ARM with the GIC that the GIC CPU IDs may be
Quoted 7 lines
Signed-off-by Ivan Gorinov <[email protected]>---
Documentation/devicetree/bindings/x86/ce4100.txt | 6 ++++++1 file changed, 6 insertions(+)
Quoted 10 lines
Documentation/devicetree/bindings/x86/ce4100.txt The CPU node · line 16
Quoted 8 lines
Documentation/devicetree/bindings/x86/ce4100.txt +0
-0@@ -14,11 +14,17 @@ The CPU node compatible = "intel,ce4100"; reg = <0>; lapic = <&lapic0>;Isn't this enough? I can't tell because whatever this points to has no binding documentation.
Local APIC is a part of CPU, not an external device (except for 486 and early Pentium). Every CPU has access to its own local APIC registers at the same base address (0xfee00000). Therefore, one "lapic" device node can work for all processors in the system.
With more changes in the code, the local APIC description could be made optional because every processor can always read its local APIC base address from MSR 0x1b. And when x2APIC mode is enabled, the local APIC registers are accessed as model specific registers instead of memory-mapped I/O.
Quoted 1 line
You could perhaps extend it and add a cell with the id value.
This may require different DT data for Linux and U-Boot, or changes in the latter.
Re: [PATCH v4 3/4] of: Documentation: Add x86 local APIC ID property
Quoted 51 lines · Ivan Gorinov
Quoted 6 lines · Rob Herring
Quoted 4 lines
Add new "intel,apic-id" property to allow using CPU descriptions in Device Tree data provided by the U-Boot loader. Address specified in 'reg' to be used as default local APIC ID to avoid breaking existing systems with DTB provided by firmware.
Is there some reason to not always use reg? For when the numbering of cpus and timers is different?
Yes, local APIC ID may differ from CPU number.
For example, in Atom E38xx (u-boot/arch/x86/dts/minnowmax.dts):cpus { #address-cells = <1>; #size-cells = <0>; cpu@0 { device_type = "cpu"; compatible = "intel,baytrail-cpu"; reg = <0>; intel,apic-id = <0>; }; cpu@1 { device_type = "cpu"; compatible = "intel,baytrail-cpu"; reg = <1>; intel,apic-id = <4>; }; };Quoted 9 lines
Of course, we do have the situation on ARM with the GIC that the GIC CPU IDs may be
Quoted 7 lines
Signed-off-by Ivan Gorinov <[email protected]>--- Documentation/devicetree/bindings/x86/ce4100.txt | 6 ++++++ 1 file changed, 6 insertions(+)Quoted 10 lines
Documentation/devicetree/bindings/x86/ce4100.txt The CPU node · line 16
Quoted 8 lines
Documentation/devicetree/bindings/x86/ce4100.txt +0
-0@@ -14,11 +14,17 @@ The CPU node compatible = "intel,ce4100"; reg = <0>; lapic = <&lapic0>;Isn't this enough? I can't tell because whatever this points to has no binding documentation.
Quoted 3 lines
Local APIC is a part of CPU, not an external device (except for 486 and early Pentium). Every CPU has access to its own local APIC registers at the same base address (
0xfee00000). Therefore, one "lapic" device node can work for all processors in the system.
Do you need a lapic node then? If you typically don't have a node, then just having the id should be fine.
Quoted 8 lines
With more changes in the code, the local APIC description could be made optional because every processor can always read its local APIC base address from MSR
0x1b. And when x2APIC mode is enabled, the local APIC registers are accessed as model specific registers instead of memory-mapped I/O.Quoted 1 line
You could perhaps extend it and add a cell with the id value.
This may require different DT data for Linux and U-Boot, or changes in the latter.
The latter case. There's one upstream for DT binding reviews.
Rob
PATCH v4 4/4 x86/devicetree: Enable multiprocessing in DT
In reply to Ivan GorinovCurrent x86 Device Tree implementation does not support multiprocessing. Use new DT bindings to describe the processors.
--- arch/x86/kernel/devicetree.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)
arch/x86/kernel/devicetree.c +25 -2
@@ -131,6 +131,30 @@ static void __init dtb_setup_hpet(void)#endif } +static void __init dtb_cpu_setup (void ) +{ +struct device_node *dn; +struct resource r; +const void *prop; +int apic_id, version; +int ret; + + version =GET_APIC_VERSION (apic_read (APIC_LVR )); +for_each_node_by_type (dn,"cpu" ) { + prop =of_get_property (dn,"intel,apic-id" ,NULL ); +if (prop) { + apic_id =be32_to_cpup (prop); + }else { +/* use the address field as APIC ID */ + ret =of_address_to_resource (dn,0 , &r); +if (WARN_ON_ONCE (ret)) +continue ; + apic_id = r.start; + } +generic_processor_info (apic_id, version); + } +} +static void __init dtb_lapic_setup (void ) {#ifdef CONFIG_X86_LOCAL_APIC
@@ -154,8 +178,6 @@ static void __init dtb_lapic_setup(void) smp_found_config =1 ; pic_mode =1 ;register_lapic_address (r.start); -generic_processor_info (boot_cpu_physical_apicid, -GET_APIC_VERSION (apic_read (APIC_LVR )));#endif }
@@ -260,6 +282,7 @@ static void __init dtb_ioapic_setup(void) {}static void __init dtb_apic_setup (void ) {dtb_lapic_setup (); +dtb_cpu_setup ();dtb_ioapic_setup (); }
Signature · 1 line
2.7.4
+ Frank
Please include me on future versions.
Quoted 33 lines · Ivan Gorinov
Fixing x86-specific DT implementation in the kernel allows reusing most of firmware code for SoC that have ARM core replaced with x86, e.g. SC9853i.
Changes since v3:
* Using
fdt_totalsize()to get DTB size before remapping instead of settinginitial_boot_paramsand callingof_get_flat_dt_size()beforeearly_init_dt_verify();* Adding new intel,apic-id property to the documentation.
Changes since v2:
*
WARN_ON_ONCEinstead ofWARN_ONto aviod multiple warnings when APIC ID is missing in CPU device tree nodes* Switched to Mutt because of white space issues:
"Preformatted" paragraph style does not work in Evolution 3.18.5.2Changes since first version:
* Splitting a single patch into three parts
Ivan Gorinov (4):
x86/devicetree: Initialize device tree before using it x86/devicetree: Fix device IRQ settings in DT
of: Documentation: Add x86 local APIC ID property x86/devicetree: Enable multiprocessing in DTDocumentation/devicetree/bindings/x86/ce4100.txt | 6 +++ arch/x86/kernel/devicetree.c | 48 +++++++++++++++++++----- 2 files changed, 44 insertions(+), 10 deletions(-)
Quoted 3 lines · Frank Rowand
+ Frank
Please include me on future versions.
I also have some interest to see the new versions in mine inbox.
Signature · 2 lines
With Best Regards, Andy Shevchenko