From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53812) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VR04A-0004Pk-G1 for qemu-devel@nongnu.org; Tue, 01 Oct 2013 09:32:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VR044-0003p2-Hm for qemu-devel@nongnu.org; Tue, 01 Oct 2013 09:32:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57066) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VR044-0003ot-8d for qemu-devel@nongnu.org; Tue, 01 Oct 2013 09:32:36 -0400 Message-ID: <1380634330.5280.46.camel@nilsson.home.kraxel.org> From: Gerd Hoffmann Date: Tue, 01 Oct 2013 15:32:10 +0200 In-Reply-To: <524AB29F.3030906@suse.de> References: <1380620399-9907-1-git-send-email-kraxel@redhat.com> <524AB29F.3030906@suse.de> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] turn firmware image filename into a machine option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Peter Maydell , open@suse.de, Mark Langsdorf , list@suse.de, qemu-devel@nongnu.org, Fabien Chouteau , Blue Swirl , Michael Walle , =?ISO-8859-1?Q?Herv=E9?= Poussineau , Paul Brook , Anthony Liguori , Andreas =?ISO-8859-1?Q?F=E4rber?= , Aurelien Jarno , Richard Henderson > > --- a/hw/arm/highbank.c > > +++ b/hw/arm/highbank.c > > @@ -250,15 +250,15 @@ static void calxeda_init(QEMUMachineInitArgs *args, enum cxmachines machine) > > sysram = g_new(MemoryRegion, 1); > > memory_region_init_ram(sysram, NULL, "highbank.sysram", 0x8000); > > memory_region_add_subregion(sysmem, 0xfff88000, sysram); > > - if (bios_name != NULL) { > > - sysboot_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); > > + if (args->firmware != NULL) { > > + sysboot_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, args->firmware); > > if (sysboot_filename != NULL) { > > uint32_t filesize = get_image_size(sysboot_filename); > > if (load_image_targphys("sysram.bin", 0xfff88000, filesize)< 0) { > > - hw_error("Unable to load %s\n", bios_name); > > + hw_error("Unable to load %s\n", args->firmware); > > } > > } else { > > - hw_error("Unable to find %s\n", bios_name); > > + hw_error("Unable to find %s\n", args->firmware); > > } > > } > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index 0c313fe..3e3c9c1 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -1112,7 +1112,8 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory, > > ram_addr_t above_4g_mem_size, > > MemoryRegion *rom_memory, > > MemoryRegion **ram_memory, > > - PcGuestInfo *guest_info) > > + PcGuestInfo *guest_info, > > + const char *firmware) > > Looking at this I think it'd make more sense to convert all users of > bios_name to instead base on local variables and fetch the firmware name > directly from the firmware machine opt. If you like, add a helper to > make the conversion easier :). > > But that way we don't have to touch function headers and bloat them even > more than they are today. For most other machines (such as the calxeda quoted above) I don't have to touch the prototypes but just replace the global bios_name with QemuMachineInitArgs->firmware. pc is special here as the bios init is deeply nested. Maybe we should pass down the whole QemuMachineInitArgs instead ... cheers, Gerd