From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [PATCH 2/2] add fw_dir option to option-rom switch Date: Tue, 12 Apr 2011 12:40:35 -0500 Message-ID: <4DA48E93.90505@us.ibm.com> References: <1302629037-16084-1-git-send-email-glommer@redhat.com> <1302629037-16084-3-git-send-email-glommer@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Gleb Natapov , mtosatti@redhat.com, qemu-devel@nongnu.org, kvm@vger.kernel.org, avi@redhat.com To: Glauber Costa Return-path: In-Reply-To: <1302629037-16084-3-git-send-email-glommer@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org List-Id: kvm.vger.kernel.org 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: