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(-)
--
2.7.4
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().
Signed-off-by: Ivan Gorinov <[email protected]>
---
arch/x86/kernel/devicetree.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 25de5f6..63d2ebc 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -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);
}
--
2.7.4
Current x86 Device Tree implementation does not support multiprocessing.
Use new DT bindings to describe the processors.
Signed-off-by: Ivan Gorinov <[email protected]>
---
arch/x86/kernel/devicetree.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 5cd387f..b28ac84 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -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();
}
--
2.7.4
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.
Signed-off-by: Ivan Gorinov <[email protected]>
---
arch/x86/kernel/devicetree.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 63d2ebc..5cd387f 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -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);
}
--
2.7.4
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.
Signed-off-by: Ivan Gorinov <[email protected]>
---
Documentation/devicetree/bindings/x86/ce4100.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/x86/ce4100.txt b/Documentation/devicetree/bindings/x86/ce4100.txt
index b49ae59..d15de48 100644
--- a/Documentation/devicetree/bindings/x86/ce4100.txt
+++ b/Documentation/devicetree/bindings/x86/ce4100.txt
@@ -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
------------
--
2.7.4
On Wed, Mar 7, 2018 at 1:47 PM, Ivan Gorinov <[email protected]> wrote:
> 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
>
> Signed-off-by: Ivan Gorinov <[email protected]>
> ---
> Documentation/devicetree/bindings/x86/ce4100.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/x86/ce4100.txt b/Documentation/devicetree/bindings/x86/ce4100.txt
> index b49ae59..d15de48 100644
> --- a/Documentation/devicetree/bindings/x86/ce4100.txt
> +++ b/Documentation/devicetree/bindings/x86/ce4100.txt
> @@ -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.
> + 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
>
On Wed, 2018-03-07 at 14:23 -0600, Rob Herring wrote:
> > 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>;
};
};
> Of course, we do have the situation on ARM with the GIC that the GIC
> CPU IDs may be
> >
> >
> > Signed-off-by: Ivan Gorinov <[email protected]>
> > ---
> > ?Documentation/devicetree/bindings/x86/ce4100.txt | 6 ++++++
> > ?1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/x86/ce4100.txt b/Documentation/devicetree/bindings/x86/ce4100.txt
> > index b49ae59..d15de48 100644
> > --- a/Documentation/devicetree/bindings/x86/ce4100.txt
> > +++ b/Documentation/devicetree/bindings/x86/ce4100.txt
> > @@ -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.
> 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.
On Wed, Mar 7, 2018 at 7:11 PM, Ivan Gorinov <[email protected]> wrote:
> On Wed, 2018-03-07 at 14:23 -0600, Rob Herring wrote:
>
>> > 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>;
> };
> };
>
>> Of course, we do have the situation on ARM with the GIC that the GIC
>> CPU IDs may be
>> >
>> >
>> > Signed-off-by: Ivan Gorinov <[email protected]>
>> > ---
>> > Documentation/devicetree/bindings/x86/ce4100.txt | 6 ++++++
>> > 1 file changed, 6 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/x86/ce4100.txt b/Documentation/devicetree/bindings/x86/ce4100.txt
>> > index b49ae59..d15de48 100644
>> > --- a/Documentation/devicetree/bindings/x86/ce4100.txt
>> > +++ b/Documentation/devicetree/bindings/x86/ce4100.txt
>> > @@ -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.
Do you need a lapic node then? If you typically don't have a node,
then just having the id should be fine.
> 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.
>
>> 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
+ Frank
Please include me on future versions.
On 03/07/18 11:45, Ivan Gorinov wrote:
> 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(-)
>
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().
Fixes: 08d53aa58cb1 ("of/fdt: export fdt blob as /sys/firmware/fdt")
Signed-off-by: Ivan Gorinov <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Rob Herring <[email protected]>
Link: https://lkml.kernel.org/r/c8c7e941efc63b5d25ebf9b6350b0f3df38f6098.1520450752.git.ivan.gorinov@intel.com
---
arch/x86/kernel/devicetree.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 25de5f6ca997..63d2ebc21825 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -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);
}
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.
Fixes: 11e4438ee330 ("irqdomain: Introduce a firmware-specific IRQ specifier structure")
Signed-off-by: Ivan Gorinov <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Rob Herring <[email protected]>
Link: https://lkml.kernel.org/r/a234dee27ea60ce76141872da0d6bdb378b2a9ee.1520450752.git.ivan.gorinov@intel.com
---
arch/x86/kernel/devicetree.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 63d2ebc21825..5cd387fcc777 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -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);
}
On Thu, Mar 8, 2018 at 4:16 AM, Frank Rowand <[email protected]> wrote:
> + Frank
>
> Please include me on future versions.
I also have some interest to see the new versions in mine inbox.
--
With Best Regards,
Andy Shevchenko