Re: [RFC PATCH] arm/gicv2: make GICv2 driver and vGICv2 optional

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

> On 3 Aug 2023, at 14:03, Luca Fancellu <Luca.Fancellu@xxxxxxx> wrote:
> 
> Hi Julien,
> 
>>> +
>>> config GICV3
>>>  bool "GICv3 driver"
>>>  depends on !NEW_VGIC
>>> @@ -92,6 +100,11 @@ config HAS_ITS
>>>         bool "GICv3 ITS MSI controller support (UNSUPPORTED)" if UNSUPPORTED
>>>         depends on GICV3 && !NEW_VGIC && !ARM_32
>>> +config VGICV2
>>> + bool "vGICv2 interface for guests"
>> 
>> This description is a bit misleading as the vGICv2 will also be used for 
>> dom0 in the case of vGICv2.
>> 
>>> + default y
>> 
>> Please add a longer help.
>> 
>>> + depends on (GICV2 || GICV3) && !NEW_VGIC
>> 
>> In the near future, I don't expect anyone to introduce a new non-GIC of 
>> interrupt controller for Arm. But I would expect new version of the GIC. So 
>> I would drop (GICV2 || GICV3).
>> 
>> Also when !NEW_VGIC is selected, this will make VGICV2 will be unselected. I 
>> was actually expecting the other way around given that new vGIC only offer 
>> v2 support.
>> 
>> The rest of the changes LGTM.
> 
> Thanks a lot for having a look on this patch, you are right the NEW_VGIC is 
> offering only v2 support at the moment, does this changes captures your
> Comments?
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 5cdba07df964..1c600b3b8d04 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -110,15 +110,19 @@ config HAS_ITS
>         depends on GICV3 && !NEW_VGIC && !ARM_32
> 
> config VGICV2
> -       bool "vGICv2 interface for guests"
> +       bool "vGICv2 interface for domains"
>        default y
> -       depends on (GICV2 || GICV3) && !NEW_VGIC
> +       help
> +         Provides a virtualised interface for the Generic Interrupt 
> Controller that
> +         can be used by Xen's domains.
> +         If unsure, say Y
> 
> config HVM
>         def_bool y
> 
> config NEW_VGIC
>        bool "Use new VGIC implementation"
> +       select VGICV2

Sorry, I meant to use here “select GICV2"

>        ---help---
> 
>        This is an alternative implementation of the ARM GIC interrupt
> diff --git a/xen/arch/arm/vgic/Makefile b/xen/arch/arm/vgic/Makefile
> index 806826948e20..60cbf7f2f94a 100644
> --- a/xen/arch/arm/vgic/Makefile
> +++ b/xen/arch/arm/vgic/Makefile
> @@ -1,5 +1,5 @@
> obj-y += vgic.o
> -obj-y += vgic-v2.o
> +obj-$(CONFIG_VGICV2) += vgic-v2.o
> obj-y += vgic-mmio.o
> -obj-y += vgic-mmio-v2.o
> +obj-$(CONFIG_VGICV2) += vgic-mmio-v2.o
> obj-y += vgic-init.o
> 
> 
> 
>> 
>> Cheers,
>> 
>> -- 
>> Julien Grall