[U-Boot] [PATCH v4 5/8] x86: qemu: add qemu_fwcfg_fdt_fixup()

[Miao Yan]

Hi Simon,

2015-12-31 13:08 GMT+08:00 Simon Glass <sjg at chromium.org>:
> Hi Maio,
>
> On 30 December 2015 at 19:55, Miao Yan <yanmiaobest at gmail.com> wrote:
>> Add a function to fix up 'cpus' node in dts files for qemu target.
>>
>> Signed-off-by: Miao Yan <yanmiaobest at gmail.com>
>> ---
>> Changes in v4:
>>   - fix a typo in commit log
>>
>>  arch/x86/cpu/qemu/fw_cfg.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
>>  arch/x86/cpu/qemu/fw_cfg.h | 11 ++++++++
>>  2 files changed, 76 insertions(+)
>
> I'm sorry for not reviewing this earlier (Christmas and all that). I
> don't think you need to update the device tree to make this work.
>
> Here's my suggestion:


I am not familiar with driver model so I am not sure if I understand
 you correctly. Are you suggesting something like the following:

+
+void create_cpu_node(void)
+{
+       int ret;
+       int cpu_online;
+       int cpu_num = 0;
+       struct udevice *dev;
+       struct cpu_platdata *plat;
+
+       for (uclass_find_first_device(UCLASS_CPU, &dev);
+            dev;
+            uclass_find_next_device(&dev)) {
+               cpu_num++;
+       }
+
+       cpu_online = qemu_fwcfg_online_cpus();
+       printf("%d cpus probed, %d cpus online\n", cpu_num, cpu_online);
+
+       for (dev = NULL; cpu_num < cpu_online; cpu_num++) {
+               ret = device_bind_driver(cpus_dev, "cpu_qemu", "cpu", &dev);
+               if (ret < 0) {
+                       printf("device_bind_driver failed with code %d\n", ret);
+                       return;
+               }
+               plat = dev_get_parent_platdata(dev);
+               plat->cpu_id = cpu_num;
+       }
+}



>
> - At present cpu_x86_bind() sets up the CPU APIC ID from the device tree
> - You can bind new CPU devices in your code on start-up
> - You can check if you have CPUs which are not available in the device
> list, by using uclass_find_first/next_device() to iterate through the
> devices without probing them
> - Then to create a new one, call device_bind_driver() with the /cpus
> node as the parent

The 'cpus' node is created in uclass_cpu_init(), and all the
'cpu' subnode are created by scanning device tree. But arch_early_init_r()
is called before uclass_cpu_init(), so at that time, there's no
'cpus' yet.

Seems we need somewhere after uclass_cpu_init() but before mp_init() ?
Or we entirely bypass the cpu uclass driver and create /cpus too ?


> - After binding, update the parent platform data:
>
>   struct cpu_platdata *plat = dev_get_parent_platdata(dev);
>
>    plat->cpu_id = ...
>
> - Then when it comes to probing, you will have all the devices you
> need, and you don't need to adjust the device tree. The device tree
> can just hold a single device, for example.
>
> I think it is better to do this than adjust the device tree because it
> removes the 32-CPU limit and should be faster. It is also simpler as
> it is a more direct method. Also I believe you only need to do this
> after relocation - e.g. in arch_early_init_r(), which is before
> mp_init() is called from cpu_init_r().

Seems I can't do it in arch_early_init_r(), when 'cpus' is available yet.

>
> I wonder if there is a general way to probe available CPUs (and their
> APIC IDs)? Or is qemu the only 'CPU' with this feature?

I am not sure about other x86 boards, but certainly fw_cfg is
not the generic way.  Maybe Bin can comment on this.

>
> Regards,
> Simon