diff mbox

[03/04] qemu-kvm: Remove the dependency for phys_ram_base for ipf.c

Message ID 49F80DFC.5060006@sgi.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jes Sorensen April 29, 2009, 8:21 a.m. UTC
Zhang, Xiantao wrote:
> Avi Kivity wrote:
>> I haven't pushed this out yet, so I can apply a replacement patch. 
> 
> We don't need flush_icache_range here, because I believe it is called in cpu_physical_memory_write. 
> Xiantao

Hi Xiantao,

Good point, I hadn't spotted that, and worse I mangled
flush_icache_range() in the process, trying to be smart :-(

Here's a version without cache flush changes, it should do the right
thing<tm>. We still need to look at the nvram stuff, but that is broken
in all setups currently.

What do you think of this one?

Cheers,
Jes

Comments

Zhang, Xiantao April 29, 2009, 8:35 a.m. UTC | #1
Jes Sorensen wrote:
> Zhang, Xiantao wrote:
>> Avi Kivity wrote:
>>> I haven't pushed this out yet, so I can apply a replacement patch.
>> 
>> We don't need flush_icache_range here, because I believe it is
>> called in cpu_physical_memory_write. Xiantao
> 
> Hi Xiantao,
> 
> Good point, I hadn't spotted that, and worse I mangled
> flush_icache_range() in the process, trying to be smart :-(
> 
> Here's a version without cache flush changes, it should do the right
> thing<tm>. We still need to look at the nvram stuff, but that is
> broken in all setups currently.
> 
> What do you think of this one?

Hi, Jes
    Except nvram stuff, I don't see the difference with my patch. Could you provide an incremental patch to fix nvram stuff ? :)
Xiantao

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jes Sorensen April 29, 2009, 8:42 a.m. UTC | #2
Zhang, Xiantao wrote:
> Jes Sorensen wrote:
>> What do you think of this one?
> 
> Hi, Jes
>     Except nvram stuff, I don't see the difference with my patch. Could you provide an incremental patch to fix nvram stuff ? :)
> Xiantao

Hi Xiantao,

The main difference is that my patch cleans up the interfaces and calls
to the various functions, and removes a bunch of global variables as
well.

The problem with the NVRAM stuff is that I don't know how it is supposed
to work. Do you have any specification or documentation for it?

Cheers,
Jes
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhang, Xiantao April 29, 2009, 2:02 p.m. UTC | #3
Jes Sorensen wrote:
> Zhang, Xiantao wrote:
>> Jes Sorensen wrote:
>>> What do you think of this one?
>> 
>> Hi, Jes
>>     Except nvram stuff, I don't see the difference with my patch.
>> Could you provide an incremental patch to fix nvram stuff ? :)
>> Xiantao 
> 
> Hi Xiantao,
> 
> The main difference is that my patch cleans up the interfaces and
> calls to the various functions, and removes a bunch of global
> variables as well. 

I still can't see the difference with the patch in Avi's tree except nvram stuff.  And I believe the global variable you mentioned should be only used for nvram. So I propose an incremental patch for that. :)

> The problem with the NVRAM stuff is that I don't know how it is
> supposed to work. Do you have any specification or documentation for
> it? 
No specification, and we just followed the guest's firmware logic for that. The link for guest firmware: http://xenbits.xensource.com/ext/efi-vfirmware.hg
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jes Sorensen April 29, 2009, 3:07 p.m. UTC | #4
Zhang, Xiantao wrote:
> Jes Sorensen wrote:
>> The main difference is that my patch cleans up the interfaces and
>> calls to the various functions, and removes a bunch of global
>> variables as well. 
> 
> I still can't see the difference with the patch in Avi's tree except nvram stuff.  And I believe the global variable you mentioned should be only used for nvram. So I propose an incremental patch for that. :)

Let me try and take a look - last I pulled from Avi, those patches
didn't show up in git.

Been trying to get the mapping and alias stuff done, so probably won't
get to this until tomorrow.

Cheers,
jes
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jes Sorensen April 30, 2009, 9:11 a.m. UTC | #5
Zhang, Xiantao wrote:
> Jes Sorensen wrote:
>> The main difference is that my patch cleans up the interfaces and
>> calls to the various functions, and removes a bunch of global
>> variables as well. 
> 
> I still can't see the difference with the patch in Avi's tree except nvram stuff.  And I believe the global variable you mentioned should be only used for nvram. So I propose an incremental patch for that. :)

Hi Xiantao,

I cannot see your patch in Avi's tree, would you mind sending me the
latest version by email, so I can look into this?

Thanks,
Jes
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity April 30, 2009, 9:23 a.m. UTC | #6
Jes Sorensen wrote:
> Zhang, Xiantao wrote:
>> Jes Sorensen wrote:
>>> The main difference is that my patch cleans up the interfaces and
>>> calls to the various functions, and removes a bunch of global
>>> variables as well. 
>>
>> I still can't see the difference with the patch in Avi's tree except 
>> nvram stuff.  And I believe the global variable you mentioned should 
>> be only used for nvram. So I propose an incremental patch for that. :)
>
> Hi Xiantao,
>
> I cannot see your patch in Avi's tree, would you mind sending me the
> latest version by email, so I can look into this?
>

I pushed my queue into a branch (named 'queue').  Will merge once I 
resolve the regressions here.
Jes Sorensen April 30, 2009, 2:09 p.m. UTC | #7
Avi Kivity wrote:
> Jes Sorensen wrote:
> I pushed my queue into a branch (named 'queue').  Will merge once I 
> resolve the regressions here.
> 

Hi Avi,

I don't see that branch - it's in the qemu-kvm repo?

Cheers,
Jes

[jes@leavenworth qemu-kvm]$ git branch -a
* master
   origin/HEAD
   origin/bios-merge
   origin/bios-patchqueue
   origin/bochs-bios-cvs
   origin/bochs-bios-vendor-drops
   origin/build
   origin/for-glommer
   origin/ia64-vtd
   origin/irq-routing-2
   origin/kvm-updates-2.6.25
   origin/kvm-updates-2.6.26
   origin/kvm-updates-2.6.27
   origin/kvm-updates/2.6.26
   origin/kvm-updates/2.6.27
   origin/kvm-updates/2.6.28
   origin/kvm-updates/2.6.29
   origin/kvm-updates/2.6.30
   origin/maint/2.6.25
   origin/maint/2.6.26
   origin/maint/2.6.26-test
   origin/maint/2.6.28
   origin/maint/2.6.29
   origin/maint/2.6.30
   origin/master
   origin/merge-tmp
   origin/origin
   origin/pending
   origin/qemu-cvs
   origin/qemu-vendor-drops
   origin/realmode
   origin/release
[jes@leavenworth qemu-kvm]$
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity April 30, 2009, 3:22 p.m. UTC | #8
Jes Sorensen wrote:
> Avi Kivity wrote:
>> Jes Sorensen wrote:
>> I pushed my queue into a branch (named 'queue').  Will merge once I 
>> resolve the regressions here.
>>
>
> Hi Avi,
>
> I don't see that branch - it's in the qemu-kvm repo?
>

Did you run 'git fetch origin'?
diff mbox

Patch

Remove ia64 dependency on phys_ram_base and assumption that
phys_ram_base + qemu_alloc_ram() is always valid. Use
cpu_physical_memory_{read,write} instead of memcpy.

The behavior of the NVRAM code is questionable, but this code should
behave the same as the old code ... it still needs to be fixed.

Signed-off-by: Jes Sorensen <jes@sgi.com>

---
 hw/ipf.c               |   46 ++++++++++-------------------
 target-ia64/firmware.c |   76 ++++++++++++++++++++++++++++++++-----------------
 target-ia64/firmware.h |    8 +----
 3 files changed, 70 insertions(+), 60 deletions(-)

Index: qemu-kvm/hw/ipf.c
===================================================================
--- qemu-kvm.orig/hw/ipf.c
+++ qemu-kvm/hw/ipf.c
@@ -54,7 +54,6 @@ 
 static RTCState *rtc_state;
 static PCIDevice *i440fx_state;
 
-uint8_t *g_fw_start;
 static uint32_t ipf_to_legacy_io(target_phys_addr_t addr)
 {
     return (uint32_t)(((addr&0x3ffffff) >> 12 << 2)|((addr) & 0x3));
@@ -453,16 +452,14 @@ 
     /*Load firware to its proper position.*/
     if (kvm_enabled()) {
         unsigned long  image_size;
-        char *image = NULL;
-        uint8_t *fw_image_start;
+        uint8_t *image = NULL;
         unsigned long nvram_addr = 0;
         unsigned long nvram_fd = 0;
         unsigned long type = READ_FROM_NVRAM;
         unsigned long i = 0;
-        ram_addr_t fw_offset = qemu_ram_alloc(GFW_SIZE);
-        uint8_t *fw_start = phys_ram_base + fw_offset;
+        unsigned long fw_offset;
+        ram_addr_t fw_mem = qemu_ram_alloc(GFW_SIZE);
 
-        g_fw_start = fw_start;
         snprintf(buf, sizeof(buf), "%s/%s", bios_dir, FW_FILENAME);
         image = read_image(buf, &image_size );
         if (NULL == image || !image_size) {
@@ -470,28 +467,26 @@ 
             fprintf(stderr, "Please check Guest firmware at %s\n", buf);
             exit(1);
         }
-        fw_image_start = fw_start + GFW_SIZE - image_size;
+        fw_offset = GFW_START + GFW_SIZE - image_size;
 
-        cpu_register_physical_memory(GFW_START, GFW_SIZE, fw_offset);
-        memcpy(fw_image_start, image, image_size);
+        cpu_register_physical_memory(GFW_START, GFW_SIZE, fw_mem);
+        cpu_physical_memory_write(fw_offset, image, image_size);
 
         free(image);
-        flush_icache_range((unsigned long)fw_image_start,
-                           (unsigned long)fw_image_start + image_size);
 
         nvram_addr = NVRAM_START;
         if (nvram) {
             nvram_fd = kvm_ia64_nvram_init(type);
             if (nvram_fd != -1) {
-                kvm_ia64_copy_from_nvram_to_GFW(nvram_fd, g_fw_start);
+                kvm_ia64_copy_from_nvram_to_GFW(nvram_fd);
                 close(nvram_fd);
             }
             i = atexit((void *)kvm_ia64_copy_from_GFW_to_nvram);
             if (i != 0)
                 fprintf(stderr, "cannot set exit function\n");
         }
-        kvm_ia64_build_hob(ram_size + above_4g_mem_size, smp_cpus,
-                           fw_start, nvram_addr);
+
+        kvm_ia64_build_hob(ram_size + above_4g_mem_size, smp_cpus, nvram_addr);
     }
 
     /*Register legacy io address space, size:64M*/
@@ -512,21 +507,15 @@ 
     }
 
     if (cirrus_vga_enabled) {
-        if (pci_enabled) {
-            pci_cirrus_vga_init(pci_bus, phys_ram_base + vga_ram_addr,
-                                vga_ram_addr, vga_ram_size);
-        } else {
-            isa_cirrus_vga_init(phys_ram_base + vga_ram_addr,
-                                vga_ram_addr, vga_ram_size);
-        }
+        if (pci_enabled)
+            pci_cirrus_vga_init(pci_bus, vga_ram_size);
+        else
+            isa_cirrus_vga_init(vga_ram_size);
     } else {
-        if (pci_enabled) {
-            pci_vga_init(pci_bus, phys_ram_base + vga_ram_addr,
-                         vga_ram_addr, vga_ram_size, 0, 0);
-        } else {
-            isa_vga_init(phys_ram_base + vga_ram_addr,
-                         vga_ram_addr, vga_ram_size);
-        }
+        if (pci_enabled)
+            pci_vga_init(pci_bus, vga_ram_size, 0, 0);
+        else
+            isa_vga_init(vga_ram_size);
     }
 
     rtc_state = rtc_init(0x70, i8259[8], 2000);
@@ -671,7 +660,6 @@ 
     .name = "itanium",
     .desc = "Itanium Platform",
     .init = (QEMUMachineInitFunc *)ipf_init_pci,
-    .ram_require = (ram_addr_t)(VGA_RAM_SIZE + GFW_SIZE),
     .max_cpus = 255,
 };
 
Index: qemu-kvm/target-ia64/firmware.c
===================================================================
--- qemu-kvm.orig/target-ia64/firmware.c
+++ qemu-kvm/target-ia64/firmware.c
@@ -91,12 +91,11 @@ 
 static int build_hob(void *hob_buf, unsigned long hob_buf_size,
                      unsigned long dom_mem_size, unsigned long vcpus,
                      unsigned long nvram_addr);
-static int load_hob(void *hob_buf,
-                    unsigned long dom_mem_size, void* hob_start);
+static int load_hob(void *hob_buf, unsigned long dom_mem_size);
 
 int
 kvm_ia64_build_hob(unsigned long memsize, unsigned long vcpus,
-                   uint8_t *fw_start, unsigned long nvram_addr)
+                   unsigned long nvram_addr)
 {
     char   *hob_buf;
 
@@ -111,7 +110,8 @@ 
         Hob_Output("Could not build hob");
         return -1;
     }
-    if (load_hob(hob_buf, memsize, fw_start + HOB_OFFSET) < 0) {
+
+    if (load_hob(hob_buf, memsize) < 0) {
         free(hob_buf);
         Hob_Output("Could not load hob");
         return -1;
@@ -249,7 +249,7 @@ 
     return -1;
 }
 static int
-load_hob(void *hob_buf, unsigned long dom_mem_size, void* hob_start)
+load_hob(void *hob_buf, unsigned long dom_mem_size)
 {
     int hob_size;
 
@@ -263,7 +263,9 @@ 
         Hob_Output("No enough memory for hob data");
         return -1;
     }
-    memcpy (hob_start, hob_buf, hob_size);
+
+    cpu_physical_memory_write(GFW_HOB_START, hob_buf, hob_size);
+
     return 0;
 }
 
@@ -526,11 +528,11 @@ 
     return 0;
 }
 
-char *read_image(const char *filename, unsigned long *size)
+uint8_t *read_image(const char *filename, unsigned long *size)
 {
     int kernel_fd = -1;
     gzFile kernel_gfd = NULL;
-    char *image = NULL, *tmp;
+    uint8_t *image = NULL, *tmp;
     unsigned int bytes;
 
     if ((filename == NULL) || (size == NULL))
@@ -643,41 +645,63 @@ 
 }
 
 int
-kvm_ia64_copy_from_nvram_to_GFW(unsigned long nvram_fd,
-                                const uint8_t *fw_start)
+kvm_ia64_copy_from_nvram_to_GFW(unsigned long nvram_fd)
 {
     struct stat file_stat;
+    uint8_t *nvram_buf;
+    int r = 0;
+
+    nvram_buf = malloc(NVRAM_SIZE);
+
     if ((fstat(nvram_fd, &file_stat) < 0) ||
         (NVRAM_SIZE  != file_stat.st_size) ||
-        (read(nvram_fd, (void *)(fw_start + NVRAM_OFFSET),
-              NVRAM_SIZE) != NVRAM_SIZE))
-        return -1;
-    return 0;
+        (read(nvram_fd, nvram_buf, NVRAM_SIZE) != NVRAM_SIZE)) {
+        r = -1;
+        goto out;
+    }
+
+    cpu_physical_memory_write(NVRAM_START, nvram_buf, NVRAM_SIZE);
+
+ out:
+    free(nvram_buf);
+    return r;
 }
 
 int
 kvm_ia64_copy_from_GFW_to_nvram()
 {
+    struct nvram_save_addr nvram_addr_buf;
+    uint8_t *nvram_buf;
     unsigned long nvram_fd;
     unsigned long type = WRITE_TO_NVRAM;
-    unsigned long *nvram_addr = (unsigned long *)(g_fw_start + NVRAM_OFFSET);
+    int ret = -1;
+
+    nvram_buf = malloc(NVRAM_SIZE);
+    if (!nvram_buf)
+        goto out_free;
+
+    cpu_physical_memory_read(NVRAM_START, (uint8_t *)&nvram_addr_buf,
+                             sizeof(struct nvram_save_addr));
+    if (nvram_addr_buf.signature != NVRAM_VALID_SIG) {
+        goto out_free;
+    }
+
+    cpu_physical_memory_read(nvram_addr_buf.addr, nvram_buf, NVRAM_SIZE);
+
     nvram_fd = kvm_ia64_nvram_init(type);
     if (nvram_fd  == -1)
         goto out;
-    if (((struct nvram_save_addr *)nvram_addr)->signature != NVRAM_VALID_SIG) {
-        close(nvram_fd);
-        goto out;
-    }
+
     lseek(nvram_fd, 0, SEEK_SET);
-    if (write(nvram_fd, ((void *)(((struct nvram_save_addr *)nvram_addr)->addr +
-        (char *)phys_ram_base)), NVRAM_SIZE) != NVRAM_SIZE) {
-        close(nvram_fd);
+    if (write(nvram_fd, nvram_buf, NVRAM_SIZE) != NVRAM_SIZE)
         goto out;
-    }
+
+    ret = 0;
+ out:
     close(nvram_fd);
-    return 0;
-out:
-    return -1;
+ out_free:
+    free(nvram_buf);
+    return ret;
 }
 
 /*
Index: qemu-kvm/target-ia64/firmware.h
===================================================================
--- qemu-kvm.orig/target-ia64/firmware.h
+++ qemu-kvm/target-ia64/firmware.h
@@ -52,13 +52,11 @@ 
 };
 
 extern const char *nvram;
-extern uint8_t *g_fw_start;
 extern int kvm_ia64_build_hob(unsigned long memsize, unsigned long vcpus,
-                              uint8_t *fw_start, unsigned long nvram_addr);
-extern char *read_image(const char *filename, unsigned long *size);
+                              unsigned long nvram_addr);
+extern uint8_t *read_image(const char *filename, unsigned long *size);
 
 extern int kvm_ia64_copy_from_GFW_to_nvram(void);
 extern int kvm_ia64_nvram_init(unsigned long type);
-extern int kvm_ia64_copy_from_nvram_to_GFW(unsigned long nvram_fd,
-                                           const uint8_t *fw_start);
+extern int kvm_ia64_copy_from_nvram_to_GFW(unsigned long nvram_fd);
 #endif //__FIRM_WARE_