| Message ID | 20210511155354.3069141-4-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on' | expand |
On Tue, May 11, 2021 at 05:53:52PM +0200, Philippe Mathieu-Daudé wrote: > Since commit fea35ca4b8e ("ppc/spapr: Receive and store device > tree blob from SLOF") the pSeries machine depends on the libfdt > fdt_check_full() call, which is available in libfdt v1.4.7. > > Add the corresponding Kconfig dependency. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> I don't love making this conditional. Pseries is by far the best tested and most widely used ppc machine type, so it seems like it would break expectations to not compile this in rather than giving an error saying you need a newer libfdt. > --- > hw/ppc/Kconfig | 1 + > hw/ppc/meson.build | 4 ++-- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig > index 66e0b15d9ef..3935b73456f 100644 > --- a/hw/ppc/Kconfig > +++ b/hw/ppc/Kconfig > @@ -1,5 +1,6 @@ > config PSERIES > bool > + depends on FDT > imply PCI_DEVICES > imply TEST_DEVICES > imply VIRTIO_VGA > diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build > index 86d6f379d1c..e82a6b4105b 100644 > --- a/hw/ppc/meson.build > +++ b/hw/ppc/meson.build > @@ -9,7 +9,7 @@ > ppc_ss.add(when: 'CONFIG_FW_CFG_PPC', if_true: files('fw_cfg.c')) > > # IBM pSeries (sPAPR) > -ppc_ss.add(when: 'CONFIG_PSERIES', if_true: files( > +ppc_ss.add(when: 'CONFIG_PSERIES', if_true: [files( > 'spapr.c', > 'spapr_caps.c', > 'spapr_vio.c', > @@ -28,7 +28,7 @@ > 'spapr_rtas_ddw.c', > 'spapr_numa.c', > 'pef.c', > -)) > +), fdt]) > ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c')) > ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files( > 'spapr_pci_vfio.c',
On 11/05/21 17:53, Philippe Mathieu-Daudé wrote: > Since commit fea35ca4b8e ("ppc/spapr: Receive and store device > tree blob from SLOF") the pSeries machine depends on the libfdt > fdt_check_full() call, which is available in libfdt v1.4.7. > > Add the corresponding Kconfig dependency. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> This is not the only one though. In particular, there should be a "depends on" also for MIPS_BOSTON (hw/mips), E500 (hw/ppc), POWERNV, PPC440 (hw/ppc), (hw/ppc), SAM460EX (hw/ppc), VIRTEX (hw/ppc), RX_GDBSIM (hw/rx), XTENSA_XTFPGA (hw/xtensa). Once you do this, TARGET_NEED_FDT can go away for PPC, MIPS and. The remaining ones use fdt functions in hw/*/boot.c so they need libfdt unconditionally RX (and TARGET_NEED_FDT should be added to default-configs/targets/nios2-softmmu.mak for the same reason). Paolo > --- > hw/ppc/Kconfig | 1 + > hw/ppc/meson.build | 4 ++-- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig > index 66e0b15d9ef..3935b73456f 100644 > --- a/hw/ppc/Kconfig > +++ b/hw/ppc/Kconfig > @@ -1,5 +1,6 @@ > config PSERIES > bool > + depends on FDT > imply PCI_DEVICES > imply TEST_DEVICES > imply VIRTIO_VGA > diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build > index 86d6f379d1c..e82a6b4105b 100644 > --- a/hw/ppc/meson.build > +++ b/hw/ppc/meson.build > @@ -9,7 +9,7 @@ > ppc_ss.add(when: 'CONFIG_FW_CFG_PPC', if_true: files('fw_cfg.c')) > > # IBM pSeries (sPAPR) > -ppc_ss.add(when: 'CONFIG_PSERIES', if_true: files( > +ppc_ss.add(when: 'CONFIG_PSERIES', if_true: [files( > 'spapr.c', > 'spapr_caps.c', > 'spapr_vio.c', > @@ -28,7 +28,7 @@ > 'spapr_rtas_ddw.c', > 'spapr_numa.c', > 'pef.c', > -)) > +), fdt]) > ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c')) > ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files( > 'spapr_pci_vfio.c', >
On 12/05/21 04:27, David Gibson wrote: > On Tue, May 11, 2021 at 05:53:52PM +0200, Philippe Mathieu-Daudé wrote: >> Since commit fea35ca4b8e ("ppc/spapr: Receive and store device >> tree blob from SLOF") the pSeries machine depends on the libfdt >> fdt_check_full() call, which is available in libfdt v1.4.7. >> >> Add the corresponding Kconfig dependency. >> >> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com> > I don't love making this conditional. Pseries is by far the best > tested and most widely used ppc machine type, so it seems like it > would break expectations to not compile this in rather than giving an > error saying you need a newer libfdt. It's not conditional; if libfdt is not found, scripts/minikconf.py will tell you about the contradiction between CONFIG_PSERIES=y and "CONFIG_PSERIES depends on FDT". So we still have the same "fdt_required" logic that is already in meson.build, but expressed as Kconfig rules instead of a random line in default-configs/targets. Paolo
On 5/12/21 9:45 AM, Paolo Bonzini wrote: > On 11/05/21 17:53, Philippe Mathieu-Daudé wrote: >> Since commit fea35ca4b8e ("ppc/spapr: Receive and store device >> tree blob from SLOF") the pSeries machine depends on the libfdt >> fdt_check_full() call, which is available in libfdt v1.4.7. >> >> Add the corresponding Kconfig dependency. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > This is not the only one though. In particular, there should be a > "depends on" also for MIPS_BOSTON (hw/mips), E500 (hw/ppc), POWERNV, > PPC440 (hw/ppc), (hw/ppc), SAM460EX (hw/ppc), VIRTEX (hw/ppc), RX_GDBSIM > (hw/rx), XTENSA_XTFPGA (hw/xtensa). > > Once you do this, TARGET_NEED_FDT can go away for PPC, MIPS and. The > remaining ones use fdt functions in hw/*/boot.c so they need libfdt > unconditionally RX (and TARGET_NEED_FDT should be added to > default-configs/targets/nios2-softmmu.mak for the same reason). Got it, thanks!
On Wed, May 12, 2021 at 10:01:20AM +0200, Paolo Bonzini wrote: > On 12/05/21 04:27, David Gibson wrote: > > On Tue, May 11, 2021 at 05:53:52PM +0200, Philippe Mathieu-Daudé wrote: > > > Since commit fea35ca4b8e ("ppc/spapr: Receive and store device > > > tree blob from SLOF") the pSeries machine depends on the libfdt > > > fdt_check_full() call, which is available in libfdt v1.4.7. > > > > > > Add the corresponding Kconfig dependency. > > > > > > Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com> > > I don't love making this conditional. Pseries is by far the best > > tested and most widely used ppc machine type, so it seems like it > > would break expectations to not compile this in rather than giving an > > error saying you need a newer libfdt. > > It's not conditional; if libfdt is not found, scripts/minikconf.py will tell > you about the contradiction between CONFIG_PSERIES=y and "CONFIG_PSERIES > depends on FDT". > > So we still have the same "fdt_required" logic that is already in > meson.build, but expressed as Kconfig rules instead of a random line in > default-configs/targets. Oh, ok.
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig index 66e0b15d9ef..3935b73456f 100644 --- a/hw/ppc/Kconfig +++ b/hw/ppc/Kconfig @@ -1,5 +1,6 @@ config PSERIES bool + depends on FDT imply PCI_DEVICES imply TEST_DEVICES imply VIRTIO_VGA diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build index 86d6f379d1c..e82a6b4105b 100644 --- a/hw/ppc/meson.build +++ b/hw/ppc/meson.build @@ -9,7 +9,7 @@ ppc_ss.add(when: 'CONFIG_FW_CFG_PPC', if_true: files('fw_cfg.c')) # IBM pSeries (sPAPR) -ppc_ss.add(when: 'CONFIG_PSERIES', if_true: files( +ppc_ss.add(when: 'CONFIG_PSERIES', if_true: [files( 'spapr.c', 'spapr_caps.c', 'spapr_vio.c', @@ -28,7 +28,7 @@ 'spapr_rtas_ddw.c', 'spapr_numa.c', 'pef.c', -)) +), fdt]) ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c')) ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files( 'spapr_pci_vfio.c',
Since commit fea35ca4b8e ("ppc/spapr: Receive and store device tree blob from SLOF") the pSeries machine depends on the libfdt fdt_check_full() call, which is available in libfdt v1.4.7. Add the corresponding Kconfig dependency. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- hw/ppc/Kconfig | 1 + hw/ppc/meson.build | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)