Hi Bin,
2016-05-10 13:12 GMT+08:00 Bin Meng bmeng.cn@gmail.com:
Hi Miao,
On Tue, May 10, 2016 at 12:35 PM, Miao Yan yanmiaobest@gmail.com wrote:
2016-05-10 11:08 GMT+08:00 Bin Meng bmeng.cn@gmail.com:
On Tue, May 10, 2016 at 10:17 AM, Tom Rini trini@konsulko.com wrote:
On Tue, May 10, 2016 at 09:20:45AM +0800, Bin Meng wrote:
On Fri, May 6, 2016 at 10:40 PM, Tom Rini trini@konsulko.com wrote:
- Move the command portion of arch/x86/cpu/qemu/fw_cfg.c into cmd/qemu_fw_cfg.c
- Move arch/x86/include/asm/fw_cfg.h to include/qemu_fw_cfg.h
- Rename ACPI table portion to arch/x86/cpu/qemu/acpi_table.c
Signed-off-by: Tom Rini trini@konsulko.com
Changes in v2:
- Depend on X86 (per Miao Yan)
arch/x86/cpu/mp_init.c | 2 +- arch/x86/cpu/qemu/Makefile | 3 +- arch/x86/cpu/qemu/acpi_table.c | 243 ++++++++++++++++++ arch/x86/cpu/qemu/cpu.c | 2 +- arch/x86/cpu/qemu/fw_cfg.c | 570 ----------------------------------------- arch/x86/cpu/qemu/qemu.c | 2 +- arch/x86/include/asm/fw_cfg.h | 157 ------------ arch/x86/lib/acpi_table.c | 2 +- cmd/Kconfig | 7 + cmd/Makefile | 1 + cmd/qemu_fw_cfg.c | 343 +++++++++++++++++++++++++ configs/qemu-x86_defconfig | 1 + include/qemu_fw_cfg.h | 162 ++++++++++++ 13 files changed, 763 insertions(+), 732 deletions(-) create mode 100644 arch/x86/cpu/qemu/acpi_table.c delete mode 100644 arch/x86/cpu/qemu/fw_cfg.c delete mode 100644 arch/x86/include/asm/fw_cfg.h create mode 100644 cmd/qemu_fw_cfg.c create mode 100644 include/qemu_fw_cfg.h
Looks good.
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Tom, will you pick this for this release, or next release?
Miao has a patch [1] to remove CONFIG_QEMU_ACPI_TABLE. If your patch comes first, Miao needs to rebase his and submit v2.
For the next release, and I'll leave it to you to pull in. Thanks!
applied to u-boot-x86/next, thanks!
Wait, you applied this already ? Did you include the diff I mentioned
The patch is not in mainline. It's currently in x86 tree and we can always fix issues before a pull request is sent out.
? This patch has build issues. Before the patch, the qfw is built unconditionally for x86-qemu, after applying this patch, qfw depends on CONFIG_CMD_QEMU_FW_CFG. This is a change of behavior, so you need to test:
1) defconfig build 2) defconfig with CONFIG_CMD_QEMU_FW_CFG disabled because it'suser visible now
This patch breaks 2):
I doubt everything can be build error free if we randomly switch something on or off from the Kconfig GUI. I did buildman testing for
Well I don't think it's "random", we have mature tools to deal with dependencies, it's not like I was directly modifying random stuff in .config with a text editor. Kconfig options should be there for a reason, otherwise why bother. And if it doesn't work then it's a bug.
this patch and nothing breaks by default. Having said that, I agree the build error you reported is something we should fix.
OK. So can you please apply the diff or ask for v3 if you like ?
arch/x86/cpu/built-in.o: In function `cpu_qemu_get_count': /home/myan/work/u-boot/arch/x86/cpu/qemu/cpu.c:28: undefined reference to `qemu_fwcfg_online_cpus' arch/x86/cpu/built-in.o: In function `qemu_chipset_init': /home/myan/work/u-boot/arch/x86/cpu/qemu/qemu.c:91: undefined reference to `qemu_fwcfg_init' arch/x86/cpu/built-in.o: In function `write_acpi_tables': /home/myan/work/u-boot/arch/x86/cpu/qemu/acpi_table.c:187: undefined reference to `qemu_fwcfg_read_firmware_list' /home/myan/work/u-boot/arch/x86/cpu/qemu/acpi_table.c:193: undefined reference to `qemu_fwcfg_find_file' /home/myan/work/u-boot/arch/x86/cpu/qemu/acpi_table.c:211: undefined reference to `qemu_fwcfg_read_entry' arch/x86/cpu/built-in.o: In function `bios_linker_allocate': /home/myan/work/u-boot/arch/x86/cpu/qemu/acpi_table.c:41: undefined reference to `qemu_fwcfg_find_file' /home/myan/work/u-boot/arch/x86/cpu/qemu/acpi_table.c:71: undefined reference to `qemu_fwcfg_read_entry' arch/x86/cpu/built-in.o: In function `bios_linker_add_pointer': /home/myan/work/u-boot/arch/x86/cpu/qemu/acpi_table.c:96: undefined reference to `qemu_fwcfg_find_file' /home/myan/work/u-boot/arch/x86/cpu/qemu/acpi_table.c:99: undefined reference to `qemu_fwcfg_find_file' arch/x86/cpu/built-in.o: In function `bios_linker_add_checksum': /home/myan/work/u-boot/arch/x86/cpu/qemu/acpi_table.c:129: undefined reference to `qemu_fwcfg_find_file' arch/x86/cpu/built-in.o: In function `write_acpi_tables': /home/myan/work/u-boot/arch/x86/cpu/qemu/acpi_table.c:239: undefined reference to `qemu_fwcfg_free_files' arch/x86/cpu/built-in.o: In function `qemu_cpu_fixup': /home/myan/work/u-boot/arch/x86/cpu/mp_init.c:454: undefined reference to `qemu_fwcfg_online_cpus' make: *** [u-boot] Error 1
And I still think for this patch, it should depend on x86 && qemu. It doesn't make sense to build qfw for other non-qemu boards.
Tom has the following statement regarding adding qemu dependency to the Kconfig option:
"In general, I want to see the least restrictive set of depends used. This should be "easy" to throw into sandbox for testing there and when we use normal boards under qemu this should also be easy to enable."
I believe Tom wanted to enable this command build testing for Sanbox, hence increase our build coverage. I think more refactoring work needs
Maybe some kind of random/allyes/allno config should be in that build coverage test.
to done to achieve that. Like you said, now QEMU's ACPI table generation depends on CONFIG_CMD_QEMU_FW_CFG. This looks to me a little bit odd. We may further split the qemu_fw_cfg.c to two parts: one only does the command line handling, and the other one does the fw_cfg stuff.
Agreed. I just thought these will be automatically sorted out when we extend qfw to other architectures.
Maybe you can send out some patches to address these?
Looks like Tom has plans for this ?
Thanks, Miao
Regards, Bin