[PATCH v2 for-4.16] xen/arm: allocate_bank_memory: don't create memory banks of size zero

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>

allocate_bank_memory can be called with a tot_size of zero, as an
example see the implementation of allocate_memory which can call
allocate_bank_memory with a tot_size of zero for the second memory bank.

If tot_size == 0, don't create an empty memory bank, just return
immediately without error. Otherwise a zero-size memory bank will be
added to the domain device tree.

Note that Linux is known to be able to cope with zero-size memory banks,
and Xen more recently gained the ability to do so as well (5a37207df520
"xen/arm: bootfdt: Ignore empty memory bank"). However, there might be
other non-Linux OSes that are not able to cope with empty memory banks
as well as Linux (and now Xen). It would be more robust to avoid
zero-size memory banks unless required.

Moreover, the code to find empty address regions in make_hypervisor_node
in Xen is not able to cope with empty memory banks today and would
result in a Xen crash. This is only a latent bug because
make_hypervisor_node is only called for Dom0 at present and
allocate_memory is only called for DomU at the moment. (But if
make_hypervisor_node was to be called for a DomU, then the Xen crash
would become manifest.)

Fixes: f2931b4233ec ("xen/arm: introduce allocate_memory")
Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
---
Changes in v2:
- improve commit message
- add in-code comment

In regards to inclusion in 4.16.

If we don't fix this issue in 4.16, default usage of Xen+Linux won't be
affected.

However:
- Non-Linux OSes that cannot cope with zero-size memory banks could
  error out. I am not aware of any but there are so many out there in
  embedded it is impossible to tell.
- downstream Xen calling make_hypervisor_node for DomUs will crash
- future Xen calling make_hypervisor_node for DomUs will have to make
  sure to fix this anyway

If we commit the patch in 4.16, we fix these issue. This patch is only 2
lines of code and very easy to review. The risk is extremely low.

Difficult to say what mistakes could have been made in analysis and
preparation because it is a trivial if-zero-return patch, which is
likely to fix latent bugs rather than introducing instability.

---
 xen/arch/arm/domain_build.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 9e92b640cd..fe38a7c73c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -395,6 +395,14 @@ static bool __init allocate_bank_memory(struct domain *d,
     struct membank *bank;
     unsigned int max_order = ~0;
 
+    /*
+     * allocate_bank_memory can be called with a tot_size of zero for
+     * the second memory bank. It is not an error and we can safely
+     * avoid creating a zero-size memory bank.
+     */
+    if ( tot_size == 0 )
+        return true;
+
     bank = &kinfo->mem.bank[kinfo->mem.nr_banks];
     bank->start = gfn_to_gaddr(sgfn);
     bank->size = tot_size;
-- 
2.25.1