All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v4 0/8] x86: qemu: add fw_cfg interface support for qemu-x86 targets
@ 2015-12-31  2:55 Miao Yan
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 1/8] x86: qemu: add fw_cfg support Miao Yan
                   ` (8 more replies)
  0 siblings, 9 replies; 28+ messages in thread
From: Miao Yan @ 2015-12-31  2:55 UTC (permalink / raw)
  To: u-boot

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.

Changes in v4:
  - drop [PATCH v3 7/8] and limit maximum supported cpu number
  - chance 'fw load' to take second parameter for initrd load address
  - change default initrd load address for qemu-x86

Miao Yan (8):
  x86: qemu: add fw_cfg support
  x86: qemu: add a cpu uclass driver for qemu target
  x86: fix a typo in function name
  x86: use actual CPU number for allocating memory
  x86: qemu: add qemu_fwcfg_fdt_fixup()
  x86: qemu: fix up cpu node in device tree
  x86: qemu: adjust ramdisk load address
  x86: qemu: 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          |  57 +++++++
 arch/x86/cpu/qemu/fw_cfg.c       | 333 +++++++++++++++++++++++++++++++++++++++
 arch/x86/cpu/qemu/fw_cfg.h       | 108 +++++++++++++
 arch/x86/cpu/qemu/qemu.c         |   7 +
 arch/x86/dts/qemu-x86_i440fx.dts |  19 +--
 arch/x86/dts/qemu-x86_q35.dts    |  19 +--
 doc/README.x86                   |  35 +++-
 include/configs/qemu-x86.h       |  13 ++
 10 files changed, 559 insertions(+), 46 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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot]  [PATCH v4 1/8] x86: qemu: add fw_cfg support
  2015-12-31  2:55 [U-Boot] [PATCH v4 0/8] x86: qemu: add fw_cfg interface support for qemu-x86 targets Miao Yan
@ 2015-12-31  2:55 ` Miao Yan
  2015-12-31  5:07   ` Simon Glass
  2016-01-04  4:05   ` Bin Meng
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 2/8] x86: qemu: add a cpu uclass driver for qemu target Miao Yan
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 28+ messages in thread
From: Miao Yan @ 2015-12-31  2:55 UTC (permalink / raw)
  To: u-boot

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>
---
Changes in v4:
  - cleanups
  - change 'fw load' to take second parameter for initrd load address

 arch/x86/cpu/qemu/Makefile |   2 +-
 arch/x86/cpu/qemu/fw_cfg.c | 268 +++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/cpu/qemu/fw_cfg.h |  97 ++++++++++++++++
 arch/x86/cpu/qemu/qemu.c   |   3 +
 4 files changed, 369 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..9de8680
--- /dev/null
+++ b/arch/x86/cpu/qemu/fw_cfg.c
@@ -0,0 +1,268 @@
+/*
+ * (C) Copyright 2015 Miao Yan <yanmiaoebst@gmail.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <command.h>
+#include <errno.h>
+#include <malloc.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;
+
+	/*
+	 * writting FW_CFG_INVALID will cause
+	 * read operation to resume at
+	 * last offset, otherwise read will
+	 * start at offset 0
+	 */
+
+	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 size, void *address)
+{
+	struct fw_cfg_dma_access dma;
+
+	dma.length = cpu_to_be32(size);
+	dma.address = cpu_to_be64((uintptr_t)address);
+	dma.control = cpu_to_be32(FW_CFG_DMA_READ);
+
+	/*
+	 * writting FW_CFG_INVALID will cause
+	 * read operation to resume at
+	 * last offset, otherwise read will
+	 * start at offset 0
+	 */
+
+	if (entry != FW_CFG_INVALID)
+		dma.control |= cpu_to_be32(FW_CFG_DMA_SELECT | (entry << 16));
+
+	barrier();
+
+	debug("qemu_fwcfg_dma_read_entry: addr %p, length %u control 0x%x\n",
+	      address, size, be32_to_cpu(dma.control));
+
+	outl(cpu_to_be32((uint32_t)&dma), FW_DMA_PORT_HIGH);
+
+	while (be32_to_cpu(dma.control) & ~FW_CFG_DMA_ERROR)
+		__asm__ __volatile__ ("pause");
+}
+
+static bool 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 bool qemu_fwcfg_dma_present(void)
+{
+	uint8_t dma_enabled;
+
+	qemu_fwcfg_read_entry_pio(FW_CFG_ID, 1, &dma_enabled);
+	if (dma_enabled & FW_CFG_DMA_ENABLED)
+		return true;
+
+	return false;
+}
+
+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);
+}
+
+int 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 le16_to_cpu(nb_cpus);
+}
+
+static int qemu_fwcfg_setup_kernel(void *load_addr, void *initrd_addr)
+{
+	char *data_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;
+	}
+
+	data_addr = load_addr;
+	qemu_fwcfg_read_entry(FW_CFG_SETUP_DATA,
+			      le32_to_cpu(setup_size), data_addr);
+	data_addr += le32_to_cpu(setup_size);
+
+	qemu_fwcfg_read_entry(FW_CFG_KERNEL_DATA,
+			      le32_to_cpu(kernel_size), data_addr);
+	data_addr += le32_to_cpu(kernel_size);
+
+	data_addr = initrd_addr;
+	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,
+				      le32_to_cpu(initrd_size), data_addr);
+		data_addr += le32_to_cpu(initrd_size);
+	}
+
+	qemu_fwcfg_read_entry(FW_CFG_CMDLINE_SIZE, 4, &cmdline_size);
+	if (cmdline_size) {
+		qemu_fwcfg_read_entry(FW_CFG_CMDLINE_DATA,
+				      le32_to_cpu(cmdline_size), data_addr);
+		if (setenv("bootargs", data_addr) < 0)
+			printf("warning: unable to change bootargs\n");
+	}
+
+	printf("loading kernel to address %p", load_addr);
+	if (initrd_size)
+		printf(" initrd %p, size 0x%x\n",
+		       initrd_addr,
+		       le32_to_cpu(initrd_size));
+	else
+		printf("\n");
+
+	return 0;
+}
+
+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 = qemu_fwcfg_present();
+	if (fwcfg_present)
+		fwcfg_dma_present = qemu_fwcfg_dma_present();
+}
+
+static int qemu_fwcfg_do_list(cmd_tbl_t *cmdtp, int flag,
+		int argc, char * const argv[])
+{
+	qemu_fwcfg_list_firmware();
+
+	return 0;
+}
+
+static int qemu_fwcfg_do_cpus(cmd_tbl_t *cmdtp, int flag,
+		int argc, char * const argv[])
+{
+	printf("%d cpu(s) online\n", qemu_fwcfg_online_cpus());
+
+	return 0;
+}
+
+static int qemu_fwcfg_do_load(cmd_tbl_t *cmdtp, int flag,
+		int argc, char * const argv[])
+{
+	char *env;
+	void *load_addr;
+	void *initrd_addr;
+
+	env = getenv("loadaddr");
+	load_addr = env ?
+		(void *)simple_strtoul(env, NULL, 16) :
+		(void *)CONFIG_LOADADDR;
+
+	env = getenv("ramdiskaddr");
+	initrd_addr = env ?
+		(void *)simple_strtoul(env, NULL, 16) :
+		(void *)CONFIG_RAMDISK_ADDR;
+
+	if (argc == 2) {
+		load_addr = (void *)simple_strtoul(argv[0], NULL, 16);
+		initrd_addr = (void *)simple_strtoul(argv[1], NULL, 16);
+	} else if (argc == 1) {
+		load_addr = (void *)simple_strtoul(argv[0], NULL, 16);
+	}
+
+	return qemu_fwcfg_setup_kernel(load_addr, initrd_addr);
+}
+
+static cmd_tbl_t fwcfg_commands[] = {
+	U_BOOT_CMD_MKENT(list, 0, 1, qemu_fwcfg_do_list, "", ""),
+	U_BOOT_CMD_MKENT(cpus, 0, 1, qemu_fwcfg_do_cpus, "", ""),
+	U_BOOT_CMD_MKENT(load, 2, 1, qemu_fwcfg_do_load, "", ""),
+};
+
+static int do_qemu_fw(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	int ret;
+	cmd_tbl_t *fwcfg_cmd;
+
+	if (!fwcfg_present) {
+		printf("QEMU fw_cfg interface not found\n");
+		return CMD_RET_USAGE;
+	}
+
+	fwcfg_cmd = find_cmd_tbl(argv[1], fwcfg_commands,
+				 ARRAY_SIZE(fwcfg_commands));
+	argc -= 2;
+	argv += 2;
+	if (!fwcfg_cmd || argc > fwcfg_cmd->maxargs)
+		return CMD_RET_USAGE;
+
+	ret = fwcfg_cmd->cmd(fwcfg_cmd, flag, argc, argv);
+
+	return cmd_process_error(fwcfg_cmd, ret);
+}
+
+U_BOOT_CMD(
+	fw,	4,	1,	do_qemu_fw,
+	"QEMU firmware interface",
+	"<command>\n"
+	"    - list                             : print firmware(s) currently loaded\n"
+	"    - cpus                             : print online cpu number\n"
+	"    - load <kernel addr> <initrd addr> : load kernel and initrd (if any), 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..03ac27d
--- /dev/null
+++ b/arch/x86/cpu/qemu/fw_cfg.h
@@ -0,0 +1,97 @@
+/*
+ * (C) Copyright 2015 Miao Yan <yanmiaobest@gmail.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __FW_CFG__
+#define __FW_CFG__
+
+#define FW_CONTROL_PORT	0x510
+#define FW_DATA_PORT		0x511
+#define FW_DMA_PORT_LOW	0x514
+#define FW_DMA_PORT_HIGH	0x518
+
+enum qemu_fwcfg_items {
+	FW_CFG_SIGNATURE	= 0x00,
+	FW_CFG_ID		= 0x01,
+	FW_CFG_UUID		= 0x02,
+	FW_CFG_RAM_SIZE		= 0x03,
+	FW_CFG_NOGRAPHIC	= 0x04,
+	FW_CFG_NB_CPUS		= 0x05,
+	FW_CFG_MACHINE_ID	= 0x06,
+	FW_CFG_KERNEL_ADDR	= 0x07,
+	FW_CFG_KERNEL_SIZE	= 0x08,
+	FW_CFG_KERNEL_CMDLINE   = 0x09,
+	FW_CFG_INITRD_ADDR	= 0x0a,
+	FW_CFG_INITRD_SIZE	= 0x0b,
+	FW_CFG_BOOT_DEVICE	= 0x0c,
+	FW_CFG_NUMA		= 0x0d,
+	FW_CFG_BOOT_MENU	= 0x0e,
+	FW_CFG_MAX_CPUS		= 0x0f,
+	FW_CFG_KERNEL_ENTRY	= 0x10,
+	FW_CFG_KERNEL_DATA	= 0x11,
+	FW_CFG_INITRD_DATA	= 0x12,
+	FW_CFG_CMDLINE_ADDR	= 0x13,
+	FW_CFG_CMDLINE_SIZE	= 0x14,
+	FW_CFG_CMDLINE_DATA	= 0x15,
+	FW_CFG_SETUP_ADDR	= 0x16,
+	FW_CFG_SETUP_SIZE	= 0x17,
+	FW_CFG_SETUP_DATA	= 0x18,
+	FW_CFG_FILE_DIR		= 0x19,
+	FW_CFG_FILE_FIRST	= 0x20,
+	FW_CFG_WRITE_CHANNEL	= 0x4000,
+	FW_CFG_ARCH_LOCAL	= 0x8000,
+	FW_CFG_INVALID		= 0xffff,
+};
+
+#define FW_CFG_FILE_SLOTS	0x10
+#define FW_CFG_MAX_ENTRY	(FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
+#define FW_CFG_ENTRY_MASK	 ~(FW_CFG_WRITE_CHANNEL | FW_CFG_ARCH_LOCAL)
+
+#define FW_CFG_MAX_FILE_PATH	56
+
+#define QEMU_FW_CFG_SIGNATURE	(('Q' << 24) | ('E' << 16) | ('M' << 8) | 'U')
+
+#define FW_CFG_DMA_ERROR	(1 << 0)
+#define FW_CFG_DMA_READ	(1 << 1)
+#define FW_CFG_DMA_SKIP	(1 << 2)
+#define FW_CFG_DMA_SELECT	(1 << 3)
+
+#define FW_CFG_DMA_ENABLED	(1 << 1)
+
+struct fw_cfg_file {
+	__be32 size;
+	__be16 select;
+	__be16 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;
+};
+
+/**
+ * Initialize QEMU fw_cfg interface
+ *
+ * @return:   None
+ */
+
+void qemu_fwcfg_init(void);
+
+/**
+ * Get system cpu number
+ *
+ * @return:   cpu number in system
+ */
+
+int qemu_fwcfg_online_cpus(void);
+
+#endif
diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
index 1f93f72..d9ae066 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)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 2/8] x86: qemu: add a cpu uclass driver for qemu target
  2015-12-31  2:55 [U-Boot] [PATCH v4 0/8] x86: qemu: add fw_cfg interface support for qemu-x86 targets Miao Yan
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 1/8] x86: qemu: add fw_cfg support Miao Yan
@ 2015-12-31  2:55 ` Miao Yan
  2015-12-31  5:07   ` Simon Glass
  2016-01-04  4:05   ` Bin Meng
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 3/8] x86: fix a typo in function name Miao Yan
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 28+ messages in thread
From: Miao Yan @ 2015-12-31  2:55 UTC (permalink / raw)
  To: u-boot

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          | 57 ++++++++++++++++++++++++++++++++++++++++
 arch/x86/dts/qemu-x86_i440fx.dts |  4 +--
 arch/x86/dts/qemu-x86_q35.dts    |  4 +--
 4 files changed, 62 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..9bf9a7c
--- /dev/null
+++ b/arch/x86/cpu/qemu/cpu.c
@@ -0,0 +1,57 @@
+/*
+ * 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)
+{
+	return qemu_fwcfg_online_cpus();
+}
+
+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 8a06229..4332204 100644
--- a/arch/x86/dts/qemu-x86_i440fx.dts
+++ b/arch/x86/dts/qemu-x86_i440fx.dts
@@ -32,14 +32,14 @@
 
 		cpu@0 {
 			device_type = "cpu";
-			compatible = "cpu-x86";
+			compatible = "cpu-qemu";
 			reg = <0>;
 			intel,apic-id = <0>;
 		};
 
 		cpu at 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 0b685c8..3e2cfac 100644
--- a/arch/x86/dts/qemu-x86_q35.dts
+++ b/arch/x86/dts/qemu-x86_q35.dts
@@ -43,14 +43,14 @@
 
 		cpu at 0 {
 			device_type = "cpu";
-			compatible = "cpu-x86";
+			compatible = "cpu-qemu";
 			reg = <0>;
 			intel,apic-id = <0>;
 		};
 
 		cpu at 1 {
 			device_type = "cpu";
-			compatible = "cpu-x86";
+			compatible = "cpu-qemu";
 			reg = <1>;
 			intel,apic-id = <1>;
 		};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot]  [PATCH v4 3/8] x86: fix a typo in function name
  2015-12-31  2:55 [U-Boot] [PATCH v4 0/8] x86: qemu: add fw_cfg interface support for qemu-x86 targets Miao Yan
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 1/8] x86: qemu: add fw_cfg support Miao Yan
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 2/8] x86: qemu: add a cpu uclass driver for qemu target Miao Yan
@ 2015-12-31  2:55 ` Miao Yan
  2015-12-31  5:08   ` Simon Glass
  2016-01-04  4:05   ` Bin Meng
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 4/8] x86: use actual CPU number for allocating memory Miao Yan
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 28+ messages in thread
From: Miao Yan @ 2015-12-31  2:55 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 4/8] x86: use actual CPU number for allocating memory
  2015-12-31  2:55 [U-Boot] [PATCH v4 0/8] x86: qemu: add fw_cfg interface support for qemu-x86 targets Miao Yan
                   ` (2 preceding siblings ...)
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 3/8] x86: fix a typo in function name Miao Yan
@ 2015-12-31  2:55 ` Miao Yan
  2015-12-31  5:08   ` Simon Glass
  2016-01-04  4:05   ` Bin Meng
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 5/8] x86: qemu: add qemu_fwcfg_fdt_fixup() Miao Yan
                   ` (4 subsequent siblings)
  8 siblings, 2 replies; 28+ messages in thread
From: Miao Yan @ 2015-12-31  2:55 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot]  [PATCH v4 5/8] x86: qemu: add qemu_fwcfg_fdt_fixup()
  2015-12-31  2:55 [U-Boot] [PATCH v4 0/8] x86: qemu: add fw_cfg interface support for qemu-x86 targets Miao Yan
                   ` (3 preceding siblings ...)
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 4/8] x86: use actual CPU number for allocating memory Miao Yan
@ 2015-12-31  2:55 ` Miao Yan
  2015-12-31  5:08   ` Simon Glass
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 6/8] x86: qemu: fix up cpu node in device tree Miao Yan
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: Miao Yan @ 2015-12-31  2:55 UTC (permalink / raw)
  To: u-boot

Add a function to fix up 'cpus' node in dts files for qemu target.

Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
---
Changes in v4:
  - fix a typo in commit log

 arch/x86/cpu/qemu/fw_cfg.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/cpu/qemu/fw_cfg.h | 11 ++++++++
 2 files changed, 76 insertions(+)

diff --git a/arch/x86/cpu/qemu/fw_cfg.c b/arch/x86/cpu/qemu/fw_cfg.c
index 9de8680..5b1caa8 100644
--- a/arch/x86/cpu/qemu/fw_cfg.c
+++ b/arch/x86/cpu/qemu/fw_cfg.c
@@ -9,6 +9,7 @@
 #include <errno.h>
 #include <malloc.h>
 #include <asm/io.h>
+#include <libfdt.h>
 #include "fw_cfg.h"
 
 static bool fwcfg_present;
@@ -103,6 +104,70 @@ int qemu_fwcfg_online_cpus(void)
 	return le16_to_cpu(nb_cpus);
 }
 
+void qemu_fwcfg_fdt_fixup(void *fdt_addr, int cpu_num)
+{
+	int i;
+	char cpus[10];
+	int off, err, sub_off, id;
+
+	off = fdt_path_offset(fdt_addr, "/cpus");
+	if (off != -FDT_ERR_NOTFOUND) {
+		printf("error detecting cpus subnode: %s (%d)\n",
+		       fdt_strerror(off), off);
+		return;
+	}
+
+	off = fdt_add_subnode(fdt_addr, 0, "cpus");
+	if (off < 0) {
+		printf("error adding cpus subnode: %s (%d)\n",
+		       fdt_strerror(off), off);
+		return;
+	}
+
+	for (i = cpu_num - 1; i >= 0; i--) {
+		sprintf(cpus, "%s@%d", "cpu", i);
+		sub_off = fdt_add_subnode(fdt_addr, off, cpus);
+		if (sub_off < 0) {
+			printf("error adding subnode cpu %d: %s (%d)\n",
+			       i, fdt_strerror(sub_off), sub_off);
+			return;
+		}
+
+		id = cpu_to_fdt32(i);
+		err = fdt_setprop(fdt_addr, sub_off, "intel,apic-id",
+				(void *)&id, sizeof(id));
+		if (err < 0) {
+			printf("error adding apic-id %d: %s (%d)\n",
+			       i, fdt_strerror(err), err);
+			return;
+		}
+
+		err = fdt_setprop(fdt_addr, sub_off, "reg",
+				(void *)&id, sizeof(id));
+		if (err < 0) {
+			printf("error adding reg %d: %s (%d)\n",
+			       i, fdt_strerror(err), err);
+			return;
+		}
+
+		err = fdt_setprop(fdt_addr, sub_off, "compatible",
+				"cpu-qemu", sizeof("cpu-qemu"));
+		if (err < 0) {
+			printf("error adding compatible %d: %s (%d)\n",
+			       i, fdt_strerror(err), err);
+			return;
+		}
+
+		err = fdt_setprop(fdt_addr, sub_off, "device_type",
+				"cpu", sizeof("cpu"));
+		if (err < 0) {
+			printf("error adding device_type %d: %s (%d)\n",
+			       i, fdt_strerror(err), err);
+			return;
+		}
+	}
+}
+
 static int qemu_fwcfg_setup_kernel(void *load_addr, void *initrd_addr)
 {
 	char *data_addr;
diff --git a/arch/x86/cpu/qemu/fw_cfg.h b/arch/x86/cpu/qemu/fw_cfg.h
index 03ac27d..f2c9221 100644
--- a/arch/x86/cpu/qemu/fw_cfg.h
+++ b/arch/x86/cpu/qemu/fw_cfg.h
@@ -94,4 +94,15 @@ void qemu_fwcfg_init(void);
 
 int qemu_fwcfg_online_cpus(void);
 
+/**
+ * Fix 'cpu' node in device tree for qemu targets
+ *
+ * @fdt_addr: device tree blob address
+ * @cpu_num:  cpu number in system
+ *
+ * @return:   None
+ */
+
+void qemu_fwcfg_fdt_fixup(void *fdt_addr, int cpu_num);
+
 #endif
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 6/8] x86: qemu: fix up cpu node in device tree
  2015-12-31  2:55 [U-Boot] [PATCH v4 0/8] x86: qemu: add fw_cfg interface support for qemu-x86 targets Miao Yan
                   ` (4 preceding siblings ...)
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 5/8] x86: qemu: add qemu_fwcfg_fdt_fixup() Miao Yan
@ 2015-12-31  2:55 ` Miao Yan
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 7/8] x86: qemu: adjust ramdisk load address Miao Yan
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Miao Yan @ 2015-12-31  2:55 UTC (permalink / raw)
  To: u-boot

Remove 'cpus' node in dts files for QEMU targets,
retrieve cpu number through 'fw_cfg' interface and
fix up 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 | 19 +------------------
 arch/x86/dts/qemu-x86_q35.dts    | 19 +------------------
 3 files changed, 6 insertions(+), 36 deletions(-)

diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
index d9ae066..3be3af0 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;
@@ -93,6 +95,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 4332204..79bafbd 100644
--- a/arch/x86/dts/qemu-x86_i440fx.dts
+++ b/arch/x86/dts/qemu-x86_i440fx.dts
@@ -26,24 +26,7 @@
 		stdout-path = "/serial";
 	};
 
-	cpus {
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		cpu at 0 {
-			device_type = "cpu";
-			compatible = "cpu-qemu";
-			reg = <0>;
-			intel,apic-id = <0>;
-		};
-
-		cpu at 1 {
-			device_type = "cpu";
-			compatible = "cpu-qemu";
-			reg = <1>;
-			intel,apic-id = <1>;
-		};
-	};
+	/* cpu node will be dynamically filled by U-Boot */
 
 	tsc-timer {
 		clock-frequency = <1000000000>;
diff --git a/arch/x86/dts/qemu-x86_q35.dts b/arch/x86/dts/qemu-x86_q35.dts
index 3e2cfac..9563344 100644
--- a/arch/x86/dts/qemu-x86_q35.dts
+++ b/arch/x86/dts/qemu-x86_q35.dts
@@ -37,24 +37,7 @@
 		stdout-path = "/serial";
 	};
 
-	cpus {
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		cpu at 0 {
-			device_type = "cpu";
-			compatible = "cpu-qemu";
-			reg = <0>;
-			intel,apic-id = <0>;
-		};
-
-		cpu at 1 {
-			device_type = "cpu";
-			compatible = "cpu-qemu";
-			reg = <1>;
-			intel,apic-id = <1>;
-		};
-	};
+	/* cpu node will be dynamically filled by U-Boot */
 
 	tsc-timer {
 		clock-frequency = <1000000000>;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot]  [PATCH v4 7/8] x86: qemu: adjust ramdisk load address
  2015-12-31  2:55 [U-Boot] [PATCH v4 0/8] x86: qemu: add fw_cfg interface support for qemu-x86 targets Miao Yan
                   ` (5 preceding siblings ...)
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 6/8] x86: qemu: fix up cpu node in device tree Miao Yan
@ 2015-12-31  2:55 ` Miao Yan
  2015-12-31  5:08   ` Simon Glass
  2016-01-04  4:05   ` Bin Meng
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 8/8] x86: qemu: add documentaion for the fw_cfg interface Miao Yan
  2016-01-04  4:08 ` [U-Boot] [PATCH v4 0/8] x86: qemu: add fw_cfg interface support for qemu-x86 targets Bin Meng
  8 siblings, 2 replies; 28+ messages in thread
From: Miao Yan @ 2015-12-31  2:55 UTC (permalink / raw)
  To: u-boot

By default, ramdisk load address is defined to 02000000 in
env string. When loading bzImage to 100000 (default address), there's
a chance that the ramdisk header would be overwritten by
the kernel. Thus increase the gap and make ramdisk load at 04000000
by default.

Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
---
 include/configs/qemu-x86.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/configs/qemu-x86.h b/include/configs/qemu-x86.h
index 4258dcb..657b8af 100644
--- a/include/configs/qemu-x86.h
+++ b/include/configs/qemu-x86.h
@@ -57,4 +57,17 @@
 #undef CONFIG_ENV_IS_IN_SPI_FLASH
 #define CONFIG_ENV_IS_NOWHERE
 
+/* default ramdisk load address */
+#define CONFIG_RAMDISK_ADDR	0x04000000
+
+#undef CONFIG_EXTRA_ENV_SETTINGS
+#define CONFIG_EXTRA_ENV_SETTINGS			\
+	CONFIG_STD_DEVICES_SETTINGS			\
+	"pciconfighost=1\0"				\
+	"netdev=eth0\0"					\
+	"consoledev=ttyS0\0"				\
+	"othbootargs=acpi=off\0"			\
+	"ramdiskaddr=0x4000000\0"			\
+	"ramdiskfile=initramfs.gz\0"
+
 #endif	/* __CONFIG_H */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 8/8] x86: qemu: add documentaion for the fw_cfg interface
  2015-12-31  2:55 [U-Boot] [PATCH v4 0/8] x86: qemu: add fw_cfg interface support for qemu-x86 targets Miao Yan
                   ` (6 preceding siblings ...)
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 7/8] x86: qemu: adjust ramdisk load address Miao Yan
@ 2015-12-31  2:55 ` Miao Yan
  2015-12-31  5:08   ` Simon Glass
  2016-01-04  4:06   ` Bin Meng
  2016-01-04  4:08 ` [U-Boot] [PATCH v4 0/8] x86: qemu: add fw_cfg interface support for qemu-x86 targets Bin Meng
  8 siblings, 2 replies; 28+ messages in thread
From: Miao Yan @ 2015-12-31  2:55 UTC (permalink / raw)
  To: u-boot

Document the usage of 'fw' command

Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
---
Changes in v4:
  - limit maximum supported cpu number to 32

 doc/README.x86 | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/doc/README.x86 b/doc/README.x86
index 1271e5e..aa3010d 100644
--- a/doc/README.x86
+++ b/doc/README.x86
@@ -295,9 +295,38 @@ 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. Note, due space limitations in dtb, the maximum supported CPU
+number is 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-i386 -nographic -bios path/to/u-boot.rom -m 1024 -kernel /path/to/bzImage
+    -append 'root=/dev/ram 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 <kernel addr> <initrd addr> : load kernel and initrd (if any) and setup for zboot
+
+=> fw load
+loading kernel to address 02000000 initrd 04000000, size 0x1b1ab50
+
+Here the kernel (bzImage) is loaded to 02000000 and initrd is to 0x04000000. Then, 'zboot'
+can be used to boot the kernel:
+
+=> zboot 02000000 - 04000000 1b1ab50
 
 CPU Microcode
 -------------
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 1/8] x86: qemu: add fw_cfg support
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 1/8] x86: qemu: add fw_cfg support Miao Yan
@ 2015-12-31  5:07   ` Simon Glass
  2015-12-31  8:42     ` Miao Yan
  2016-01-04  4:05   ` Bin Meng
  1 sibling, 1 reply; 28+ messages in thread
From: Simon Glass @ 2015-12-31  5:07 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On 30 December 2015 at 19:55, 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>
> ---
> Changes in v4:
>   - cleanups
>   - change 'fw load' to take second parameter for initrd load address
>
>  arch/x86/cpu/qemu/Makefile |   2 +-
>  arch/x86/cpu/qemu/fw_cfg.c | 268 +++++++++++++++++++++++++++++++++++++++++++++
>  arch/x86/cpu/qemu/fw_cfg.h |  97 ++++++++++++++++
>  arch/x86/cpu/qemu/qemu.c   |   3 +
>  4 files changed, 369 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/cpu/qemu/fw_cfg.c
>  create mode 100644 arch/x86/cpu/qemu/fw_cfg.h

Reviewed-by: Simon Glass <sjg@chromium.org>

But a few nits...

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

Can you put the new file first, so that the list is in alphabetical order?

>  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..9de8680
> --- /dev/null
> +++ b/arch/x86/cpu/qemu/fw_cfg.c
> @@ -0,0 +1,268 @@
> +/*
> + * (C) Copyright 2015 Miao Yan <yanmiaoebst@gmail.com>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <errno.h>
> +#include <malloc.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)

Function comment - what does this do?

> +{
> +       uint32_t i = 0;
> +       uint8_t *data = address;
> +
> +       /*
> +        * writting FW_CFG_INVALID will cause
> +        * read operation to resume at
> +        * last offset, otherwise read will
> +        * start at offset 0

Please can you format comments to 75 columns or thereabout?

> +        */
> +
> +       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 size, void *address)
> +{
> +       struct fw_cfg_dma_access dma;
> +
> +       dma.length = cpu_to_be32(size);
> +       dma.address = cpu_to_be64((uintptr_t)address);
> +       dma.control = cpu_to_be32(FW_CFG_DMA_READ);
> +
> +       /*
> +        * writting FW_CFG_INVALID will cause
> +        * read operation to resume at
> +        * last offset, otherwise read will
> +        * start at offset 0
> +        */
> +
> +       if (entry != FW_CFG_INVALID)
> +               dma.control |= cpu_to_be32(FW_CFG_DMA_SELECT | (entry << 16));
> +
> +       barrier();
> +
> +       debug("qemu_fwcfg_dma_read_entry: addr %p, length %u control 0x%x\n",
> +             address, size, be32_to_cpu(dma.control));
> +
> +       outl(cpu_to_be32((uint32_t)&dma), FW_DMA_PORT_HIGH);
> +
> +       while (be32_to_cpu(dma.control) & ~FW_CFG_DMA_ERROR)
> +               __asm__ __volatile__ ("pause");
> +}
> +
> +static bool 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 bool qemu_fwcfg_dma_present(void)
> +{
> +       uint8_t dma_enabled;
> +
> +       qemu_fwcfg_read_entry_pio(FW_CFG_ID, 1, &dma_enabled);
> +       if (dma_enabled & FW_CFG_DMA_ENABLED)
> +               return true;
> +
> +       return false;
> +}
> +
> +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);
> +}
> +
> +int qemu_fwcfg_online_cpus(void)
> +{
> +       uint16_t nb_cpus;
> +
> +       if (!fwcfg_present)
> +               return 1;

-ENODEV

> +
> +       qemu_fwcfg_read_entry(FW_CFG_NB_CPUS, 2, &nb_cpus);
> +
> +       return le16_to_cpu(nb_cpus);
> +}
> +
> +static int qemu_fwcfg_setup_kernel(void *load_addr, void *initrd_addr)

Function comment

> +{
> +       char *data_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;
> +       }
> +
> +       data_addr = load_addr;
> +       qemu_fwcfg_read_entry(FW_CFG_SETUP_DATA,
> +                             le32_to_cpu(setup_size), data_addr);
> +       data_addr += le32_to_cpu(setup_size);
> +
> +       qemu_fwcfg_read_entry(FW_CFG_KERNEL_DATA,
> +                             le32_to_cpu(kernel_size), data_addr);
> +       data_addr += le32_to_cpu(kernel_size);
> +
> +       data_addr = initrd_addr;
> +       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,
> +                                     le32_to_cpu(initrd_size), data_addr);
> +               data_addr += le32_to_cpu(initrd_size);
> +       }
> +
> +       qemu_fwcfg_read_entry(FW_CFG_CMDLINE_SIZE, 4, &cmdline_size);
> +       if (cmdline_size) {
> +               qemu_fwcfg_read_entry(FW_CFG_CMDLINE_DATA,
> +                                     le32_to_cpu(cmdline_size), data_addr);
> +               if (setenv("bootargs", data_addr) < 0)
> +                       printf("warning: unable to change bootargs\n");
> +       }
> +
> +       printf("loading kernel to address %p", load_addr);
> +       if (initrd_size)
> +               printf(" initrd %p, size 0x%x\n",
> +                      initrd_addr,
> +                      le32_to_cpu(initrd_size));
> +       else
> +               printf("\n");
> +
> +       return 0;
> +}
> +
> +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 = qemu_fwcfg_present();
> +       if (fwcfg_present)
> +               fwcfg_dma_present = qemu_fwcfg_dma_present();
> +}
> +
> +static int qemu_fwcfg_do_list(cmd_tbl_t *cmdtp, int flag,
> +               int argc, char * const argv[])
> +{
> +       qemu_fwcfg_list_firmware();

Check return value and return CMD_RET_FAILURE if non-zero.

> +
> +       return 0;
> +}
> +
> +static int qemu_fwcfg_do_cpus(cmd_tbl_t *cmdtp, int flag,
> +               int argc, char * const argv[])
> +{
> +       printf("%d cpu(s) online\n", qemu_fwcfg_online_cpus());

But qemu_fwcfg_online_cpus() can fail.

> +
> +       return 0;
> +}
> +
> +static int qemu_fwcfg_do_load(cmd_tbl_t *cmdtp, int flag,
> +               int argc, char * const argv[])
> +{
> +       char *env;
> +       void *load_addr;
> +       void *initrd_addr;
> +
> +       env = getenv("loadaddr");
> +       load_addr = env ?
> +               (void *)simple_strtoul(env, NULL, 16) :
> +               (void *)CONFIG_LOADADDR;
> +
> +       env = getenv("ramdiskaddr");
> +       initrd_addr = env ?
> +               (void *)simple_strtoul(env, NULL, 16) :
> +               (void *)CONFIG_RAMDISK_ADDR;
> +
> +       if (argc == 2) {
> +               load_addr = (void *)simple_strtoul(argv[0], NULL, 16);
> +               initrd_addr = (void *)simple_strtoul(argv[1], NULL, 16);
> +       } else if (argc == 1) {
> +               load_addr = (void *)simple_strtoul(argv[0], NULL, 16);
> +       }
> +
> +       return qemu_fwcfg_setup_kernel(load_addr, initrd_addr);
> +}
> +
> +static cmd_tbl_t fwcfg_commands[] = {
> +       U_BOOT_CMD_MKENT(list, 0, 1, qemu_fwcfg_do_list, "", ""),
> +       U_BOOT_CMD_MKENT(cpus, 0, 1, qemu_fwcfg_do_cpus, "", ""),
> +       U_BOOT_CMD_MKENT(load, 2, 1, qemu_fwcfg_do_load, "", ""),
> +};
> +
> +static int do_qemu_fw(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +       int ret;
> +       cmd_tbl_t *fwcfg_cmd;
> +
> +       if (!fwcfg_present) {
> +               printf("QEMU fw_cfg interface not found\n");
> +               return CMD_RET_USAGE;
> +       }
> +
> +       fwcfg_cmd = find_cmd_tbl(argv[1], fwcfg_commands,
> +                                ARRAY_SIZE(fwcfg_commands));
> +       argc -= 2;
> +       argv += 2;
> +       if (!fwcfg_cmd || argc > fwcfg_cmd->maxargs)
> +               return CMD_RET_USAGE;
> +
> +       ret = fwcfg_cmd->cmd(fwcfg_cmd, flag, argc, argv);
> +
> +       return cmd_process_error(fwcfg_cmd, ret);
> +}
> +
> +U_BOOT_CMD(
> +       fw,     4,      1,      do_qemu_fw,

I worry that 'fw' is too generic. Perhaps qfw would be better?

> +       "QEMU firmware interface",
> +       "<command>\n"
> +       "    - list                             : print firmware(s) currently loaded\n"
> +       "    - cpus                             : print online cpu number\n"

print number of online CPUs

> +       "    - load <kernel addr> <initrd addr> : load kernel and initrd (if any), 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..03ac27d
> --- /dev/null
> +++ b/arch/x86/cpu/qemu/fw_cfg.h
> @@ -0,0 +1,97 @@
> +/*
> + * (C) Copyright 2015 Miao Yan <yanmiaobest@gmail.com>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef __FW_CFG__
> +#define __FW_CFG__
> +
> +#define FW_CONTROL_PORT        0x510
> +#define FW_DATA_PORT           0x511
> +#define FW_DMA_PORT_LOW        0x514
> +#define FW_DMA_PORT_HIGH       0x518
> +
> +enum qemu_fwcfg_items {
> +       FW_CFG_SIGNATURE        = 0x00,
> +       FW_CFG_ID               = 0x01,
> +       FW_CFG_UUID             = 0x02,
> +       FW_CFG_RAM_SIZE         = 0x03,
> +       FW_CFG_NOGRAPHIC        = 0x04,
> +       FW_CFG_NB_CPUS          = 0x05,
> +       FW_CFG_MACHINE_ID       = 0x06,
> +       FW_CFG_KERNEL_ADDR      = 0x07,
> +       FW_CFG_KERNEL_SIZE      = 0x08,
> +       FW_CFG_KERNEL_CMDLINE   = 0x09,
> +       FW_CFG_INITRD_ADDR      = 0x0a,
> +       FW_CFG_INITRD_SIZE      = 0x0b,
> +       FW_CFG_BOOT_DEVICE      = 0x0c,
> +       FW_CFG_NUMA             = 0x0d,
> +       FW_CFG_BOOT_MENU        = 0x0e,
> +       FW_CFG_MAX_CPUS         = 0x0f,
> +       FW_CFG_KERNEL_ENTRY     = 0x10,
> +       FW_CFG_KERNEL_DATA      = 0x11,
> +       FW_CFG_INITRD_DATA      = 0x12,
> +       FW_CFG_CMDLINE_ADDR     = 0x13,
> +       FW_CFG_CMDLINE_SIZE     = 0x14,
> +       FW_CFG_CMDLINE_DATA     = 0x15,
> +       FW_CFG_SETUP_ADDR       = 0x16,
> +       FW_CFG_SETUP_SIZE       = 0x17,
> +       FW_CFG_SETUP_DATA       = 0x18,
> +       FW_CFG_FILE_DIR         = 0x19,
> +       FW_CFG_FILE_FIRST       = 0x20,
> +       FW_CFG_WRITE_CHANNEL    = 0x4000,
> +       FW_CFG_ARCH_LOCAL       = 0x8000,
> +       FW_CFG_INVALID          = 0xffff,
> +};
> +
> +#define FW_CFG_FILE_SLOTS      0x10
> +#define FW_CFG_MAX_ENTRY       (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
> +#define FW_CFG_ENTRY_MASK       ~(FW_CFG_WRITE_CHANNEL | FW_CFG_ARCH_LOCAL)
> +
> +#define FW_CFG_MAX_FILE_PATH   56
> +
> +#define QEMU_FW_CFG_SIGNATURE  (('Q' << 24) | ('E' << 16) | ('M' << 8) | 'U')
> +
> +#define FW_CFG_DMA_ERROR       (1 << 0)
> +#define FW_CFG_DMA_READ        (1 << 1)
> +#define FW_CFG_DMA_SKIP        (1 << 2)
> +#define FW_CFG_DMA_SELECT      (1 << 3)
> +
> +#define FW_CFG_DMA_ENABLED     (1 << 1)
> +
> +struct fw_cfg_file {
> +       __be32 size;
> +       __be16 select;
> +       __be16 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;
> +};
> +
> +/**
> + * Initialize QEMU fw_cfg interface
> + *
> + * @return:   None

You can drop this line

> + */
> +
> +void qemu_fwcfg_init(void);
> +
> +/**
> + * Get system cpu number
> + *
> + * @return:   cpu number in system
> + */
> +
> +int qemu_fwcfg_online_cpus(void);
> +
> +#endif
> diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
> index 1f93f72..d9ae066 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)
> --
> 1.9.1
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 2/8] x86: qemu: add a cpu uclass driver for qemu target
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 2/8] x86: qemu: add a cpu uclass driver for qemu target Miao Yan
@ 2015-12-31  5:07   ` Simon Glass
  2016-01-04  4:05   ` Bin Meng
  1 sibling, 0 replies; 28+ messages in thread
From: Simon Glass @ 2015-12-31  5:07 UTC (permalink / raw)
  To: u-boot

On 30 December 2015 at 19:55, 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.

Please try to wrap your commit messages to around 75 columns.

>
> Signed-off-by: <yanmiaobest@gmail.com>
> ---
>  arch/x86/cpu/qemu/Makefile       |  2 +-
>  arch/x86/cpu/qemu/cpu.c          | 57 ++++++++++++++++++++++++++++++++++++++++
>  arch/x86/dts/qemu-x86_i440fx.dts |  4 +--
>  arch/x86/dts/qemu-x86_q35.dts    |  4 +--
>  4 files changed, 62 insertions(+), 5 deletions(-)
>  create mode 100644 arch/x86/cpu/qemu/cpu.c

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 3/8] x86: fix a typo in function name
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 3/8] x86: fix a typo in function name Miao Yan
@ 2015-12-31  5:08   ` Simon Glass
  2016-01-04  4:05   ` Bin Meng
  1 sibling, 0 replies; 28+ messages in thread
From: Simon Glass @ 2015-12-31  5:08 UTC (permalink / raw)
  To: u-boot

On 30 December 2015 at 19:55, 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: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 4/8] x86: use actual CPU number for allocating memory
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 4/8] x86: use actual CPU number for allocating memory Miao Yan
@ 2015-12-31  5:08   ` Simon Glass
  2016-01-04  4:05   ` Bin Meng
  1 sibling, 0 replies; 28+ messages in thread
From: Simon Glass @ 2015-12-31  5:08 UTC (permalink / raw)
  To: u-boot

On 30 December 2015 at 19:55, Miao Yan <yanmiaobest@gmail.com> wrote:
> 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(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 5/8] x86: qemu: add qemu_fwcfg_fdt_fixup()
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 5/8] x86: qemu: add qemu_fwcfg_fdt_fixup() Miao Yan
@ 2015-12-31  5:08   ` Simon Glass
  2015-12-31  9:02     ` Miao Yan
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Glass @ 2015-12-31  5:08 UTC (permalink / raw)
  To: u-boot

Hi Maio,

On 30 December 2015 at 19:55, Miao Yan <yanmiaobest@gmail.com> wrote:
> Add a function to fix up 'cpus' node in dts files for qemu target.
>
> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
> ---
> Changes in v4:
>   - fix a typo in commit log
>
>  arch/x86/cpu/qemu/fw_cfg.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
>  arch/x86/cpu/qemu/fw_cfg.h | 11 ++++++++
>  2 files changed, 76 insertions(+)

I'm sorry for not reviewing this earlier (Christmas and all that). I
don't think you need to update the device tree to make this work.

Here's my suggestion:

- At present cpu_x86_bind() sets up the CPU APIC ID from the device tree
- You can bind new CPU devices in your code on start-up
- You can check if you have CPUs which are not available in the device
list, by using uclass_find_first/next_device() to iterate through the
devices without probing them
- Then to create a new one, call device_bind_driver() with the /cpus
node as the parent
- After binding, update the parent platform data:

  struct cpu_platdata *plat = dev_get_parent_platdata(dev);

   plat->cpu_id = ...

- Then when it comes to probing, you will have all the devices you
need, and you don't need to adjust the device tree. The device tree
can just hold a single device, for example.

I think it is better to do this than adjust the device tree because it
removes the 32-CPU limit and should be faster. It is also simpler as
it is a more direct method. Also I believe you only need to do this
after relocation - e.g. in arch_early_init_r(), which is before
mp_init() is called from cpu_init_r().

I wonder if there is a general way to probe available CPUs (and their
APIC IDs)? Or is qemu the only 'CPU' with this feature?

Regards,
Simon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 7/8] x86: qemu: adjust ramdisk load address
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 7/8] x86: qemu: adjust ramdisk load address Miao Yan
@ 2015-12-31  5:08   ` Simon Glass
  2016-01-04  4:05   ` Bin Meng
  1 sibling, 0 replies; 28+ messages in thread
From: Simon Glass @ 2015-12-31  5:08 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On 30 December 2015 at 19:55, Miao Yan <yanmiaobest@gmail.com> wrote:
> By default, ramdisk load address is defined to 02000000 in
> env string. When loading bzImage to 100000 (default address), there's
> a chance that the ramdisk header would be overwritten by
> the kernel. Thus increase the gap and make ramdisk load at 04000000
> by default.
>
> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
> ---
>  include/configs/qemu-x86.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

But you add other env options here to. Can you mention more of these
in the commit message too?

>
> diff --git a/include/configs/qemu-x86.h b/include/configs/qemu-x86.h
> index 4258dcb..657b8af 100644
> --- a/include/configs/qemu-x86.h
> +++ b/include/configs/qemu-x86.h
> @@ -57,4 +57,17 @@
>  #undef CONFIG_ENV_IS_IN_SPI_FLASH
>  #define CONFIG_ENV_IS_NOWHERE
>
> +/* default ramdisk load address */
> +#define CONFIG_RAMDISK_ADDR    0x04000000
> +
> +#undef CONFIG_EXTRA_ENV_SETTINGS
> +#define CONFIG_EXTRA_ENV_SETTINGS                      \
> +       CONFIG_STD_DEVICES_SETTINGS                     \
> +       "pciconfighost=1\0"                             \
> +       "netdev=eth0\0"                                 \
> +       "consoledev=ttyS0\0"                            \
> +       "othbootargs=acpi=off\0"                        \
> +       "ramdiskaddr=0x4000000\0"                       \
> +       "ramdiskfile=initramfs.gz\0"
> +
>  #endif /* __CONFIG_H */
> --
> 1.9.1
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 8/8] x86: qemu: add documentaion for the fw_cfg interface
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 8/8] x86: qemu: add documentaion for the fw_cfg interface Miao Yan
@ 2015-12-31  5:08   ` Simon Glass
  2016-01-04  4:06   ` Bin Meng
  1 sibling, 0 replies; 28+ messages in thread
From: Simon Glass @ 2015-12-31  5:08 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On 30 December 2015 at 19:55, Miao Yan <yanmiaobest@gmail.com> wrote:
> Document the usage of 'fw' command
>
> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
> ---
> Changes in v4:
>   - limit maximum supported cpu number to 32
>
>  doc/README.x86 | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/doc/README.x86 b/doc/README.x86
> index 1271e5e..aa3010d 100644
> --- a/doc/README.x86
> +++ b/doc/README.x86
> @@ -295,9 +295,38 @@ 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.

This is a little unclear, because it seems like your code actually
fixes this by automatically supporting more CPUs.

> +to instantiate. Note, due space limitations in dtb, the maximum supported CPU
> +number is 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-i386 -nographic -bios path/to/u-boot.rom -m 1024 -kernel /path/to/bzImage
> +    -append 'root=/dev/ram 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 <kernel addr> <initrd addr> : load kernel and initrd (if any) and setup for zboot
> +
> +=> fw load
> +loading kernel to address 02000000 initrd 04000000, size 0x1b1ab50
> +
> +Here the kernel (bzImage) is loaded to 02000000 and initrd is to 0x04000000. Then, 'zboot'
> +can be used to boot the kernel:
> +
> +=> zboot 02000000 - 04000000 1b1ab50
>
>  CPU Microcode
>  -------------
> --
> 1.9.1
>

Great documentation!

Regards,
Simon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 1/8] x86: qemu: add fw_cfg support
  2015-12-31  5:07   ` Simon Glass
@ 2015-12-31  8:42     ` Miao Yan
  2015-12-31 12:22       ` Simon Glass
  0 siblings, 1 reply; 28+ messages in thread
From: Miao Yan @ 2015-12-31  8:42 UTC (permalink / raw)
  To: u-boot

Hi Simon,

2015-12-31 13:07 GMT+08:00 Simon Glass <sjg@chromium.org>:
> Hi Miao,
>
> On 30 December 2015 at 19:55, 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>
>> ---
>> Changes in v4:
>>   - cleanups
>>   - change 'fw load' to take second parameter for initrd load address
>>
>>  arch/x86/cpu/qemu/Makefile |   2 +-
>>  arch/x86/cpu/qemu/fw_cfg.c | 268 +++++++++++++++++++++++++++++++++++++++++++++
>>  arch/x86/cpu/qemu/fw_cfg.h |  97 ++++++++++++++++
>>  arch/x86/cpu/qemu/qemu.c   |   3 +
>>  4 files changed, 369 insertions(+), 1 deletion(-)
>>  create mode 100644 arch/x86/cpu/qemu/fw_cfg.c
>>  create mode 100644 arch/x86/cpu/qemu/fw_cfg.h
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> But a few nits...
>
>>
>> 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
>
> Can you put the new file first, so that the list is in alphabetical order?
>
>>  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..9de8680
>> --- /dev/null
>> +++ b/arch/x86/cpu/qemu/fw_cfg.c
>> @@ -0,0 +1,268 @@
>> +/*
>> + * (C) Copyright 2015 Miao Yan <yanmiaoebst@gmail.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <command.h>
>> +#include <errno.h>
>> +#include <malloc.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)
>
> Function comment - what does this do?
>
>> +{
>> +       uint32_t i = 0;
>> +       uint8_t *data = address;
>> +
>> +       /*
>> +        * writting FW_CFG_INVALID will cause
>> +        * read operation to resume at
>> +        * last offset, otherwise read will
>> +        * start at offset 0
>
> Please can you format comments to 75 columns or thereabout?
>
>> +        */
>> +
>> +       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 size, void *address)
>> +{
>> +       struct fw_cfg_dma_access dma;
>> +
>> +       dma.length = cpu_to_be32(size);
>> +       dma.address = cpu_to_be64((uintptr_t)address);
>> +       dma.control = cpu_to_be32(FW_CFG_DMA_READ);
>> +
>> +       /*
>> +        * writting FW_CFG_INVALID will cause
>> +        * read operation to resume at
>> +        * last offset, otherwise read will
>> +        * start at offset 0
>> +        */
>> +
>> +       if (entry != FW_CFG_INVALID)
>> +               dma.control |= cpu_to_be32(FW_CFG_DMA_SELECT | (entry << 16));
>> +
>> +       barrier();
>> +
>> +       debug("qemu_fwcfg_dma_read_entry: addr %p, length %u control 0x%x\n",
>> +             address, size, be32_to_cpu(dma.control));
>> +
>> +       outl(cpu_to_be32((uint32_t)&dma), FW_DMA_PORT_HIGH);
>> +
>> +       while (be32_to_cpu(dma.control) & ~FW_CFG_DMA_ERROR)
>> +               __asm__ __volatile__ ("pause");
>> +}
>> +
>> +static bool 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 bool qemu_fwcfg_dma_present(void)
>> +{
>> +       uint8_t dma_enabled;
>> +
>> +       qemu_fwcfg_read_entry_pio(FW_CFG_ID, 1, &dma_enabled);
>> +       if (dma_enabled & FW_CFG_DMA_ENABLED)
>> +               return true;
>> +
>> +       return false;
>> +}
>> +
>> +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);
>> +}
>> +
>> +int qemu_fwcfg_online_cpus(void)
>> +{
>> +       uint16_t nb_cpus;
>> +
>> +       if (!fwcfg_present)
>> +               return 1;
>
> -ENODEV


Can we return 1 cpu if fw_cfg interface is not avaliable (which is
quite unlikey),
and print a warning maybe ? Because there has to be one cpu at least
and returning
-ENODEV wouldn't make much difference.

I'll fix rest of your comments and thanks for the review.

Miao


>
>> +
>> +       qemu_fwcfg_read_entry(FW_CFG_NB_CPUS, 2, &nb_cpus);
>> +
>> +       return le16_to_cpu(nb_cpus);
>> +}
>> +
>> +static int qemu_fwcfg_setup_kernel(void *load_addr, void *initrd_addr)
>
> Function comment
>
>> +{
>> +       char *data_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;
>> +       }
>> +
>> +       data_addr = load_addr;
>> +       qemu_fwcfg_read_entry(FW_CFG_SETUP_DATA,
>> +                             le32_to_cpu(setup_size), data_addr);
>> +       data_addr += le32_to_cpu(setup_size);
>> +
>> +       qemu_fwcfg_read_entry(FW_CFG_KERNEL_DATA,
>> +                             le32_to_cpu(kernel_size), data_addr);
>> +       data_addr += le32_to_cpu(kernel_size);
>> +
>> +       data_addr = initrd_addr;
>> +       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,
>> +                                     le32_to_cpu(initrd_size), data_addr);
>> +               data_addr += le32_to_cpu(initrd_size);
>> +       }
>> +
>> +       qemu_fwcfg_read_entry(FW_CFG_CMDLINE_SIZE, 4, &cmdline_size);
>> +       if (cmdline_size) {
>> +               qemu_fwcfg_read_entry(FW_CFG_CMDLINE_DATA,
>> +                                     le32_to_cpu(cmdline_size), data_addr);
>> +               if (setenv("bootargs", data_addr) < 0)
>> +                       printf("warning: unable to change bootargs\n");
>> +       }
>> +
>> +       printf("loading kernel to address %p", load_addr);
>> +       if (initrd_size)
>> +               printf(" initrd %p, size 0x%x\n",
>> +                      initrd_addr,
>> +                      le32_to_cpu(initrd_size));
>> +       else
>> +               printf("\n");
>> +
>> +       return 0;
>> +}
>> +
>> +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 = qemu_fwcfg_present();
>> +       if (fwcfg_present)
>> +               fwcfg_dma_present = qemu_fwcfg_dma_present();
>> +}
>> +
>> +static int qemu_fwcfg_do_list(cmd_tbl_t *cmdtp, int flag,
>> +               int argc, char * const argv[])
>> +{
>> +       qemu_fwcfg_list_firmware();
>
> Check return value and return CMD_RET_FAILURE if non-zero.
>
>> +
>> +       return 0;
>> +}
>> +
>> +static int qemu_fwcfg_do_cpus(cmd_tbl_t *cmdtp, int flag,
>> +               int argc, char * const argv[])
>> +{
>> +       printf("%d cpu(s) online\n", qemu_fwcfg_online_cpus());
>
> But qemu_fwcfg_online_cpus() can fail.
>
>> +
>> +       return 0;
>> +}
>> +
>> +static int qemu_fwcfg_do_load(cmd_tbl_t *cmdtp, int flag,
>> +               int argc, char * const argv[])
>> +{
>> +       char *env;
>> +       void *load_addr;
>> +       void *initrd_addr;
>> +
>> +       env = getenv("loadaddr");
>> +       load_addr = env ?
>> +               (void *)simple_strtoul(env, NULL, 16) :
>> +               (void *)CONFIG_LOADADDR;
>> +
>> +       env = getenv("ramdiskaddr");
>> +       initrd_addr = env ?
>> +               (void *)simple_strtoul(env, NULL, 16) :
>> +               (void *)CONFIG_RAMDISK_ADDR;
>> +
>> +       if (argc == 2) {
>> +               load_addr = (void *)simple_strtoul(argv[0], NULL, 16);
>> +               initrd_addr = (void *)simple_strtoul(argv[1], NULL, 16);
>> +       } else if (argc == 1) {
>> +               load_addr = (void *)simple_strtoul(argv[0], NULL, 16);
>> +       }
>> +
>> +       return qemu_fwcfg_setup_kernel(load_addr, initrd_addr);
>> +}
>> +
>> +static cmd_tbl_t fwcfg_commands[] = {
>> +       U_BOOT_CMD_MKENT(list, 0, 1, qemu_fwcfg_do_list, "", ""),
>> +       U_BOOT_CMD_MKENT(cpus, 0, 1, qemu_fwcfg_do_cpus, "", ""),
>> +       U_BOOT_CMD_MKENT(load, 2, 1, qemu_fwcfg_do_load, "", ""),
>> +};
>> +
>> +static int do_qemu_fw(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>> +{
>> +       int ret;
>> +       cmd_tbl_t *fwcfg_cmd;
>> +
>> +       if (!fwcfg_present) {
>> +               printf("QEMU fw_cfg interface not found\n");
>> +               return CMD_RET_USAGE;
>> +       }
>> +
>> +       fwcfg_cmd = find_cmd_tbl(argv[1], fwcfg_commands,
>> +                                ARRAY_SIZE(fwcfg_commands));
>> +       argc -= 2;
>> +       argv += 2;
>> +       if (!fwcfg_cmd || argc > fwcfg_cmd->maxargs)
>> +               return CMD_RET_USAGE;
>> +
>> +       ret = fwcfg_cmd->cmd(fwcfg_cmd, flag, argc, argv);
>> +
>> +       return cmd_process_error(fwcfg_cmd, ret);
>> +}
>> +
>> +U_BOOT_CMD(
>> +       fw,     4,      1,      do_qemu_fw,
>
> I worry that 'fw' is too generic. Perhaps qfw would be better?
>
>> +       "QEMU firmware interface",
>> +       "<command>\n"
>> +       "    - list                             : print firmware(s) currently loaded\n"
>> +       "    - cpus                             : print online cpu number\n"
>
> print number of online CPUs
>
>> +       "    - load <kernel addr> <initrd addr> : load kernel and initrd (if any), 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..03ac27d
>> --- /dev/null
>> +++ b/arch/x86/cpu/qemu/fw_cfg.h
>> @@ -0,0 +1,97 @@
>> +/*
>> + * (C) Copyright 2015 Miao Yan <yanmiaobest@gmail.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#ifndef __FW_CFG__
>> +#define __FW_CFG__
>> +
>> +#define FW_CONTROL_PORT        0x510
>> +#define FW_DATA_PORT           0x511
>> +#define FW_DMA_PORT_LOW        0x514
>> +#define FW_DMA_PORT_HIGH       0x518
>> +
>> +enum qemu_fwcfg_items {
>> +       FW_CFG_SIGNATURE        = 0x00,
>> +       FW_CFG_ID               = 0x01,
>> +       FW_CFG_UUID             = 0x02,
>> +       FW_CFG_RAM_SIZE         = 0x03,
>> +       FW_CFG_NOGRAPHIC        = 0x04,
>> +       FW_CFG_NB_CPUS          = 0x05,
>> +       FW_CFG_MACHINE_ID       = 0x06,
>> +       FW_CFG_KERNEL_ADDR      = 0x07,
>> +       FW_CFG_KERNEL_SIZE      = 0x08,
>> +       FW_CFG_KERNEL_CMDLINE   = 0x09,
>> +       FW_CFG_INITRD_ADDR      = 0x0a,
>> +       FW_CFG_INITRD_SIZE      = 0x0b,
>> +       FW_CFG_BOOT_DEVICE      = 0x0c,
>> +       FW_CFG_NUMA             = 0x0d,
>> +       FW_CFG_BOOT_MENU        = 0x0e,
>> +       FW_CFG_MAX_CPUS         = 0x0f,
>> +       FW_CFG_KERNEL_ENTRY     = 0x10,
>> +       FW_CFG_KERNEL_DATA      = 0x11,
>> +       FW_CFG_INITRD_DATA      = 0x12,
>> +       FW_CFG_CMDLINE_ADDR     = 0x13,
>> +       FW_CFG_CMDLINE_SIZE     = 0x14,
>> +       FW_CFG_CMDLINE_DATA     = 0x15,
>> +       FW_CFG_SETUP_ADDR       = 0x16,
>> +       FW_CFG_SETUP_SIZE       = 0x17,
>> +       FW_CFG_SETUP_DATA       = 0x18,
>> +       FW_CFG_FILE_DIR         = 0x19,
>> +       FW_CFG_FILE_FIRST       = 0x20,
>> +       FW_CFG_WRITE_CHANNEL    = 0x4000,
>> +       FW_CFG_ARCH_LOCAL       = 0x8000,
>> +       FW_CFG_INVALID          = 0xffff,
>> +};
>> +
>> +#define FW_CFG_FILE_SLOTS      0x10
>> +#define FW_CFG_MAX_ENTRY       (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
>> +#define FW_CFG_ENTRY_MASK       ~(FW_CFG_WRITE_CHANNEL | FW_CFG_ARCH_LOCAL)
>> +
>> +#define FW_CFG_MAX_FILE_PATH   56
>> +
>> +#define QEMU_FW_CFG_SIGNATURE  (('Q' << 24) | ('E' << 16) | ('M' << 8) | 'U')
>> +
>> +#define FW_CFG_DMA_ERROR       (1 << 0)
>> +#define FW_CFG_DMA_READ        (1 << 1)
>> +#define FW_CFG_DMA_SKIP        (1 << 2)
>> +#define FW_CFG_DMA_SELECT      (1 << 3)
>> +
>> +#define FW_CFG_DMA_ENABLED     (1 << 1)
>> +
>> +struct fw_cfg_file {
>> +       __be32 size;
>> +       __be16 select;
>> +       __be16 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;
>> +};
>> +
>> +/**
>> + * Initialize QEMU fw_cfg interface
>> + *
>> + * @return:   None
>
> You can drop this line
>
>> + */
>> +
>> +void qemu_fwcfg_init(void);
>> +
>> +/**
>> + * Get system cpu number
>> + *
>> + * @return:   cpu number in system
>> + */
>> +
>> +int qemu_fwcfg_online_cpus(void);
>> +
>> +#endif
>> diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
>> index 1f93f72..d9ae066 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)
>> --
>> 1.9.1
>>
>
> Regards,
> Simon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 5/8] x86: qemu: add qemu_fwcfg_fdt_fixup()
  2015-12-31  5:08   ` Simon Glass
@ 2015-12-31  9:02     ` Miao Yan
  2015-12-31 12:22       ` Simon Glass
  2016-01-04  3:42       ` Bin Meng
  0 siblings, 2 replies; 28+ messages in thread
From: Miao Yan @ 2015-12-31  9:02 UTC (permalink / raw)
  To: u-boot

Hi Simon,

2015-12-31 13:08 GMT+08:00 Simon Glass <sjg@chromium.org>:
> Hi Maio,
>
> On 30 December 2015 at 19:55, Miao Yan <yanmiaobest@gmail.com> wrote:
>> Add a function to fix up 'cpus' node in dts files for qemu target.
>>
>> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
>> ---
>> Changes in v4:
>>   - fix a typo in commit log
>>
>>  arch/x86/cpu/qemu/fw_cfg.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
>>  arch/x86/cpu/qemu/fw_cfg.h | 11 ++++++++
>>  2 files changed, 76 insertions(+)
>
> I'm sorry for not reviewing this earlier (Christmas and all that). I
> don't think you need to update the device tree to make this work.
>
> Here's my suggestion:


I am not familiar with driver model so I am not sure if I understand
 you correctly. Are you suggesting something like the following:

+
+void create_cpu_node(void)
+{
+       int ret;
+       int cpu_online;
+       int cpu_num = 0;
+       struct udevice *dev;
+       struct cpu_platdata *plat;
+
+       for (uclass_find_first_device(UCLASS_CPU, &dev);
+            dev;
+            uclass_find_next_device(&dev)) {
+               cpu_num++;
+       }
+
+       cpu_online = qemu_fwcfg_online_cpus();
+       printf("%d cpus probed, %d cpus online\n", cpu_num, cpu_online);
+
+       for (dev = NULL; cpu_num < cpu_online; cpu_num++) {
+               ret = device_bind_driver(cpus_dev, "cpu_qemu", "cpu", &dev);
+               if (ret < 0) {
+                       printf("device_bind_driver failed with code %d\n", ret);
+                       return;
+               }
+               plat = dev_get_parent_platdata(dev);
+               plat->cpu_id = cpu_num;
+       }
+}



>
> - At present cpu_x86_bind() sets up the CPU APIC ID from the device tree
> - You can bind new CPU devices in your code on start-up
> - You can check if you have CPUs which are not available in the device
> list, by using uclass_find_first/next_device() to iterate through the
> devices without probing them
> - Then to create a new one, call device_bind_driver() with the /cpus
> node as the parent

The 'cpus' node is created in uclass_cpu_init(), and all the
'cpu' subnode are created by scanning device tree. But arch_early_init_r()
is called before uclass_cpu_init(), so at that time, there's no
'cpus' yet.

Seems we need somewhere after uclass_cpu_init() but before mp_init() ?
Or we entirely bypass the cpu uclass driver and create /cpus too ?


> - After binding, update the parent platform data:
>
>   struct cpu_platdata *plat = dev_get_parent_platdata(dev);
>
>    plat->cpu_id = ...
>
> - Then when it comes to probing, you will have all the devices you
> need, and you don't need to adjust the device tree. The device tree
> can just hold a single device, for example.
>
> I think it is better to do this than adjust the device tree because it
> removes the 32-CPU limit and should be faster. It is also simpler as
> it is a more direct method. Also I believe you only need to do this
> after relocation - e.g. in arch_early_init_r(), which is before
> mp_init() is called from cpu_init_r().

Seems I can't do it in arch_early_init_r(), when 'cpus' is available yet.

>
> I wonder if there is a general way to probe available CPUs (and their
> APIC IDs)? Or is qemu the only 'CPU' with this feature?

I am not sure about other x86 boards, but certainly fw_cfg is
not the generic way.  Maybe Bin can comment on this.

>
> Regards,
> Simon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 1/8] x86: qemu: add fw_cfg support
  2015-12-31  8:42     ` Miao Yan
@ 2015-12-31 12:22       ` Simon Glass
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Glass @ 2015-12-31 12:22 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On 31 December 2015 at 01:42, Miao Yan <yanmiaobest@gmail.com> wrote:
> Hi Simon,
>
> 2015-12-31 13:07 GMT+08:00 Simon Glass <sjg@chromium.org>:
>> Hi Miao,
>>
>> On 30 December 2015 at 19:55, 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>
>>> ---
>>> Changes in v4:
>>>   - cleanups
>>>   - change 'fw load' to take second parameter for initrd load address
>>>
>>>  arch/x86/cpu/qemu/Makefile |   2 +-
>>>  arch/x86/cpu/qemu/fw_cfg.c | 268 +++++++++++++++++++++++++++++++++++++++++++++
>>>  arch/x86/cpu/qemu/fw_cfg.h |  97 ++++++++++++++++
>>>  arch/x86/cpu/qemu/qemu.c   |   3 +
>>>  4 files changed, 369 insertions(+), 1 deletion(-)
>>>  create mode 100644 arch/x86/cpu/qemu/fw_cfg.c
>>>  create mode 100644 arch/x86/cpu/qemu/fw_cfg.h
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> But a few nits...
[snip]
>>> +int qemu_fwcfg_online_cpus(void)
>>> +{
>>> +       uint16_t nb_cpus;
>>> +
>>> +       if (!fwcfg_present)
>>> +               return 1;
>>
>> -ENODEV
>
>
> Can we return 1 cpu if fw_cfg interface is not avaliable (which is
> quite unlikey),
> and print a warning maybe ? Because there has to be one cpu at least
> and returning
> -ENODEV wouldn't make much difference.

I see. In that case I think it is better if qemu_fwcfg_online_cpus()
returns an error and its caller can detect -ENODEV and use '1' as the
value. Then the decision as to how to deal with the error is handled
at the layer that is taking action.

>
> I'll fix rest of your comments and thanks for the review.
>
> Miao
>
[snip]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 5/8] x86: qemu: add qemu_fwcfg_fdt_fixup()
  2015-12-31  9:02     ` Miao Yan
@ 2015-12-31 12:22       ` Simon Glass
  2016-01-04  3:42       ` Bin Meng
  1 sibling, 0 replies; 28+ messages in thread
From: Simon Glass @ 2015-12-31 12:22 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On 31 December 2015 at 02:02, Miao Yan <yanmiaobest@gmail.com> wrote:
> Hi Simon,
>
> 2015-12-31 13:08 GMT+08:00 Simon Glass <sjg@chromium.org>:
>> Hi Maio,
>>
>> On 30 December 2015 at 19:55, Miao Yan <yanmiaobest@gmail.com> wrote:
>>> Add a function to fix up 'cpus' node in dts files for qemu target.
>>>
>>> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
>>> ---
>>> Changes in v4:
>>>   - fix a typo in commit log
>>>
>>>  arch/x86/cpu/qemu/fw_cfg.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
>>>  arch/x86/cpu/qemu/fw_cfg.h | 11 ++++++++
>>>  2 files changed, 76 insertions(+)
>>
>> I'm sorry for not reviewing this earlier (Christmas and all that). I
>> don't think you need to update the device tree to make this work.
>>
>> Here's my suggestion:
>
>
> I am not familiar with driver model so I am not sure if I understand
>  you correctly. Are you suggesting something like the following:
>
> +
> +void create_cpu_node(void)
> +{
> +       int ret;
> +       int cpu_online;
> +       int cpu_num = 0;
> +       struct udevice *dev;
> +       struct cpu_platdata *plat;
> +
> +       for (uclass_find_first_device(UCLASS_CPU, &dev);
> +            dev;
> +            uclass_find_next_device(&dev)) {
> +               cpu_num++;
> +       }
> +
> +       cpu_online = qemu_fwcfg_online_cpus();
> +       printf("%d cpus probed, %d cpus online\n", cpu_num, cpu_online);
> +
> +       for (dev = NULL; cpu_num < cpu_online; cpu_num++) {
> +               ret = device_bind_driver(cpus_dev, "cpu_qemu", "cpu", &dev);

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

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

return ret

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

Yes that looks right.

>
>
>
>>
>> - At present cpu_x86_bind() sets up the CPU APIC ID from the device tree
>> - You can bind new CPU devices in your code on start-up
>> - You can check if you have CPUs which are not available in the device
>> list, by using uclass_find_first/next_device() to iterate through the
>> devices without probing them
>> - Then to create a new one, call device_bind_driver() with the /cpus
>> node as the parent
>
> The 'cpus' node is created in uclass_cpu_init(), and all the
> 'cpu' subnode are created by scanning device tree. But arch_early_init_r()
> is called before uclass_cpu_init(), so at that time, there's no
> 'cpus' yet.
>
> Seems we need somewhere after uclass_cpu_init() but before mp_init() ?
> Or we entirely bypass the cpu uclass driver and create /cpus too ?

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

>
>
>> - After binding, update the parent platform data:
>>
>>   struct cpu_platdata *plat = dev_get_parent_platdata(dev);
>>
>>    plat->cpu_id = ...
>>
>> - Then when it comes to probing, you will have all the devices you
>> need, and you don't need to adjust the device tree. The device tree
>> can just hold a single device, for example.
>>
>> I think it is better to do this than adjust the device tree because it
>> removes the 32-CPU limit and should be faster. It is also simpler as
>> it is a more direct method. Also I believe you only need to do this
>> after relocation - e.g. in arch_early_init_r(), which is before
>> mp_init() is called from cpu_init_r().
>
> Seems I can't do it in arch_early_init_r(), when 'cpus' is available yet.
>
>>
>> I wonder if there is a general way to probe available CPUs (and their
>> APIC IDs)? Or is qemu the only 'CPU' with this feature?
>
> I am not sure about other x86 boards, but certainly fw_cfg is
> not the generic way.  Maybe Bin can comment on this.

Regards,
Simon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 5/8] x86: qemu: add qemu_fwcfg_fdt_fixup()
  2015-12-31  9:02     ` Miao Yan
  2015-12-31 12:22       ` Simon Glass
@ 2016-01-04  3:42       ` Bin Meng
  1 sibling, 0 replies; 28+ messages in thread
From: Bin Meng @ 2016-01-04  3:42 UTC (permalink / raw)
  To: u-boot

On Thu, Dec 31, 2015 at 5:02 PM, Miao Yan <yanmiaobest@gmail.com> wrote:
> Hi Simon,
>
> 2015-12-31 13:08 GMT+08:00 Simon Glass <sjg@chromium.org>:
>> Hi Maio,
>>
>> On 30 December 2015 at 19:55, Miao Yan <yanmiaobest@gmail.com> wrote:
>>> Add a function to fix up 'cpus' node in dts files for qemu target.
>>>
>>> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
>>> ---
>>> Changes in v4:
>>>   - fix a typo in commit log
>>>
>>>  arch/x86/cpu/qemu/fw_cfg.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
>>>  arch/x86/cpu/qemu/fw_cfg.h | 11 ++++++++
>>>  2 files changed, 76 insertions(+)
>>
>> I'm sorry for not reviewing this earlier (Christmas and all that). I
>> don't think you need to update the device tree to make this work.
>>
>> Here's my suggestion:
>
>
> I am not familiar with driver model so I am not sure if I understand
>  you correctly. Are you suggesting something like the following:
>
> +
> +void create_cpu_node(void)
> +{
> +       int ret;
> +       int cpu_online;
> +       int cpu_num = 0;
> +       struct udevice *dev;
> +       struct cpu_platdata *plat;
> +
> +       for (uclass_find_first_device(UCLASS_CPU, &dev);
> +            dev;
> +            uclass_find_next_device(&dev)) {
> +               cpu_num++;
> +       }
> +
> +       cpu_online = qemu_fwcfg_online_cpus();
> +       printf("%d cpus probed, %d cpus online\n", cpu_num, cpu_online);
> +
> +       for (dev = NULL; cpu_num < cpu_online; cpu_num++) {
> +               ret = device_bind_driver(cpus_dev, "cpu_qemu", "cpu", &dev);
> +               if (ret < 0) {
> +                       printf("device_bind_driver failed with code %d\n", ret);
> +                       return;
> +               }
> +               plat = dev_get_parent_platdata(dev);
> +               plat->cpu_id = cpu_num;
> +       }
> +}
>
>
>
>>
>> - At present cpu_x86_bind() sets up the CPU APIC ID from the device tree
>> - You can bind new CPU devices in your code on start-up
>> - You can check if you have CPUs which are not available in the device
>> list, by using uclass_find_first/next_device() to iterate through the
>> devices without probing them
>> - Then to create a new one, call device_bind_driver() with the /cpus
>> node as the parent
>
> The 'cpus' node is created in uclass_cpu_init(), and all the
> 'cpu' subnode are created by scanning device tree. But arch_early_init_r()
> is called before uclass_cpu_init(), so at that time, there's no
> 'cpus' yet.
>
> Seems we need somewhere after uclass_cpu_init() but before mp_init() ?
> Or we entirely bypass the cpu uclass driver and create /cpus too ?
>
>
>> - After binding, update the parent platform data:
>>
>>   struct cpu_platdata *plat = dev_get_parent_platdata(dev);
>>
>>    plat->cpu_id = ...
>>
>> - Then when it comes to probing, you will have all the devices you
>> need, and you don't need to adjust the device tree. The device tree
>> can just hold a single device, for example.
>>
>> I think it is better to do this than adjust the device tree because it
>> removes the 32-CPU limit and should be faster. It is also simpler as
>> it is a more direct method. Also I believe you only need to do this
>> after relocation - e.g. in arch_early_init_r(), which is before
>> mp_init() is called from cpu_init_r().
>
> Seems I can't do it in arch_early_init_r(), when 'cpus' is available yet.
>
>>
>> I wonder if there is a general way to probe available CPUs (and their
>> APIC IDs)? Or is qemu the only 'CPU' with this feature?
>
> I am not sure about other x86 boards, but certainly fw_cfg is
> not the generic way.  Maybe Bin can comment on this.
>

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

Regards,
Bin

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 1/8] x86: qemu: add fw_cfg support
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 1/8] x86: qemu: add fw_cfg support Miao Yan
  2015-12-31  5:07   ` Simon Glass
@ 2016-01-04  4:05   ` Bin Meng
  1 sibling, 0 replies; 28+ messages in thread
From: Bin Meng @ 2016-01-04  4:05 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On Thu, Dec 31, 2015 at 10:55 AM, 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>
> ---
> Changes in v4:
>   - cleanups
>   - change 'fw load' to take second parameter for initrd load address
>
>  arch/x86/cpu/qemu/Makefile |   2 +-
>  arch/x86/cpu/qemu/fw_cfg.c | 268 +++++++++++++++++++++++++++++++++++++++++++++
>  arch/x86/cpu/qemu/fw_cfg.h |  97 ++++++++++++++++
>  arch/x86/cpu/qemu/qemu.c   |   3 +
>  4 files changed, 369 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..9de8680
> --- /dev/null
> +++ b/arch/x86/cpu/qemu/fw_cfg.c
> @@ -0,0 +1,268 @@
> +/*
> + * (C) Copyright 2015 Miao Yan <yanmiaoebst@gmail.com>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <errno.h>
> +#include <malloc.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;
> +
> +       /*
> +        * writting FW_CFG_INVALID will cause
> +        * read operation to resume at
> +        * last offset, otherwise read will
> +        * start at offset 0
> +        */
> +
> +       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 size, void *address)
> +{
> +       struct fw_cfg_dma_access dma;
> +
> +       dma.length = cpu_to_be32(size);
> +       dma.address = cpu_to_be64((uintptr_t)address);
> +       dma.control = cpu_to_be32(FW_CFG_DMA_READ);
> +
> +       /*
> +        * writting FW_CFG_INVALID will cause
> +        * read operation to resume at
> +        * last offset, otherwise read will
> +        * start at offset 0
> +        */
> +
> +       if (entry != FW_CFG_INVALID)
> +               dma.control |= cpu_to_be32(FW_CFG_DMA_SELECT | (entry << 16));
> +
> +       barrier();
> +
> +       debug("qemu_fwcfg_dma_read_entry: addr %p, length %u control 0x%x\n",
> +             address, size, be32_to_cpu(dma.control));
> +
> +       outl(cpu_to_be32((uint32_t)&dma), FW_DMA_PORT_HIGH);
> +
> +       while (be32_to_cpu(dma.control) & ~FW_CFG_DMA_ERROR)
> +               __asm__ __volatile__ ("pause");
> +}
> +
> +static bool 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 bool qemu_fwcfg_dma_present(void)
> +{
> +       uint8_t dma_enabled;
> +
> +       qemu_fwcfg_read_entry_pio(FW_CFG_ID, 1, &dma_enabled);
> +       if (dma_enabled & FW_CFG_DMA_ENABLED)
> +               return true;
> +
> +       return false;
> +}
> +
> +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);
> +}
> +
> +int 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 le16_to_cpu(nb_cpus);
> +}
> +
> +static int qemu_fwcfg_setup_kernel(void *load_addr, void *initrd_addr)
> +{
> +       char *data_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;
> +       }
> +
> +       data_addr = load_addr;
> +       qemu_fwcfg_read_entry(FW_CFG_SETUP_DATA,
> +                             le32_to_cpu(setup_size), data_addr);
> +       data_addr += le32_to_cpu(setup_size);
> +
> +       qemu_fwcfg_read_entry(FW_CFG_KERNEL_DATA,
> +                             le32_to_cpu(kernel_size), data_addr);
> +       data_addr += le32_to_cpu(kernel_size);
> +
> +       data_addr = initrd_addr;
> +       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,
> +                                     le32_to_cpu(initrd_size), data_addr);
> +               data_addr += le32_to_cpu(initrd_size);
> +       }
> +
> +       qemu_fwcfg_read_entry(FW_CFG_CMDLINE_SIZE, 4, &cmdline_size);
> +       if (cmdline_size) {
> +               qemu_fwcfg_read_entry(FW_CFG_CMDLINE_DATA,
> +                                     le32_to_cpu(cmdline_size), data_addr);
> +               if (setenv("bootargs", data_addr) < 0)
> +                       printf("warning: unable to change bootargs\n");
> +       }
> +
> +       printf("loading kernel to address %p", load_addr);
> +       if (initrd_size)
> +               printf(" initrd %p, size 0x%x\n",
> +                      initrd_addr,
> +                      le32_to_cpu(initrd_size));
> +       else
> +               printf("\n");
> +
> +       return 0;
> +}
> +
> +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 = qemu_fwcfg_present();
> +       if (fwcfg_present)
> +               fwcfg_dma_present = qemu_fwcfg_dma_present();
> +}
> +
> +static int qemu_fwcfg_do_list(cmd_tbl_t *cmdtp, int flag,
> +               int argc, char * const argv[])
> +{
> +       qemu_fwcfg_list_firmware();
> +
> +       return 0;
> +}
> +
> +static int qemu_fwcfg_do_cpus(cmd_tbl_t *cmdtp, int flag,
> +               int argc, char * const argv[])
> +{
> +       printf("%d cpu(s) online\n", qemu_fwcfg_online_cpus());
> +
> +       return 0;
> +}
> +
> +static int qemu_fwcfg_do_load(cmd_tbl_t *cmdtp, int flag,
> +               int argc, char * const argv[])
> +{
> +       char *env;
> +       void *load_addr;
> +       void *initrd_addr;
> +
> +       env = getenv("loadaddr");
> +       load_addr = env ?
> +               (void *)simple_strtoul(env, NULL, 16) :
> +               (void *)CONFIG_LOADADDR;
> +
> +       env = getenv("ramdiskaddr");
> +       initrd_addr = env ?
> +               (void *)simple_strtoul(env, NULL, 16) :
> +               (void *)CONFIG_RAMDISK_ADDR;

This does not build as CONFIG_RAMDISK_ADDR is introduced in a later
patch. Please reorder the patches.

> +
> +       if (argc == 2) {
> +               load_addr = (void *)simple_strtoul(argv[0], NULL, 16);
> +               initrd_addr = (void *)simple_strtoul(argv[1], NULL, 16);
> +       } else if (argc == 1) {
> +               load_addr = (void *)simple_strtoul(argv[0], NULL, 16);
> +       }
> +
> +       return qemu_fwcfg_setup_kernel(load_addr, initrd_addr);
> +}
> +
> +static cmd_tbl_t fwcfg_commands[] = {
> +       U_BOOT_CMD_MKENT(list, 0, 1, qemu_fwcfg_do_list, "", ""),
> +       U_BOOT_CMD_MKENT(cpus, 0, 1, qemu_fwcfg_do_cpus, "", ""),
> +       U_BOOT_CMD_MKENT(load, 2, 1, qemu_fwcfg_do_load, "", ""),
> +};
> +
> +static int do_qemu_fw(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +       int ret;
> +       cmd_tbl_t *fwcfg_cmd;
> +
> +       if (!fwcfg_present) {
> +               printf("QEMU fw_cfg interface not found\n");
> +               return CMD_RET_USAGE;
> +       }
> +
> +       fwcfg_cmd = find_cmd_tbl(argv[1], fwcfg_commands,
> +                                ARRAY_SIZE(fwcfg_commands));
> +       argc -= 2;
> +       argv += 2;
> +       if (!fwcfg_cmd || argc > fwcfg_cmd->maxargs)
> +               return CMD_RET_USAGE;
> +
> +       ret = fwcfg_cmd->cmd(fwcfg_cmd, flag, argc, argv);
> +
> +       return cmd_process_error(fwcfg_cmd, ret);
> +}
> +
> +U_BOOT_CMD(
> +       fw,     4,      1,      do_qemu_fw,
> +       "QEMU firmware interface",
> +       "<command>\n"
> +       "    - list                             : print firmware(s) currently loaded\n"
> +       "    - cpus                             : print online cpu number\n"
> +       "    - load <kernel addr> <initrd addr> : load kernel and initrd (if any), 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..03ac27d
> --- /dev/null
> +++ b/arch/x86/cpu/qemu/fw_cfg.h
> @@ -0,0 +1,97 @@
> +/*
> + * (C) Copyright 2015 Miao Yan <yanmiaobest@gmail.com>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef __FW_CFG__
> +#define __FW_CFG__
> +
> +#define FW_CONTROL_PORT        0x510
> +#define FW_DATA_PORT           0x511
> +#define FW_DMA_PORT_LOW        0x514
> +#define FW_DMA_PORT_HIGH       0x518
> +
> +enum qemu_fwcfg_items {
> +       FW_CFG_SIGNATURE        = 0x00,
> +       FW_CFG_ID               = 0x01,
> +       FW_CFG_UUID             = 0x02,
> +       FW_CFG_RAM_SIZE         = 0x03,
> +       FW_CFG_NOGRAPHIC        = 0x04,
> +       FW_CFG_NB_CPUS          = 0x05,
> +       FW_CFG_MACHINE_ID       = 0x06,
> +       FW_CFG_KERNEL_ADDR      = 0x07,
> +       FW_CFG_KERNEL_SIZE      = 0x08,
> +       FW_CFG_KERNEL_CMDLINE   = 0x09,
> +       FW_CFG_INITRD_ADDR      = 0x0a,
> +       FW_CFG_INITRD_SIZE      = 0x0b,
> +       FW_CFG_BOOT_DEVICE      = 0x0c,
> +       FW_CFG_NUMA             = 0x0d,
> +       FW_CFG_BOOT_MENU        = 0x0e,
> +       FW_CFG_MAX_CPUS         = 0x0f,
> +       FW_CFG_KERNEL_ENTRY     = 0x10,
> +       FW_CFG_KERNEL_DATA      = 0x11,
> +       FW_CFG_INITRD_DATA      = 0x12,
> +       FW_CFG_CMDLINE_ADDR     = 0x13,
> +       FW_CFG_CMDLINE_SIZE     = 0x14,
> +       FW_CFG_CMDLINE_DATA     = 0x15,
> +       FW_CFG_SETUP_ADDR       = 0x16,
> +       FW_CFG_SETUP_SIZE       = 0x17,
> +       FW_CFG_SETUP_DATA       = 0x18,
> +       FW_CFG_FILE_DIR         = 0x19,
> +       FW_CFG_FILE_FIRST       = 0x20,
> +       FW_CFG_WRITE_CHANNEL    = 0x4000,
> +       FW_CFG_ARCH_LOCAL       = 0x8000,
> +       FW_CFG_INVALID          = 0xffff,
> +};
> +
> +#define FW_CFG_FILE_SLOTS      0x10
> +#define FW_CFG_MAX_ENTRY       (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)

Sorry, one more nits: please add space around '+'

> +#define FW_CFG_ENTRY_MASK       ~(FW_CFG_WRITE_CHANNEL | FW_CFG_ARCH_LOCAL)
> +
> +#define FW_CFG_MAX_FILE_PATH   56
> +
> +#define QEMU_FW_CFG_SIGNATURE  (('Q' << 24) | ('E' << 16) | ('M' << 8) | 'U')
> +
> +#define FW_CFG_DMA_ERROR       (1 << 0)
> +#define FW_CFG_DMA_READ        (1 << 1)
> +#define FW_CFG_DMA_SKIP        (1 << 2)
> +#define FW_CFG_DMA_SELECT      (1 << 3)
> +
> +#define FW_CFG_DMA_ENABLED     (1 << 1)
> +
> +struct fw_cfg_file {
> +       __be32 size;
> +       __be16 select;
> +       __be16 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;
> +};
> +
> +/**
> + * Initialize QEMU fw_cfg interface
> + *
> + * @return:   None
> + */
> +
> +void qemu_fwcfg_init(void);
> +
> +/**
> + * Get system cpu number
> + *
> + * @return:   cpu number in system
> + */
> +
> +int qemu_fwcfg_online_cpus(void);
> +
> +#endif
> diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
> index 1f93f72..d9ae066 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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 2/8] x86: qemu: add a cpu uclass driver for qemu target
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 2/8] x86: qemu: add a cpu uclass driver for qemu target Miao Yan
  2015-12-31  5:07   ` Simon Glass
@ 2016-01-04  4:05   ` Bin Meng
  1 sibling, 0 replies; 28+ messages in thread
From: Bin Meng @ 2016-01-04  4:05 UTC (permalink / raw)
  To: u-boot

On Thu, Dec 31, 2015 at 10:55 AM, 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          | 57 ++++++++++++++++++++++++++++++++++++++++
>  arch/x86/dts/qemu-x86_i440fx.dts |  4 +--
>  arch/x86/dts/qemu-x86_q35.dts    |  4 +--
>  4 files changed, 62 insertions(+), 5 deletions(-)
>  create mode 100644 arch/x86/cpu/qemu/cpu.c
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 3/8] x86: fix a typo in function name
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 3/8] x86: fix a typo in function name Miao Yan
  2015-12-31  5:08   ` Simon Glass
@ 2016-01-04  4:05   ` Bin Meng
  1 sibling, 0 replies; 28+ messages in thread
From: Bin Meng @ 2016-01-04  4:05 UTC (permalink / raw)
  To: u-boot

On Thu, Dec 31, 2015 at 10:55 AM, 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>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 4/8] x86: use actual CPU number for allocating memory
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 4/8] x86: use actual CPU number for allocating memory Miao Yan
  2015-12-31  5:08   ` Simon Glass
@ 2016-01-04  4:05   ` Bin Meng
  1 sibling, 0 replies; 28+ messages in thread
From: Bin Meng @ 2016-01-04  4:05 UTC (permalink / raw)
  To: u-boot

On Thu, Dec 31, 2015 at 10:55 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
> 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(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 7/8] x86: qemu: adjust ramdisk load address
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 7/8] x86: qemu: adjust ramdisk load address Miao Yan
  2015-12-31  5:08   ` Simon Glass
@ 2016-01-04  4:05   ` Bin Meng
  1 sibling, 0 replies; 28+ messages in thread
From: Bin Meng @ 2016-01-04  4:05 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On Thu, Dec 31, 2015 at 10:55 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
> By default, ramdisk load address is defined to 02000000 in
> env string. When loading bzImage to 100000 (default address), there's

The default load address is 1000000 (16MB). See CONFIG_LOADADDR in x86-common.h

> a chance that the ramdisk header would be overwritten by
> the kernel. Thus increase the gap and make ramdisk load at 04000000
> by default.
>
> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
> ---
>  include/configs/qemu-x86.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/include/configs/qemu-x86.h b/include/configs/qemu-x86.h
> index 4258dcb..657b8af 100644
> --- a/include/configs/qemu-x86.h
> +++ b/include/configs/qemu-x86.h
> @@ -57,4 +57,17 @@
>  #undef CONFIG_ENV_IS_IN_SPI_FLASH
>  #define CONFIG_ENV_IS_NOWHERE
>
> +/* default ramdisk load address */
> +#define CONFIG_RAMDISK_ADDR    0x04000000
> +
> +#undef CONFIG_EXTRA_ENV_SETTINGS
> +#define CONFIG_EXTRA_ENV_SETTINGS                      \
> +       CONFIG_STD_DEVICES_SETTINGS                     \
> +       "pciconfighost=1\0"                             \
> +       "netdev=eth0\0"                                 \
> +       "consoledev=ttyS0\0"                            \
> +       "othbootargs=acpi=off\0"                        \
> +       "ramdiskaddr=0x4000000\0"                       \
> +       "ramdiskfile=initramfs.gz\0"
> +

Please move the changes to x86-common.h. This can be generic for all
x86 targets.

>  #endif /* __CONFIG_H */
> --

Regards,
Bin

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 8/8] x86: qemu: add documentaion for the fw_cfg interface
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 8/8] x86: qemu: add documentaion for the fw_cfg interface Miao Yan
  2015-12-31  5:08   ` Simon Glass
@ 2016-01-04  4:06   ` Bin Meng
  1 sibling, 0 replies; 28+ messages in thread
From: Bin Meng @ 2016-01-04  4:06 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On Thu, Dec 31, 2015 at 10:55 AM, Miao Yan <yanmiaobest@gmail.com> wrote:
> Document the usage of 'fw' command
>
> Signed-off-by: Miao Yan <yanmiaobest@gmail.com>
> ---
> Changes in v4:
>   - limit maximum supported cpu number to 32
>
>  doc/README.x86 | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/doc/README.x86 b/doc/README.x86
> index 1271e5e..aa3010d 100644
> --- a/doc/README.x86
> +++ b/doc/README.x86
> @@ -295,9 +295,38 @@ 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. Note, due space limitations in dtb, the maximum supported CPU
> +number is 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

Please move the ',' to the end of the line above

> +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-i386 -nographic -bios path/to/u-boot.rom -m 1024 -kernel /path/to/bzImage
> +    -append 'root=/dev/ram 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 <kernel addr> <initrd addr> : load kernel and initrd (if any) and setup for zboot
> +
> +=> fw load
> +loading kernel to address 02000000 initrd 04000000, size 0x1b1ab50

This 'size' looks confusing as it is unclear which size is this for.
So we may need enhance 'fw load' to print kernel size as well,
something like:

loading kernel to address 02000000 size xxxxxxxx, initrd 04000000 size
xxxxxxxx. Note you can omit 0x in the size to keep consitency.

> +
> +Here the kernel (bzImage) is loaded to 02000000 and initrd is to 0x04000000. Then, 'zboot'
> +can be used to boot the kernel:
> +
> +=> zboot 02000000 - 04000000 1b1ab50
>
>  CPU Microcode
>  -------------
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH v4 0/8] x86: qemu: add fw_cfg interface support for qemu-x86 targets
  2015-12-31  2:55 [U-Boot] [PATCH v4 0/8] x86: qemu: add fw_cfg interface support for qemu-x86 targets Miao Yan
                   ` (7 preceding siblings ...)
  2015-12-31  2:55 ` [U-Boot] [PATCH v4 8/8] x86: qemu: add documentaion for the fw_cfg interface Miao Yan
@ 2016-01-04  4:08 ` Bin Meng
  8 siblings, 0 replies; 28+ messages in thread
From: Bin Meng @ 2016-01-04  4:08 UTC (permalink / raw)
  To: u-boot

Hi Miao,

On Thu, Dec 31, 2015 at 10:55 AM, 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.
>
> Changes in v4:
>   - drop [PATCH v3 7/8] and limit maximum supported cpu number
>   - chance 'fw load' to take second parameter for initrd load address
>   - change default initrd load address for qemu-x86
>
> Miao Yan (8):
>   x86: qemu: add fw_cfg support
>   x86: qemu: add a cpu uclass driver for qemu target
>   x86: fix a typo in function name
>   x86: use actual CPU number for allocating memory
>   x86: qemu: add qemu_fwcfg_fdt_fixup()
>   x86: qemu: fix up cpu node in device tree
>   x86: qemu: adjust ramdisk load address
>   x86: qemu: 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          |  57 +++++++
>  arch/x86/cpu/qemu/fw_cfg.c       | 333 +++++++++++++++++++++++++++++++++++++++
>  arch/x86/cpu/qemu/fw_cfg.h       | 108 +++++++++++++
>  arch/x86/cpu/qemu/qemu.c         |   7 +
>  arch/x86/dts/qemu-x86_i440fx.dts |  19 +--
>  arch/x86/dts/qemu-x86_q35.dts    |  19 +--
>  doc/README.x86                   |  35 +++-
>  include/configs/qemu-x86.h       |  13 ++
>  10 files changed, 559 insertions(+), 46 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
>
> --

Please add those git tags (eg: Reviewed-by, Tested-By, Acked-by) when
you rework your patch series. This helps future reviews of the same
patch series.

Regards,
Bin

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2016-01-04  4:08 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-31  2:55 [U-Boot] [PATCH v4 0/8] x86: qemu: add fw_cfg interface support for qemu-x86 targets Miao Yan
2015-12-31  2:55 ` [U-Boot] [PATCH v4 1/8] x86: qemu: add fw_cfg support Miao Yan
2015-12-31  5:07   ` Simon Glass
2015-12-31  8:42     ` Miao Yan
2015-12-31 12:22       ` Simon Glass
2016-01-04  4:05   ` Bin Meng
2015-12-31  2:55 ` [U-Boot] [PATCH v4 2/8] x86: qemu: add a cpu uclass driver for qemu target Miao Yan
2015-12-31  5:07   ` Simon Glass
2016-01-04  4:05   ` Bin Meng
2015-12-31  2:55 ` [U-Boot] [PATCH v4 3/8] x86: fix a typo in function name Miao Yan
2015-12-31  5:08   ` Simon Glass
2016-01-04  4:05   ` Bin Meng
2015-12-31  2:55 ` [U-Boot] [PATCH v4 4/8] x86: use actual CPU number for allocating memory Miao Yan
2015-12-31  5:08   ` Simon Glass
2016-01-04  4:05   ` Bin Meng
2015-12-31  2:55 ` [U-Boot] [PATCH v4 5/8] x86: qemu: add qemu_fwcfg_fdt_fixup() Miao Yan
2015-12-31  5:08   ` Simon Glass
2015-12-31  9:02     ` Miao Yan
2015-12-31 12:22       ` Simon Glass
2016-01-04  3:42       ` Bin Meng
2015-12-31  2:55 ` [U-Boot] [PATCH v4 6/8] x86: qemu: fix up cpu node in device tree Miao Yan
2015-12-31  2:55 ` [U-Boot] [PATCH v4 7/8] x86: qemu: adjust ramdisk load address Miao Yan
2015-12-31  5:08   ` Simon Glass
2016-01-04  4:05   ` Bin Meng
2015-12-31  2:55 ` [U-Boot] [PATCH v4 8/8] x86: qemu: add documentaion for the fw_cfg interface Miao Yan
2015-12-31  5:08   ` Simon Glass
2016-01-04  4:06   ` Bin Meng
2016-01-04  4:08 ` [U-Boot] [PATCH v4 0/8] x86: qemu: add fw_cfg interface support for qemu-x86 targets Bin Meng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.