| Message ID | 1261475822-31679-1-git-send-email-avi@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 12/22/2009 03:57 AM, Avi Kivity wrote: > The first such option rom will load at address 0, which isn't very nice, > and the second will report a conflict and abort, which is horrible. > > Signed-off-by: Avi Kivity<avi@redhat.com> > This fixes extboot in qemu-kvm? Regards, Anthony Liguori
On 12/22/2009 04:50 PM, Anthony Liguori wrote: > On 12/22/2009 03:57 AM, Avi Kivity wrote: >> The first such option rom will load at address 0, which isn't very nice, >> and the second will report a conflict and abort, which is horrible. >> >> Signed-off-by: Avi Kivity<avi@redhat.com> > > This fixes extboot in qemu-kvm? Yes.
On Tue, Dec 22, 2009 at 11:57:02AM +0200, Avi Kivity wrote: > The first such option rom will load at address 0, which isn't very nice, > and the second will report a conflict and abort, which is horrible. > > Signed-off-by: Avi Kivity <avi@redhat.com> > --- > > Changes from v1: > - use ->fw_file instead of ->addr to distinguish between host-loaded and > firmware-loaded roms > - add the same check in a couple more places This patch is a lot better and does not break the versatile platform anymore. I do wonder though if the same change should also be done for find_rom() and rom_copy(). > hw/loader.c | 8 +++++++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > diff --git a/hw/loader.c b/hw/loader.c > index 2ceb8eb..eef385e 100644 > --- a/hw/loader.c > +++ b/hw/loader.c > @@ -636,6 +636,9 @@ static void rom_reset(void *unused) > Rom *rom; > > QTAILQ_FOREACH(rom, &roms, next) { > + if (rom->fw_file) { > + continue; > + } > if (rom->data == NULL) > continue; > cpu_physical_memory_write_rom(rom->addr, rom->data, rom->romsize); > @@ -654,6 +657,9 @@ int rom_load_all(void) > Rom *rom; > > QTAILQ_FOREACH(rom, &roms, next) { > + if (rom->fw_file) { > + continue; > + } > if (addr > rom->addr) { > fprintf(stderr, "rom: requested regions overlap " > "(rom %s. free=0x" TARGET_FMT_plx > @@ -752,7 +758,7 @@ void do_info_roms(Monitor *mon) > Rom *rom; > > QTAILQ_FOREACH(rom, &roms, next) { > - if (rom->addr) { > + if (!rom->fw_file) { > monitor_printf(mon, "addr=" TARGET_FMT_plx > " size=0x%06zx mem=%s name=\"%s\" \n", > rom->addr, rom->romsize, > -- > 1.6.5.3 > > > >
On 12/24/2009 02:17 AM, Aurelien Jarno wrote: > On Tue, Dec 22, 2009 at 11:57:02AM +0200, Avi Kivity wrote: > >> The first such option rom will load at address 0, which isn't very nice, >> and the second will report a conflict and abort, which is horrible. >> >> Signed-off-by: Avi Kivity<avi@redhat.com> >> --- >> >> Changes from v1: >> - use ->fw_file instead of ->addr to distinguish between host-loaded and >> firmware-loaded roms >> - add the same check in a couple more places >> > This patch is a lot better and does not break the versatile platform > anymore. I do wonder though if the same change should also be done for > find_rom() and rom_copy(). > Since I'm not sure what these are used for, I left them as is. IMO the API should be improved by splitting the functions dealing with qemu-loaded and firmware-loaded ROMs to avoid confusion.
On Thu, Dec 24, 2009 at 08:38:32AM +0200, Avi Kivity wrote: > On 12/24/2009 02:17 AM, Aurelien Jarno wrote: > >On Tue, Dec 22, 2009 at 11:57:02AM +0200, Avi Kivity wrote: > >>The first such option rom will load at address 0, which isn't very nice, > >>and the second will report a conflict and abort, which is horrible. > >> > >>Signed-off-by: Avi Kivity<avi@redhat.com> > >>--- > >> > >>Changes from v1: > >>- use ->fw_file instead of ->addr to distinguish between host-loaded and > >> firmware-loaded roms > >>- add the same check in a couple more places > >This patch is a lot better and does not break the versatile platform > >anymore. I do wonder though if the same change should also be done for > >find_rom() and rom_copy(). > > Since I'm not sure what these are used for, I left them as is. IMO > the API should be improved by splitting the functions dealing with > qemu-loaded and firmware-loaded ROMs to avoid confusion. > I have committed your patch to both stable-0.12 and HEAD. As I think these functions have to be changed too, I have committed a patch to do it, but to HEAD only.
diff --git a/hw/loader.c b/hw/loader.c index 2ceb8eb..eef385e 100644 --- a/hw/loader.c +++ b/hw/loader.c @@ -636,6 +636,9 @@ static void rom_reset(void *unused) Rom *rom; QTAILQ_FOREACH(rom, &roms, next) { + if (rom->fw_file) { + continue; + } if (rom->data == NULL) continue; cpu_physical_memory_write_rom(rom->addr, rom->data, rom->romsize); @@ -654,6 +657,9 @@ int rom_load_all(void) Rom *rom; QTAILQ_FOREACH(rom, &roms, next) { + if (rom->fw_file) { + continue; + } if (addr > rom->addr) { fprintf(stderr, "rom: requested regions overlap " "(rom %s. free=0x" TARGET_FMT_plx @@ -752,7 +758,7 @@ void do_info_roms(Monitor *mon) Rom *rom; QTAILQ_FOREACH(rom, &roms, next) { - if (rom->addr) { + if (!rom->fw_file) { monitor_printf(mon, "addr=" TARGET_FMT_plx " size=0x%06zx mem=%s name=\"%s\" \n", rom->addr, rom->romsize,
The first such option rom will load at address 0, which isn't very nice, and the second will report a conflict and abort, which is horrible. Signed-off-by: Avi Kivity <avi@redhat.com> --- Changes from v1: - use ->fw_file instead of ->addr to distinguish between host-loaded and firmware-loaded roms - add the same check in a couple more places hw/loader.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)