From 039e1c24cd8d7b86b7e061ac02d84c2e8c7beefd Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 29 May 2026 18:46:36 +0100 Subject: [PATCH] hw/nvram/fw_cfg: Enforce standard layout for fw_cfg_init_mem_dma() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently fw_cfg_init_mem_dma() allows the caller to customize the register layout, by specifying separately the offsets for control, data and DMA registers, plus the width of the data register. In practice, all the boards using this function specify the same standard layout: "base + 8, base, 8, base + 16", meaning that the data register is 8 bytes and the registers are data at offset 0, control/selector at offset 8, and DMA at offset 16. Allowing every board to be different is gratuitous and useless variation which leads to code in guest OSes having architecture ifdeffery to cope with it. Avoid potentially introducing any more of this by removing all the arguments from fw_cfg_init_mem_dma(), so that the callers only specify the base address. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Message-id: 20260529174639.451353-3-peter.maydell@linaro.org Reviewed-by: Michael S. Tsirkin --- hw/arm/virt.c | 2 +- hw/loongarch/fw_cfg.c | 3 +-- hw/nvram/fw_cfg.c | 10 ++++------ hw/riscv/virt.c | 3 +-- include/hw/nvram/fw_cfg.h | 23 ++++++++++++++++++++--- 5 files changed, 27 insertions(+), 14 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index d8d27f2ef6..abee62fcbc 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1944,7 +1944,7 @@ static FWCfgState *create_fw_cfg(const VirtMachineState *vms, AddressSpace *as) FWCfgState *fw_cfg; char *nodename; - fw_cfg = fw_cfg_init_mem_dma(base + 8, base, 8, base + 16, as); + fw_cfg = fw_cfg_init_mem_dma(base, as); fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)ms->smp.cpus); nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base); diff --git a/hw/loongarch/fw_cfg.c b/hw/loongarch/fw_cfg.c index d2a79efbf7..4c976ce1e5 100644 --- a/hw/loongarch/fw_cfg.c +++ b/hw/loongarch/fw_cfg.c @@ -23,8 +23,7 @@ FWCfgState *virt_fw_cfg_init(ram_addr_t ram_size, MachineState *ms) int max_cpus = ms->smp.max_cpus; int smp_cpus = ms->smp.cpus; - fw_cfg = fw_cfg_init_mem_dma(VIRT_FWCFG_BASE + 8, VIRT_FWCFG_BASE, 8, - VIRT_FWCFG_BASE + 16, &address_space_memory); + fw_cfg = fw_cfg_init_mem_dma(VIRT_FWCFG_BASE, &address_space_memory); fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)max_cpus); fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size); fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus); diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 1d7d835421..59cf92293c 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -1088,13 +1088,11 @@ static FWCfgState *fw_cfg_init_mem_internal(hwaddr ctl_addr, return s; } -FWCfgState *fw_cfg_init_mem_dma(hwaddr ctl_addr, - hwaddr data_addr, uint32_t data_width, - hwaddr dma_addr, AddressSpace *dma_as) +FWCfgState *fw_cfg_init_mem_dma(hwaddr base_addr, AddressSpace *dma_as) { - assert(dma_addr && dma_as); - return fw_cfg_init_mem_internal(ctl_addr, data_addr, data_width, - dma_addr, dma_as); + assert(dma_as); + return fw_cfg_init_mem_internal(base_addr + 8, base_addr, 8, + base_addr + 16, dma_as); } FWCfgState *fw_cfg_init_mem_nodma(hwaddr ctl_addr, hwaddr data_addr, diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index b68067cfdd..bd9f77aad3 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -1109,8 +1109,7 @@ static FWCfgState *create_fw_cfg(const MachineState *ms, hwaddr base) { FWCfgState *fw_cfg; - fw_cfg = fw_cfg_init_mem_dma(base + 8, base, 8, base + 16, - &address_space_memory); + fw_cfg = fw_cfg_init_mem_dma(base, &address_space_memory); fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)ms->smp.cpus); return fw_cfg; diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h index 56f17a0bdc..45a3747908 100644 --- a/include/hw/nvram/fw_cfg.h +++ b/include/hw/nvram/fw_cfg.h @@ -309,9 +309,26 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase, AddressSpace *dma_as); FWCfgState *fw_cfg_init_mem_nodma(hwaddr ctl_addr, hwaddr data_addr, unsigned data_width); -FWCfgState *fw_cfg_init_mem_dma(hwaddr ctl_addr, - hwaddr data_addr, uint32_t data_width, - hwaddr dma_addr, AddressSpace *dma_as); +/** + * fw_cfg_init_mem_dma: + * @base_addr: address to map the device at + * @as: the device will do DMA to/from this AddressSpace + * + * Create and map a fw_cfg device at the specified base address. + * + * This always creates a device with DMA support, and the "standard" + * register layout: + * - offset 0 : data, 64 bits + * - offset 8 : selector, 16 bits + * - offset 16 : DMA address, 64 bits + * + * The device will be created, configured and realized, and its + * memory regions for the registers will be mapped at the specified + * address. + * + * Returns the device object. + */ +FWCfgState *fw_cfg_init_mem_dma(hwaddr base_addr, AddressSpace *dma_as); FWCfgState *fw_cfg_find(void); bool fw_cfg_dma_enabled(void *opaque);