diff mbox series

[v2,3/7] xen/common: Move Arm's bootfdt to common

Message ID b26a07209b54cd036e42a8b00f036201821eb775.1702607884.git.sanastasio@raptorengineering.com (mailing list archive)
State New, archived
Headers show
Series Early Boot Allocation on Power | expand

Commit Message

Shawn Anastasio Dec. 15, 2023, 2:43 a.m. UTC
Move Arm's bootfdt.c to xen/common so that it can be used by other
device tree architectures like PPC and RISCV. Only a minor change to
conditionalize a call to a function only available on EFI-supporting
targets was made to the code itself.

Suggested-by: Julien Grall <julien@xen.org>
Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>
---
 xen/arch/arm/Makefile                          |  1 -
 xen/common/Makefile                            |  1 +
 xen/common/device-tree/Makefile                |  1 +
 xen/{arch/arm => common/device-tree}/bootfdt.c | 15 +++++++++------
 4 files changed, 11 insertions(+), 7 deletions(-)
 create mode 100644 xen/common/device-tree/Makefile
 rename xen/{arch/arm => common/device-tree}/bootfdt.c (98%)

Comments

Jan Beulich Dec. 19, 2023, 5:03 p.m. UTC | #1
On 15.12.2023 03:43, Shawn Anastasio wrote:
> Move Arm's bootfdt.c to xen/common so that it can be used by other
> device tree architectures like PPC and RISCV. Only a minor change to
> conditionalize a call to a function only available on EFI-supporting
> targets was made to the code itself.
> 
> Suggested-by: Julien Grall <julien@xen.org>
> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>
> ---
>  xen/arch/arm/Makefile                          |  1 -
>  xen/common/Makefile                            |  1 +
>  xen/common/device-tree/Makefile                |  1 +
>  xen/{arch/arm => common/device-tree}/bootfdt.c | 15 +++++++++------
>  4 files changed, 11 insertions(+), 7 deletions(-)
>  create mode 100644 xen/common/device-tree/Makefile
>  rename xen/{arch/arm => common/device-tree}/bootfdt.c (98%)

I think this wants to come with an update to ./MAINTAINERS, such that
the file doesn't change maintainership.

> --- 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.

Jan
Julien Grall Dec. 19, 2023, 6:29 p.m. UTC | #2
Hi Jan,

On 19/12/2023 17:03, Jan Beulich wrote:
> On 15.12.2023 03:43, Shawn Anastasio wrote:
>> Move Arm's bootfdt.c to xen/common so that it can be used by other
>> device tree architectures like PPC and RISCV. Only a minor change to
>> conditionalize a call to a function only available on EFI-supporting
>> targets was made to the code itself.
>>
>> Suggested-by: Julien Grall <julien@xen.org>
>> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>
>> ---
>>   xen/arch/arm/Makefile                          |  1 -
>>   xen/common/Makefile                            |  1 +
>>   xen/common/device-tree/Makefile                |  1 +
>>   xen/{arch/arm => common/device-tree}/bootfdt.c | 15 +++++++++------
>>   4 files changed, 11 insertions(+), 7 deletions(-)
>>   create mode 100644 xen/common/device-tree/Makefile
>>   rename xen/{arch/arm => common/device-tree}/bootfdt.c (98%)
> 
> I think this wants to come with an update to ./MAINTAINERS, such that
> the file doesn't change maintainership.
> 
>> --- 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.

Cheers,
Jan Beulich Dec. 20, 2023, 8:09 a.m. UTC | #3
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
Julien Grall Dec. 20, 2023, 1:53 p.m. UTC | #4
Hi Shawn,

On 15/12/2023 02:43, Shawn Anastasio wrote:
> Move Arm's bootfdt.c to xen/common so that it can be used by other
> device tree architectures like PPC and RISCV. Only a minor change to
> conditionalize a call to a function only available on EFI-supporting
> targets was made to the code itself. >
> Suggested-by: Julien Grall <julien@xen.org>
> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>

With the MAINTAINERS file updated (I would add under DEVICE TREE) and 
one note below:

Acked-by: Julien Grall <jgrall@amazon.com>

> --- 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)

As discussed in a separate subthread, I would be ok with any of the 
options proposed. So feel free ot keep my Acked-by once this is settled.

Cheers,
Shawn Anastasio Dec. 20, 2023, 8:58 p.m. UTC | #5
On 12/20/23 2:09 AM, Jan Beulich wrote:
> 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?
> 

To clarify, you're suggesting we add an efi_enabled stub somewhere in
arch/ppc? I'm not against that, though it does seem a little silly to
have to define EFI-specific functions on an architecture that will never
support EFI.

Stil, if you think it's preferable to adding the ifdef here then I'm not
against it.

> Jan

Thanks,
Shawn
Julien Grall Dec. 20, 2023, 10:08 p.m. UTC | #6
Hi,

On 20/12/2023 20:58, Shawn Anastasio wrote:
> On 12/20/23 2:09 AM, Jan Beulich wrote:
>> 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?
>>
> 
> To clarify, you're suggesting we add an efi_enabled stub somewhere in
> arch/ppc? I'm not against that, though it does seem a little silly to
> have to define EFI-specific functions on an architecture that will never
> support EFI.

(This is not an argument for adding efi_enabled in arch/ppc)

I am curious to know why you think that. This is just software and 
therefore doesn't seem to be technically impossible. I mean who 
originally thought that ACPI would come to Arm? :) And yet we now have 
HWs (mainly servers) which provides only ACPI + UEFI.

And before, I got asked where is the support in Xen. Yes, the work is 
still on-going :).

Anyway, back to the original ask, one option would be to introduce 
efi_enabled stub in an common header. Maybe xen/efi.h?

But I would also be ok with the existing #ifdef for now.

Cheers,
Timothy Pearson Dec. 20, 2023, 10:10 p.m. UTC | #7
----- Original Message -----
> From: "Julien Grall" <julien@xen.org>
> To: "Shawn Anastasio" <sanastasio@raptorengineering.com>, "Jan Beulich" <jbeulich@suse.com>
> Cc: "Timothy Pearson" <tpearson@raptorengineering.com>, "xen-devel" <xen-devel@lists.xenproject.org>
> Sent: Wednesday, December 20, 2023 4:08:30 PM
> Subject: Re: [PATCH v2 3/7] xen/common: Move Arm's bootfdt to common

> Hi,
> 
> On 20/12/2023 20:58, Shawn Anastasio wrote:
>> On 12/20/23 2:09 AM, Jan Beulich wrote:
>>> 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?
>>>
>> 
>> To clarify, you're suggesting we add an efi_enabled stub somewhere in
>> arch/ppc? I'm not against that, though it does seem a little silly to
>> have to define EFI-specific functions on an architecture that will never
>> support EFI.
> 
> (This is not an argument for adding efi_enabled in arch/ppc)
> 
> I am curious to know why you think that. This is just software and
> therefore doesn't seem to be technically impossible. I mean who
> originally thought that ACPI would come to Arm? :) And yet we now have
> HWs (mainly servers) which provides only ACPI + UEFI.

It's not a technical restriction, it's an architecture specifiction and compatibility (standardization) restriction.  POWER has its own interfaces (OPAL etc.) that provide the functionality ACPI provides on x86/ARM, and fragmenting the ecosystem across two interface standards is not something that any of the key stakeholders are currently willing to do.

Just some background, I have no comment on the actual technical implementation in the patch. :)

Thanks!
Jan Beulich Dec. 21, 2023, 7:11 a.m. UTC | #8
On 20.12.2023 23:08, Julien Grall wrote:
> Hi,
> 
> On 20/12/2023 20:58, Shawn Anastasio wrote:
>> On 12/20/23 2:09 AM, Jan Beulich wrote:
>>> 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?
>>>
>>
>> To clarify, you're suggesting we add an efi_enabled stub somewhere in
>> arch/ppc? I'm not against that, though it does seem a little silly to
>> have to define EFI-specific functions on an architecture that will never
>> support EFI.
> 
> (This is not an argument for adding efi_enabled in arch/ppc)
> 
> I am curious to know why you think that. This is just software and 
> therefore doesn't seem to be technically impossible. I mean who 
> originally thought that ACPI would come to Arm? :) And yet we now have 
> HWs (mainly servers) which provides only ACPI + UEFI.
> 
> And before, I got asked where is the support in Xen. Yes, the work is 
> still on-going :).
> 
> Anyway, back to the original ask, one option would be to introduce 
> efi_enabled stub in an common header. Maybe xen/efi.h?

Right, and having a somewhat odd #ifdef there (covering for the lack of
CONFIG_EFI) would imo be preferable to having it in a random .c file.

Jan
diff mbox series

Patch

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 33c677672f..64fdf84170 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -10,7 +10,6 @@  obj-$(CONFIG_TEE) += tee/
 obj-$(CONFIG_HAS_VPCI) += vpci.o
 
 obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
-obj-y += bootfdt.init.o
 obj-y += cpuerrata.o
 obj-y += cpufeature.o
 obj-y += decode.o
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 69d6aa626c..6e175626d5 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -77,6 +77,7 @@  obj-$(CONFIG_UBSAN) += ubsan/
 
 obj-$(CONFIG_NEEDS_LIBELF) += libelf/
 obj-$(CONFIG_HAS_DEVICE_TREE) += libfdt/
+obj-$(CONFIG_HAS_DEVICE_TREE) += device-tree/
 
 CONF_FILE := $(if $(patsubst /%,,$(KCONFIG_CONFIG)),$(objtree)/)$(KCONFIG_CONFIG)
 $(obj)/config.gz: $(CONF_FILE)
diff --git a/xen/common/device-tree/Makefile b/xen/common/device-tree/Makefile
new file mode 100644
index 0000000000..66c2500df9
--- /dev/null
+++ b/xen/common/device-tree/Makefile
@@ -0,0 +1 @@ 
+obj-y += bootfdt.init.o
diff --git a/xen/arch/arm/bootfdt.c b/xen/common/device-tree/bootfdt.c
similarity index 98%
rename from xen/arch/arm/bootfdt.c
rename to xen/common/device-tree/bootfdt.c
index f496a8cf94..ae9fa1e3d6 100644
--- 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
         rc = process_memory_node(fdt, node, name, depth,
                                  address_cells, size_cells, &bootinfo.mem);
     else if ( depth == 1 && !dt_node_cmp(name, "reserved-memory") )