diff mbox

turn firmware image filename into a machine option

Message ID 1380620399-9907-1-git-send-email-kraxel@redhat.com
State New
Headers show

Commit Message

Gerd Hoffmann Oct. 1, 2013, 9:39 a.m. UTC
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/alpha/dp264.c        |  2 +-
 hw/arm/highbank.c       |  8 ++++----
 hw/i386/pc.c            |  6 ++++--
 hw/i386/pc_piix.c       |  3 ++-
 hw/i386/pc_q35.c        |  3 ++-
 hw/i386/pc_sysfw.c      |  8 +++++---
 hw/lm32/milkymist.c     |  8 ++++----
 hw/mips/mips_fulong2e.c |  8 ++++----
 hw/mips/mips_jazz.c     |  9 ++++++---
 hw/mips/mips_malta.c    |  8 ++++----
 hw/mips/mips_mipssim.c  |  7 ++++---
 hw/mips/mips_r4k.c      |  9 +++++----
 hw/ppc/mac_newworld.c   |  9 +++++----
 hw/ppc/mac_oldworld.c   |  9 +++++----
 hw/ppc/ppc405_boards.c  | 24 ++++++++++++++----------
 hw/ppc/prep.c           | 11 ++++++-----
 hw/ppc/spapr.c          |  6 +++---
 hw/s390x/ipl.c          |  1 +
 hw/sh4/shix.c           | 11 ++++++-----
 hw/sparc/leon3.c        |  6 +++---
 hw/sparc/sun4m.c        |  2 +-
 hw/sparc64/sun4u.c      |  2 +-
 include/hw/boards.h     |  1 +
 include/hw/i386/pc.h    |  6 ++++--
 include/sysemu/sysemu.h |  2 --
 vl.c                    | 12 +++++++++---
 26 files changed, 104 insertions(+), 77 deletions(-)

Comments

Peter Maydell Oct. 1, 2013, 10:55 a.m. UTC | #1
On 1 October 2013 18:39, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/alpha/dp264.c        |  2 +-
>  hw/arm/highbank.c       |  8 ++++----
>  hw/i386/pc.c            |  6 ++++--
>  hw/i386/pc_piix.c       |  3 ++-
>  hw/i386/pc_q35.c        |  3 ++-
>  hw/i386/pc_sysfw.c      |  8 +++++---
>  hw/lm32/milkymist.c     |  8 ++++----
>  hw/mips/mips_fulong2e.c |  8 ++++----
>  hw/mips/mips_jazz.c     |  9 ++++++---
>  hw/mips/mips_malta.c    |  8 ++++----
>  hw/mips/mips_mipssim.c  |  7 ++++---
>  hw/mips/mips_r4k.c      |  9 +++++----
>  hw/ppc/mac_newworld.c   |  9 +++++----
>  hw/ppc/mac_oldworld.c   |  9 +++++----
>  hw/ppc/ppc405_boards.c  | 24 ++++++++++++++----------
>  hw/ppc/prep.c           | 11 ++++++-----
>  hw/ppc/spapr.c          |  6 +++---
>  hw/s390x/ipl.c          |  1 +
>  hw/sh4/shix.c           | 11 ++++++-----
>  hw/sparc/leon3.c        |  6 +++---
>  hw/sparc/sun4m.c        |  2 +-
>  hw/sparc64/sun4u.c      |  2 +-
>  include/hw/boards.h     |  1 +
>  include/hw/i386/pc.h    |  6 ++++--
>  include/sysemu/sysemu.h |  2 --
>  vl.c                    | 12 +++++++++---

No documentation or definition of what the semantics of
specifying a "firmware image" filename are?

Why is this a machine option rather than a property of
the ROM/flash device?

thanks
-- PMM
Gerd Hoffmann Oct. 1, 2013, 11:22 a.m. UTC | #2
On Di, 2013-10-01 at 19:55 +0900, Peter Maydell wrote:
> On 1 October 2013 18:39, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  hw/alpha/dp264.c        |  2 +-
> >  hw/arm/highbank.c       |  8 ++++----
> >  hw/i386/pc.c            |  6 ++++--
> >  hw/i386/pc_piix.c       |  3 ++-
> >  hw/i386/pc_q35.c        |  3 ++-
> >  hw/i386/pc_sysfw.c      |  8 +++++---
> >  hw/lm32/milkymist.c     |  8 ++++----
> >  hw/mips/mips_fulong2e.c |  8 ++++----
> >  hw/mips/mips_jazz.c     |  9 ++++++---
> >  hw/mips/mips_malta.c    |  8 ++++----
> >  hw/mips/mips_mipssim.c  |  7 ++++---
> >  hw/mips/mips_r4k.c      |  9 +++++----
> >  hw/ppc/mac_newworld.c   |  9 +++++----
> >  hw/ppc/mac_oldworld.c   |  9 +++++----
> >  hw/ppc/ppc405_boards.c  | 24 ++++++++++++++----------
> >  hw/ppc/prep.c           | 11 ++++++-----
> >  hw/ppc/spapr.c          |  6 +++---
> >  hw/s390x/ipl.c          |  1 +
> >  hw/sh4/shix.c           | 11 ++++++-----
> >  hw/sparc/leon3.c        |  6 +++---
> >  hw/sparc/sun4m.c        |  2 +-
> >  hw/sparc64/sun4u.c      |  2 +-
> >  include/hw/boards.h     |  1 +
> >  include/hw/i386/pc.h    |  6 ++++--
> >  include/sysemu/sysemu.h |  2 --
> >  vl.c                    | 12 +++++++++---
> 
> No documentation or definition of what the semantics of
> specifying a "firmware image" filename are?

"-machine firmware=$file" has the same effect as "-bios $file", which is
simliar to '-machine kernel=$file' and '-kernel $file'.

With this patch the firmware/bios filename goes into QemuOpts and thus
can be set via -readconfig $file.  It's also possible to set the default
firmware image per machine type (via QEMUMachine->default_machine_opts =
"firmware=$filename").

[ I'll make the commit message more verbose for v2 ]

> Why is this a machine option rather than a property of
> the ROM/flash device?

Not all machines have a flash device.  Also flash drives don't want a
simple (readonly) image file but a (writable) blockdev as backing
storage, at least the pflash device emulation I've briefly looked at.

cheers,
  Gerd
Peter Maydell Oct. 1, 2013, 11:32 a.m. UTC | #3
On 1 October 2013 20:22, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On Di, 2013-10-01 at 19:55 +0900, Peter Maydell wrote:
>> No documentation or definition of what the semantics of
>> specifying a "firmware image" filename are?
>
> "-machine firmware=$file" has the same effect as "-bios $file", which is
> simliar to '-machine kernel=$file' and '-kernel $file'.

It's similar in that it has semantics that might vary between target cpu
architecture or even between target machines, and which aren't
very well documented... -bios we're stuck with because it's a legacy
option, but if we're going to change/augment the syntax here it
would be good to be sure that we end up with something that's
(a) consistent across boards/architectures (b) not going to turn
into another awkward legacy option.

>> Why is this a machine option rather than a property of
>> the ROM/flash device?
>
> Not all machines have a flash device.

No, but they must have *something* that the firmware code lives in...

> Also flash drives don't want a
> simple (readonly) image file but a (writable) blockdev as backing
> storage, at least the pflash device emulation I've briefly looked at.

...so how does this work for machines where the firmware lives in
a flash device? Does -firmware=foo override setting the backing
image for the flash device, or vice versa, or do machines with flash
devices not support -firmware=foo, or something else?

-- PMM
Gerd Hoffmann Oct. 1, 2013, 12:16 p.m. UTC | #4
Hi,

> -bios we're stuck with because it's a legacy
> option,

What is legacy about it?

Well, the x86-centric name of course.  Thats why the machine option is
named 'firmware' instead.

It also doesn't use QemuOpts, which is fixed by this patch.

Anything else?

> >> Why is this a machine option rather than a property of
> >> the ROM/flash device?
> >
> > Not all machines have a flash device.
> 
> No, but they must have *something* that the firmware code lives in...

Some don't.  They just memcpy the firmware into guest ram instead.

Most have a memory region for the firmware which gets mapped into guest
address space.  Sometimes pulled out of thin air, sometimes modeled as
proper device.

> > Also flash drives don't want a
> > simple (readonly) image file but a (writable) blockdev as backing
> > storage, at least the pflash device emulation I've briefly looked at.
> 
> ...so how does this work for machines where the firmware lives in
> a flash device?

pc: In case '-pflash $blockdev' is present '-bios $image' is ignored.

My patch doesn't change that behavior, i.e. -machine firmware=$image is
ignored too in case -plflash is present on the command line.

Given that pflash operates on a blockdev I don't see a obvious way to
change that ...

cheers,
  Gerd
Andreas Färber Oct. 1, 2013, 12:20 p.m. UTC | #5
Hi,

Am 01.10.2013 14:16, schrieb Gerd Hoffmann:
>> -bios we're stuck with because it's a legacy
>> option,
> 
> What is legacy about it?
> 
> Well, the x86-centric name of course.  Thats why the machine option is
> named 'firmware' instead.
> 
> It also doesn't use QemuOpts, which is fixed by this patch.
> 
> Anything else?

The basic assumption that there is only one piece of firmware. Just like
-bios, -machine firmware= will allow only one entry. sPAPR has two.

Regards,
Andreas
Peter Maydell Oct. 1, 2013, 1 p.m. UTC | #6
On 1 October 2013 21:16, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
>> -bios we're stuck with because it's a legacy
>> option,
>
> What is legacy about it?

The fact that we've had it since forever, it was introduced for
a specific x86 use case and has since accreted behaviour
rather than being designed.

>> >> Why is this a machine option rather than a property of
>> >> the ROM/flash device?
>> >
>> > Not all machines have a flash device.
>>
>> No, but they must have *something* that the firmware code lives in...
>
> Some don't.  They just memcpy the firmware into guest ram instead.

...is that a good thing? What does the real hardware do? Should we
be doing that instead? Isn't this awkward on reset?

>> > Also flash drives don't want a
>> > simple (readonly) image file but a (writable) blockdev as backing
>> > storage, at least the pflash device emulation I've briefly looked at.
>>
>> ...so how does this work for machines where the firmware lives in
>> a flash device?
>
> pc: In case '-pflash $blockdev' is present '-bios $image' is ignored.
>
> My patch doesn't change that behavior, i.e. -machine firmware=$image is
> ignored too in case -plflash is present on the command line.
>
> Given that pflash operates on a blockdev I don't see a obvious way to
> change that ...

We could make pflash accept a plain file which is then used as the
read-only initialization for a blockdev which isn't backed by anything,
for example. That would be useful for other pflash usage scenarios
too.

-- PMM
Gerd Hoffmann Oct. 1, 2013, 1:32 p.m. UTC | #7
> > --- 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
Gerd Hoffmann Oct. 1, 2013, 1:41 p.m. UTC | #8
On Di, 2013-10-01 at 14:20 +0200, Andreas Färber wrote:
> Hi,
> 
> Am 01.10.2013 14:16, schrieb Gerd Hoffmann:
> >> -bios we're stuck with because it's a legacy
> >> option,
> > 
> > What is legacy about it?
> > 
> > Well, the x86-centric name of course.  Thats why the machine option is
> > named 'firmware' instead.
> > 
> > It also doesn't use QemuOpts, which is fixed by this patch.
> > 
> > Anything else?
> 
> The basic assumption that there is only one piece of firmware. Just like
> -bios, -machine firmware= will allow only one entry. sPAPR has two.

--verbose please.  What they are needed for?

Strictly speaking we have multiple firmwares on x86 too.  Everything but
seabios is device-specific though and gets passed to the guest via pci
rom bar.

cheers,
  Gerd
Andreas Färber Oct. 1, 2013, 1:46 p.m. UTC | #9
Am 01.10.2013 15:41, schrieb Gerd Hoffmann:
> On Di, 2013-10-01 at 14:20 +0200, Andreas Färber wrote:
>> Hi,
>>
>> Am 01.10.2013 14:16, schrieb Gerd Hoffmann:
>>>> -bios we're stuck with because it's a legacy
>>>> option,
>>>
>>> What is legacy about it?
>>>
>>> Well, the x86-centric name of course.  Thats why the machine option is
>>> named 'firmware' instead.
>>>
>>> It also doesn't use QemuOpts, which is fixed by this patch.
>>>
>>> Anything else?
>>
>> The basic assumption that there is only one piece of firmware. Just like
>> -bios, -machine firmware= will allow only one entry. sPAPR has two.
> 
> --verbose please.  What they are needed for?

SLOF and RTAS. SLOF is the firmware, and RTAS is requested by the OS
through OpenFirmware client interface.

It is not device-specific, it is more or less part of the firmware but
copied to RAM. I'll let Alex or Alexey comment in more details.

Cheers,
Andreas
Alexey Kardashevskiy Oct. 1, 2013, 2:23 p.m. UTC | #10
On 10/01/2013 11:46 PM, Andreas Färber wrote:
> Am 01.10.2013 15:41, schrieb Gerd Hoffmann:
>> On Di, 2013-10-01 at 14:20 +0200, Andreas Färber wrote:
>>> Hi,
>>>
>>> Am 01.10.2013 14:16, schrieb Gerd Hoffmann:
>>>>> -bios we're stuck with because it's a legacy
>>>>> option,
>>>>
>>>> What is legacy about it?
>>>>
>>>> Well, the x86-centric name of course.  Thats why the machine option is
>>>> named 'firmware' instead.
>>>>
>>>> It also doesn't use QemuOpts, which is fixed by this patch.
>>>>
>>>> Anything else?
>>>
>>> The basic assumption that there is only one piece of firmware. Just like
>>> -bios, -machine firmware= will allow only one entry. sPAPR has two.
>>
>> --verbose please.  What they are needed for?
> 
> SLOF and RTAS. SLOF is the firmware, and RTAS is requested by the OS
> through OpenFirmware client interface.
> 
> It is not device-specific, it is more or less part of the firmware but
> copied to RAM. I'll let Alex or Alexey comment in more details.

SLOF is what is loaded from the very beginning, it configures PCI, cooks
the device tree and boots the guest system (directly or via yaboot/grub,
from disk, network or ram). Normal firmware, as usual. It knows all the
details about the machine so the guest system (linux) does not need to know
details about PCI host bus adapter or anything like this.

RTAS is an agent which always lives in RAM when the guest system (linux,
aix) is up and running. It is a light-weight version of SLOF which is left
in RAM by SLOF and can do board/machine specific tasks such as PCI config
space access or PCI hotplug - something what SLOF already knows about and
something what the guest does not want to know about in details. This came
from IBM pHyp (traditional server PPC64 hypervisor) and it is quite a big
firmware. In the case of KVM, it is very small stub which simply passes
requests to QEMU which does the rest. But it is still a separate binary
image even in the current QEMU.

May be some day it will become bigger as from time to time the community
wants things to be done in a certain way which would mean extending rtas,
however we (powerpc-server folks) want to hope it won't happen ever :)

Adding Ben in copy, he might have something to add.
Gerd Hoffmann Oct. 1, 2013, 2:40 p.m. UTC | #11
Hi,

> SLOF is what is loaded from the very beginning, it configures PCI, cooks
> the device tree and boots the guest system (directly or via yaboot/grub,
> from disk, network or ram). Normal firmware, as usual. It knows all the
> details about the machine so the guest system (linux) does not need to know
> details about PCI host bus adapter or anything like this.

So pretty much like seabios on x86.

> RTAS is an agent which always lives in RAM when the guest system (linux,
> aix) is up and running. It is a light-weight version of SLOF which is left
> in RAM by SLOF and can do board/machine specific tasks such as PCI config
> space access or PCI hotplug - something what SLOF already knows about and
> something what the guest does not want to know about in details. This came
> from IBM pHyp (traditional server PPC64 hypervisor) and it is quite a big
> firmware. In the case of KVM, it is very small stub which simply passes
> requests to QEMU which does the rest. But it is still a separate binary
> image even in the current QEMU.

How that does get loaded?  Is it there at machine init?  Or does SLOF
load RTAS from somewhere?

thanks,
  Gerd
Alexander Graf Oct. 1, 2013, 2:45 p.m. UTC | #12
On 10/01/2013 04:40 PM, Gerd Hoffmann wrote:
>    Hi,
>
>> SLOF is what is loaded from the very beginning, it configures PCI, cooks
>> the device tree and boots the guest system (directly or via yaboot/grub,
>> from disk, network or ram). Normal firmware, as usual. It knows all the
>> details about the machine so the guest system (linux) does not need to know
>> details about PCI host bus adapter or anything like this.
> So pretty much like seabios on x86.
>
>> RTAS is an agent which always lives in RAM when the guest system (linux,
>> aix) is up and running. It is a light-weight version of SLOF which is left
>> in RAM by SLOF and can do board/machine specific tasks such as PCI config
>> space access or PCI hotplug - something what SLOF already knows about and
>> something what the guest does not want to know about in details. This came
>> from IBM pHyp (traditional server PPC64 hypervisor) and it is quite a big
>> firmware. In the case of KVM, it is very small stub which simply passes
>> requests to QEMU which does the rest. But it is still a separate binary
>> image even in the current QEMU.
> How that does get loaded?  Is it there at machine init?  Or does SLOF
> load RTAS from somewhere?

It gets loaded to a fixed address similar to the device tree. But 
there's no reason that couldn't be changed to on demand loading or even 
an integrated RTAS blob inside of SLOF.


Alex
Paolo Bonzini Oct. 1, 2013, 2:57 p.m. UTC | #13
Il 01/10/2013 15:00, Peter Maydell ha scritto:
> > Some don't.  They just memcpy the firmware into guest ram instead.
> 
> ...is that a good thing? What does the real hardware do? Should we
> be doing that instead? Isn't this awkward on reset?

They probably have a ROM (which should be the real "-machine
firmware=...") that loads firmware from NAND flash ("-drive
if=mtd,file=...") into RAM, and then gets out of the way.

Perhaps it's what QEMU should do, but not even the few machines that
support "-drive if=mtd" do it to this level of accuracy.

Paolo
Gerd Hoffmann Oct. 1, 2013, 3:05 p.m. UTC | #14
On Di, 2013-10-01 at 22:00 +0900, Peter Maydell wrote:
> On 1 October 2013 21:16, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >   Hi,
> >
> >> -bios we're stuck with because it's a legacy
> >> option,
> >
> > What is legacy about it?
> 
> The fact that we've had it since forever, it was introduced for
> a specific x86 use case and has since accreted behaviour
> rather than being designed.

The x86 use case is where the x86 centric name comes from.  But that's
it.  Almost every machine has some kind of firmware which does the
initialization, that certainly isn't a x86-only / legacy concept ...

> >> >> Why is this a machine option rather than a property of
> >> >> the ROM/flash device?
> >> >
> >> > Not all machines have a flash device.
> >>
> >> No, but they must have *something* that the firmware code lives in...
> >
> > Some don't.  They just memcpy the firmware into guest ram instead.
> 
> ...is that a good thing? What does the real hardware do? Should we
> be doing that instead? Isn't this awkward on reset?

Isn't ideal of course.  But it is how it is, for whatever reasons, and
I'm not going to implement proper rom/flash emulation for each and every
qemu machine type.

I just need some way to have different default firmware images for
different pc machine type versions.  Requests to considering other
firmware loading needs (such as the two images needed for sPAPR) when
doing this is reasonable.  Blaming me for how inconistent qemu's
firmware loading is today is not ok.

> > Given that pflash operates on a blockdev I don't see a obvious way to
> > change that ...
> 
> We could make pflash accept a plain file which is then used as the
> read-only initialization for a blockdev which isn't backed by anything,
> for example.

Magically creating something for users convenience (blkdev in this case)
isn't without problems.  I did that with usb-storage, which creates a
second device (scsi-disk) automatically.  Caused all sorts of trouble
later on.  Don't feel like repeating that mistake.

cheers,
  Gerd
Peter Maydell Oct. 1, 2013, 3:12 p.m. UTC | #15
On 2 October 2013 00:05, Gerd Hoffmann <kraxel@redhat.com> wrote:
> I just need some way to have different default firmware images for
> different pc machine type versions.  Requests to considering other
> firmware loading needs (such as the two images needed for sPAPR) when
> doing this is reasonable.  Blaming me for how inconistent qemu's
> firmware loading is today is not ok.

You're defining new command line syntax -- it seems reasonable
to think about what it ought to actually mean rather than just
saying "it should mean the same collection of random stuff
that -bios means". If you're going to do that you should just stick
with -bios...

-- PMM
Paolo Bonzini Oct. 1, 2013, 3:28 p.m. UTC | #16
Il 01/10/2013 17:12, Peter Maydell ha scritto:
>> > I just need some way to have different default firmware images for
>> > different pc machine type versions.  Requests to considering other
>> > firmware loading needs (such as the two images needed for sPAPR) when
>> > doing this is reasonable.  Blaming me for how inconistent qemu's
>> > firmware loading is today is not ok.
> You're defining new command line syntax -- it seems reasonable
> to think about what it ought to actually mean rather than just
> saying "it should mean the same collection of random stuff
> that -bios means". If you're going to do that you should just stick
> with -bios...

What it ought to mean could be "the ROM that bootstraps the machine by
loading stuff from a drive (could be NAND, SD or a hardware component
such as virtio/IDE/SCSI)".  But most machines sidestep this by loading
the kernel directly in RAM.

Paolo
Anthony Liguori Oct. 1, 2013, 3:42 p.m. UTC | #17
On Tue, Oct 1, 2013 at 10:28 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 01/10/2013 17:12, Peter Maydell ha scritto:
> >> > I just need some way to have different default firmware images for
> >> > different pc machine type versions.  Requests to considering other
> >> > firmware loading needs (such as the two images needed for sPAPR) when
> >> > doing this is reasonable.  Blaming me for how inconistent qemu's
> >> > firmware loading is today is not ok.
> > You're defining new command line syntax -- it seems reasonable
> > to think about what it ought to actually mean rather than just
> > saying "it should mean the same collection of random stuff
> > that -bios means". If you're going to do that you should just stick
> > with -bios...
>
> What it ought to mean could be "the ROM that bootstraps the machine by
> loading stuff from a drive (could be NAND, SD or a hardware component
> such as virtio/IDE/SCSI)".  But most machines sidestep this by loading
> the kernel directly in RAM.
>

Firmware should not be a special concept in QEMU but it is.  On real
hardware, the firmware is stored in some sort of memory device that happens
to be addressed at a specific location.  This should be modelled as we do
with -pflash as a block device and a ROM device.

But we have a world today where firmware is special so we need to deal with
that.  And since the vast majority of systems have exactly one firmware
blob, we should model the 99% case and let folks figure out how to deal
with the remaining 1%.

BTW, sPAPR is never a good reason to change how something is modelled.  It
is not a hardware system.  It's a virtual machine.  The concepts it has are
not going to cleanly relate to other systems and RTAS is a very good
example of this.  RTAS has more in common with Linux's VDSO than anything
on real hardware.

Regards,

Anthony Liguori


> Paolo
>
Benjamin Herrenschmidt Oct. 1, 2013, 9:23 p.m. UTC | #18
On Wed, 2013-10-02 at 00:23 +1000, Alexey Kardashevskiy wrote:

> SLOF is what is loaded from the very beginning, it configures PCI, cooks
> the device tree and boots the guest system (directly or via yaboot/grub,
> from disk, network or ram). Normal firmware, as usual. It knows all the
> details about the machine so the guest system (linux) does not need to know
> details about PCI host bus adapter or anything like this.
> 
> RTAS is an agent which always lives in RAM when the guest system (linux,
> aix) is up and running.

> It is a light-weight version of SLOF which is left
> in RAM by SLOF and can do board/machine specific tasks such as PCI config
> space access or PCI hotplug

Not exactly ... on traditional IBM firmware, RTAS is some kind of
spin-off of OFW that remains functional at runtime. On jx2x SLOF, it's a
piece of C/asm that operates as standalone code within the context of
the OS. Under qemu, however, RTAS is provided by qemu and is just a 5
instruction trampoline around a hypercall, the actual RTAS functions are
provided by qemu.

As such, we *could* get rid of the RTAS blob from qemu and just put
knowledge about that 5 instructions trampoline in SLOF itself with the
ability to instanciate it.

>  - something what SLOF already knows about and
> something what the guest does not want to know about in details. This came
> from IBM pHyp (traditional server PPC64 hypervisor) and it is quite a big
> firmware. In the case of KVM, it is very small stub which simply passes
> requests to QEMU which does the rest. But it is still a separate binary
> image even in the current QEMU.
> 
> May be some day it will become bigger as from time to time the community
> wants things to be done in a certain way which would mean extending rtas,
> however we (powerpc-server folks) want to hope it won't happen ever :)

Creating a full fledged RTAS is a massive non-sense. I don't understand
what's going on with the qemu community and why people don't seem to
understand what a trainwreck it is to create more layers of undebuggable
firmware blobs and extra project dependencies...

Having that stuff in qemu (and partially in the kernel even) makes
things a lot easier to maintain and debug.

Ben.
 
> Adding Ben in copy, he might have something to add.
> 
>
Alexey Kardashevskiy Oct. 2, 2013, 1:18 a.m. UTC | #19
On 10/02/2013 07:23 AM, Benjamin Herrenschmidt wrote:
> On Wed, 2013-10-02 at 00:23 +1000, Alexey Kardashevskiy wrote:
> 
>> SLOF is what is loaded from the very beginning, it configures PCI, cooks
>> the device tree and boots the guest system (directly or via yaboot/grub,
>> from disk, network or ram). Normal firmware, as usual. It knows all the
>> details about the machine so the guest system (linux) does not need to know
>> details about PCI host bus adapter or anything like this.
>>
>> RTAS is an agent which always lives in RAM when the guest system (linux,
>> aix) is up and running.
> 
>> It is a light-weight version of SLOF which is left
>> in RAM by SLOF and can do board/machine specific tasks such as PCI config
>> space access or PCI hotplug
> 
> Not exactly ... on traditional IBM firmware, RTAS is some kind of
> spin-off of OFW that remains functional at runtime. On jx2x SLOF, it's a
> piece of C/asm that operates as standalone code within the context of
> the OS. Under qemu, however, RTAS is provided by qemu and is just a 5
> instruction trampoline around a hypercall, the actual RTAS functions are
> provided by qemu.
> 
> As such, we *could* get rid of the RTAS blob from qemu and just put
> knowledge about that 5 instructions trampoline in SLOF itself with the
> ability to instanciate it.
> 
>>  - something what SLOF already knows about and
>> something what the guest does not want to know about in details. This came
>> from IBM pHyp (traditional server PPC64 hypervisor) and it is quite a big
>> firmware. In the case of KVM, it is very small stub which simply passes
>> requests to QEMU which does the rest. But it is still a separate binary
>> image even in the current QEMU.
>>
>> May be some day it will become bigger as from time to time the community
>> wants things to be done in a certain way which would mean extending rtas,
>> however we (powerpc-server folks) want to hope it won't happen ever :)
> 
> Creating a full fledged RTAS is a massive non-sense. I don't understand
> what's going on with the qemu community and why people don't seem to
> understand what a trainwreck it is to create more layers of undebuggable
> firmware blobs and extra project dependencies...
> 
> Having that stuff in qemu (and partially in the kernel even) makes
> things a lot easier to maintain and debug.


You misunderstood, the intention was to ignore the fact of existing of RTAS
(yet another firmware) and behave as there is only one firmware and we
probably should shut up and let people do changes which will make RTAS
extention or existense mostly impossible :)


> 
> Ben.
>  
>> Adding Ben in copy, he might have something to add.
>>
>>
> 
>
diff mbox

Patch

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 20795ac..ef47dcb 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -105,7 +105,7 @@  static void clipper_init(QEMUMachineInitArgs *args)
     /* Load PALcode.  Given that this is not "real" cpu palcode,
        but one explicitly written for the emulation, we might as
        well load it directly from and ELF image.  */
-    palcode_filename = (bios_name ? bios_name : "palcode-clipper");
+    palcode_filename = (args->firmware ? args->firmware : "palcode-clipper");
     palcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, palcode_filename);
     if (palcode_filename == NULL) {
         hw_error("no palcode provided\n");
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index fe98ef1..70f90b3 100644
--- 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)
 {
     int linux_boot, i;
     MemoryRegion *ram, *option_rom_mr;
@@ -1144,7 +1145,8 @@  FWCfgState *pc_memory_init(MemoryRegion *system_memory,
 
 
     /* Initialize PC system firmware */
-    pc_system_firmware_init(rom_memory, guest_info->isapc_ram_fw);
+    pc_system_firmware_init(rom_memory, guest_info->isapc_ram_fw,
+                            firmware);
 
     option_rom_mr = g_malloc(sizeof(*option_rom_mr));
     memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c6042c7..2f5c31d 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -131,7 +131,8 @@  static void pc_init1(QEMUMachineInitArgs *args,
                        args->kernel_filename, args->kernel_cmdline,
                        args->initrd_filename,
                        below_4g_mem_size, above_4g_mem_size,
-                       rom_memory, &ram_memory, guest_info);
+                       rom_memory, &ram_memory, guest_info,
+                       args->firmware);
     }
 
     gsi_state = g_malloc0(sizeof(*gsi_state));
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index ca84e1c..15fd1a0 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -118,7 +118,8 @@  static void pc_q35_init(QEMUMachineInitArgs *args)
                        args->kernel_filename, args->kernel_cmdline,
                        args->initrd_filename,
                        below_4g_mem_size, above_4g_mem_size,
-                       rom_memory, &ram_memory, guest_info);
+                       rom_memory, &ram_memory, guest_info,
+                       args->firmware);
     }
 
     /* irq lines */
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index e917c83..87b8aba 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -103,7 +103,8 @@  static void pc_system_flash_init(MemoryRegion *rom_memory,
     pc_isa_bios_init(rom_memory, flash_mem, size);
 }
 
-static void old_pc_system_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
+static void old_pc_system_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw,
+                                   const char *bios_name)
 {
     char *filename;
     MemoryRegion *bios, *isa_bios;
@@ -162,7 +163,8 @@  static void old_pc_system_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
                                 bios);
 }
 
-void pc_system_firmware_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
+void pc_system_firmware_init(MemoryRegion *rom_memory, bool isapc_ram_fw,
+                             const char *firmware)
 {
     DriveInfo *pflash_drv;
 
@@ -170,7 +172,7 @@  void pc_system_firmware_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
 
     if (isapc_ram_fw || pflash_drv == NULL) {
         /* When a pflash drive is not found, use rom-mode */
-        old_pc_system_rom_init(rom_memory, isapc_ram_fw);
+        old_pc_system_rom_init(rom_memory, isapc_ram_fw, firmware);
         return;
     }
 
diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
index f1744ec..e3a6c0c 100644
--- a/hw/lm32/milkymist.c
+++ b/hw/lm32/milkymist.c
@@ -131,10 +131,10 @@  milkymist_init(QEMUMachineInitArgs *args)
     }
 
     /* load bios rom */
-    if (bios_name == NULL) {
-        bios_name = BIOS_FILENAME;
+    if (args->firmware == NULL) {
+        args->firmware = BIOS_FILENAME;
     }
-    bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+    bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, args->firmware);
 
     if (bios_filename) {
         load_image_targphys(bios_filename, BIOS_OFFSET, BIOS_SIZE);
@@ -145,7 +145,7 @@  milkymist_init(QEMUMachineInitArgs *args)
     /* if no kernel is given no valid bios rom is a fatal error */
     if (!kernel_filename && !dinfo && !bios_filename) {
         fprintf(stderr, "qemu: could not load Milkymist One bios '%s'\n",
-                bios_name);
+                args->firmware);
         exit(1);
     }
 
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 9ef3a97..72f5613 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -322,10 +322,10 @@  static void mips_fulong2e_init(QEMUMachineInitArgs *args)
         kernel_entry = load_kernel (env);
         write_bootloader(env, memory_region_get_ram_ptr(bios), kernel_entry);
     } else {
-        if (bios_name == NULL) {
-                bios_name = FULONG_BIOSNAME;
+        if (args->firmware == NULL) {
+                args->firmware = FULONG_BIOSNAME;
         }
-        filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+        filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, args->firmware);
         if (filename) {
             bios_size = load_image_targphys(filename, 0x1fc00000LL,
                                             BIOS_SIZE);
@@ -336,7 +336,7 @@  static void mips_fulong2e_init(QEMUMachineInitArgs *args)
 
         if ((bios_size < 0 || bios_size > BIOS_SIZE) &&
             !kernel_filename && !qtest_enabled()) {
-            error_report("Could not load MIPS bios '%s'", bios_name);
+            error_report("Could not load MIPS bios '%s'", args->firmware);
             exit(1);
         }
     }
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 49bdd02..9d2af7c 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -112,7 +112,8 @@  static void mips_jazz_init(MemoryRegion *address_space,
                            MemoryRegion *address_space_io,
                            ram_addr_t ram_size,
                            const char *cpu_model,
-                           enum jazz_model_e jazz_model)
+                           enum jazz_model_e jazz_model,
+                           const char *bios_name)
 {
     char *filename;
     int bios_size, n;
@@ -310,7 +311,8 @@  void mips_magnum_init(QEMUMachineInitArgs *args)
     ram_addr_t ram_size = args->ram_size;
     const char *cpu_model = args->cpu_model;
         mips_jazz_init(get_system_memory(), get_system_io(),
-                       ram_size, cpu_model, JAZZ_MAGNUM);
+                       ram_size, cpu_model, JAZZ_MAGNUM,
+                       args->firmware);
 }
 
 static
@@ -319,7 +321,8 @@  void mips_pica61_init(QEMUMachineInitArgs *args)
     ram_addr_t ram_size = args->ram_size;
     const char *cpu_model = args->cpu_model;
     mips_jazz_init(get_system_memory(), get_system_io(),
-                   ram_size, cpu_model, JAZZ_PICA61);
+                   ram_size, cpu_model, JAZZ_PICA61,
+                   args->firmware);
 }
 
 static QEMUMachine mips_magnum_machine = {
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 05c8771..e92dc54 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1022,10 +1022,10 @@  void mips_malta_init(QEMUMachineInitArgs *args)
         /* Load firmware from flash. */
         if (!dinfo) {
             /* Load a BIOS image. */
-            if (bios_name == NULL) {
-                bios_name = BIOS_FILENAME;
+            if (args->firmware == NULL) {
+                args->firmware = BIOS_FILENAME;
             }
-            filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+            filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, args->firmware);
             if (filename) {
                 bios_size = load_image_targphys(filename, FLASH_ADDRESS,
                                                 BIOS_SIZE);
@@ -1036,7 +1036,7 @@  void mips_malta_init(QEMUMachineInitArgs *args)
             if ((bios_size < 0 || bios_size > BIOS_SIZE) &&
                 !kernel_filename && !qtest_enabled()) {
                 error_report("Could not load MIPS bios '%s', and no "
-                             "-kernel argument was specified", bios_name);
+                             "-kernel argument was specified", args->firmware);
                 exit(1);
             }
         }
diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index 242bab9..e539099 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -181,9 +181,10 @@  mips_mipssim_init(QEMUMachineInitArgs *args)
     /* Map the BIOS / boot exception handler. */
     memory_region_add_subregion(address_space_mem, 0x1fc00000LL, bios);
     /* Load a BIOS / boot exception handler image. */
-    if (bios_name == NULL)
-        bios_name = BIOS_FILENAME;
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+    if (args->firmware == NULL) {
+        args->firmware = BIOS_FILENAME;
+    }
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, args->firmware);
     if (filename) {
         bios_size = load_image_targphys(filename, 0x1fc00000LL, BIOS_SIZE);
         g_free(filename);
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index e94b543..a90ec4e 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -216,9 +216,10 @@  void mips_r4k_init(QEMUMachineInitArgs *args)
        but initialize the hardware ourselves. When a kernel gets
        preloaded we also initialize the hardware, since the BIOS wasn't
        run. */
-    if (bios_name == NULL)
-        bios_name = BIOS_FILENAME;
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+    if (args->firmware == NULL) {
+        args->firmware = BIOS_FILENAME;
+    }
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, args->firmware);
     if (filename) {
         bios_size = get_image_size(filename);
     } else {
@@ -248,7 +249,7 @@  void mips_r4k_init(QEMUMachineInitArgs *args)
     } else if (!qtest_enabled()) {
 	/* not fatal */
         fprintf(stderr, "qemu: Warning, could not load MIPS bios '%s'\n",
-		bios_name);
+                args->firmware);
     }
     if (filename) {
         g_free(filename);
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 5e79575..82396b2 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -205,9 +205,10 @@  static void ppc_core99_init(QEMUMachineInitArgs *args)
     /* allocate and load BIOS */
     memory_region_init_ram(bios, NULL, "ppc_core99.bios", BIOS_SIZE);
     vmstate_register_ram_global(bios);
-    if (bios_name == NULL)
-        bios_name = PROM_FILENAME;
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+    if (args->firmware == NULL) {
+        args->firmware = PROM_FILENAME;
+    }
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, args->firmware);
     memory_region_set_readonly(bios, true);
     memory_region_add_subregion(get_system_memory(), PROM_ADDR, bios);
 
@@ -221,7 +222,7 @@  static void ppc_core99_init(QEMUMachineInitArgs *args)
         bios_size = -1;
     }
     if (bios_size < 0 || bios_size > BIOS_SIZE) {
-        hw_error("qemu: could not load PowerPC bios '%s'\n", bios_name);
+        hw_error("qemu: could not load PowerPC bios '%s'\n", args->firmware);
         exit(1);
     }
 
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 2f27754..712aff9 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -135,9 +135,10 @@  static void ppc_heathrow_init(QEMUMachineInitArgs *args)
     /* allocate and load BIOS */
     memory_region_init_ram(bios, NULL, "ppc_heathrow.bios", BIOS_SIZE);
     vmstate_register_ram_global(bios);
-    if (bios_name == NULL)
-        bios_name = PROM_FILENAME;
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+    if (args->firmware == NULL) {
+        args->firmware = PROM_FILENAME;
+    }
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, args->firmware);
     memory_region_set_readonly(bios, true);
     memory_region_add_subregion(sysmem, PROM_ADDR, bios);
 
@@ -150,7 +151,7 @@  static void ppc_heathrow_init(QEMUMachineInitArgs *args)
         bios_size = -1;
     }
     if (bios_size < 0 || bios_size > BIOS_SIZE) {
-        hw_error("qemu: could not load PowerPC bios '%s'\n", bios_name);
+        hw_error("qemu: could not load PowerPC bios '%s'\n", args->firmware);
         exit(1);
     }
 
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index f1a8f67..26be692 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -248,20 +248,22 @@  static void ref405ep_init(QEMUMachineInitArgs *args)
         bios = g_new(MemoryRegion, 1);
         memory_region_init_ram(bios, NULL, "ef405ep.bios", BIOS_SIZE);
         vmstate_register_ram_global(bios);
-        if (bios_name == NULL)
-            bios_name = BIOS_FILENAME;
-        filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+        if (args->firmware == NULL) {
+            args->firmware = BIOS_FILENAME;
+        }
+        filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, args->firmware);
         if (filename) {
             bios_size = load_image(filename, memory_region_get_ram_ptr(bios));
             g_free(filename);
             if (bios_size < 0 || bios_size > BIOS_SIZE) {
-                error_report("Could not load PowerPC BIOS '%s'", bios_name);
+                error_report("Could not load PowerPC BIOS '%s'",
+                             args->firmware);
                 exit(1);
             }
             bios_size = (bios_size + 0xfff) & ~0xfff;
             memory_region_add_subregion(sysmem, (uint32_t)(-bios_size), bios);
         } else if (!qtest_enabled() || kernel_filename != NULL) {
-            error_report("Could not load PowerPC BIOS '%s'", bios_name);
+            error_report("Could not load PowerPC BIOS '%s'", args->firmware);
             exit(1);
         } else {
             /* Avoid an uninitialized variable warning */
@@ -564,23 +566,25 @@  static void taihu_405ep_init(QEMUMachineInitArgs *args)
 #ifdef DEBUG_BOARD_INIT
         printf("Load BIOS from file\n");
 #endif
-        if (bios_name == NULL)
-            bios_name = BIOS_FILENAME;
+        if (args->firmware == NULL) {
+            args->firmware = BIOS_FILENAME;
+        }
         bios = g_new(MemoryRegion, 1);
         memory_region_init_ram(bios, NULL, "taihu_405ep.bios", BIOS_SIZE);
         vmstate_register_ram_global(bios);
-        filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+        filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, args->firmware);
         if (filename) {
             bios_size = load_image(filename, memory_region_get_ram_ptr(bios));
             g_free(filename);
             if (bios_size < 0 || bios_size > BIOS_SIZE) {
-                error_report("Could not load PowerPC BIOS '%s'", bios_name);
+                error_report("Could not load PowerPC BIOS '%s'",
+                             args->firmware);
                 exit(1);
             }
             bios_size = (bios_size + 0xfff) & ~0xfff;
             memory_region_add_subregion(sysmem, (uint32_t)(-bios_size), bios);
         } else if (!qtest_enabled()) {
-            error_report("Could not load PowerPC BIOS '%s'", bios_name);
+            error_report("Could not load PowerPC BIOS '%s'", args->firmware);
             exit(1);
         }
         memory_region_set_readonly(bios, true);
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index aad0f69..76340f7 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -514,9 +514,10 @@  static void ppc_prep_init(QEMUMachineInitArgs *args)
     memory_region_set_readonly(bios, true);
     memory_region_add_subregion(sysmem, (uint32_t)(-BIOS_SIZE), bios);
     vmstate_register_ram_global(bios);
-    if (bios_name == NULL)
-        bios_name = BIOS_FILENAME;
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+    if (args->firmware == NULL) {
+        args->firmware = BIOS_FILENAME;
+    }
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, args->firmware);
     if (filename) {
         bios_size = load_elf(filename, NULL, NULL, NULL,
                              NULL, NULL, 1, ELF_MACHINE, 0);
@@ -530,7 +531,7 @@  static void ppc_prep_init(QEMUMachineInitArgs *args)
             }
             if (bios_size > BIOS_SIZE) {
                 fprintf(stderr, "qemu: PReP bios '%s' is too large (0x%x)\n",
-                        bios_name, bios_size);
+                        args->firmware, bios_size);
                 exit(1);
             }
         }
@@ -539,7 +540,7 @@  static void ppc_prep_init(QEMUMachineInitArgs *args)
     }
     if (bios_size < 0 && !qtest_enabled()) {
         fprintf(stderr, "qemu: could not load PPC PReP bios '%s'\n",
-                bios_name);
+                args->firmware);
         exit(1);
     }
     if (filename) {
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 004184d..b05aa4b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1311,10 +1311,10 @@  static void ppc_spapr_init(QEMUMachineInitArgs *args)
         }
     }
 
-    if (bios_name == NULL) {
-        bios_name = FW_FILE_NAME;
+    if (args->firmware == NULL) {
+        args->firmware = FW_FILE_NAME;
     }
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, args->firmware);
     fw_size = load_image_targphys(filename, 0, FW_MAX_SIZE);
     if (fw_size < 0) {
         hw_error("qemu: could not load LPAR rtas '%s'\n", filename);
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index d69adb2..43ab38a 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -61,6 +61,7 @@  typedef struct S390IPLState {
 
 static int s390_ipl_init(SysBusDevice *dev)
 {
+    const char *bios_name = qemu_opt_get(qemu_get_machine_opts(), "firmware");
     S390IPLState *ipl = S390_IPL(dev);
     ram_addr_t kernel_size = 0;
 
diff --git a/hw/sh4/shix.c b/hw/sh4/shix.c
index 1ff37f5..e53b5ec 100644
--- a/hw/sh4/shix.c
+++ b/hw/sh4/shix.c
@@ -73,14 +73,15 @@  static void shix_init(QEMUMachineInitArgs *args)
     memory_region_add_subregion(sysmem, 0x0c000000, &sdram[1]);
 
     /* Load BIOS in 0 (and access it through P2, 0xA0000000) */
-    if (bios_name == NULL)
-        bios_name = BIOS_FILENAME;
-    printf("%s: load BIOS '%s'\n", __func__, bios_name);
-    ret = load_image_targphys(bios_name, 0, 0x4000);
+    if (args->firmware == NULL) {
+        args->firmware = BIOS_FILENAME;
+    }
+    printf("%s: load BIOS '%s'\n", __func__, args->firmware);
+    ret = load_image_targphys(args->firmware, 0, 0x4000);
     if (ret < 0) {		/* Check bios size */
 	fprintf(stderr, "ret=%d\n", ret);
 	fprintf(stderr, "qemu: could not load SHIX bios '%s'\n",
-		bios_name);
+                args->firmware);
 	exit(1);
     }
 
diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 390f3e4..34053b8 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -159,10 +159,10 @@  static void leon3_generic_hw_init(QEMUMachineInitArgs *args)
     memory_region_add_subregion(address_space_mem, 0x00000000, prom);
 
     /* Load boot prom */
-    if (bios_name == NULL) {
-        bios_name = PROM_FILENAME;
+    if (args->firmware == NULL) {
+        args->firmware = PROM_FILENAME;
     }
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, args->firmware);
 
     bios_size = get_image_size(filename);
 
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index a0d366c..0c85a7b 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -871,7 +871,7 @@  static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
         empty_slot_init(args->ram_size, hwdef->max_mem - args->ram_size);
     }
 
-    prom_init(hwdef->slavio_base, bios_name);
+    prom_init(hwdef->slavio_base, args->firmware);
 
     slavio_intctl = slavio_intctl_init(hwdef->intctl_base,
                                        hwdef->intctl_base + 0x10000ULL,
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 6f271d9..cd73149 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -831,7 +831,7 @@  static void sun4uv_init(MemoryRegion *address_space_mem,
     /* set up devices */
     ram_init(0, args->ram_size);
 
-    prom_init(hwdef->prom_addr, bios_name);
+    prom_init(hwdef->prom_addr, args->firmware);
 
     ivec_irqs = qemu_allocate_irqs(cpu_set_ivec_irq, cpu, IVEC_MAX);
     pci_bus = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, ivec_irqs, &pci_bus2,
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 5a7ae9f..d4872a9 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -13,6 +13,7 @@  typedef struct QEMUMachineInitArgs {
     const char *kernel_cmdline;
     const char *initrd_filename;
     const char *cpu_model;
+    const char *firmware;
 } QEMUMachineInitArgs;
 
 typedef void QEMUMachineInitFunc(QEMUMachineInitArgs *args);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 9b2ddc4..caaede8 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -128,7 +128,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);
 qemu_irq *pc_allocate_cpu_irq(void);
 DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
 void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
@@ -211,7 +212,8 @@  static inline bool isa_ne2000_init(ISABus *bus, int base, int irq, NICInfo *nd)
 
 /* pc_sysfw.c */
 void pc_system_firmware_init(MemoryRegion *rom_memory,
-                             bool isapc_ram_fw);
+                             bool isapc_ram_fw,
+                             const char *firmware);
 
 /* pvpanic.c */
 void pvpanic_init(ISABus *bus);
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index cd5791e..92c76a7 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -12,8 +12,6 @@ 
 
 /* vl.c */
 
-extern const char *bios_name;
-
 extern const char *qemu_name;
 extern uint8_t qemu_uuid[];
 extern bool qemu_uuid_set;
diff --git a/vl.c b/vl.c
index 983cdc6..6911814 100644
--- a/vl.c
+++ b/vl.c
@@ -181,7 +181,6 @@  int main(int argc, char **argv)
 
 static const char *data_dir[16];
 static int data_dir_idx;
-const char *bios_name = NULL;
 enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
 DisplayType display_type = DT_DEFAULT;
 static int display_remote;
@@ -428,6 +427,10 @@  static QemuOptsList qemu_machine_opts = {
             .name = "usb",
             .type = QEMU_OPT_BOOL,
             .help = "Set on/off to enable/disable usb",
+        },{
+            .name = "firmware",
+            .type = QEMU_OPT_STRING,
+            .help = "firmware image",
         },
         { /* End of list */ }
     },
@@ -2825,6 +2828,7 @@  int main(int argc, char **argv, char **envp)
     const char *icount_option = NULL;
     const char *initrd_filename;
     const char *kernel_filename, *kernel_cmdline;
+    const char *firmware = NULL;
     const char *boot_order = NULL;
     DisplayState *ds;
     int cyls, heads, secs, translation;
@@ -3228,7 +3232,7 @@  int main(int argc, char **argv, char **envp)
                 }
                 break;
             case QEMU_OPTION_bios:
-                bios_name = optarg;
+                qemu_opts_set(qemu_find_opts("machine"), 0, "firmware", optarg);
                 break;
             case QEMU_OPTION_singlestep:
                 singlestep = 1;
@@ -4049,6 +4053,7 @@  int main(int argc, char **argv, char **envp)
     kernel_filename = qemu_opt_get(machine_opts, "kernel");
     initrd_filename = qemu_opt_get(machine_opts, "initrd");
     kernel_cmdline = qemu_opt_get(machine_opts, "append");
+    firmware = qemu_opt_get(machine_opts, "firmware");
 
     if (!boot_order) {
         boot_order = machine->default_boot_order;
@@ -4243,7 +4248,8 @@  int main(int argc, char **argv, char **envp)
                                  .kernel_filename = kernel_filename,
                                  .kernel_cmdline = kernel_cmdline,
                                  .initrd_filename = initrd_filename,
-                                 .cpu_model = cpu_model };
+                                 .cpu_model = cpu_model,
+                                 .firmware = firmware };
     machine->init(&args);
 
     audio_init();