[PATCH] hw/arm/virt: ARM_VIRT must select ARM_GIC

[PATCH] hw/arm/virt: ARM_VIRT must select ARM_GIC

[Andrew Jones]

The removal of the selection of A15MPCORE from ARM_VIRT also
removed what A15MPCORE selects, ARM_GIC. We still need ARM_GIC.

Fixes: bec3c97e0cf9 ("hw/arm/virt: Remove dependency on Cortex-A15 MPCore peripherals")
Reported-by: Miroslav Rezanina <mrezanin@redhat.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 7d022eeefdbc..e69a9009cf0a 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
+    select ARM_GIC
     select ACPI
     select ARM_SMMUV3
     select GPIO_KEY
-- 
2.26.2

Re: [PATCH] hw/arm/virt: ARM_VIRT must select ARM_GIC

[Miroslav Rezanina]

----- Original Message -----
> From: "Andrew Jones" <drjones@redhat.com>
> To: qemu-devel@nongnu.org, qemu-arm@nongnu.org
> Cc: "peter maydell" <peter.maydell@linaro.org>, philmd@redhat.com, "Miroslav Rezanina" <mrezanin@redhat.com>
> Sent: Wednesday, November 11, 2020 3:34:40 PM
> Subject: [PATCH] hw/arm/virt: ARM_VIRT must select ARM_GIC
> 
> The removal of the selection of A15MPCORE from ARM_VIRT also
> removed what A15MPCORE selects, ARM_GIC. We still need ARM_GIC.
> 
> Fixes: bec3c97e0cf9 ("hw/arm/virt: Remove dependency on Cortex-A15 MPCore
> peripherals")
> Reported-by: Miroslav Rezanina <mrezanin@redhat.com>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  hw/arm/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 7d022eeefdbc..e69a9009cf0a 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
> +    select ARM_GIC
>      select ACPI
>      select ARM_SMMUV3
>      select GPIO_KEY
> --
> 2.26.2
> 
> 

Problems with missing dependencies solved by this patch.

Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>

-- 
Miroslav Rezanina
Software Engineer - Virtualization Team Maintainer

Re: [PATCH] hw/arm/virt: ARM_VIRT must select ARM_GIC

[Peter Maydell]

On Wed, 11 Nov 2020 at 15:05, Miroslav Rezanina <mrezanin@redhat.com> wrote:
>
> ----- Original Message -----
> > From: "Andrew Jones" <drjones@redhat.com>
> > To: qemu-devel@nongnu.org, qemu-arm@nongnu.org
> > Cc: "peter maydell" <peter.maydell@linaro.org>, philmd@redhat.com, "Miroslav Rezanina" <mrezanin@redhat.com>
> > Sent: Wednesday, November 11, 2020 3:34:40 PM
> > Subject: [PATCH] hw/arm/virt: ARM_VIRT must select ARM_GIC
> >
> > The removal of the selection of A15MPCORE from ARM_VIRT also
> > removed what A15MPCORE selects, ARM_GIC. We still need ARM_GIC.

>
> Problems with missing dependencies solved by this patch.
>
> Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>

This is the second of this kind of "missing select" bug I've
seen recently. I don't suppose there's some kind of testing
we could add to 'make check' that automatically catches them?

thanks
-- PMM

Re: [PATCH] hw/arm/virt: ARM_VIRT must select ARM_GIC

[Philippe Mathieu-Daudé]

On 11/11/20 3:34 PM, Andrew Jones wrote:
> The removal of the selection of A15MPCORE from ARM_VIRT also
> removed what A15MPCORE selects, ARM_GIC. We still need ARM_GIC.
> 
> Fixes: bec3c97e0cf9 ("hw/arm/virt: Remove dependency on Cortex-A15 MPCore peripherals")
> Reported-by: Miroslav Rezanina <mrezanin@redhat.com>
> Signed-off-by: Andrew Jones <drjones@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  hw/arm/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 7d022eeefdbc..e69a9009cf0a 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
> +    select ARM_GIC
>      select ACPI
>      select ARM_SMMUV3
>      select GPIO_KEY
> 

Re: [PATCH] hw/arm/virt: ARM_VIRT must select ARM_GIC

[Andrew Jones]

On Wed, Nov 11, 2020 at 04:00:25PM +0000, Peter Maydell wrote:
> On Wed, 11 Nov 2020 at 15:05, Miroslav Rezanina <mrezanin@redhat.com> wrote:
> >
> > ----- Original Message -----
> > > From: "Andrew Jones" <drjones@redhat.com>
> > > To: qemu-devel@nongnu.org, qemu-arm@nongnu.org
> > > Cc: "peter maydell" <peter.maydell@linaro.org>, philmd@redhat.com, "Miroslav Rezanina" <mrezanin@redhat.com>
> > > Sent: Wednesday, November 11, 2020 3:34:40 PM
> > > Subject: [PATCH] hw/arm/virt: ARM_VIRT must select ARM_GIC
> > >
> > > The removal of the selection of A15MPCORE from ARM_VIRT also
> > > removed what A15MPCORE selects, ARM_GIC. We still need ARM_GIC.
> 
> >
> > Problems with missing dependencies solved by this patch.
> >
> > Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
> 
> This is the second of this kind of "missing select" bug I've
> seen recently. I don't suppose there's some kind of testing
> we could add to 'make check' that automatically catches them?
>

Miroslav is finding them because the RHEL build of QEMU is more selective
than upstream as to what gets pulled in. Effort keeps going into making
upstream more configurable, but it's not quite there yet for RHEL's
purposes. I'm not sure how best to test something like this upstream.
I guess it would require the flexible configuration support we don't yet
have.

Thanks,
drew

Re: [PATCH] hw/arm/virt: ARM_VIRT must select ARM_GIC

[Miroslav Rezanina]

----- Original Message -----
> From: "Andrew Jones" <drjones@redhat.com>
> To: "Peter Maydell" <peter.maydell@linaro.org>
> Cc: "Miroslav Rezanina" <mrezanin@redhat.com>, "QEMU Developers" <qemu-devel@nongnu.org>, "qemu-arm"
> <qemu-arm@nongnu.org>, "Philippe Mathieu-Daudé" <philmd@redhat.com>
> Sent: Wednesday, November 11, 2020 6:39:32 PM
> Subject: Re: [PATCH] hw/arm/virt: ARM_VIRT must select ARM_GIC
> 
> On Wed, Nov 11, 2020 at 04:00:25PM +0000, Peter Maydell wrote:
> > On Wed, 11 Nov 2020 at 15:05, Miroslav Rezanina <mrezanin@redhat.com>
> > wrote:
> > >
> > > ----- Original Message -----
> > > > From: "Andrew Jones" <drjones@redhat.com>
> > > > To: qemu-devel@nongnu.org, qemu-arm@nongnu.org
> > > > Cc: "peter maydell" <peter.maydell@linaro.org>, philmd@redhat.com,
> > > > "Miroslav Rezanina" <mrezanin@redhat.com>
> > > > Sent: Wednesday, November 11, 2020 3:34:40 PM
> > > > Subject: [PATCH] hw/arm/virt: ARM_VIRT must select ARM_GIC
> > > >
> > > > The removal of the selection of A15MPCORE from ARM_VIRT also
> > > > removed what A15MPCORE selects, ARM_GIC. We still need ARM_GIC.
> > 
> > >
> > > Problems with missing dependencies solved by this patch.
> > >
> > > Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
> > 
> > This is the second of this kind of "missing select" bug I've
> > seen recently. I don't suppose there's some kind of testing
> > we could add to 'make check' that automatically catches them?
> >
> 
> Miroslav is finding them because the RHEL build of QEMU is more selective
> than upstream as to what gets pulled in. Effort keeps going into making
> upstream more configurable, but it's not quite there yet for RHEL's
> purposes. I'm not sure how best to test something like this upstream.
> I guess it would require the flexible configuration support we don't yet
> have.
> 

We use --without-default-devices and then add only supported devices - this allow
us to find this kind of errors. To prevent them by test we would need complete matrix
possible of device configurations (or just usable combinations). Not always is broken
build wrong - it can be caused by unusable configuration.

In this case, we have correct dependency - CONFIG_ARM_GIC_KVM requires CONFIG_KVM and CONFIG_ARM_GIC.
In RC1 commit bec3c97e0cf9 (hw/arm/virt: Remove dependency on Cortex-A15 MPCore peripherals) removed
indirect CONFIG_ARM_GIC from CONFIG_ARM_VIRT. So anyone using CONFIG_ARM_GIC_KVM with CONFIG_ARM_VIRT
now gets build error. 

Question here is: Is okay to use CONFIG_ARM_GIC_KVM with CONFIG_ARM_VIRT without explicit CONFIG_ARM_GIC?
I can imagine both answers:

1) yes - CONFIG_ARM_VIRT implicate presence of CONFIG_ARM_GIC so no need to add it explicitly
2) no - If you want to use CONFIG_ARM_GIC_KVM you have to always add CONFIG_ARM_GIC explicitly too

I'm little bit confused now - why KConfig uses depends instead of select? If using option without dependent
one is not possible shouldn't be logical to just get it?


Mirek
> Thanks,
> drew
> 

-- 
Miroslav Rezanina
Software Engineer - Virtualization Team Maintainer

Re: [PATCH] hw/arm/virt: ARM_VIRT must select ARM_GIC

[Peter Maydell]

On Wed, 11 Nov 2020 at 14:34, Andrew Jones <drjones@redhat.com> wrote:
>
> The removal of the selection of A15MPCORE from ARM_VIRT also
> removed what A15MPCORE selects, ARM_GIC. We still need ARM_GIC.
>
> Fixes: bec3c97e0cf9 ("hw/arm/virt: Remove dependency on Cortex-A15 MPCore peripherals")
> Reported-by: Miroslav Rezanina <mrezanin@redhat.com>
> Signed-off-by: Andrew Jones <drjones@redhat.com>



Applied to target-arm.next, thanks.

-- PMM