Re: [v7][PATCH 13/16] libxl: construct e820 map with RDM information for HVM guest

["Chen, Tiejun" <tiejun.chen@intel.com>]

>>   tools/libxl/libxl_dom.c      |  5 +++
>>   tools/libxl/libxl_internal.h | 24 +++++++++++++
>>   tools/libxl/libxl_x86.c      | 83 ++++++++++++++++++++++++++++++++++++++++++++
> ...
>> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
>> index 62ef120..41da479 100644
>> --- a/tools/libxl/libxl_dom.c
>> +++ b/tools/libxl/libxl_dom.c
>> @@ -1004,6 +1004,11 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
>>           goto out;
>>       }
>>
>> +    if (libxl__domain_construct_e820(gc, d_config, domid, &args)) {
>> +        LOG(ERROR, "setting domain memory map failed");
>> +        goto out;
>> +    }
>
> This is platform-independent code, isn't it ?  In which case this will
> break the build on ARM, I think.
>
> Would an ARM maintainer please confirm.
>

I think you're right. I should make this specific to arch since here 
we're talking e820.

So I tried to refactor this patch,

diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
index d04871c..939178a 100644
--- a/tools/libxl/libxl_arch.h
+++ b/tools/libxl/libxl_arch.h
@@ -49,4 +49,11 @@ int libxl__arch_vnuma_build_vmemrange(libxl__gc *gc,
  _hidden
  int libxl__arch_domain_map_irq(libxl__gc *gc, uint32_t domid, int irq);

+/* arch specific to construct memory mapping function */
+_hidden
+int libxl__arch_domain_construct_memmap(libxl__gc *gc,
+                                        libxl_domain_config *d_config,
+                                        uint32_t domid,
+                                        struct xc_hvm_build_args *args);
+
  #endif
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index f09c860..1526467 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -926,6 +926,14 @@ int libxl__arch_domain_map_irq(libxl__gc *gc, 
uint32_t domid, int irq)
      return xc_domain_bind_pt_spi_irq(CTX->xch, domid, irq, irq);
  }

+int libxl__arch_domain_construct_memmap(libxl__gc *gc,
+                                        libxl_domain_config *d_config,
+                                        uint32_t domid,
+                                        struct xc_hvm_build_args *args)
+{
+    return 0;
+}
+
  /*
   * Local variables:
   * mode: C
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 62ef120..691c1f6 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1004,6 +1004,11 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
          goto out;
      }

+    if (libxl__arch_domain_construct_memmap(gc, d_config, domid, &args)) {
+        LOG(ERROR, "setting domain memory map failed");
+        goto out;
+    }
+
      ret = hvm_build_set_params(ctx->xch, domid, info, state->store_port,
                                 &state->store_mfn, state->console_port,
                                 &state->console_mfn, state->store_domid,
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index ed2bd38..66b3d7f 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -438,6 +438,89 @@ int libxl__arch_domain_map_irq(libxl__gc *gc, 
uint32_t domid, int irq)
  }

  /*
+ * Here we're just trying to set these kinds of e820 mappings:
+ *
+ * #1. Low memory region
+ *
+ * Low RAM starts at least from 1M to make sure all standard regions
+ * of the PC memory map, like BIOS, VGA memory-mapped I/O and vgabios,
+ * have enough space.
+ * Note: Those stuffs below 1M are still constructed with multiple
+ * e820 entries by hvmloader. At this point we don't change anything.
+ *
+ * #2. RDM region if it exists
+ *
+ * #3. High memory region if it exists
+ *
+ * Note: these regions are not overlapping since we already check
+ * to adjust them. Please refer to libxl__domain_device_construct_rdm().
+ */
+#define GUEST_LOW_MEM_START_DEFAULT 0x100000
+int libxl__arch_domain_construct_memmap(libxl__gc *gc,
+                                        libxl_domain_config *d_config,
+                                        uint32_t domid,
+                                        struct xc_hvm_build_args *args)
+{
...


> Aside from that I have no issues with this patch.
>

But if you think I should resend this let me know.

Thanks
Tiejun