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

Re: [Xen-devel] [RFC][PATCH 4/5] tools:firmware:hvmloader: reserve RMRR mappings in e820

On 2014/8/8 15:43, Jan Beulich wrote:
On 08.08.14 at 09:30, <tiejun.chen@xxxxxxxxx> wrote:
On 2014/8/8 14:42, Jan Beulich wrote:
You never allocate memory for the map, i.e. you invoke the
hypercall with a NULL second argument. This just happens to work,
but is very unlikely what you intended to do.

Looks scratch_alloc() should be used to allocate in hvmloader, so what
about this?

--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -766,6 +766,16 @@ struct shared_info *get_shared_info(void)
       return shared_info;

+struct e820map *get_rmrr_map_info(void)
+    struct e820map *e820_map = scratch_alloc(sizeof(struct e820map), 0);
+    if ( hypercall_memory_op(XENMEM_RMRR_memory_map, e820_map) != 0 )
+        BUG();
+    return e820_map;

More reasonable, albeit now you lost the fetch-just-once behavior.
Furthermore - do you really need this to be a dynamic allocation?


diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 80d822f..d55773e 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -766,6 +766,17 @@ struct shared_info *get_shared_info(void)
     return shared_info;

+struct e820map *get_rmrr_map_info(void)
+    if ( rmrr_e820map.nr_map )
+        return &rmrr_e820map;
+    if ( hypercall_memory_op(XENMEM_RMRR_memory_map, &rmrr_e820map) != 0 )
+        BUG();
+    return &rmrr_e820map;
 uint16_t get_cpu_mhz(void)
     struct shared_info *shared_info = get_shared_info();
diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
index a70e4aa..433be99 100644
--- a/tools/firmware/hvmloader/util.h
+++ b/tools/firmware/hvmloader/util.h
@@ -236,6 +236,8 @@ unsigned long create_pir_tables(void);
 void smp_initialise(void);

 #include "e820.h"
+struct e820map rmrr_e820map;
+struct e820map *get_rmrr_map_info(void);
 int build_e820_table(struct e820entry *e820,
                      unsigned int lowmem_reserved_base,
                      unsigned int bios_image_base);

The structure you use right now is fixed size. Which gets me to
another point though: Is it said in any part of the VT-d spec that
there is an upper limit to the number of RMRRs? The re-use of the

After look up some relevant info in VT-D specs, I think we have no any direct field to limit the number of RMRRs. Just in 8.1 DMA Remapping Reporting Structure, there are some fields to be referred here:

#1 Length:

in bytes, of the description table including the length of the associated remapping structures.

#2 Remapping Structures[]:

A list of structures. The list will contain one or more DMA Remapping Hardware Unit Definition (DRHD) structures, and zero or more Reserved Memory Region Reporting (RMRR) and Root Port ATS Capability Reporting (ATSR) structures.

Seems no any explicit restriction.


E820 interface structure looks pretty odd here (I simply didn't get
to looking at your patches in full yet).


Xen-devel mailing list



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