From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:48333) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9iNC-0003ex-MB for qemu-devel@nongnu.org; Tue, 12 Apr 2011 14:31:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q9iNA-0000mD-RF for qemu-devel@nongnu.org; Tue, 12 Apr 2011 14:31:34 -0400 Received: from e9.ny.us.ibm.com ([32.97.182.139]:34744) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9iNA-0000lz-Oc for qemu-devel@nongnu.org; Tue, 12 Apr 2011 14:31:32 -0400 Received: from d01dlp02.pok.ibm.com (d01dlp02.pok.ibm.com [9.56.224.85]) by e9.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p3CI3c5m007781 for ; Tue, 12 Apr 2011 14:03:38 -0400 Received: from d01relay02.pok.ibm.com (d01relay02.pok.ibm.com [9.56.227.234]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id 36CC46E8036 for ; Tue, 12 Apr 2011 14:31:31 -0400 (EDT) Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d01relay02.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p3CIVUsd404452 for ; Tue, 12 Apr 2011 14:31:31 -0400 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p3CIVUko025432 for ; Tue, 12 Apr 2011 12:31:30 -0600 Message-ID: <4DA49A80.3080701@us.ibm.com> Date: Tue, 12 Apr 2011 13:31:28 -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> <4DA48E93.90505@us.ibm.com> <1302631996.3021.28.camel@mothafucka.localdomain> In-Reply-To: <1302631996.3021.28.camel@mothafucka.localdomain> Content-Type: text/plain; charset=UTF-8; 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 01:13 PM, Glauber Costa wrote: > On Tue, 2011-04-12 at 12:40 -0500, Anthony Liguori wrote: >> 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, > This would be good, if we included sga along the roms we ship. We're > still, however, left with the problem that in the future, people may > want to use their own roms. Fortunately, I don't think that there are myriads of people writing 16-bit option roms so I don't think this is a pressing problem :-) > And the fact that not all of them should > live in genroms persists. Actually genroms should disappear. We should make -option-rom work by using a dummy PCI device or something like that. Genroms is a nasty hack. Regards, Anthony Liguori >> 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: >