Re: [PATCH 03/04] qemu-kvm: Remove the dependency for phys_ram_base

Jes Sorensen <[email protected]>
Newsgroups org.kernel.vger.kvm-ia64,org.kernel.vger.kvm
Message-ID <[email protected]>
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
0100-qemu-ia64-cpu-memory-write.patch (text/x-patch, 9.1 KB)
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 <[email protected]>

---
 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_
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.