Message ID | 706158FABBBA044BAD4FE898A02E4BC236A2BC04@pdsmsx503.ccr.corp.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Zhang, Xiantao wrote: > From aaf97331da3d6cd34522441218c8c9ab3c1067f6 Mon Sep 17 00:00:00 2001 > From: Xiantao Zhang <xiantao.zhang@intel.com> > Date: Tue, 28 Apr 2009 16:55:47 +0800 > Subject: [PATCH] qemu-kvm: Remove the dependency for phys_ram_base for ipf.c > > Upstream has dropped phys_ram_base, so ia64 also remove > the dependency for that. > > +++ b/hw/ipf.c > @@ -54,7 +54,8 @@ static fdctrl_t *floppy_controller; > static RTCState *rtc_state; > static PCIDevice *i440fx_state; > > -uint8_t *g_fw_start; > +ram_addr_t gfw_start; > + > static uint32_t ipf_to_legacy_io(target_phys_addr_t addr) > { > return (uint32_t)(((addr&0x3ffffff) >> 12 << 2)|((addr) & 0x3)); > @@ -454,15 +455,15 @@ static void ipf_init1(ram_addr_t ram_size, int vga_ram_size, > if (kvm_enabled()) { > unsigned long image_size; > char *image = NULL; > - uint8_t *fw_image_start; > + ram_addr_t fw_image_start; > 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; > > - g_fw_start = fw_start; > + ram_addr = qemu_ram_alloc(GFW_SIZE); > + gfw_start = (ram_addr_t)qemu_get_ram_ptr(ram_addr); > qemu_get_ram_ptr() returns a pointer. Don't cast it to a ram_addr_t, leave it a pointer. But why not use cpu_physical_memory_write() (or cpu_physical_memory_write_rom())? It's much simpler and cleaner.
Avi Kivity wrote: > Zhang, Xiantao wrote: >> From aaf97331da3d6cd34522441218c8c9ab3c1067f6 Mon Sep 17 00:00:00 >> 2001 >> From: Xiantao Zhang <xiantao.zhang@intel.com> >> Date: Tue, 28 Apr 2009 16:55:47 +0800 >> Subject: [PATCH] qemu-kvm: Remove the dependency for phys_ram_base >> for ipf.c >> >> Upstream has dropped phys_ram_base, so ia64 also remove >> the dependency for that. >> >> +++ b/hw/ipf.c >> @@ -54,7 +54,8 @@ static fdctrl_t *floppy_controller; static >> RTCState *rtc_state; static PCIDevice *i440fx_state; >> >> -uint8_t *g_fw_start; >> +ram_addr_t gfw_start; >> + >> static uint32_t ipf_to_legacy_io(target_phys_addr_t addr) { >> return (uint32_t)(((addr&0x3ffffff) >> 12 << 2)|((addr) & 0x3)); >> @@ -454,15 +455,15 @@ static void ipf_init1(ram_addr_t ram_size, int >> vga_ram_size, if (kvm_enabled()) { unsigned long >> image_size; char *image = NULL; >> - uint8_t *fw_image_start; >> + ram_addr_t fw_image_start; >> 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; >> >> - g_fw_start = fw_start; >> + ram_addr = qemu_ram_alloc(GFW_SIZE); >> + gfw_start = (ram_addr_t)qemu_get_ram_ptr(ram_addr); >> > > qemu_get_ram_ptr() returns a pointer. Don't cast it to a ram_addr_t, > leave it a pointer. > > But why not use cpu_physical_memory_write() (or > cpu_physical_memory_write_rom())? It's much simpler and cleaner. Good suggestion! I just followed the original logic. Updated the patch. Xiantao
Zhang, Xiantao wrote: >> >> qemu_get_ram_ptr() returns a pointer. Don't cast it to a ram_addr_t, >> leave it a pointer. >> >> But why not use cpu_physical_memory_write() (or >> cpu_physical_memory_write_rom())? It's much simpler and cleaner. >> > > Good suggestion! I just followed the original logic. Updated the patch. > Xiantao Thanks, applied.
>>>>> "Avi" == Avi Kivity <avi@redhat.com> writes: Avi> Zhang, Xiantao wrote: >>> qemu_get_ram_ptr() returns a pointer. Don't cast it to a >>> ram_addr_t, leave it a pointer. >>> >>> But why not use cpu_physical_memory_write() (or >>> cpu_physical_memory_write_rom())? It's much simpler and cleaner. >>> >> Good suggestion! I just followed the original logic. Updated the >> patch. Xiantao Avi> Thanks, applied. Hi, I am not crazy about this patch. You need to use cpy_physical_memory_rw() in the hob and nvram code too, not just in the ipf.c code. What about the flush_icache_range() call you removed - is it safe to just discard that? I was in the process of working through this myself, but I am not quite finished. If you don't mind waiting a couple hours, I should have something a fair bit simpler to solve the same problem. Biggest issue is the flush_icache_range() one. 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 wrote: >>>>>> "Avi" == Avi Kivity <avi@redhat.com> writes: >>>>>> > > Avi> Zhang, Xiantao wrote: > >>>> qemu_get_ram_ptr() returns a pointer. Don't cast it to a >>>> ram_addr_t, leave it a pointer. >>>> >>>> But why not use cpu_physical_memory_write() (or >>>> cpu_physical_memory_write_rom())? It's much simpler and cleaner. >>>> >>>> >>> Good suggestion! I just followed the original logic. Updated the >>> patch. Xiantao >>> > > Avi> Thanks, applied. > > Hi, > > I am not crazy about this patch. You need to use cpy_physical_memory_rw() > in the hob and nvram code too, not just in the ipf.c code. > > What about the flush_icache_range() call you removed - is it safe to > just discard that? > > I was in the process of working through this myself, but I am not > quite finished. If you don't mind waiting a couple hours, I should > have something a fair bit simpler to solve the same problem. > > Biggest issue is the flush_icache_range() one. > > I haven't pushed this out yet, so I can apply a replacement patch.
Jes Sorensen wrote: >>>>>> "Avi" == Avi Kivity <avi@redhat.com> writes: > > Avi> Zhang, Xiantao wrote: >>>> qemu_get_ram_ptr() returns a pointer. Don't cast it to a >>>> ram_addr_t, leave it a pointer. >>>> >>>> But why not use cpu_physical_memory_write() (or >>>> cpu_physical_memory_write_rom())? It's much simpler and cleaner. >>>> >>> Good suggestion! I just followed the original logic. Updated the >>> patch. Xiantao > > Avi> Thanks, applied. > > Hi, Hi, Jes > I am not crazy about this patch. You need to use > cpy_physical_memory_rw() in the hob and nvram code too, not just in > the ipf.c code. Agree, maybe you can make an increment patch for that. > What about the flush_icache_range() call you removed - is it safe to > just discard that?" Yes, I think cpu_physical_memory_write also called the flush_icache_range, so don't need to duplicate it. > I was in the process of working through this myself, but I am not > quite finished. If you don't mind waiting a couple hours, I should > have something a fair bit simpler to solve the same problem. > > Biggest issue is the flush_icache_range() one. > > 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
Avi Kivity wrote: > Jes Sorensen wrote: >>>>>>> "Avi" == Avi Kivity <avi@redhat.com> writes: >>>>>>> >> >> Avi> Zhang, Xiantao wrote: >> >>>>> qemu_get_ram_ptr() returns a pointer. Don't cast it to a >>>>> ram_addr_t, leave it a pointer. >>>>> >>>>> But why not use cpu_physical_memory_write() (or >>>>> cpu_physical_memory_write_rom())? It's much simpler and cleaner. >>>>> >>>>> >>>> Good suggestion! I just followed the original logic. Updated the >>>> patch. Xiantao >>>> >> >> Avi> Thanks, applied. >> >> Hi, >> >> I am not crazy about this patch. You need to use >> cpy_physical_memory_rw() in the hob and nvram code too, not just in >> the ipf.c code. >> >> What about the flush_icache_range() call you removed - is it safe to >> just discard that? >> >> I was in the process of working through this myself, but I am not >> quite finished. If you don't mind waiting a couple hours, I should >> have something a fair bit simpler to solve the same problem. >> >> Biggest issue is the flush_icache_range() one. >> >> > > 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-- 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
diff --git a/hw/ipf.c b/hw/ipf.c index 5ed2667..ccd7665 100644 --- a/hw/ipf.c +++ b/hw/ipf.c @@ -54,7 +54,8 @@ static fdctrl_t *floppy_controller; static RTCState *rtc_state; static PCIDevice *i440fx_state; -uint8_t *g_fw_start; +ram_addr_t gfw_start; + static uint32_t ipf_to_legacy_io(target_phys_addr_t addr) { return (uint32_t)(((addr&0x3ffffff) >> 12 << 2)|((addr) & 0x3)); @@ -454,15 +455,15 @@ static void ipf_init1(ram_addr_t ram_size, int vga_ram_size, if (kvm_enabled()) { unsigned long image_size; char *image = NULL; - uint8_t *fw_image_start; + ram_addr_t fw_image_start; 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; - g_fw_start = fw_start; + ram_addr = qemu_ram_alloc(GFW_SIZE); + gfw_start = (ram_addr_t)qemu_get_ram_ptr(ram_addr); + snprintf(buf, sizeof(buf), "%s/%s", bios_dir, FW_FILENAME); image = read_image(buf, &image_size ); if (NULL == image || !image_size) { @@ -470,20 +471,20 @@ static void ipf_init1(ram_addr_t ram_size, int vga_ram_size, fprintf(stderr, "Please check Guest firmware at %s\n", buf); exit(1); } - fw_image_start = fw_start + GFW_SIZE - image_size; - - cpu_register_physical_memory(GFW_START, GFW_SIZE, fw_offset); - memcpy(fw_image_start, image, image_size); - - free(image); + /* Load Guest Firmware to the proper postion. */ + fw_image_start = gfw_start + GFW_SIZE - image_size; + memcpy((void *)fw_image_start, image, image_size); flush_icache_range((unsigned long)fw_image_start, (unsigned long)fw_image_start + image_size); + free(image); + + cpu_register_physical_memory(GFW_START, GFW_SIZE, ram_addr); - nvram_addr = NVRAM_START; if (nvram) { + nvram_addr = NVRAM_START; 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, gfw_start); close(nvram_fd); } i = atexit((void *)kvm_ia64_copy_from_GFW_to_nvram); @@ -491,7 +492,7 @@ static void ipf_init1(ram_addr_t ram_size, int vga_ram_size, fprintf(stderr, "cannot set exit function\n"); } kvm_ia64_build_hob(ram_size + above_4g_mem_size, smp_cpus, - fw_start, nvram_addr); + gfw_start, nvram_addr); } /*Register legacy io address space, size:64M*/ @@ -513,19 +514,15 @@ static void ipf_init1(ram_addr_t ram_size, int vga_ram_size, 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); + pci_cirrus_vga_init(pci_bus, vga_ram_size); } else { - isa_cirrus_vga_init(phys_ram_base + vga_ram_addr, - vga_ram_addr, vga_ram_size); + 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); + pci_vga_init(pci_bus, vga_ram_size, 0, 0); } else { - isa_vga_init(phys_ram_base + vga_ram_addr, - vga_ram_addr, vga_ram_size); + isa_vga_init(vga_ram_size); } } @@ -671,7 +668,6 @@ QEMUMachine ipf_machine = { .name = "itanium", .desc = "Itanium Platform", .init = (QEMUMachineInitFunc *)ipf_init_pci, - .ram_require = (ram_addr_t)(VGA_RAM_SIZE + GFW_SIZE), .max_cpus = 255, }; diff --git a/target-ia64/firmware.c b/target-ia64/firmware.c index 87a8178..2ed2e0d 100644 --- a/target-ia64/firmware.c +++ b/target-ia64/firmware.c @@ -92,11 +92,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); + unsigned long dom_mem_size, ram_addr_t hob_start); int kvm_ia64_build_hob(unsigned long memsize, unsigned long vcpus, - uint8_t *fw_start, unsigned long nvram_addr) + ram_addr_t fw_start, unsigned long nvram_addr) { char *hob_buf; @@ -249,7 +249,7 @@ err_out: 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, ram_addr_t hob_start) { int hob_size; @@ -263,7 +263,7 @@ load_hob(void *hob_buf, unsigned long dom_mem_size, void* hob_start) Hob_Output("No enough memory for hob data"); return -1; } - memcpy (hob_start, hob_buf, hob_size); + memcpy ((void *)hob_start, hob_buf, hob_size); return 0; } @@ -644,7 +644,7 @@ out: int kvm_ia64_copy_from_nvram_to_GFW(unsigned long nvram_fd, - const uint8_t *fw_start) + const ram_addr_t fw_start) { struct stat file_stat; if ((fstat(nvram_fd, &file_stat) < 0) || @@ -659,8 +659,13 @@ int kvm_ia64_copy_from_GFW_to_nvram() { unsigned long nvram_fd; + void* real_nvram_start; + target_phys_addr_t nvram_size = NVRAM_SIZE; unsigned long type = WRITE_TO_NVRAM; - unsigned long *nvram_addr = (unsigned long *)(g_fw_start + NVRAM_OFFSET); + unsigned long *nvram_addr = (unsigned long *)(gfw_start + NVRAM_OFFSET); + + + nvram_fd = kvm_ia64_nvram_init(type); if (nvram_fd == -1) goto out; @@ -669,11 +674,15 @@ kvm_ia64_copy_from_GFW_to_nvram() 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) { + + real_nvram_start = cpu_physical_memory_map(((struct nvram_save_addr*)nvram_addr)->addr, + &nvram_size, 1); + if (write(nvram_fd, real_nvram_start, NVRAM_SIZE) != NVRAM_SIZE) { close(nvram_fd); goto out; } + cpu_physical_memory_unmap(real_nvram_start, NVRAM_SIZE, 1, NVRAM_SIZE); + close(nvram_fd); return 0; out: diff --git a/target-ia64/firmware.h b/target-ia64/firmware.h index c1707ac..548352c 100644 --- a/target-ia64/firmware.h +++ b/target-ia64/firmware.h @@ -52,13 +52,13 @@ struct nvram_save_addr { }; extern const char *nvram; -extern uint8_t *g_fw_start; +extern ram_addr_t gfw_start; extern int kvm_ia64_build_hob(unsigned long memsize, unsigned long vcpus, - uint8_t *fw_start, unsigned long nvram_addr); + ram_addr_t fw_start, unsigned long nvram_addr); extern char *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); + const ram_addr_t fw_start); #endif //__FIRM_WARE_