Re: [PATCH v4 1/5] arm: x86: qemu: move qfw to DM uclass, add Arm support

Hi Asherah,

On Wed, Feb 24, 2021 at 11:23 AM Asherah Connor ashe@kivikakk.ee wrote:

Updates the QFW driver to use the driver model, splitting the driver into qfw_pio and qfw_mmio (for non-x86) in their own uclass.

Signed-off-by: Asherah Connor ashe@kivikakk.ee

Changes in v4:

• PIO definitions are now #defines
• qfw_*_plat structs are no longer in header files
• Remove yield/pause in DMA wait loop
• Change struct udevice *qfw_get_dev(void) to int qfw_get_dev(struct udevice **)

arch/arm/Kconfig | 1 + arch/x86/cpu/qemu/cpu.c | 9 +- arch/x86/cpu/qemu/qemu.c | 47 +------- arch/x86/cpu/qfw_cpu.c | 11 +- cmd/qfw.c | 52 ++++----- common/Makefile | 2 + common/qfw.c | 105 +++++++++++++++++ drivers/misc/Makefile | 6 +- drivers/misc/qfw.c | 238 ++++++++++++++------------------------- drivers/misc/qfw_mmio.c | 119 ++++++++++++++++++++ drivers/misc/qfw_pio.c | 69 ++++++++++++ include/dm/uclass-id.h | 1 + include/qfw.h | 61 ++++++---- 13 files changed, 466 insertions(+), 255 deletions(-) create mode 100644 common/qfw.c create mode 100644 drivers/misc/qfw_mmio.c create mode 100644 drivers/misc/qfw_pio.c

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..c78e374644 100644 --- a/arch/x86/cpu/qemu/cpu.c +++ b/arch/x86/cpu/qemu/cpu.c @@ -22,7 +22,14 @@ 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();
  

•   int ret;
  

•   struct udevice *qfw_dev;
  

•   ret = qfw_get_dev(&qfw_dev);
  

•   if (ret)
  

•           return ret;
  

•   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..605f51e1b8 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> @@ -18,46 +19,10 @@ 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 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_pio) = {

•   .name = "qfw_pio",
  

};

#endif

static void enable_pm_piix(void) @@ -132,10 +97,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..9700908064 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 */
  

•   ret = qfw_get_dev(&qfwdev);
  

•   if (ret) {
  

•           printf("unable to find qfw device\n");
  

•           return ret;
  

•   }
  

•   /* 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..87af408a8d 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);
  

•   qfw_read_entry(qfw_dev, FW_CFG_SETUP_SIZE, 4, &setup_size);
  

•   qfw_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,28 +31,28 @@ static int qemu_fwcfg_setup_kernel(void *load_addr, void *initrd_addr) }

    data_addr = load_addr;

•   qemu_fwcfg_read_entry(FW_CFG_SETUP_DATA,
  

•                         le32_to_cpu(setup_size), data_addr);
  

•   qfw_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,
  

•                         le32_to_cpu(kernel_size), data_addr);
  

•   qfw_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);
  

•   qfw_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,
  

•                                 le32_to_cpu(initrd_size), data_addr);
  

•           qfw_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);
  

•   qfw_read_entry(qfw_dev, 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);
  

•           qfw_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
             * when invoking qemu), do not update bootargs
  

@@ -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()) {
  

•   ret = qfw_get_dev(&qfw_dev);
  

•   if (ret) {
            printf("QEMU fw_cfg interface not found\n");
            return CMD_RET_USAGE;
    }
  

diff --git a/common/Makefile b/common/Makefile index daeea67cf2..f174a06c33 100644 --- a/common/Makefile +++ b/common/Makefile @@ -137,3 +137,5 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o

obj-$(CONFIG_AVB_VERIFY) += avb_verify.o

+obj-$(CONFIG_QFW) += qfw.o diff --git a/common/qfw.c b/common/qfw.c new file mode 100644 index 0000000000..c0ffa20b74 --- /dev/null +++ b/common/qfw.c @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • (C) Copyright 2015 Miao Yan yanmiaobest@gmail.com
• • (C) Copyright 2021 Asherah Connor ashe@kivikakk.ee
• */

+#include <dm.h> +#include <dm/uclass.h> +#include <qfw.h> +#include <stdlib.h>

+int qfw_get_dev(struct udevice **devp) +{

•   return uclass_first_device(UCLASS_QFW, devp);
  

+}

+int qemu_fwcfg_online_cpus(struct udevice *dev) +{

•   u16 nb_cpus;
  

•   qfw_read_entry(dev, FW_CFG_NB_CPUS, 2, &nb_cpus);
  

•   return le16_to_cpu(nb_cpus);
  

+}

+int qemu_fwcfg_read_firmware_list(struct udevice *dev) +{

•   int i;
  

•   u32 count;
  

•   struct fw_file *file;
  

•   struct list_head *entry;
  

•   struct qfw_dev *qdev = dev_get_uclass_priv(dev);
  

•   /* don't read it twice */
  

•   if (!list_empty(&qdev->fw_list))
  

•           return 0;
  

•   qfw_read_entry(dev, FW_CFG_FILE_DIR, 4, &count);
  

•   if (!count)
  

•           return 0;
  

•   count = be32_to_cpu(count);
  

•   for (i = 0; i < count; i++) {
  

•           file = malloc(sizeof(*file));
  

•           if (!file) {
  

•                   printf("error: allocating resource\n");
  

•                   goto err;
  

•           }
  

•           qfw_read_entry(dev, FW_CFG_INVALID,
  

•                          sizeof(struct fw_cfg_file), &file->cfg);
  

•           file->addr = 0;
  

•           list_add_tail(&file->list, &qdev->fw_list);
  

•   }
  

•   return 0;
  

+err:

•   list_for_each(entry, &qdev->fw_list) {
  

•           file = list_entry(entry, struct fw_file, list);
  

•           free(file);
  

•   }
  

•   return -ENOMEM;
  

+}

+struct fw_file *qemu_fwcfg_find_file(struct udevice *dev, const char *name) +{

•   struct list_head *entry;
  

•   struct fw_file *file;
  

•   struct qfw_dev *qdev = dev_get_uclass_priv(dev);
  

•   list_for_each(entry, &qdev->fw_list) {
  

•           file = list_entry(entry, struct fw_file, list);
  

•           if (!strcmp(file->cfg.name, name))
  

•                   return file;
  

•   }
  

•   return NULL;
  

+}

+struct fw_file *qemu_fwcfg_file_iter_init(struct udevice *dev,

•                                     struct fw_cfg_file_iter *iter)
  

+{

•   struct qfw_dev *qdev = dev_get_uclass_priv(dev);
  

•   iter->entry = qdev->fw_list.next;
  

•   iter->end = &qdev->fw_list;
  

•   return list_entry((struct list_head *)iter->entry,
  

•                     struct fw_file, list);
  

+}

+struct fw_file *qemu_fwcfg_file_iter_next(struct fw_cfg_file_iter *iter) +{

•   iter->entry = ((struct list_head *)iter->entry)->next;
  

•   return list_entry((struct list_head *)iter->entry,
  

•                     struct fw_file, list);
  

+}

+bool qemu_fwcfg_file_iter_end(struct fw_cfg_file_iter *iter) +{

•   return iter->entry == iter->end;
  

+}

diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index d737203704..2988289ea3 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -55,7 +55,11 @@ obj-$(CONFIG_NUVOTON_NCT6102D) += nuvoton_nct6102d.o obj-$(CONFIG_P2SB) += p2sb-uclass.o obj-$(CONFIG_PCA9551_LED) += pca9551_led.o obj-$(CONFIG_$(SPL_)PWRSEQ) += pwrseq-uclass.o -obj-$(CONFIG_QFW) += qfw.o +ifdef CONFIG_QFW +obj-y += qfw.o +obj-$(CONFIG_X86) += qfw_pio.o +obj-$(CONFIG_ARM) += qfw_mmio.o

This does not scale when we reuse this driver on RISC-V, as I saw your QEMU patch.

So we can do like this:

CONFIG_QFW_PIO and CONFIG_QFW_MMIO

Let QEMU x86 select CONFIG_QFW_PIO, and ARM (RISC-V in the future) select CONFIG_QFW_MMIO

+endif obj-$(CONFIG_ROCKCHIP_EFUSE) += rockchip-efuse.o obj-$(CONFIG_ROCKCHIP_OTP) += rockchip-otp.o obj-$(CONFIG_SANDBOX) += syscon_sandbox.o misc_sandbox.o

[snip]

Regards, Bin