Current x86 implementation of Device Tree does not support multiprocessing,
and the bindings documentation describes the "reg" property as CPU number
instead of hardware-assigned local APIC ID.
Ivan Gorinov (2):
of: Documentation: Specify local APIC ID in "reg"
x86/devicetree: Use CPU description from Device Tree
Documentation/devicetree/bindings/x86/ce4100.txt | 38 ++++++++++++++++-----
arch/x86/kernel/devicetree.c | 42 +++++++++++++++++-------
2 files changed, 60 insertions(+), 20 deletions(-)
--
2.7.4
Set the "reg" property to the processor's local APIC ID.
Local APIC ID is assigned by hardware and may differ from CPU number.
Signed-off-by: Ivan Gorinov <[email protected]>
---
Documentation/devicetree/bindings/x86/ce4100.txt | 38 ++++++++++++++++++------
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/x86/ce4100.txt b/Documentation/devicetree/bindings/x86/ce4100.txt
index b49ae59..5a4bd83 100644
--- a/Documentation/devicetree/bindings/x86/ce4100.txt
+++ b/Documentation/devicetree/bindings/x86/ce4100.txt
@@ -7,17 +7,37 @@ Many of the "generic" devices like HPET or IO APIC have the ce4100
name in their compatible property because they first appeared in this
SoC.
-The CPU node
-------------
- cpu@0 {
- device_type = "cpu";
- compatible = "intel,ce4100";
- reg = <0>;
- lapic = <&lapic0>;
+The CPU nodes
+-------------
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu@0 {
+ device_type = "cpu";
+ compatible = "intel,ce4100";
+ reg = <0x00>;
+ };
+
+ cpu@1 {
+ device_type = "cpu";
+ compatible = "intel,ce4100";
+ reg = <0x02>;
+ };
};
-The reg property describes the CPU number. The lapic property points to
-the local APIC timer.
+A "cpu" node describes one logical processor (hardware thread).
+
+Required properties:
+
+- device_type
+ Device type, must be "cpu".
+
+- reg
+ Local APIC ID, a unique number assigned to each processor by
+ hardware. This ID is used to specify the destination of interrupt
+ messages with "physical" destination mode, including startup IPI.
The SoC node
------------
--
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 | 42 +++++++++++++++++++++++++++++++-----------
1 file changed, 31 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 5cd387f..64671db 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -131,31 +131,49 @@ static void __init dtb_setup_hpet(void)
#endif
}
+static void __init dtb_cpu_setup(void)
+{
+ struct device_node *dn;
+ const void *prop;
+ int prop_bytes;
+ 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, "reg", &prop_bytes);
+ if (!prop || prop_bytes < sizeof(u32)) {
+ pr_warn("%pOF: missing local APIC ID\n", dn);
+ continue;
+ }
+ apic_id = be32_to_cpup(prop);
+ generic_processor_info(apic_id, version);
+ }
+}
+
static void __init dtb_lapic_setup(void)
{
#ifdef CONFIG_X86_LOCAL_APIC
struct device_node *dn;
struct resource r;
+ unsigned long lapic_addr = APIC_DEFAULT_PHYS_BASE;
int ret;
dn = of_find_compatible_node(NULL, NULL, "intel,ce4100-lapic");
- if (!dn)
- return;
-
- ret = of_address_to_resource(dn, 0, &r);
- if (WARN_ON(ret))
- return;
+ if (dn) {
+ ret = of_address_to_resource(dn, 0, &r);
+ if (WARN_ON(ret))
+ return;
+ lapic_addr = r.start;
+ }
/* Did the boot loader setup the local APIC ? */
if (!boot_cpu_has(X86_FEATURE_APIC)) {
- if (apic_force_enable(r.start))
+ if (apic_force_enable(lapic_addr))
return;
}
- 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)));
+ register_lapic_address(lapic_addr);
#endif
}
@@ -260,6 +278,7 @@ static void __init dtb_ioapic_setup(void) {}
static void __init dtb_apic_setup(void)
{
dtb_lapic_setup();
+ dtb_cpu_setup();
dtb_ioapic_setup();
}
@@ -297,6 +316,7 @@ void __init x86_dtb_init(void)
if (!of_have_populated_dt())
return;
+ smp_found_config = 1;
dtb_setup_hpet();
dtb_apic_setup();
}
--
2.7.4
On Mon, Mar 12, 2018 at 03:22:33PM -0700, Ivan Gorinov wrote:
> 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 | 42 +++++++++++++++++++++++++++++++-----------
> 1 file changed, 31 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
> index 5cd387f..64671db 100644
> --- a/arch/x86/kernel/devicetree.c
> +++ b/arch/x86/kernel/devicetree.c
> @@ -131,31 +131,49 @@ static void __init dtb_setup_hpet(void)
> #endif
> }
>
> +static void __init dtb_cpu_setup(void)
> +{
> + struct device_node *dn;
> + const void *prop;
> + int prop_bytes;
> + 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, "reg", &prop_bytes);
> + if (!prop || prop_bytes < sizeof(u32)) {
> + pr_warn("%pOF: missing local APIC ID\n", dn);
> + continue;
> + }
> + apic_id = be32_to_cpup(prop);
Please use of_property_read_u32().
Thanks,
Mark.
On Mon, Mar 12, 2018 at 03:21:48PM -0700, Ivan Gorinov wrote:
> Set the "reg" property to the processor's local APIC ID.
> Local APIC ID is assigned by hardware and may differ from CPU number.
>
> Signed-off-by: Ivan Gorinov <[email protected]>
> ---
> Documentation/devicetree/bindings/x86/ce4100.txt | 38 ++++++++++++++++++------
> 1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/x86/ce4100.txt b/Documentation/devicetree/bindings/x86/ce4100.txt
> index b49ae59..5a4bd83 100644
> --- a/Documentation/devicetree/bindings/x86/ce4100.txt
> +++ b/Documentation/devicetree/bindings/x86/ce4100.txt
> @@ -7,17 +7,37 @@ Many of the "generic" devices like HPET or IO APIC have the ce4100
> name in their compatible property because they first appeared in this
> SoC.
>
> -The CPU node
> -------------
> - cpu@0 {
> - device_type = "cpu";
> - compatible = "intel,ce4100";
> - reg = <0>;
> - lapic = <&lapic0>;
> +The CPU nodes
> +-------------
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu@0 {
> + device_type = "cpu";
> + compatible = "intel,ce4100";
> + reg = <0x00>;
> + };
> +
> + cpu@1 {
> + device_type = "cpu";
> + compatible = "intel,ce4100";
> + reg = <0x02>;
> + };
The unit-address (the bit after the '@' in the node name) should match
the reg, so this node should be named cpu@2.
If there's another ID associated with each CPU, then this should be
described in another property.
> };
>
> -The reg property describes the CPU number. The lapic property points to
> -the local APIC timer.
Why was the lapic phandle removed?
Thanks,
Mark.
> +A "cpu" node describes one logical processor (hardware thread).
> +
> +Required properties:
> +
> +- device_type
> + Device type, must be "cpu".
> +
> +- reg
> + Local APIC ID, a unique number assigned to each processor by
> + hardware. This ID is used to specify the destination of interrupt
> + messages with "physical" destination mode, including startup IPI.
>
> The SoC node
> ------------
> --
> 2.7.4
>
On Tue, 2018-03-13 at 11:01 +0000, Mark Rutland wrote:
> > + cpu@1 {
> > + device_type = "cpu";
> > + compatible = "intel,ce4100";
> > + reg = <0x02>;
> > + };
> The unit-address (the bit after the '@' in the node name) should match
> the reg, so this node should be named cpu@2.
OK
> > -The reg property describes the CPU number. The lapic property points to
> > -the local APIC timer.
> Why was the lapic phandle removed?
The "lapic" node may not be required.
Local APIC is an essential part of every logical CPU described by a "cpu"
node, with registers accessed as memory-mapped I/O (except for x2APIC mode).
Current implementation of local APIC kernel driver requires base address to
be the same on all CPUs, default 0xfee00000. If the base address is changed
by firmware, one optional node can describe new address for all CPUs.
Hi Ivan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180314]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Ivan-Gorinov/x86-devicetree-Enable-multiprocessing/20180314-192547
config: i386-randconfig-x012-201810 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
arch/x86/kernel/devicetree.c: In function 'dtb_cpu_setup':
arch/x86/kernel/devicetree.c:139:6: warning: unused variable 'ret' [-Wunused-variable]
int ret;
^~~
arch/x86/kernel/devicetree.c: In function 'x86_dtb_init':
>> arch/x86/kernel/devicetree.c:314:19: error: lvalue required as left operand of assignment
smp_found_config = 1;
^
vim +314 arch/x86/kernel/devicetree.c
132
133 static void __init dtb_cpu_setup(void)
134 {
135 struct device_node *dn;
136 const void *prop;
137 int prop_bytes;
138 int apic_id, version;
> 139 int ret;
140
141 version = GET_APIC_VERSION(apic_read(APIC_LVR));
142 for_each_node_by_type(dn, "cpu") {
143 prop = of_get_property(dn, "reg", &prop_bytes);
144 if (!prop || prop_bytes < sizeof(u32)) {
145 pr_warn("%pOF: missing local APIC ID\n", dn);
146 continue;
147 }
148 apic_id = be32_to_cpup(prop);
149 generic_processor_info(apic_id, version);
150 }
151 }
152
153 static void __init dtb_lapic_setup(void)
154 {
155 #ifdef CONFIG_X86_LOCAL_APIC
156 struct device_node *dn;
157 struct resource r;
158 unsigned long lapic_addr = APIC_DEFAULT_PHYS_BASE;
159 int ret;
160
161 dn = of_find_compatible_node(NULL, NULL, "intel,ce4100-lapic");
162 if (dn) {
163 ret = of_address_to_resource(dn, 0, &r);
164 if (WARN_ON(ret))
165 return;
166 lapic_addr = r.start;
167 }
168
169 /* Did the boot loader setup the local APIC ? */
170 if (!boot_cpu_has(X86_FEATURE_APIC)) {
171 if (apic_force_enable(lapic_addr))
172 return;
173 }
174 pic_mode = 1;
175 register_lapic_address(lapic_addr);
176 #endif
177 }
178
179 #ifdef CONFIG_X86_IO_APIC
180 static unsigned int ioapic_id;
181
182 struct of_ioapic_type {
183 u32 out_type;
184 u32 trigger;
185 u32 polarity;
186 };
187
188 static struct of_ioapic_type of_ioapic_type[] =
189 {
190 {
191 .out_type = IRQ_TYPE_EDGE_RISING,
192 .trigger = IOAPIC_EDGE,
193 .polarity = 1,
194 },
195 {
196 .out_type = IRQ_TYPE_LEVEL_LOW,
197 .trigger = IOAPIC_LEVEL,
198 .polarity = 0,
199 },
200 {
201 .out_type = IRQ_TYPE_LEVEL_HIGH,
202 .trigger = IOAPIC_LEVEL,
203 .polarity = 1,
204 },
205 {
206 .out_type = IRQ_TYPE_EDGE_FALLING,
207 .trigger = IOAPIC_EDGE,
208 .polarity = 0,
209 },
210 };
211
212 static int dt_irqdomain_alloc(struct irq_domain *domain, unsigned int virq,
213 unsigned int nr_irqs, void *arg)
214 {
215 struct of_phandle_args *irq_data = (void *)arg;
216 struct of_ioapic_type *it;
217 struct irq_alloc_info tmp;
218
219 if (WARN_ON(irq_data->args_count < 2))
220 return -EINVAL;
221 if (irq_data->args[1] >= ARRAY_SIZE(of_ioapic_type))
222 return -EINVAL;
223
224 it = &of_ioapic_type[irq_data->args[1]];
225 ioapic_set_alloc_attr(&tmp, NUMA_NO_NODE, it->trigger, it->polarity);
226 tmp.ioapic_id = mpc_ioapic_id(mp_irqdomain_ioapic_idx(domain));
227 tmp.ioapic_pin = irq_data->args[0];
228
229 return mp_irqdomain_alloc(domain, virq, nr_irqs, &tmp);
230 }
231
232 static const struct irq_domain_ops ioapic_irq_domain_ops = {
233 .alloc = dt_irqdomain_alloc,
234 .free = mp_irqdomain_free,
235 .activate = mp_irqdomain_activate,
236 .deactivate = mp_irqdomain_deactivate,
237 };
238
239 static void __init dtb_add_ioapic(struct device_node *dn)
240 {
241 struct resource r;
242 int ret;
243 struct ioapic_domain_cfg cfg = {
244 .type = IOAPIC_DOMAIN_DYNAMIC,
245 .ops = &ioapic_irq_domain_ops,
246 .dev = dn,
247 };
248
249 ret = of_address_to_resource(dn, 0, &r);
250 if (ret) {
251 printk(KERN_ERR "Can't obtain address from device node %pOF.\n", dn);
252 return;
253 }
254 mp_register_ioapic(++ioapic_id, r.start, gsi_top, &cfg);
255 }
256
257 static void __init dtb_ioapic_setup(void)
258 {
259 struct device_node *dn;
260
261 for_each_compatible_node(dn, NULL, "intel,ce4100-ioapic")
262 dtb_add_ioapic(dn);
263
264 if (nr_ioapics) {
265 of_ioapic = 1;
266 return;
267 }
268 printk(KERN_ERR "Error: No information about IO-APIC in OF.\n");
269 }
270 #else
271 static void __init dtb_ioapic_setup(void) {}
272 #endif
273
274 static void __init dtb_apic_setup(void)
275 {
276 dtb_lapic_setup();
277 dtb_cpu_setup();
278 dtb_ioapic_setup();
279 }
280
281 #ifdef CONFIG_OF_FLATTREE
282 static void __init x86_flattree_get_config(void)
283 {
284 u32 size, map_len;
285 void *dt;
286
287 if (!initial_dtb)
288 return;
289
290 map_len = max(PAGE_SIZE - (initial_dtb & ~PAGE_MASK), (u64)128);
291
292 initial_boot_params = dt = early_memremap(initial_dtb, map_len);
293 size = of_get_flat_dt_size();
294 if (map_len < size) {
295 early_memunmap(dt, map_len);
296 initial_boot_params = dt = early_memremap(initial_dtb, size);
297 map_len = size;
298 }
299
300 unflatten_and_copy_device_tree();
301 early_memunmap(dt, map_len);
302 }
303 #else
304 static inline void x86_flattree_get_config(void) { }
305 #endif
306
307 void __init x86_dtb_init(void)
308 {
309 x86_flattree_get_config();
310
311 if (!of_have_populated_dt())
312 return;
313
> 314 smp_found_config = 1;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation