[PATCH] [HVM] [RFC] Moving the e820 table creation into hvmloader

[PATCH] [HVM] [RFC] Moving the e820 table creation into hvmloader

[Stefan Berger]

This patch moves the creation of the e820 tables from libxc into the
hvmloader. I have implemented some functions around the e820 tables for
adding and reserving memory areas. I am using these to build the tables
and reserve memory for BIOS and ACPI data. ACPI memory area is only
reserved if ACPI is enabled for the domain.

Signed-off-by: Stefan Berger <stefanb@us.ibm.com>

[PATCH] [HVM] [RFC] Moving the e820 table creation into hvmloader

[Stefan Berger]

[-- Attachment #1: Type: text/plain, Size: 412 bytes --]

Missing patch included this time.

This patch moves the creation of the e820 tables from libxc into the
hvmloader. I have implemented some functions around the e820 tables for
adding and reserving memory areas. I am using these to build the tables
and reserve memory for BIOS and ACPI data. ACPI memory area is only
reserved if ACPI is enabled for the domain.

Signed-off-by: Stefan Berger <stefanb@us.ibm.com>


[-- Attachment #2: e820-move.diff --]
[-- Type: text/x-patch, Size: 18075 bytes --]

Index: root/xen-unstable.hg/tools/firmware/hvmloader/Makefile
===================================================================
--- root.orig/xen-unstable.hg/tools/firmware/hvmloader/Makefile
+++ root/xen-unstable.hg/tools/firmware/hvmloader/Makefile
@@ -40,7 +40,7 @@ OBJCOPY  = objcopy
 CFLAGS  += $(DEFINES) -I. $(XENINC) -fno-builtin -O2 -msoft-float
 LDFLAGS  = -nostdlib -Wl,-N -Wl,-Ttext -Wl,$(LOADADDR)
 
-SRCS = hvmloader.c acpi_madt.c mp_tables.c util.c smbios.c acpi_utils.c
+SRCS = hvmloader.c acpi_madt.c mp_tables.c util.c smbios.c acpi_utils.c e820.c
 OBJS = $(patsubst %.c,%.o,$(SRCS))
 
 .PHONY: all
Index: root/xen-unstable.hg/tools/firmware/hvmloader/e820.c
===================================================================
--- /dev/null
+++ root/xen-unstable.hg/tools/firmware/hvmloader/e820.c
@@ -0,0 +1,189 @@
+/*
+ * e820.c: Functions for handling the e820 table.
+ *
+ * Leendert van Doorn, leendert@watson.ibm.com
+ * Stefan Berger, stefanb@us.ibm.com
+ * Copyright (c) 2005, International Business Machines Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ */
+
+#include "acpi/acpi2_0.h"
+#include <xen/hvm/e820.h>
+#include "util.h"
+#include "hypercall.h"
+#include "e820.h"
+#include <xen/memory.h>
+#include <xenctrl.h>
+
+void
+print_e820_map(struct e820entry *map, int entries)
+{
+	struct e820entry *m;
+
+	if (entries > 32)
+		entries = 32;
+
+	for (m = map; m < &map[entries]; m++) {
+		printf("%08lx%08lx - %08lx%08lx ",
+			(unsigned long) (m->addr >> 32),
+			(unsigned long) (m->addr),
+			(unsigned long) ((m->addr+m->size) >> 32),
+			(unsigned long) ((m->addr+m->size)));
+
+		switch (m->type) {
+		case E820_RAM:
+			printf("(RAM)\n"); break;
+		case E820_RESERVED:
+			printf("(Reserved)\n"); break;
+		case E820_ACPI:
+			printf("(ACPI Data)\n"); break;
+		case E820_NVS:
+			printf("(ACPI NVS)\n"); break;
+		default:
+			printf("(Type %ld)\n", m->type); break;
+		}
+	}
+}
+
+static void e820_init(void)
+{
+	*E820_MAP_NR = 0;
+}
+
+static int e820_add_ram(unsigned char *start, uint64_t addr, uint64_t size)
+{
+        struct e820entry *e820entry = (struct e820entry *)start;
+        uint64_t end = addr + size;
+	int c;
+
+        for (c = 0; c < *E820_MAP_NR;  c++) {
+                /*
+                   check for overlap of new memory region with an existing one
+                 */
+                if ((addr >= e820entry[c].addr &&
+                     addr <  e820entry[c].addr + e820entry[c].size) ||
+                    (end  >= e820entry[c].addr &&
+                     end  <  e820entry[c].addr + e820entry[c].size)) {
+                         return -1;
+                }
+        }
+
+        for (c = 0; c < *E820_MAP_NR; c++) {
+                if (e820entry[c].addr > end) {
+                        memcpy(&e820entry[c+1],
+                               &e820entry[c],
+                               (*E820_MAP_NR + 1 - c) *
+                                     sizeof(struct e820entry));
+                        break;
+                }
+        }
+
+        e820entry[c].addr = addr;
+        e820entry[c].size = size;
+        e820entry[c].type = E820_RAM;
+
+        (*E820_MAP_NR)++;
+	return 0;
+}
+
+uint64_t e820_reserve_range(unsigned char *e820,
+                            uint64_t start, uint64_t size,
+                            uint32_t type)
+{
+        uint64_t rc = 0;
+        struct e820entry *e820entry = (struct e820entry *)e820;
+        int c = 0;
+
+        while (c < (*E820_MAP_NR)) {
+                if (e820entry[c].type  == E820_RAM &&
+                    e820entry[c].addr  <= start    &&
+                    e820entry[c].addr + e820entry[c].size >= start + size) {
+                        int sz = e820entry[c].size - size;
+                        /* beginnings the same ? */
+                        if (e820entry[c].addr < start) {
+                                (*E820_MAP_NR)++;
+                                memmove(&e820entry[c+1],
+                                        &e820entry[c],
+                                        (*E820_MAP_NR - c) *
+                                            sizeof(struct e820entry));
+                                e820entry[c].size = start -
+                                                    e820entry[c].addr;
+                                sz -= e820entry[c].size;
+                                c++;
+                        }
+                        /* are the ends the same? */
+                        if (start + size <
+                            e820entry[c].addr + e820entry[c].size) {
+                                /* not the same */
+                                (*E820_MAP_NR)++;
+                                memmove(&e820entry[c+1],
+                                        &e820entry[c],
+                                        (*E820_MAP_NR - c) *
+                                            sizeof(struct e820entry));
+                                e820entry[c+1].addr = start + size;
+                                e820entry[c+1].size = sz;
+                        }
+                        e820entry[c].addr = start;
+                        e820entry[c].size = size;
+                        e820entry[c].type = type;
+                        rc = start;
+                        break;
+                }
+                c++;
+        }
+        return rc;
+}
+
+
+int build_e820_map(unsigned char *start)
+{
+    unsigned long long mem_size, extra_mem_size = 0;
+    int rc;
+    domid_t domid;
+
+    printf("Building e820 map at %x.\n",start);
+    domid = DOMID_SELF;
+    rc = hypercall_memory_op(XENMEM_current_reservation, &domid);
+    if (rc > 0) {
+        mem_size = rc * PAGE_SIZE;
+        /* account for VGA hole from 0xa0000 - 0xc0000 */
+        mem_size += (0xc0000 - 0xa0000);
+        printf("Got memory: 0x%x bytes\n",mem_size);
+
+        /*
+         * Physical address space from HVM_BELOW_4G_RAM_END to 4G is reserved
+         * for PCI devices MMIO. So if HVM has more than HVM_BELOW_4G_RAM_END
+         * RAM, memory beyond HVM_BELOW_4G_RAM_END will go to 4G above.
+         */
+        if ( mem_size > HVM_BELOW_4G_RAM_END ) {
+             extra_mem_size = mem_size - HVM_BELOW_4G_RAM_END;
+             mem_size = HVM_BELOW_4G_RAM_END;
+        }
+
+        e820_init();
+        e820_add_ram(start, 0x0, mem_size - PAGE_SIZE * 3);
+
+        if ( extra_mem_size )
+            e820_add_ram(start, (1ULL << 32), extra_mem_size);
+
+        /* ??? */
+        e820_reserve_range(start, 0x9F000, 0x1000 , E820_RESERVED);
+        /* BIOS */
+        e820_reserve_range(start, 0xF0000, 0x10000, E820_RESERVED);
+
+        rc = 0;
+    }
+    return rc;
+}
Index: root/xen-unstable.hg/tools/firmware/hvmloader/util.h
===================================================================
--- root.orig/xen-unstable.hg/tools/firmware/hvmloader/util.h
+++ root/xen-unstable.hg/tools/firmware/hvmloader/util.h
@@ -22,6 +22,7 @@ char *strncpy(char *dest, const char *sr
 unsigned strlen(const char *s);
 int memcmp(const void *s1, const void *s2, unsigned n);
 void *memcpy(void *dest, const void *src, unsigned n);
+void *memmove(void *dest, const void *src, unsigned n);
 void *memset(void *s, int c, unsigned n);
 char *itoa(char *a, unsigned int i);
 
@@ -37,5 +38,7 @@ void uuid_to_string(char *dest, uint8_t 
 
 /* Debug output */
 void puts(const char *s);
+int printf(const char *fmt, ...);
+
 
 #endif /* __HVMLOADER_UTIL_H__ */
Index: root/xen-unstable.hg/tools/firmware/hvmloader/e820.h
===================================================================
--- /dev/null
+++ root/xen-unstable.hg/tools/firmware/hvmloader/e820.h
@@ -0,0 +1,15 @@
+#ifndef E820_H
+#define E820_H
+
+#define E820_MAP_NR ((unsigned char *)E820_MAP_PAGE + E820_MAP_NR_OFFSET)
+#define E820_MAP    ((struct e820entry *)(E820_MAP_PAGE + E820_MAP_OFFSET))
+
+void print_e820_map(struct e820entry *map, int entries);
+int build_e820_map(unsigned char *start);
+
+uint64_t e820_reserve_range(unsigned char *e820,
+                            uint64_t start,
+                            uint64_t size,
+                            uint32_t type);
+
+#endif
Index: root/xen-unstable.hg/tools/firmware/hvmloader/util.c
===================================================================
--- root.orig/xen-unstable.hg/tools/firmware/hvmloader/util.c
+++ root/xen-unstable.hg/tools/firmware/hvmloader/util.c
@@ -21,6 +21,12 @@
 #include "acpi/acpi2_0.h"  /* for ACPI_PHYSICAL_ADDRESS */
 #include "util.h"
 #include <stdint.h>
+#include <stdarg.h>
+
+static void putchar(int ch);
+static void _doprint(void (*put)(int), char const *fmt, va_list ap);
+static char *printnum(char *p, unsigned long num, int base);
+
 
 void outw(uint16_t addr, uint16_t val)
 {
@@ -90,6 +96,20 @@ void *memcpy(void *dest, const void *src
 	return dest;
 }
 
+void *memmove(void *dest, const void *src, unsigned n)
+{
+	if ((long)dest > (long)src) {
+		n--;
+		while (n > 0) {
+			((char *)dest)[n] = ((char *)src)[n];
+			n--;
+		}
+	} else {
+		memcpy(dest, src, n);
+	}
+	return dest;
+}
+
 void puts(const char *s)
 {
 	while (*s)
@@ -229,3 +249,112 @@ uuid_to_string(char *dest, uint8_t *uuid
 	}
 	*p = 0;
 }
+
+
+/*
+ * Lightweight printf that doesn't drag in everything under the sun.
+ */
+int
+printf(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	_doprint(putchar, fmt, ap);
+	va_end(ap);
+	return 0; /* for gcc compat */
+}
+
+int
+vprintf(const char *fmt, va_list ap)
+{
+	_doprint(putchar, fmt, ap);
+	return 0; /* for gcc compat */
+}
+
+static void
+putchar(int ch)
+{
+	outb(0xE9, ch);
+}
+
+#define	isdigit(c)	((c) >= '0' && (c) <= '9')
+
+
+/*
+ * A stripped down version of doprint,
+ * but still powerful enough for most tasks.
+ */
+static void
+_doprint(void (*put)(int), char const *fmt, va_list ap)
+{
+	register char *str, c;
+	int lflag, zflag, nflag;
+	char buffer[17];
+	unsigned value;
+	int i, slen, pad;
+
+	for ( ; *fmt != '\0'; fmt++) {
+		pad = zflag = nflag = lflag = 0;
+		if (*fmt == '%') {
+			c = *++fmt;
+			if (c == '-' || isdigit(c)) {
+				if (c == '-') {
+					nflag = 1;
+					c = *++fmt;
+				}
+				zflag = c == '0';
+				for (pad = 0; isdigit(c); c = *++fmt)
+					pad = (pad * 10) + c - '0';
+			}
+			if (c == 'l') { /* long extension */
+				lflag = 1;
+				c = *++fmt;
+			}
+			if (c == 'd' || c == 'u' || c == 'o' || c == 'x') {
+				if (lflag)
+					value = va_arg(ap, unsigned);
+				else
+					value = (unsigned) va_arg(ap, unsigned int);
+				str = buffer;
+				printnum(str, value,
+					c == 'o' ? 8 : (c == 'x' ? 16 : 10));
+				goto printn;
+			} else if (c == 'O' || c == 'D' || c == 'X') {
+				value = va_arg(ap, unsigned);
+				str = buffer;
+				printnum(str, value,
+					c == 'O' ? 8 : (c == 'X' ? 16 : 10));
+			printn:
+				slen = strlen(str);
+				for (i = pad - slen; i > 0; i--)
+					put(zflag ? '0' : ' ');
+				while (*str) put(*str++);
+			} else if (c == 's') {
+				str = va_arg(ap, char *);
+				slen = strlen(str);
+				if (nflag == 0)
+					for (i = pad - slen; i > 0; i--) put(' ');
+				while (*str) put(*str++);
+				if (nflag)
+					for (i = pad - slen; i > 0; i--) put(' ');
+			} else if (c == 'c')
+				put(va_arg(ap, int));
+			else
+				put(*fmt);
+		} else
+			put(*fmt);
+	}
+}
+
+static char *
+printnum(char *p, unsigned long num, int base)
+{
+	unsigned long n;
+
+	if ((n = num/base) > 0)
+		p = printnum(p, n, base);
+	*p++ = "0123456789ABCDEF"[(int)(num % base)];
+	*p = '\0';
+	return p;
+}
Index: root/xen-unstable.hg/tools/firmware/hvmloader/hvmloader.c
===================================================================
--- root.orig/xen-unstable.hg/tools/firmware/hvmloader/hvmloader.c
+++ root/xen-unstable.hg/tools/firmware/hvmloader/hvmloader.c
@@ -29,6 +29,8 @@
 #include "smbios.h"
 #include <xen/version.h>
 #include <xen/hvm/params.h>
+#include <xen/hvm/e820.h>
+#include "e820.h"
 
 /* memory map */
 #define HYPERCALL_PHYSICAL_ADDRESS	0x00080000
@@ -162,6 +164,49 @@ init_hypercalls(void)
 	puts("\n");
 }
 
+static void
+init_acpi(unsigned char *e820_start)
+{
+	if (get_acpi_enabled() != 0) {
+		uint64_t acpi_start, acpi_room;
+		puts("Loading ACPI ...\n");
+		/* ACPI data:
+		   reserve enough memory below the BIOS from the e820 table
+		   for the fixed ACPI table and some entries that may be
+		   dynammically added *behind* the fixed table, i.e.
+		   SSDT entries.
+		*/
+		acpi_room = sizeof(acpi) + (16 * 1024);
+		acpi_room = (acpi_room + 0xff) & ~0xff;
+
+		acpi_start = (uint32_t)e820_reserve_range(e820_start,
+		                          ACPI_PHYSICAL_ADDRESS,
+		                          acpi_room,
+		                          E820_ACPI);
+		if (acpi_start == ACPI_PHYSICAL_ADDRESS) {
+			unsigned char *acpi_ptr = (unsigned char *)
+			                          (unsigned long)acpi_start;
+			unsigned char *freemem;
+ 			unsigned char *limit = (unsigned char *)
+ 			                       (acpi_ptr + acpi_room);
+			acpi_madt_update((unsigned char *)acpi);
+
+			/* copy fixed table */
+			memcpy(acpi_ptr, acpi, sizeof(acpi));
+			/*
+			   free memory for dynamically added acpi data
+			   starts right after the fixed table
+			 */
+			freemem = (acpi_ptr + sizeof(acpi));
+			acpi_update(acpi_ptr,
+			            sizeof(acpi),
+			            limit,
+			            &freemem);
+		}
+	}
+}
+
+
 int
 main(void)
 {
@@ -169,6 +214,9 @@ main(void)
 
 	init_hypercalls();
 
+	/* build e820 memory map */
+	build_e820_map((unsigned char *)E820_MAP);
+
 	puts("Writing SMBIOS tables ...\n");
 	hvm_write_smbios_tables();
 
@@ -187,24 +235,7 @@ main(void)
 			vgabios_stdvga, sizeof(vgabios_stdvga));
 	}
 
-	if (get_acpi_enabled() != 0) {
-		puts("Loading ACPI ...\n");
-		acpi_madt_update((unsigned char *) acpi);
-		if (ACPI_PHYSICAL_ADDRESS+sizeof(acpi) <= 0xF0000) {
-			unsigned char *freemem = (unsigned char *)
-			         (ACPI_PHYSICAL_ADDRESS + sizeof(acpi));
-			/*
-			 * Make sure acpi table does not overlap rombios
-			 * currently acpi less than 8K will be OK.
-			 */
-			 memcpy((void *)ACPI_PHYSICAL_ADDRESS, acpi,
-			 					sizeof(acpi));
-			acpi_update((unsigned char *)ACPI_PHYSICAL_ADDRESS,
-			            sizeof(acpi),
-			            (unsigned char *)0xF0000,
-			            &freemem);
-		}
-	}
+	init_acpi((unsigned char *)E820_MAP);
 
 	if (check_amd()) {
 		/* AMD implies this is SVM */
Index: root/xen-unstable.hg/tools/libxc/xc_hvm_build.c
===================================================================
--- root.orig/xen-unstable.hg/tools/libxc/xc_hvm_build.c
+++ root/xen-unstable.hg/tools/libxc/xc_hvm_build.c
@@ -48,60 +48,6 @@ static void xc_set_hvm_param(int handle,
         PERROR("set HVM parameter failed (%d)", rc);
 }
 
-static void build_e820map(void *e820_page, unsigned long long mem_size)
-{
-    struct e820entry *e820entry =
-        (struct e820entry *)(((unsigned char *)e820_page) + E820_MAP_OFFSET);
-    unsigned long long extra_mem_size = 0;
-    unsigned char nr_map = 0;
-
-    /*
-     * Physical address space from HVM_BELOW_4G_RAM_END to 4G is reserved
-     * for PCI devices MMIO. So if HVM has more than HVM_BELOW_4G_RAM_END
-     * RAM, memory beyond HVM_BELOW_4G_RAM_END will go to 4G above.
-     */
-    if ( mem_size > HVM_BELOW_4G_RAM_END )
-    {
-        extra_mem_size = mem_size - HVM_BELOW_4G_RAM_END;
-        mem_size = HVM_BELOW_4G_RAM_END;
-    }
-
-    e820entry[nr_map].addr = 0x0;
-    e820entry[nr_map].size = 0x9F000;
-    e820entry[nr_map].type = E820_RAM;
-    nr_map++;
-
-    e820entry[nr_map].addr = 0x9F000;
-    e820entry[nr_map].size = 0x1000;
-    e820entry[nr_map].type = E820_RESERVED;
-    nr_map++;
-
-    e820entry[nr_map].addr = 0xEA000;
-    e820entry[nr_map].size = 0x01000;
-    e820entry[nr_map].type = E820_ACPI;
-    nr_map++;
-
-    e820entry[nr_map].addr = 0xF0000;
-    e820entry[nr_map].size = 0x10000;
-    e820entry[nr_map].type = E820_RESERVED;
-    nr_map++;
-
-    /* Low RAM goes here. Remove 3 pages for ioreq, bufioreq, and xenstore. */
-    e820entry[nr_map].addr = 0x100000;
-    e820entry[nr_map].size = mem_size - 0x100000 - PAGE_SIZE * 3;
-    e820entry[nr_map].type = E820_RAM;
-    nr_map++;
-
-    if ( extra_mem_size )
-    {
-        e820entry[nr_map].addr = (1ULL << 32);
-        e820entry[nr_map].size = extra_mem_size;
-        e820entry[nr_map].type = E820_RAM;
-        nr_map++;
-    }
-
-    *(((unsigned char *)e820_page) + E820_MAP_NR_OFFSET) = nr_map;
-}
 
 static void set_hvm_info_checksum(struct hvm_info_table *t)
 {
@@ -164,7 +110,6 @@ static int setup_guest(int xc_handle,
     unsigned long i, nr_pages = (unsigned long)memsize << (20 - PAGE_SHIFT);
     unsigned long shared_page_nr;
     shared_info_t *shared_info;
-    void *e820_page;
     struct domain_setup_info dsi;
     uint64_t v_end;
     int rc;
@@ -239,14 +184,6 @@ static int setup_guest(int xc_handle,
 
     xc_set_hvm_param(xc_handle, dom, HVM_PARAM_PAE_ENABLED, pae);
 
-    if ( (e820_page = xc_map_foreign_range(
-              xc_handle, dom, PAGE_SIZE, PROT_READ | PROT_WRITE,
-              page_array[E820_MAP_PAGE >> PAGE_SHIFT])) == NULL )
-        goto error_out;
-    memset(e820_page, 0, PAGE_SIZE);
-    build_e820map(e820_page, v_end);
-    munmap(e820_page, PAGE_SIZE);
-
     /* shared_info page starts its life empty. */
     if ( (shared_info = xc_map_foreign_range(
               xc_handle, dom, PAGE_SIZE, PROT_READ | PROT_WRITE,

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] [HVM] [RFC] Moving the e820 table creation into hvmloader

[Keir Fraser]

On 7/11/06 18:22, "Stefan Berger" <stefanb@us.ibm.com> wrote:

> This patch moves the creation of the e820 tables from libxc into the
> hvmloader. I have implemented some functions around the e820 tables for
> adding and reserving memory areas. I am using these to build the tables
> and reserve memory for BIOS and ACPI data. ACPI memory area is only
> reserved if ACPI is enabled for the domain.

We can just make the ACPI range statically a page bigger. It's no big loss
(consider that we squander >100kB of memory between 0xC0000 and 0xF0000
right now), and at some point in the near future we'll get rid of the acpi
option and always produce ACPI tables. There are just a few interrupt issues
to sort out first.

The printk patch could be useful, however, since hvmloader is going to get
more complicated in other ways and we'll want to get tracing out of it.

 -- Keir

Re: [PATCH] [HVM] [RFC] Moving the e820 table creation into hvmloader

[Stefan Berger]

[-- Attachment #1.1: Type: text/plain, Size: 1131 bytes --]

Keir Fraser <Keir.Fraser@cl.cam.ac.uk> wrote on 11/07/2006 01:51:50 PM:

> On 7/11/06 18:22, "Stefan Berger" <stefanb@us.ibm.com> wrote:
> 
> > This patch moves the creation of the e820 tables from libxc into the
> > hvmloader. I have implemented some functions around the e820 tables 
for
> > adding and reserving memory areas. I am using these to build the 
tables
> > and reserve memory for BIOS and ACPI data. ACPI memory area is only
> > reserved if ACPI is enabled for the domain.
> 
> We can just make the ACPI range statically a page bigger. It's no big 
loss
> (consider that we squander >100kB of memory between 0xC0000 and 0xF0000
> right now), and at some point in the near future we'll get rid of the 
acpi
> option and always produce ACPI tables. There are just a few interrupt 
issues
> to sort out first.

So I suppose you are not going to take the patch.

> 
> The printk patch could be useful, however, since hvmloader is going to 
get
> more complicated in other ways and we'll want to get tracing out of it.

Which printk patch? Are you going to disable debugging there altogther?

 Stefan

> 
>  -- Keir
> 
> 

[-- Attachment #1.2: Type: text/html, Size: 1534 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] [HVM] [RFC] Moving the e820 table creation into hvmloader

[Stefan Berger]

[-- Attachment #1.1: Type: text/plain, Size: 1126 bytes --]

Keir Fraser <keir@xensource.com> wrote on 11/08/2006 09:41:24 AM:

> On 8/11/06 14:17, "Stefan Berger" <stefanb@us.ibm.com> wrote:

> > We can just make the ACPI range statically a page bigger. It's no big 
loss
> > (consider that we squander >100kB of memory between 0xC0000 and 
0xF0000
> > right now), and at some point in the near future we'll get rid of the 
acpi
> > option and always produce ACPI tables. There are just a few interrupt 
issues
> > to sort out first. 
> 
> So I suppose you are not going to take the patch. 

> Correct. A bunch of complexity there for negligible benefit that I can 
see.

In that case libxc should #include the acpi2_0.h and get its 
ACPI_PHYSICAL_BASE & ACPI_TABLE_SIZE parameters from there.

> 
> > 
> > The printk patch could be useful, however, since hvmloader is going to 
get
> > more complicated in other ways and we'll want to get tracing out of 
it. 
> 
> Which printk patch? Are you going to disable debugging there altogther? 
> 
> The printk stuff that is in the patch you sent, which you took from 
vmxassist.

I will let you apply that part then.


  Stefan
> 
>  -- Keir

[-- Attachment #1.2: Type: text/html, Size: 1594 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] [HVM] [RFC] Moving the e820 table creation into hvmloader

[Stefan Berger]

[-- Attachment #1.1: Type: text/plain, Size: 1471 bytes --]

Keir Fraser <keir@xensource.com> wrote on 11/08/2006 10:57:33 AM:

> On 8/11/06 15:06, "Stefan Berger" <stefanb@us.ibm.com> wrote:

> > Correct. A bunch of complexity there for negligible benefit that I can 
see.
> 
> In that case libxc should #include the acpi2_0.h and get its 
> ACPI_PHYSICAL_BASE & ACPI_TABLE_SIZE parameters from there. 
> 
> Or hvmloader can grok the e820 enough to find where it ought to 
> place the ACPI tables. That can be done with much less code than is 
> required to arbitrarily insert/delete entries in the e820 (which is 
> what I think we can do without). In fact hvmloader can also easily 
> change the size of an existing ACPI e820 entry. So we?d end up with 
> libxc selecting the ACPI base address, and hvmloader picking that up

That base address needs to be the same value as the static ACPI tables 
were built for. Unfortunately the tables are address-dependent. So they 
really should use the same #define...

> and poking the length. This seems a reasonable split of duties to 
> me, and can be done with a small patch.
> 
> I presume your immediate problem is simply that currently the e820 
> entry specifies 4kB for the ACPI region, but you need more if you 
> append the TPM SSDT?

Yes, it's this SSDT that needs some more memory. Also that other spec that 
I'd like to support requires a space of 64kb for logs. I can live with 
less than that, for example 10kb.

   Stefan

> 
>  -- Keir

[-- Attachment #1.2: Type: text/html, Size: 1973 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] [HVM] [RFC] Moving the e820 table creation into hvmloader

[Stefan Berger]

[-- Attachment #1.1: Type: text/plain, Size: 1542 bytes --]

Keir Fraser <keir@xensource.com> wrote on 11/08/2006 11:49:20 AM:

> On 8/11/06 16:26, "Stefan Berger" <stefanb@us.ibm.com> wrote:

> That base address needs to be the same value as the static ACPI 
> tables were built for. Unfortunately the tables are address-
> dependent. So they really should use the same #define... 
> I intend to link the relocation code into hvmloader, rather than 
> running it at build time. So the base address can be dynamic at load 
time.
> 
> > and poking the length. This seems a reasonable split of duties to 
> > me, and can be done with a small patch.
> > 
> > I presume your immediate problem is simply that currently the e820 
> > entry specifies 4kB for the ACPI region, but you need more if you 
> > append the TPM SSDT? 
> 
> Yes, it's this SSDT that needs some more memory. Also that other 
> spec that I'd like to support requires a space of 64kb for logs. I 
> can live with less than that, for example 10kb. 
> 
> Is that to be marked as ACPI too? So we can just tack it on the end 
> of a big ACPI region at, say, 0xE0000? Frankly, for now, we can just

Yes, it has to be marked such that it's not claimed by an OS. Probably 
it's the best to mark it as ACPI and put it into the same memory chunk as 
the rest of ACPI.

   Stefan

> say that all of 0xE0000-0xF0000 is ACPI. We never mark any of that 
> region as E820_RAM, so the memory is not currently usable by the 
> guest anyway. So it?s not like we?re wasting any extra memory by doing 
this.
> 
>  -- Keir

[-- Attachment #1.2: Type: text/html, Size: 2026 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] [HVM] [RFC] Moving the e820 table creation into hvmloader

[Stefan Berger]

[-- Attachment #1.1: Type: text/plain, Size: 957 bytes --]

Keir Fraser <keir@xensource.com> wrote on 11/08/2006 06:08:03 PM:

> 
> 
> 
> On 8/11/06 10:35 pm, "Stefan Berger" <stefanb@us.ibm.com> wrote:

> > Is that to be marked as ACPI too? So we can just tack it on the end 
> > of a big ACPI region at, say, 0xE0000? Frankly, for now, we can just 
> 
> Yes, it has to be marked such that it's not claimed by an OS. 
> Probably it's the best to mark it as ACPI and put it into the same 
> memory chunk as the rest of ACPI. 
> 
> I believe regions marked ACPI are allowed to be reclaimed by the OS 
> after it has parsed the ACPI tables. So perhaps the log memory needs
> to be marked reserved, or not covered by an e820 region at all. 

Probably then 'reserved', although when I put it into the ACPI area it 
worked. The driver that accesses that log will try to map it every time 
one opens a sysfs file.

http://www.linux-m32r.org/lxr/http/source/drivers/char/tpm/tpm_bios.c?v=2.6.18#L386

  Stefan
> 
>  -- Keir

[-- Attachment #1.2: Type: text/html, Size: 1320 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel