On 19.12.2023 19:29, Julien Grall wrote:
> On 19/12/2023 17:03, Jan Beulich wrote:
>> On 15.12.2023 03:43, Shawn Anastasio wrote:
>>> --- a/xen/arch/arm/bootfdt.c
>>> +++ b/xen/common/device-tree/bootfdt.c
>>> @@ -431,12 +431,15 @@ static int __init early_scan_node(const void *fdt,
>>> {
>>> int rc = 0;
>>>
>>> - /*
>>> - * If Xen has been booted via UEFI, the memory banks are
>>> - * populated. So we should skip the parsing.
>>> - */
>>> - if ( !efi_enabled(EFI_BOOT) &&
>>> - device_tree_node_matches(fdt, node, "memory") )
>>> + if ( device_tree_node_matches(fdt, node, "memory") )
>>> +#if defined(CONFIG_ARM_EFI)
>>> + /*
>>> + * If Xen has been booted via UEFI, the memory banks are
>>> + * populated. So we should skip the parsing.
>>> + */
>>> + if ( efi_enabled(EFI_BOOT) )
>>> + return rc;
>>> +#endif
>>
>> I'm not a DT maintainer, but I don't like this kind of #ifdef, the more
>> that maybe PPC and quite likely RISC-V are likely to also want to support
>> EFI boot. But of course there may be something inherently Arm-specific
>> here that I'm unaware of.
>
> Right now, I can't think how this is Arm specific. If you are using
> UEFI, then you are expected to use the UEFI memory map rather than the
> content of the device-tree.
>
> However, we don't have a CONFIG_EFI option. It would be nice to
> introduce one but I am not sure I would introduce it just for this #ifdef.
Right, hence why I also wasn't suggesting to go that route right away.
efi/common-stub.c already has a stub for efi_enabled(). Using that file
may be too involved to arrange for in PPC, but supplying such a stub
elsewhere for the time being looks like it wouldn't too much effort
(and would eliminate the need for any #ifdef here afaict). Shawn?
Jan
