[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [RFC][v3][PATCH 3/6] tools:firmware:hvmloader: reserve RMRR mappings in e820
On 2014/8/15 17:58, Andrew Cooper wrote:
On 15/08/14 09:27, Tiejun Chen wrote:
We need to reserve all RMRR mappings in e820 to avoid any
potential guest memory conflict.
Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
---
tools/firmware/hvmloader/e820.c | 94 +++++++++++++++++++++++++++++++++++++++++
tools/firmware/hvmloader/e820.h | 6 +++
tools/firmware/hvmloader/util.c | 16 +++++++
tools/firmware/hvmloader/util.h | 2 +
4 files changed, 118 insertions(+)
diff --git a/tools/firmware/hvmloader/e820.c b/tools/firmware/hvmloader/e820.c
index 2e05e93..7f54eab 100644
--- a/tools/firmware/hvmloader/e820.c
+++ b/tools/firmware/hvmloader/e820.c
@@ -68,12 +68,89 @@ void dump_e820_table(struct e820entry *e820, unsigned int
nr)
}
}
Please apply Xen coding style to all of this file - there are a lot of
errors.
I asked previously what Xen coding style is, and I always try to follow
some existing code but seems its not enough. Here I still can't find out
what you're pointing, so please tell me exactly what I should change
exactly.
And why don't you guy generate a checkpatch.pl file?
+static unsigned int construct_rmrr_e820_maps(unsigned int nr,
+ struct rmrr_map *e820_rmrr_map,
+ struct e820entry *e820)
+{
+ unsigned int i = 0, j = 0, m = 0, sum_nr = 0;
+ uint64_t start, end, rmrr_start, rmrr_end;
+ unsigned int insert = 0, do_insert = 0;
+
+do_real_construct:
+ sum_nr = nr + e820_rmrr_map->nr_map;
+ for ( i = 0; i < e820_rmrr_map->nr_map; i++ )
+ {
+ rmrr_start = e820_rmrr_map->map[i].addr;
+ rmrr_end = e820_rmrr_map->map[i].addr +
+ e820_rmrr_map->map[i].size + 1;
+
+ for ( j = 0; j < nr; j++ )
+ {
+ end = e820[j].addr + e820[j].size;
+ start = e820[j+1].addr;
+
+ /* Between those existing e820 entries. */
+ if ( (rmrr_start > end) && (rmrr_end < start) )
+ {
+ if (do_insert)
+ {
+ /* Move to free this entry. */
+ for ( m = sum_nr - 1; m > j; m-- )
+ {
+ e820[m].addr = e820[m-1].addr;
+ e820[m].size = e820[m-1].size;
+ e820[m].type = e820[m-1].type;
+ }
+
+ /* Then fill RMRR into that entry. */
+ e820[j+1].addr = rmrr_start;
+ e820[j+1].size = rmrr_end - rmrr_start;
+ e820[j+1].type = E820_RESERVED;
+ nr++;
+ }
+ insert++;
+ }
+ /* Already at the end. */
+ else if ( (rmrr_start > end) && !start )
+ {
+ if (do_insert)
+ {
+ e820[nr].addr = rmrr_start;
+ e820[nr].size = rmrr_end - rmrr_start;
+ e820[nr].type = E820_RESERVED;
+ nr++;
+ }
+ insert++;
+ }
+ }
+ }
+
+ /* Just return if done. */
+ if (do_insert)
+ return nr;
+
+ /* Fine to construct RMRR mappings into e820. */
+ if ( insert == e820_rmrr_map->nr_map)
+ {
+ do_insert = 1;
+ goto do_real_construct;
+ }
+ /* Overlap. */
+ else
+ {
+ printf("RMRR overlap with thoese existing e820 entries!\n");
+ printf("So we don't construct RMRR mapping in e820!\n");
+ }
+
+ return nr;
+}
/* Create an E820 table based on memory parameters provided in hvm_info. */
int build_e820_table(struct e820entry *e820,
unsigned int lowmem_reserved_base,
unsigned int bios_image_base)
{
unsigned int nr = 0;
+ struct rmrr_map *rmrr_maps;
if ( !lowmem_reserved_base )
lowmem_reserved_base = 0xA0000;
@@ -169,6 +246,23 @@ int build_e820_table(struct e820entry *e820,
nr++;
}
+ /* We'd better reserve RMRR mapping for each VM to avoid potential
+ * memory conflict.
+ */
+ rmrr_maps = get_rmrr_map_info();
+ if ( rmrr_maps->nr_map )
+ {
+ if ( (nr + rmrr_maps->nr_map) > E820MAX )
+ {
+ printf(" No free space to insert all RMRR mapping entry!!\n");
+ return nr;
+ }
+ else
+ {
+ nr = construct_rmrr_e820_maps(nr, rmrr_maps, e820);
+ }
+ }
+
return nr;
}
diff --git a/tools/firmware/hvmloader/e820.h b/tools/firmware/hvmloader/e820.h
index b2ead7f..a61b80b 100644
--- a/tools/firmware/hvmloader/e820.h
+++ b/tools/firmware/hvmloader/e820.h
@@ -15,6 +15,12 @@ struct e820entry {
uint32_t type;
} __attribute__((packed));
+#define E820MAX 128
+
+struct rmrr_map {
+ unsigned int nr_map;
+ struct e820entry map[E820MAX];
+};
#endif /* __HVMLOADER_E820_H__ */
/*
diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 80d822f..f63434a 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -766,6 +766,22 @@ struct shared_info *get_shared_info(void)
return shared_info;
}
+struct rmrr_map *get_rmrr_map_info(void)
+{
+ static int no_rmrr = 1;
+
+ if ( no_rmrr == 0 )
+ return &rmrr_e820map;
+
+ if ( hypercall_memory_op(XENMEM_reserved_device_memory_map,
+ &rmrr_e820map) != 0 )
This *only* works because you are not passing a xen_rmrr_memory_map_t as
per the hypercall definition, and Xen is clobbering your heap in a way
which matches the hvmloader layout.
I will check this.
Thanks
Tiejun
+ BUG();
+
+ no_rmrr = 0;
+
+ 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..5f48a86 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 rmrr_map rmrr_e820map;
Unless I am mistaken, this will put an rmrr_map object named
rmrr_e820map in each translation unit which includes util.h I suspect
it is not what you actually want to do.
~Andrew
+struct rmrr_map *get_rmrr_map_info(void);
int build_e820_table(struct e820entry *e820,
unsigned int lowmem_reserved_base,
unsigned int bios_image_base);
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
