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

[Michal Orzel <michal.orzel@xxxxxxx>]

On 03/08/2023 09:35, Julien Grall wrote:
> 
> 
> Hi,
> 
> On 03/08/2023 08:15, Michal Orzel wrote:
>> On 02/08/2023 19:39, Julien Grall wrote:
>>>
>>>
>>> Hi Luca,
>>>
>>> On 02/08/2023 16:05, Luca Fancellu wrote:
>>>>> On 2 Aug 2023, at 15:48, Michal Orzel <michal.orzel@xxxxxxx> wrote:
>>>>>
>>>>> 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.
>>>
>>> There are a lot of information printed before the console is properly
>>> brought up. I wonder if we should look at adding early console like
>>> Linux does?
>> I guess it is not a matter of "if" but "when" and I think it's just that no 
>> one has time to do that
>> since it is more a like a feature for developers rather than for customers 
>> (they just report the issue
>> and we need to fix it :)).
> 
> Sure. This is always the case, but it also means developpers will
> continue to waste time when investigating early boot issues. I wouldn't
> be surprised if the total time wasted is more than the actual effort to
> add support :).
most probably

> 
>> There are so many things that can go wrong during early boot including the 
>> entire boofdt parsing
>> and having earlycon would be so desirable.
> It is not clear what to take from your reply. Earlier you were concerned
> about the error not showing up in the log if the .config is not correct.
> 
> There is no really quick fix for that as a .config may work for platform
> A but not platform B. The only viable solution is having an early console.
> 
> Anything else like forcing to always have one GICvX selected is just a
> hack that would work for one set of people but not the others.
Yes, having early console would solve these issues and for now we might need to 
be
on the mercy of users to do wise selection. That said, the scenario with GIC is 
not really
the same as for something that can work on platform A and not on B. No GIC 
built-in
cannot work on any platform, hence my concern (it is different than selecting 
wrong GIC).
But because I do not see any simple solution other than building always both 
and you don't like
what Luca just did (fallback to GICv2), I'm ok. Initializing UART prior to GIC 
would also require
quite some work to defer the IRQ translation that depends on the GIC.

~Michal