From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [RFC][PATCH 4/5] tools:firmware:hvmloader: reserve RMRR mappings in e820 Date: Fri, 08 Aug 2014 10:11:45 +0800 Message-ID: <53E431E1.3070200@intel.com> References: <1407409371-31728-1-git-send-email-tiejun.chen@intel.com> <1407409371-31728-5-git-send-email-tiejun.chen@intel.com> <53E36B10.1050805@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53E36B10.1050805@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , JBeulich@suse.com, ian.jackson@eu.citrix.com, stefano.stabellini@eu.citrix.com, ian.campbell@citrix.com, yang.z.zhang@intel.com, kevin.tian@intel.com Cc: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 2014/8/7 20:03, Andrew Cooper wrote: > On 07/08/14 12:02, Tiejun Chen wrote: >> We need to reserve all RMRR mappings in e820 to avoid any >> potential guest memory conflict. >> >> Signed-off-by: Tiejun Chen >> --- >> tools/firmware/hvmloader/e820.c | 14 ++++++++++++++ >> tools/firmware/hvmloader/e820.h | 6 ++++++ >> tools/firmware/hvmloader/util.c | 13 +++++++++++++ >> tools/firmware/hvmloader/util.h | 1 + >> 4 files changed, 34 insertions(+) >> >> diff --git a/tools/firmware/hvmloader/e820.c b/tools/firmware/hvmloader/e820.c >> index 2e05e93..8cf8c75 100644 >> --- a/tools/firmware/hvmloader/e820.c >> +++ b/tools/firmware/hvmloader/e820.c >> @@ -74,6 +74,8 @@ int build_e820_table(struct e820entry *e820, >> unsigned int bios_image_base) >> { >> unsigned int nr = 0; >> + struct e820map *e820_rmrr_map; >> + unsigned int i = 0; >> >> if ( !lowmem_reserved_base ) >> lowmem_reserved_base = 0xA0000; >> @@ -124,6 +126,18 @@ int build_e820_table(struct e820entry *e820, >> e820[nr].type = E820_RAM; >> nr++; >> >> + /* We'd better reserve RMRR mapping for each VM to avoid potential >> + * memory conflict. >> + */ >> + e820_rmrr_map = get_rmrr_map_info(); >> + for ( i = 0; i <= e820_rmrr_map->nr_map; i++ ) >> + { >> + e820[nr].addr = e820_rmrr_map->map[i].addr; >> + e820[nr].size = e820_rmrr_map->map[i].size + 1; >> + e820[nr].type = E820_RESERVED; >> + nr++; >> + } >> + >> /* >> * Explicitly reserve space for special pages. >> * This space starts at RESERVED_MEMBASE an extends to cover various >> diff --git a/tools/firmware/hvmloader/e820.h b/tools/firmware/hvmloader/e820.h >> index b2ead7f..ae4cc55 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 e820map { >> + int nr_map; > > This value should be unsigned. I'm not sure if we need to change this since here I just copy this from the xen/include/asm-x86/e820.h file, struct e820map { int nr_map; struct e820entry map[E820MAX]; }; > >> + struct e820entry map[E820MAX]; >> +}; >> #endif /* __HVMLOADER_E820_H__ */ >> >> /* >> diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c >> index 80d822f..270700f 100644 >> --- a/tools/firmware/hvmloader/util.c >> +++ b/tools/firmware/hvmloader/util.c >> @@ -766,6 +766,19 @@ struct shared_info *get_shared_info(void) >> return shared_info; >> } >> >> +struct e820map *get_rmrr_map_info(void) >> +{ >> + static struct e820map *e820_map = NULL; >> + >> + if ( e820_map != NULL ) >> + return e820_map; >> + >> + if ( hypercall_memory_op(XENMEM_RMRR_memory_map, e820_map) != 0 ) >> + BUG(); > > This instructs Xen to clobber the memory starting at 0, and works > because HVMLoader is in protected, non-paging mode at this point. I > don't think this is what you meant to do, and will repeatedly make the Sorry I can't understand this explicitly. Here I just want a way to get RMRR mapping info under hvmloader circumstance. Could you elaborate what you mean? Or show me a proper way I should do as you expect. Thanks Tiejun > hypercall rather than caching the result. > >> + >> + return e820_map; >> +} >> + >> 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..26bfb8c 100644 >> --- a/tools/firmware/hvmloader/util.h >> +++ b/tools/firmware/hvmloader/util.h >> @@ -236,6 +236,7 @@ unsigned long create_pir_tables(void); >> void smp_initialise(void); >> >> #include "e820.h" >> +struct e820map *get_rmrr_map_info(void); >> int build_e820_table(struct e820entry *e820, >> unsigned int lowmem_reserved_base, >> unsigned int bios_image_base); > >