On 16/6/26 11:29, Peter Maydell wrote:
On Wed, 3 Jun 2026 at 08:08, Philippe Mathieu-Daudé <[email protected]> wrote:
Hi Peter,
On 29/5/26 19:46, Peter Maydell wrote:
We implement the fw_cfg device for more architectures and machines
that we let on about in our documentation. Luckily most of the new
ones (notably riscv and loongarch) have followed the straightforward
layout that the Arm virt board picked.
Restructure the documentation to present this as the "standard"
layout, followed by the other layouts used by various other boards
for historical reasons. This adds PA-RISC, SPARC, PPC and MIPS.
Signed-off-by: Peter Maydell <[email protected]>
---
docs/specs/fw_cfg.rst | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/docs/specs/fw_cfg.rst b/docs/specs/fw_cfg.rst
index 31ae31576b..7e2fe0851d 100644
--- a/docs/specs/fw_cfg.rst
+++ b/docs/specs/fw_cfg.rst
@@ -84,15 +84,35 @@ increasing address order, similar to memcpy().
Register Locations
------------------
+For a memory-mapped fw_cfg device, the standard register layout is:
+
+ * base address : Data Register (64 bit)
+ * base address + 8 : Selector Register (16 bit)
+ * base address + 16 : DMA Address Register (64 bit)
+
+Some architectures or machines have a different layout for historical reasons:
+
x86, x86_64
* Selector Register IOport: 0x510
* Data Register IOport: 0x511
* DMA Address IOport: 0x514
+64-bit SPARC:
+ * base address : Selector Register (16 bit)
+ * base address + 1 : Data Register (8 bit)
IIUC they have both the same base address.
This one's confusing, because the code for both sparc64 and x86
fw_cfg is more lax than what we document.
The sun4u code does:
qdev_prop_set_bit(dev, "dma_enabled", false);
and:
memory_region_add_subregion(pci_address_space_io(ebus), BIOS_CFG_IOPORT,
&FW_CFG_IO(dev)->comb_iomem);
so it uses the fw_cfg's comb_iomem MemoryRegion. This is the
same thing the x86 ioport version uses (except that for sparc64 we
disable the DMA register), so it's the same layout, with
the selector register at the base address and the data register
nominally at an offset 1 greater than that.
The implementation in fw_cfg.c (fw_cfg_comb_mem_ops) doesn't actually
check the address, only the size of data written, and dispatches like this:
- 1 byte writes are ignored (they used to be data writes in ancient QEMU)
- 2 byte writes are selector register writes
- 1 byte reads are data register reads
- everything else is rejected by the .valid.accepts function
and you can use either baseaddr or baseaddr+1 for any of that.
What I opted to document here is (a) the same thing we already
document for the x86 ioport interface, which uses comb_iomem
and so also is not actually checking the address values, and
(b) what the Linux driver in drivers/firmware/qemu_fw_cfg.c uses:
# elif (defined(CONFIG_X86) || defined(CONFIG_SPARC64)) /* x86, sun4u */
# define FW_CFG_CTRL_OFF 0x00
# define FW_CFG_DATA_OFF 0x01
# define FW_CFG_DMA_OFF 0x04
# else
We could document the lax conditions that our implementation
actually enforces for both sparc and x86, I guess, but they
felt to me more like an accident of implementation. What do
you think?
Looking at it as an accident of the implementation, we are good
indeed. Thanks for the clarification.
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
thanks
-- PMM
