[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [v8][PATCH 06/17] tools/libxc: check if modules space is overlapping with reserved device memory



On 2014/12/3 3:55, Konrad Rzeszutek Wilk wrote:
On Mon, Dec 01, 2014 at 05:24:24PM +0800, Tiejun Chen wrote:
In case of reserved device memory overlapping with ram, it also probably

s/also//

Fixed.

overlap with modules space so we need to check these reserved device
s/overlap/overlaps/

Fixed.


What is 'modules space'?

Please see modules_init(), and looks it includs acpi and smbios currently.


memory as well.

s/reserved device memory/E820_RSV/ ?

I don't find we have such a definition.



Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
---
  tools/libxc/xc_hvm_build_x86.c | 94 +++++++++++++++++++++++++++++++++++-------
  1 file changed, 79 insertions(+), 15 deletions(-)

diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
index c81a25b..ddcf06d 100644
--- a/tools/libxc/xc_hvm_build_x86.c
+++ b/tools/libxc/xc_hvm_build_x86.c
@@ -54,9 +54,82 @@

  #define VGA_HOLE_SIZE (0x20)

+/*
+ * Check whether there exists mmio hole in the specified memory range.
+ * Returns 1 if exists, else returns 0.
+ */
+static int check_mmio_hole(uint64_t start, uint64_t memsize,
+                           uint64_t mmio_start, uint64_t mmio_size)
+{
+    if ( start + memsize <= mmio_start || start >= mmio_start + mmio_size )
+        return 0;
+    else
+        return 1;
+}
+
+/* Getting all reserved device memory map info. */
+static struct xen_reserved_device_memory
+*xc_get_reserved_device_memory_map(xc_interface *xch, unsigned int nr_entries,
+                                   uint32_t dom)
+{
+    struct xen_reserved_device_memory *xrdm = NULL;
+    int rc = xc_reserved_device_memory_map(xch, dom, xrdm, &nr_entries);
+
+    if ( rc < 0 )
+    {
+        if ( errno == ENOBUFS )
+        {
+            if ( (xrdm = malloc(nr_entries *
+                                sizeof(xen_reserved_device_memory_t))) == NULL 
)
+            {
+                PERROR("Could not allocate memory.");
+                return 0;
+            }
+            rc = xc_reserved_device_memory_map(xch, dom, xrdm, &nr_entries);
+            if ( rc )
+            {
+                PERROR("Could not get reserved device memory maps.");
+                free(xrdm);
+                return 0;

Uhhh, is that the right error to return?

Don't you mean ERR_PTR logic? Or 'return NULL' ?

OOPS, return NULL.



+            }
+        }
+        else
+            PERROR("Could not get reserved device memory maps.");
+    }
+
+    return xrdm;
+}
+
+static int xc_check_modules_space(xc_interface *xch, uint64_t *mstart_out,
+                                  uint64_t *mend_out, uint32_t dom)
+{
+    unsigned int i = 0, nr_entries = 0;
+    uint64_t rdm_start = 0, rdm_end = 0;
+    struct xen_reserved_device_memory *rdm_map =
+                        xc_get_reserved_device_memory_map(xch, nr_entries, 
dom);
+

You need to check whether 'rdm_map' is NULL.

You're right.

Actually nr_entries is always 0 if rdm_map is NULL with my original design. But now this should be checked as you mentioned, so

+    if ( !rdm_map )
+        return 0;
+


+    for ( i = 0; i < nr_entries; i++ )
+    {
+        rdm_start = (uint64_t)rdm_map[i].start_pfn << XC_PAGE_SHIFT;
+        rdm_end = rdm_start + ((uint64_t)rdm_map[i].nr_pages << XC_PAGE_SHIFT);
+
+        /* Just use check_mmio_hole() to check modules ranges. */
+        if ( check_mmio_hole(rdm_start,
+                             rdm_end - rdm_start,
+                             *mstart_out, *mend_out) )
+            return -1;
+    }
+
+    free(rdm_map);
+
+    return 0;
+}
+
  static int modules_init(struct xc_hvm_build_args *args,
                          uint64_t vend, struct elf_binary *elf,
-                        uint64_t *mstart_out, uint64_t *mend_out)
+                        uint64_t *mstart_out, uint64_t *mend_out,
+                        xc_interface *xch,
+                        uint32_t dom)
  {
  #define MODULE_ALIGN 1UL << 7
  #define MB_ALIGN     1UL << 20
@@ -80,6 +153,10 @@ static int modules_init(struct xc_hvm_build_args *args,
      if ( *mend_out > vend )
          return -1;

+    /* Is it overlapping with reserved device memory? */
+    if ( xc_check_modules_space(xch, mstart_out, mend_out, dom) )
+        return -1;
+
      if ( args->acpi_module.length != 0 )
          args->acpi_module.guest_addr_out = *mstart_out;
      if ( args->smbios_module.length != 0 )
@@ -226,19 +303,6 @@ static int loadmodules(xc_interface *xch,
      return rc;
  }

-/*
- * Check whether there exists mmio hole in the specified memory range.
- * Returns 1 if exists, else returns 0.
- */
-static int check_mmio_hole(uint64_t start, uint64_t memsize,
-                           uint64_t mmio_start, uint64_t mmio_size)
-{
-    if ( start + memsize <= mmio_start || start >= mmio_start + mmio_size )
-        return 0;
-    else
-        return 1;
-}
-

This movement of 'check_mmio_hole' needs to be a seperate patch.

Okay.

Thanks
Tiejun


  static int setup_guest(xc_interface *xch,
                         uint32_t dom, struct xc_hvm_build_args *args,
                         char *image, unsigned long image_size)
@@ -282,7 +346,7 @@ static int setup_guest(xc_interface *xch,
          goto error_out;
      }

-    if ( modules_init(args, v_end, &elf, &m_start, &m_end) != 0 )
+    if ( modules_init(args, v_end, &elf, &m_start, &m_end, xch, dom) != 0 )
      {
          ERROR("Insufficient space to load modules.");
          goto error_out;
--
1.9.1



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.