From patchwork Wed Apr 29 08:21:16 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jes Sorensen X-Patchwork-Id: 20583 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n3T8LShW006518 for ; Wed, 29 Apr 2009 08:21:28 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753372AbZD2IVY (ORCPT ); Wed, 29 Apr 2009 04:21:24 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752219AbZD2IVY (ORCPT ); Wed, 29 Apr 2009 04:21:24 -0400 Received: from relay2.sgi.com ([192.48.179.30]:44801 "EHLO relay.sgi.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752042AbZD2IVW (ORCPT ); Wed, 29 Apr 2009 04:21:22 -0400 Received: from eye3.emea.sgi.com (eye3.emea.sgi.com [144.253.156.24]) by relay2.corp.sgi.com (Postfix) with ESMTP id DF75B304053; Wed, 29 Apr 2009 01:21:20 -0700 (PDT) Message-ID: <49F80DFC.5060006@sgi.com> Date: Wed, 29 Apr 2009 10:21:16 +0200 From: Jes Sorensen User-Agent: Thunderbird 2.0.0.21 (X11/20090320) MIME-Version: 1.0 To: "Zhang, Xiantao" CC: Avi Kivity , "kvm-ia64@vger.kernel.org" , "kvm@vger.kernel.org" Subject: Re: [PATCH 03/04] qemu-kvm: Remove the dependency for phys_ram_base for ipf.c References: <706158FABBBA044BAD4FE898A02E4BC236A2BC04@pdsmsx503.ccr.corp.intel.com> <49F6D13C.1060908@redhat.com> <706158FABBBA044BAD4FE898A02E4BC236A2BC2D@pdsmsx503.ccr.corp.intel.com> <49F6E2DC.3070405@redhat.com> <49F6FB55.3000503@redhat.com> <706158FABBBA044BAD4FE898A02E4BC236A2BD9A@pdsmsx503.ccr.corp.intel.com> In-Reply-To: <706158FABBBA044BAD4FE898A02E4BC236A2BD9A@pdsmsx503.ccr.corp.intel.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org 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. 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 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 --- 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_