Re: [PATCH v2 03/16] hw/i386/x86: Remove X86MachineClass::fwcfg_dma_enabled field

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 5/5/25 11:06, Thomas Huth wrote:
> On 02/05/2025 12.45, Philippe Mathieu-Daudé wrote:
>> On 2/5/25 11:08, Mark Cave-Ayland wrote:
>>> On 01/05/2025 19:36, Philippe Mathieu-Daudé wrote:
>>>
>>>> The X86MachineClass::fwcfg_dma_enabled boolean was only used
>>>> by the pc-q35-2.6 and pc-i440fx-2.6 machines, which got
>>>> removed. Remove it and simplify.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>>   include/hw/i386/x86.h | 2 --
>>>>   hw/i386/microvm.c     | 3 ---
>>>>   hw/i386/multiboot.c   | 7 +------
>>>>   hw/i386/x86-common.c  | 3 +--
>>>>   hw/i386/x86.c         | 2 --
>>>>   5 files changed, 2 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
>>>> index fc460b82f82..29d37af11e6 100644
>>>> --- a/include/hw/i386/x86.h
>>>> +++ b/include/hw/i386/x86.h
>>>> @@ -29,8 +29,6 @@
>>>>   struct X86MachineClass {
>>>>       MachineClass parent;
>>>> -    /* use DMA capable linuxboot option rom */
>>>> -    bool fwcfg_dma_enabled;
>>>>       /* CPU and apic information: */
>>>>       bool apic_xrupt_override;
>>>>   };
>>
>>
>>>> diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
>>>> index 6e6b96bc345..bfa7e8f1e83 100644
>>>> --- a/hw/i386/multiboot.c
>>>> +++ b/hw/i386/multiboot.c
>>>> @@ -153,7 +153,6 @@ int load_multiboot(X86MachineState *x86ms,
>>>>                      int kernel_file_size,
>>>>                      uint8_t *header)
>>>>   {
>>>> -    bool multiboot_dma_enabled = X86_MACHINE_GET_CLASS(x86ms)- 
>>>> >fwcfg_dma_enabled;
>>>>       int i, is_multiboot = 0;
>>>>       uint32_t flags = 0;
>>>>       uint32_t mh_entry_addr;
>>>> @@ -402,11 +401,7 @@ int load_multiboot(X86MachineState *x86ms,
>>>>       fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, mb_bootinfo_data,
>>>>                        sizeof(bootinfo));
>>>> -    if (multiboot_dma_enabled) {
>>>> -        option_rom[nb_option_roms].name = "multiboot_dma.bin";
>>>> -    } else {
>>>> -        option_rom[nb_option_roms].name = "multiboot.bin";
>>>> -    }
>>>> +    option_rom[nb_option_roms].name = "multiboot_dma.bin";
>>>
>>> Question: now that all machines support DMA-capable fw_cfg, does that 
>>> mean that the non-DMA options roms above can also be removed?
>>
>> All x86 machines, but there are still 2 not supporting it: HPPA and
>> MIPS Loongson-3:
>>
>> hw/hppa/machine.c:204:    fw_cfg = fw_cfg_init_mem(addr, addr + 4);
>>
>> hw/mips/loongson3_virt.c:289:    fw_cfg = 
>> fw_cfg_init_mem_wide(cfg_addr, cfg_addr + 8, 8, 0, NULL);
>>
> 
> But these don't use "multiboot.bin", do they? So I think you could 
> remove pc-bios/multiboot.bin now from the repo?
> 
> Same question for "linuxboot.bin" : All users in hw/i386 seem to enable 
> DMA, so fw_cfg_dma_enabled() should always return true here? If so, I 
> think the normal "linuxboot.bin" could go away, too?

You are right!