* [PATCH v5 0/2] x86/devicetree: Enable multiprocessing @ 2018-03-12 22:21 Ivan Gorinov 2018-03-12 22:21 ` [PATCH v5 1/2] of: Documentation: Specify local APIC ID in "reg" Ivan Gorinov 2018-03-12 22:22 ` [PATCH v5 2/2] x86/devicetree: Use CPU description from Device Tree Ivan Gorinov 0 siblings, 2 replies; 7+ messages in thread From: Ivan Gorinov @ 2018-03-12 22:21 UTC (permalink / raw) To: Thomas Gleixner, Frank Rowand Cc: Linux Kernel Mailing List, Ingo Molnar, Rob Herring, Mark Rutland 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 1/2] of: Documentation: Specify local APIC ID in "reg" 2018-03-12 22:21 [PATCH v5 0/2] x86/devicetree: Enable multiprocessing Ivan Gorinov @ 2018-03-12 22:21 ` Ivan Gorinov 2018-03-13 11:01 ` Mark Rutland 2018-03-12 22:22 ` [PATCH v5 2/2] x86/devicetree: Use CPU description from Device Tree Ivan Gorinov 1 sibling, 1 reply; 7+ messages in thread From: Ivan Gorinov @ 2018-03-12 22:21 UTC (permalink / raw) To: Thomas Gleixner, Frank Rowand Cc: Linux Kernel Mailing List, Ingo Molnar, Rob Herring, Mark Rutland 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 <ivan.gorinov@intel.com> --- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v5 1/2] of: Documentation: Specify local APIC ID in "reg" 2018-03-12 22:21 ` [PATCH v5 1/2] of: Documentation: Specify local APIC ID in "reg" Ivan Gorinov @ 2018-03-13 11:01 ` Mark Rutland 2018-03-13 17:50 ` Ivan Gorinov 0 siblings, 1 reply; 7+ messages in thread From: Mark Rutland @ 2018-03-13 11:01 UTC (permalink / raw) To: Ivan Gorinov Cc: Thomas Gleixner, Frank Rowand, Linux Kernel Mailing List, Ingo Molnar, Rob Herring 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 <ivan.gorinov@intel.com> > --- > 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 1/2] of: Documentation: Specify local APIC ID in "reg" 2018-03-13 11:01 ` Mark Rutland @ 2018-03-13 17:50 ` Ivan Gorinov 0 siblings, 0 replies; 7+ messages in thread From: Ivan Gorinov @ 2018-03-13 17:50 UTC (permalink / raw) To: Mark Rutland Cc: Thomas Gleixner, Frank Rowand, Andy Shevchenko, Linux Kernel Mailing List, Ingo Molnar, Rob Herring 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 2/2] x86/devicetree: Use CPU description from Device Tree 2018-03-12 22:21 [PATCH v5 0/2] x86/devicetree: Enable multiprocessing Ivan Gorinov 2018-03-12 22:21 ` [PATCH v5 1/2] of: Documentation: Specify local APIC ID in "reg" Ivan Gorinov @ 2018-03-12 22:22 ` Ivan Gorinov 2018-03-13 11:03 ` Mark Rutland 2018-03-14 12:18 ` kbuild test robot 1 sibling, 2 replies; 7+ messages in thread From: Ivan Gorinov @ 2018-03-12 22:22 UTC (permalink / raw) To: Thomas Gleixner, Frank Rowand Cc: Linux Kernel Mailing List, Ingo Molnar, Rob Herring, Mark Rutland Current x86 Device Tree implementation does not support multiprocessing. Use new DT bindings to describe the processors. Signed-off-by: Ivan Gorinov <ivan.gorinov@intel.com> --- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] x86/devicetree: Use CPU description from Device Tree 2018-03-12 22:22 ` [PATCH v5 2/2] x86/devicetree: Use CPU description from Device Tree Ivan Gorinov @ 2018-03-13 11:03 ` Mark Rutland 2018-03-14 12:18 ` kbuild test robot 1 sibling, 0 replies; 7+ messages in thread From: Mark Rutland @ 2018-03-13 11:03 UTC (permalink / raw) To: Ivan Gorinov Cc: Thomas Gleixner, Frank Rowand, Linux Kernel Mailing List, Ingo Molnar, Rob Herring 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 <ivan.gorinov@intel.com> > --- > 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] x86/devicetree: Use CPU description from Device Tree 2018-03-12 22:22 ` [PATCH v5 2/2] x86/devicetree: Use CPU description from Device Tree Ivan Gorinov 2018-03-13 11:03 ` Mark Rutland @ 2018-03-14 12:18 ` kbuild test robot 1 sibling, 0 replies; 7+ messages in thread From: kbuild test robot @ 2018-03-14 12:18 UTC (permalink / raw) To: Ivan Gorinov Cc: kbuild-all, Thomas Gleixner, Frank Rowand, Linux Kernel Mailing List, Ingo Molnar, Rob Herring, Mark Rutland [-- Attachment #1: Type: text/plain, Size: 6380 bytes --] 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 [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 29380 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-14 12:19 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-12 22:21 [PATCH v5 0/2] x86/devicetree: Enable multiprocessing Ivan Gorinov 2018-03-12 22:21 ` [PATCH v5 1/2] of: Documentation: Specify local APIC ID in "reg" Ivan Gorinov 2018-03-13 11:01 ` Mark Rutland 2018-03-13 17:50 ` Ivan Gorinov 2018-03-12 22:22 ` [PATCH v5 2/2] x86/devicetree: Use CPU description from Device Tree Ivan Gorinov 2018-03-13 11:03 ` Mark Rutland 2018-03-14 12:18 ` kbuild test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).