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

[Bin Meng]

On Thu, Dec 31, 2015 at 5:02 PM, Miao Yan  wrote:
> Hi Simon,
>
> 2015-12-31 13:08 GMT+08:00 Simon Glass :
>> Hi Maio,
>>
>> On 30 December 2015 at 19:55, Miao Yan  wrote:
>>> Add a function to fix up 'cpus' node in dts files for qemu target.
>>>
>>> Signed-off-by: Miao Yan 
>>> ---
>>> 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;
> +uclass_find_next_device()) {
> +   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", );
> +   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.
>

There is one generic way of probing available CPUs using 'cpuid'
instruction that we may add this support to U-Boot in the future. But
I doubt QEMU creates correct and consistent 'cpuid' results when
invoked via '-smp n' or even
'cpus=,cores=,threads=,sockets='. This needs to be double
checked and tested with various command line parameters passed to
QEMU.

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [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 :
> Hi Maio,
>
> On 30 December 2015 at 19:55, Miao Yan  wrote:
>> Add a function to fix up 'cpus' node in dts files for qemu target.
>>
>> Signed-off-by: Miao Yan 
>> ---
>> 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;
+uclass_find_next_device()) {
+   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", );
+   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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [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  wrote:
> Hi Simon,
>
> 2015-12-31 13:08 GMT+08:00 Simon Glass :
>> Hi Maio,
>>
>> On 30 December 2015 at 19:55, Miao Yan  wrote:
>>> Add a function to fix up 'cpus' node in dts files for qemu target.
>>>
>>> Signed-off-by: Miao Yan 
>>> ---
>>> 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;
> +uclass_find_next_device()) {
> +   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", );

Use sprintf() to give it a better name (e.g. cpu@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Simon Glass]

Hi Maio,

On 30 December 2015 at 19:55, Miao Yan  wrote:
> Add a function to fix up 'cpus' node in dts files for qemu target.
>
> Signed-off-by: Miao Yan 
> ---
> 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:

- 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
- 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().

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?

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot