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

[Simon Glass]

Hi Miao,

On 31 December 2015 at 02:02, Miao Yan <yanmiaobest at gmail.com> wrote:
> 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);

Use sprintf() to give it a better name (e.g. cpu at 2)

> +               if (ret < 0) {
> +                       printf("device_bind_driver failed with code %d\n", ret);
> +                       return;

return ret

> +               }
> +               plat = dev_get_parent_platdata(dev);
> +               plat->cpu_id = cpu_num;
> +       }
> +}

Yes that looks right.

>
>
>
>>
>> - 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 ?

Can't you leave the 'cpus' node in the device tree? It can be empty if you like.

>
>
>> - 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