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

[Dongjiu Geng <gengdongjiu1@xxxxxxxxx>]

Dongjiu Geng <gengdongjiu1@xxxxxxxxx> 于2022年1月13日周四 11:47写道：
>
> 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

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