| Message ID | 1302629037-16084-3-git-send-email-glommer@redhat.com |
|---|---|
| State | New |
| Headers | show |
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<glommer@redhat.com> > CC: Gleb Natapov<gleb@redhat.com> > --- > 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:
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. And the fact that not all of them should live in genroms persists. > 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<glommer@redhat.com> > > CC: Gleb Natapov<gleb@redhat.com> > > --- > > 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: >
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<glommer@redhat.com> >>> CC: Gleb Natapov<gleb@redhat.com> >>> --- >>> 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: >
On Tue, 2011-04-12 at 13:31 -0500, Anthony Liguori wrote: > 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 :-) For sure, but if we had this discussion a while ago, sgabios wouldn't exist back then, and now it does =p > > 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. In a side track, I think sgabios is a pretty useful addition to qemu. Would you consider including it together with the other roms ? This would definitely increase the out-of-the box experience. I think we don't even need to have a -vga sga switch then. Since sgabios does not prevent output from going to the normal vga, we could always install it, or install when there is a -serial parameter present.
On 04/12/2011 01:47 PM, Glauber Costa wrote: > For sure, but if we had this discussion a while ago, > sgabios wouldn't exist back then, and now it does =p Actually, it's been around for ages :-) >>> 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. > In a side track, I think sgabios is a pretty useful addition to qemu. > Would you consider including it together with the other roms ? Definitely. > This would definitely increase the out-of-the box experience. > > I think we don't even need to have a -vga sga switch then. Since sgabios > does not prevent output from going to the normal vga, we could always > install it, or install when there is a -serial parameter present. Is that true? It needs to load during video bios init, no? So doesn't that preclude having a normal video bios installed? Regards, Anthony Liguori
On Tue, 2011-04-12 at 14:19 -0500, Anthony Liguori wrote: > On 04/12/2011 01:47 PM, Glauber Costa wrote: > > For sure, but if we had this discussion a while ago, > > sgabios wouldn't exist back then, and now it does =p > > Actually, it's been around for ages :-) > > >>> 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. > > In a side track, I think sgabios is a pretty useful addition to qemu. > > Would you consider including it together with the other roms ? > > Definitely. > > > This would definitely increase the out-of-the box experience. > > > > I think we don't even need to have a -vga sga switch then. Since sgabios > > does not prevent output from going to the normal vga, we could always > > install it, or install when there is a -serial parameter present. > > Is that true? It needs to load during video bios init, no? So doesn't > that preclude having a normal video bios installed? No. sgabios chains all int10h calls.
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:
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. 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 <glommer@redhat.com> CC: Gleb Natapov <gleb@redhat.com> --- 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(-)