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

[Dongjiu Geng <gengdongjiu1@xxxxxxxxx>]

Hi, Stefano
    Thanks for this reply.

Stefano Stabellini <sstabellini@xxxxxxxxxx> 于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@xxxxxxxxx>
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@xxxxxxxxx>

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