From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] kvm-userspace: Load PCI option ROMs Date: Tue, 23 Dec 2008 19:15:46 +0200 Message-ID: <49511CC2.3010509@redhat.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: "kvm@vger.kernel.org" , "Shan, Haitao" , "Han, Weidong" , "Yang, Sheng" To: "Liu, Kechao" Return-path: Received: from mx2.redhat.com ([66.187.237.31]:58997 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752831AbYLWRP7 (ORCPT ); Tue, 23 Dec 2008 12:15:59 -0500 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: Liu, Kechao wrote: > Load assigned devices' PCI option ROMs to the RAM of > guest OS. And pass the corresponding devfns to BIOS. > > Sorry about the late review. > + > + addr = qemu_ram_alloc(size); > + phys_addr = addr + phys_ram_base; > + > + /* Write ROM data and devfn to phys_addr */ > + memcpy((void *)phys_addr, rom, rom->rom_size * 512); > + *(uint8_t *)(phys_addr + rom->rom_size * 512) = devfn; > phys_ram_base is dangerous. It works in this case, but we'd like to avoid it. Please use cpu_physical_memory_write_rom(). > + > + cpu_register_physical_memory(0xd0000 + offset, size, addr); > Best to register this as a rom, though kvm will ignore it (we don't implement read-only memory at this time). > + > +/* > + * Scan the assigned devices for the devices that have an option ROM, > + * and then load the corresponding ROM data to RAM. > + */ > +ram_addr_t assigned_dev_load_option_roms(ram_addr_t rom_base_offset) > +{ > + ram_addr_t offset = rom_base_offset; > + AssignedDevInfo *adev; > + > + LIST_FOREACH(adev, &adev_head, next) { > + int size; > + void *buf; > + FILE *fp; > + char rom_file[64]; > + char cmd[64]; > + > + snprintf(rom_file, sizeof(rom_file), > + "/sys/bus/pci/devices/0000:%02x:%02x.%01x/rom", > + adev->bus, adev->dev, adev->func); > + > + if (access(rom_file, F_OK)) > + continue; > + > + /* Write something to the ROM file to enable it */ > + snprintf(cmd, sizeof(cmd), "echo 1 > %s", rom_file); > + system(cmd); > system()?! Please just write to the file. > + > + fp = fopen(rom_file, "rb"); > + if (fp == NULL) > + continue; > + > + /* Read the data of the ROM file to the buffer */ > + fseek(fp, 0, SEEK_END); > + size = ftell(fp); > + fseek(fp, 0, SEEK_SET); > + buf = malloc(size); > + fread(buf, size, 1, fp); > + > Missing error checking. > + /* Scan the buffer for suitable ROMs and increase the offset */ > + offset += scan_option_rom(adev->assigned_dev->dev.devfn, buf, offset); > + > + free(buf); > + fclose(fp); > + } > + > + return offset; > +} > diff --git a/qemu/hw/device-assignment.h b/qemu/hw/device-assignment.h > -- error compiling committee.c: too many arguments to function