Re: [PATCH 7/7] hw/nvram: Do not build FW_CFG if not required

[Philippe Mathieu-Daudé]

On 4/26/21 9:35 PM, Philippe Mathieu-Daudé wrote:
> If the Kconfig 'FW_CFG' symbol is not selected, it is pointless
> to build the fw_cfg device. Update the stubs.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  stubs/fw_cfg.c       | 49 ++++++++++++++++++++++++++++++++++++++++++--
>  hw/nvram/meson.build |  2 +-
>  2 files changed, 48 insertions(+), 3 deletions(-)

Answering here to Laszlo's comment from:
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg05858.html

On 4/28/21 6:44 PM, Laszlo Ersek wrote:
> I don't understand why we need to add *more code* (stubs / boilerplate)
> if our goal is (apparently) to build QEMU with *fewer* devices.

The list of callers:

hw/acpi/bios-linker-loader.c:177:    return fw_cfg &&
fw_cfg_dma_enabled(fw_cfg);
hw/acpi/core.c:640:        fw_cfg_add_file(fw_cfg, "etc/system-states",
g_memdup(suspend, 6), 6);
hw/acpi/ghes.c:383:    fw_cfg_add_file(s, ACPI_GHES_ERRORS_FW_CFG_FILE,
hardware_error->data,
hw/acpi/ghes.c:387:    fw_cfg_add_file_callback(s,
ACPI_GHES_DATA_ADDR_FW_CFG_FILE, NULL, NULL,
hw/acpi/nvdimm.c:912:    fw_cfg_add_file(fw_cfg, NVDIMM_DSM_MEM_FILE,
state->dsm_mem->data,
hw/acpi/vmgenid.c:128:    fw_cfg_add_file(s, VMGENID_GUID_FW_CFG_FILE,
guid->data,
hw/acpi/vmgenid.c:131:    fw_cfg_add_file_callback(s,
VMGENID_ADDR_FW_CFG_FILE, NULL, NULL, NULL,
hw/arm/virt-acpi-build.c:870:    fw_cfg_add_file(vms->fw_cfg,
ACPI_BUILD_TPMLOG_FILE, tables.tcpalog->data,
hw/arm/virt.c:1531:        fw_cfg_add_file(vms->fw_cfg,
"etc/smbios/smbios-tables",
hw/arm/virt.c:1533:        fw_cfg_add_file(vms->fw_cfg,
"etc/smbios/smbios-anchor",
hw/core/loader.c:1017:        fw_cfg_add_file(fw_cfg, fw_file_name,
data, rom->romsize);
hw/core/loader.c:1074:        fw_cfg_add_file_callback(fw_cfg, fw_file_name,
hw/core/loader.c:1254:    fw_cfg_set_order_override(fw_cfg, order);
hw/core/loader.c:1261:    fw_cfg_reset_order_override(fw_cfg);
hw/core/loader.c:919:        fw_cfg_modify_file(fw_cfg, id +
strlen("/rom@"), host, length);
hw/display/ramfb.c:131:    fw_cfg_add_file_callback(fw_cfg, "etc/ramfb",
hw/hppa/machine.c:104:    fw_cfg_add_file(fw_cfg,
"/etc/firmware-min-version",
hw/hppa/machine.c:108:    fw_cfg_add_file(fw_cfg, "/etc/cpu/tlb_entries",
hw/hppa/machine.c:112:    fw_cfg_add_file(fw_cfg, "/etc/cpu/btlb_entries",
hw/hppa/machine.c:116:    fw_cfg_add_file(fw_cfg, "/etc/power-button-addr",
hw/i386/acpi-build.c:2638:    fw_cfg_add_file(x86ms->fw_cfg,
ACPI_BUILD_TPMLOG_FILE,
hw/i386/acpi-build.c:2648:        fw_cfg_add_file(x86ms->fw_cfg,
"etc/tpm/config",
hw/i386/acpi-build.c:2667:
fw_cfg_add_file_callback(x86ms->fw_cfg, ACPI_BUILD_RSDP_FILE,
hw/i386/fw_cfg.c:130:    fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
hw/i386/fw_cfg.c:181:    fw_cfg_add_file(fw_cfg,
"etc/msr_feature_control", val, sizeof(*val));
hw/i386/fw_cfg.c:85:        fw_cfg_add_file(fw_cfg,
"etc/smbios/smbios-tables",
hw/i386/fw_cfg.c:87:        fw_cfg_add_file(fw_cfg,
"etc/smbios/smbios-anchor",
hw/i386/microvm.c:329:    fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
hw/i386/pc.c:977:        fw_cfg_add_file(fw_cfg,
"etc/reserved-memory-end", val, sizeof(*val));
hw/i386/x86.c:1078:    if (linuxboot_dma_enabled &&
fw_cfg_dma_enabled(fw_cfg)) {
hw/isa/lpc_ich9.c:421:        fw_cfg_add_file(fw_cfg,
"etc/smi/supported-features",
hw/isa/lpc_ich9.c:428:        fw_cfg_add_file_callback(fw_cfg,
"etc/smi/requested-features",
hw/isa/lpc_ich9.c:433:        fw_cfg_add_file_callback(fw_cfg,
"etc/smi/features-ok",
hw/misc/pvpanic-isa.c:60:    fw_cfg_add_file(fw_cfg, "etc/pvpanic-port",
pvpanic_port,
hw/misc/vmcoreinfo.c:60:    fw_cfg_add_file_callback(fw_cfg,
FW_CFG_VMCOREINFO_FILENAME,
hw/ppc/mac_newworld.c:526:            fw_cfg_add_file(fw_cfg,
"ndrv/qemu_vga.ndrv", ndrv_file, ndrv_size);
hw/ppc/mac_oldworld.c:371:            fw_cfg_add_file(fw_cfg,
"ndrv/qemu_vga.ndrv", ndrv_file, ndrv_size);
hw/vfio/igd.c:565:    fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size",
hw/vfio/pci-quirks.c:1201:    fw_cfg_add_file(fw_cfg_find(),
"etc/igd-opregion",
softmmu/vl.c:1183:        if (!fw_cfg_add_from_generator(fw_cfg, name,
gen_id, errp)) {
softmmu/vl.c:1196:    fw_cfg_set_order_override(fw_cfg,
FW_CFG_ORDER_OVERRIDE_USER);
softmmu/vl.c:1197:    fw_cfg_add_file(fw_cfg, name, buf, size);
softmmu/vl.c:1198:    fw_cfg_reset_order_override(fw_cfg);

>From this list,

I'd like to simplify hw/acpi/bios-linker-loader.c, but later.

The remaining core components are hw/core/loader.c and softmmu/vl.c:

hw/core/loader.c:1017:        fw_cfg_add_file(fw_cfg, fw_file_name,
data, rom->romsize);
hw/core/loader.c:1074:        fw_cfg_add_file_callback(fw_cfg, fw_file_name,
hw/core/loader.c:1254:    fw_cfg_set_order_override(fw_cfg, order);
hw/core/loader.c:1261:    fw_cfg_reset_order_override(fw_cfg);
hw/core/loader.c:919:        fw_cfg_modify_file(fw_cfg, id +
strlen("/rom@"), host, length);

softmmu/vl.c:1183:        if (!fw_cfg_add_from_generator(fw_cfg, name,
gen_id, errp)) {
softmmu/vl.c:1196:    fw_cfg_set_order_override(fw_cfg,
FW_CFG_ORDER_OVERRIDE_USER);
softmmu/vl.c:1197:    fw_cfg_add_file(fw_cfg, name, buf, size);
softmmu/vl.c:1198:    fw_cfg_reset_order_override(fw_cfg);

Bah, I thought vl.c was generic code, but it is target-specific,
so we could add '#include CONFIG_DEVICES' and add more #ifdef'ry
on CONFIG_FW_CFG.

Less stubs VS more #ifdef'ry...

It is not clear to me how to modularize this from the core code.

TBC...

Thanks,

Phil.