From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:50256) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9ha9-0007EO-7w for qemu-devel@nongnu.org; Tue, 12 Apr 2011 13:40:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q9ha8-0001SE-3J for qemu-devel@nongnu.org; Tue, 12 Apr 2011 13:40:53 -0400 Received: from e9.ny.us.ibm.com ([32.97.182.139]:54492) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9ha8-0001Ra-0f for qemu-devel@nongnu.org; Tue, 12 Apr 2011 13:40:52 -0400 Received: from d01dlp01.pok.ibm.com (d01dlp01.pok.ibm.com [9.56.224.56]) by e9.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p3CHCsqW013847 for ; Tue, 12 Apr 2011 13:12:54 -0400 Received: from d01relay02.pok.ibm.com (d01relay02.pok.ibm.com [9.56.227.234]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id CC59438C8042 for ; Tue, 12 Apr 2011 13:40:36 -0400 (EDT) Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d01relay02.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p3CHegjU043108 for ; Tue, 12 Apr 2011 13:40:42 -0400 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p3CHebr7015441 for ; Tue, 12 Apr 2011 11:40:37 -0600 Message-ID: <4DA48E93.90505@us.ibm.com> Date: Tue, 12 Apr 2011 12:40:35 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1302629037-16084-1-git-send-email-glommer@redhat.com> <1302629037-16084-3-git-send-email-glommer@redhat.com> In-Reply-To: <1302629037-16084-3-git-send-email-glommer@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] add fw_dir option to option-rom switch List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Glauber Costa Cc: Gleb Natapov , mtosatti@redhat.com, qemu-devel@nongnu.org, kvm@vger.kernel.org, avi@redhat.com On 04/12/2011 12:23 PM, Glauber Costa wrote: > The option-rom puts all roms passed by this switch in the genroms directory, > through the fw_dir option. But as it turns out, not all roms should be placed there. > > VGA roms are a of a different kind. They live in a different segment, and are scanned > first. This patch allows qemu to use external vgaroms such as sgabios, a rom for a serial > graphics adapter. Wouldn't it make more sense to have a new PCI device that had sgabios that could play the role of the VGA device. Then you could say -vga sga Regards, Anthony Liguori > Without this patch, we lose all the initial BIOS output: until the option rom is initialized, > the only available vga rom is the default cirrus/vesa one. > > We're also vulnerable to option rom enumeration order: if a vga oprom is initialized > before, say, gpxe, we are able to see its output in the adapter. If it is initialized > after, we miss it. > > So I believe the proper solution is to allow users to specify that a rom belongs in vgaroms > directory instead. > > Signed-off-by: Glauber Costa > CC: Gleb Natapov > --- > hw/pc.c | 7 ++++++- > qemu-config.c | 3 +++ > qemu-options.hx | 7 +++++-- > sysemu.h | 1 + > vl.c | 1 + > 5 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/hw/pc.c b/hw/pc.c > index 8d351ba..736efbb 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -1055,7 +1055,12 @@ void pc_memory_init(ram_addr_t ram_size, > } > > for (i = 0; i< nb_option_roms; i++) { > - rom_add_option(option_rom[i].name, option_rom[i].bootindex); > + /* do it here, instead of in vl.c, to avoid cluttering that file with x86 material */ > + if (!option_rom[i].fw_dir) { > + option_rom[i].fw_dir = "genroms"; > + } > + rom_add_file(option_rom[i].name, option_rom[i].fw_dir, 0, > + option_rom[i].bootindex); > } > } > > diff --git a/qemu-config.c b/qemu-config.c > index 6d9c238..97b8515 100644 > --- a/qemu-config.c > +++ b/qemu-config.c > @@ -450,6 +450,9 @@ QemuOptsList qemu_option_rom_opts = { > }, { > .name = "romfile", > .type = QEMU_OPT_STRING, > + }, { > + .name = "fw_dir", > + .type = QEMU_OPT_STRING, > }, > { /* end if list */ } > }, > diff --git a/qemu-options.hx b/qemu-options.hx > index 96927cc..d9eec62 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -2079,10 +2079,10 @@ to cope with initialization race conditions. > ETEXI > > DEF("option-rom", HAS_ARG, QEMU_OPTION_option_rom, \ > - "-option-rom rom[,bootindex=idx] load a file, rom, into the option ROM space\n", > + "-option-rom rom[,bootindex=idx][,fw_dir=dir] load a file, rom, into the option ROM space\n", > QEMU_ARCH_ALL) > STEXI > -@item -option-rom @var{file}[,bootindex=@var{bootindex}] > +@item -option-rom @var{file}[,bootindex=@var{bootindex}][,fw_dir=@var{dir}] > @findex -option-rom > Load the contents of @var{file} as an option ROM. > This option is useful to load things like EtherBoot. > @@ -2091,6 +2091,9 @@ This option is useful to load things like EtherBoot. > @item bootindex=@var{bootindex} > Defines which boot index the option rom will be given in boot entry vectors, > allowing fine-grained selection of devices boot order. > +@item fw_dir=@var{dir} (x86 only) > +Specify under which firmware directory entry this rom should live. Current > +allowed values are vgaroms and genroms (default). > @end table > > ETEXI > diff --git a/sysemu.h b/sysemu.h > index 3f7e17e..2f8be32 100644 > --- a/sysemu.h > +++ b/sysemu.h > @@ -160,6 +160,7 @@ extern uint64_t node_cpumask[MAX_NODES]; > typedef struct QEMUOptionRom { > const char *name; > int32_t bootindex; > + const char *fw_dir; > } QEMUOptionRom; > extern QEMUOptionRom option_rom[MAX_OPTION_ROMS]; > extern int nb_option_roms; > diff --git a/vl.c b/vl.c > index 8bcf2ae..e1d7868 100644 > --- a/vl.c > +++ b/vl.c > @@ -2675,6 +2675,7 @@ int main(int argc, char **argv, char **envp) > fprintf(stderr, "Option ROM file is not specified\n"); > exit(1); > } > + option_rom[nb_option_roms].fw_dir = qemu_opt_get(opts, "fw_dir"); > nb_option_roms++; > break; > case QEMU_OPTION_semihosting: