Initially I merely noticed the regression addressed by the 1st patch,
but looking more closely revealed further deficiencies. After having
moved the FIXME in patch 3 I couldn't resist and address that issue
at least partly (patch 4), seeing that three and a half years have
passed and nothing was done to improve the situation.

1: build: work around bash issue
2: build: correctly report non-empty section sizes upon .o -> .init.o conversion
3: build: also check for empty .bss.* in .o -> .init.o conversion
4: EFI: free unused boot mem in at least some cases

Jan

Older bash fails to honor "set -e" for certain built-in commands
("while" here), despite the command's status correctly bein non-zero.
The subsequent objcopy invocation now being separated by a semicolon
results in no failure. Insert an explicit "exit" (replacing ; by &&
ought to be another possible workaround).

Fixes: e321576f4047 ("xen/build: start using if_changed")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I've done a pretty light-weight audit of possible further instances of
this issue, but didn't find any.

--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -193,7 +193,7 @@ define cmd_obj_init_o
             echo "Error: size of $<:$$name is 0x$$sz" >&2; \
             exit $$(expr $$idx + 1);; \
         esac; \
-    done; \
+    done || exit $$?; \
     $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
 endef
 

Hi Jan,

On 06/08/2020 10:04, Jan Beulich wrote:
> Older bash fails to honor "set -e" for certain built-in commands

"Older" is pretty vague. May I ask the exact version you run into the issue?

> ("while" here), despite the command's status correctly bein non-zero.
> The subsequent objcopy invocation now being separated by a semicolon
> results in no failure. Insert an explicit "exit" (replacing ; by &&
> ought to be another possible workaround).
> 
> Fixes: e321576f4047 ("xen/build: start using if_changed")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> I've done a pretty light-weight audit of possible further instances of
> this issue, but didn't find any.
> 
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -193,7 +193,7 @@ define cmd_obj_init_o
>               echo "Error: size of $<:$$name is 0x$$sz" >&2; \
>               exit $$(expr $$idx + 1);; \
>           esac; \
> -    done; \
> +    done || exit $$?; \
>       $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
>   endef
>   
> 

Cheers,

-- 
Julien Grall

On 06.08.2020 11:07, Julien Grall wrote:
> On 06/08/2020 10:04, Jan Beulich wrote:
>> Older bash fails to honor "set -e" for certain built-in commands
> 
> "Older" is pretty vague. May I ask the exact version you run into the issue?

If I knew in what version the issue got fixed, I'd have specified
that version. I've observed it with 3.2.57(2).

Jan

On 06/08/2020 10:14, Jan Beulich wrote:
> On 06.08.2020 11:07, Julien Grall wrote:
>> On 06/08/2020 10:04, Jan Beulich wrote:
>>> Older bash fails to honor "set -e" for certain built-in commands
>>
>> "Older" is pretty vague. May I ask the exact version you run into the issue?
> 
> If I knew in what version the issue got fixed, I'd have specified
> that version.

Right, but this is not what I asked. What I requested is that you 
provide the version you observe the issue.

This doesn't mean other versions are not affected, but at least it give 
us a clue where this can happen.

> I've observed it with 3.2.57(2).

Thank you. Please mention it in the commit message.

Cheers,

-- 
Julien Grall

On 06.08.2020 11:20, Julien Grall wrote:
> On 06/08/2020 10:14, Jan Beulich wrote:
>> I've observed it with 3.2.57(2).
> 
> Thank you. Please mention it in the commit message.

Well, added. If I had seen any use of the precise version, I would
have done so right away. Would you mind educating me how knowing
the precise version is useful, without knowing up to which version
the issue exists?

Jan

On 06/08/2020 10:25, Jan Beulich wrote:
> On 06.08.2020 11:20, Julien Grall wrote:
>> On 06/08/2020 10:14, Jan Beulich wrote:
>>> I've observed it with 3.2.57(2).
>>
>> Thank you. Please mention it in the commit message.
> 
> Well, added. If I had seen any use of the precise version, I would
> have done so right away. Would you mind educating me how knowing
> the precise version is useful, without knowing up to which version
> the issue exists?

Indeed, this wouldn't tell us whether other bash versions are affected.

However, as a reviewer, I need to be able to verify what you wrote is 
correct and that your patch is the correct way to solve it.

So providing the bash version where you observed the issue will at least 
fulfill this part.

Cheers,

-- 
Julien Grall

On 06/08/2020 10:14, Jan Beulich wrote:
> On 06.08.2020 11:07, Julien Grall wrote:
>> On 06/08/2020 10:04, Jan Beulich wrote:
>>> Older bash fails to honor "set -e" for certain built-in commands
>> "Older" is pretty vague. May I ask the exact version you run into the issue?
> If I knew in what version the issue got fixed, I'd have specified
> that version. I've observed it with 3.2.57(2).

Its still very useful information for the commit message.

I tend to phrase something like this as "Older versions of bash (at
least, 3.2.57(2))" which gives a clear indication that the problem was
observed with the specified version, without suggesting that this is a
boundary of when the issue was introduced/fixed.

With this adjusted, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
to avoid a second posting.

~Andrew

The originally used sed expression converted not just multiple leading
zeroes (as intended), but also trailing ones, rendering the error
message somewhat confusing. Collapse zeroes in just the one place where
we need them collapsed, and leave objdump's output as is for all other
purposes.

Fixes: 48115d14743e ("Move more kernel decompression bits to .init.* sections")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -185,11 +185,11 @@ cmd_cc_o_S = $(CC) $(a_flags) -c $< -o $
 
 quiet_cmd_obj_init_o = INIT_O  $@
 define cmd_obj_init_o
-    $(OBJDUMP) -h $< | sed -n '/[0-9]/{s,00*,0,g;p;}' | while read idx name sz rest; do \
+    $(OBJDUMP) -h $< | while read idx name sz rest; do \
         case "$$name" in \
         .*.local) ;; \
         .text|.text.*|.data|.data.*|.bss) \
-            test $$sz != 0 || continue; \
+            test $$(echo $$sz | sed 's,00*,0,') != 0 || continue; \
             echo "Error: size of $<:$$name is 0x$$sz" >&2; \
             exit $$(expr $$idx + 1);; \
         esac; \

On 06/08/2020 10:05, Jan Beulich wrote:
> The originally used sed expression converted not just multiple leading
> zeroes (as intended), but also trailing ones, rendering the error
> message somewhat confusing. Collapse zeroes in just the one place where
> we need them collapsed, and leave objdump's output as is for all other
> purposes.
>
> Fixes: 48115d14743e ("Move more kernel decompression bits to .init.* sections")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

We're gaining such sections, and like .text.* and .data.* they shouldn't
be present in objects subject to automatic to-init conversion. Oddly
enough for quite some time we did have an instance breaking this rule,
which gets fixed at this occasion, by breaking out the EFI boot
allocator functions into its own translation unit.

Fixes: c5b9805bc1f7 ("efi: create new early memory allocator")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
This likely has a (weak) dependency on "x86/EFI: sanitize build logic"
sent several weeks ago, due to the new source file added, as explicit
dependencies upon the individual objects in x86/Makefile go away there.

--- a/xen/Makefile
+++ b/xen/Makefile
@@ -355,7 +355,7 @@ $(TARGET): delete-unfresh-files
 	$(MAKE) -C tools
 	$(MAKE) -f $(BASEDIR)/Rules.mk include/xen/compile.h
 	[ -e include/asm ] || ln -sf asm-$(TARGET_ARCH) include/asm
-	[ -e arch/$(TARGET_ARCH)/efi ] && for f in boot.c runtime.c compat.c efi.h;\
+	[ -e arch/$(TARGET_ARCH)/efi ] && for f in $$(cd common/efi; echo *.[ch]); \
 		do test -r arch/$(TARGET_ARCH)/efi/$$f || \
 		   ln -nsf ../../../common/efi/$$f arch/$(TARGET_ARCH)/efi/; \
 		done; \
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -188,7 +188,7 @@ define cmd_obj_init_o
     $(OBJDUMP) -h $< | while read idx name sz rest; do \
         case "$$name" in \
         .*.local) ;; \
-        .text|.text.*|.data|.data.*|.bss) \
+        .text|.text.*|.data|.data.*|.bss|.bss.*) \
             test $$(echo $$sz | sed 's,00*,0,') != 0 || continue; \
             echo "Error: size of $<:$$name is 0x$$sz" >&2; \
             exit $$(expr $$idx + 1);; \
--- a/xen/arch/arm/efi/Makefile
+++ b/xen/arch/arm/efi/Makefile
@@ -1,4 +1,4 @@
 CFLAGS-y += -fshort-wchar
 
-obj-y +=  boot.init.o runtime.o
+obj-y +=  boot.init.o ebmalloc.o runtime.o
 obj-$(CONFIG_ACPI) +=  efi-dom0.init.o
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -8,7 +8,7 @@ cmd_objcopy_o_ihex = $(OBJCOPY) -I ihex
 
 boot.init.o: buildid.o
 
-EFIOBJ := boot.init.o compat.o runtime.o
+EFIOBJ := boot.init.o ebmalloc.o compat.o runtime.o
 
 $(call cc-option-add,cflags-stack-boundary,CC,-mpreferred-stack-boundary=4)
 $(EFIOBJ): CFLAGS-stack-boundary := $(cflags-stack-boundary)
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -112,7 +112,6 @@ static CHAR16 *FormatDec(UINT64 Val, CHA
 static CHAR16 *FormatHex(UINT64 Val, UINTN Width, CHAR16 *Buffer);
 static void  DisplayUint(UINT64 Val, INTN Width);
 static CHAR16 *wstrcpy(CHAR16 *d, const CHAR16 *s);
-static void noreturn blexit(const CHAR16 *str);
 static void PrintErrMesg(const CHAR16 *mesg, EFI_STATUS ErrCode);
 static char *get_value(const struct file *cfg, const char *section,
                               const char *item);
@@ -155,56 +154,6 @@ static CHAR16 __initdata newline[] = L"\
 #define PrintStr(s) StdOut->OutputString(StdOut, s)
 #define PrintErr(s) StdErr->OutputString(StdErr, s)
 
-#ifdef CONFIG_ARM
-/*
- * TODO: Enable EFI boot allocator on ARM.
- * This code can be common for x86 and ARM.
- * Things TODO on ARM before enabling ebmalloc:
- *   - estimate required EBMALLOC_SIZE value,
- *   - where (in which section) ebmalloc_mem[] should live; if in
- *     .bss.page_aligned, as it is right now, then whole BSS zeroing
- *     have to be disabled in xen/arch/arm/arm64/head.S; though BSS
- *     should be initialized somehow before use of variables living there,
- *   - use ebmalloc() in ARM/common EFI boot code,
- *   - call free_ebmalloc_unused_mem() somewhere in init code.
- */
-#define EBMALLOC_SIZE	MB(0)
-#else
-#define EBMALLOC_SIZE	MB(1)
-#endif
-
-static char __section(".bss.page_aligned") __aligned(PAGE_SIZE)
-    ebmalloc_mem[EBMALLOC_SIZE];
-static unsigned long __initdata ebmalloc_allocated;
-
-/* EFI boot allocator. */
-static void __init __maybe_unused *ebmalloc(size_t size)
-{
-    void *ptr = ebmalloc_mem + ebmalloc_allocated;
-
-    ebmalloc_allocated += ROUNDUP(size, sizeof(void *));
-
-    if ( ebmalloc_allocated > sizeof(ebmalloc_mem) )
-        blexit(L"Out of static memory\r\n");
-
-    return ptr;
-}
-
-static void __init __maybe_unused free_ebmalloc_unused_mem(void)
-{
-#if 0 /* FIXME: Putting a hole in the BSS breaks the IOMMU mappings for dom0. */
-    unsigned long start, end;
-
-    start = (unsigned long)ebmalloc_mem + PAGE_ALIGN(ebmalloc_allocated);
-    end = (unsigned long)ebmalloc_mem + sizeof(ebmalloc_mem);
-
-    destroy_xen_mappings(start, end);
-    init_xenheap_pages(__pa(start), __pa(end));
-
-    printk(XENLOG_INFO "Freed %lukB unused BSS memory\n", (end - start) >> 10);
-#endif
-}
-
 /*
  * Include architecture specific implementation here, which references the
  * static globals defined above.
@@ -321,7 +270,7 @@ static bool __init match_guid(const EFI_
            !memcmp(guid1->Data4, guid2->Data4, sizeof(guid1->Data4));
 }
 
-static void __init noreturn blexit(const CHAR16 *str)
+void __init noreturn blexit(const CHAR16 *str)
 {
     if ( str )
         PrintStr((CHAR16 *)str);
--- /dev/null
+++ b/xen/common/efi/ebmalloc.c
@@ -0,0 +1,52 @@
+#include "efi.h"
+#include <xen/init.h>
+
+#ifdef CONFIG_ARM
+/*
+ * TODO: Enable EFI boot allocator on ARM.
+ * This code can be common for x86 and ARM.
+ * Things TODO on ARM before enabling ebmalloc:
+ *   - estimate required EBMALLOC_SIZE value,
+ *   - where (in which section) ebmalloc_mem[] should live; if in
+ *     .bss.page_aligned, as it is right now, then whole BSS zeroing
+ *     have to be disabled in xen/arch/arm/arm64/head.S; though BSS
+ *     should be initialized somehow before use of variables living there,
+ *   - use ebmalloc() in ARM/common EFI boot code,
+ *   - call free_ebmalloc_unused_mem() somewhere in init code.
+ */
+#define EBMALLOC_SIZE	MB(0)
+#else
+#define EBMALLOC_SIZE	MB(1)
+#endif
+
+static char __section(".bss.page_aligned") __aligned(PAGE_SIZE)
+    ebmalloc_mem[EBMALLOC_SIZE];
+static unsigned long __initdata ebmalloc_allocated;
+
+/* EFI boot allocator. */
+void __init *ebmalloc(size_t size)
+{
+    void *ptr = ebmalloc_mem + ebmalloc_allocated;
+
+    ebmalloc_allocated += ROUNDUP(size, sizeof(void *));
+
+    if ( ebmalloc_allocated > sizeof(ebmalloc_mem) )
+        blexit(L"Out of static memory\r\n");
+
+    return ptr;
+}
+
+void __init free_ebmalloc_unused_mem(void)
+{
+#if 0 /* FIXME: Putting a hole in the BSS breaks the IOMMU mappings for dom0. */
+    unsigned long start, end;
+
+    start = (unsigned long)ebmalloc_mem + PAGE_ALIGN(ebmalloc_allocated);
+    end = (unsigned long)ebmalloc_mem + sizeof(ebmalloc_mem);
+
+    destroy_xen_mappings(start, end);
+    init_xenheap_pages(__pa(start), __pa(end));
+
+    printk(XENLOG_INFO "Freed %lukB unused BSS memory\n", (end - start) >> 10);
+#endif
+}
--- a/xen/common/efi/efi.h
+++ b/xen/common/efi/efi.h
@@ -40,4 +40,10 @@ extern UINT64 efi_boot_max_var_store_siz
 extern UINT64 efi_apple_properties_addr;
 extern UINTN efi_apple_properties_len;
 
+void noreturn blexit(const CHAR16 *str);
+
 const CHAR16 *wmemchr(const CHAR16 *s, CHAR16 c, UINTN n);
+
+/* EFI boot allocator. */
+void *ebmalloc(size_t size);
+void free_ebmalloc_unused_mem(void);

Hi Jan,

On 06/08/2020 10:05, Jan Beulich wrote:
> We're gaining such sections, and like .text.* and .data.* they shouldn't
> be present in objects subject to automatic to-init conversion. Oddly
> enough for quite some time we did have an instance breaking this rule,
> which gets fixed at this occasion, by breaking out the EFI boot
> allocator functions into its own translation unit.
> 
> Fixes: c5b9805bc1f7 ("efi: create new early memory allocator")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

For the Arm bits:

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

Cheers,

-- 
Julien Grall

On 06/08/2020 10:05, Jan Beulich wrote:
> We're gaining such sections, and like .text.* and .data.* they shouldn't
> be present in objects subject to automatic to-init conversion. Oddly
> enough for quite some time we did have an instance breaking this rule,
> which gets fixed at this occasion, by breaking out the EFI boot
> allocator functions into its own translation unit.
>
> Fixes: c5b9805bc1f7 ("efi: create new early memory allocator")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
> This likely has a (weak) dependency on "x86/EFI: sanitize build logic"
> sent several weeks ago, due to the new source file added, as explicit
> dependencies upon the individual objects in x86/Makefile go away there.
>
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -355,7 +355,7 @@ $(TARGET): delete-unfresh-files
>  	$(MAKE) -C tools
>  	$(MAKE) -f $(BASEDIR)/Rules.mk include/xen/compile.h
>  	[ -e include/asm ] || ln -sf asm-$(TARGET_ARCH) include/asm
> -	[ -e arch/$(TARGET_ARCH)/efi ] && for f in boot.c runtime.c compat.c efi.h;\
> +	[ -e arch/$(TARGET_ARCH)/efi ] && for f in $$(cd common/efi; echo *.[ch]); \
>  		do test -r arch/$(TARGET_ARCH)/efi/$$f || \
>  		   ln -nsf ../../../common/efi/$$f arch/$(TARGET_ARCH)/efi/; \
>  		done; \

Maybe not for this patch, but we need to start removing this (and other)
symlinking in the tree for proper out-of-tree builds to work.

AFAICT, this logic predates both Kconfig and x86's blur into having EFI
support in xen.gz.

Can't we remove all of this by having CONFIG_XEN_PE expressed/selectable
properly in Kconfig, and gathering all the objects normally, rather than
bodging all of common/efi/ through arch/efi/ ?

~Andrew

On 06.08.2020 18:16, Andrew Cooper wrote:
> On 06/08/2020 10:05, Jan Beulich wrote:
>> We're gaining such sections, and like .text.* and .data.* they shouldn't
>> be present in objects subject to automatic to-init conversion. Oddly
>> enough for quite some time we did have an instance breaking this rule,
>> which gets fixed at this occasion, by breaking out the EFI boot
>> allocator functions into its own translation unit.
>>
>> Fixes: c5b9805bc1f7 ("efi: create new early memory allocator")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks.

>> ---
>> This likely has a (weak) dependency on "x86/EFI: sanitize build logic"
>> sent several weeks ago, due to the new source file added, as explicit
>> dependencies upon the individual objects in x86/Makefile go away there.
>>
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -355,7 +355,7 @@ $(TARGET): delete-unfresh-files
>>  	$(MAKE) -C tools
>>  	$(MAKE) -f $(BASEDIR)/Rules.mk include/xen/compile.h
>>  	[ -e include/asm ] || ln -sf asm-$(TARGET_ARCH) include/asm
>> -	[ -e arch/$(TARGET_ARCH)/efi ] && for f in boot.c runtime.c compat.c efi.h;\
>> +	[ -e arch/$(TARGET_ARCH)/efi ] && for f in $$(cd common/efi; echo *.[ch]); \
>>  		do test -r arch/$(TARGET_ARCH)/efi/$$f || \
>>  		   ln -nsf ../../../common/efi/$$f arch/$(TARGET_ARCH)/efi/; \
>>  		done; \
> 
> Maybe not for this patch, but we need to start removing this (and other)
> symlinking in the tree for proper out-of-tree builds to work.

Yes, but indeed not right here.

> AFAICT, this logic predates both Kconfig and x86's blur into having EFI
> support in xen.gz.

Yes, it was a result of making parts of that code also usable by Arm64.

> Can't we remove all of this by having CONFIG_XEN_PE expressed/selectable
> properly in Kconfig, and gathering all the objects normally, rather than
> bodging all of common/efi/ through arch/efi/ ?

_If_ we settle on Kconfig to be allowed to check compiler (and linker)
features, then yes. This continues to be a pending topic though, so
the switch can't be made like this at this point in time. (It could be
made a Kconfig item now - which, when enabled, implies the assertion
that a capable tool chain is in use.)

Jan

On 07/08/2020 11:56, Jan Beulich wrote:
> On 06.08.2020 18:16, Andrew Cooper wrote:
>> On 06/08/2020 10:05, Jan Beulich wrote:
>> Can't we remove all of this by having CONFIG_XEN_PE expressed/selectable
>> properly in Kconfig, and gathering all the objects normally, rather than
>> bodging all of common/efi/ through arch/efi/ ?
> _If_ we settle on Kconfig to be allowed to check compiler (and linker)
> features, then yes. This continues to be a pending topic though, so
> the switch can't be made like this at this point in time. (It could be
> made a Kconfig item now - which, when enabled, implies the assertion
> that a capable tool chain is in use.)

I am still of the opinion that nothing needs discussing, but you are
obviously not.

Please raise this as a topic and lets discuss it, because it has a
meaningful impacting on a large number of pending series.

~Andrew

On 07.08.2020 17:12, Andrew Cooper wrote:
> On 07/08/2020 11:56, Jan Beulich wrote:
>> On 06.08.2020 18:16, Andrew Cooper wrote:
>>> On 06/08/2020 10:05, Jan Beulich wrote:
>>> Can't we remove all of this by having CONFIG_XEN_PE expressed/selectable
>>> properly in Kconfig, and gathering all the objects normally, rather than
>>> bodging all of common/efi/ through arch/efi/ ?
>> _If_ we settle on Kconfig to be allowed to check compiler (and linker)
>> features, then yes. This continues to be a pending topic though, so
>> the switch can't be made like this at this point in time. (It could be
>> made a Kconfig item now - which, when enabled, implies the assertion
>> that a capable tool chain is in use.)
> 
> I am still of the opinion that nothing needs discussing, but you are
> obviously not.
> 
> Please raise this as a topic and lets discuss it, because it has a
> meaningful impacting on a large number of pending series.

Preferably I would have put this on this month's community meeting
agenda, but I'll be ooo next week, so that's not going to help, I'm
afraid. I guess I should put it up in email form when I'm back,
albeit I wasn't thinking it should need to be me to start the
discussion. Instead my view was that such a discussion should (have
been, now after-the-fact) be started by whoever wants to introduce
a new feature. You did say this was discussed in Chicago, but while
I'm pretty sure I was in all relevant sessions, I don't think this
can have been mentioned more than just in passing.

Jan

Address at least the primary reason why 52bba67f8b87 ("efi/boot: Don't
free ebmalloc area at all") was put in place: Make xen_in_range() aware
of the freed range. This is in particular relevant for EFI-enabled
builds not actually running on EFI, as the entire range will be unused
in this case.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
The remaining issue could be addressed too, by making the area 2M in
size and 2M-aligned.

--- a/xen/arch/x86/efi/stub.c
+++ b/xen/arch/x86/efi/stub.c
@@ -52,6 +52,12 @@ bool efi_enabled(unsigned int feature)
 
 void __init efi_init_memory(void) { }
 
+bool __hwdom_init efi_boot_mem_unused(unsigned long *start, unsigned long *end)
+{
+    *start = *end = (unsigned long)_end;
+    return false;
+}
+
 void efi_update_l4_pgtable(unsigned int l4idx, l4_pgentry_t l4e) { }
 
 bool efi_rs_using_pgtables(void)
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -608,7 +608,7 @@ static void __init kexec_reserve_area(st
 #endif
 }
 
-static inline bool using_2M_mapping(void)
+bool using_2M_mapping(void)
 {
     return !l1_table_offset((unsigned long)__2M_text_end) &&
            !l1_table_offset((unsigned long)__2M_rodata_start) &&
@@ -1970,7 +1970,7 @@ int __hwdom_init xen_in_range(unsigned l
     paddr_t start, end;
     int i;
 
-    enum { region_s3, region_ro, region_rw, nr_regions };
+    enum { region_s3, region_ro, region_rw, region_bss, nr_regions };
     static struct {
         paddr_t s, e;
     } xen_regions[nr_regions] __hwdom_initdata;
@@ -1994,7 +1994,14 @@ int __hwdom_init xen_in_range(unsigned l
         xen_regions[region_ro].e = __pa(&__2M_rodata_end);
         /* hypervisor .data + .bss */
         xen_regions[region_rw].s = __pa(&__2M_rwdata_start);
-        xen_regions[region_rw].e = __pa(&__2M_rwdata_end);
+        if ( efi_boot_mem_unused(&start, &end) )
+        {
+            xen_regions[region_rw].e = __pa(start);
+            xen_regions[region_bss].s = __pa(end);
+            xen_regions[region_bss].e = __pa(&__2M_rwdata_end);
+        }
+        else
+            xen_regions[region_rw].e = __pa(&__2M_rwdata_end);
     }
 
     start = (paddr_t)mfn << PAGE_SHIFT;
--- a/xen/common/efi/ebmalloc.c
+++ b/xen/common/efi/ebmalloc.c
@@ -1,5 +1,9 @@
 #include "efi.h"
 #include <xen/init.h>
+#include <xen/mm.h>
+#ifdef CONFIG_X86
+#include <asm/setup.h>
+#endif
 
 #ifdef CONFIG_ARM
 /*
@@ -21,7 +25,7 @@
 
 static char __section(".bss.page_aligned") __aligned(PAGE_SIZE)
     ebmalloc_mem[EBMALLOC_SIZE];
-static unsigned long __initdata ebmalloc_allocated;
+static unsigned long __read_mostly ebmalloc_allocated;
 
 /* EFI boot allocator. */
 void __init *ebmalloc(size_t size)
@@ -36,17 +40,32 @@ void __init *ebmalloc(size_t size)
     return ptr;
 }
 
+bool __hwdom_init efi_boot_mem_unused(unsigned long *start, unsigned long *end)
+{
+    *start = (unsigned long)ebmalloc_mem + PAGE_ALIGN(ebmalloc_allocated);
+    *end = (unsigned long)ebmalloc_mem + sizeof(ebmalloc_mem);
+
+    return *start < *end;
+}
+
 void __init free_ebmalloc_unused_mem(void)
 {
-#if 0 /* FIXME: Putting a hole in the BSS breaks the IOMMU mappings for dom0. */
     unsigned long start, end;
 
-    start = (unsigned long)ebmalloc_mem + PAGE_ALIGN(ebmalloc_allocated);
-    end = (unsigned long)ebmalloc_mem + sizeof(ebmalloc_mem);
+#ifdef CONFIG_X86
+    /* FIXME: Putting a hole in .bss would shatter the large page mapping. */
+    if ( using_2M_mapping() )
+    {
+        ebmalloc_allocated = sizeof(ebmalloc_mem);
+        return;
+    }
+#endif
+
+    if ( !efi_boot_mem_unused(&start, &end) )
+        return;
 
     destroy_xen_mappings(start, end);
     init_xenheap_pages(__pa(start), __pa(end));
 
     printk(XENLOG_INFO "Freed %lukB unused BSS memory\n", (end - start) >> 10);
-#endif
 }
--- a/xen/include/asm-x86/setup.h
+++ b/xen/include/asm-x86/setup.h
@@ -9,6 +9,8 @@ extern const char __2M_rodata_start[], _
 extern char __2M_init_start[], __2M_init_end[];
 extern char __2M_rwdata_start[], __2M_rwdata_end[];
 
+bool using_2M_mapping(void);
+
 extern unsigned long xenheap_initial_phys_start;
 extern uint64_t boot_tsc_stamp;
 
--- a/xen/include/xen/efi.h
+++ b/xen/include/xen/efi.h
@@ -33,6 +33,7 @@ struct compat_pf_efi_runtime_call;
 
 bool efi_enabled(unsigned int feature);
 void efi_init_memory(void);
+bool efi_boot_mem_unused(unsigned long *start, unsigned long *end);
 bool efi_rs_using_pgtables(void);
 unsigned long efi_get_time(void);
 void efi_halt_system(void);