[PATCH v3 1/4] arm: x86: qemu: move qfw to DM, include Arm support

Heinrich Schuchardt xypron.glpk at gmx.de
Tue Feb 23 13:59:52 CET 2021


On 23.02.21 12:43, Asherah Connor wrote:
> Updates the QFW driver to use the driver model, and adds support for QFW
> on Arm platforms by configuring from the device tree and using MMIO
> accordingly.  A sandbox driver for QFW is also included, and a simple DM
> unit test for it.

For which architectures does the fw_cfg device exist?

It it is only ARM and X86, than I am missing such a dependency on
CONFIG_CMD_QFW.

>
> Signed-off-by: Asherah Connor <ashe at kivikakk.ee>
> ---
>
> Changes in v3:
> - ARCH_QEMU now implies CMD_QFW, not QFW
> - rename qemu_fwcfg_read_entry_pio to ..._io
>
>  arch/arm/Kconfig         |   1 +
>  arch/x86/cpu/qemu/cpu.c  |   7 +-
>  arch/x86/cpu/qemu/qemu.c |  54 ++------
>  arch/x86/cpu/qfw_cpu.c   |  11 +-
>  cmd/qfw.c                |  44 +++---
>  drivers/misc/Kconfig     |   1 +
>  drivers/misc/qfw.c       | 289 +++++++++++++++++++++++++++------------
>  include/qfw.h            |  63 +++++----
>  8 files changed, 292 insertions(+), 178 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index d51abbeaf0..cd01dc458a 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -937,6 +937,7 @@ config ARCH_QEMU
>  	imply DM_RNG
>  	imply DM_RTC
>  	imply RTC_PL031
> +	imply CMD_QFW
>
>  config ARCH_RMOBILE
>  	bool "Renesas ARM SoCs"
> diff --git a/arch/x86/cpu/qemu/cpu.c b/arch/x86/cpu/qemu/cpu.c
> index 9ce86b379c..ab1b797f9a 100644
> --- a/arch/x86/cpu/qemu/cpu.c
> +++ b/arch/x86/cpu/qemu/cpu.c
> @@ -22,7 +22,12 @@ int cpu_qemu_get_desc(const struct udevice *dev, char *buf, int size)
>
>  static int cpu_qemu_get_count(const struct udevice *dev)
>  {
> -	return qemu_fwcfg_online_cpus();
> +	struct udevice *qfw_dev = qemu_fwcfg_dev();
> +
> +	if (!qfw_dev)
> +		return -ENODEV;
> +
> +	return qemu_fwcfg_online_cpus(qfw_dev);
>  }
>
>  static const struct cpu_ops cpu_qemu_ops = {
> diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
> index 044a429c13..e255af9a4a 100644
> --- a/arch/x86/cpu/qemu/qemu.c
> +++ b/arch/x86/cpu/qemu/qemu.c
> @@ -8,6 +8,7 @@
>  #include <init.h>
>  #include <pci.h>
>  #include <qfw.h>
> +#include <dm/platdata.h>
>  #include <asm/irq.h>
>  #include <asm/post.h>
>  #include <asm/processor.h>
> @@ -19,45 +20,20 @@ static bool i440fx;
>  #ifdef CONFIG_QFW
>
>  /* on x86, the qfw registers are all IO ports */
> -#define FW_CONTROL_PORT	0x510
> -#define FW_DATA_PORT		0x511
> -#define FW_DMA_PORT_LOW	0x514
> -#define FW_DMA_PORT_HIGH	0x518
> -
> -static void qemu_x86_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
> -	 *
> -	 * Note: on platform where the control register is IO port, the
> -	 * endianness is little endian.
> -	 */
> -	if (entry != FW_CFG_INVALID)
> -		outw(cpu_to_le16(entry), FW_CONTROL_PORT);
> -
> -	/* the endianness of data register is string-preserving */
> -	while (size--)
> -		data[i++] = inb(FW_DATA_PORT);
> -}
> -
> -static void qemu_x86_fwcfg_read_entry_dma(struct fw_cfg_dma_access *dma)
> -{
> -	/* the DMA address register is big endian */
> -	outl(cpu_to_be32((uintptr_t)dma), FW_DMA_PORT_HIGH);
> -
> -	while (be32_to_cpu(dma->control) & ~FW_CFG_DMA_ERROR)
> -		__asm__ __volatile__ ("pause");
> -}
> +static const struct qfw_plat x86_qfw_plat = {
> +	.io = {
> +		.control_port	= 0x510,
> +		.data_port	= 0x511,
> +		.dma_port_low	= 0x514,
> +		.dma_port_high	= 0x518,
> +	},
> +};

If these numbers are constants, why should they be copied to platform
data? This only increases code size.

I think there is nothing wrong with using constants here.


>
> -static struct fw_cfg_arch_ops fwcfg_x86_ops = {
> -	.arch_read_pio = qemu_x86_fwcfg_read_entry_pio,
> -	.arch_read_dma = qemu_x86_fwcfg_read_entry_dma
> +U_BOOT_DRVINFO(x86_qfw) = {
> +	.name = "qfw",
> +	.plat = &x86_qfw_plat,
>  };
> +
>  #endif
>
>  static void enable_pm_piix(void)
> @@ -132,10 +108,6 @@ static void qemu_chipset_init(void)
>
>  		enable_pm_ich9();
>  	}
> -
> -#ifdef CONFIG_QFW
> -	qemu_fwcfg_init(&fwcfg_x86_ops);
> -#endif
>  }
>
>  #if !CONFIG_IS_ENABLED(SPL_X86_32BIT_INIT)
> diff --git a/arch/x86/cpu/qfw_cpu.c b/arch/x86/cpu/qfw_cpu.c
> index b959eaddde..c8fb918494 100644
> --- a/arch/x86/cpu/qfw_cpu.c
> +++ b/arch/x86/cpu/qfw_cpu.c
> @@ -18,7 +18,7 @@ int qemu_cpu_fixup(void)
>  	int cpu_num;
>  	int cpu_online;
>  	struct uclass *uc;
> -	struct udevice *dev, *pdev;
> +	struct udevice *dev, *pdev, *qfwdev;
>  	struct cpu_plat *plat;
>  	char *cpu;
>
> @@ -39,6 +39,13 @@ int qemu_cpu_fixup(void)
>  		return -ENODEV;
>  	}
>
> +	/* get qfw dev */
> +	qfwdev = qemu_fwcfg_dev();
> +	if (!qfwdev) {
> +		printf("unable to find qfw device\n");
> +		return -ENODEV;
> +	}
> +
>  	/* calculate cpus that are already bound */
>  	cpu_num = 0;
>  	for (uclass_find_first_device(UCLASS_CPU, &dev);
> @@ -48,7 +55,7 @@ int qemu_cpu_fixup(void)
>  	}
>
>  	/* get actual cpu number */
> -	cpu_online = qemu_fwcfg_online_cpus();
> +	cpu_online = qemu_fwcfg_online_cpus(qfwdev);
>  	if (cpu_online < 0) {
>  		printf("unable to get online cpu number: %d\n", cpu_online);
>  		return cpu_online;
> diff --git a/cmd/qfw.c b/cmd/qfw.c
> index bb571487f0..ec80a9a3b5 100644
> --- a/cmd/qfw.c
> +++ b/cmd/qfw.c
> @@ -8,19 +8,22 @@
>  #include <env.h>
>  #include <errno.h>
>  #include <qfw.h>
> +#include <dm.h>
> +
> +static struct udevice *qfw_dev;
>
>  /*
>   * This function prepares kernel for zboot. It loads kernel data
>   * to 'load_addr', initrd to 'initrd_addr' and kernel command
>   * line using qemu fw_cfg interface.
>   */
> -static int qemu_fwcfg_setup_kernel(void *load_addr, void *initrd_addr)
> +static int qemu_fwcfg_cmd_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);
> +	qemu_fwcfg_read_entry(qfw_dev, FW_CFG_SETUP_SIZE, 4, &setup_size);
> +	qemu_fwcfg_read_entry(qfw_dev, FW_CFG_KERNEL_SIZE, 4, &kernel_size);
>
>  	if (setup_size == 0 || kernel_size == 0) {
>  		printf("warning: no kernel available\n");
> @@ -28,27 +31,27 @@ static int qemu_fwcfg_setup_kernel(void *load_addr, void *initrd_addr)
>  	}
>
>  	data_addr = load_addr;
> -	qemu_fwcfg_read_entry(FW_CFG_SETUP_DATA,
> +	qemu_fwcfg_read_entry(qfw_dev, 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,
> +	qemu_fwcfg_read_entry(qfw_dev, 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);
> +	qemu_fwcfg_read_entry(qfw_dev, 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,
> +		qemu_fwcfg_read_entry(qfw_dev, 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);
> +	qemu_fwcfg_read_entry(qfw_dev, FW_CFG_CMDLINE_SIZE, 4, &cmdline_size);
>  	if (cmdline_size) {
> -		qemu_fwcfg_read_entry(FW_CFG_CMDLINE_DATA,
> +		qemu_fwcfg_read_entry(qfw_dev, FW_CFG_CMDLINE_DATA,
>  				      le32_to_cpu(cmdline_size), data_addr);
>  		/*
>  		 * if kernel cmdline only contains '\0', (e.g. no -append
> @@ -72,19 +75,18 @@ static int qemu_fwcfg_setup_kernel(void *load_addr, void *initrd_addr)
>  	return 0;
>  }
>
> -static int qemu_fwcfg_list_firmware(void)
> +static int qemu_fwcfg_cmd_list_firmware(void)
>  {
>  	int ret;
>  	struct fw_cfg_file_iter iter;
>  	struct fw_file *file;
>
>  	/* make sure fw_list is loaded */
> -	ret = qemu_fwcfg_read_firmware_list();
> +	ret = qemu_fwcfg_read_firmware_list(qfw_dev);
>  	if (ret)
>  		return ret;
>
> -
> -	for (file = qemu_fwcfg_file_iter_init(&iter);
> +	for (file = qemu_fwcfg_file_iter_init(qfw_dev, &iter);
>  	     !qemu_fwcfg_file_iter_end(&iter);
>  	     file = qemu_fwcfg_file_iter_next(&iter)) {
>  		printf("%-56s\n", file->cfg.name);
> @@ -96,7 +98,7 @@ static int qemu_fwcfg_list_firmware(void)
>  static int qemu_fwcfg_do_list(struct cmd_tbl *cmdtp, int flag,
>  			      int argc, char *const argv[])
>  {
> -	if (qemu_fwcfg_list_firmware() < 0)
> +	if (qemu_fwcfg_cmd_list_firmware() < 0)
>  		return CMD_RET_FAILURE;
>
>  	return 0;
> @@ -105,14 +107,7 @@ static int qemu_fwcfg_do_list(struct cmd_tbl *cmdtp, int flag,
>  static int qemu_fwcfg_do_cpus(struct cmd_tbl *cmdtp, int flag,
>  			      int argc, char *const argv[])
>  {
> -	int ret = qemu_fwcfg_online_cpus();
> -	if (ret < 0) {
> -		printf("QEMU fw_cfg interface not found\n");
> -		return CMD_RET_FAILURE;
> -	}
> -
> -	printf("%d cpu(s) online\n", qemu_fwcfg_online_cpus());
> -
> +	printf("%d cpu(s) online\n", qemu_fwcfg_online_cpus(qfw_dev));
>  	return 0;
>  }
>
> @@ -153,7 +148,7 @@ static int qemu_fwcfg_do_load(struct cmd_tbl *cmdtp, int flag,
>  		return CMD_RET_FAILURE;
>  	}
>
> -	return qemu_fwcfg_setup_kernel(load_addr, initrd_addr);
> +	return qemu_fwcfg_cmd_setup_kernel(load_addr, initrd_addr);
>  }
>
>  static struct cmd_tbl fwcfg_commands[] = {
> @@ -168,7 +163,8 @@ static int do_qemu_fw(struct cmd_tbl *cmdtp, int flag, int argc,
>  	int ret;
>  	struct cmd_tbl *fwcfg_cmd;
>
> -	if (!qemu_fwcfg_present()) {
> +	qfw_dev = qemu_fwcfg_dev();
> +	if (!qfw_dev) {
>  		printf("QEMU fw_cfg interface not found\n");
>  		return CMD_RET_USAGE;
>  	}
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 7d2a299779..0a65f29acd 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -367,6 +367,7 @@ config WINBOND_W83627
>
>  config QFW
>  	bool
> +	depends on MISC
>  	help
>  	  Hidden option to enable QEMU fw_cfg interface. This will be selected by
>  	  either CONFIG_CMD_QFW or CONFIG_GENERATE_ACPI_TABLE.
> diff --git a/drivers/misc/qfw.c b/drivers/misc/qfw.c
> index f6eb6583ed..eae3afd23b 100644
> --- a/drivers/misc/qfw.c
> +++ b/drivers/misc/qfw.c
> @@ -9,17 +9,18 @@
>  #include <log.h>
>  #include <malloc.h>
>  #include <qfw.h>
> +#include <dm.h>
>  #include <asm/io.h>
> +#include <misc.h>
>  #ifdef CONFIG_GENERATE_ACPI_TABLE
>  #include <asm/tables.h>
>  #endif
> -#include <linux/list.h>
>
> -static bool fwcfg_present;
> -static bool fwcfg_dma_present;
> -static struct fw_cfg_arch_ops *fwcfg_arch_ops;
> -
> -static LIST_HEAD(fw_list);
> +/* Determined at runtime. */
> +struct qfw_priv {
> +	bool dma_present;
> +	struct list_head fw_list;
> +};
>
>  #ifdef CONFIG_GENERATE_ACPI_TABLE
>  /*
> @@ -32,7 +33,8 @@ static LIST_HEAD(fw_list);
>   *          be ignored.
>   * @return: 0 on success, or negative value on failure
>   */
> -static int bios_linker_allocate(struct bios_linker_entry *entry, ulong *addr)
> +static int bios_linker_allocate(struct udevice *dev,
> +				struct bios_linker_entry *entry, ulong *addr)
>  {
>  	uint32_t size, align;
>  	struct fw_file *file;
> @@ -45,7 +47,7 @@ static int bios_linker_allocate(struct bios_linker_entry *entry, ulong *addr)
>  		return -EINVAL;
>  	}
>
> -	file = qemu_fwcfg_find_file(entry->alloc.file);
> +	file = qemu_fwcfg_find_file(dev, entry->alloc.file);
>  	if (!file) {
>  		printf("error: can't find file %s\n", entry->alloc.file);
>  		return -ENOENT;
> @@ -75,7 +77,7 @@ static int bios_linker_allocate(struct bios_linker_entry *entry, ulong *addr)
>  	debug("bios_linker_allocate: allocate file %s, size %u, zone %d, align %u, addr 0x%lx\n",
>  	      file->cfg.name, size, entry->alloc.zone, align, aligned_addr);
>
> -	qemu_fwcfg_read_entry(be16_to_cpu(file->cfg.select),
> +	qemu_fwcfg_read_entry(dev, be16_to_cpu(file->cfg.select),
>  			      size, (void *)aligned_addr);
>  	file->addr = aligned_addr;
>
> @@ -94,16 +96,17 @@ static int bios_linker_allocate(struct bios_linker_entry *entry, ulong *addr)
>   *          ACPI tables
>   * @return: 0 on success, or negative value on failure
>   */
> -static int bios_linker_add_pointer(struct bios_linker_entry *entry)
> +static int bios_linker_add_pointer(struct udevice *dev,
> +				   struct bios_linker_entry *entry)
>  {
>  	struct fw_file *dest, *src;
>  	uint32_t offset = le32_to_cpu(entry->pointer.offset);
>  	uint64_t pointer = 0;
>
> -	dest = qemu_fwcfg_find_file(entry->pointer.dest_file);
> +	dest = qemu_fwcfg_find_file(dev, entry->pointer.dest_file);
>  	if (!dest || !dest->addr)
>  		return -ENOENT;
> -	src = qemu_fwcfg_find_file(entry->pointer.src_file);
> +	src = qemu_fwcfg_find_file(dev, entry->pointer.src_file);
>  	if (!src || !src->addr)
>  		return -ENOENT;
>
> @@ -127,13 +130,14 @@ static int bios_linker_add_pointer(struct bios_linker_entry *entry)
>   *          checksums
>   * @return: 0 on success, or negative value on failure
>   */
> -static int bios_linker_add_checksum(struct bios_linker_entry *entry)
> +static int bios_linker_add_checksum(struct udevice *dev,
> +				    struct bios_linker_entry *entry)
>  {
>  	struct fw_file *file;
>  	uint8_t *data, cksum = 0;
>  	uint8_t *cksum_start;
>
> -	file = qemu_fwcfg_find_file(entry->cksum.file);
> +	file = qemu_fwcfg_find_file(dev, entry->cksum.file);
>  	if (!file || !file->addr)
>  		return -ENOENT;
>
> @@ -154,15 +158,22 @@ ulong write_acpi_tables(ulong addr)
>  	struct bios_linker_entry *table_loader;
>  	struct bios_linker_entry *entry;
>  	uint32_t size;
> +	struct udevice *dev;
> +
> +	dev = qemu_fwcfg_dev();
> +	if (!dev) {
> +		printf("error: no qfw\n");
> +		return addr;
> +	}
>
>  	/* make sure fw_list is loaded */
> -	ret = qemu_fwcfg_read_firmware_list();
> +	ret = qemu_fwcfg_read_firmware_list(dev);
>  	if (ret) {
>  		printf("error: can't read firmware file list\n");
>  		return addr;
>  	}
>
> -	file = qemu_fwcfg_find_file("etc/table-loader");
> +	file = qemu_fwcfg_find_file(dev, "etc/table-loader");
>  	if (!file) {
>  		printf("error: can't find etc/table-loader\n");
>  		return addr;
> @@ -180,24 +191,24 @@ ulong write_acpi_tables(ulong addr)
>  		return addr;
>  	}
>
> -	qemu_fwcfg_read_entry(be16_to_cpu(file->cfg.select),
> +	qemu_fwcfg_read_entry(dev, be16_to_cpu(file->cfg.select),
>  			      size, table_loader);
>
>  	for (i = 0; i < (size / sizeof(*entry)); i++) {
>  		entry = table_loader + i;
>  		switch (le32_to_cpu(entry->command)) {
>  		case BIOS_LINKER_LOADER_COMMAND_ALLOCATE:
> -			ret = bios_linker_allocate(entry, &addr);
> +			ret = bios_linker_allocate(dev, entry, &addr);
>  			if (ret)
>  				goto out;
>  			break;
>  		case BIOS_LINKER_LOADER_COMMAND_ADD_POINTER:
> -			ret = bios_linker_add_pointer(entry);
> +			ret = bios_linker_add_pointer(dev, entry);
>  			if (ret)
>  				goto out;
>  			break;
>  		case BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM:
> -			ret = bios_linker_add_checksum(entry);
> +			ret = bios_linker_add_checksum(dev, entry);
>  			if (ret)
>  				goto out;
>  			break;
> @@ -209,7 +220,7 @@ ulong write_acpi_tables(ulong addr)
>  out:
>  	if (ret) {
>  		struct fw_cfg_file_iter iter;
> -		for (file = qemu_fwcfg_file_iter_init(&iter);
> +		for (file = qemu_fwcfg_file_iter_init(dev, &iter);
>  		     !qemu_fwcfg_file_iter_end(&iter);
>  		     file = qemu_fwcfg_file_iter_next(&iter)) {
>  			if (file->addr) {
> @@ -226,31 +237,91 @@ out:
>  ulong acpi_get_rsdp_addr(void)
>  {
>  	struct fw_file *file;
> +	struct udevice *dev;
>
> -	file = qemu_fwcfg_find_file("etc/acpi/rsdp");
> +	dev = qemu_fwcfg_dev();
> +	if (!dev) {
> +		printf("error: no qfw\n");
> +		return 0;
> +	}
> +
> +	file = qemu_fwcfg_find_file(dev, "etc/acpi/rsdp");
>  	return file->addr;
>  }
>  #endif
>
> -/* Read configuration item using fw_cfg PIO interface */
> -static void qemu_fwcfg_read_entry_pio(uint16_t entry,
> -		uint32_t size, void *address)
> +/* Read configuration item using fw_cfg PIO/MMIO interface */
> +static void qemu_fwcfg_read_entry_io(struct qfw_plat *plat, u16 entry,
> +				     u32 size, void *address)
>  {
> -	debug("qemu_fwcfg_read_entry_pio: entry 0x%x, size %u address %p\n",
> +	debug("qemu_fwcfg_read_entry_io: entry 0x%x, size %u address %p\n",
>  	      entry, size, address);
>
> -	return fwcfg_arch_ops->arch_read_pio(entry, size, address);
> +	/*
> +	 * writing FW_CFG_INVALID will cause read operation to resume at last
> +	 * offset, otherwise read will start at offset 0
> +	 *
> +	 * Note: on platform where the control register is IO port, the
> +	 * endianness is little endian.  Where it is on MMIO, the register is
> +	 * big endian.
> +	 */
> +	if (entry != FW_CFG_INVALID) {
> +		if (plat->mmio)
> +			plat->mmio->selector = cpu_to_be16(entry);
> +#ifdef CONFIG_X86
> +		else
> +			outw(cpu_to_le16(entry), plat->io.control_port);
> +#endif
> +	}
> +
> +	/* the endianness of data register is string-preserving */
> +
> +	if (plat->mmio) {
> +		while (size >= 8) {
> +			*(u64 *)address = plat->mmio->data64;
> +			address += 8;
> +			size -= 8;
> +		}
> +		while (size >= 4) {
> +			*(u32 *)address = plat->mmio->data32;
> +			address += 4;
> +			size -= 4;
> +		}
> +		while (size >= 2) {
> +			*(u16 *)address = plat->mmio->data16;
> +			address += 2;
> +			size -= 2;
> +		}
> +		while (size >= 1) {
> +			*(u8 *)address = plat->mmio->data8;
> +			address += 1;
> +			size -= 1;
> +		}
> +	}
> +#ifdef CONFIG_X86
> +	else {
> +		u32 i = 0;
> +		u8 *data = address;
> +
> +		while (size--)
> +			data[i++] = inb(plat->io.data_port);
> +	}
> +#endif
>  }
>
>  /* Read configuration item using fw_cfg DMA interface */
> -static void qemu_fwcfg_read_entry_dma(uint16_t entry,
> -		uint32_t size, void *address)
> +static void qemu_fwcfg_read_entry_dma(struct qfw_plat *plat, u16 entry,
> +				      u32 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);
> +	struct {
> +		__be32 control;
> +		__be32 length;
> +		__be64 address;
> +	} dma = {
> +		.length = cpu_to_be32(size),
> +		.address = cpu_to_be64((uintptr_t)address),
> +		.control = cpu_to_be32(FW_CFG_DMA_READ),
> +	};
>
>  	/*
>  	 * writting FW_CFG_INVALID will cause read operation to resume at
> @@ -264,51 +335,58 @@ static void qemu_fwcfg_read_entry_dma(uint16_t entry,
>  	debug("qemu_fwcfg_read_entry_dma: entry 0x%x, size %u address %p, control 0x%x\n",
>  	      entry, size, address, be32_to_cpu(dma.control));
>
> -	fwcfg_arch_ops->arch_read_dma(&dma);
> -}
> +	/* the DMA address register is big-endian */
> +	if (plat->mmio)
> +		plat->mmio->dma = cpu_to_be64((uintptr_t)&dma);
> +#ifdef CONFIG_X86
> +	else
> +		outl(cpu_to_be32((uintptr_t)&dma), plat->io.dma_port_high);
> +#endif
>
> -bool qemu_fwcfg_present(void)
> -{
> -	return fwcfg_present;
> -}
>
> -bool qemu_fwcfg_dma_present(void)
> -{
> -	return fwcfg_dma_present;
> +	while (be32_to_cpu(dma.control) & ~FW_CFG_DMA_ERROR)
> +#ifdef CONFIG_X86
> +		__asm__ __volatile__ ("pause");
> +#else
> +		__asm__ __volatile__ ("yield");

ARM yield is meant to be used on multi-threaded systems to indicate that
the thread can be swapped. Why would we need it in U-Boot which is
single-threaded?

Can't we simply use

while (be32_to_cpu(dma.control) & ~FW_CFG_DMA_ERROR);

with no command in the loop for all architectures?

Best regards

Heinrich

> +#endif
>  }
>
> -void qemu_fwcfg_read_entry(uint16_t entry, uint32_t length, void *address)
> +void qemu_fwcfg_read_entry(struct udevice *dev, u16 entry, u32 length,
> +			   void *address)
>  {
> -	if (fwcfg_dma_present)
> -		qemu_fwcfg_read_entry_dma(entry, length, address);
> +	struct qfw_plat *plat = dev_get_plat(dev);
> +	struct qfw_priv *priv = dev_get_priv(dev);
> +
> +	if (priv->dma_present)
> +		qemu_fwcfg_read_entry_dma(plat, entry, length, address);
>  	else
> -		qemu_fwcfg_read_entry_pio(entry, length, address);
> +		qemu_fwcfg_read_entry_io(plat, entry, length, address);
>  }
>
> -int qemu_fwcfg_online_cpus(void)
> +int qemu_fwcfg_online_cpus(struct udevice *dev)
>  {
> -	uint16_t nb_cpus;
> -
> -	if (!fwcfg_present)
> -		return -ENODEV;
> +	u16 nb_cpus;
>
> -	qemu_fwcfg_read_entry(FW_CFG_NB_CPUS, 2, &nb_cpus);
> +	qemu_fwcfg_read_entry(dev, FW_CFG_NB_CPUS, 2, &nb_cpus);
>
>  	return le16_to_cpu(nb_cpus);
>  }
>
> -int qemu_fwcfg_read_firmware_list(void)
> +int qemu_fwcfg_read_firmware_list(struct udevice *dev)
>  {
>  	int i;
> -	uint32_t count;
> +	u32 count;
>  	struct fw_file *file;
>  	struct list_head *entry;
>
> +	struct qfw_priv *priv = dev_get_priv(dev);
> +
>  	/* don't read it twice */
> -	if (!list_empty(&fw_list))
> +	if (!list_empty(&priv->fw_list))
>  		return 0;
>
> -	qemu_fwcfg_read_entry(FW_CFG_FILE_DIR, 4, &count);
> +	qemu_fwcfg_read_entry(dev, FW_CFG_FILE_DIR, 4, &count);
>  	if (!count)
>  		return 0;
>
> @@ -319,16 +397,16 @@ int qemu_fwcfg_read_firmware_list(void)
>  			printf("error: allocating resource\n");
>  			goto err;
>  		}
> -		qemu_fwcfg_read_entry(FW_CFG_INVALID,
> +		qemu_fwcfg_read_entry(dev, FW_CFG_INVALID,
>  				      sizeof(struct fw_cfg_file), &file->cfg);
>  		file->addr = 0;
> -		list_add_tail(&file->list, &fw_list);
> +		list_add_tail(&file->list, &priv->fw_list);
>  	}
>
>  	return 0;
>
>  err:
> -	list_for_each(entry, &fw_list) {
> +	list_for_each(entry, &priv->fw_list) {
>  		file = list_entry(entry, struct fw_file, list);
>  		free(file);
>  	}
> @@ -336,12 +414,14 @@ err:
>  	return -ENOMEM;
>  }
>
> -struct fw_file *qemu_fwcfg_find_file(const char *name)
> +struct fw_file *qemu_fwcfg_find_file(struct udevice *dev, const char *name)
>  {
>  	struct list_head *entry;
>  	struct fw_file *file;
>
> -	list_for_each(entry, &fw_list) {
> +	struct qfw_priv *priv = dev_get_priv(dev);
> +
> +	list_for_each(entry, &priv->fw_list) {
>  		file = list_entry(entry, struct fw_file, list);
>  		if (!strcmp(file->cfg.name, name))
>  			return file;
> @@ -350,9 +430,13 @@ struct fw_file *qemu_fwcfg_find_file(const char *name)
>  	return NULL;
>  }
>
> -struct fw_file *qemu_fwcfg_file_iter_init(struct fw_cfg_file_iter *iter)
> +struct fw_file *qemu_fwcfg_file_iter_init(struct udevice *dev,
> +					  struct fw_cfg_file_iter *iter)
>  {
> -	iter->entry = fw_list.next;
> +	struct qfw_priv *priv = dev_get_priv(dev);
> +
> +	iter->entry = priv->fw_list.next;
> +	iter->end = &priv->fw_list;
>  	return list_entry((struct list_head *)iter->entry,
>  			  struct fw_file, list);
>  }
> @@ -366,29 +450,62 @@ struct fw_file *qemu_fwcfg_file_iter_next(struct fw_cfg_file_iter *iter)
>
>  bool qemu_fwcfg_file_iter_end(struct fw_cfg_file_iter *iter)
>  {
> -	return iter->entry == &fw_list;
> +	return iter->entry == iter->end;
>  }
>
> -void qemu_fwcfg_init(struct fw_cfg_arch_ops *ops)
> +static int qfw_of_to_plat(struct udevice *dev)
>  {
> -	uint32_t qemu;
> -	uint32_t dma_enabled;
> -
> -	fwcfg_present = false;
> -	fwcfg_dma_present = false;
> -	fwcfg_arch_ops = NULL;
> -
> -	if (!ops || !ops->arch_read_pio || !ops->arch_read_dma)
> -		return;
> -	fwcfg_arch_ops = ops;
> -
> -	qemu_fwcfg_read_entry_pio(FW_CFG_SIGNATURE, 4, &qemu);
> -	if (be32_to_cpu(qemu) == QEMU_FW_CFG_SIGNATURE)
> -		fwcfg_present = true;
> -
> -	if (fwcfg_present) {
> -		qemu_fwcfg_read_entry_pio(FW_CFG_ID, 1, &dma_enabled);
> -		if (dma_enabled & FW_CFG_DMA_ENABLED)
> -			fwcfg_dma_present = true;
> -	}
> +	struct qfw_plat *plat = dev_get_plat(dev);
> +
> +	plat->mmio = map_physmem(dev_read_addr(dev),
> +				 sizeof(struct qfw_mmio),
> +				 MAP_NOCACHE);
> +
> +	return 0;
> +}
> +
> +static int qfw_probe(struct udevice *dev)
> +{
> +	u32 qemu, dma_enabled;
> +	struct qfw_plat *plat = dev_get_plat(dev);
> +	struct qfw_priv *priv = dev_get_priv(dev);
> +
> +	INIT_LIST_HEAD(&priv->fw_list);
> +
> +	qemu_fwcfg_read_entry_io(plat, FW_CFG_SIGNATURE, 4, &qemu);
> +	if (be32_to_cpu(qemu) != QEMU_FW_CFG_SIGNATURE)
> +		return -ENODEV;
> +
> +	qemu_fwcfg_read_entry_io(plat, FW_CFG_ID, 1, &dma_enabled);
> +	if (dma_enabled & FW_CFG_DMA_ENABLED)
> +		priv->dma_present = true;
> +
> +	return 0;
> +}
> +
> +static const struct udevice_id qfw_ids[] = {
> +	{ .compatible = "qemu,fw-cfg-mmio" },
> +	{}
> +};
> +
> +U_BOOT_DRIVER(qfw) = {
> +	.name		= "qfw",
> +	.id		= UCLASS_MISC,
> +	.of_match	= qfw_ids,
> +	.of_to_plat	= qfw_of_to_plat,
> +	.plat_auto	= sizeof(struct qfw_plat),
> +	.priv_auto	= sizeof(struct qfw_priv),
> +	.probe		= qfw_probe,
> +};
> +
> +struct udevice *qemu_fwcfg_dev(void)
> +{
> +	struct udevice *dev;
> +	int ret;
> +
> +	/* XXX: decide what to do here */
> +	ret = uclass_first_device(UCLASS_MISC, &dev);
> +	if (ret)
> +		return NULL;
> +	return dev;
>  }
> diff --git a/include/qfw.h b/include/qfw.h
> index cea8e11d44..f9c6828841 100644
> --- a/include/qfw.h
> +++ b/include/qfw.h
> @@ -82,19 +82,7 @@ struct fw_file {
>  };
>
>  struct fw_cfg_file_iter {
> -	struct list_head *entry; /* structure to iterate file list */
> -};
> -
> -struct fw_cfg_dma_access {
> -	__be32 control;
> -	__be32 length;
> -	__be64 address;
> -};
> -
> -struct fw_cfg_arch_ops {
> -	void (*arch_read_pio)(uint16_t selector, uint32_t size,
> -			void *address);
> -	void (*arch_read_dma)(struct fw_cfg_dma_access *dma);
> +	struct list_head *entry, *end; /* structures to iterate file list */
>  };
>
>  struct bios_linker_entry {
> @@ -146,32 +134,59 @@ struct bios_linker_entry {
>  	};
>  } __packed;
>
> +struct qfw_mmio {
> +	/*
> +	 * Each access to the 64-bit data register can be 8/16/32/64 bits wide.
> +	 */
> +	union {
> +		u8 data8;
> +		u16 data16;
> +		u32 data32;
> +		u64 data64;
> +	};
> +	u16 selector;
> +	u8 padding[6];
> +	u64 dma;
> +};
> +
> +struct qfw_plat {
> +	/* MMIO used on Arm. */
> +	volatile struct qfw_mmio *mmio;
> +	/* IO used on x86. */
> +	struct {
> +		u16 control_port;
> +		u16 data_port;
> +		u16 dma_port_low;
> +		u16 dma_port_high;
> +	} io;
> +};
> +
> +struct udevice;
>  /**
> - * Initialize QEMU fw_cfg interface
> + * Get QEMU firmware config device
>   *
> - * @ops: arch specific read operations
> + * @return struct udevice * if present, NULL otherwise
>   */
> -void qemu_fwcfg_init(struct fw_cfg_arch_ops *ops);
> +struct udevice *qemu_fwcfg_dev(void);
>
> -void qemu_fwcfg_read_entry(uint16_t entry, uint32_t length, void *address);
> -int qemu_fwcfg_read_firmware_list(void);
> -struct fw_file *qemu_fwcfg_find_file(const char *name);
> +void qemu_fwcfg_read_entry(struct udevice *dev, u16 entry, u32 length,
> +			   void *address);
> +int qemu_fwcfg_read_firmware_list(struct udevice *dev);
> +struct fw_file *qemu_fwcfg_find_file(struct udevice *dev, const char *name);
>
>  /**
>   * Get system cpu number
>   *
>   * @return:   cpu number in system
>   */
> -int qemu_fwcfg_online_cpus(void);
> +int qemu_fwcfg_online_cpus(struct udevice *dev);
>
>  /* helper functions to iterate firmware file list */
> -struct fw_file *qemu_fwcfg_file_iter_init(struct fw_cfg_file_iter *iter);
> +struct fw_file *qemu_fwcfg_file_iter_init(struct udevice *dev,
> +					  struct fw_cfg_file_iter *iter);
>  struct fw_file *qemu_fwcfg_file_iter_next(struct fw_cfg_file_iter *iter);
>  bool qemu_fwcfg_file_iter_end(struct fw_cfg_file_iter *iter);
>
> -bool qemu_fwcfg_present(void);
> -bool qemu_fwcfg_dma_present(void);
> -
>  /**
>   * qemu_cpu_fixup() - Fix up the CPUs for QEMU
>   *
>



More information about the U-Boot mailing list