> On 2 Aug 2023, at 15:48, Michal Orzel <[email protected]> wrote:
>
> Hi,
>
> On 02/08/2023 16:42, Luca Fancellu wrote:
>>
>>
>>> On 2 Aug 2023, at 15:26, Michal Orzel <[email protected]> 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 <[email protected]>
>>>> ---
>>>> 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.
Anyway I understand your point, do you think something like that could be ok?
I’ve checked and it works, it
compile only if at least one GIC driver is enabled
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 264d2f2d4b09..85b4a7f08932 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -1096,6 +1096,9 @@ void __init start_xen(unsigned long boot_phys_offset,
preinit_xen_time();
+ /* Don't build if at least one GIC driver is enabled */
+ BUILD_BUG_ON(!(IS_ENABLED(CONFIG_GICV3) || IS_ENABLED(CONFIG_GICV2)
+ || IS_ENABLED(CONFIG_NEW_VGIC)));
gic_preinit();
arm_uart_init();
>
> ~Michal
