Re: [PATCH 6/7] hw/{arm,hppa,riscv}: Add fw_cfg arch-specific stub

[Philippe Mathieu-Daudé]

On 4/28/21 6:44 PM, Laszlo Ersek wrote:
> On 04/26/21 21:35, Philippe Mathieu-Daudé wrote:
>> The ARM, HPPA and RISC-V architectures don't declare any fw_cfg
>> specific key. To simplify the buildsys machinery and allow building
>> QEMU without the fw_cfg device (in the next commit), first add a
>> per-architecture empty stub defining the fw_cfg_arch_key_name().
>>
>> Update the MAINTAINERS section to cover the various target-specific
>> fw_cfg.c files.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  hw/arm/fw_cfg.c      | 19 +++++++++++++++++++
>>  hw/hppa/fw_cfg.c     | 19 +++++++++++++++++++
>>  hw/riscv/fw_cfg.c    | 19 +++++++++++++++++++
>>  MAINTAINERS          |  2 +-
>>  hw/arm/meson.build   |  1 +
>>  hw/hppa/meson.build  |  1 +
>>  hw/riscv/meson.build |  1 +
>>  7 files changed, 61 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/arm/fw_cfg.c
>>  create mode 100644 hw/hppa/fw_cfg.c
>>  create mode 100644 hw/riscv/fw_cfg.c
> 
> So, I haven't commented on the Kconfig symbol wrangling yet (my comment
> would be a blanket "Acked-by" anyway... sorry, not really my cup of
> tea), but at this point:
> 
> I don't understand why we need to add *more code* (stubs / boilerplate)
> if our goal is (apparently) to build QEMU with *fewer* devices.
> 
> Sorry for being dense. My total knowledge about stubs in QEMU is this:
> for some QMP methods (and for some QGA methods, dependent on OS), we
> need stubs. When they are invoked, they report "sorry, not implemented".
> That's it: all I know about stubs.
> 
> So... the commit message here says "simplify the buildsys", and the next
> commit message says, paraphrased, "don't build fw_cfg unless we need it"
> -- but why does that require more C-language code? It seems like we have
> some function *calls* that shouldn't exist in an fw-cfg-less machine, in
> the first place.
> 
> Again, sorry, I'm totally dense on this.

Eh no problem, I don't like this neither.

If you don't mind I'll reply in the patch 7/7.