On Wed, 3 Dec 2025 00:28:22 +0800 Zhao Liu <[email protected]> wrote:
> From: Philippe Mathieu-Daudé <[email protected]> > > Now all calls of x86 machines to fw_cfg_init_io_dma() pass DMA > arguments, so the FWCfgState (FWCfgIoState) created by x86 machines > enables DMA by default. > > Although other callers of fw_cfg_init_io_dma() besides x86 also pass > DMA arguments to create DMA-enabled FwCfgIoState, the "dma_enabled" > property of FwCfgIoState cannot yet be removed, because Sun4u and Sun4v > still create DMA-disabled FwCfgIoState (bypass fw_cfg_init_io_dma()) in > sun4uv_init() (hw/sparc64/sun4u.c). > > Maybe reusing fw_cfg_init_io_dma() for them would be a better choice, or > adding fw_cfg_init_io_nodma(). However, before that, first simplify the > handling of FwCfgState in x86. > > Considering that FwCfgIoState in x86 enables DMA by default, remove the > handling for DMA-disabled cases and replace DMA checks with assertions > to ensure that the default DMA-enabled setting is not broken. > > Then 'linuxboot.bin' isn't used anymore, and it will be removed in the > next commit. > > Signed-off-by: Philippe Mathieu-Daudé <[email protected]> > Signed-off-by: Zhao Liu <[email protected]> > --- > Changes since v4: > * Keep "dma_enabled" property in fw_cfg_io_properties[]. > * Replace DMA checks with assertions for x86 machines. > --- > hw/i386/fw_cfg.c | 16 ++++++++-------- > hw/i386/x86-common.c | 6 ++---- > 2 files changed, 10 insertions(+), 12 deletions(-) > > diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c > index 5c0bcd5f8a9f..5670e8553eaa 100644 > --- a/hw/i386/fw_cfg.c > +++ b/hw/i386/fw_cfg.c > @@ -215,18 +215,18 @@ void fw_cfg_build_feature_control(MachineState *ms, > FWCfgState *fw_cfg) > #ifdef CONFIG_ACPI > void fw_cfg_add_acpi_dsdt(Aml *scope, FWCfgState *fw_cfg) > { > + uint8_t io_size; > + Aml *dev = aml_device("FWCF"); > + Aml *crs = aml_resource_template(); > + > /* > * when using port i/o, the 8-bit data register *always* overlaps > * with half of the 16-bit control register. Hence, the total size > - * of the i/o region used is FW_CFG_CTL_SIZE; when using DMA, the > - * DMA control register is located at FW_CFG_DMA_IO_BASE + 4 > + * of the i/o region used is FW_CFG_CTL_SIZE; And the DMA control > + * register is located at FW_CFG_DMA_IO_BASE + 4 > */ > - Object *obj = OBJECT(fw_cfg); > - uint8_t io_size = object_property_get_bool(obj, "dma_enabled", NULL) ? > - ROUND_UP(FW_CFG_CTL_SIZE, 4) + sizeof(dma_addr_t) : > - FW_CFG_CTL_SIZE; > - Aml *dev = aml_device("FWCF"); > - Aml *crs = aml_resource_template(); > + assert(fw_cfg_dma_enabled(fw_cfg)); this was supposed to be machine agnostic, but given that it only used by x86 it shouldn't cause issues, so Acked-by: Igor Mammedov <[email protected]> > + io_size = ROUND_UP(FW_CFG_CTL_SIZE, 4) + sizeof(dma_addr_t); > > aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002"))); > > diff --git a/hw/i386/x86-common.c b/hw/i386/x86-common.c > index 1ee55382dab8..e8dc4d903bd6 100644 > --- a/hw/i386/x86-common.c > +++ b/hw/i386/x86-common.c > @@ -1002,10 +1002,8 @@ void x86_load_linux(X86MachineState *x86ms, > } > > option_rom[nb_option_roms].bootindex = 0; > - option_rom[nb_option_roms].name = "linuxboot.bin"; > - if (fw_cfg_dma_enabled(fw_cfg)) { > - option_rom[nb_option_roms].name = "linuxboot_dma.bin"; > - } > + assert(fw_cfg_dma_enabled(fw_cfg)); > + option_rom[nb_option_roms].name = "linuxboot_dma.bin"; > nb_option_roms++; > } >
