| Message ID | 1451530525-30913-6-git-send-email-yanmiaobest@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Bin Meng |
| Headers | show |
Hi Maio, On 30 December 2015 at 19:55, Miao Yan <yanmiaobest@gmail.com> wrote: > Add a function to fix up 'cpus' node in dts files for qemu target. > > Signed-off-by: Miao Yan <yanmiaobest@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: - 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
Hi Simon, 2015-12-31 13:08 GMT+08:00 Simon Glass <sjg@chromium.org>: > Hi Maio, > > On 30 December 2015 at 19:55, Miao Yan <yanmiaobest@gmail.com> wrote: >> Add a function to fix up 'cpus' node in dts files for qemu target. >> >> Signed-off-by: Miao Yan <yanmiaobest@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
Hi Miao, On 31 December 2015 at 02:02, Miao Yan <yanmiaobest@gmail.com> wrote: > Hi Simon, > > 2015-12-31 13:08 GMT+08:00 Simon Glass <sjg@chromium.org>: >> Hi Maio, >> >> On 30 December 2015 at 19:55, Miao Yan <yanmiaobest@gmail.com> wrote: >>> Add a function to fix up 'cpus' node in dts files for qemu target. >>> >>> Signed-off-by: Miao Yan <yanmiaobest@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@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
On Thu, Dec 31, 2015 at 5:02 PM, Miao Yan <yanmiaobest@gmail.com> wrote: > Hi Simon, > > 2015-12-31 13:08 GMT+08:00 Simon Glass <sjg@chromium.org>: >> Hi Maio, >> >> On 30 December 2015 at 19:55, Miao Yan <yanmiaobest@gmail.com> wrote: >>> Add a function to fix up 'cpus' node in dts files for qemu target. >>> >>> Signed-off-by: Miao Yan <yanmiaobest@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. > 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=<a>,cores=<b>,threads=<c>,sockets=<d>'. This needs to be double checked and tested with various command line parameters passed to QEMU. Regards, Bin
diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c index 9de8680..5b1caa8 100644 --- a/arch/x86/cpu/qemu/fw_cfg.c +++ b/arch/x86/cpu/qemu/fw_cfg.c @@ -9,6 +9,7 @@ #include <errno.h> #include <malloc.h> #include <asm/io.h> +#include <libfdt.h> #include "fw_cfg.h" static bool fwcfg_present; @@ -103,6 +104,70 @@ int qemu_fwcfg_online_cpus(void) return le16_to_cpu(nb_cpus); } +void qemu_fwcfg_fdt_fixup(void *fdt_addr, int cpu_num) +{ + int i; + char cpus[10]; + int off, err, sub_off, id; + + off = fdt_path_offset(fdt_addr, "/cpus"); + if (off != -FDT_ERR_NOTFOUND) { + printf("error detecting cpus subnode: %s (%d)\n", + fdt_strerror(off), off); + return; + } + + off = fdt_add_subnode(fdt_addr, 0, "cpus"); + if (off < 0) { + printf("error adding cpus subnode: %s (%d)\n", + fdt_strerror(off), off); + return; + } + + for (i = cpu_num - 1; i >= 0; i--) { + sprintf(cpus, "%s@%d", "cpu", i); + sub_off = fdt_add_subnode(fdt_addr, off, cpus); + if (sub_off < 0) { + printf("error adding subnode cpu %d: %s (%d)\n", + i, fdt_strerror(sub_off), sub_off); + return; + } + + id = cpu_to_fdt32(i); + err = fdt_setprop(fdt_addr, sub_off, "intel,apic-id", + (void *)&id, sizeof(id)); + if (err < 0) { + printf("error adding apic-id %d: %s (%d)\n", + i, fdt_strerror(err), err); + return; + } + + err = fdt_setprop(fdt_addr, sub_off, "reg", + (void *)&id, sizeof(id)); + if (err < 0) { + printf("error adding reg %d: %s (%d)\n", + i, fdt_strerror(err), err); + return; + } + + err = fdt_setprop(fdt_addr, sub_off, "compatible", + "cpu-qemu", sizeof("cpu-qemu")); + if (err < 0) { + printf("error adding compatible %d: %s (%d)\n", + i, fdt_strerror(err), err); + return; + } + + err = fdt_setprop(fdt_addr, sub_off, "device_type", + "cpu", sizeof("cpu")); + if (err < 0) { + printf("error adding device_type %d: %s (%d)\n", + i, fdt_strerror(err), err); + return; + } + } +} + static int qemu_fwcfg_setup_kernel(void *load_addr, void *initrd_addr) { char *data_addr; diff --git a/arch/x86/cpu/qemu/fw_cfg.h b/arch/x86/cpu/qemu/fw_cfg.h index 03ac27d..f2c9221 100644 --- a/arch/x86/cpu/qemu/fw_cfg.h +++ b/arch/x86/cpu/qemu/fw_cfg.h @@ -94,4 +94,15 @@ void qemu_fwcfg_init(void); int qemu_fwcfg_online_cpus(void); +/** + * Fix 'cpu' node in device tree for qemu targets + * + * @fdt_addr: device tree blob address + * @cpu_num: cpu number in system + * + * @return: None + */ + +void qemu_fwcfg_fdt_fixup(void *fdt_addr, int cpu_num); + #endif
Add a function to fix up 'cpus' node in dts files for qemu target. Signed-off-by: Miao Yan <yanmiaobest@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(+)