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

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi,

On 02/08/2023 16:42, Luca Fancellu wrote:
> 
> 
>> On 2 Aug 2023, at 15:26, Michal Orzel <michal.orzel@xxxxxxx> wrote:
>>
>> Hi Luca,
>>
>> On 02/08/2023 15:53, Luca Fancellu wrote:
>>>
>>>
>>> Introduce Kconfig GICV2 to be able to compile the GICv2 driver only
>>> when needed, the option is active by default.
>>>
>>> Introduce Kconfig VGICV2 that depends on GICV2 or GICV3 and compiles
>>> the GICv2 emulation for guests, it is required only when using GICV2
>>> driver, otherwise using GICV3 it is optional and can be deselected
>>> if the user doesn't want to offer the vGICv2 interface to guests or
>>> maybe its GICv3 hardware can't offer the GICv2 compatible mode.
>>>
>>> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
>>> ---
>>> xen/arch/arm/Kconfig        | 13 +++++++++++++
>>> xen/arch/arm/Makefile       |  4 ++--
>>> xen/arch/arm/domain_build.c |  4 ++++
>>> xen/arch/arm/gic-v3.c       |  4 ++++
>>> xen/arch/arm/vgic.c         |  2 ++
>>> 5 files changed, 25 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>>> index fd57a82dd284..dc702f08ace7 100644
>>> --- a/xen/arch/arm/Kconfig
>>> +++ b/xen/arch/arm/Kconfig
>>> @@ -78,6 +78,14 @@ config ARM_EFI
>>>          UEFI firmware. A UEFI stub is provided to allow Xen to
>>>          be booted as an EFI application.
>>>
>>> +config GICV2
>> So, now it would be possible to deselect both GIC drivers and Xen would not 
>> complain when building.
>> This means that Xen would fail on boot without any message as it happens 
>> before serial driver initialization.
>> Since having GIC driver built in is a must-have I think we need to make sure 
>> that at least one is enabled.
> 
> Hi Michal,
> 
> I tried and I had:
> 
> Starting kernel ...
> 
> - UART enabled -
> - Boot CPU booting -
> - Current EL 0000000000000008 -
> - Initialize CPU -
> - Turning on paging -
> - Zero BSS -
> - Ready -
> (XEN) Checking for initrd in /chosen
> (XEN) RAM: 0000000080000000 - 00000000feffffff
> (XEN) RAM: 0000000880000000 - 00000008ffffffff
> (XEN)
> (XEN) MODULE[0]: 0000000084000000 - 000000008415d000 Xen
> (XEN) MODULE[1]: 00000000fd6c5000 - 00000000fd6c8000 Device Tree
> (XEN) MODULE[2]: 0000000080080000 - 00000000814f1a00 Kernel
> (XEN)  RESVD[0]: 0000000080000000 - 0000000080010000
> (XEN)  RESVD[1]: 0000000018000000 - 00000000187fffff
> (XEN)
> (XEN)
> (XEN) Command line: noreboot dom0_mem=1024M console=dtuart dtuart=serial0 
> bootscrub=0
> (XEN) PFN compression on bits 20...22
> (XEN) Domain heap initialised
> (XEN) Booting using Device Tree
> (XEN) Platform: Generic System
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) Unable to find compatible GIC in the device tree
> (XEN) ****************************************
> (XEN)
> (XEN) Manual reset required ('noreboot' specified)
Having early printk enabled all the time is not common and not enabled in 
release builds FWIK.
So in general, user would just see "Starting kernel" from u-boot and had to 
debug what's going on.

> 
> Wouldn’t be enough to suggest the user that at least one GIC needs to be 
> selected? In the help it
> also states “if unsure, say Y"
I always think it is better to meet the users needs by preventing unwise 
mistakes like unselecting both drivers.
As always, it is up to maintainers.

~Michal