Message ID | 20210426193520.4115528-7-philmd@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | hw/nvram/fw_cfg: Do not build device if not needed (Spring cleanup) | expand |
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. Thanks Laszlo > > diff --git a/hw/arm/fw_cfg.c b/hw/arm/fw_cfg.c > new file mode 100644 > index 00000000000..de2bca9c76c > --- /dev/null > +++ b/hw/arm/fw_cfg.c > @@ -0,0 +1,19 @@ > +/* > + * QEMU fw_cfg helpers (ARM specific) > + * > + * Copyright (c) 2021 Red Hat, Inc. > + * > + * Author: > + * Philippe Mathieu-Daudé <philmd@redhat.com> > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + */ > + > +#include "qemu/osdep.h" > +#include "hw/mips/fw_cfg.h" > +#include "hw/nvram/fw_cfg.h" > + > +const char *fw_cfg_arch_key_name(uint16_t key) > +{ > + return NULL; > +} > diff --git a/hw/hppa/fw_cfg.c b/hw/hppa/fw_cfg.c > new file mode 100644 > index 00000000000..322b03068c7 > --- /dev/null > +++ b/hw/hppa/fw_cfg.c > @@ -0,0 +1,19 @@ > +/* > + * QEMU fw_cfg helpers (HPPA specific) > + * > + * Copyright (c) 2021 Red Hat, Inc. > + * > + * Author: > + * Philippe Mathieu-Daudé <philmd@redhat.com> > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + */ > + > +#include "qemu/osdep.h" > +#include "hw/mips/fw_cfg.h" > +#include "hw/nvram/fw_cfg.h" > + > +const char *fw_cfg_arch_key_name(uint16_t key) > +{ > + return NULL; > +} > diff --git a/hw/riscv/fw_cfg.c b/hw/riscv/fw_cfg.c > new file mode 100644 > index 00000000000..8e3d2a8bdea > --- /dev/null > +++ b/hw/riscv/fw_cfg.c > @@ -0,0 +1,19 @@ > +/* > + * QEMU fw_cfg helpers (RISC-V specific) > + * > + * Copyright (c) 2021 Red Hat, Inc. > + * > + * Author: > + * Philippe Mathieu-Daudé <philmd@redhat.com> > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + */ > + > +#include "qemu/osdep.h" > +#include "hw/mips/fw_cfg.h" > +#include "hw/nvram/fw_cfg.h" > + > +const char *fw_cfg_arch_key_name(uint16_t key) > +{ > + return NULL; > +} > diff --git a/MAINTAINERS b/MAINTAINERS > index 36055f14c59..ab8f030d4c0 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2162,7 +2162,7 @@ R: Laszlo Ersek <lersek@redhat.com> > R: Gerd Hoffmann <kraxel@redhat.com> > S: Supported > F: docs/specs/fw_cfg.txt > -F: hw/nvram/fw_cfg*.c > +F: hw/*/fw_cfg*.c > F: stubs/fw_cfg.c > F: include/hw/nvram/fw_cfg.h > F: include/standard-headers/linux/qemu_fw_cfg.h > diff --git a/hw/arm/meson.build b/hw/arm/meson.build > index be39117b9b6..fd278de916f 100644 > --- a/hw/arm/meson.build > +++ b/hw/arm/meson.build > @@ -1,6 +1,7 @@ > arm_ss = ss.source_set() > arm_ss.add(files('boot.c'), fdt) > arm_ss.add(when: 'CONFIG_PLATFORM_BUS', if_true: files('sysbus-fdt.c')) > +arm_ss.add(when: 'CONFIG_FW_CFG', if_true: files('fw_cfg.c')) > arm_ss.add(when: 'CONFIG_ARM_VIRT', if_true: files('virt.c')) > arm_ss.add(when: 'CONFIG_ACPI', if_true: files('virt-acpi-build.c')) > arm_ss.add(when: 'CONFIG_DIGIC', if_true: files('digic_boards.c')) > diff --git a/hw/hppa/meson.build b/hw/hppa/meson.build > index 1deae83aee8..10494cc24b7 100644 > --- a/hw/hppa/meson.build > +++ b/hw/hppa/meson.build > @@ -1,4 +1,5 @@ > hppa_ss = ss.source_set() > hppa_ss.add(when: 'CONFIG_DINO', if_true: files('pci.c', 'machine.c', 'dino.c', 'lasi.c')) > +hppa_ss.add(when: 'CONFIG_FW_CFG', if_true: files('fw_cfg.c')) > > hw_arch += {'hppa': hppa_ss} > diff --git a/hw/riscv/meson.build b/hw/riscv/meson.build > index 275c0f7eb7c..ab4d3adb924 100644 > --- a/hw/riscv/meson.build > +++ b/hw/riscv/meson.build > @@ -8,5 +8,6 @@ > riscv_ss.add(when: 'CONFIG_SIFIVE_U', if_true: files('sifive_u.c')) > riscv_ss.add(when: 'CONFIG_SPIKE', if_true: files('spike.c')) > riscv_ss.add(when: 'CONFIG_MICROCHIP_PFSOC', if_true: files('microchip_pfsoc.c')) > +riscv_ss.add(when: 'CONFIG_FW_CFG', if_true: files('fw_cfg.c')) > > hw_arch += {'riscv': riscv_ss} >
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.
diff --git a/hw/arm/fw_cfg.c b/hw/arm/fw_cfg.c new file mode 100644 index 00000000000..de2bca9c76c --- /dev/null +++ b/hw/arm/fw_cfg.c @@ -0,0 +1,19 @@ +/* + * QEMU fw_cfg helpers (ARM specific) + * + * Copyright (c) 2021 Red Hat, Inc. + * + * Author: + * Philippe Mathieu-Daudé <philmd@redhat.com> + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" +#include "hw/mips/fw_cfg.h" +#include "hw/nvram/fw_cfg.h" + +const char *fw_cfg_arch_key_name(uint16_t key) +{ + return NULL; +} diff --git a/hw/hppa/fw_cfg.c b/hw/hppa/fw_cfg.c new file mode 100644 index 00000000000..322b03068c7 --- /dev/null +++ b/hw/hppa/fw_cfg.c @@ -0,0 +1,19 @@ +/* + * QEMU fw_cfg helpers (HPPA specific) + * + * Copyright (c) 2021 Red Hat, Inc. + * + * Author: + * Philippe Mathieu-Daudé <philmd@redhat.com> + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" +#include "hw/mips/fw_cfg.h" +#include "hw/nvram/fw_cfg.h" + +const char *fw_cfg_arch_key_name(uint16_t key) +{ + return NULL; +} diff --git a/hw/riscv/fw_cfg.c b/hw/riscv/fw_cfg.c new file mode 100644 index 00000000000..8e3d2a8bdea --- /dev/null +++ b/hw/riscv/fw_cfg.c @@ -0,0 +1,19 @@ +/* + * QEMU fw_cfg helpers (RISC-V specific) + * + * Copyright (c) 2021 Red Hat, Inc. + * + * Author: + * Philippe Mathieu-Daudé <philmd@redhat.com> + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" +#include "hw/mips/fw_cfg.h" +#include "hw/nvram/fw_cfg.h" + +const char *fw_cfg_arch_key_name(uint16_t key) +{ + return NULL; +} diff --git a/MAINTAINERS b/MAINTAINERS index 36055f14c59..ab8f030d4c0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2162,7 +2162,7 @@ R: Laszlo Ersek <lersek@redhat.com> R: Gerd Hoffmann <kraxel@redhat.com> S: Supported F: docs/specs/fw_cfg.txt -F: hw/nvram/fw_cfg*.c +F: hw/*/fw_cfg*.c F: stubs/fw_cfg.c F: include/hw/nvram/fw_cfg.h F: include/standard-headers/linux/qemu_fw_cfg.h diff --git a/hw/arm/meson.build b/hw/arm/meson.build index be39117b9b6..fd278de916f 100644 --- a/hw/arm/meson.build +++ b/hw/arm/meson.build @@ -1,6 +1,7 @@ arm_ss = ss.source_set() arm_ss.add(files('boot.c'), fdt) arm_ss.add(when: 'CONFIG_PLATFORM_BUS', if_true: files('sysbus-fdt.c')) +arm_ss.add(when: 'CONFIG_FW_CFG', if_true: files('fw_cfg.c')) arm_ss.add(when: 'CONFIG_ARM_VIRT', if_true: files('virt.c')) arm_ss.add(when: 'CONFIG_ACPI', if_true: files('virt-acpi-build.c')) arm_ss.add(when: 'CONFIG_DIGIC', if_true: files('digic_boards.c')) diff --git a/hw/hppa/meson.build b/hw/hppa/meson.build index 1deae83aee8..10494cc24b7 100644 --- a/hw/hppa/meson.build +++ b/hw/hppa/meson.build @@ -1,4 +1,5 @@ hppa_ss = ss.source_set() hppa_ss.add(when: 'CONFIG_DINO', if_true: files('pci.c', 'machine.c', 'dino.c', 'lasi.c')) +hppa_ss.add(when: 'CONFIG_FW_CFG', if_true: files('fw_cfg.c')) hw_arch += {'hppa': hppa_ss} diff --git a/hw/riscv/meson.build b/hw/riscv/meson.build index 275c0f7eb7c..ab4d3adb924 100644 --- a/hw/riscv/meson.build +++ b/hw/riscv/meson.build @@ -8,5 +8,6 @@ riscv_ss.add(when: 'CONFIG_SIFIVE_U', if_true: files('sifive_u.c')) riscv_ss.add(when: 'CONFIG_SPIKE', if_true: files('spike.c')) riscv_ss.add(when: 'CONFIG_MICROCHIP_PFSOC', if_true: files('microchip_pfsoc.c')) +riscv_ss.add(when: 'CONFIG_FW_CFG', if_true: files('fw_cfg.c')) hw_arch += {'riscv': riscv_ss}
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