[U-Boot] [PATCH 0/7] add fw_cfg interface support for qemu-x86 targets
The fw_cfg interface provided by QEMU allow guests to retrieve various information about the system, e.g. cpu number, variaous firmware data, kernel setup, etc. The fw_cfg interface can be accessed through 3 IO ports (on x86), using x86 in/out instructions.
- 0x510: select configuration items to access - 0x511: reading this port will return data selected in 0x510 - 0x514: this can be used to detect if DMA interface is available
If fw_cfg DMA interface is available, it can be used to accelerate accesses.
This patchset adds the following supports for qemu-x86 targets:
+ the fw_cfg driver itself
+ add a U-Boot command 'fw' to support direct accessing kernel informtion from fw_cfg interface, this saves the time of loading them from hard disk or network again, through emulated devices.
+ use fw_cfg to get cpu number at runtime, so smp boot no longer relies on the cpu node hard-coded in dts files.
Miao Yan (7): x86: qemu: add fw_cfg support x86: qemu: add a cpu uclass driver for qemu target x86: fix a typo in function name x86: qemu: use actual CPU number for allocating memory x86: qemu: add qemu_fwcfg_fdt_fixup() x86: qemu: fixup cpu node in device tree qemu-x86: add documentaion for the fw_cfg interface
arch/x86/cpu/mp_init.c | 12 +- arch/x86/cpu/qemu/Makefile | 2 +- arch/x86/cpu/qemu/cpu.c | 58 ++++++++ arch/x86/cpu/qemu/fw_cfg.c | 281 +++++++++++++++++++++++++++++++++++++++ arch/x86/cpu/qemu/fw_cfg.h | 85 ++++++++++++ arch/x86/cpu/qemu/qemu.c | 7 + arch/x86/dts/qemu-x86_i440fx.dts | 18 +-- arch/x86/dts/qemu-x86_q35.dts | 19 +-- doc/README.x86 | 36 ++++- 9 files changed, 473 insertions(+), 45 deletions(-) create mode 100644 arch/x86/cpu/qemu/cpu.c create mode 100644 arch/x86/cpu/qemu/fw_cfg.c create mode 100644 arch/x86/cpu/qemu/fw_cfg.h
The QEMU fw_cfg interface allows the guest to retrieve various data information from QEMU. For example, APCI/SMBios tables, number of online cpus, kernel data and command line, etc.
This patch adds support for QEMU fw_cfg interface.
Signed-off-by: Miao Yan yanmiaobest@gmail.com --- arch/x86/cpu/qemu/Makefile | 2 +- arch/x86/cpu/qemu/fw_cfg.c | 215 +++++++++++++++++++++++++++++++++++++++++++++ arch/x86/cpu/qemu/fw_cfg.h | 84 ++++++++++++++++++ arch/x86/cpu/qemu/qemu.c | 3 + 4 files changed, 303 insertions(+), 1 deletion(-) create mode 100644 arch/x86/cpu/qemu/fw_cfg.c create mode 100644 arch/x86/cpu/qemu/fw_cfg.h
diff --git a/arch/x86/cpu/qemu/Makefile b/arch/x86/cpu/qemu/Makefile index 3f3958a..ad424ec 100644 --- a/arch/x86/cpu/qemu/Makefile +++ b/arch/x86/cpu/qemu/Makefile @@ -7,5 +7,5 @@ ifndef CONFIG_EFI_STUB obj-y += car.o dram.o endif -obj-y += qemu.o +obj-y += qemu.o fw_cfg.o obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi.o dsdt.o diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c new file mode 100644 index 0000000..e7615d1 --- /dev/null +++ b/arch/x86/cpu/qemu/fw_cfg.c @@ -0,0 +1,215 @@ +/* + * (C) Copyright 2015 Miao Yan yanmiaoebst@gmail.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <command.h> +#include <environment.h> +#include <stdlib.h> +#include <malloc.h> +#include <errno.h> +#include <fs.h> +#include <asm/io.h> +#include "fw_cfg.h" + +static bool fwcfg_present; +static bool fwcfg_dma_present; + +static void qemu_fwcfg_read_entry_pio(uint16_t entry, + uint32_t size, void *address) +{ + uint32_t i = 0; + uint8_t *data = address; + + if (entry != FW_CFG_INVALID) + outw(entry, FW_CONTROL_PORT); + while (size--) + data[i++] = inb(FW_DATA_PORT); +} + +static void qemu_fwcfg_read_entry_dma(uint16_t entry, + uint32_t length, void *address) +{ + struct fw_cfg_dma_access dma; + + debug("qemu_fwcfg_dma_read_entry: addr %p, length %u control 0x%x\n", + address, length, dma.control); + + dma.length = cpu_to_be32(length); + dma.address = cpu_to_be64((uintptr_t)address); + dma.control = cpu_to_be32(FW_CFG_DMA_READ); + if (entry != FW_CFG_INVALID) + dma.control |= cpu_to_be32(FW_CFG_DMA_SELECT | (entry << 16)); + + barrier(); + + outl(cpu_to_be32((uint32_t)&dma), FW_DMA_PORT + 4); + + while (dma.control & ~FW_CFG_DMA_ERROR) + __asm__ __volatile__ ("rep;nop"); +} + +static int qemu_fwcfg_present(void) +{ + uint32_t qemu; + + qemu_fwcfg_read_entry_pio(FW_CFG_SIGNATURE, 4, &qemu); + return be32_to_cpu(qemu) == QEMU_FW_CFG_SIGNATURE; +} + +static int qemu_fwcfg_dma_present(void) +{ + uint8_t dma_enabled; + uint32_t qemu; + + qemu_fwcfg_read_entry_pio(FW_CFG_ID, 1, &dma_enabled); + if (dma_enabled & 0x1) { + qemu = inl(FW_DMA_PORT); + if (be32_to_cpu(qemu) == QEMU_FW_CFG_SIGNATURE) + return 1; + } + return 0; +} + +static void qemu_fwcfg_read_entry(uint16_t entry, + uint32_t length, void *address) +{ + if (fwcfg_dma_present) + qemu_fwcfg_read_entry_dma(entry, length, address); + else + qemu_fwcfg_read_entry_pio(entry, length, address); +} + +uint16_t qemu_fwcfg_online_cpus(void) +{ + uint16_t nb_cpus; + if (!fwcfg_present) + return 1; + + qemu_fwcfg_read_entry(FW_CFG_NB_CPUS, 2, &nb_cpus); + return nb_cpus; +} + +static int qemu_fwcfg_setup_kernel(void *load_addr) +{ + char *cmd_addr; + uint32_t setup_size, kernel_size, cmdline_size, initrd_size; + + qemu_fwcfg_read_entry(FW_CFG_SETUP_SIZE, 4, &setup_size); + qemu_fwcfg_read_entry(FW_CFG_KERNEL_SIZE, 4, &kernel_size); + + if (setup_size == 0 || kernel_size == 0) { + printf("warning: no kernel available\n"); + return -1; + } + qemu_fwcfg_read_entry(FW_CFG_SETUP_DATA, setup_size, load_addr); + qemu_fwcfg_read_entry(FW_CFG_KERNEL_DATA, kernel_size, + (char *)load_addr + setup_size); + + qemu_fwcfg_read_entry(FW_CFG_INITRD_SIZE, 4, &initrd_size); + if (initrd_size == 0) { + printf("warning: no initrd available\n"); + } else { + qemu_fwcfg_read_entry(FW_CFG_INITRD_DATA, initrd_size, + (char *)load_addr + + setup_size + kernel_size); + } + + qemu_fwcfg_read_entry(FW_CFG_CMDLINE_SIZE, 4, &cmdline_size); + cmd_addr = (char *)load_addr + setup_size + kernel_size + initrd_size; + qemu_fwcfg_read_entry(FW_CFG_CMDLINE_DATA, cmdline_size, cmd_addr); + + printf("loading kernel to address %p", load_addr); + if (initrd_size) + printf(" initrd %p\n", + (char *)load_addr + setup_size + kernel_size); + else + printf("\n"); + + return setenv("bootargs", cmd_addr); +} + +static int qemu_fwcfg_list_firmware(void) +{ + int i; + uint32_t count; + struct fw_cfg_files *files; + + qemu_fwcfg_read_entry(FW_CFG_FILE_DIR, 4, &count); + if (!count) + return 0; + + count = be32_to_cpu(count); + files = malloc(count * sizeof(struct fw_cfg_file)); + if (!files) + return -ENOMEM; + + files->count = count; + qemu_fwcfg_read_entry(FW_CFG_INVALID, + count * sizeof(struct fw_cfg_file), + files->files); + + for (i = 0; i < files->count; i++) + printf("%-56s\n", files->files[i].name); + free(files); + return 0; +} + +void qemu_fwcfg_init(void) +{ + fwcfg_present = false; + fwcfg_dma_present = false; + + if (qemu_fwcfg_present()) { + fwcfg_present = true; + if (qemu_fwcfg_dma_present()) + fwcfg_dma_present = true; + } +} + +int do_qemu_fw(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + void *load_addr; + + if (!fwcfg_present) { + printf("no qemu fw_cfg found\n"); + return 0; + } + + if (!strncmp(argv[1], "list", 4)) { + qemu_fwcfg_list_firmware(); + return 0; + } else if (!strncmp(argv[1], "cpus", 4)) { + printf("%u\n", qemu_fwcfg_online_cpus()); + return 0; + } else if (!strncmp(argv[1], "load", 4)) { + if (argc == 3) { + load_addr = (void *)simple_strtoul(argv[2], NULL, 16); + } else { + load_addr = getenv("loadaddr"); + if (!load_addr) + load_addr = (void *)CONFIG_SYS_LOAD_ADDR; + else + load_addr = (void *)simple_strtoul(load_addr, + NULL, 16); + } + + return qemu_fwcfg_setup_kernel(load_addr); + + } else { + return -1; + } + + return 0; +} + +U_BOOT_CMD( + fw, 3, 1, do_qemu_fw, + "qemu firmware interface", + "<command>\n" + " - list : print firmware(s) currently loaded\n" + " - cpus : print online cpu number\n" + " - load <addr> : load kernel (if any) to address <addr>, and setup for zboot\n" +) diff --git a/arch/x86/cpu/qemu/fw_cfg.h b/arch/x86/cpu/qemu/fw_cfg.h new file mode 100644 index 0000000..66e0c8a --- /dev/null +++ b/arch/x86/cpu/qemu/fw_cfg.h @@ -0,0 +1,84 @@ +/* + * (C) Copyright 2015 Miao Yan yanmiaobest@gmail.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef __FW_CFG__ +#define __FW_CFG__ + +#include <common.h> +#include <asm/io.h> + +#define FW_CONTROL_PORT 0x510 +#define FW_DATA_PORT 0x511 +#define FW_DMA_PORT 0x514 + +#define FW_CFG_SIGNATURE 0x00 +#define FW_CFG_ID 0x01 +#define FW_CFG_UUID 0x02 +#define FW_CFG_RAM_SIZE 0x03 +#define FW_CFG_NOGRAPHIC 0x04 +#define FW_CFG_NB_CPUS 0x05 +#define FW_CFG_MACHINE_ID 0x06 +#define FW_CFG_KERNEL_ADDR 0x07 +#define FW_CFG_KERNEL_SIZE 0x08 +#define FW_CFG_KERNEL_CMDLINE 0x09 +#define FW_CFG_INITRD_ADDR 0x0a +#define FW_CFG_INITRD_SIZE 0x0b +#define FW_CFG_BOOT_DEVICE 0x0c +#define FW_CFG_NUMA 0x0d +#define FW_CFG_BOOT_MENU 0x0e +#define FW_CFG_MAX_CPUS 0x0f +#define FW_CFG_KERNEL_ENTRY 0x10 +#define FW_CFG_KERNEL_DATA 0x11 +#define FW_CFG_INITRD_DATA 0x12 +#define FW_CFG_CMDLINE_ADDR 0x13 +#define FW_CFG_CMDLINE_SIZE 0x14 +#define FW_CFG_CMDLINE_DATA 0x15 +#define FW_CFG_SETUP_ADDR 0x16 +#define FW_CFG_SETUP_SIZE 0x17 +#define FW_CFG_SETUP_DATA 0x18 +#define FW_CFG_FILE_DIR 0x19 + +#define FW_CFG_FILE_FIRST 0x20 +#define FW_CFG_FILE_SLOTS 0x10 +#define FW_CFG_MAX_ENTRY (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS) + +#define FW_CFG_WRITE_CHANNEL 0x4000 +#define FW_CFG_ARCH_LOCAL 0x8000 +#define FW_CFG_ENTRY_MASK ~(FW_CFG_WRITE_CHANNEL | FW_CFG_ARCH_LOCAL) + +#define FW_CFG_INVALID 0xffff + +#define FW_CFG_MAX_FILE_PATH 56 + +#define QEMU_FW_CFG_SIGNATURE (('Q' << 24) | ('E' << 16) | ('M' << 8) | 'U') + +#define FW_CFG_DMA_ERROR (0x1) +#define FW_CFG_DMA_READ (0x2) +#define FW_CFG_DMA_SKIP (0x4) +#define FW_CFG_DMA_SELECT (0x8) + +struct fw_cfg_file { + uint32_t size; + uint16_t select; + uint16_t reserved; + char name[FW_CFG_MAX_FILE_PATH]; +}; + +struct fw_cfg_files { + __be32 count; + struct fw_cfg_file files[]; +}; + +struct fw_cfg_dma_access { + __be32 control; + __be32 length; + __be64 address; +}; + +void qemu_fwcfg_init(void); +uint16_t qemu_fwcfg_online_cpus(void); + +#endif diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c index 84fb082..c0a79d2 100644 --- a/arch/x86/cpu/qemu/qemu.c +++ b/arch/x86/cpu/qemu/qemu.c @@ -11,6 +11,7 @@ #include <asm/processor.h> #include <asm/arch/device.h> #include <asm/arch/qemu.h> +#include "fw_cfg.h"
static bool i440fx;
@@ -57,6 +58,8 @@ static void qemu_chipset_init(void) x86_pci_write_config32(PCI_BDF(0, 0, 0), PCIEX_BAR, CONFIG_PCIE_ECAM_BASE | BAR_EN); } + + qemu_fwcfg_init(); }
int arch_cpu_init(void)
Hi Miao,
On Mon, Dec 28, 2015 at 5:18 PM, Miao Yan yanmiaobest@gmail.com wrote:
The QEMU fw_cfg interface allows the guest to retrieve various data information from QEMU. For example, APCI/SMBios tables, number of online cpus, kernel data and command line, etc.
This patch adds support for QEMU fw_cfg interface.
Signed-off-by: Miao Yan yanmiaobest@gmail.com
arch/x86/cpu/qemu/Makefile | 2 +- arch/x86/cpu/qemu/fw_cfg.c | 215 +++++++++++++++++++++++++++++++++++++++++++++ arch/x86/cpu/qemu/fw_cfg.h | 84 ++++++++++++++++++ arch/x86/cpu/qemu/qemu.c | 3 + 4 files changed, 303 insertions(+), 1 deletion(-) create mode 100644 arch/x86/cpu/qemu/fw_cfg.c create mode 100644 arch/x86/cpu/qemu/fw_cfg.h
diff --git a/arch/x86/cpu/qemu/Makefile b/arch/x86/cpu/qemu/Makefile index 3f3958a..ad424ec 100644 --- a/arch/x86/cpu/qemu/Makefile +++ b/arch/x86/cpu/qemu/Makefile @@ -7,5 +7,5 @@ ifndef CONFIG_EFI_STUB obj-y += car.o dram.o endif -obj-y += qemu.o +obj-y += qemu.o fw_cfg.o obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi.o dsdt.o diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c new file mode 100644 index 0000000..e7615d1 --- /dev/null +++ b/arch/x86/cpu/qemu/fw_cfg.c @@ -0,0 +1,215 @@ +/*
- (C) Copyright 2015 Miao Yan yanmiaoebst@gmail.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <command.h> +#include <environment.h> +#include <stdlib.h>
environment.h and stdlib.h are not needed.
+#include <malloc.h> +#include <errno.h>
nits: move <error.h> to before <malloc.h>
+#include <fs.h>
fs.h is not needed.
+#include <asm/io.h> +#include "fw_cfg.h"
+static bool fwcfg_present; +static bool fwcfg_dma_present;
+static void qemu_fwcfg_read_entry_pio(uint16_t entry,
nits: please leave only one space between 'static' and 'void'
uint32_t size, void *address)+{
uint32_t i = 0;uint8_t *data = address;if (entry != FW_CFG_INVALID)outw(entry, FW_CONTROL_PORT);
Missing branch to handle (entry == FW_CFG_INVALID).
while (size--)data[i++] = inb(FW_DATA_PORT);+}
+static void qemu_fwcfg_read_entry_dma(uint16_t entry,
nits: please leave only one space between 'static' and 'void'
uint32_t length, void *address)
To keep consistency with qemu_fwcfg_read_entry_pio(), either change 'length' to 'size', or change 'size' parameter of qemu_fwcfg_read_entry_pio() to 'length'.
+{
struct fw_cfg_dma_access dma;debug("qemu_fwcfg_dma_read_entry: addr %p, length %u control 0x%x\n",address, length, dma.control);
dma.control is not initialized yet. We need move this debug before "barrier()" below.
dma.length = cpu_to_be32(length);dma.address = cpu_to_be64((uintptr_t)address);dma.control = cpu_to_be32(FW_CFG_DMA_READ);if (entry != FW_CFG_INVALID)dma.control |= cpu_to_be32(FW_CFG_DMA_SELECT | (entry << 16));
Missing branch to handle (entry == FW_CFG_INVALID).
barrier();outl(cpu_to_be32((uint32_t)&dma), FW_DMA_PORT + 4);
#define this "FW_DMA_PORT + 4" to a port macro?
while (dma.control & ~FW_CFG_DMA_ERROR)__asm__ __volatile__ ("rep;nop");
Just use "pause" instruction.
+}
+static int qemu_fwcfg_present(void)
int -> bool
+{
uint32_t qemu;qemu_fwcfg_read_entry_pio(FW_CFG_SIGNATURE, 4, &qemu);return be32_to_cpu(qemu) == QEMU_FW_CFG_SIGNATURE;+}
+static int qemu_fwcfg_dma_present(void)
int -> bool
+{
uint8_t dma_enabled;uint32_t qemu;qemu_fwcfg_read_entry_pio(FW_CFG_ID, 1, &dma_enabled);if (dma_enabled & 0x1) {
#define 0x1 to something meaningful?
qemu = inl(FW_DMA_PORT);if (be32_to_cpu(qemu) == QEMU_FW_CFG_SIGNATURE)return 1;}return 0;+}
+static void qemu_fwcfg_read_entry(uint16_t entry,
uint32_t length, void *address)+{
if (fwcfg_dma_present)qemu_fwcfg_read_entry_dma(entry, length, address);elseqemu_fwcfg_read_entry_pio(entry, length, address);+}
+uint16_t qemu_fwcfg_online_cpus(void)
Can we change the return value to int?
+{
uint16_t nb_cpus;
nits: needs a blank line here.
if (!fwcfg_present)return 1;qemu_fwcfg_read_entry(FW_CFG_NB_CPUS, 2, &nb_cpus);
No need to be16_to_cpu()? How do we know which has endian problem? Is this documented somewhere in the QEMU doc?
nits: needs a blank line here.
return nb_cpus;+}
+static int qemu_fwcfg_setup_kernel(void *load_addr) +{
char *cmd_addr;
We can rename this to something like: char *data_addr = load_addr; then (see below)
uint32_t setup_size, kernel_size, cmdline_size, initrd_size;qemu_fwcfg_read_entry(FW_CFG_SETUP_SIZE, 4, &setup_size);qemu_fwcfg_read_entry(FW_CFG_KERNEL_SIZE, 4, &kernel_size);
No need to endian conversion for setup_size and kernel_size?
if (setup_size == 0 || kernel_size == 0) {printf("warning: no kernel available\n");return -1;}qemu_fwcfg_read_entry(FW_CFG_SETUP_DATA, setup_size, load_addr);
load_addr -> data_addr
and do: data_addr += setup_size;
qemu_fwcfg_read_entry(FW_CFG_KERNEL_DATA, kernel_size,(char *)load_addr + setup_size);
load_addr + setup_size -> data_addr;
qemu_fwcfg_read_entry(FW_CFG_INITRD_SIZE, 4, &initrd_size);if (initrd_size == 0) {printf("warning: no initrd available\n");} else {
data_addr += kernel_size;
qemu_fwcfg_read_entry(FW_CFG_INITRD_DATA, initrd_size,(char *)load_addr +setup_size + kernel_size);
load_addr + setup_size + kernel_size -> data_addr;
}qemu_fwcfg_read_entry(FW_CFG_CMDLINE_SIZE, 4, &cmdline_size);cmd_addr = (char *)load_addr + setup_size + kernel_size + initrd_size;
data_addr += initrd_size;
qemu_fwcfg_read_entry(FW_CFG_CMDLINE_DATA, cmdline_size, cmd_addr);
cmd_addr -> data_addr;
printf("loading kernel to address %p", load_addr);if (initrd_size)printf(" initrd %p\n",(char *)load_addr + setup_size + kernel_size);elseprintf("\n");return setenv("bootargs", cmd_addr);+}
+static int qemu_fwcfg_list_firmware(void) +{
int i;uint32_t count;struct fw_cfg_files *files;qemu_fwcfg_read_entry(FW_CFG_FILE_DIR, 4, &count);if (!count)return 0;count = be32_to_cpu(count);files = malloc(count * sizeof(struct fw_cfg_file));if (!files)return -ENOMEM;files->count = count;qemu_fwcfg_read_entry(FW_CFG_INVALID,count * sizeof(struct fw_cfg_file),files->files);for (i = 0; i < files->count; i++)printf("%-56s\n", files->files[i].name);free(files);
nits: needs a blank line here.
return 0;+}
+void qemu_fwcfg_init(void) +{
fwcfg_present = false;fwcfg_dma_present = false;if (qemu_fwcfg_present()) {fwcfg_present = true;if (qemu_fwcfg_dma_present())fwcfg_dma_present = true;}
This function can be simplified to:
fwcfg_present = qemu_fwcfg_present(); if (fwcfg_present) fwcfg_dma_present = qemu_fwcfg_dma_present();
+}
+int do_qemu_fw(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{
void *load_addr;if (!fwcfg_present) {printf("no qemu fw_cfg found\n");return 0;}if (!strncmp(argv[1], "list", 4)) {qemu_fwcfg_list_firmware();return 0;} else if (!strncmp(argv[1], "cpus", 4)) {printf("%u\n", qemu_fwcfg_online_cpus());
Can we put more words here instead of just printing the cpu numbers?
return 0;} else if (!strncmp(argv[1], "load", 4)) {if (argc == 3) {load_addr = (void *)simple_strtoul(argv[2], NULL, 16);} else {load_addr = getenv("loadaddr");if (!load_addr)load_addr = (void *)CONFIG_SYS_LOAD_ADDR;elseload_addr = (void *)simple_strtoul(load_addr,NULL, 16);}return qemu_fwcfg_setup_kernel(load_addr);} else {return -1;}
Can you rewrite the command logic utilizing find_cmd_tbl() and cmd_process_error()? See arch/x86/lib/fsp/cmd_fsp.c for example.
return 0;+}
+U_BOOT_CMD(
fw, 3, 1, do_qemu_fw,"qemu firmware interface",
nits: qemu -> QEMU
"<command>\n"" - list : print firmware(s) currently loaded\n"" - cpus : print online cpu number\n"" - load <addr> : load kernel (if any) to address <addr>, and setup for zboot\n"+) diff --git a/arch/x86/cpu/qemu/fw_cfg.h b/arch/x86/cpu/qemu/fw_cfg.h new file mode 100644 index 0000000..66e0c8a --- /dev/null +++ b/arch/x86/cpu/qemu/fw_cfg.h @@ -0,0 +1,84 @@ +/*
- (C) Copyright 2015 Miao Yan yanmiaobest@gmail.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#ifndef __FW_CFG__ +#define __FW_CFG__
+#include <common.h> +#include <asm/io.h>
Not needed. Please remove these two includes.
+#define FW_CONTROL_PORT 0x510 +#define FW_DATA_PORT 0x511 +#define FW_DMA_PORT 0x514
+#define FW_CFG_SIGNATURE 0x00 +#define FW_CFG_ID 0x01 +#define FW_CFG_UUID 0x02 +#define FW_CFG_RAM_SIZE 0x03 +#define FW_CFG_NOGRAPHIC 0x04 +#define FW_CFG_NB_CPUS 0x05 +#define FW_CFG_MACHINE_ID 0x06 +#define FW_CFG_KERNEL_ADDR 0x07 +#define FW_CFG_KERNEL_SIZE 0x08 +#define FW_CFG_KERNEL_CMDLINE 0x09 +#define FW_CFG_INITRD_ADDR 0x0a +#define FW_CFG_INITRD_SIZE 0x0b +#define FW_CFG_BOOT_DEVICE 0x0c +#define FW_CFG_NUMA 0x0d +#define FW_CFG_BOOT_MENU 0x0e +#define FW_CFG_MAX_CPUS 0x0f +#define FW_CFG_KERNEL_ENTRY 0x10 +#define FW_CFG_KERNEL_DATA 0x11 +#define FW_CFG_INITRD_DATA 0x12 +#define FW_CFG_CMDLINE_ADDR 0x13 +#define FW_CFG_CMDLINE_SIZE 0x14 +#define FW_CFG_CMDLINE_DATA 0x15 +#define FW_CFG_SETUP_ADDR 0x16 +#define FW_CFG_SETUP_SIZE 0x17 +#define FW_CFG_SETUP_DATA 0x18 +#define FW_CFG_FILE_DIR 0x19
Can we use enum for the above entries?
+#define FW_CFG_FILE_FIRST 0x20 +#define FW_CFG_FILE_SLOTS 0x10 +#define FW_CFG_MAX_ENTRY (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
+#define FW_CFG_WRITE_CHANNEL 0x4000 +#define FW_CFG_ARCH_LOCAL 0x8000 +#define FW_CFG_ENTRY_MASK ~(FW_CFG_WRITE_CHANNEL | FW_CFG_ARCH_LOCAL)
+#define FW_CFG_INVALID 0xffff
+#define FW_CFG_MAX_FILE_PATH 56
+#define QEMU_FW_CFG_SIGNATURE (('Q' << 24) | ('E' << 16) | ('M' << 8) | 'U')
+#define FW_CFG_DMA_ERROR (0x1) +#define FW_CFG_DMA_READ (0x2) +#define FW_CFG_DMA_SKIP (0x4) +#define FW_CFG_DMA_SELECT (0x8)
For above 4, we can use (1 << 0) / (1 << 1) / (1 << 2) / (1 << 3) to indicate they are bit definitions.
+struct fw_cfg_file {
uint32_t size;uint16_t select;uint16_t reserved;char name[FW_CFG_MAX_FILE_PATH];+};
+struct fw_cfg_files {
__be32 count;struct fw_cfg_file files[];
Ah, this fw_cfg interface is weird. count is big endian, but fw_cfg_files is not.
+};
+struct fw_cfg_dma_access {
__be32 control;__be32 length;__be64 address;
These are big endians again. Weird.
+};
+void qemu_fwcfg_init(void); +uint16_t qemu_fwcfg_online_cpus(void);
Can you add the comment header block for these two functions?
+#endif diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c index 84fb082..c0a79d2 100644 --- a/arch/x86/cpu/qemu/qemu.c +++ b/arch/x86/cpu/qemu/qemu.c @@ -11,6 +11,7 @@ #include <asm/processor.h> #include <asm/arch/device.h> #include <asm/arch/qemu.h> +#include "fw_cfg.h"
static bool i440fx;
@@ -57,6 +58,8 @@ static void qemu_chipset_init(void) x86_pci_write_config32(PCI_BDF(0, 0, 0), PCIEX_BAR, CONFIG_PCIE_ECAM_BASE | BAR_EN); }
qemu_fwcfg_init();}
int arch_cpu_init(void)
Regards, Bin
Hi Bin,
2015-12-29 14:19 GMT+08:00 Bin Meng bmeng.cn@gmail.com:
Hi Miao,
On Mon, Dec 28, 2015 at 5:18 PM, Miao Yan yanmiaobest@gmail.com wrote:
The QEMU fw_cfg interface allows the guest to retrieve various data information from QEMU. For example, APCI/SMBios tables, number of online cpus, kernel data and command line, etc.
This patch adds support for QEMU fw_cfg interface.
Signed-off-by: Miao Yan yanmiaobest@gmail.com
arch/x86/cpu/qemu/Makefile | 2 +- arch/x86/cpu/qemu/fw_cfg.c | 215 +++++++++++++++++++++++++++++++++++++++++++++ arch/x86/cpu/qemu/fw_cfg.h | 84 ++++++++++++++++++ arch/x86/cpu/qemu/qemu.c | 3 + 4 files changed, 303 insertions(+), 1 deletion(-) create mode 100644 arch/x86/cpu/qemu/fw_cfg.c create mode 100644 arch/x86/cpu/qemu/fw_cfg.h
diff --git a/arch/x86/cpu/qemu/Makefile b/arch/x86/cpu/qemu/Makefile index 3f3958a..ad424ec 100644 --- a/arch/x86/cpu/qemu/Makefile +++ b/arch/x86/cpu/qemu/Makefile @@ -7,5 +7,5 @@ ifndef CONFIG_EFI_STUB obj-y += car.o dram.o endif -obj-y += qemu.o +obj-y += qemu.o fw_cfg.o obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi.o dsdt.o diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c new file mode 100644 index 0000000..e7615d1 --- /dev/null +++ b/arch/x86/cpu/qemu/fw_cfg.c @@ -0,0 +1,215 @@ +/*
- (C) Copyright 2015 Miao Yan yanmiaoebst@gmail.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <command.h> +#include <environment.h> +#include <stdlib.h>
environment.h and stdlib.h are not needed.
+#include <malloc.h> +#include <errno.h>
nits: move <error.h> to before <malloc.h>
+#include <fs.h>
fs.h is not needed.
+#include <asm/io.h> +#include "fw_cfg.h"
+static bool fwcfg_present; +static bool fwcfg_dma_present;
+static void qemu_fwcfg_read_entry_pio(uint16_t entry,
nits: please leave only one space between 'static' and 'void'
uint32_t size, void *address)+{
uint32_t i = 0;uint8_t *data = address;if (entry != FW_CFG_INVALID)outw(entry, FW_CONTROL_PORT);Missing branch to handle (entry == FW_CFG_INVALID).
This is on purpose. QEMU has a 'offset' associated with the selected data items, writting 'entry' will reset 'offset' to zero. If you want to do more than one read on a data item, then subsequent reads should pass 'FW_CFG_INVALID' so it won't start reading from the beginning again.
But maybe it deserves a better interface. Any idea ?
while (size--)data[i++] = inb(FW_DATA_PORT);+}
+static void qemu_fwcfg_read_entry_dma(uint16_t entry,
nits: please leave only one space between 'static' and 'void'
uint32_t length, void *address)To keep consistency with qemu_fwcfg_read_entry_pio(), either change 'length' to 'size', or change 'size' parameter of qemu_fwcfg_read_entry_pio() to 'length'.
+{
struct fw_cfg_dma_access dma;debug("qemu_fwcfg_dma_read_entry: addr %p, length %u control 0x%x\n",address, length, dma.control);dma.control is not initialized yet. We need move this debug before "barrier()" below.
dma.length = cpu_to_be32(length);dma.address = cpu_to_be64((uintptr_t)address);dma.control = cpu_to_be32(FW_CFG_DMA_READ);if (entry != FW_CFG_INVALID)dma.control |= cpu_to_be32(FW_CFG_DMA_SELECT | (entry << 16));Missing branch to handle (entry == FW_CFG_INVALID).
See above.
barrier();outl(cpu_to_be32((uint32_t)&dma), FW_DMA_PORT + 4);#define this "FW_DMA_PORT + 4" to a port macro?
while (dma.control & ~FW_CFG_DMA_ERROR)__asm__ __volatile__ ("rep;nop");Just use "pause" instruction.
+}
+static int qemu_fwcfg_present(void)
int -> bool
+{
uint32_t qemu;qemu_fwcfg_read_entry_pio(FW_CFG_SIGNATURE, 4, &qemu);return be32_to_cpu(qemu) == QEMU_FW_CFG_SIGNATURE;+}
+static int qemu_fwcfg_dma_present(void)
int -> bool
+{
uint8_t dma_enabled;uint32_t qemu;qemu_fwcfg_read_entry_pio(FW_CFG_ID, 1, &dma_enabled);if (dma_enabled & 0x1) {#define 0x1 to something meaningful?
qemu = inl(FW_DMA_PORT);if (be32_to_cpu(qemu) == QEMU_FW_CFG_SIGNATURE)return 1;}return 0;+}
+static void qemu_fwcfg_read_entry(uint16_t entry,
uint32_t length, void *address)+{
if (fwcfg_dma_present)qemu_fwcfg_read_entry_dma(entry, length, address);elseqemu_fwcfg_read_entry_pio(entry, length, address);+}
+uint16_t qemu_fwcfg_online_cpus(void)
Can we change the return value to int?
+{
uint16_t nb_cpus;nits: needs a blank line here.
if (!fwcfg_present)return 1;qemu_fwcfg_read_entry(FW_CFG_NB_CPUS, 2, &nb_cpus);No need to be16_to_cpu()? How do we know which has endian problem? Is this documented somewhere in the QEMU doc?
nits: needs a blank line here.
return nb_cpus;+}
+static int qemu_fwcfg_setup_kernel(void *load_addr) +{
char *cmd_addr;We can rename this to something like: char *data_addr = load_addr; then (see below)
uint32_t setup_size, kernel_size, cmdline_size, initrd_size;qemu_fwcfg_read_entry(FW_CFG_SETUP_SIZE, 4, &setup_size);qemu_fwcfg_read_entry(FW_CFG_KERNEL_SIZE, 4, &kernel_size);No need to endian conversion for setup_size and kernel_size?
if (setup_size == 0 || kernel_size == 0) {printf("warning: no kernel available\n");return -1;}qemu_fwcfg_read_entry(FW_CFG_SETUP_DATA, setup_size, load_addr);load_addr -> data_addr
and do: data_addr += setup_size;
qemu_fwcfg_read_entry(FW_CFG_KERNEL_DATA, kernel_size,(char *)load_addr + setup_size);load_addr + setup_size -> data_addr;
qemu_fwcfg_read_entry(FW_CFG_INITRD_SIZE, 4, &initrd_size);if (initrd_size == 0) {printf("warning: no initrd available\n");} else {data_addr += kernel_size;
qemu_fwcfg_read_entry(FW_CFG_INITRD_DATA, initrd_size,(char *)load_addr +setup_size + kernel_size);load_addr + setup_size + kernel_size -> data_addr;
}qemu_fwcfg_read_entry(FW_CFG_CMDLINE_SIZE, 4, &cmdline_size);cmd_addr = (char *)load_addr + setup_size + kernel_size + initrd_size;data_addr += initrd_size;
qemu_fwcfg_read_entry(FW_CFG_CMDLINE_DATA, cmdline_size, cmd_addr);cmd_addr -> data_addr;
printf("loading kernel to address %p", load_addr);if (initrd_size)printf(" initrd %p\n",(char *)load_addr + setup_size + kernel_size);elseprintf("\n");return setenv("bootargs", cmd_addr);+}
+static int qemu_fwcfg_list_firmware(void) +{
int i;uint32_t count;struct fw_cfg_files *files;qemu_fwcfg_read_entry(FW_CFG_FILE_DIR, 4, &count);if (!count)return 0;count = be32_to_cpu(count);files = malloc(count * sizeof(struct fw_cfg_file));if (!files)return -ENOMEM;files->count = count;qemu_fwcfg_read_entry(FW_CFG_INVALID,count * sizeof(struct fw_cfg_file),files->files);for (i = 0; i < files->count; i++)printf("%-56s\n", files->files[i].name);free(files);nits: needs a blank line here.
return 0;+}
+void qemu_fwcfg_init(void) +{
fwcfg_present = false;fwcfg_dma_present = false;if (qemu_fwcfg_present()) {fwcfg_present = true;if (qemu_fwcfg_dma_present())fwcfg_dma_present = true;}This function can be simplified to:
fwcfg_present = qemu_fwcfg_present(); if (fwcfg_present) fwcfg_dma_present = qemu_fwcfg_dma_present();+}
+int do_qemu_fw(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{
void *load_addr;if (!fwcfg_present) {printf("no qemu fw_cfg found\n");return 0;}if (!strncmp(argv[1], "list", 4)) {qemu_fwcfg_list_firmware();return 0;} else if (!strncmp(argv[1], "cpus", 4)) {printf("%u\n", qemu_fwcfg_online_cpus());Can we put more words here instead of just printing the cpu numbers?
What words do you have in mind ? "cpus number" ?
return 0;} else if (!strncmp(argv[1], "load", 4)) {if (argc == 3) {load_addr = (void *)simple_strtoul(argv[2], NULL, 16);} else {load_addr = getenv("loadaddr");if (!load_addr)load_addr = (void *)CONFIG_SYS_LOAD_ADDR;elseload_addr = (void *)simple_strtoul(load_addr,NULL, 16);}return qemu_fwcfg_setup_kernel(load_addr);} else {return -1;}Can you rewrite the command logic utilizing find_cmd_tbl() and cmd_process_error()? See arch/x86/lib/fsp/cmd_fsp.c for example.
return 0;+}
+U_BOOT_CMD(
fw, 3, 1, do_qemu_fw,"qemu firmware interface",nits: qemu -> QEMU
"<command>\n"" - list : print firmware(s) currently loaded\n"" - cpus : print online cpu number\n"" - load <addr> : load kernel (if any) to address <addr>, and setup for zboot\n"+) diff --git a/arch/x86/cpu/qemu/fw_cfg.h b/arch/x86/cpu/qemu/fw_cfg.h new file mode 100644 index 0000000..66e0c8a --- /dev/null +++ b/arch/x86/cpu/qemu/fw_cfg.h @@ -0,0 +1,84 @@ +/*
- (C) Copyright 2015 Miao Yan yanmiaobest@gmail.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#ifndef __FW_CFG__ +#define __FW_CFG__
+#include <common.h> +#include <asm/io.h>
Not needed. Please remove these two includes.
+#define FW_CONTROL_PORT 0x510 +#define FW_DATA_PORT 0x511 +#define FW_DMA_PORT 0x514
+#define FW_CFG_SIGNATURE 0x00 +#define FW_CFG_ID 0x01 +#define FW_CFG_UUID 0x02 +#define FW_CFG_RAM_SIZE 0x03 +#define FW_CFG_NOGRAPHIC 0x04 +#define FW_CFG_NB_CPUS 0x05 +#define FW_CFG_MACHINE_ID 0x06 +#define FW_CFG_KERNEL_ADDR 0x07 +#define FW_CFG_KERNEL_SIZE 0x08 +#define FW_CFG_KERNEL_CMDLINE 0x09 +#define FW_CFG_INITRD_ADDR 0x0a +#define FW_CFG_INITRD_SIZE 0x0b +#define FW_CFG_BOOT_DEVICE 0x0c +#define FW_CFG_NUMA 0x0d +#define FW_CFG_BOOT_MENU 0x0e +#define FW_CFG_MAX_CPUS 0x0f +#define FW_CFG_KERNEL_ENTRY 0x10 +#define FW_CFG_KERNEL_DATA 0x11 +#define FW_CFG_INITRD_DATA 0x12 +#define FW_CFG_CMDLINE_ADDR 0x13 +#define FW_CFG_CMDLINE_SIZE 0x14 +#define FW_CFG_CMDLINE_DATA 0x15 +#define FW_CFG_SETUP_ADDR 0x16 +#define FW_CFG_SETUP_SIZE 0x17 +#define FW_CFG_SETUP_DATA 0x18 +#define FW_CFG_FILE_DIR 0x19
Can we use enum for the above entries?
+#define FW_CFG_FILE_FIRST 0x20 +#define FW_CFG_FILE_SLOTS 0x10 +#define FW_CFG_MAX_ENTRY (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
+#define FW_CFG_WRITE_CHANNEL 0x4000 +#define FW_CFG_ARCH_LOCAL 0x8000 +#define FW_CFG_ENTRY_MASK ~(FW_CFG_WRITE_CHANNEL | FW_CFG_ARCH_LOCAL)
+#define FW_CFG_INVALID 0xffff
+#define FW_CFG_MAX_FILE_PATH 56
+#define QEMU_FW_CFG_SIGNATURE (('Q' << 24) | ('E' << 16) | ('M' << 8) | 'U')
+#define FW_CFG_DMA_ERROR (0x1) +#define FW_CFG_DMA_READ (0x2) +#define FW_CFG_DMA_SKIP (0x4) +#define FW_CFG_DMA_SELECT (0x8)
For above 4, we can use (1 << 0) / (1 << 1) / (1 << 2) / (1 << 3) to indicate they are bit definitions.
+struct fw_cfg_file {
uint32_t size;uint16_t select;uint16_t reserved;char name[FW_CFG_MAX_FILE_PATH];+};
+struct fw_cfg_files {
__be32 count;struct fw_cfg_file files[];Ah, this fw_cfg interface is weird. count is big endian, but fw_cfg_files is not.
fw_cfg_file is big endian. I forgot to change them. Will fix.
+};
+struct fw_cfg_dma_access {
__be32 control;__be32 length;__be64 address;These are big endians again. Weird.
+};
+void qemu_fwcfg_init(void); +uint16_t qemu_fwcfg_online_cpus(void);
Can you add the comment header block for these two functions?
+#endif diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c index 84fb082..c0a79d2 100644 --- a/arch/x86/cpu/qemu/qemu.c +++ b/arch/x86/cpu/qemu/qemu.c @@ -11,6 +11,7 @@ #include <asm/processor.h> #include <asm/arch/device.h> #include <asm/arch/qemu.h> +#include "fw_cfg.h"
static bool i440fx;
@@ -57,6 +58,8 @@ static void qemu_chipset_init(void) x86_pci_write_config32(PCI_BDF(0, 0, 0), PCIEX_BAR, CONFIG_PCIE_ECAM_BASE | BAR_EN); }
qemu_fwcfg_init();}
int arch_cpu_init(void)
Regards, Bin
I'll fix the rest of your comments. Thanks for the review.
Miao
Hi Miao,
On Tue, Dec 29, 2015 at 2:41 PM, Miao Yan yanmiaobest@gmail.com wrote:
Hi Bin,
2015-12-29 14:19 GMT+08:00 Bin Meng bmeng.cn@gmail.com:
Hi Miao,
On Mon, Dec 28, 2015 at 5:18 PM, Miao Yan yanmiaobest@gmail.com wrote:
The QEMU fw_cfg interface allows the guest to retrieve various data information from QEMU. For example, APCI/SMBios tables, number of online cpus, kernel data and command line, etc.
This patch adds support for QEMU fw_cfg interface.
Signed-off-by: Miao Yan yanmiaobest@gmail.com
arch/x86/cpu/qemu/Makefile | 2 +- arch/x86/cpu/qemu/fw_cfg.c | 215 +++++++++++++++++++++++++++++++++++++++++++++ arch/x86/cpu/qemu/fw_cfg.h | 84 ++++++++++++++++++ arch/x86/cpu/qemu/qemu.c | 3 + 4 files changed, 303 insertions(+), 1 deletion(-) create mode 100644 arch/x86/cpu/qemu/fw_cfg.c create mode 100644 arch/x86/cpu/qemu/fw_cfg.h
diff --git a/arch/x86/cpu/qemu/Makefile b/arch/x86/cpu/qemu/Makefile index 3f3958a..ad424ec 100644 --- a/arch/x86/cpu/qemu/Makefile +++ b/arch/x86/cpu/qemu/Makefile @@ -7,5 +7,5 @@ ifndef CONFIG_EFI_STUB obj-y += car.o dram.o endif -obj-y += qemu.o +obj-y += qemu.o fw_cfg.o obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi.o dsdt.o diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c new file mode 100644 index 0000000..e7615d1 --- /dev/null +++ b/arch/x86/cpu/qemu/fw_cfg.c @@ -0,0 +1,215 @@ +/*
- (C) Copyright 2015 Miao Yan yanmiaoebst@gmail.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <command.h> +#include <environment.h> +#include <stdlib.h>
environment.h and stdlib.h are not needed.
+#include <malloc.h> +#include <errno.h>
nits: move <error.h> to before <malloc.h>
+#include <fs.h>
fs.h is not needed.
+#include <asm/io.h> +#include "fw_cfg.h"
+static bool fwcfg_present; +static bool fwcfg_dma_present;
+static void qemu_fwcfg_read_entry_pio(uint16_t entry,
nits: please leave only one space between 'static' and 'void'
uint32_t size, void *address)+{
uint32_t i = 0;uint8_t *data = address;if (entry != FW_CFG_INVALID)outw(entry, FW_CONTROL_PORT);Missing branch to handle (entry == FW_CFG_INVALID).
This is on purpose. QEMU has a 'offset' associated with the selected data items, writting 'entry' will reset 'offset' to zero. If you want to do more than one read on a data item, then subsequent reads should pass 'FW_CFG_INVALID' so it won't start reading from the beginning again.
But maybe it deserves a better interface. Any idea ?
OK, this makes sense. Can you please add a comment block here?
while (size--)data[i++] = inb(FW_DATA_PORT);+}
+static void qemu_fwcfg_read_entry_dma(uint16_t entry,
nits: please leave only one space between 'static' and 'void'
uint32_t length, void *address)To keep consistency with qemu_fwcfg_read_entry_pio(), either change 'length' to 'size', or change 'size' parameter of qemu_fwcfg_read_entry_pio() to 'length'.
+{
struct fw_cfg_dma_access dma;debug("qemu_fwcfg_dma_read_entry: addr %p, length %u control 0x%x\n",address, length, dma.control);dma.control is not initialized yet. We need move this debug before "barrier()" below.
dma.length = cpu_to_be32(length);dma.address = cpu_to_be64((uintptr_t)address);dma.control = cpu_to_be32(FW_CFG_DMA_READ);if (entry != FW_CFG_INVALID)dma.control |= cpu_to_be32(FW_CFG_DMA_SELECT | (entry << 16));Missing branch to handle (entry == FW_CFG_INVALID).
See above.
barrier();outl(cpu_to_be32((uint32_t)&dma), FW_DMA_PORT + 4);#define this "FW_DMA_PORT + 4" to a port macro?
while (dma.control & ~FW_CFG_DMA_ERROR)__asm__ __volatile__ ("rep;nop");Just use "pause" instruction.
+}
+static int qemu_fwcfg_present(void)
int -> bool
+{
uint32_t qemu;qemu_fwcfg_read_entry_pio(FW_CFG_SIGNATURE, 4, &qemu);return be32_to_cpu(qemu) == QEMU_FW_CFG_SIGNATURE;+}
+static int qemu_fwcfg_dma_present(void)
int -> bool
+{
uint8_t dma_enabled;uint32_t qemu;qemu_fwcfg_read_entry_pio(FW_CFG_ID, 1, &dma_enabled);if (dma_enabled & 0x1) {#define 0x1 to something meaningful?
qemu = inl(FW_DMA_PORT);if (be32_to_cpu(qemu) == QEMU_FW_CFG_SIGNATURE)return 1;}return 0;+}
+static void qemu_fwcfg_read_entry(uint16_t entry,
uint32_t length, void *address)+{
if (fwcfg_dma_present)qemu_fwcfg_read_entry_dma(entry, length, address);elseqemu_fwcfg_read_entry_pio(entry, length, address);+}
+uint16_t qemu_fwcfg_online_cpus(void)
Can we change the return value to int?
+{
uint16_t nb_cpus;nits: needs a blank line here.
if (!fwcfg_present)return 1;qemu_fwcfg_read_entry(FW_CFG_NB_CPUS, 2, &nb_cpus);No need to be16_to_cpu()? How do we know which has endian problem? Is this documented somewhere in the QEMU doc?
nits: needs a blank line here.
return nb_cpus;+}
+static int qemu_fwcfg_setup_kernel(void *load_addr) +{
char *cmd_addr;We can rename this to something like: char *data_addr = load_addr; then (see below)
uint32_t setup_size, kernel_size, cmdline_size, initrd_size;qemu_fwcfg_read_entry(FW_CFG_SETUP_SIZE, 4, &setup_size);qemu_fwcfg_read_entry(FW_CFG_KERNEL_SIZE, 4, &kernel_size);No need to endian conversion for setup_size and kernel_size?
if (setup_size == 0 || kernel_size == 0) {printf("warning: no kernel available\n");return -1;}qemu_fwcfg_read_entry(FW_CFG_SETUP_DATA, setup_size, load_addr);load_addr -> data_addr
and do: data_addr += setup_size;
qemu_fwcfg_read_entry(FW_CFG_KERNEL_DATA, kernel_size,(char *)load_addr + setup_size);load_addr + setup_size -> data_addr;
qemu_fwcfg_read_entry(FW_CFG_INITRD_SIZE, 4, &initrd_size);if (initrd_size == 0) {printf("warning: no initrd available\n");} else {data_addr += kernel_size;
qemu_fwcfg_read_entry(FW_CFG_INITRD_DATA, initrd_size,(char *)load_addr +setup_size + kernel_size);load_addr + setup_size + kernel_size -> data_addr;
}qemu_fwcfg_read_entry(FW_CFG_CMDLINE_SIZE, 4, &cmdline_size);cmd_addr = (char *)load_addr + setup_size + kernel_size + initrd_size;data_addr += initrd_size;
qemu_fwcfg_read_entry(FW_CFG_CMDLINE_DATA, cmdline_size, cmd_addr);cmd_addr -> data_addr;
printf("loading kernel to address %p", load_addr);if (initrd_size)printf(" initrd %p\n",(char *)load_addr + setup_size + kernel_size);elseprintf("\n");return setenv("bootargs", cmd_addr);+}
+static int qemu_fwcfg_list_firmware(void) +{
int i;uint32_t count;struct fw_cfg_files *files;qemu_fwcfg_read_entry(FW_CFG_FILE_DIR, 4, &count);if (!count)return 0;count = be32_to_cpu(count);files = malloc(count * sizeof(struct fw_cfg_file));if (!files)return -ENOMEM;files->count = count;qemu_fwcfg_read_entry(FW_CFG_INVALID,count * sizeof(struct fw_cfg_file),files->files);for (i = 0; i < files->count; i++)printf("%-56s\n", files->files[i].name);free(files);nits: needs a blank line here.
return 0;+}
+void qemu_fwcfg_init(void) +{
fwcfg_present = false;fwcfg_dma_present = false;if (qemu_fwcfg_present()) {fwcfg_present = true;if (qemu_fwcfg_dma_present())fwcfg_dma_present = true;}This function can be simplified to:
fwcfg_present = qemu_fwcfg_present(); if (fwcfg_present) fwcfg_dma_present = qemu_fwcfg_dma_present();+}
+int do_qemu_fw(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{
void *load_addr;if (!fwcfg_present) {printf("no qemu fw_cfg found\n");return 0;}if (!strncmp(argv[1], "list", 4)) {qemu_fwcfg_list_firmware();return 0;} else if (!strncmp(argv[1], "cpus", 4)) {printf("%u\n", qemu_fwcfg_online_cpus());Can we put more words here instead of just printing the cpu numbers?
What words do you have in mind ? "cpus number" ?
Maybe: %u cpus online?
return 0;} else if (!strncmp(argv[1], "load", 4)) {if (argc == 3) {load_addr = (void *)simple_strtoul(argv[2], NULL, 16);} else {load_addr = getenv("loadaddr");if (!load_addr)load_addr = (void *)CONFIG_SYS_LOAD_ADDR;elseload_addr = (void *)simple_strtoul(load_addr,NULL, 16);}return qemu_fwcfg_setup_kernel(load_addr);} else {return -1;}Can you rewrite the command logic utilizing find_cmd_tbl() and cmd_process_error()? See arch/x86/lib/fsp/cmd_fsp.c for example.
return 0;+}
+U_BOOT_CMD(
fw, 3, 1, do_qemu_fw,"qemu firmware interface",nits: qemu -> QEMU
"<command>\n"" - list : print firmware(s) currently loaded\n"" - cpus : print online cpu number\n"" - load <addr> : load kernel (if any) to address <addr>, and setup for zboot\n"+) diff --git a/arch/x86/cpu/qemu/fw_cfg.h b/arch/x86/cpu/qemu/fw_cfg.h new file mode 100644 index 0000000..66e0c8a --- /dev/null +++ b/arch/x86/cpu/qemu/fw_cfg.h @@ -0,0 +1,84 @@ +/*
- (C) Copyright 2015 Miao Yan yanmiaobest@gmail.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#ifndef __FW_CFG__ +#define __FW_CFG__
+#include <common.h> +#include <asm/io.h>
Not needed. Please remove these two includes.
+#define FW_CONTROL_PORT 0x510 +#define FW_DATA_PORT 0x511 +#define FW_DMA_PORT 0x514
+#define FW_CFG_SIGNATURE 0x00 +#define FW_CFG_ID 0x01 +#define FW_CFG_UUID 0x02 +#define FW_CFG_RAM_SIZE 0x03 +#define FW_CFG_NOGRAPHIC 0x04 +#define FW_CFG_NB_CPUS 0x05 +#define FW_CFG_MACHINE_ID 0x06 +#define FW_CFG_KERNEL_ADDR 0x07 +#define FW_CFG_KERNEL_SIZE 0x08 +#define FW_CFG_KERNEL_CMDLINE 0x09 +#define FW_CFG_INITRD_ADDR 0x0a +#define FW_CFG_INITRD_SIZE 0x0b +#define FW_CFG_BOOT_DEVICE 0x0c +#define FW_CFG_NUMA 0x0d +#define FW_CFG_BOOT_MENU 0x0e +#define FW_CFG_MAX_CPUS 0x0f +#define FW_CFG_KERNEL_ENTRY 0x10 +#define FW_CFG_KERNEL_DATA 0x11 +#define FW_CFG_INITRD_DATA 0x12 +#define FW_CFG_CMDLINE_ADDR 0x13 +#define FW_CFG_CMDLINE_SIZE 0x14 +#define FW_CFG_CMDLINE_DATA 0x15 +#define FW_CFG_SETUP_ADDR 0x16 +#define FW_CFG_SETUP_SIZE 0x17 +#define FW_CFG_SETUP_DATA 0x18 +#define FW_CFG_FILE_DIR 0x19
Can we use enum for the above entries?
+#define FW_CFG_FILE_FIRST 0x20 +#define FW_CFG_FILE_SLOTS 0x10 +#define FW_CFG_MAX_ENTRY (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
+#define FW_CFG_WRITE_CHANNEL 0x4000 +#define FW_CFG_ARCH_LOCAL 0x8000 +#define FW_CFG_ENTRY_MASK ~(FW_CFG_WRITE_CHANNEL | FW_CFG_ARCH_LOCAL)
+#define FW_CFG_INVALID 0xffff
+#define FW_CFG_MAX_FILE_PATH 56
+#define QEMU_FW_CFG_SIGNATURE (('Q' << 24) | ('E' << 16) | ('M' << 8) | 'U')
+#define FW_CFG_DMA_ERROR (0x1) +#define FW_CFG_DMA_READ (0x2) +#define FW_CFG_DMA_SKIP (0x4) +#define FW_CFG_DMA_SELECT (0x8)
For above 4, we can use (1 << 0) / (1 << 1) / (1 << 2) / (1 << 3) to indicate they are bit definitions.
+struct fw_cfg_file {
uint32_t size;uint16_t select;uint16_t reserved;char name[FW_CFG_MAX_FILE_PATH];+};
+struct fw_cfg_files {
__be32 count;struct fw_cfg_file files[];Ah, this fw_cfg interface is weird. count is big endian, but fw_cfg_files is not.
fw_cfg_file is big endian. I forgot to change them. Will fix.
+};
+struct fw_cfg_dma_access {
__be32 control;__be32 length;__be64 address;These are big endians again. Weird.
+};
+void qemu_fwcfg_init(void); +uint16_t qemu_fwcfg_online_cpus(void);
Can you add the comment header block for these two functions?
+#endif diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c index 84fb082..c0a79d2 100644 --- a/arch/x86/cpu/qemu/qemu.c +++ b/arch/x86/cpu/qemu/qemu.c @@ -11,6 +11,7 @@ #include <asm/processor.h> #include <asm/arch/device.h> #include <asm/arch/qemu.h> +#include "fw_cfg.h"
static bool i440fx;
@@ -57,6 +58,8 @@ static void qemu_chipset_init(void) x86_pci_write_config32(PCI_BDF(0, 0, 0), PCIEX_BAR, CONFIG_PCIE_ECAM_BASE | BAR_EN); }
qemu_fwcfg_init();}
int arch_cpu_init(void)
Regards, Bin
I'll fix the rest of your comments. Thanks for the review.
Regards, Bin
Add a cpu uclass driver for qemu. Previously, the qemu target gets cpu number from board dts files, which are manually created at compile time. This does not scale when more cpus are assigned to guest as the dts files must be modified as well.
This patch adds a cpu uclass driver for qemu targets to directly read online cpu number from firmware.
Signed-off-by: yanmiaobest@gmail.com --- arch/x86/cpu/qemu/Makefile | 2 +- arch/x86/cpu/qemu/cpu.c | 58 ++++++++++++++++++++++++++++++++++++++++ arch/x86/dts/qemu-x86_i440fx.dts | 4 +-- arch/x86/dts/qemu-x86_q35.dts | 4 +-- 4 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 arch/x86/cpu/qemu/cpu.c
diff --git a/arch/x86/cpu/qemu/Makefile b/arch/x86/cpu/qemu/Makefile index ad424ec..027a12f 100644 --- a/arch/x86/cpu/qemu/Makefile +++ b/arch/x86/cpu/qemu/Makefile @@ -7,5 +7,5 @@ ifndef CONFIG_EFI_STUB obj-y += car.o dram.o endif -obj-y += qemu.o fw_cfg.o +obj-y += qemu.o fw_cfg.o cpu.o obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi.o dsdt.o diff --git a/arch/x86/cpu/qemu/cpu.c b/arch/x86/cpu/qemu/cpu.c new file mode 100644 index 0000000..9d95518 --- /dev/null +++ b/arch/x86/cpu/qemu/cpu.c @@ -0,0 +1,58 @@ +/* + * Copyright (C) 2015, Miao Yan yanmiaobest@gmail.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <cpu.h> +#include <dm.h> +#include <errno.h> +#include <asm/cpu.h> +#include "fw_cfg.h" + +DECLARE_GLOBAL_DATA_PTR; + +int cpu_qemu_bind(struct udevice *dev) +{ + struct cpu_platdata *plat = dev_get_parent_platdata(dev); + + plat->cpu_id = fdtdec_get_int(gd->fdt_blob, dev->of_offset, + "intel,apic-id", -1); + + return 0; +} + +int cpu_qemu_get_desc(struct udevice *dev, char *buf, int size) +{ + if (size < CPU_MAX_NAME_LEN) + return -ENOSPC; + + cpu_get_name(buf); + + return 0; +} + +static int cpu_qemu_get_count(struct udevice *dev) +{ + int num = (int)qemu_fwcfg_online_cpus(); + return num; +} + +static const struct cpu_ops cpu_qemu_ops = { + .get_desc = cpu_qemu_get_desc, + .get_count = cpu_qemu_get_count, +}; + +static const struct udevice_id cpu_qemu_ids[] = { + { .compatible = "cpu-qemu" }, + { } +}; + +U_BOOT_DRIVER(cpu_qemu_drv) = { + .name = "cpu_qemu", + .id = UCLASS_CPU, + .of_match = cpu_qemu_ids, + .bind = cpu_qemu_bind, + .ops = &cpu_qemu_ops, +}; diff --git a/arch/x86/dts/qemu-x86_i440fx.dts b/arch/x86/dts/qemu-x86_i440fx.dts index 8da7e52..8c9d35a 100644 --- a/arch/x86/dts/qemu-x86_i440fx.dts +++ b/arch/x86/dts/qemu-x86_i440fx.dts @@ -31,14 +31,14 @@
cpu@0 { device_type = "cpu"; - compatible = "cpu-x86"; + compatible = "cpu-qemu"; reg = <0>; intel,apic-id = <0>; };
cpu@1 { device_type = "cpu"; - compatible = "cpu-x86"; + compatible = "cpu-qemu"; reg = <1>; intel,apic-id = <1>; }; diff --git a/arch/x86/dts/qemu-x86_q35.dts b/arch/x86/dts/qemu-x86_q35.dts index df30c89..c980f45 100644 --- a/arch/x86/dts/qemu-x86_q35.dts +++ b/arch/x86/dts/qemu-x86_q35.dts @@ -42,14 +42,14 @@
cpu@0 { device_type = "cpu"; - compatible = "cpu-x86"; + compatible = "cpu-qemu"; reg = <0>; intel,apic-id = <0>; };
cpu@1 { device_type = "cpu"; - compatible = "cpu-x86"; + compatible = "cpu-qemu"; reg = <1>; intel,apic-id = <1>; };
Hi Miao,
On Mon, Dec 28, 2015 at 5:18 PM, Miao Yan yanmiaobest@gmail.com wrote:
Add a cpu uclass driver for qemu. Previously, the qemu target gets cpu number from board dts files, which are manually created at compile time. This does not scale when more cpus are assigned to guest as the dts files must be modified as well.
This patch adds a cpu uclass driver for qemu targets to directly read online cpu number from firmware.
Signed-off-by: yanmiaobest@gmail.com
arch/x86/cpu/qemu/Makefile | 2 +- arch/x86/cpu/qemu/cpu.c | 58 ++++++++++++++++++++++++++++++++++++++++ arch/x86/dts/qemu-x86_i440fx.dts | 4 +-- arch/x86/dts/qemu-x86_q35.dts | 4 +-- 4 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 arch/x86/cpu/qemu/cpu.c
diff --git a/arch/x86/cpu/qemu/Makefile b/arch/x86/cpu/qemu/Makefile index ad424ec..027a12f 100644 --- a/arch/x86/cpu/qemu/Makefile +++ b/arch/x86/cpu/qemu/Makefile @@ -7,5 +7,5 @@ ifndef CONFIG_EFI_STUB obj-y += car.o dram.o endif -obj-y += qemu.o fw_cfg.o +obj-y += qemu.o fw_cfg.o cpu.o obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi.o dsdt.o diff --git a/arch/x86/cpu/qemu/cpu.c b/arch/x86/cpu/qemu/cpu.c new file mode 100644 index 0000000..9d95518 --- /dev/null +++ b/arch/x86/cpu/qemu/cpu.c @@ -0,0 +1,58 @@ +/*
- Copyright (C) 2015, Miao Yan yanmiaobest@gmail.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <cpu.h> +#include <dm.h> +#include <errno.h> +#include <asm/cpu.h> +#include "fw_cfg.h"
+DECLARE_GLOBAL_DATA_PTR;
+int cpu_qemu_bind(struct udevice *dev) +{
struct cpu_platdata *plat = dev_get_parent_platdata(dev);plat->cpu_id = fdtdec_get_int(gd->fdt_blob, dev->of_offset,"intel,apic-id", -1);return 0;+}
+int cpu_qemu_get_desc(struct udevice *dev, char *buf, int size) +{
if (size < CPU_MAX_NAME_LEN)return -ENOSPC;cpu_get_name(buf);return 0;+}
+static int cpu_qemu_get_count(struct udevice *dev) +{
int num = (int)qemu_fwcfg_online_cpus();return num;
Can we just 'return qemu_fwcfg_online_cpus()' here?
+}
+static const struct cpu_ops cpu_qemu_ops = {
.get_desc = cpu_qemu_get_desc,.get_count = cpu_qemu_get_count,+};
+static const struct udevice_id cpu_qemu_ids[] = {
{ .compatible = "cpu-qemu" },{ }+};
+U_BOOT_DRIVER(cpu_qemu_drv) = {
.name = "cpu_qemu",.id = UCLASS_CPU,.of_match = cpu_qemu_ids,.bind = cpu_qemu_bind,.ops = &cpu_qemu_ops,+}; diff --git a/arch/x86/dts/qemu-x86_i440fx.dts b/arch/x86/dts/qemu-x86_i440fx.dts index 8da7e52..8c9d35a 100644 --- a/arch/x86/dts/qemu-x86_i440fx.dts +++ b/arch/x86/dts/qemu-x86_i440fx.dts @@ -31,14 +31,14 @@
cpu@0 { device_type = "cpu";
compatible = "cpu-x86";
compatible = "cpu-qemu"; reg = <0>; intel,apic-id = <0>; }; cpu@1 { device_type = "cpu";
compatible = "cpu-x86";
compatible = "cpu-qemu"; reg = <1>; intel,apic-id = <1>; };diff --git a/arch/x86/dts/qemu-x86_q35.dts b/arch/x86/dts/qemu-x86_q35.dts index df30c89..c980f45 100644 --- a/arch/x86/dts/qemu-x86_q35.dts +++ b/arch/x86/dts/qemu-x86_q35.dts @@ -42,14 +42,14 @@
cpu@0 { device_type = "cpu";
compatible = "cpu-x86";
compatible = "cpu-qemu"; reg = <0>; intel,apic-id = <0>; }; cpu@1 { device_type = "cpu";
compatible = "cpu-x86";
compatible = "cpu-qemu"; reg = <1>; intel,apic-id = <1>; };--
Other than that: Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin
Rename 'find_cpu_by_apid_id' to 'find_cpu_by_apic_id'. This should be a typo.
Signed-off-by: Miao Yan yanmiaobest@gmail.com --- arch/x86/cpu/mp_init.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c index 4334f5b..2f34317 100644 --- a/arch/x86/cpu/mp_init.c +++ b/arch/x86/cpu/mp_init.c @@ -104,7 +104,7 @@ static void ap_do_flight_plan(struct udevice *cpu) } }
-static int find_cpu_by_apid_id(int apic_id, struct udevice **devp) +static int find_cpu_by_apic_id(int apic_id, struct udevice **devp) { struct udevice *dev;
@@ -137,7 +137,7 @@ static void ap_init(unsigned int cpu_index) enable_lapic();
apic_id = lapicid(); - ret = find_cpu_by_apid_id(apic_id, &dev); + ret = find_cpu_by_apic_id(apic_id, &dev); if (ret) { debug("Unknown CPU apic_id %x\n", apic_id); goto done; @@ -432,7 +432,7 @@ static int init_bsp(struct udevice **devp) lapic_setup();
apic_id = lapicid(); - ret = find_cpu_by_apid_id(apic_id, devp); + ret = find_cpu_by_apic_id(apic_id, devp); if (ret) { printf("Cannot find boot CPU, APIC ID %d\n", apic_id); return ret;
On Mon, Dec 28, 2015 at 5:18 PM, Miao Yan yanmiaobest@gmail.com wrote:
Rename 'find_cpu_by_apid_id' to 'find_cpu_by_apic_id'. This should be a typo.
Signed-off-by: Miao Yan yanmiaobest@gmail.com
arch/x86/cpu/mp_init.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
Use actual CPU number , instead of maximum cpu configured, to allocate stack memory in 'load_sipi_vector'
Signed-off-by: Miao Yan yanmiaobest@gmail.com --- arch/x86/cpu/mp_init.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c index 2f34317..2a3ce48 100644 --- a/arch/x86/cpu/mp_init.c +++ b/arch/x86/cpu/mp_init.c @@ -210,7 +210,7 @@ static int save_bsp_msrs(char *start, int size) return msr_count; }
-static int load_sipi_vector(atomic_t **ap_countp) +static int load_sipi_vector(atomic_t **ap_countp, int num_cpus) { struct sipi_params_16bit *params16; struct sipi_params *params; @@ -239,7 +239,7 @@ static int load_sipi_vector(atomic_t **ap_countp) params->idt_ptr = (uint32_t)x86_get_idt();
params->stack_size = CONFIG_AP_STACK_SIZE; - size = params->stack_size * CONFIG_MAX_CPUS; + size = params->stack_size * num_cpus; stack = memalign(size, 4096); if (!stack) return -ENOMEM; @@ -483,7 +483,7 @@ int mp_init(struct mp_params *p) mp_info.records = p->flight_plan;
/* Load the SIPI vector */ - ret = load_sipi_vector(&ap_count); + ret = load_sipi_vector(&ap_count, num_cpus); if (ap_count == NULL) return -1;
Hi Miao,
Please remove the qemu tag from the commit title, as this is a change to common x86 codes.
On Mon, Dec 28, 2015 at 5:18 PM, Miao Yan yanmiaobest@gmail.com wrote:
Use actual CPU number , instead of maximum cpu configured,
nits: remove the space after 'number'
to allocate stack memory in 'load_sipi_vector'
Signed-off-by: Miao Yan yanmiaobest@gmail.com
arch/x86/cpu/mp_init.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c index 2f34317..2a3ce48 100644 --- a/arch/x86/cpu/mp_init.c +++ b/arch/x86/cpu/mp_init.c @@ -210,7 +210,7 @@ static int save_bsp_msrs(char *start, int size) return msr_count; }
-static int load_sipi_vector(atomic_t **ap_countp) +static int load_sipi_vector(atomic_t **ap_countp, int num_cpus) { struct sipi_params_16bit *params16; struct sipi_params *params; @@ -239,7 +239,7 @@ static int load_sipi_vector(atomic_t **ap_countp) params->idt_ptr = (uint32_t)x86_get_idt();
params->stack_size = CONFIG_AP_STACK_SIZE;
size = params->stack_size * CONFIG_MAX_CPUS;
size = params->stack_size * num_cpus; stack = memalign(size, 4096); if (!stack) return -ENOMEM;@@ -483,7 +483,7 @@ int mp_init(struct mp_params *p) mp_info.records = p->flight_plan;
/* Load the SIPI vector */
ret = load_sipi_vector(&ap_count);
ret = load_sipi_vector(&ap_count, num_cpus); if (ap_count == NULL) return -1;--
Other than that, Reviewed-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin
Add a function to fixup 'cpus' node in dts files for qemu target.
Signed-off-by: Miao Yan yanmiaobest@gmail.com --- arch/x86/cpu/qemu/fw_cfg.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++ arch/x86/cpu/qemu/fw_cfg.h | 1 + 2 files changed, 67 insertions(+)
diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c index e7615d1..d6b0276 100644 --- a/arch/x86/cpu/qemu/fw_cfg.c +++ b/arch/x86/cpu/qemu/fw_cfg.c @@ -12,6 +12,8 @@ #include <errno.h> #include <fs.h> #include <asm/io.h> +#include <libfdt.h> +#include <libfdt_env.h> #include "fw_cfg.h"
static bool fwcfg_present; @@ -92,6 +94,70 @@ uint16_t qemu_fwcfg_online_cpus(void) return nb_cpus; }
+void qemu_fwcfg_fdt_fixup(void *fdt_addr, uint16_t 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: %s (%d)\n", + 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: %s (%d)\n", + fdt_strerror(err), err); + return; + } + + err = fdt_setprop(fdt_addr, sub_off, "reg", + (void *)&id, sizeof(id)); + if (err < 0) { + printf("error adding reg: %s (%d)\n", + 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: %s (%d)\n", + 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: %s (%d)\n", + fdt_strerror(err), err); + return; + } + } +} + static int qemu_fwcfg_setup_kernel(void *load_addr) { char *cmd_addr; diff --git a/arch/x86/cpu/qemu/fw_cfg.h b/arch/x86/cpu/qemu/fw_cfg.h index 66e0c8a..d4bd46f 100644 --- a/arch/x86/cpu/qemu/fw_cfg.h +++ b/arch/x86/cpu/qemu/fw_cfg.h @@ -80,5 +80,6 @@ struct fw_cfg_dma_access {
void qemu_fwcfg_init(void); uint16_t qemu_fwcfg_online_cpus(void); +void qemu_fwcfg_fdt_fixup(void *fdt_addr, uint16_t cpu_num);
#endif
Hi Miao,
On Mon, Dec 28, 2015 at 5:18 PM, Miao Yan yanmiaobest@gmail.com wrote:
Add a function to fixup 'cpus' node in dts files for qemu target.
nits: fixup -> fix up
Signed-off-by: Miao Yan yanmiaobest@gmail.com
arch/x86/cpu/qemu/fw_cfg.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++ arch/x86/cpu/qemu/fw_cfg.h | 1 + 2 files changed, 67 insertions(+)
diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c index e7615d1..d6b0276 100644 --- a/arch/x86/cpu/qemu/fw_cfg.c +++ b/arch/x86/cpu/qemu/fw_cfg.c @@ -12,6 +12,8 @@ #include <errno.h> #include <fs.h> #include <asm/io.h> +#include <libfdt.h> +#include <libfdt_env.h>
libfdt_env.h is not needed.
#include "fw_cfg.h"
static bool fwcfg_present; @@ -92,6 +94,70 @@ uint16_t qemu_fwcfg_online_cpus(void) return nb_cpus; }
+void qemu_fwcfg_fdt_fixup(void *fdt_addr, uint16_t cpu_num)
Can we use 'int' for '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: %s (%d)\n",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: %s (%d)\n",fdt_strerror(err), err);return;}err = fdt_setprop(fdt_addr, sub_off, "reg",(void *)&id, sizeof(id));if (err < 0) {printf("error adding reg: %s (%d)\n",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: %s (%d)\n",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: %s (%d)\n",fdt_strerror(err), err);return;}}+}
static int qemu_fwcfg_setup_kernel(void *load_addr) { char *cmd_addr; diff --git a/arch/x86/cpu/qemu/fw_cfg.h b/arch/x86/cpu/qemu/fw_cfg.h index 66e0c8a..d4bd46f 100644 --- a/arch/x86/cpu/qemu/fw_cfg.h +++ b/arch/x86/cpu/qemu/fw_cfg.h @@ -80,5 +80,6 @@ struct fw_cfg_dma_access {
void qemu_fwcfg_init(void); uint16_t qemu_fwcfg_online_cpus(void); +void qemu_fwcfg_fdt_fixup(void *fdt_addr, uint16_t cpu_num);
Please add a comment header block for this function.
#endif
Regards, Bin
Remove 'cpus' node in dts files for QEMU targets, retrieve cpu number through 'fw_cfg' interface and fixup device tree blob at runtime.
Signed-off-by: Miao Yan yanmiaobest@gmail.com --- arch/x86/cpu/qemu/qemu.c | 4 ++++ arch/x86/dts/qemu-x86_i440fx.dts | 18 +----------------- arch/x86/dts/qemu-x86_q35.dts | 19 +------------------ 3 files changed, 6 insertions(+), 35 deletions(-)
diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c index c0a79d2..e643e04 100644 --- a/arch/x86/cpu/qemu/qemu.c +++ b/arch/x86/cpu/qemu/qemu.c @@ -15,6 +15,8 @@
static bool i440fx;
+DECLARE_GLOBAL_DATA_PTR; + static void qemu_chipset_init(void) { u16 device, xbcs; @@ -96,6 +98,8 @@ int arch_early_init_r(void) { qemu_chipset_init();
+ qemu_fwcfg_fdt_fixup((void *)gd->fdt_blob, qemu_fwcfg_online_cpus()); + return 0; }
diff --git a/arch/x86/dts/qemu-x86_i440fx.dts b/arch/x86/dts/qemu-x86_i440fx.dts index 8c9d35a..3f32ec9 100644 --- a/arch/x86/dts/qemu-x86_i440fx.dts +++ b/arch/x86/dts/qemu-x86_i440fx.dts @@ -25,24 +25,8 @@ stdout-path = "/serial"; };
- cpus { - #address-cells = <1>; - #size-cells = <0>;
- cpu@0 { - device_type = "cpu"; - compatible = "cpu-qemu"; - reg = <0>; - intel,apic-id = <0>; - }; - - cpu@1 { - device_type = "cpu"; - compatible = "cpu-qemu"; - reg = <1>; - intel,apic-id = <1>; - }; - }; + /* cpu node will be dynamically filled by qemu */
pci { compatible = "pci-x86"; diff --git a/arch/x86/dts/qemu-x86_q35.dts b/arch/x86/dts/qemu-x86_q35.dts index c980f45..c1c6a9a 100644 --- a/arch/x86/dts/qemu-x86_q35.dts +++ b/arch/x86/dts/qemu-x86_q35.dts @@ -36,24 +36,7 @@ stdout-path = "/serial"; };
- cpus { - #address-cells = <1>; - #size-cells = <0>; - - cpu@0 { - device_type = "cpu"; - compatible = "cpu-qemu"; - reg = <0>; - intel,apic-id = <0>; - }; - - cpu@1 { - device_type = "cpu"; - compatible = "cpu-qemu"; - reg = <1>; - intel,apic-id = <1>; - }; - }; + /* cpu node will be dynamically filled by qemu */
pci { compatible = "pci-x86";
Hi Miao,
nits: fixup -> fix up in the commit title
On Mon, Dec 28, 2015 at 5:18 PM, Miao Yan yanmiaobest@gmail.com wrote:
Remove 'cpus' node in dts files for QEMU targets, retrieve cpu number through 'fw_cfg' interface and fixup device tree blob at runtime.
nits: fixup -> fix up
Signed-off-by: Miao Yan yanmiaobest@gmail.com
arch/x86/cpu/qemu/qemu.c | 4 ++++ arch/x86/dts/qemu-x86_i440fx.dts | 18 +----------------- arch/x86/dts/qemu-x86_q35.dts | 19 +------------------ 3 files changed, 6 insertions(+), 35 deletions(-)
diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c index c0a79d2..e643e04 100644 --- a/arch/x86/cpu/qemu/qemu.c +++ b/arch/x86/cpu/qemu/qemu.c @@ -15,6 +15,8 @@
static bool i440fx;
+DECLARE_GLOBAL_DATA_PTR;
static void qemu_chipset_init(void) { u16 device, xbcs; @@ -96,6 +98,8 @@ int arch_early_init_r(void) { qemu_chipset_init();
qemu_fwcfg_fdt_fixup((void *)gd->fdt_blob, qemu_fwcfg_online_cpus());return 0;}
diff --git a/arch/x86/dts/qemu-x86_i440fx.dts b/arch/x86/dts/qemu-x86_i440fx.dts index 8c9d35a..3f32ec9 100644 --- a/arch/x86/dts/qemu-x86_i440fx.dts +++ b/arch/x86/dts/qemu-x86_i440fx.dts @@ -25,24 +25,8 @@ stdout-path = "/serial"; };
cpus {#address-cells = <1>;#size-cells = <0>;cpu@0 {device_type = "cpu";compatible = "cpu-qemu";reg = <0>;intel,apic-id = <0>;};cpu@1 {device_type = "cpu";compatible = "cpu-qemu";reg = <1>;intel,apic-id = <1>;};};
/* cpu node will be dynamically filled by qemu */
by U-Boot
pci { compatible = "pci-x86";diff --git a/arch/x86/dts/qemu-x86_q35.dts b/arch/x86/dts/qemu-x86_q35.dts index c980f45..c1c6a9a 100644 --- a/arch/x86/dts/qemu-x86_q35.dts +++ b/arch/x86/dts/qemu-x86_q35.dts @@ -36,24 +36,7 @@ stdout-path = "/serial"; };
cpus {#address-cells = <1>;#size-cells = <0>;cpu@0 {device_type = "cpu";compatible = "cpu-qemu";reg = <0>;intel,apic-id = <0>;};cpu@1 {device_type = "cpu";compatible = "cpu-qemu";reg = <1>;intel,apic-id = <1>;};};
/* cpu node will be dynamically filled by qemu */
by U-Boot
pci { compatible = "pci-x86";--
Also, this patch does not apply on top of u-boot-x86/next. Please rebase your series on top of u-boot-x86/next.
Regards, Bin
Document the usage of 'fw' command
Signed-off-by: Miao Yan yanmiaobest@gmail.com --- doc/README.x86 | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-)
diff --git a/doc/README.x86 b/doc/README.x86 index 1271e5e..f39c157 100644 --- a/doc/README.x86 +++ b/doc/README.x86 @@ -295,9 +295,39 @@ show QEMU's VGA console window. Note this will disable QEMU's serial output. If you want to check both consoles, use '-serial stdio'.
Multicore is also supported by QEMU via '-smp n' where n is the number of cores -to instantiate. Currently the default U-Boot built for QEMU supports 2 cores. -In order to support more cores, you need add additional cpu nodes in the device -tree and change CONFIG_MAX_CPUS accordingly. +to instantiate. U-Boot uses fw_cfg interface provided by QEMU to detect certain +system information, such as cpu number, so 'n' can be any number allowed by +QEMU. + +The fw_cfg interface in QEMU also provides information about kernel data, initrd +,command-line arguments and more. U-Boot supports directly accessing these informtion +from fw_cfg interface, this saves the time of loading them from hard disk or +network again, through emulated devices. To use it , simply providing them in +QEMU command line: + +$ qemu-system-x86_64 -nographic -bios path/to/u-boot.rom -m 1024 -kernel /path/to/bzImage + -append 'root=/dev/sda1 console=ttyS0' -initrd /path/to/initrd -smp 8 + +Note: -initrd and -smp are both optional + +Then start QEMU, in U-Boot command line use the following U-Boot command to setup kernel: + + => fw +fw - qemu firmware interface + +Usage: +fw <command> + - list : print firmware(s) currently loaded + - cpus : print online cpu number + - load <addr> : load kernel (if any) to address <addr> + +=> fw load +loading kernel to address 01000000, initrd 015dd010 + +Here the kernel (bzImage) is loaded to 01000000 and initrd is to 0x15dd010. Then, 'zboot' +can be used to boot the kernel: + +=> zboot 01000000 - 015dd010
CPU Microcode -------------
Hi Miao,
nits: please use "x86: qemu" as the tag in the commit title.
On Mon, Dec 28, 2015 at 5:18 PM, Miao Yan yanmiaobest@gmail.com wrote:
Document the usage of 'fw' command
Signed-off-by: Miao Yan yanmiaobest@gmail.com
doc/README.x86 | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-)
diff --git a/doc/README.x86 b/doc/README.x86 index 1271e5e..f39c157 100644 --- a/doc/README.x86 +++ b/doc/README.x86 @@ -295,9 +295,39 @@ show QEMU's VGA console window. Note this will disable QEMU's serial output. If you want to check both consoles, use '-serial stdio'.
Multicore is also supported by QEMU via '-smp n' where n is the number of cores -to instantiate. Currently the default U-Boot built for QEMU supports 2 cores. -In order to support more cores, you need add additional cpu nodes in the device -tree and change CONFIG_MAX_CPUS accordingly. +to instantiate. U-Boot uses fw_cfg interface provided by QEMU to detect certain +system information, such as cpu number, so 'n' can be any number allowed by +QEMU.
I've tested this. Looks when n > 47, U-Boot mp_init() will fail. I guess we may need limit U-Boot maximum supported cpu number to some reasonable number, say 32?
+The fw_cfg interface in QEMU also provides information about kernel data, initrd +,command-line arguments and more. U-Boot supports directly accessing these informtion +from fw_cfg interface, this saves the time of loading them from hard disk or +network again, through emulated devices. To use it , simply providing them in +QEMU command line:
+$ qemu-system-x86_64 -nographic -bios path/to/u-boot.rom -m 1024 -kernel /path/to/bzImage
Please use qemu-system-i386 for consistency with other instances mentioned in the doc.
- -append 'root=/dev/sda1 console=ttyS0' -initrd /path/to/initrd -smp 8
+Note: -initrd and -smp are both optional
+Then start QEMU, in U-Boot command line use the following U-Boot command to setup kernel:
- => fw
+fw - qemu firmware interface
+Usage: +fw <command>
- list : print firmware(s) currently loaded
- cpus : print online cpu number
- load <addr> : load kernel (if any) to address <addr>
+=> fw load +loading kernel to address 01000000, initrd 015dd010
+Here the kernel (bzImage) is loaded to 01000000 and initrd is to 0x15dd010. Then, 'zboot'
Looks 01000000 is not kernel (bzImage) address. It is setup data address. Can we enhance the 'fw load' command to output all available loaded components address and size?
+can be used to boot the kernel:
+=> zboot 01000000 - 015dd010
zboot has 4 parameters, the second parameter is kernel image size, which can be zero. The last one is initrd image size, which should not omitted.
My test shows the kernel does not recognize my initramfs.gz, kernel log below:
[ 4.328688] Unpacking initramfs... [ 4.331688] Initramfs unpacking failed: junk in compressed archive
CPU Microcode
--
Regards, Bin
Hi Bin,
2015-12-29 14:19 GMT+08:00 Bin Meng bmeng.cn@gmail.com:
Hi Miao,
nits: please use "x86: qemu" as the tag in the commit title.
On Mon, Dec 28, 2015 at 5:18 PM, Miao Yan yanmiaobest@gmail.com wrote:
Document the usage of 'fw' command
Signed-off-by: Miao Yan yanmiaobest@gmail.com
doc/README.x86 | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-)
diff --git a/doc/README.x86 b/doc/README.x86 index 1271e5e..f39c157 100644 --- a/doc/README.x86 +++ b/doc/README.x86 @@ -295,9 +295,39 @@ show QEMU's VGA console window. Note this will disable QEMU's serial output. If you want to check both consoles, use '-serial stdio'.
Multicore is also supported by QEMU via '-smp n' where n is the number of cores -to instantiate. Currently the default U-Boot built for QEMU supports 2 cores. -In order to support more cores, you need add additional cpu nodes in the device -tree and change CONFIG_MAX_CPUS accordingly. +to instantiate. U-Boot uses fw_cfg interface provided by QEMU to detect certain +system information, such as cpu number, so 'n' can be any number allowed by +QEMU.
I've tested this. Looks when n > 47, U-Boot mp_init() will fail. I guess we may need limit U-Boot maximum supported cpu number to some reasonable number, say 32?
Did you see this error:
error adding apic-id: FDT_ERR_NOSPACE (-3)
No spaces left in dtb when there are too many cpus. I guess reserve more spaces for dtb should do. U-Boot itself should not limit the cpu number.
+The fw_cfg interface in QEMU also provides information about kernel data, initrd +,command-line arguments and more. U-Boot supports directly accessing these informtion +from fw_cfg interface, this saves the time of loading them from hard disk or +network again, through emulated devices. To use it , simply providing them in +QEMU command line:
+$ qemu-system-x86_64 -nographic -bios path/to/u-boot.rom -m 1024 -kernel /path/to/bzImage
Please use qemu-system-i386 for consistency with other instances mentioned in the doc.
Maybe changing them to qemu-system-x86_64 is better ?
- -append 'root=/dev/sda1 console=ttyS0' -initrd /path/to/initrd -smp 8
+Note: -initrd and -smp are both optional
+Then start QEMU, in U-Boot command line use the following U-Boot command to setup kernel:
- => fw
+fw - qemu firmware interface
+Usage: +fw <command>
- list : print firmware(s) currently loaded
- cpus : print online cpu number
- load <addr> : load kernel (if any) to address <addr>
+=> fw load +loading kernel to address 01000000, initrd 015dd010
+Here the kernel (bzImage) is loaded to 01000000 and initrd is to 0x15dd010. Then, 'zboot'
Looks 01000000 is not kernel (bzImage) address. It is setup data address. Can we enhance the 'fw load' command to output all available loaded components address and size?
+can be used to boot the kernel:
+=> zboot 01000000 - 015dd010
zboot has 4 parameters, the second parameter is kernel image size, which can be zero. The last one is initrd image size, which should not omitted.
I'll have a try.
Miao
My test shows the kernel does not recognize my initramfs.gz, kernel log below:
[ 4.328688] Unpacking initramfs... [ 4.331688] Initramfs unpacking failed: junk in compressed archive
CPU Microcode
--
Regards, Bin
Hi Miao,
On Tue, Dec 29, 2015 at 2:47 PM, Miao Yan yanmiaobest@gmail.com wrote:
Hi Bin,
2015-12-29 14:19 GMT+08:00 Bin Meng bmeng.cn@gmail.com:
Hi Miao,
nits: please use "x86: qemu" as the tag in the commit title.
On Mon, Dec 28, 2015 at 5:18 PM, Miao Yan yanmiaobest@gmail.com wrote:
Document the usage of 'fw' command
Signed-off-by: Miao Yan yanmiaobest@gmail.com
doc/README.x86 | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-)
diff --git a/doc/README.x86 b/doc/README.x86 index 1271e5e..f39c157 100644 --- a/doc/README.x86 +++ b/doc/README.x86 @@ -295,9 +295,39 @@ show QEMU's VGA console window. Note this will disable QEMU's serial output. If you want to check both consoles, use '-serial stdio'.
Multicore is also supported by QEMU via '-smp n' where n is the number of cores -to instantiate. Currently the default U-Boot built for QEMU supports 2 cores. -In order to support more cores, you need add additional cpu nodes in the device -tree and change CONFIG_MAX_CPUS accordingly. +to instantiate. U-Boot uses fw_cfg interface provided by QEMU to detect certain +system information, such as cpu number, so 'n' can be any number allowed by +QEMU.
I've tested this. Looks when n > 47, U-Boot mp_init() will fail. I guess we may need limit U-Boot maximum supported cpu number to some reasonable number, say 32?
Did you see this error:
error adding apic-id: FDT_ERR_NOSPACE (-3)No spaces left in dtb when there are too many cpus. I guess reserve more spaces for dtb should do. U-Boot itself should not limit the cpu number.
Yes, I see. So we may need fix this fdt size problem if we don't limit the maximum supported cpu numbers in U-Boot.
+The fw_cfg interface in QEMU also provides information about kernel data, initrd +,command-line arguments and more. U-Boot supports directly accessing these informtion +from fw_cfg interface, this saves the time of loading them from hard disk or +network again, through emulated devices. To use it , simply providing them in +QEMU command line:
+$ qemu-system-x86_64 -nographic -bios path/to/u-boot.rom -m 1024 -kernel /path/to/bzImage
Please use qemu-system-i386 for consistency with other instances mentioned in the doc.
Maybe changing them to qemu-system-x86_64 is better ?
I would like to still use -i386 version because U-Boot is compiled and run in 32-bit mode only, even if -x86_64 version can run 32-bit U-Boot.
- -append 'root=/dev/sda1 console=ttyS0' -initrd /path/to/initrd -smp 8
+Note: -initrd and -smp are both optional
+Then start QEMU, in U-Boot command line use the following U-Boot command to setup kernel:
- => fw
+fw - qemu firmware interface
+Usage: +fw <command>
- list : print firmware(s) currently loaded
- cpus : print online cpu number
- load <addr> : load kernel (if any) to address <addr>
+=> fw load +loading kernel to address 01000000, initrd 015dd010
+Here the kernel (bzImage) is loaded to 01000000 and initrd is to 0x15dd010. Then, 'zboot'
Looks 01000000 is not kernel (bzImage) address. It is setup data address. Can we enhance the 'fw load' command to output all available loaded components address and size?
+can be used to boot the kernel:
+=> zboot 01000000 - 015dd010
zboot has 4 parameters, the second parameter is kernel image size, which can be zero. The last one is initrd image size, which should not omitted.
I'll have a try.
Miao
My test shows the kernel does not recognize my initramfs.gz, kernel log below:
[ 4.328688] Unpacking initramfs... [ 4.331688] Initramfs unpacking failed: junk in compressed archive
CPU Microcode
--
Regards, Bin
Hi Miao,
Thanks for the patches.
I made an initial attempt to support the fw_cfg in U-boot for QEMU to get acpi tables by fw_cfg for qemu-x86 targets.
The idea was if we find acpi tables in fw_cfg try loading them, otherwise fallback to the builtin acpi tables.
The patch was dropped mainly because ACPI tables loaded by fw_cfg had some issues which I could not figure out. The patches were discussed(me, Simon and Bin) offline(off the mailing list), so I am forwarding it to you and the community.
If we can collaborate the patches in the sense that we are able to load ACPI tables(and other tables) successfully, it would be great.
Regards, Saket Sinha
On Mon, Dec 28, 2015 at 2:48 PM, Miao Yan yanmiaobest@gmail.com wrote:
The fw_cfg interface provided by QEMU allow guests to retrieve various information about the system, e.g. cpu number, variaous firmware data, kernel setup, etc. The fw_cfg interface can be accessed through 3 IO ports (on x86), using x86 in/out instructions.
- 0x510: select configuration items to access
- 0x511: reading this port will return data selected in 0x510
- 0x514: this can be used to detect if DMA interface is available
If fw_cfg DMA interface is available, it can be used to accelerate accesses.
This patchset adds the following supports for qemu-x86 targets:
the fw_cfg driver itself
add a U-Boot command 'fw' to support direct accessing kernel informtion from fw_cfg interface, this saves the time of loading them from hard disk or network again, through emulated devices.
use fw_cfg to get cpu number at runtime, so smp boot no longer relies on the cpu node hard-coded in dts files.
Miao Yan (7): x86: qemu: add fw_cfg support x86: qemu: add a cpu uclass driver for qemu target x86: fix a typo in function name x86: qemu: use actual CPU number for allocating memory x86: qemu: add qemu_fwcfg_fdt_fixup() x86: qemu: fixup cpu node in device tree qemu-x86: add documentaion for the fw_cfg interface
arch/x86/cpu/mp_init.c | 12 +- arch/x86/cpu/qemu/Makefile | 2 +- arch/x86/cpu/qemu/cpu.c | 58 ++++++++ arch/x86/cpu/qemu/fw_cfg.c | 281 +++++++++++++++++++++++++++++++++++++++ arch/x86/cpu/qemu/fw_cfg.h | 85 ++++++++++++ arch/x86/cpu/qemu/qemu.c | 7 + arch/x86/dts/qemu-x86_i440fx.dts | 18 +-- arch/x86/dts/qemu-x86_q35.dts | 19 +-- doc/README.x86 | 36 ++++- 9 files changed, 473 insertions(+), 45 deletions(-) create mode 100644 arch/x86/cpu/qemu/cpu.c create mode 100644 arch/x86/cpu/qemu/fw_cfg.c create mode 100644 arch/x86/cpu/qemu/fw_cfg.h
-- 1.9.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
2015-12-28 18:05 GMT+08:00 Saket Sinha saket.sinha89@gmail.com:
Hi Miao,
Thanks for the patches.
I made an initial attempt to support the fw_cfg in U-boot for QEMU to get acpi tables by fw_cfg for qemu-x86 targets.
The idea was if we find acpi tables in fw_cfg try loading them, otherwise fallback to the builtin acpi tables.
The patch was dropped mainly because ACPI tables loaded by fw_cfg had some issues which I could not figure out. The patches were discussed(me, Simon and Bin) offline(off the mailing list), so I am forwarding it to you and the community.
If we can collaborate the patches in the sense that we are able to load ACPI tables(and other tables) successfully, it would be great.
The main purpose of my patch is: + directly loads kernel from qemu + eliminate the cpu number limits in smp boot
I am not familiar with the acpi part, but if you have a basic configuration to reproduce the error, maybe I can have a try.
Thanks, Miao
Regards, Saket Sinha
On Mon, Dec 28, 2015 at 2:48 PM, Miao Yan yanmiaobest@gmail.com wrote:
The fw_cfg interface provided by QEMU allow guests to retrieve various information about the system, e.g. cpu number, variaous firmware data, kernel setup, etc. The fw_cfg interface can be accessed through 3 IO ports (on x86), using x86 in/out instructions.
- 0x510: select configuration items to access
- 0x511: reading this port will return data selected in 0x510
- 0x514: this can be used to detect if DMA interface is available
If fw_cfg DMA interface is available, it can be used to accelerate accesses.
This patchset adds the following supports for qemu-x86 targets:
the fw_cfg driver itself
add a U-Boot command 'fw' to support direct accessing kernel informtion from fw_cfg interface, this saves the time of loading them from hard disk or network again, through emulated devices.
use fw_cfg to get cpu number at runtime, so smp boot no longer relies on the cpu node hard-coded in dts files.
Miao Yan (7): x86: qemu: add fw_cfg support x86: qemu: add a cpu uclass driver for qemu target x86: fix a typo in function name x86: qemu: use actual CPU number for allocating memory x86: qemu: add qemu_fwcfg_fdt_fixup() x86: qemu: fixup cpu node in device tree qemu-x86: add documentaion for the fw_cfg interface
arch/x86/cpu/mp_init.c | 12 +- arch/x86/cpu/qemu/Makefile | 2 +- arch/x86/cpu/qemu/cpu.c | 58 ++++++++ arch/x86/cpu/qemu/fw_cfg.c | 281 +++++++++++++++++++++++++++++++++++++++ arch/x86/cpu/qemu/fw_cfg.h | 85 ++++++++++++ arch/x86/cpu/qemu/qemu.c | 7 + arch/x86/dts/qemu-x86_i440fx.dts | 18 +-- arch/x86/dts/qemu-x86_q35.dts | 19 +-- doc/README.x86 | 36 ++++- 9 files changed, 473 insertions(+), 45 deletions(-) create mode 100644 arch/x86/cpu/qemu/cpu.c create mode 100644 arch/x86/cpu/qemu/fw_cfg.c create mode 100644 arch/x86/cpu/qemu/fw_cfg.h
-- 1.9.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Miao,
Find my response inline.
The main purpose of my patch is:
- directly loads kernel from qemu
- eliminate the cpu number limits in smp boot
Our patches are similar in case of fw_cfg apis support it brings to u-boot but our use case of using them are different. My patches target tables (ACPI tables to be specific.)
I am not familiar with the acpi part, but if you have a basic configuration to reproduce the error, maybe I can have a try.
I shared the patch with you on the separate thread. If you read through the thread, you can find the details how to reproduce it.
Anyways, I am sharing how you can test my patch to reproduce the error.
1. Apply the patch file(sahred seprately) to u-boot-x86 tree. patch -p1 *.patch
2. make qemu-x86_defconfig, make all
3. qemu-system-i386 -nographic -bios u-boot.rom -net nic -net user,tftp=/tftpboot
Regards, Saket Sinha
Hi Saket,
2015-12-28 18:43 GMT+08:00 Saket Sinha saket.sinha89@gmail.com:
Hi Miao,
Find my response inline.
The main purpose of my patch is:
- directly loads kernel from qemu
- eliminate the cpu number limits in smp boot
Our patches are similar in case of fw_cfg apis support it brings to u-boot but our use case of using them are different. My patches target tables (ACPI tables to be specific.)
I am not familiar with the acpi part, but if you have a basic configuration to reproduce the error, maybe I can have a try.
I shared the patch with you on the separate thread. If you read through the thread, you can find the details how to reproduce it.
Are you referring to "Initial fw_cfg support for qemu-x86 targets" ? I didn't find any patch in that email you sent to me.
But I did give Seabios a try and it works. So I don't think there's anything wrong with the acpi tables generated by qemu. It seems other than just loading files into memory, guests are supposed to perform some "fix-ups" to those table according to qemu's instructions.
But I can't say what's wrong without seeing your original patches.
Anyways, I am sharing how you can test my patch to reproduce the error.
- Apply the patch file(sahred seprately) to u-boot-x86 tree.
patch -p1 *.patch
make qemu-x86_defconfig, make all
qemu-system-i386 -nographic -bios u-boot.rom -net nic -net
user,tftp=/tftpboot
Regards, Saket Sinha
Hi Miao,
Are you referring to "Initial fw_cfg support for qemu-x86 targets" ? I didn't find any patch in that email you sent to me.
YES.
The first thread of the mail contains the patch. Anyways I am copy-pasting patch below.
But I did give Seabios a try and it works. So I don't think there's anything wrong with the acpi tables generated by qemu. It seems other than just loading files into memory, guests are supposed to perform some "fix-ups" to those table according to qemu's instructions.
Good. Then there may be some error in my patch.
But I can't say what's wrong without seeing your original patches.
Please find the patch below -
--- arch/x86/cpu/qemu/Makefile | 2 +- arch/x86/cpu/qemu/fw_cfg.c | 288 ++++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/fw_cfg.h | 81 ++++++++++++ arch/x86/lib/tables.c | 3 + 4 files changed, 373 insertions(+), 1 deletion(-) create mode 100644 arch/x86/cpu/qemu/fw_cfg.c create mode 100644 arch/x86/include/asm/fw_cfg.h
diff --git a/arch/x86/cpu/qemu/Makefile b/arch/x86/cpu/qemu/Makefile index be79723..52a82f0 100644 --- a/arch/x86/cpu/qemu/Makefile +++ b/arch/x86/cpu/qemu/Makefile @@ -4,5 +4,5 @@ # SPDX-License-Identifier: GPL-2.0+ #
-obj-y += car.o dram.o qemu.o +obj-y += car.o dram.o qemu.o fw_cfg.o obj-$(CONFIG_PCI) += pci.o diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c new file mode 100644 index 0000000..6e8c4d3 --- /dev/null +++ b/arch/x86/cpu/qemu/fw_cfg.c @@ -0,0 +1,288 @@ +#include <asm/fw_cfg.h> +#include <linux/string.h> +#include <malloc.h> +#include <common.h> +#include <asm/post.h> +#include <asm/arch/qemu.h> + + +#define FW_CFG_PORT_CTL 0x0510 +#define FW_CFG_PORT_DATA 0x0511 + +static unsigned char fw_cfg_detected = 0xff; +static FWCfgFiles *fw_files; + +static int fw_cfg_present(void) +{ + static const char qsig[] = "QEMU"; + unsigned char sig[4]; + + if (fw_cfg_detected == 0xff) { + fw_cfg_get(FW_CFG_SIGNATURE, sig, sizeof(sig)); + fw_cfg_detected = (memcmp(sig, qsig, 4) == 0) ? 1 : 0; + } + return fw_cfg_detected; +} + +void fw_cfg_get(int entry, void *dst, int dstlen) +{ + outw(entry, FW_CFG_PORT_CTL); + insb(FW_CFG_PORT_DATA, dst, dstlen); +} + +static void fw_cfg_init_file(void) +{ + u32 i, size, count = 0; + + if (fw_files != NULL) + return; + + fw_cfg_get(FW_CFG_FILE_DIR, &count, sizeof(count)); + count = swab32(count); + size = count * sizeof(FWCfgFile) + sizeof(count); + fw_files = malloc(size); + fw_cfg_get(FW_CFG_FILE_DIR, fw_files, size); + fw_files->count = swab32(fw_files->count); + for (i = 0; i < count; i++) { + fw_files->f[i].size = swab32(fw_files->f[i].size); + fw_files->f[i].select = swab16(fw_files->f[i].select); + } +} + +static FWCfgFile *fw_cfg_find_file(const char *name) +{ + int i; + + fw_cfg_init_file(); + for (i = 0; i < fw_files->count; i++) + if (strcmp(fw_files->f[i].name, name) == 0) + return fw_files->f + i; + return NULL; +} + +int fw_cfg_check_file(const char *name) +{ + FWCfgFile *file; + + if (!fw_cfg_present()) + return -1; + file = fw_cfg_find_file(name); + if (!file) + return -1; + return file->size; +} + +void fw_cfg_load_file(const char *name, void *dst) +{ + FWCfgFile *file; + + if (!fw_cfg_present()) + return; + file = fw_cfg_find_file(name); + if (!file) + return; + fw_cfg_get(file->select, dst, file->size); +} + +int fw_cfg_max_cpus(void) +{ + unsigned short max_cpus; + + if (!fw_cfg_present()) + return -1; + + fw_cfg_get(FW_CFG_MAX_CPUS, &max_cpus, sizeof(max_cpus)); + return max_cpus; +} + +/* ---------------------------------------------------------------------- */ + +/* + * Starting with release 1.7 qemu provides acpi tables via fw_cfg. + * Main advantage is that new (virtual) hardware which needs acpi + * support JustWorks[tm] without having to patch & update the firmware + * accordingly. + * + * Qemu provides a etc/table-loader file with instructions for the + * firmware: + * - A "load" instruction to fetch acpi data from fw_cfg. + * - A "pointer" instruction to patch a pointer. This is needed to + * get table-to-table references right, it is basically a + * primitive dynamic linker for acpi tables. + * - A "checksum" instruction to generate acpi table checksums. + * + * If a etc/table-loader file is found we'll go try loading the acpi + * tables from fw_cfg. + */ + +#define BIOS_LINKER_LOADER_FILESZ 56 + +struct BiosLinkerLoaderEntry { + uint32_t command; + union { + /* + * COMMAND_ALLOCATE - allocate a table from @alloc.file + * subject to @alloc.align alignment (must be power of 2) + * and @alloc.zone (can be HIGH or FSEG) requirements. + * + * Must appear exactly once for each file, and before + * this file is referenced by any other command. + */ + struct { + char file[BIOS_LINKER_LOADER_FILESZ]; + uint32_t align; + uint8_t zone; + } alloc; + + /* + * COMMAND_ADD_POINTER - patch the table (originating from + * @dest_file) at @pointer.offset, by adding a pointer to the table + * originating from @src_file. 1,2,4 or 8 byte unsigned + * addition is used depending on @pointer.size. + */ + struct { + char dest_file[BIOS_LINKER_LOADER_FILESZ]; + char src_file[BIOS_LINKER_LOADER_FILESZ]; + uint32_t offset; + uint8_t size; + } pointer; + + /* + * COMMAND_ADD_CHECKSUM - calculate checksum of the range specified by + * @cksum_start and @cksum_length fields, + * and then add the value at @cksum.offset. + * Checksum simply sums -X for each byte X in the range + * using 8-bit math. + */ + struct { + char file[BIOS_LINKER_LOADER_FILESZ]; + uint32_t offset; + uint32_t start; + uint32_t length; + } cksum; + + /* padding */ + char pad[124]; + }; +} __attribute__((packed)); +typedef struct BiosLinkerLoaderEntry BiosLinkerLoaderEntry; + +enum { + BIOS_LINKER_LOADER_COMMAND_ALLOCATE = 0x1, + BIOS_LINKER_LOADER_COMMAND_ADD_POINTER = 0x2, + BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM = 0x3, +}; + +enum { + BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH = 0x1, + BIOS_LINKER_LOADER_ALLOC_ZONE_FSEG = 0x2, +}; + +unsigned long fw_cfg_acpi_tables(unsigned long start) +{ + BiosLinkerLoaderEntry *s; + unsigned long *addrs, current; + uint32_t *ptr4; + uint64_t *ptr8; + int rc, i, j, src, dst, max; + + rc = fw_cfg_check_file("etc/table-loader"); + if (rc < 0) + return 0; + + + max = rc / sizeof(*s); + s = malloc(rc); + addrs = malloc(max * sizeof(*addrs)); + fw_cfg_load_file("etc/table-loader", s); + + current = start; + for (i = 0; i < max && s[i].command != 0; i++) { + switch (s[i].command) { + case BIOS_LINKER_LOADER_COMMAND_ALLOCATE: + current = ALIGN(current, s[i].alloc.align); + rc = fw_cfg_check_file(s[i].alloc.file); + if (rc < 0) + goto err; + fw_cfg_load_file(s[i].alloc.file, (void*)current); + addrs[i] = current; + current += rc; + break; + + case BIOS_LINKER_LOADER_COMMAND_ADD_POINTER: + src = -1; dst = -1; + for (j = 0; j < i; j++) { + if (s[j].command != BIOS_LINKER_LOADER_COMMAND_ALLOCATE) + continue; + if (strcmp(s[j].alloc.file, s[i].pointer.dest_file) == 0) + dst = j; + if (strcmp(s[j].alloc.file, s[i].pointer.src_file) == 0) + src = j; + } + if (src == -1 || dst == -1) + goto err; + + switch (s[i].pointer.size) { + case 4: + ptr4 = (uint32_t*)(addrs[dst] + s[i].pointer.offset); + *ptr4 += addrs[src]; + break; + + case 8: + ptr8 = (uint64_t*)(addrs[dst] + s[i].pointer.offset); + *ptr8 += addrs[src]; + break; + + default: + /* + * Should not happen. acpi knows 1 and 2 byte ptrs + * too, but we are operating with 32bit offsets which + * would simply not fit in there ... + */ + goto err; + } + break; + + case BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM: + dst = -1; + for (j = 0; j < i; j++) { + if (s[j].command != BIOS_LINKER_LOADER_COMMAND_ALLOCATE) + continue; + if (strcmp(s[j].alloc.file, s[i].cksum.file) == 0) + dst = j; + } + if (dst == -1) + goto err; + + ptr4 = (uint32_t*)(addrs[dst] + s[i].cksum.offset); + *ptr4 = 0; + *ptr4 = acpi_checksum((void *)(addrs[dst] + s[i].cksum.start), + s[i].cksum.length); + break; + + default: + goto err; + }; + } + + free(s); + free(addrs); + return ALIGN(current, 16); + +err: + free(s); + free(addrs); + return 0; +} + +u8 acpi_checksum(u8 *table, u32 length) +{ + u8 ret=0; + + while(length --){ + ret += *table; + table++; + } + return -ret; +} + diff --git a/arch/x86/include/asm/fw_cfg.h b/arch/x86/include/asm/fw_cfg.h new file mode 100644 index 0000000..9f512ca --- /dev/null +++ b/arch/x86/include/asm/fw_cfg.h @@ -0,0 +1,81 @@ +#include <linux/types.h> + +#define FW_CFG_SIGNATURE 0x00 +#define FW_CFG_ID 0x01 +#define FW_CFG_UUID 0x02 +#define FW_CFG_RAM_SIZE 0x03 +#define FW_CFG_NOGRAPHIC 0x04 +#define FW_CFG_NB_CPUS 0x05 +#define FW_CFG_MACHINE_ID 0x06 +#define FW_CFG_KERNEL_ADDR 0x07 +#define FW_CFG_KERNEL_SIZE 0x08 +#define FW_CFG_KERNEL_CMDLINE 0x09 +#define FW_CFG_INITRD_ADDR 0x0a +#define FW_CFG_INITRD_SIZE 0x0b +#define FW_CFG_BOOT_DEVICE 0x0c +#define FW_CFG_NUMA 0x0d +#define FW_CFG_BOOT_MENU 0x0e +#define FW_CFG_MAX_CPUS 0x0f +#define FW_CFG_KERNEL_ENTRY 0x10 +#define FW_CFG_KERNEL_DATA 0x11 +#define FW_CFG_INITRD_DATA 0x12 +#define FW_CFG_CMDLINE_ADDR 0x13 +#define FW_CFG_CMDLINE_SIZE 0x14 +#define FW_CFG_CMDLINE_DATA 0x15 +#define FW_CFG_SETUP_ADDR 0x16 +#define FW_CFG_SETUP_SIZE 0x17 +#define FW_CFG_SETUP_DATA 0x18 +#define FW_CFG_FILE_DIR 0x19 + +#define FW_CFG_FILE_FIRST 0x20 +#define FW_CFG_FILE_SLOTS 0x10 +#define FW_CFG_MAX_ENTRY (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS) + +#define FW_CFG_WRITE_CHANNEL 0x4000 +#define FW_CFG_ARCH_LOCAL 0x8000 +#define FW_CFG_ENTRY_MASK ~(FW_CFG_WRITE_CHANNEL | FW_CFG_ARCH_LOCAL) + +#define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0) +#define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1) +#define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2) +#define FW_CFG_E820_TABLE (FW_CFG_ARCH_LOCAL + 3) +#define FW_CFG_HPET (FW_CFG_ARCH_LOCAL + 4) + +#define FW_CFG_INVALID 0xffff + +typedef struct FWCfgFile { + uint32_t size; /* file size */ + uint16_t select; /* write this to 0x510 to read it */ + uint16_t reserved; + char name[56]; +} FWCfgFile; + +typedef struct FWCfgFiles { + uint32_t count; + FWCfgFile f[]; +} FWCfgFiles; + +typedef struct FwCfgE820Entry { + uint64_t address; + uint64_t length; + uint32_t type; +} FwCfgE820Entry __attribute((__aligned__(4))); + + +#define SMBIOS_FIELD_ENTRY 0 +#define SMBIOS_TABLE_ENTRY 1 + +typedef struct FwCfgSmbios { + uint16_t length; + uint8_t headertype; + uint8_t tabletype; + uint16_t fieldoffset; +} FwCfgSmbios; + + +void fw_cfg_get(int entry, void *dst, int dstlen); +int fw_cfg_check_file(const char *name); +void fw_cfg_load_file(const char *name, void *dst); +int fw_cfg_max_cpus(void); +unsigned long fw_cfg_acpi_tables(unsigned long start); +u8 acpi_checksum(u8 *table, u32 length); diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c index 8031201..eb49478 100644 --- a/arch/x86/lib/tables.c +++ b/arch/x86/lib/tables.c @@ -7,6 +7,7 @@ #include <common.h> #include <asm/sfi.h> #include <asm/tables.h> +#include <asm/fw_cfg.h>
u8 table_compute_checksum(void *v, int len) { @@ -32,4 +33,6 @@ void write_tables(void) rom_table_end = write_sfi_table(rom_table_end); rom_table_end = ALIGN(rom_table_end, 1024); #endif + rom_table_end = fw_cfg_acpi_tables(rom_table_end); + rom_table_end = ALIGN(rom_table_end, 1024); }
Apart from this, there is Bin's fix for a bug in the patch -
diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c index c6ef5cd..b5f3e0a 100644 --- a/arch/x86/cpu/qemu/fw_cfg.c +++ b/arch/x86/cpu/qemu/fw_cfg.c @@ -34,7 +34,8 @@ void fw_cfg_get(int entry, void *dst, int dstlen)
static void fw_cfg_init_file(void) {
u32 i, size, count = 0;
u32 i, size;volatile u32 count = 0; if (fw_files != NULL) return;
Lets see if we can get this patch functional and able to utilize fw_cfg to load tables for U-boot like SeaBIOS for qemu targets.
Regards, Saket Sinha
participants (3)
-
Bin Meng -
Miao Yan -
Saket Sinha