[coreboot] New patch to review for coreboot: e787b43 libpayload: Always use virtual pointers in struct sysinfo_t

[Nico Huber (nico.huber@secunet.com)]

Nico Huber (nico.huber at secunet.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/1855

-gerrit

commit e787b438000b24934caad53185028558f9a82ec6
Author: Nico Huber <nico.huber at secunet.com>
Date:   Tue Nov 13 17:11:01 2012 +0100

    libpayload: Always use virtual pointers in struct sysinfo_t
    
    We had mixed virtual and physical pointers in struct sysinfo_t. Some
    being virtual by accident which led to problems when we tried to
    reinitialize lib_sysinfo after relocating FILO (to get intentionally
    virtual pointers valid again). I guess this didn't cause much trouble
    before, as lib_get_sysinfo() was always called with physical addresses
    being equal to their virtual counterparts.
    
    For FILO, two possibilities seem practical: Either, have all pointers in
    struct sysinfo_t physical, so relocation doesn't hurt. Or, have all
    pointers virtual and call lib_get_sysinfo() again after relocation.
    
    This patch goes the latter way, changing the following pointers for
    situations where virtual pointers differ from physical:
      .extra_version
      .build
      .compile_time
      .compile_by
      .compile_host
      .compile_domain
      .compiler
      .linker
      .assembler
      .cb_version
      .vdat_addr
      .tstamp_table
      .cbmem_cons
      .mrc_cache
    We could also just correct the accidentally virtual pointers. But, IMO,
    this would lower the risk of future confusion.
    
    Note 1: Looks like .version gets never set.
    
    Note 2: .option_table and .framebuffer were virtual pointers but treated
            like physical ones. Even in FILO, this led to no problems as
            they were set before relocation.
    
    Change-Id: I4c456f56f049d9f8fc40e62520b1d8ec3dad48f8
    Signed-off-by: Nico Huber <nico.huber at secunet.com>
---
 payloads/libpayload/arch/i386/coreboot.c       | 21 +++++++++------------
 payloads/libpayload/drivers/options.c          |  2 +-
 payloads/libpayload/drivers/video/corebootfb.c |  6 +++---
 3 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/payloads/libpayload/arch/i386/coreboot.c b/payloads/libpayload/arch/i386/coreboot.c
index 5037011..fc22d9a 100644
--- a/payloads/libpayload/arch/i386/coreboot.c
+++ b/payloads/libpayload/arch/i386/coreboot.c
@@ -78,12 +78,6 @@ static void cb_parse_serial(void *ptr, struct sysinfo_t *info)
 	info->serial = ((struct cb_serial *)ptr);
 }
 
-static void cb_parse_version(void *ptr, struct sysinfo_t *info)
-{
-	struct cb_string *ver = ptr;
-	info->cb_version = (char *)ver->string;
-}
-
 #ifdef CONFIG_CHROMEOS
 static void cb_parse_vbnv(unsigned char *ptr, struct sysinfo_t *info)
 {
@@ -109,24 +103,27 @@ static void cb_parse_vdat(unsigned char *ptr, struct sysinfo_t *info)
 {
 	struct cb_vdat *vdat = (struct cb_vdat *) ptr;
 
-	info->vdat_addr = vdat->vdat_addr;
+	info->vdat_addr = phys_to_virt(vdat->vdat_addr);
 	info->vdat_size = vdat->vdat_size;
 }
 #endif
 
 static void cb_parse_tstamp(unsigned char *ptr, struct sysinfo_t *info)
 {
-	info->tstamp_table = ((struct cb_cbmem_tab *)ptr)->cbmem_tab;
+	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
+	info->tstamp_table = phys_to_virt(cbmem->cbmem_tab);
 }
 
 static void cb_parse_cbmem_cons(unsigned char *ptr, struct sysinfo_t *info)
 {
-	info->cbmem_cons = ((struct cb_cbmem_tab *)ptr)->cbmem_tab;
+	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
+	info->cbmem_cons = phys_to_virt(cbmem->cbmem_tab);
 }
 
 static void cb_parse_mrc_cache(unsigned char *ptr, struct sysinfo_t *info)
 {
-	info->mrc_cache = ((struct cb_cbmem_tab *)ptr)->cbmem_tab;
+	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
+	info->mrc_cache = phys_to_virt(cbmem->cbmem_tab);
 }
 
 #ifdef CONFIG_NVRAM
@@ -153,7 +150,7 @@ static void cb_parse_framebuffer(void *ptr, struct sysinfo_t *info)
 
 static void cb_parse_string(unsigned char *ptr, char **info)
 {
-	*info = (char *)((struct cb_string *)ptr)->string;
+	*info = (char *)phys_to_virt(((struct cb_string *)ptr)->string);
 }
 
 static int cb_parse_header(void *addr, int len, struct sysinfo_t *info)
@@ -205,7 +202,7 @@ static int cb_parse_header(void *addr, int len, struct sysinfo_t *info)
 			cb_parse_serial(ptr, info);
 			break;
 		case CB_TAG_VERSION:
-			cb_parse_version(ptr, info);
+			cb_parse_string(ptr, &info->cb_version);
 			break;
 		case CB_TAG_EXTRA_VERSION:
 			cb_parse_string(ptr, &info->extra_version);
diff --git a/payloads/libpayload/drivers/options.c b/payloads/libpayload/drivers/options.c
index a01f977..d15d81b 100644
--- a/payloads/libpayload/drivers/options.c
+++ b/payloads/libpayload/drivers/options.c
@@ -54,7 +54,7 @@ struct nvram_accessor *use_mem = &(struct nvram_accessor) {
 
 struct cb_cmos_option_table *get_system_option_table(void)
 {
-	return phys_to_virt(lib_sysinfo.option_table);
+	return lib_sysinfo.option_table;
 }
 
 int options_checksum_valid(const struct nvram_accessor *nvram)
diff --git a/payloads/libpayload/drivers/video/corebootfb.c b/payloads/libpayload/drivers/video/corebootfb.c
index 0fb1740..5672d1f 100644
--- a/payloads/libpayload/drivers/video/corebootfb.c
+++ b/payloads/libpayload/drivers/video/corebootfb.c
@@ -63,11 +63,11 @@ static const u32 vga_colors[] = {
 };
 
 /* Addresses for the various components */
-static unsigned long fbinfo;
+static struct cb_framebuffer *fbinfo;
 static unsigned long fbaddr;
 static unsigned long chars;
 
-#define FI ((struct cb_framebuffer *) phys_to_virt(fbinfo))
+#define FI (fbinfo)
 #define FB ((unsigned char *) phys_to_virt(fbaddr))
 #define CHARS ((unsigned short *) phys_to_virt(chars))
 
@@ -233,7 +233,7 @@ static int corebootfb_init(void)
 	if (lib_sysinfo.framebuffer == NULL)
 		return -1;
 
-	fbinfo = (unsigned long)lib_sysinfo.framebuffer;
+	fbinfo = lib_sysinfo.framebuffer;
 
 	fbaddr = FI->physical_address;