* [RFC PATCH 1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on'
2021-05-11 15:53 [RFC PATCH 0/5] buildsys: Do not use internal fdt library when asked for the system one Philippe Mathieu-Daudé
@ 2021-05-11 15:53 ` Philippe Mathieu-Daudé
2021-05-12 2:24 ` David Gibson
2021-05-12 8:02 ` Paolo Bonzini
2021-05-11 15:53 ` [RFC PATCH 2/5] Kconfig: Declare 'FDT' host symbol Philippe Mathieu-Daudé
` (4 subsequent siblings)
5 siblings, 2 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-11 15:53 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Thomas Huth, Eduardo Habkost, Michael S. Tsirkin,
Alexey Kardashevskiy, Richard Henderson, Greg Kurz,
Igor Mammedov, open list:ARM TCG CPUs, qemu-ppc,
John Paul Adrian Glaubitz, Paolo Bonzini,
Philippe Mathieu-Daudé,
David Gibson
Per the kconfig.rst:
A device should be listed [...] ``imply`` if (depending on
the QEMU command line) the board may or may not be started
without it.
This is the case with the NVDIMM device, so use the 'imply'
weak reverse dependency to select the symbol.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
default-configs/devices/ppc64-softmmu.mak | 1 -
hw/arm/Kconfig | 1 +
hw/i386/Kconfig | 1 +
hw/mem/Kconfig | 2 --
hw/ppc/Kconfig | 1 +
5 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/default-configs/devices/ppc64-softmmu.mak b/default-configs/devices/ppc64-softmmu.mak
index ae0841fa3a1..cca52665d90 100644
--- a/default-configs/devices/ppc64-softmmu.mak
+++ b/default-configs/devices/ppc64-softmmu.mak
@@ -8,4 +8,3 @@ CONFIG_POWERNV=y
# For pSeries
CONFIG_PSERIES=y
-CONFIG_NVDIMM=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index b887f6a5b17..67723d9ea6a 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -6,6 +6,7 @@ config ARM_VIRT
imply VFIO_PLATFORM
imply VFIO_XGMAC
imply TPM_TIS_SYSBUS
+ imply NVDIMM
select ARM_GIC
select ACPI
select ARM_SMMUV3
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 7f91f30877f..66838fa397b 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -23,6 +23,7 @@ config PC
imply TPM_TIS_ISA
imply VGA_PCI
imply VIRTIO_VGA
+ imply NVDIMM
select FDC
select I8259
select I8254
diff --git a/hw/mem/Kconfig b/hw/mem/Kconfig
index a0ef2cf648e..8b19fdc49f1 100644
--- a/hw/mem/Kconfig
+++ b/hw/mem/Kconfig
@@ -7,6 +7,4 @@ config MEM_DEVICE
config NVDIMM
bool
- default y
- depends on (PC || PSERIES || ARM_VIRT)
select MEM_DEVICE
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index e51e0e5e5ac..66e0b15d9ef 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -3,6 +3,7 @@ config PSERIES
imply PCI_DEVICES
imply TEST_DEVICES
imply VIRTIO_VGA
+ imply NVDIMM
select DIMM
select PCI
select SPAPR_VSCSI
--
2.26.3
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [RFC PATCH 1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on'
2021-05-11 15:53 ` [RFC PATCH 1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on' Philippe Mathieu-Daudé
@ 2021-05-12 2:24 ` David Gibson
2021-05-12 3:57 ` Philippe Mathieu-Daudé
2021-05-12 8:02 ` Paolo Bonzini
1 sibling, 1 reply; 32+ messages in thread
From: David Gibson @ 2021-05-12 2:24 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Thomas Huth, Eduardo Habkost, Michael S. Tsirkin,
Alexey Kardashevskiy, Richard Henderson, qemu-devel, Greg Kurz,
Igor Mammedov, open list:ARM TCG CPUs, qemu-ppc,
John Paul Adrian Glaubitz, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 2728 bytes --]
On Tue, May 11, 2021 at 05:53:50PM +0200, Philippe Mathieu-Daudé wrote:
> Per the kconfig.rst:
>
> A device should be listed [...] ``imply`` if (depending on
> the QEMU command line) the board may or may not be started
> without it.
>
> This is the case with the NVDIMM device, so use the 'imply'
> weak reverse dependency to select the symbol.
Uh.. It should definitely be possible to start a pseries machine
without NVDIMM. I would have guessed the same for PC.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> default-configs/devices/ppc64-softmmu.mak | 1 -
> hw/arm/Kconfig | 1 +
> hw/i386/Kconfig | 1 +
> hw/mem/Kconfig | 2 --
> hw/ppc/Kconfig | 1 +
> 5 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/default-configs/devices/ppc64-softmmu.mak b/default-configs/devices/ppc64-softmmu.mak
> index ae0841fa3a1..cca52665d90 100644
> --- a/default-configs/devices/ppc64-softmmu.mak
> +++ b/default-configs/devices/ppc64-softmmu.mak
> @@ -8,4 +8,3 @@ CONFIG_POWERNV=y
>
> # For pSeries
> CONFIG_PSERIES=y
> -CONFIG_NVDIMM=y
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index b887f6a5b17..67723d9ea6a 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -6,6 +6,7 @@ config ARM_VIRT
> imply VFIO_PLATFORM
> imply VFIO_XGMAC
> imply TPM_TIS_SYSBUS
> + imply NVDIMM
> select ARM_GIC
> select ACPI
> select ARM_SMMUV3
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index 7f91f30877f..66838fa397b 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -23,6 +23,7 @@ config PC
> imply TPM_TIS_ISA
> imply VGA_PCI
> imply VIRTIO_VGA
> + imply NVDIMM
> select FDC
> select I8259
> select I8254
> diff --git a/hw/mem/Kconfig b/hw/mem/Kconfig
> index a0ef2cf648e..8b19fdc49f1 100644
> --- a/hw/mem/Kconfig
> +++ b/hw/mem/Kconfig
> @@ -7,6 +7,4 @@ config MEM_DEVICE
>
> config NVDIMM
> bool
> - default y
> - depends on (PC || PSERIES || ARM_VIRT)
> select MEM_DEVICE
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index e51e0e5e5ac..66e0b15d9ef 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -3,6 +3,7 @@ config PSERIES
> imply PCI_DEVICES
> imply TEST_DEVICES
> imply VIRTIO_VGA
> + imply NVDIMM
> select DIMM
> select PCI
> select SPAPR_VSCSI
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on'
2021-05-12 2:24 ` David Gibson
@ 2021-05-12 3:57 ` Philippe Mathieu-Daudé
2021-05-12 4:53 ` Thomas Huth
0 siblings, 1 reply; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-12 3:57 UTC (permalink / raw)
To: David Gibson
Cc: Peter Maydell, Thomas Huth, Eduardo Habkost, Michael S. Tsirkin,
Alexey Kardashevskiy, Richard Henderson, qemu-devel, Greg Kurz,
Igor Mammedov, open list:ARM TCG CPUs, qemu-ppc,
John Paul Adrian Glaubitz, Paolo Bonzini
On 5/12/21 4:24 AM, David Gibson wrote:
> On Tue, May 11, 2021 at 05:53:50PM +0200, Philippe Mathieu-Daudé wrote:
>> Per the kconfig.rst:
>>
>> A device should be listed [...] ``imply`` if (depending on
>> the QEMU command line) the board may or may not be started
>> without it.
>>
>> This is the case with the NVDIMM device, so use the 'imply'
>> weak reverse dependency to select the symbol.
>
> Uh.. It should definitely be possible to start a pseries machine
> without NVDIMM. I would have guessed the same for PC.
Yes, this is what this patch does. With it we can build with:
CONFIG_NVDIMM=n
>
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> default-configs/devices/ppc64-softmmu.mak | 1 -
>> hw/arm/Kconfig | 1 +
>> hw/i386/Kconfig | 1 +
>> hw/mem/Kconfig | 2 --
>> hw/ppc/Kconfig | 1 +
>> 5 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/default-configs/devices/ppc64-softmmu.mak b/default-configs/devices/ppc64-softmmu.mak
>> index ae0841fa3a1..cca52665d90 100644
>> --- a/default-configs/devices/ppc64-softmmu.mak
>> +++ b/default-configs/devices/ppc64-softmmu.mak
>> @@ -8,4 +8,3 @@ CONFIG_POWERNV=y
>>
>> # For pSeries
>> CONFIG_PSERIES=y
>> -CONFIG_NVDIMM=y
>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>> index b887f6a5b17..67723d9ea6a 100644
>> --- a/hw/arm/Kconfig
>> +++ b/hw/arm/Kconfig
>> @@ -6,6 +6,7 @@ config ARM_VIRT
>> imply VFIO_PLATFORM
>> imply VFIO_XGMAC
>> imply TPM_TIS_SYSBUS
>> + imply NVDIMM
>> select ARM_GIC
>> select ACPI
>> select ARM_SMMUV3
>> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
>> index 7f91f30877f..66838fa397b 100644
>> --- a/hw/i386/Kconfig
>> +++ b/hw/i386/Kconfig
>> @@ -23,6 +23,7 @@ config PC
>> imply TPM_TIS_ISA
>> imply VGA_PCI
>> imply VIRTIO_VGA
>> + imply NVDIMM
>> select FDC
>> select I8259
>> select I8254
>> diff --git a/hw/mem/Kconfig b/hw/mem/Kconfig
>> index a0ef2cf648e..8b19fdc49f1 100644
>> --- a/hw/mem/Kconfig
>> +++ b/hw/mem/Kconfig
>> @@ -7,6 +7,4 @@ config MEM_DEVICE
>>
>> config NVDIMM
>> bool
>> - default y
>> - depends on (PC || PSERIES || ARM_VIRT)
>> select MEM_DEVICE
>> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
>> index e51e0e5e5ac..66e0b15d9ef 100644
>> --- a/hw/ppc/Kconfig
>> +++ b/hw/ppc/Kconfig
>> @@ -3,6 +3,7 @@ config PSERIES
>> imply PCI_DEVICES
>> imply TEST_DEVICES
>> imply VIRTIO_VGA
>> + imply NVDIMM
>> select DIMM
>> select PCI
>> select SPAPR_VSCSI
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on'
2021-05-12 3:57 ` Philippe Mathieu-Daudé
@ 2021-05-12 4:53 ` Thomas Huth
2021-05-12 5:08 ` Philippe Mathieu-Daudé
2021-05-12 7:02 ` David Gibson
0 siblings, 2 replies; 32+ messages in thread
From: Thomas Huth @ 2021-05-12 4:53 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, David Gibson
Cc: Peter Maydell, Eduardo Habkost, Michael S. Tsirkin,
Alexey Kardashevskiy, Richard Henderson, qemu-devel, Greg Kurz,
Igor Mammedov, open list:ARM TCG CPUs, qemu-ppc,
John Paul Adrian Glaubitz, Paolo Bonzini
On 12/05/2021 05.57, Philippe Mathieu-Daudé wrote:
> On 5/12/21 4:24 AM, David Gibson wrote:
>> On Tue, May 11, 2021 at 05:53:50PM +0200, Philippe Mathieu-Daudé wrote:
>>> Per the kconfig.rst:
>>>
>>> A device should be listed [...] ``imply`` if (depending on
>>> the QEMU command line) the board may or may not be started
>>> without it.
>>>
>>> This is the case with the NVDIMM device, so use the 'imply'
>>> weak reverse dependency to select the symbol.
>>
>> Uh.. It should definitely be possible to start a pseries machine
>> without NVDIMM. I would have guessed the same for PC.
>
> Yes, this is what this patch does. With it we can build with:
> CONFIG_NVDIMM=n
But with "imply" you could end up with a PSERIES that does not have NVDIMM
when also using --without-default-devices, couldn't you? Why don't you use
"select" instead of "imply" ?
Thomas
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on'
2021-05-12 4:53 ` Thomas Huth
@ 2021-05-12 5:08 ` Philippe Mathieu-Daudé
2021-05-12 5:30 ` Thomas Huth
2021-05-12 7:02 ` David Gibson
1 sibling, 1 reply; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-12 5:08 UTC (permalink / raw)
To: Thomas Huth, David Gibson
Cc: Peter Maydell, Eduardo Habkost, Michael S. Tsirkin,
Alexey Kardashevskiy, Richard Henderson, qemu-devel, Greg Kurz,
Igor Mammedov, open list:ARM TCG CPUs, qemu-ppc,
John Paul Adrian Glaubitz, Paolo Bonzini
On 5/12/21 6:53 AM, Thomas Huth wrote:
> On 12/05/2021 05.57, Philippe Mathieu-Daudé wrote:
>> On 5/12/21 4:24 AM, David Gibson wrote:
>>> On Tue, May 11, 2021 at 05:53:50PM +0200, Philippe Mathieu-Daudé wrote:
>>>> Per the kconfig.rst:
>>>>
>>>> A device should be listed [...] ``imply`` if (depending on
>>>> the QEMU command line) the board may or may not be started
>>>> without it.
>>>>
>>>> This is the case with the NVDIMM device, so use the 'imply'
>>>> weak reverse dependency to select the symbol.
>>>
>>> Uh.. It should definitely be possible to start a pseries machine
>>> without NVDIMM. I would have guessed the same for PC.
>>
>> Yes, this is what this patch does. With it we can build with:
>> CONFIG_NVDIMM=n
>
> But with "imply" you could end up with a PSERIES that does not have
> NVDIMM when also using --without-default-devices, couldn't you?
Not currently, because of the CONFIG_NVDIMM=y.
> Why don't you use "select" instead of "imply" ?
Because as David said earlier:
"It should definitely be possible to start a pseries machine
without NVDIMM."
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on'
2021-05-12 5:08 ` Philippe Mathieu-Daudé
@ 2021-05-12 5:30 ` Thomas Huth
0 siblings, 0 replies; 32+ messages in thread
From: Thomas Huth @ 2021-05-12 5:30 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, David Gibson
Cc: Peter Maydell, Eduardo Habkost, Michael S. Tsirkin,
Alexey Kardashevskiy, Richard Henderson, qemu-devel, Greg Kurz,
Igor Mammedov, open list:ARM TCG CPUs, qemu-ppc,
John Paul Adrian Glaubitz, Paolo Bonzini
On 12/05/2021 07.08, Philippe Mathieu-Daudé wrote:
> On 5/12/21 6:53 AM, Thomas Huth wrote:
>> On 12/05/2021 05.57, Philippe Mathieu-Daudé wrote:
>>> On 5/12/21 4:24 AM, David Gibson wrote:
>>>> On Tue, May 11, 2021 at 05:53:50PM +0200, Philippe Mathieu-Daudé wrote:
>>>>> Per the kconfig.rst:
>>>>>
>>>>> A device should be listed [...] ``imply`` if (depending on
>>>>> the QEMU command line) the board may or may not be started
>>>>> without it.
>>>>>
>>>>> This is the case with the NVDIMM device, so use the 'imply'
>>>>> weak reverse dependency to select the symbol.
>>>>
>>>> Uh.. It should definitely be possible to start a pseries machine
>>>> without NVDIMM. I would have guessed the same for PC.
>>>
>>> Yes, this is what this patch does. With it we can build with:
>>> CONFIG_NVDIMM=n
>>
>> But with "imply" you could end up with a PSERIES that does not have
>> NVDIMM when also using --without-default-devices, couldn't you?
>
> Not currently, because of the CONFIG_NVDIMM=y.
>
>> Why don't you use "select" instead of "imply" ?
>
> Because as David said earlier:
>
> "It should definitely be possible to start a pseries machine
> without NVDIMM."
Oops, sorry, looks like I did not have enough coffee yet and misread David's
comment ... patch looks fine, indeed.
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on'
2021-05-12 4:53 ` Thomas Huth
2021-05-12 5:08 ` Philippe Mathieu-Daudé
@ 2021-05-12 7:02 ` David Gibson
2021-05-13 15:20 ` Paolo Bonzini
1 sibling, 1 reply; 32+ messages in thread
From: David Gibson @ 2021-05-12 7:02 UTC (permalink / raw)
To: Thomas Huth
Cc: Peter Maydell, Eduardo Habkost, Michael S. Tsirkin,
Alexey Kardashevskiy, Richard Henderson, qemu-devel, Greg Kurz,
Igor Mammedov, open list:ARM TCG CPUs, qemu-ppc,
John Paul Adrian Glaubitz, Paolo Bonzini,
Philippe Mathieu-Daudé
[-- Attachment #1: Type: text/plain, Size: 1374 bytes --]
On Wed, May 12, 2021 at 06:53:22AM +0200, Thomas Huth wrote:
> On 12/05/2021 05.57, Philippe Mathieu-Daudé wrote:
> > On 5/12/21 4:24 AM, David Gibson wrote:
> > > On Tue, May 11, 2021 at 05:53:50PM +0200, Philippe Mathieu-Daudé wrote:
> > > > Per the kconfig.rst:
> > > >
> > > > A device should be listed [...] ``imply`` if (depending on
> > > > the QEMU command line) the board may or may not be started
> > > > without it.
> > > >
> > > > This is the case with the NVDIMM device, so use the 'imply'
> > > > weak reverse dependency to select the symbol.
> > >
> > > Uh.. It should definitely be possible to start a pseries machine
> > > without NVDIMM. I would have guessed the same for PC.
> >
> > Yes, this is what this patch does. With it we can build with:
> > CONFIG_NVDIMM=n
>
> But with "imply" you could end up with a PSERIES that does not have NVDIMM
> when also using --without-default-devices, couldn't you? Why don't you use
> "select" instead of "imply" ?
Oh.. clearly I misunderstand the semantics of "imply". If we don't
need NVDIMM for PSERIES, why does there need to be any Kconfig
connection between them at all?
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on'
2021-05-12 7:02 ` David Gibson
@ 2021-05-13 15:20 ` Paolo Bonzini
2021-05-13 23:33 ` David Gibson
0 siblings, 1 reply; 32+ messages in thread
From: Paolo Bonzini @ 2021-05-13 15:20 UTC (permalink / raw)
To: David Gibson, Thomas Huth
Cc: Peter Maydell, Eduardo Habkost, Michael S. Tsirkin,
Alexey Kardashevskiy, Richard Henderson, qemu-devel, Greg Kurz,
open list:ARM TCG CPUs, qemu-ppc, John Paul Adrian Glaubitz,
Igor Mammedov, Philippe Mathieu-Daudé
On 12/05/21 09:02, David Gibson wrote:
>> But with "imply" you could end up with a PSERIES that does not have NVDIMM
>> when also using --without-default-devices, couldn't you? Why don't you use
>> "select" instead of "imply" ?
> Oh.. clearly I misunderstand the semantics of "imply". If we don't
> need NVDIMM for PSERIES, why does there need to be any Kconfig
> connection between them at all?
Because you still want it in the binary by default (i.e. unless
--without-default-devices).
Basically,
config PSERIES
imply NVDIMM
is the same as
config NVDIMM
default y if PSERIES
Both of them are a way to say "PSERIES can work with NVDIMM so you want
to include it unless you want some fine tuning". In Linux "imply" is
very rarely used, while in QEMU it's quite common because it keeps the
many per-board defaults close together.
Paolo
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [RFC PATCH 1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on'
2021-05-13 15:20 ` Paolo Bonzini
@ 2021-05-13 23:33 ` David Gibson
0 siblings, 0 replies; 32+ messages in thread
From: David Gibson @ 2021-05-13 23:33 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peter Maydell, Thomas Huth, Eduardo Habkost, Michael S. Tsirkin,
Alexey Kardashevskiy, Richard Henderson, qemu-devel, Greg Kurz,
open list:ARM TCG CPUs, qemu-ppc, John Paul Adrian Glaubitz,
Igor Mammedov, Philippe Mathieu-Daudé
[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]
On Thu, May 13, 2021 at 05:20:06PM +0200, Paolo Bonzini wrote:
> On 12/05/21 09:02, David Gibson wrote:
> > > But with "imply" you could end up with a PSERIES that does not have NVDIMM
> > > when also using --without-default-devices, couldn't you? Why don't you use
> > > "select" instead of "imply" ?
> > Oh.. clearly I misunderstand the semantics of "imply". If we don't
> > need NVDIMM for PSERIES, why does there need to be any Kconfig
> > connection between them at all?
>
> Because you still want it in the binary by default (i.e. unless
> --without-default-devices).
>
> Basically,
>
> config PSERIES
> imply NVDIMM
>
> is the same as
>
> config NVDIMM
> default y if PSERIES
Ah, ok, I get it now. "imply" is a terrible word for this, but ok.
> Both of them are a way to say "PSERIES can work with NVDIMM so you want to
> include it unless you want some fine tuning". In Linux "imply" is very
> rarely used, while in QEMU it's quite common because it keeps the many
> per-board defaults close together.
>
> Paolo
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on'
2021-05-11 15:53 ` [RFC PATCH 1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on' Philippe Mathieu-Daudé
2021-05-12 2:24 ` David Gibson
@ 2021-05-12 8:02 ` Paolo Bonzini
1 sibling, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2021-05-12 8:02 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Peter Maydell, Thomas Huth, Eduardo Habkost, Michael S. Tsirkin,
Alexey Kardashevskiy, Richard Henderson, Greg Kurz,
open list:ARM TCG CPUs, qemu-ppc, John Paul Adrian Glaubitz,
Igor Mammedov, David Gibson
On 11/05/21 17:53, Philippe Mathieu-Daudé wrote:
> Per the kconfig.rst:
>
> A device should be listed [...] ``imply`` if (depending on
> the QEMU command line) the board may or may not be started
> without it.
>
> This is the case with the NVDIMM device, so use the 'imply'
> weak reverse dependency to select the symbol.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Unrelated to the rest, so I've queued this one.
Paolo
> ---
> default-configs/devices/ppc64-softmmu.mak | 1 -
> hw/arm/Kconfig | 1 +
> hw/i386/Kconfig | 1 +
> hw/mem/Kconfig | 2 --
> hw/ppc/Kconfig | 1 +
> 5 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/default-configs/devices/ppc64-softmmu.mak b/default-configs/devices/ppc64-softmmu.mak
> index ae0841fa3a1..cca52665d90 100644
> --- a/default-configs/devices/ppc64-softmmu.mak
> +++ b/default-configs/devices/ppc64-softmmu.mak
> @@ -8,4 +8,3 @@ CONFIG_POWERNV=y
>
> # For pSeries
> CONFIG_PSERIES=y
> -CONFIG_NVDIMM=y
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index b887f6a5b17..67723d9ea6a 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -6,6 +6,7 @@ config ARM_VIRT
> imply VFIO_PLATFORM
> imply VFIO_XGMAC
> imply TPM_TIS_SYSBUS
> + imply NVDIMM
> select ARM_GIC
> select ACPI
> select ARM_SMMUV3
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index 7f91f30877f..66838fa397b 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -23,6 +23,7 @@ config PC
> imply TPM_TIS_ISA
> imply VGA_PCI
> imply VIRTIO_VGA
> + imply NVDIMM
> select FDC
> select I8259
> select I8254
> diff --git a/hw/mem/Kconfig b/hw/mem/Kconfig
> index a0ef2cf648e..8b19fdc49f1 100644
> --- a/hw/mem/Kconfig
> +++ b/hw/mem/Kconfig
> @@ -7,6 +7,4 @@ config MEM_DEVICE
>
> config NVDIMM
> bool
> - default y
> - depends on (PC || PSERIES || ARM_VIRT)
> select MEM_DEVICE
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index e51e0e5e5ac..66e0b15d9ef 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -3,6 +3,7 @@ config PSERIES
> imply PCI_DEVICES
> imply TEST_DEVICES
> imply VIRTIO_VGA
> + imply NVDIMM
> select DIMM
> select PCI
> select SPAPR_VSCSI
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [RFC PATCH 2/5] Kconfig: Declare 'FDT' host symbol
2021-05-11 15:53 [RFC PATCH 0/5] buildsys: Do not use internal fdt library when asked for the system one Philippe Mathieu-Daudé
2021-05-11 15:53 ` [RFC PATCH 1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on' Philippe Mathieu-Daudé
@ 2021-05-11 15:53 ` Philippe Mathieu-Daudé
2021-05-12 7:37 ` Paolo Bonzini
2021-05-11 15:53 ` [RFC PATCH 3/5] hw/ppc: Have pSeries depends on libfdt (via host Kconfig FDT symbol) Philippe Mathieu-Daudé
` (3 subsequent siblings)
5 siblings, 1 reply; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-11 15:53 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Alexey Kardashevskiy, Greg Kurz, qemu-ppc,
John Paul Adrian Glaubitz, Paolo Bonzini,
Philippe Mathieu-Daudé,
David Gibson
The CONFIG_FDT symbol depends on the availability of the
fdt library on the host. To be able to have other symbols
depends on it, declare it symbol in Kconfig.host.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Kconfig.host | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Kconfig.host b/Kconfig.host
index 24255ef4419..0a512696865 100644
--- a/Kconfig.host
+++ b/Kconfig.host
@@ -41,3 +41,6 @@ config PVRDMA
config MULTIPROCESS_ALLOWED
bool
imply MULTIPROCESS
+
+config FDT
+ bool
--
2.26.3
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [RFC PATCH 2/5] Kconfig: Declare 'FDT' host symbol
2021-05-11 15:53 ` [RFC PATCH 2/5] Kconfig: Declare 'FDT' host symbol Philippe Mathieu-Daudé
@ 2021-05-12 7:37 ` Paolo Bonzini
0 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2021-05-12 7:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Alexey Kardashevskiy, Greg Kurz, qemu-ppc,
John Paul Adrian Glaubitz, David Gibson
On 11/05/21 17:53, Philippe Mathieu-Daudé wrote:
> The CONFIG_FDT symbol depends on the availability of the
> fdt library on the host. To be able to have other symbols
> depends on it, declare it symbol in Kconfig.host.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Kconfig.host | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Kconfig.host b/Kconfig.host
> index 24255ef4419..0a512696865 100644
> --- a/Kconfig.host
> +++ b/Kconfig.host
> @@ -41,3 +41,6 @@ config PVRDMA
> config MULTIPROCESS_ALLOWED
> bool
> imply MULTIPROCESS
> +
> +config FDT
> + bool
>
You need to add it to host_kconfig as well, don't you?
Paolo
^ permalink raw reply [flat|nested] 32+ messages in thread
* [RFC PATCH 3/5] hw/ppc: Have pSeries depends on libfdt (via host Kconfig FDT symbol)
2021-05-11 15:53 [RFC PATCH 0/5] buildsys: Do not use internal fdt library when asked for the system one Philippe Mathieu-Daudé
2021-05-11 15:53 ` [RFC PATCH 1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on' Philippe Mathieu-Daudé
2021-05-11 15:53 ` [RFC PATCH 2/5] Kconfig: Declare 'FDT' host symbol Philippe Mathieu-Daudé
@ 2021-05-11 15:53 ` Philippe Mathieu-Daudé
2021-05-12 2:27 ` David Gibson
2021-05-12 7:45 ` Paolo Bonzini
2021-05-11 15:53 ` [RFC PATCH 4/5] hw/ppc/fdt: Drop dependency on libfdt Philippe Mathieu-Daudé
` (2 subsequent siblings)
5 siblings, 2 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-11 15:53 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Alexey Kardashevskiy, Greg Kurz, qemu-ppc,
John Paul Adrian Glaubitz, Paolo Bonzini,
Philippe Mathieu-Daudé,
David Gibson
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(-)
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',
--
2.26.3
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [RFC PATCH 3/5] hw/ppc: Have pSeries depends on libfdt (via host Kconfig FDT symbol)
2021-05-11 15:53 ` [RFC PATCH 3/5] hw/ppc: Have pSeries depends on libfdt (via host Kconfig FDT symbol) Philippe Mathieu-Daudé
@ 2021-05-12 2:27 ` David Gibson
2021-05-12 8:01 ` Paolo Bonzini
2021-05-12 7:45 ` Paolo Bonzini
1 sibling, 1 reply; 32+ messages in thread
From: David Gibson @ 2021-05-12 2:27 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Thomas Huth, Alexey Kardashevskiy, qemu-devel, Greg Kurz,
qemu-ppc, John Paul Adrian Glaubitz, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 1960 bytes --]
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',
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [RFC PATCH 3/5] hw/ppc: Have pSeries depends on libfdt (via host Kconfig FDT symbol)
2021-05-12 2:27 ` David Gibson
@ 2021-05-12 8:01 ` Paolo Bonzini
2021-05-13 3:46 ` David Gibson
0 siblings, 1 reply; 32+ messages in thread
From: Paolo Bonzini @ 2021-05-12 8:01 UTC (permalink / raw)
To: David Gibson, Philippe Mathieu-Daudé
Cc: Thomas Huth, Alexey Kardashevskiy, qemu-devel, Greg Kurz,
qemu-ppc, John Paul Adrian Glaubitz
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
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [RFC PATCH 3/5] hw/ppc: Have pSeries depends on libfdt (via host Kconfig FDT symbol)
2021-05-12 8:01 ` Paolo Bonzini
@ 2021-05-13 3:46 ` David Gibson
0 siblings, 0 replies; 32+ messages in thread
From: David Gibson @ 2021-05-13 3:46 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Thomas Huth, Alexey Kardashevskiy, qemu-devel, Greg Kurz,
qemu-ppc, John Paul Adrian Glaubitz, Philippe Mathieu-Daudé
[-- Attachment #1: Type: text/plain, Size: 1343 bytes --]
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.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 3/5] hw/ppc: Have pSeries depends on libfdt (via host Kconfig FDT symbol)
2021-05-11 15:53 ` [RFC PATCH 3/5] hw/ppc: Have pSeries depends on libfdt (via host Kconfig FDT symbol) Philippe Mathieu-Daudé
2021-05-12 2:27 ` David Gibson
@ 2021-05-12 7:45 ` Paolo Bonzini
2021-05-12 8:27 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 32+ messages in thread
From: Paolo Bonzini @ 2021-05-12 7:45 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Alexey Kardashevskiy, Greg Kurz, qemu-ppc,
John Paul Adrian Glaubitz, David Gibson
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',
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [RFC PATCH 3/5] hw/ppc: Have pSeries depends on libfdt (via host Kconfig FDT symbol)
2021-05-12 7:45 ` Paolo Bonzini
@ 2021-05-12 8:27 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-12 8:27 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
Cc: Thomas Huth, Alexey Kardashevskiy, Greg Kurz, qemu-ppc,
John Paul Adrian Glaubitz, David Gibson
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!
^ permalink raw reply [flat|nested] 32+ messages in thread
* [RFC PATCH 4/5] hw/ppc/fdt: Drop dependency on libfdt
2021-05-11 15:53 [RFC PATCH 0/5] buildsys: Do not use internal fdt library when asked for the system one Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2021-05-11 15:53 ` [RFC PATCH 3/5] hw/ppc: Have pSeries depends on libfdt (via host Kconfig FDT symbol) Philippe Mathieu-Daudé
@ 2021-05-11 15:53 ` Philippe Mathieu-Daudé
2021-05-12 2:30 ` David Gibson
2021-05-11 15:53 ` [RFC PATCH 5/5] meson: Do not use internal fdt library if user asked for the system one Philippe Mathieu-Daudé
2021-05-11 15:57 ` [RFC PATCH 0/5] buildsys: Do not use internal fdt library when " Philippe Mathieu-Daudé
5 siblings, 1 reply; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-11 15:53 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Alexey Kardashevskiy, Greg Kurz, qemu-ppc,
John Paul Adrian Glaubitz, Paolo Bonzini,
Philippe Mathieu-Daudé,
David Gibson
hw/ppc/fdt.c defines the ppc_create_page_sizes_prop() function,
which is unrelated to the libfdt. Remove the incorrect library
dependency on the file.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/ppc/meson.build | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index e82a6b4105b..580e6e42c8a 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -3,9 +3,9 @@
'ppc.c',
'ppc_booke.c',
))
-ppc_ss.add(when: 'CONFIG_FDT_PPC', if_true: [files(
+ppc_ss.add(when: 'CONFIG_FDT_PPC', if_true: files(
'fdt.c',
-), fdt])
+))
ppc_ss.add(when: 'CONFIG_FW_CFG_PPC', if_true: files('fw_cfg.c'))
# IBM pSeries (sPAPR)
--
2.26.3
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [RFC PATCH 4/5] hw/ppc/fdt: Drop dependency on libfdt
2021-05-11 15:53 ` [RFC PATCH 4/5] hw/ppc/fdt: Drop dependency on libfdt Philippe Mathieu-Daudé
@ 2021-05-12 2:30 ` David Gibson
2021-05-12 7:59 ` Paolo Bonzini
0 siblings, 1 reply; 32+ messages in thread
From: David Gibson @ 2021-05-12 2:30 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Thomas Huth, Alexey Kardashevskiy, qemu-devel, Greg Kurz,
qemu-ppc, John Paul Adrian Glaubitz, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 1373 bytes --]
On Tue, May 11, 2021 at 05:53:53PM +0200, Philippe Mathieu-Daudé wrote:
> hw/ppc/fdt.c defines the ppc_create_page_sizes_prop() function,
> which is unrelated to the libfdt. Remove the incorrect library
> dependency on the file.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
This is definitely wrong as it stands. AFAICT this doesn't add a
build of hw/ppc/fdt.c anywhere, but it is definitely needed by both
pseries and powernv machine types, who select FDT_PPC for this exact
reason.
I will grant you that it is badly named. It is in fact related to
libfdt, just rather indirectly.
> ---
> hw/ppc/meson.build | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> index e82a6b4105b..580e6e42c8a 100644
> --- a/hw/ppc/meson.build
> +++ b/hw/ppc/meson.build
> @@ -3,9 +3,9 @@
> 'ppc.c',
> 'ppc_booke.c',
> ))
> -ppc_ss.add(when: 'CONFIG_FDT_PPC', if_true: [files(
> +ppc_ss.add(when: 'CONFIG_FDT_PPC', if_true: files(
> 'fdt.c',
> -), fdt])
> +))
> ppc_ss.add(when: 'CONFIG_FW_CFG_PPC', if_true: files('fw_cfg.c'))
>
> # IBM pSeries (sPAPR)
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [RFC PATCH 4/5] hw/ppc/fdt: Drop dependency on libfdt
2021-05-12 2:30 ` David Gibson
@ 2021-05-12 7:59 ` Paolo Bonzini
2021-05-13 3:46 ` David Gibson
0 siblings, 1 reply; 32+ messages in thread
From: Paolo Bonzini @ 2021-05-12 7:59 UTC (permalink / raw)
To: David Gibson, Philippe Mathieu-Daudé
Cc: Thomas Huth, Alexey Kardashevskiy, qemu-devel, Greg Kurz,
qemu-ppc, John Paul Adrian Glaubitz
On 12/05/21 04:30, David Gibson wrote:
> On Tue, May 11, 2021 at 05:53:53PM +0200, Philippe Mathieu-Daudé wrote:
>> hw/ppc/fdt.c defines the ppc_create_page_sizes_prop() function,
>> which is unrelated to the libfdt. Remove the incorrect library
>> dependency on the file.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> This is definitely wrong as it stands. AFAICT this doesn't add a
> build of hw/ppc/fdt.c anywhere, but it is definitely needed by both
> pseries and powernv machine types, who select FDT_PPC for this exact
> reason.
>
> I will grant you that it is badly named. It is in fact related to
> libfdt, just rather indirectly.
The patch makes sense in general. The file is only needed for pseries
and powernv, not for e.g. e500 which does need fdt.
I would get rid of FDT_PPC completely. First, before patch 3, you can
move fdt.c to PSERIES and POWERNV (it's too small to need its own
Kconfig symbol) and only leave
ppc_ss.add(when: 'CONFIG_FDT_PPC', if_true: fdt)
Since you are at it, remove the silly #ifdef TARGET_PPC64 in the
hw/ppc/fdt.c file.
Then in patch 3:
- add to Kconfig.host
config FDT
bool
config LIBFDT
bool
depends on FDT
- for all the boards I listed in my review, add "select LIBFDT" in
addition to "depends on FDT".
- add to meson.build
softmmu_ss.add(when: 'CONFIG_LIBFDT', if_true: fdt)
Paolo
>> ---
>> hw/ppc/meson.build | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
>> index e82a6b4105b..580e6e42c8a 100644
>> --- a/hw/ppc/meson.build
>> +++ b/hw/ppc/meson.build
>> @@ -3,9 +3,9 @@
>> 'ppc.c',
>> 'ppc_booke.c',
>> ))
>> -ppc_ss.add(when: 'CONFIG_FDT_PPC', if_true: [files(
>> +ppc_ss.add(when: 'CONFIG_FDT_PPC', if_true: files(
>> 'fdt.c',
>> -), fdt])
>> +))
>> ppc_ss.add(when: 'CONFIG_FW_CFG_PPC', if_true: files('fw_cfg.c'))
>>
>> # IBM pSeries (sPAPR)
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [RFC PATCH 4/5] hw/ppc/fdt: Drop dependency on libfdt
2021-05-12 7:59 ` Paolo Bonzini
@ 2021-05-13 3:46 ` David Gibson
2021-05-13 15:26 ` Paolo Bonzini
0 siblings, 1 reply; 32+ messages in thread
From: David Gibson @ 2021-05-13 3:46 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Thomas Huth, Alexey Kardashevskiy, qemu-devel, Greg Kurz,
qemu-ppc, John Paul Adrian Glaubitz, Philippe Mathieu-Daudé
[-- Attachment #1: Type: text/plain, Size: 2593 bytes --]
On Wed, May 12, 2021 at 09:59:00AM +0200, Paolo Bonzini wrote:
> On 12/05/21 04:30, David Gibson wrote:
> > On Tue, May 11, 2021 at 05:53:53PM +0200, Philippe Mathieu-Daudé wrote:
> > > hw/ppc/fdt.c defines the ppc_create_page_sizes_prop() function,
> > > which is unrelated to the libfdt. Remove the incorrect library
> > > dependency on the file.
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >
> > This is definitely wrong as it stands. AFAICT this doesn't add a
> > build of hw/ppc/fdt.c anywhere, but it is definitely needed by both
> > pseries and powernv machine types, who select FDT_PPC for this exact
> > reason.
> >
> > I will grant you that it is badly named. It is in fact related to
> > libfdt, just rather indirectly.
>
> The patch makes sense in general. The file is only needed for pseries and
> powernv, not for e.g. e500 which does need fdt.
Yes, agreed.
> I would get rid of FDT_PPC completely. First, before patch 3, you can move
> fdt.c to PSERIES and POWERNV (it's too small to need its own Kconfig symbol)
> and only leave
>
> ppc_ss.add(when: 'CONFIG_FDT_PPC', if_true: fdt)
Uh... why do we need even this?
> Since you are at it, remove the silly #ifdef TARGET_PPC64 in the
> hw/ppc/fdt.c file.
>
> Then in patch 3:
>
> - add to Kconfig.host
>
> config FDT
> bool
>
> config LIBFDT
> bool
> depends on FDT
Um.. I'm not sure what semantic difference you're envisaging between
FDT and LIBFDT.
> - for all the boards I listed in my review, add "select LIBFDT" in addition
> to "depends on FDT".
>
> - add to meson.build
>
> softmmu_ss.add(when: 'CONFIG_LIBFDT', if_true: fdt)
>
> Paolo
>
> > > ---
> > > hw/ppc/meson.build | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> > > index e82a6b4105b..580e6e42c8a 100644
> > > --- a/hw/ppc/meson.build
> > > +++ b/hw/ppc/meson.build
> > > @@ -3,9 +3,9 @@
> > > 'ppc.c',
> > > 'ppc_booke.c',
> > > ))
> > > -ppc_ss.add(when: 'CONFIG_FDT_PPC', if_true: [files(
> > > +ppc_ss.add(when: 'CONFIG_FDT_PPC', if_true: files(
> > > 'fdt.c',
> > > -), fdt])
> > > +))
> > > ppc_ss.add(when: 'CONFIG_FW_CFG_PPC', if_true: files('fw_cfg.c'))
> > > # IBM pSeries (sPAPR)
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [RFC PATCH 4/5] hw/ppc/fdt: Drop dependency on libfdt
2021-05-13 3:46 ` David Gibson
@ 2021-05-13 15:26 ` Paolo Bonzini
2021-05-13 23:35 ` David Gibson
0 siblings, 1 reply; 32+ messages in thread
From: Paolo Bonzini @ 2021-05-13 15:26 UTC (permalink / raw)
To: David Gibson
Cc: Thomas Huth, Alexey Kardashevskiy, qemu-devel, Greg Kurz,
qemu-ppc, John Paul Adrian Glaubitz, Philippe Mathieu-Daudé
On 13/05/21 05:46, David Gibson wrote:
>> The patch makes sense in general. The file is only needed for pseries and
>> powernv, not for e.g. e500 which does need fdt.
>
> Yes, agreed.
>
>> I would get rid of FDT_PPC completely. First, before patch 3, you can move
>> fdt.c to PSERIES and POWERNV (it's too small to need its own Kconfig symbol)
>> and only leave
>>
>> ppc_ss.add(when: 'CONFIG_FDT_PPC', if_true: fdt)
>
> Uh... why do we need even this?
To tell Meson that a board requires QEMU to be linked with libfdt. This
symbol is then renamed to CONFIG_LIBFDT once it can be used with all
targets (rather than just hw/ppc).
>> Since you are at it, remove the silly #ifdef TARGET_PPC64 in the
>> hw/ppc/fdt.c file.
>>
>> Then in patch 3:
>>
>> - add to Kconfig.host
>>
>> config FDT
>> bool
>>
>> config LIBFDT
>> bool
>> depends on FDT
>
> Um.. I'm not sure what semantic difference you're envisaging between
> FDT and LIBFDT.
"FDT" is set by meson.build if the library is available, LIBFDT is set
by the board to link with the library. In other words CONFIG_FDT is
per-build, while CONFIG_LIBFDT is per-target.
If a board selects LIBFDT but the library is not available, minikconf
will report a contradiction due to "CONFIG_PSERIES=y" -> "config PSERIES
select LIBFDT" -> "config LIBFDT depends on FDT" -> "CONFIG_FDT=n".
>> - for all the boards I listed in my review, add "select LIBFDT" in addition
>> to "depends on FDT".
This is actually unnecessary---"depends on FDT" is not needed in the
boards because LIBFDT already has the dependency.
Paolo
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 4/5] hw/ppc/fdt: Drop dependency on libfdt
2021-05-13 15:26 ` Paolo Bonzini
@ 2021-05-13 23:35 ` David Gibson
2021-05-14 5:29 ` Paolo Bonzini
0 siblings, 1 reply; 32+ messages in thread
From: David Gibson @ 2021-05-13 23:35 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Thomas Huth, Alexey Kardashevskiy, qemu-devel, Greg Kurz,
qemu-ppc, John Paul Adrian Glaubitz, Philippe Mathieu-Daudé
[-- Attachment #1: Type: text/plain, Size: 2188 bytes --]
On Thu, May 13, 2021 at 05:26:37PM +0200, Paolo Bonzini wrote:
> On 13/05/21 05:46, David Gibson wrote:
> > > The patch makes sense in general. The file is only needed for pseries and
> > > powernv, not for e.g. e500 which does need fdt.
> >
> > Yes, agreed.
> >
> > > I would get rid of FDT_PPC completely. First, before patch 3, you can move
> > > fdt.c to PSERIES and POWERNV (it's too small to need its own Kconfig symbol)
> > > and only leave
> > >
> > > ppc_ss.add(when: 'CONFIG_FDT_PPC', if_true: fdt)
> >
> > Uh... why do we need even this?
>
> To tell Meson that a board requires QEMU to be linked with libfdt. This
> symbol is then renamed to CONFIG_LIBFDT once it can be used with all targets
> (rather than just hw/ppc).
Oh, I thought CONFIG_LIBFDT already did that.
> > > Since you are at it, remove the silly #ifdef TARGET_PPC64 in the
> > > hw/ppc/fdt.c file.
> > >
> > > Then in patch 3:
> > >
> > > - add to Kconfig.host
> > >
> > > config FDT
> > > bool
> > >
> > > config LIBFDT
> > > bool
> > > depends on FDT
> >
> > Um.. I'm not sure what semantic difference you're envisaging between
> > FDT and LIBFDT.
>
> "FDT" is set by meson.build if the library is available, LIBFDT is set by
> the board to link with the library. In other words CONFIG_FDT is per-build,
> while CONFIG_LIBFDT is per-target.
Oof... that's highly non-obvious. Could we call it HAVE_LIBFDT and
USE_LIBFDT instead?
> If a board selects LIBFDT but the library is not available, minikconf will
> report a contradiction due to "CONFIG_PSERIES=y" -> "config PSERIES select
> LIBFDT" -> "config LIBFDT depends on FDT" -> "CONFIG_FDT=n".
>
> > > - for all the boards I listed in my review, add "select LIBFDT" in addition
> > > to "depends on FDT".
>
> This is actually unnecessary---"depends on FDT" is not needed in the boards
> because LIBFDT already has the dependency.
>
> Paolo
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 4/5] hw/ppc/fdt: Drop dependency on libfdt
2021-05-13 23:35 ` David Gibson
@ 2021-05-14 5:29 ` Paolo Bonzini
2021-05-14 8:22 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 32+ messages in thread
From: Paolo Bonzini @ 2021-05-14 5:29 UTC (permalink / raw)
To: David Gibson
Cc: Thomas Huth, Alexey Kardashevskiy, qemu-devel, Greg Kurz,
qemu-ppc, John Paul Adrian Glaubitz, Philippe Mathieu-Daudé
On 14/05/21 01:35, David Gibson wrote:
>> "FDT" is set by meson.build if the library is available, LIBFDT is set by
>> the board to link with the library. In other words CONFIG_FDT is per-build,
>> while CONFIG_LIBFDT is per-target.
> Oof... that's highly non-obvious. Could we call it HAVE_LIBFDT and
> USE_LIBFDT instead?
>
Certainly okay by me.
Paolo
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 4/5] hw/ppc/fdt: Drop dependency on libfdt
2021-05-14 5:29 ` Paolo Bonzini
@ 2021-05-14 8:22 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-14 8:22 UTC (permalink / raw)
To: Paolo Bonzini, David Gibson
Cc: Thomas Huth, Alexey Kardashevskiy, qemu-devel, Greg Kurz,
qemu-ppc, John Paul Adrian Glaubitz
On 5/14/21 7:29 AM, Paolo Bonzini wrote:
> On 14/05/21 01:35, David Gibson wrote:
>>> "FDT" is set by meson.build if the library is available, LIBFDT is
>>> set by
>>> the board to link with the library. In other words CONFIG_FDT is
>>> per-build,
>>> while CONFIG_LIBFDT is per-target.
>> Oof... that's highly non-obvious. Could we call it HAVE_LIBFDT and
>> USE_LIBFDT instead?
Just to be sure I understood correctly, for the next version I'll
use HAVE_LIBFDT for the build-related logic (Meson) and USE_LIBFDT
for the per-target dependencies (Kconfig).
(Only reply if I misunderstood).
> Certainly okay by me.
>
> Paolo
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [RFC PATCH 5/5] meson: Do not use internal fdt library if user asked for the system one
2021-05-11 15:53 [RFC PATCH 0/5] buildsys: Do not use internal fdt library when asked for the system one Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2021-05-11 15:53 ` [RFC PATCH 4/5] hw/ppc/fdt: Drop dependency on libfdt Philippe Mathieu-Daudé
@ 2021-05-11 15:53 ` Philippe Mathieu-Daudé
2021-05-12 7:32 ` Paolo Bonzini
2021-05-11 15:57 ` [RFC PATCH 0/5] buildsys: Do not use internal fdt library when " Philippe Mathieu-Daudé
5 siblings, 1 reply; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-11 15:53 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Alexey Kardashevskiy, Greg Kurz, qemu-ppc,
John Paul Adrian Glaubitz, Paolo Bonzini,
Philippe Mathieu-Daudé,
David Gibson
If the user explicitly asked for the system libfdt library, but
the library is not usable (usually too old), we should not silently
default to the internal one.
Respect the user decision, and only default to 'internal' if in
auto mode.
Fixes: fbb4121d592 ("dtc: Convert Makefile bits to meson bits")
Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
BugLink: https://bugs.launchpad.net/qemu/+bug/1907427
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/255
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index 0b41ff41188..1ffb4bccdb2 100644
--- a/meson.build
+++ b/meson.build
@@ -1612,7 +1612,7 @@
int main(void) { fdt_check_full(NULL, 0); return 0; }''',
dependencies: fdt)
fdt_opt = 'system'
- elif have_internal
+ elif have_internal and fdt_opt in ['enabled', 'auto']
fdt_opt = 'internal'
else
fdt_opt = 'disabled'
--
2.26.3
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [RFC PATCH 5/5] meson: Do not use internal fdt library if user asked for the system one
2021-05-11 15:53 ` [RFC PATCH 5/5] meson: Do not use internal fdt library if user asked for the system one Philippe Mathieu-Daudé
@ 2021-05-12 7:32 ` Paolo Bonzini
0 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2021-05-12 7:32 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Alexey Kardashevskiy, Greg Kurz, qemu-ppc,
John Paul Adrian Glaubitz, David Gibson
On 11/05/21 17:53, Philippe Mathieu-Daudé wrote:
> diff --git a/meson.build b/meson.build
> index 0b41ff41188..1ffb4bccdb2 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1612,7 +1612,7 @@
> int main(void) { fdt_check_full(NULL, 0); return 0; }''',
> dependencies: fdt)
> fdt_opt = 'system'
> - elif have_internal
> + elif have_internal and fdt_opt in ['enabled', 'auto']
> fdt_opt = 'internal'
> else
This will disable FDT silently for --enable-fdt=system instead of failing
the build. What about:
diff --git a/meson.build b/meson.build
index 60040cd7cf..efb38ca780 100644
--- a/meson.build
+++ b/meson.build
@@ -1610,11 +1610,18 @@ if have_system
fdt = cc.find_library('fdt', kwargs: static_kwargs,
required: fdt_opt == 'system' or
fdt_opt == 'enabled' and not have_internal)
- if fdt.found() and cc.links('''
+ if fdt.found() and not cc.links('''
#include <libfdt.h>
#include <libfdt_env.h>
int main(void) { fdt_check_full(NULL, 0); return 0; }''',
dependencies: fdt)
+ if fdt_opt == 'system' or
+ fdt_opt == 'enabled' and not have_internal then
+ error('libfdt is too old, version 1.5.1 required')
+ endif
+ fdt = not_found
+ endif
+ if fdt.found()
fdt_opt = 'system'
elif have_internal
fdt_opt = 'internal'
Paolo
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 0/5] buildsys: Do not use internal fdt library when asked for the system one
2021-05-11 15:53 [RFC PATCH 0/5] buildsys: Do not use internal fdt library when asked for the system one Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2021-05-11 15:53 ` [RFC PATCH 5/5] meson: Do not use internal fdt library if user asked for the system one Philippe Mathieu-Daudé
@ 2021-05-11 15:57 ` Philippe Mathieu-Daudé
2021-05-12 3:56 ` Philippe Mathieu-Daudé
5 siblings, 1 reply; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-11 15:57 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Alexey Kardashevskiy, Greg Kurz, qemu-ppc,
John Paul Adrian Glaubitz, Paolo Bonzini, David Gibson
On 5/11/21 5:53 PM, Philippe Mathieu-Daudé wrote:
> Attempt to fix the issue reported by John when building
> with an outdated libfdt.
Unencoded version of this cover:
For now it changes:
hw/ppc/spapr_hcall.c: In function ‘h_update_dt’:
hw/ppc/spapr_hcall.c:1966:9: warning: implicit declaration of function
‘fdt_check_full’; did you mean ‘fdt_check_header’?
[-Wimplicit-function-declaration]
1966 | if (fdt_check_full(fdt, cb)) {
| ^~~~~~~~~~~~~~
| fdt_check_header
hw/ppc/spapr_hcall.c:1966:9: warning: nested extern declaration of
‘fdt_check_full’ [-Wnested-externs]
[...]
/usr/bin/ld: libqemu-ppc64-softmmu.fa.p/hw_ppc_spapr_hcall.c.o: in
function `h_update_dt':
hw/ppc/spapr_hcall.c:1966: undefined reference to `fdt_check_full'
collect2: error: ld returned 1 exit status
by:
qemu/meson.build:1352:4: ERROR: Running configure command failed.
The following clauses were found for PSERIES
CONFIG_PSERIES=y
config PSERIES depends on FDT
which is not better, but one step at a time...
John said: https://gitlab.com/qemu-project/qemu/-/issues/255#note_572421108
Distributions usually don't used embedded copies of libraries,
so the configure script should require the correct minimum version.
Personally I'd rather allow users to build the most of QEMU with what is
available, that is all possible machines except pSeries, making pSeries
machine selected by default and deselected if not possible, with this
change:
-- >8 --
diff --git a/default-configs/devices/ppc64-softmmu.mak
b/default-configs/devices/ppc64-softmmu.mak
index cca52665d90..62339661fca 100644
--- a/default-configs/devices/ppc64-softmmu.mak
+++ b/default-configs/devices/ppc64-softmmu.mak
@@ -5,6 +5,3 @@ include ppc-softmmu.mak
# For PowerNV
CONFIG_POWERNV=y
-
-# For pSeries
-CONFIG_PSERIES=y
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 3935b73456f..706debd4fee 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -1,5 +1,6 @@
config PSERIES
bool
+ default y
depends on FDT
imply PCI_DEVICES
imply TEST_DEVICES
---
But I suppose it breaks user expectations.
Thoughts?
;)
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [RFC PATCH 0/5] buildsys: Do not use internal fdt library when asked for the system one
2021-05-11 15:57 ` [RFC PATCH 0/5] buildsys: Do not use internal fdt library when " Philippe Mathieu-Daudé
@ 2021-05-12 3:56 ` Philippe Mathieu-Daudé
2021-05-12 7:34 ` Paolo Bonzini
0 siblings, 1 reply; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-12 3:56 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini
Cc: Thomas Huth, Alexey Kardashevskiy, Greg Kurz, qemu-ppc,
John Paul Adrian Glaubitz, David Gibson
Hi Paolo,
On 5/11/21 5:57 PM, Philippe Mathieu-Daudé wrote:
> On 5/11/21 5:53 PM, Philippe Mathieu-Daudé wrote:
>> Attempt to fix the issue reported by John when building
>> with an outdated libfdt.
>
> Unencoded version of this cover:
>
> For now it changes:
>
> hw/ppc/spapr_hcall.c: In function ‘h_update_dt’:
> hw/ppc/spapr_hcall.c:1966:9: warning: implicit declaration of function
> ‘fdt_check_full’; did you mean ‘fdt_check_header’?
> [-Wimplicit-function-declaration]
> 1966 | if (fdt_check_full(fdt, cb)) {
> | ^~~~~~~~~~~~~~
> | fdt_check_header
> hw/ppc/spapr_hcall.c:1966:9: warning: nested extern declaration of
> ‘fdt_check_full’ [-Wnested-externs]
> [...]
> /usr/bin/ld: libqemu-ppc64-softmmu.fa.p/hw_ppc_spapr_hcall.c.o: in
> function `h_update_dt':
> hw/ppc/spapr_hcall.c:1966: undefined reference to `fdt_check_full'
> collect2: error: ld returned 1 exit status
>
> by:
>
> qemu/meson.build:1352:4: ERROR: Running configure command failed.
> The following clauses were found for PSERIES
>
> CONFIG_PSERIES=y
> config PSERIES depends on FDT
>
This is triggered with:
fdt support: NO
having:
default-configs/targets/ppc64-softmmu.mak:6:TARGET_NEED_FDT=y
So this code doesn't seem to work:
if not fdt.found() and fdt_required.length() > 0
error('fdt not available but required by targets ' + ',
'.join(fdt_required))
endif
BTW I disagree FDT is target-dependent, it is machine-dependent IMO.
> which is not better, but one step at a time...
>
> John said: https://gitlab.com/qemu-project/qemu/-/issues/255#note_572421108
>
> Distributions usually don't used embedded copies of libraries,
> so the configure script should require the correct minimum version.
>
> Personally I'd rather allow users to build the most of QEMU with what is
> available, that is all possible machines except pSeries, making pSeries
> machine selected by default and deselected if not possible, with this
> change:
>
> -- >8 --
> diff --git a/default-configs/devices/ppc64-softmmu.mak
> b/default-configs/devices/ppc64-softmmu.mak
> index cca52665d90..62339661fca 100644
> --- a/default-configs/devices/ppc64-softmmu.mak
> +++ b/default-configs/devices/ppc64-softmmu.mak
> @@ -5,6 +5,3 @@ include ppc-softmmu.mak
>
> # For PowerNV
> CONFIG_POWERNV=y
> -
> -# For pSeries
> -CONFIG_PSERIES=y
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index 3935b73456f..706debd4fee 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -1,5 +1,6 @@
> config PSERIES
> bool
> + default y
> depends on FDT
> imply PCI_DEVICES
> imply TEST_DEVICES
> ---
>
> But I suppose it breaks user expectations.
>
> Thoughts?
>
> ;)
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [RFC PATCH 0/5] buildsys: Do not use internal fdt library when asked for the system one
2021-05-12 3:56 ` Philippe Mathieu-Daudé
@ 2021-05-12 7:34 ` Paolo Bonzini
0 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2021-05-12 7:34 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Alexey Kardashevskiy, Greg Kurz, qemu-ppc,
John Paul Adrian Glaubitz, David Gibson
On 12/05/21 05:56, Philippe Mathieu-Daudé wrote:
>> qemu/meson.build:1352:4: ERROR: Running configure command failed.
>> The following clauses were found for PSERIES
>>
>> CONFIG_PSERIES=y
>> config PSERIES depends on FDT
>>
> This is triggered with:
>
> fdt support: NO
>
> having:
>
> default-configs/targets/ppc64-softmmu.mak:6:TARGET_NEED_FDT=y
>
> So this code doesn't seem to work:
>
> if not fdt.found() and fdt_required.length() > 0
> error('fdt not available but required by targets ' + ',
> '.join(fdt_required))
> endif
>
> BTW I disagree FDT is target-dependent, it is machine-dependent IMO.
>
I agree. It is much better to depend on FDT consistently for machines
that require it.
Paolo
^ permalink raw reply [flat|nested] 32+ messages in thread