From patchwork Fri May 1 10:30:55 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jes Sorensen X-Patchwork-Id: 21404 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 n41AVCWe030957 for ; Fri, 1 May 2009 10:31:12 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752792AbZEAKbH (ORCPT ); Fri, 1 May 2009 06:31:07 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753135AbZEAKbG (ORCPT ); Fri, 1 May 2009 06:31:06 -0400 Received: from relay3.sgi.com ([192.48.156.57]:49779 "EHLO relay.sgi.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752528AbZEAKbE (ORCPT ); Fri, 1 May 2009 06:31:04 -0400 Received: from eye3.emea.sgi.com (eye3.emea.sgi.com [144.253.156.24]) by relay3.corp.sgi.com (Postfix) with ESMTP id 9A10BAC013; Fri, 1 May 2009 03:30:59 -0700 (PDT) Message-ID: <49FACF5F.8000305@sgi.com> Date: Fri, 01 May 2009 12:30:55 +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> <49F80DFC.5060006@sgi.com> <706158FABBBA044BAD4FE898A02E4BC236A2C04D@pdsmsx503.ccr.corp.intel.com> <49F812EF.5060006@sgi.com> <706158FABBBA044BAD4FE898A02E4BC236A2C11C@pdsmsx503.ccr.corp.intel.com> In-Reply-To: <706158FABBBA044BAD4FE898A02E4BC236A2C11C@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: > Jes Sorensen wrote: > 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, Here is an incremental version of the patch. I think the differences should be pretty obvious now :-) It fixes the memcpy issues in the hob and nvram code and also cleans up the interfaces a lot. Avi, please add. Cheers, Jes Fix ia64 code to use copy_physical_memory_{read,write} in hob and nvram code, removing dependencies of qemu_get_ram_ptr() usage. This results in cleaned up APIs and removal of unnecessary global variables. This is an incremental patch on top of previous patch posted by Xiantao. Signed-off-by: Jes Sorensen --- hw/ipf.c | 38 ++++++++++--------------- target-ia64/firmware.c | 73 +++++++++++++++++++++++++++++-------------------- target-ia64/firmware.h | 6 +--- 3 files changed, 62 insertions(+), 55 deletions(-) Index: qemu-kvm/hw/ipf.c =================================================================== --- qemu-kvm.orig/hw/ipf.c +++ qemu-kvm/hw/ipf.c @@ -54,8 +54,6 @@ static RTCState *rtc_state; static PCIDevice *i440fx_state; -void *gfw_start; - static uint32_t ipf_to_legacy_io(target_phys_addr_t addr) { return (uint32_t)(((addr&0x3ffffff) >> 12 << 2)|((addr) & 0x3)); @@ -455,15 +453,12 @@ if (kvm_enabled()) { unsigned long image_size; uint8_t *image = NULL; - target_phys_addr_t fw_image_start; - unsigned long nvram_addr = 0; + unsigned long nvram_addr; unsigned long nvram_fd = 0; unsigned long type = READ_FROM_NVRAM; unsigned long i = 0; - - ram_addr = qemu_ram_alloc(GFW_SIZE); - gfw_start = qemu_get_ram_ptr(ram_addr); - cpu_register_physical_memory(GFW_START, GFW_SIZE, ram_addr); + unsigned long fw_offset; + ram_addr_t fw_mem = qemu_ram_alloc(GFW_SIZE); snprintf(buf, sizeof(buf), "%s/%s", bios_dir, FW_FILENAME); image = read_image(buf, &image_size ); @@ -472,26 +467,27 @@ fprintf(stderr, "Please check Guest firmware at %s\n", buf); exit(1); } + fw_offset = GFW_START + GFW_SIZE - image_size; - /* Load Guest Firmware to the proper postion. */ - fw_image_start = GFW_START + GFW_SIZE - image_size; - cpu_physical_memory_write(fw_image_start, image, image_size); - free(image); + cpu_register_physical_memory(GFW_START, GFW_SIZE, fw_mem); + cpu_physical_memory_write(fw_offset, image, image_size); + free(image); 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, gfw_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, - gfw_start, nvram_addr); + } else + nvram_addr = 0; + + kvm_ia64_build_hob(ram_size + above_4g_mem_size, smp_cpus, nvram_addr); } /*Register legacy io address space, size:64M*/ @@ -512,17 +508,15 @@ } if (cirrus_vga_enabled) { - if (pci_enabled) { + if (pci_enabled) pci_cirrus_vga_init(pci_bus, vga_ram_size); - } else { + else isa_cirrus_vga_init(vga_ram_size); - } } else { - if (pci_enabled) { + if (pci_enabled) pci_vga_init(pci_bus, vga_ram_size, 0, 0); - } else { + else isa_vga_init(vga_ram_size); - } } rtc_state = rtc_init(0x70, i8259[8], 2000); 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, - void* 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 ((void *)hob_start, hob_buf, hob_size); + + cpu_physical_memory_write(GFW_HOB_START, hob_buf, hob_size); + return 0; } @@ -643,50 +645,63 @@ } int -kvm_ia64_copy_from_nvram_to_GFW(unsigned long nvram_fd, - const void* 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; - void* real_nvram_start; - target_phys_addr_t nvram_size = NVRAM_SIZE; unsigned long type = WRITE_TO_NVRAM; - unsigned long *nvram_addr = (unsigned long *)(gfw_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); - 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); + lseek(nvram_fd, 0, SEEK_SET); + if (write(nvram_fd, nvram_buf, NVRAM_SIZE) != NVRAM_SIZE) goto out; - } - cpu_physical_memory_unmap(real_nvram_start, NVRAM_SIZE, 1, NVRAM_SIZE); + 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 void *gfw_start; extern int kvm_ia64_build_hob(unsigned long memsize, unsigned long vcpus, - void* fw_start, unsigned long nvram_addr); + 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 void* fw_start); +extern int kvm_ia64_copy_from_nvram_to_GFW(unsigned long nvram_fd); #endif //__FIRM_WARE_