Re: [PATCH] xen/arm: fix the build error for GIC

[Dongjiu Geng <gengdongjiu1@xxxxxxxxx>]

Stefano Stabellini <sstabellini@xxxxxxxxxx> 于2021年12月24日周五 08:08写道：
>
> On Wed, 22 Dec 2021, Julien Grall wrote:
> > Hello,
> >
> > On 22/12/2021 09:38, Dongjiu Geng wrote:
> > > when enable CONFIG_NEW_VGIC in ARM64 QEMU Platform, it will build failed.
> > > so fix it and make it can select GICV2.
> >
> > Last time I checked QEMU, it was only able to support GICv3 virtualization.
> > Has it changed since?
> >
> > >
> > > Signed-off-by: Dongjiu Geng <gengdongjiu1@xxxxxxxxx>
> > > ---
> > > $ make dist-xen XEN_TARGET_ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- 
> > > -j10
> > > make -C xen install
> > > make[1]: Entering directory
> > > '/home/workspace/gengdongjiu/old_machine/XEN/xen/xen'
> > > make -f
> > > /home/workspace/gengdongjiu/old_machine/XEN/xen/xen/tools/kconfig/Makefile.kconfig
> > > ARCH=arm64 SRCARCH=arm HOSTCC="gcc" HOSTCXX="g++" syncconfig
> > > make[2]: Entering directory
> > > '/home/workspace/gengdongjiu/old_machine/XEN/xen/xen'
> > > gcc -Wp,-MD,tools/kconfig/.conf.o.d      -c -o tools/kconfig/conf.o
> > > tools/kconfig/conf.c
> > > gcc -Wp,-MD,tools/kconfig/.confdata.o.d      -c -o 
> > > tools/kconfig/confdata.o
> > > tools/kconfig/confdata.c
> > > gcc -Wp,-MD,tools/kconfig/.expr.o.d      -c -o tools/kconfig/expr.o
> > > tools/kconfig/expr.c
> > > flex -otools/kconfig/lexer.lex.c -L tools/kconfig/lexer.l
> > > bison -o tools/kconfig/parser.tab.c --defines=tools/kconfig/parser.tab.h 
> > > -t
> > > -l tools/kconfig/parser.y
> > > gcc -Wp,-MD,tools/kconfig/.preprocess.o.d      -c -o
> > > tools/kconfig/preprocess.o tools/kconfig/preprocess.c
> > > gcc -Wp,-MD,tools/kconfig/.symbol.o.d      -c -o tools/kconfig/symbol.o
> > > tools/kconfig/symbol.c
> > > gcc -Wp,-MD,tools/kconfig/.lexer.lex.o.d     -I
> > > /home/workspace/gengdongjiu/old_machine/XEN/xen/xen/tools/kconfig -c -o
> > > tools/kconfig/lexer.lex.o tools/kconfig/lexer.lex.c
> > > gcc -Wp,-MD,tools/kconfig/.parser.tab.o.d     -I
> > > /home/workspace/gengdongjiu/old_machine/XEN/xen/xen/tools/kconfig -c -o
> > > tools/kconfig/parser.tab.o tools/kconfig/parser.tab.c
> > > gcc  -o tools/kconfig/conf tools/kconfig/conf.o tools/kconfig/confdata.o
> > > tools/kconfig/expr.o tools/kconfig/lexer.lex.o tools/kconfig/parser.tab.o
> > > tools/kconfig/preprocess.o tools/kconfig/symbol.o
> > > 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]
> > > make[2]: Leaving directory
> > > '/home/workspace/gengdongjiu/old_machine/XEN/xen/xen'
> > > make -f Rules.mk _install
> > > make[2]: Entering directory
> > > '/home/workspace/gengdongjiu/old_machine/XEN/xen/xen'
> > > ---
> > >   xen/arch/arm/platforms/Kconfig | 1 -
> > >   1 file changed, 1 deletion(-)
> > >
> > > diff --git a/xen/arch/arm/platforms/Kconfig 
> > > b/xen/arch/arm/platforms/Kconfig
> > > index c93a6b2756..6d3ee99456 100644
> > > --- a/xen/arch/arm/platforms/Kconfig
> > > +++ b/xen/arch/arm/platforms/Kconfig
> > > @@ -15,7 +15,6 @@ config ALL_PLAT
> > >   config QEMU
> > >     bool "QEMU aarch virt machine support"
> > >     depends on ARM_64
> > > -   select GICV3
> >
> > Assuming you can use Xen with GICv2 on QEMU, then this change is OK. But we
> > are at risk to expose it again via another platform.
> >
> > So I think it would be better to also to add a dependency !GICV3 for 
> > NEW_VGIC.
>
> I agree. I gave it a try and turns out it is not possible to simply add:
>
> depends on !GICV3
>
> under NEW_VGIC because there is already a !NEW_VGIC under GICV3.
> Apparently it creates a circular dependency. Instead, I did the
> following and it looks like it is behaving the way we want.
>
>
> 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---

Thanks for the suggestion, I have updated it and resumbit patch v2,
please review it again.

>
>         This is an alternative implementation of the ARM GIC interrupt