[PATCH v3] xen/arm: Allow QEMU platform to be built with GICv2

[PATCH v3] xen/arm: Allow QEMU platform to be built with GICv2

[Dongjiu Geng]

Trying to select PLATFORM_QEMU with NEW_VGIC will result to Kconfig
complain about unmet dependencies:

tools/kconfig/conf  --syncconfig Kconfig

WARNING: unmet direct dependencies detected for GICV3
   Depends on [n]: ARM_64 [=y] && !NEW_VGIC [=y]
   Selected by [y]:
   - QEMU [=y] && <choice> && ARM_64 [=y]

WARNING: unmet direct dependencies detected for GICV3
   Depends on [n]: ARM_64 [=y] && !NEW_VGIC [=y]
   Selected by [y]:
   - QEMU [=y] && <choice> && ARM_64 [=y]

WARNING: unmet direct dependencies detected for GICV3
   Depends on [n]: ARM_64 [=y] && !NEW_VGIC [=y]
   Selected by [y]:
   - QEMU [=y] && <choice> && ARM_64 [=y]

It turns out that QEMU has been supporting GICv2 virtualization since
v3.1.0. So an easy way to solve the issue and allow more custom support
is to remove the dependencies on GICv3.

Signed-off-by: Dongjiu Geng <gengdongjiu1@gmail.com>
---
change since v1:
1. Address Stefano's comments to add dependency

change since v2:
1. Address Julien's comments to update the commit messages.
2. enable GICV3 in arch/arm/configs/tiny64_defconfig
---
 xen/arch/arm/Kconfig                  | 5 +++--
 xen/arch/arm/configs/tiny64_defconfig | 2 +-
 xen/arch/arm/platforms/Kconfig        | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index ecfa6822e4..373c698018 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -35,7 +35,7 @@ config ACPI
 
 config GICV3
 	bool "GICv3 driver"
-	depends on ARM_64 && !NEW_VGIC
+	depends on ARM_64
 	default y
 	---help---
 
@@ -44,13 +44,14 @@ config GICV3
 
 config HAS_ITS
         bool "GICv3 ITS MSI controller support (UNSUPPORTED)" if UNSUPPORTED
-        depends on GICV3 && !NEW_VGIC
+        depends on GICV3
 
 config HVM
         def_bool y
 
 config NEW_VGIC
 	bool "Use new VGIC implementation"
+	depends on !GICV3
 	---help---
 
 	This is an alternative implementation of the ARM GIC interrupt
diff --git a/xen/arch/arm/configs/tiny64_defconfig b/xen/arch/arm/configs/tiny64_defconfig
index cc6d93f2f8..165603f7e0 100644
--- a/xen/arch/arm/configs/tiny64_defconfig
+++ b/xen/arch/arm/configs/tiny64_defconfig
@@ -4,7 +4,7 @@ CONFIG_ARM=y
 #
 # Architecture Features
 #
-# CONFIG_GICV3 is not set
+CONFIG_GICV3=y
 # CONFIG_MEM_ACCESS is not set
 # CONFIG_SBSA_VUART_CONSOLE is not set
 
diff --git a/xen/arch/arm/platforms/Kconfig b/xen/arch/arm/platforms/Kconfig
index c93a6b2756..925f6ef8da 100644
--- a/xen/arch/arm/platforms/Kconfig
+++ b/xen/arch/arm/platforms/Kconfig
@@ -15,7 +15,7 @@ config ALL_PLAT
 config QEMU
 	bool "QEMU aarch virt machine support"
 	depends on ARM_64
-	select GICV3
+	select GICv3 if !NEW_VGIC
 	select HAS_PL011
 	---help---
 	Enable all the required drivers for QEMU aarch64 virt emulated
-- 
2.25.1

Re: [PATCH v3] xen/arm: Allow QEMU platform to be built with GICv2

[Julien Grall]

Hi,

On Tue, 28 Dec 2021 at 05:14, Dongjiu Geng <gengdongjiu1@gmail.com> wrote:
>
> Trying to select PLATFORM_QEMU with NEW_VGIC will result to Kconfig
> complain about unmet dependencies:
>
> tools/kconfig/conf  --syncconfig Kconfig
>
> WARNING: unmet direct dependencies detected for GICV3
>    Depends on [n]: ARM_64 [=y] && !NEW_VGIC [=y]
>    Selected by [y]:
>    - QEMU [=y] && <choice> && ARM_64 [=y]
>
> WARNING: unmet direct dependencies detected for GICV3
>    Depends on [n]: ARM_64 [=y] && !NEW_VGIC [=y]
>    Selected by [y]:
>    - QEMU [=y] && <choice> && ARM_64 [=y]
>
> WARNING: unmet direct dependencies detected for GICV3
>    Depends on [n]: ARM_64 [=y] && !NEW_VGIC [=y]
>    Selected by [y]:
>    - QEMU [=y] && <choice> && ARM_64 [=y]
>
> It turns out that QEMU has been supporting GICv2 virtualization since
> v3.1.0. So an easy way to solve the issue and allow more custom support
> is to remove the dependencies on GICv3.

You took my proposed commit message but the diff in
this version doesn't match it.

>
> Signed-off-by: Dongjiu Geng <gengdongjiu1@gmail.com>
> ---
> change since v1:
> 1. Address Stefano's comments to add dependency
>
> change since v2:
> 1. Address Julien's comments to update the commit messages.
> 2. enable GICV3 in arch/arm/configs/tiny64_defconfig
> ---
>  xen/arch/arm/Kconfig                  | 5 +++--
>  xen/arch/arm/configs/tiny64_defconfig | 2 +-
>  xen/arch/arm/platforms/Kconfig        | 2 +-
>  3 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index ecfa6822e4..373c698018 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig

Are the changes necessary in arch/arm/Kconfig to solve the issue on QEMU?
If not, then they should be in a separate patch.
If yes, then they ought to be explained in the commit message.

> @@ -35,7 +35,7 @@ config ACPI
>
>  config GICV3
>         bool "GICv3 driver"
> -       depends on ARM_64 && !NEW_VGIC
> +       depends on ARM_64
>         default y
>         ---help---
>
> @@ -44,13 +44,14 @@ config GICV3
>
>  config HAS_ITS
>          bool "GICv3 ITS MSI controller support (UNSUPPORTED)" if UNSUPPORTED
> -        depends on GICV3 && !NEW_VGIC
> +        depends on GICV3
>
>  config HVM
>          def_bool y
>
>  config NEW_VGIC
>         bool "Use new VGIC implementation"
> +       depends on !GICV3
>         ---help---
>
>         This is an alternative implementation of the ARM GIC interrupt
> diff --git a/xen/arch/arm/configs/tiny64_defconfig b/xen/arch/arm/configs/tiny64_defconfig
> index cc6d93f2f8..165603f7e0 100644
> --- a/xen/arch/arm/configs/tiny64_defconfig
> +++ b/xen/arch/arm/configs/tiny64_defconfig
> @@ -4,7 +4,7 @@ CONFIG_ARM=y
>  #
>  # Architecture Features
>  #
> -# CONFIG_GICV3 is not set
> +CONFIG_GICV3=y

The goal of tiny64_defconfig is to have nothing enabled by default.
So we should not enable GICV3 here.

>  # CONFIG_MEM_ACCESS is not set
>  # CONFIG_SBSA_VUART_CONSOLE is not set
>
> diff --git a/xen/arch/arm/platforms/Kconfig b/xen/arch/arm/platforms/Kconfig
> index c93a6b2756..925f6ef8da 100644
> --- a/xen/arch/arm/platforms/Kconfig
> +++ b/xen/arch/arm/platforms/Kconfig
> @@ -15,7 +15,7 @@ config ALL_PLAT
>  config QEMU
>         bool "QEMU aarch virt machine support"
>         depends on ARM_64
> -       select GICV3
> +       select GICv3 if !NEW_VGIC

There was an open question in v2. In general, it is better to wait
until the discussion is closed or you mention it after ---. This
avoids being lost.

Cheers,

-- 
Julien Grall

Re: [PATCH v3] xen/arm: Allow QEMU platform to be built with GICv2

[Dongjiu Geng]

Julien Grall <julien.grall.oss@gmail.com> 于2021年12月28日周二 18:39写道：
>
> Hi,
>
> On Tue, 28 Dec 2021 at 05:14, Dongjiu Geng <gengdongjiu1@gmail.com> wrote:
> >
> > Trying to select PLATFORM_QEMU with NEW_VGIC will result to Kconfig
> > complain about unmet dependencies:
> >
> > tools/kconfig/conf  --syncconfig Kconfig
> >
> > WARNING: unmet direct dependencies detected for GICV3
> >    Depends on [n]: ARM_64 [=y] && !NEW_VGIC [=y]
> >    Selected by [y]:
> >    - QEMU [=y] && <choice> && ARM_64 [=y]
> >
> > WARNING: unmet direct dependencies detected for GICV3
> >    Depends on [n]: ARM_64 [=y] && !NEW_VGIC [=y]
> >    Selected by [y]:
> >    - QEMU [=y] && <choice> && ARM_64 [=y]
> >
> > WARNING: unmet direct dependencies detected for GICV3
> >    Depends on [n]: ARM_64 [=y] && !NEW_VGIC [=y]
> >    Selected by [y]:
> >    - QEMU [=y] && <choice> && ARM_64 [=y]
> >
> > It turns out that QEMU has been supporting GICv2 virtualization since
> > v3.1.0. So an easy way to solve the issue and allow more custom support
> > is to remove the dependencies on GICv3.
>
> You took my proposed commit message but the diff in
> this version doesn't match it.

Thanks for the point out, I will update it.

>
> >
> > Signed-off-by: Dongjiu Geng <gengdongjiu1@gmail.com>
> > ---
> > change since v1:
> > 1. Address Stefano's comments to add dependency
> >
> > change since v2:
> > 1. Address Julien's comments to update the commit messages.
> > 2. enable GICV3 in arch/arm/configs/tiny64_defconfig
> > ---
> >  xen/arch/arm/Kconfig                  | 5 +++--
> >  xen/arch/arm/configs/tiny64_defconfig | 2 +-
> >  xen/arch/arm/platforms/Kconfig        | 2 +-
> >  3 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index ecfa6822e4..373c698018 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
>
> Are the changes necessary in arch/arm/Kconfig to solve the issue on QEMU?
> If not, then they should be in a separate patch.
I will submit it in a separate patch.

> If yes, then they ought to be explained in the commit message.
>
> > @@ -35,7 +35,7 @@ config ACPI
> >
> >  config GICV3
> >         bool "GICv3 driver"
> > -       depends on ARM_64 && !NEW_VGIC
> > +       depends on ARM_64
> >         default y
> >         ---help---
> >
> > @@ -44,13 +44,14 @@ config GICV3
> >
> >  config HAS_ITS
> >          bool "GICv3 ITS MSI controller support (UNSUPPORTED)" if UNSUPPORTED
> > -        depends on GICV3 && !NEW_VGIC
> > +        depends on GICV3
> >
> >  config HVM
> >          def_bool y
> >
> >  config NEW_VGIC
> >         bool "Use new VGIC implementation"
> > +       depends on !GICV3
> >         ---help---
> >
> >         This is an alternative implementation of the ARM GIC interrupt
> > diff --git a/xen/arch/arm/configs/tiny64_defconfig b/xen/arch/arm/configs/tiny64_defconfig
> > index cc6d93f2f8..165603f7e0 100644
> > --- a/xen/arch/arm/configs/tiny64_defconfig
> > +++ b/xen/arch/arm/configs/tiny64_defconfig
> > @@ -4,7 +4,7 @@ CONFIG_ARM=y
> >  #
> >  # Architecture Features
> >  #
> > -# CONFIG_GICV3 is not set
> > +CONFIG_GICV3=y
>
> The goal of tiny64_defconfig is to have nothing enabled by default.
> So we should not enable GICV3 here.
>
> >  # CONFIG_MEM_ACCESS is not set
> >  # CONFIG_SBSA_VUART_CONSOLE is not set
> >
> > diff --git a/xen/arch/arm/platforms/Kconfig b/xen/arch/arm/platforms/Kconfig
> > index c93a6b2756..925f6ef8da 100644
> > --- a/xen/arch/arm/platforms/Kconfig
> > +++ b/xen/arch/arm/platforms/Kconfig
> > @@ -15,7 +15,7 @@ config ALL_PLAT
> >  config QEMU
> >         bool "QEMU aarch virt machine support"
> >         depends on ARM_64
> > -       select GICV3
> > +       select GICv3 if !NEW_VGIC
>
> There was an open question in v2. In general, it is better to wait
> until the discussion is closed or you mention it after ---. This
> avoids being lost.

Ok,  but I think my modification does not affect to tiny64_defconfig.

>
> Cheers,
>
> --
> Julien Grall

Re: [PATCH v3] xen/arm: Allow QEMU platform to be built with GICv2

[Dongjiu Geng]

Julien Grall <julien.grall.oss@gmail.com> 于2021年12月28日周二 18:39写道：
>
> Hi,
>
> On Tue, 28 Dec 2021 at 05:14, Dongjiu Geng <gengdongjiu1@gmail.com> wrote:
> >
> > Trying to select PLATFORM_QEMU with NEW_VGIC will result to Kconfig
> > complain about unmet dependencies:
> >
> > tools/kconfig/conf  --syncconfig Kconfig
> >
> > WARNING: unmet direct dependencies detected for GICV3
> >    Depends on [n]: ARM_64 [=y] && !NEW_VGIC [=y]
> >    Selected by [y]:
> >    - QEMU [=y] && <choice> && ARM_64 [=y]
> >
> > WARNING: unmet direct dependencies detected for GICV3
> >    Depends on [n]: ARM_64 [=y] && !NEW_VGIC [=y]
> >    Selected by [y]:
> >    - QEMU [=y] && <choice> && ARM_64 [=y]
> >
> > WARNING: unmet direct dependencies detected for GICV3
> >    Depends on [n]: ARM_64 [=y] && !NEW_VGIC [=y]
> >    Selected by [y]:
> >    - QEMU [=y] && <choice> && ARM_64 [=y]
> >
> > It turns out that QEMU has been supporting GICv2 virtualization since
> > v3.1.0. So an easy way to solve the issue and allow more custom support
> > is to remove the dependencies on GICv3.
>
> You took my proposed commit message but the diff in
> this version doesn't match it.
>
> >
> > Signed-off-by: Dongjiu Geng <gengdongjiu1@gmail.com>
> > ---
> > change since v1:
> > 1. Address Stefano's comments to add dependency
> >
> > change since v2:
> > 1. Address Julien's comments to update the commit messages.
> > 2. enable GICV3 in arch/arm/configs/tiny64_defconfig
> > ---
> >  xen/arch/arm/Kconfig                  | 5 +++--
> >  xen/arch/arm/configs/tiny64_defconfig | 2 +-
> >  xen/arch/arm/platforms/Kconfig        | 2 +-
> >  3 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index ecfa6822e4..373c698018 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
>
> Are the changes necessary in arch/arm/Kconfig to solve the issue on QEMU?
> If not, then they should be in a separate patch.
> If yes, then they ought to be explained in the commit message.
>
> > @@ -35,7 +35,7 @@ config ACPI
> >
> >  config GICV3
> >         bool "GICv3 driver"
> > -       depends on ARM_64 && !NEW_VGIC
> > +       depends on ARM_64
> >         default y
> >         ---help---
> >
> > @@ -44,13 +44,14 @@ config GICV3
> >
> >  config HAS_ITS
> >          bool "GICv3 ITS MSI controller support (UNSUPPORTED)" if UNSUPPORTED
> > -        depends on GICV3 && !NEW_VGIC
> > +        depends on GICV3
> >
> >  config HVM
> >          def_bool y
> >
> >  config NEW_VGIC
> >         bool "Use new VGIC implementation"
> > +       depends on !GICV3
> >         ---help---
> >
> >         This is an alternative implementation of the ARM GIC interrupt
> > diff --git a/xen/arch/arm/configs/tiny64_defconfig b/xen/arch/arm/configs/tiny64_defconfig
> > index cc6d93f2f8..165603f7e0 100644
> > --- a/xen/arch/arm/configs/tiny64_defconfig
> > +++ b/xen/arch/arm/configs/tiny64_defconfig
> > @@ -4,7 +4,7 @@ CONFIG_ARM=y
> >  #
> >  # Architecture Features
> >  #
> > -# CONFIG_GICV3 is not set
> > +CONFIG_GICV3=y
>
> The goal of tiny64_defconfig is to have nothing enabled by default.
> So we should not enable GICV3 here.
>
> >  # CONFIG_MEM_ACCESS is not set
> >  # CONFIG_SBSA_VUART_CONSOLE is not set
> >
> > diff --git a/xen/arch/arm/platforms/Kconfig b/xen/arch/arm/platforms/Kconfig
> > index c93a6b2756..925f6ef8da 100644
> > --- a/xen/arch/arm/platforms/Kconfig
> > +++ b/xen/arch/arm/platforms/Kconfig
> > @@ -15,7 +15,7 @@ config ALL_PLAT
> >  config QEMU
> >         bool "QEMU aarch virt machine support"
> >         depends on ARM_64
> > -       select GICV3
> > +       select GICv3 if !NEW_VGIC
>
> There was an open question in v2. In general, it is better to wait
> until the discussion is closed or you mention it after ---. This
> avoids being lost.

Sorry for the noise, Stefano,  do you have any suggestion for this
patch?  thanks a lot.

>
> Cheers,
>
> --
> Julien Grall

Re: [PATCH v3] xen/arm: Allow QEMU platform to be built with GICv2

[Stefano Stabellini]

On Wed, 12 Jan 2022, Dongjiu Geng wrote:
> > > diff --git a/xen/arch/arm/platforms/Kconfig b/xen/arch/arm/platforms/Kconfig
> > > index c93a6b2756..925f6ef8da 100644
> > > --- a/xen/arch/arm/platforms/Kconfig
> > > +++ b/xen/arch/arm/platforms/Kconfig
> > > @@ -15,7 +15,7 @@ config ALL_PLAT
> > >  config QEMU
> > >         bool "QEMU aarch virt machine support"
> > >         depends on ARM_64
> > > -       select GICV3
> > > +       select GICv3 if !NEW_VGIC
> >
> > There was an open question in v2. In general, it is better to wait
> > until the discussion is closed or you mention it after ---. This
> > avoids being lost.
> 
> Sorry for the noise, Stefano,  do you have any suggestion for this
> patch?  thanks a lot.

Looking back at v2, the original open question I think was:


> While writing a proposal for the commit message, I remembered that the 
> goal of CONFIG_QEMU was to select all the required drivers to be able to 
> boot Xen on QEMU.
> 
> AFAICT, if you start from tiny64_defconfig, you would not have GICv3 
> selected. So we would technically break users of QEMU with GICv3.
> 
> I am not entirely sure how to approach it. I can think of 2 options:
> 
>   1) Use 'select GICv3 if !NEW_VGIC'
>   2) Add a specific platform for QEMU new vGIC
> 
> I have the feeling that 1) will result to same unmet dependency issue. 
> Stefano any opinions?

I think it would be better to avoid introducing one more QEMU platform
if we can. The current patch as a bug, it should be:

select GICV3 if !NEW_VGIC

note "GICV3" instead of "GICv3".


But unfortunately it doesn't work:

arch/arm/Kconfig:52:error: recursive dependency detected!
arch/arm/Kconfig:52:    symbol NEW_VGIC depends on GICV3
arch/arm/Kconfig:36:    symbol GICV3 is selected by NEW_VGIC
For a resolution refer to Documentation/kbuild/kconfig-language.rst
subsection "Kconfig recursive dependency limitations"


If QEMU supports GICv2 virtualization since v3.1.0, which is from 2018,
then maybe the easiest way to solve the problem is simply to remove
"select GICV3" from QEMU as it is not an hard requirement any longer.
However, it is true that existing users of tiny64_defconfig and QEMU
would get a different behavior.

I don't think it is a problem but if you guys think it is, then the only
option is to introduce a new QEMU platform like "QEMU_GREATER_3.1.0"
which doesn't select GICV3 (it only selects HAS_PL011) because it is not
a requirement anymore.

Or we could have:

QEMU
    bool "QEMU aarch virt machine support >= v3.1.0"
    depends on ARM_64
    select HAS_PL011

QEMU_LEGACY
    bool "QEMU aarch virt machine support < v3.1.0"
    depends on ARM_64
    select GICV3
	select HAS_PL011

Re: [PATCH v3] xen/arm: Allow QEMU platform to be built with GICv2

[Dongjiu Geng]

Hi, Stefano
    Thanks for this reply.

Stefano Stabellini <sstabellini@kernel.org> 于2022年1月13日周四 09:52写道：
>
> On Wed, 12 Jan 2022, Dongjiu Geng wrote:
> > > > diff --git a/xen/arch/arm/platforms/Kconfig b/xen/arch/arm/platforms/Kconfig
> > > > index c93a6b2756..925f6ef8da 100644
> > > > --- a/xen/arch/arm/platforms/Kconfig
> > > > +++ b/xen/arch/arm/platforms/Kconfig
> > > > @@ -15,7 +15,7 @@ config ALL_PLAT
> > > >  config QEMU
> > > >         bool "QEMU aarch virt machine support"
> > > >         depends on ARM_64
> > > > -       select GICV3
> > > > +       select GICv3 if !NEW_VGIC
> > >
> > > There was an open question in v2. In general, it is better to wait
> > > until the discussion is closed or you mention it after ---. This
> > > avoids being lost.
> >
> > Sorry for the noise, Stefano,  do you have any suggestion for this
> > patch?  thanks a lot.
>
> Looking back at v2, the original open question I think was:
>
>
> > While writing a proposal for the commit message, I remembered that the
> > goal of CONFIG_QEMU was to select all the required drivers to be able to
> > boot Xen on QEMU.
> >
> > AFAICT, if you start from tiny64_defconfig, you would not have GICv3
> > selected. So we would technically break users of QEMU with GICv3.
> >
> > I am not entirely sure how to approach it. I can think of 2 options:
> >
> >   1) Use 'select GICv3 if !NEW_VGIC'
> >   2) Add a specific platform for QEMU new vGIC
> >
> > I have the feeling that 1) will result to same unmet dependency issue.
> > Stefano any opinions?
>
> I think it would be better to avoid introducing one more QEMU platform
> if we can. The current patch as a bug, it should be:
>
> select GICV3 if !NEW_VGIC
>
> note "GICV3" instead of "GICv3".
>
>
> But unfortunately it doesn't work:
>
> arch/arm/Kconfig:52:error: recursive dependency detected!
> arch/arm/Kconfig:52:    symbol NEW_VGIC depends on GICV3
> arch/arm/Kconfig:36:    symbol GICV3 is selected by NEW_VGIC
> For a resolution refer to Documentation/kbuild/kconfig-language.rst
> subsection "Kconfig recursive dependency limitations"
>
>
> If QEMU supports GICv2 virtualization since v3.1.0, which is from 2018,
> then maybe the easiest way to solve the problem is simply to remove
> "select GICV3" from QEMU as it is not an hard requirement any longer.
> However, it is true that existing users of tiny64_defconfig and QEMU
> would get a different behavior.
>
> I don't think it is a problem but if you guys think it is, then the only
> option is to introduce a new QEMU platform like "QEMU_GREATER_3.1.0"
> which doesn't select GICV3 (it only selects HAS_PL011) because it is not
> a requirement anymore.

Yes,  this is my original patch V1's modification which does not select GICV3

>
> Or we could have:
>
> QEMU
>     bool "QEMU aarch virt machine support >= v3.1.0"
>     depends on ARM_64
>     select HAS_PL011
>
> QEMU_LEGACY
>     bool "QEMU aarch virt machine support < v3.1.0"
>     depends on ARM_64
>     select GICV3
>         select HAS_PL011

Thanks for the suggestion,  I can make the modification below if you
think it is ok.
Hi Julien and Stefano, waiting for your confirmation .


commit 2b5749fc4e0769df9cfc55795e0dbb453000a9c9 (HEAD -> master_submit)
Author: Dongjiu Geng <gengdongjiu1@gmail.com>
Date:   Thu Jan 13 11:30:33 2022 +0800

    xen/arm: Allow QEMU platform to be built with GICv2

    It turns out that QEMU has been supporting GICv2 virtualization since
    v3.1.0. so remove the dependencies on GICv3. If we want to use GICv3,
    we can select the QEMU_LEGACY configuration.

    Signed-off-by: Dongjiu Geng <gengdongjiu1@gmail.com>

diff --git a/xen/arch/arm/platforms/Kconfig b/xen/arch/arm/platforms/Kconfig
index c93a6b2756..41e82a42ee 100644
--- a/xen/arch/arm/platforms/Kconfig
+++ b/xen/arch/arm/platforms/Kconfig
@@ -13,7 +13,15 @@ config ALL_PLAT
        automatically select any of the related drivers.

 config QEMU
-       bool "QEMU aarch virt machine support"
+       bool "QEMU aarch virt machine support >= v3.1.0"
+       depends on ARM_64
+       select HAS_PL011
+       ---help---
+       Enable all the required drivers for QEMU aarch64 virt emulated
+       machine.
+
+config QEMU_LEGACY
+       bool "QEMU aarch virt machine support < v3.1.0"
        depends on ARM_64
        select GICV3
        select HAS_PL011

Re: [PATCH v3] xen/arm: Allow QEMU platform to be built with GICv2

[Dongjiu Geng]

Dongjiu Geng <gengdongjiu1@gmail.com> 于2022年1月13日周四 11:47写道：
>
> Hi, Stefano
>     Thanks for this reply.
>
> Stefano Stabellini <sstabellini@kernel.org> 于2022年1月13日周四 09:52写道：
> >
> > On Wed, 12 Jan 2022, Dongjiu Geng wrote:
> > > > > diff --git a/xen/arch/arm/platforms/Kconfig b/xen/arch/arm/platforms/Kconfig
> > > > > index c93a6b2756..925f6ef8da 100644
> > > > > --- a/xen/arch/arm/platforms/Kconfig
> > > > > +++ b/xen/arch/arm/platforms/Kconfig
> > > > > @@ -15,7 +15,7 @@ config ALL_PLAT
> > > > >  config QEMU
> > > > >         bool "QEMU aarch virt machine support"
> > > > >         depends on ARM_64
> > > > > -       select GICV3
> > > > > +       select GICv3 if !NEW_VGIC
> > > >
> > > > There was an open question in v2. In general, it is better to wait
> > > > until the discussion is closed or you mention it after ---. This
> > > > avoids being lost.
> > >
> > > Sorry for the noise, Stefano,  do you have any suggestion for this
> > > patch?  thanks a lot.
> >
> > Looking back at v2, the original open question I think was:
> >
> >
> > > While writing a proposal for the commit message, I remembered that the
> > > goal of CONFIG_QEMU was to select all the required drivers to be able to
> > > boot Xen on QEMU.
> > >
> > > AFAICT, if you start from tiny64_defconfig, you would not have GICv3
> > > selected. So we would technically break users of QEMU with GICv3.
> > >
> > > I am not entirely sure how to approach it. I can think of 2 options:
> > >
> > >   1) Use 'select GICv3 if !NEW_VGIC'
> > >   2) Add a specific platform for QEMU new vGIC
> > >
> > > I have the feeling that 1) will result to same unmet dependency issue.
> > > Stefano any opinions?
> >
> > I think it would be better to avoid introducing one more QEMU platform
> > if we can. The current patch as a bug, it should be:
> >
> > select GICV3 if !NEW_VGIC
> >
> > note "GICV3" instead of "GICv3".
> >
> >
> > But unfortunately it doesn't work:
> >
> > arch/arm/Kconfig:52:error: recursive dependency detected!
> > arch/arm/Kconfig:52:    symbol NEW_VGIC depends on GICV3
> > arch/arm/Kconfig:36:    symbol GICV3 is selected by NEW_VGIC
> > For a resolution refer to Documentation/kbuild/kconfig-language.rst
> > subsection "Kconfig recursive dependency limitations"
> >
> >
> > If QEMU supports GICv2 virtualization since v3.1.0, which is from 2018,
> > then maybe the easiest way to solve the problem is simply to remove
> > "select GICV3" from QEMU as it is not an hard requirement any longer.
> > However, it is true that existing users of tiny64_defconfig and QEMU
> > would get a different behavior.
> >
> > I don't think it is a problem but if you guys think it is, then the only
> > option is to introduce a new QEMU platform like "QEMU_GREATER_3.1.0"
> > which doesn't select GICV3 (it only selects HAS_PL011) because it is not
> > a requirement anymore.
>
> Yes,  this is my original patch V1's modification which does not select GICV3
>
> >
> > Or we could have:
> >
> > QEMU
> >     bool "QEMU aarch virt machine support >= v3.1.0"
> >     depends on ARM_64
> >     select HAS_PL011
> >
> > QEMU_LEGACY
> >     bool "QEMU aarch virt machine support < v3.1.0"
> >     depends on ARM_64
> >     select GICV3
> >         select HAS_PL011
>
> Thanks for the suggestion,  I can make the modification below if you
> think it is ok.
> Hi Julien and Stefano, waiting for your confirmation .
>
>
> commit 2b5749fc4e0769df9cfc55795e0dbb453000a9c9 (HEAD -> master_submit)
> Author: Dongjiu Geng <gengdongjiu1@gmail.com>
> Date:   Thu Jan 13 11:30:33 2022 +0800
>
>     xen/arm: Allow QEMU platform to be built with GICv2
>
>     It turns out that QEMU has been supporting GICv2 virtualization since
>     v3.1.0. so remove the dependencies on GICv3. If we want to use GICv3,
>     we can select the QEMU_LEGACY configuration.
>
>     Signed-off-by: Dongjiu Geng <gengdongjiu1@gmail.com>
>
> diff --git a/xen/arch/arm/platforms/Kconfig b/xen/arch/arm/platforms/Kconfig
> index c93a6b2756..41e82a42ee 100644
> --- a/xen/arch/arm/platforms/Kconfig
> +++ b/xen/arch/arm/platforms/Kconfig
> @@ -13,7 +13,15 @@ config ALL_PLAT
>         automatically select any of the related drivers.
>
>  config QEMU
> -       bool "QEMU aarch virt machine support"
> +       bool "QEMU aarch virt machine support >= v3.1.0"
> +       depends on ARM_64
> +       select HAS_PL011
> +       ---help---
> +       Enable all the required drivers for QEMU aarch64 virt emulated
> +       machine.
> +
> +config QEMU_LEGACY
> +       bool "QEMU aarch virt machine support < v3.1.0"
>         depends on ARM_64
>         select GICV3
>         select HAS_PL011

If there is no objection, I will submit the patch.