[Qemu-devel] [PATCH 0/4] more rom loader patches.

[Qemu-devel] [PATCH 0/4] more rom loader patches.

[Gerd Hoffmann]

  Hi,

A collection of rom loader bits, check individual patches for details.

cheers,
  Gerd

[Qemu-devel] [PATCH 1/4] rom loader: use qemu_strdup.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/loader.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/loader.c b/hw/loader.c
index 7aa1a67..6baafa8 100644
--- a/hw/loader.c
+++ b/hw/loader.c
@@ -559,7 +559,7 @@ int rom_add_file(const char *file,
     rom->name = qemu_strdup(file);
     rom->path = qemu_find_file(QEMU_FILE_TYPE_BIOS, rom->name);
     if (rom->path == NULL) {
-        rom->path = strdup(file);
+        rom->path = qemu_strdup(file);
     }
 
     fd = open(rom->path, O_RDONLY | O_BINARY);
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/4] rom loader: make vga+rom loading target specific

[Gerd Hoffmann]

This patch adds a loader-target.c file for target-specific
rom loading functions.  The rom_add_vga() and rom_add_option()
macros are transformed into functions and sticked in there.  They
load the bios on TARGET_I386 and no nothing on other targets.

With this in place we can move the rom loading calls from pc.c to the
individual drivers.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 Makefile.target    |    2 +-
 hw/loader-target.c |   34 ++++++++++++++++++++++++++++++++++
 hw/loader.h        |    6 ++----
 3 files changed, 37 insertions(+), 5 deletions(-)
 create mode 100644 hw/loader-target.c

diff --git a/Makefile.target b/Makefile.target
index 2726487..6054a68 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -154,7 +154,7 @@ endif #CONFIG_BSD_USER
 # System emulator target
 ifdef CONFIG_SOFTMMU
 
-obj-y = vl.o monitor.o pci.o machine.o gdbstub.o
+obj-y = vl.o monitor.o pci.o machine.o gdbstub.o loader-target.o
 # virtio has to be here due to weird dependency between PCI and virtio-net.
 # need to fix this properly
 obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o virtio-pci.o
diff --git a/hw/loader-target.c b/hw/loader-target.c
new file mode 100644
index 0000000..54a6745
--- /dev/null
+++ b/hw/loader-target.c
@@ -0,0 +1,34 @@
+/*
+ * target specific rom code
+ */
+
+#include "hw.h"
+#include "loader.h"
+
+#ifdef TARGET_I386
+
+int rom_add_vga(const char *file)
+{
+    return rom_add_file(file, PC_ROM_MIN_VGA, PC_ROM_MAX, PC_ROM_ALIGN);
+}
+
+int rom_add_option(const char *file)
+{
+    return rom_add_file(file, PC_ROM_MIN_OPTION, PC_ROM_MAX, PC_ROM_ALIGN);
+}
+
+#else
+
+int __attribute__((weak)) rom_add_vga(const char *file)
+{
+    /* nothing */
+    return 0;
+}
+
+int __attribute__((weak)) rom_add_option(const char *file)
+{
+    /* nothing */
+    return 0;
+}
+
+#endif
diff --git a/hw/loader.h b/hw/loader.h
index 945c662..d013253 100644
--- a/hw/loader.h
+++ b/hw/loader.h
@@ -38,9 +38,7 @@ void do_info_roms(Monitor *mon);
 #define PC_ROM_ALIGN       0x800
 #define PC_ROM_SIZE        (PC_ROM_MAX - PC_ROM_MIN_VGA)
 
-#define rom_add_vga(_f)                                                 \
-    rom_add_file(_f, PC_ROM_MIN_VGA,    PC_ROM_MAX, PC_ROM_ALIGN)
-#define rom_add_option(_f)                                              \
-    rom_add_file(_f, PC_ROM_MIN_OPTION, PC_ROM_MAX, PC_ROM_ALIGN)
+int rom_add_vga(const char *file);
+int rom_add_option(const char *file);
 
 #endif
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 2/4] rom loader: make vga+rom loading target specific

[Blue Swirl]

On Tue, Oct 13, 2009 at 2:06 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> This patch adds a loader-target.c file for target-specific
> rom loading functions.  The rom_add_vga() and rom_add_option()
> macros are transformed into functions and sticked in there.  They
> load the bios on TARGET_I386 and no nothing on other targets.

IIRC the VGA cards on non-x86 machines contain ROMs too, some (PPC?
Alpha?) machine had a simple x86 emulator just to run the ROM code.
Then you could enable the VGA ROM loading unconditionally. So maybe
the patch isn't necessary?

Otherwise you could add two files without macros, then the files can
be compiled just once.

Re: [Qemu-devel] [PATCH 2/4] rom loader: make vga+rom loading target specific

[Gerd Hoffmann]

On 10/13/09 16:58, Blue Swirl wrote:
> On Tue, Oct 13, 2009 at 2:06 PM, Gerd Hoffmann<kraxel@redhat.com>  wrote:
>> This patch adds a loader-target.c file for target-specific
>> rom loading functions.  The rom_add_vga() and rom_add_option()
>> macros are transformed into functions and sticked in there.  They
>> load the bios on TARGET_I386 and no nothing on other targets.
>
> IIRC the VGA cards on non-x86 machines contain ROMs too, some (PPC?
> Alpha?) machine had a simple x86 emulator just to run the ROM code.
> Then you could enable the VGA ROM loading unconditionally. So maybe
> the patch isn't necessary?

Is the vgabios actually loaded to 0xc0000 (like x86 does) or does it 
just sit in the PCI ROM bar?

> Otherwise you could add two files without macros, then the files can
> be compiled just once.

... with lot of Makefile.target changes which I wanted to avoid.

Or is there a simple way to say "all but i386 please use $thisfile.c"?

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 2/4] rom loader: make vga+rom loading target specific

[Blue Swirl]

On Tue, Oct 13, 2009 at 6:29 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 10/13/09 16:58, Blue Swirl wrote:
>>
>> On Tue, Oct 13, 2009 at 2:06 PM, Gerd Hoffmann<kraxel@redhat.com>  wrote:
>>>
>>> This patch adds a loader-target.c file for target-specific
>>> rom loading functions.  The rom_add_vga() and rom_add_option()
>>> macros are transformed into functions and sticked in there.  They
>>> load the bios on TARGET_I386 and no nothing on other targets.
>>
>> IIRC the VGA cards on non-x86 machines contain ROMs too, some (PPC?
>> Alpha?) machine had a simple x86 emulator just to run the ROM code.
>> Then you could enable the VGA ROM loading unconditionally. So maybe
>> the patch isn't necessary?
>
> Is the vgabios actually loaded to 0xc0000 (like x86 does) or does it just
> sit in the PCI ROM bar?

No idea, probably not loaded.

>> Otherwise you could add two files without macros, then the files can
>> be compiled just once.
>
> ... with lot of Makefile.target changes which I wanted to avoid.
>
> Or is there a simple way to say "all but i386 please use $thisfile.c"?

ifdef TARGET_I386
obj-y += loader-target.o
else
obj-y += loader-dummy.o
endif
?

Re: [Qemu-devel] [PATCH 2/4] rom loader: make vga+rom loading target specific

[Gerd Hoffmann]

>> Or is there a simple way to say "all but i386 please use $thisfile.c"?
>
> ifdef TARGET_I386
> obj-y += loader-target.o
> else
> obj-y += loader-dummy.o
> endif
> ?

Yep, should do, although I'd call it loader-i386.o or loader-pc.o then.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 2/4] rom loader: make vga+rom loading target specific

[Natalia Portillo]

SmartFirmware on Genesi Pegasos boards do.

I have one and when I boot I see the ATI Radeon 9200 BIOS message,  
then openfirmware prompt.

Macs do require openfirmware forth rom (powerpc macs) or EFI rom  
(intel macs, not know if compiled for x86 or efi interpreter)

I've head the AmigaONE openfirmware implementation does the same as  
SmartFirmware (emulator), not tested any other architecture.

El 13/10/2009, a las 15:58, Blue Swirl escribió:

> On Tue, Oct 13, 2009 at 2:06 PM, Gerd Hoffmann <kraxel@redhat.com>  
> wrote:
>> This patch adds a loader-target.c file for target-specific
>> rom loading functions.  The rom_add_vga() and rom_add_option()
>> macros are transformed into functions and sticked in there.  They
>> load the bios on TARGET_I386 and no nothing on other targets.
>
> IIRC the VGA cards on non-x86 machines contain ROMs too, some (PPC?
> Alpha?) machine had a simple x86 emulator just to run the ROM code.
> Then you could enable the VGA ROM loading unconditionally. So maybe
> the patch isn't necessary?
>
> Otherwise you could add two files without macros, then the files can
> be compiled just once.
>
>

Re: [Qemu-devel] [PATCH 2/4] rom loader: make vga+rom loading target specific

[Gerd Hoffmann]

On 10/14/09 00:03, Natalia Portillo wrote:
> SmartFirmware on Genesi Pegasos boards do.
>
> I have one and when I boot I see the ATI Radeon 9200 BIOS message, then
> openfirmware prompt.

Big question is whenever they actually load the rom to 0xc0000 or 
whenever they let the x86 emulator read the rom data directly from the 
pci address space.  I suspect the later.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 2/4] rom loader: make vga+rom loading target specific

[Natalia Portillo]

I suspect the later also.

Theorically, SmartFirmware source is available, as well as AmigaONE's  
implementation.

Dunno if the x86 emulator is included in the source.

What I know is that my SF is not enough stable to run Linux and check  
the OF memory map. (sadly)

El 14/10/2009, a las 11:21, Gerd Hoffmann escribió:

> On 10/14/09 00:03, Natalia Portillo wrote:
>> SmartFirmware on Genesi Pegasos boards do.
>>
>> I have one and when I boot I see the ATI Radeon 9200 BIOS message,  
>> then
>> openfirmware prompt.
>
> Big question is whenever they actually load the rom to 0xc0000 or  
> whenever they let the x86 emulator read the rom data directly from  
> the pci address space.  I suspect the later.
>
> cheers,
>  Gerd
>

[Qemu-devel] [PATCH 3/4] vga roms: move loading from pc.c to vga drivers.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/cirrus_vga.c |    6 +++++-
 hw/pc.c         |   12 ------------
 hw/vga-isa.c    |    3 +++
 hw/vga-pci.c    |    4 ++++
 hw/vga_int.h    |    2 ++
 5 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 9dfe76a..f4c9cdf 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -32,6 +32,7 @@
 #include "console.h"
 #include "vga_int.h"
 #include "kvm.h"
+#include "loader.h"
 
 /*
  * TODO:
@@ -3162,6 +3163,7 @@ void isa_cirrus_vga_init(void)
                                      s->vga.screen_dump, s->vga.text_update,
                                      &s->vga);
     vmstate_register(0, &vmstate_cirrus_vga, s);
+    rom_add_vga(VGABIOS_CIRRUS_FILENAME);
     /* XXX ISA-LFB support */
 }
 
@@ -3245,7 +3247,9 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
                           PCI_ADDRESS_SPACE_MEM, cirrus_pci_mmio_map);
      }
      vmstate_register(0, &vmstate_pci_cirrus_vga, d);
-     /* XXX: ROM BIOS */
+
+     /* ROM BIOS */
+     rom_add_vga(VGABIOS_CIRRUS_FILENAME);
      return 0;
 }
 
diff --git a/hw/pc.c b/hw/pc.c
index 240161a..c4fa3f7 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -47,8 +47,6 @@
 //#define DEBUG_MULTIBOOT
 
 #define BIOS_FILENAME "bios.bin"
-#define VGABIOS_FILENAME "vgabios.bin"
-#define VGABIOS_CIRRUS_FILENAME "vgabios-cirrus.bin"
 
 #define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
 
@@ -1043,7 +1041,6 @@ static void pc_init1(ram_addr_t ram_size,
     IsaIrqState *isa_irq_state;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     DriveInfo *fd[MAX_FD];
-    int using_vga = cirrus_vga_enabled || std_vga_enabled || vmsvga_enabled;
     void *fw_cfg;
 
     if (ram_size >= 0xe0000000 ) {
@@ -1133,15 +1130,6 @@ static void pc_init1(ram_addr_t ram_size,
     option_rom_offset = qemu_ram_alloc(PC_ROM_SIZE);
     cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, option_rom_offset);
 
-    if (using_vga) {
-        /* VGA BIOS load */
-        if (cirrus_vga_enabled) {
-            rom_add_vga(VGABIOS_CIRRUS_FILENAME);
-        } else {
-            rom_add_vga(VGABIOS_FILENAME);
-        }
-    }
-
     /* map all the bios at the top of memory */
     cpu_register_physical_memory((uint32_t)(-bios_size),
                                  bios_size, bios_offset | IO_MEM_ROM);
diff --git a/hw/vga-isa.c b/hw/vga-isa.c
index 7fa31d3..6b387af 100644
--- a/hw/vga-isa.c
+++ b/hw/vga-isa.c
@@ -27,6 +27,7 @@
 #include "vga_int.h"
 #include "pixel_ops.h"
 #include "qemu-timer.h"
+#include "loader.h"
 
 int isa_vga_init(void)
 {
@@ -46,5 +47,7 @@ int isa_vga_init(void)
     cpu_register_physical_memory(VBE_DISPI_LFB_PHYSICAL_ADDRESS,
                                  VGA_RAM_SIZE, s->vram_offset);
 #endif
+    /* ROM BIOS */
+    rom_add_vga(VGABIOS_FILENAME);
     return 0;
 }
diff --git a/hw/vga-pci.c b/hw/vga-pci.c
index 5e75938..b5fd666 100644
--- a/hw/vga-pci.c
+++ b/hw/vga-pci.c
@@ -28,6 +28,7 @@
 #include "vga_int.h"
 #include "pixel_ops.h"
 #include "qemu-timer.h"
+#include "loader.h"
 
 typedef struct PCIVGAState {
     PCIDevice dev;
@@ -117,6 +118,9 @@ static int pci_vga_initfn(PCIDevice *dev)
         pci_register_bar(&d->dev, PCI_ROM_SLOT, bios_total_size,
                          PCI_ADDRESS_SPACE_MEM_PREFETCH, vga_map);
      }
+
+     /* ROM BIOS */
+     rom_add_vga(VGABIOS_FILENAME);
      return 0;
 }
 
diff --git a/hw/vga_int.h b/hw/vga_int.h
index c162c07..b843e52 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -220,6 +220,8 @@ extern const uint8_t sr_mask[8];
 extern const uint8_t gr_mask[16];
 
 #define VGA_RAM_SIZE (8192 * 1024)
+#define VGABIOS_FILENAME "vgabios.bin"
+#define VGABIOS_CIRRUS_FILENAME "vgabios-cirrus.bin"
 
 extern CPUReadMemoryFunc * const vga_mem_read[3];
 extern CPUWriteMemoryFunc * const vga_mem_write[3];
-- 
1.6.2.5

[Qemu-devel] [PATCH 4/4] use rom loader for pc bios.

[Gerd Hoffmann]

The pc bios shows up in 'info roms' now.

Note that the BIOS is mapped to two places: The complete rom at the top
of the memory, and the first 128k at 0xe0000.  Only the first place is
listed in 'info roms'.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index c4fa3f7..06e9143 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1108,8 +1108,8 @@ static void pc_init1(ram_addr_t ram_size,
         goto bios_error;
     }
     bios_offset = qemu_ram_alloc(bios_size);
-    ret = load_image(filename, qemu_get_ram_ptr(bios_offset));
-    if (ret != bios_size) {
+    ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size));
+    if (ret != 0) {
     bios_error:
         fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name);
         exit(1);
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 4/4] use rom loader for pc bios.

[Carl-Daniel Hailfinger]

On 13.10.2009 13:06, Gerd Hoffmann wrote:
> The pc bios shows up in 'info roms' now.
>
> Note that the BIOS is mapped to two places: The complete rom at the top
> of the memory, and the first 128k at 0xe0000.  Only the first place is
> listed in 'info roms'.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/pc.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index c4fa3f7..06e9143 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -1108,8 +1108,8 @@ static void pc_init1(ram_addr_t ram_size,
>          goto bios_error;
>      }
>      bios_offset = qemu_ram_alloc(bios_size);
> -    ret = load_image(filename, qemu_get_ram_ptr(bios_offset));
> -    if (ret != bios_size) {
> +    ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size));
>   

Memory leak? AFAICS the qemu_ram_alloc above should be removed and maybe
bios_offset needs to be killed completely.

> +    if (ret != 0) {
>      bios_error:
>          fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name);
>          exit(1);
>   

Regards,
Carl-Daniel

-- 
Developer quote of the week: 
"We are juggling too many chainsaws and flaming arrows and tigers."

Re: [Qemu-devel] [PATCH 4/4] use rom loader for pc bios.

[Gerd Hoffmann]

>>       bios_offset = qemu_ram_alloc(bios_size);
>> -    ret = load_image(filename, qemu_get_ram_ptr(bios_offset));
>> -    if (ret != bios_size) {
>> +    ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size));
>>
>
> Memory leak?

I don't think so.

> AFAICS the qemu_ram_alloc above should be removed and maybe
> bios_offset needs to be killed completely.

Why?  You still need the memory for the bios rom, even if the way it 
gets loaded changes.

cheers,
   Gerd

[Qemu-devel] [PATCH v2 0/4] more rom loader patches.

[Gerd Hoffmann]

  Hi,

A collection of rom loader bits, check individual patches for details.

New in v2: split loader-target.c into two files so we can compile it
once as suggested by blueswirl.

cheers,
  Gerd

[Qemu-devel] [PATCH 2/4] rom loader: make vga+rom loading target specific

[Gerd Hoffmann]

This patch adds a loader-target.c file for target-specific
rom loading functions.  The rom_add_vga() and rom_add_option()
macros are transformed into functions and sticked in there.  They
load the bios on TARGET_I386 and no nothing on other targets.

With this in place we can move the rom loading calls from pc.c to the
individual drivers.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 Makefile.hw       |    7 +++++++
 hw/loader-dummy.c |   18 ++++++++++++++++++
 hw/loader-i386.c  |   16 ++++++++++++++++
 hw/loader.h       |    6 ++----
 4 files changed, 43 insertions(+), 4 deletions(-)
 create mode 100644 hw/loader-dummy.c
 create mode 100644 hw/loader-i386.c

diff --git a/Makefile.hw b/Makefile.hw
index f4b4469..7da6775 100644
--- a/Makefile.hw
+++ b/Makefile.hw
@@ -12,7 +12,14 @@ VPATH=$(SRC_PATH):$(SRC_PATH)/hw
 QEMU_CFLAGS+=-I.. -I$(SRC_PATH)/fpu
 
 obj-y =
+
 obj-y += loader.o
+ifdef TARGET_I386
+obj-y += loader-i386.o
+else
+obj-y += loader-dummy.o
+endif
+
 obj-y += virtio.o
 obj-y += fw_cfg.o
 obj-y += watchdog.o
diff --git a/hw/loader-dummy.c b/hw/loader-dummy.c
new file mode 100644
index 0000000..31e12ce
--- /dev/null
+++ b/hw/loader-dummy.c
@@ -0,0 +1,18 @@
+/*
+ * target specific rom code -- dummy
+ */
+
+#include "hw.h"
+#include "loader.h"
+
+int rom_add_vga(const char *file)
+{
+    /* nothing */
+    return 0;
+}
+
+int rom_add_option(const char *file)
+{
+    /* nothing */
+    return 0;
+}
diff --git a/hw/loader-i386.c b/hw/loader-i386.c
new file mode 100644
index 0000000..06eaf4d
--- /dev/null
+++ b/hw/loader-i386.c
@@ -0,0 +1,16 @@
+/*
+ * target specific rom code -- i386
+ */
+
+#include "hw.h"
+#include "loader.h"
+
+int rom_add_vga(const char *file)
+{
+    return rom_add_file(file, PC_ROM_MIN_VGA, PC_ROM_MAX, PC_ROM_ALIGN);
+}
+
+int rom_add_option(const char *file)
+{
+    return rom_add_file(file, PC_ROM_MIN_OPTION, PC_ROM_MAX, PC_ROM_ALIGN);
+}
diff --git a/hw/loader.h b/hw/loader.h
index 945c662..d013253 100644
--- a/hw/loader.h
+++ b/hw/loader.h
@@ -38,9 +38,7 @@ void do_info_roms(Monitor *mon);
 #define PC_ROM_ALIGN       0x800
 #define PC_ROM_SIZE        (PC_ROM_MAX - PC_ROM_MIN_VGA)
 
-#define rom_add_vga(_f)                                                 \
-    rom_add_file(_f, PC_ROM_MIN_VGA,    PC_ROM_MAX, PC_ROM_ALIGN)
-#define rom_add_option(_f)                                              \
-    rom_add_file(_f, PC_ROM_MIN_OPTION, PC_ROM_MAX, PC_ROM_ALIGN)
+int rom_add_vga(const char *file);
+int rom_add_option(const char *file);
 
 #endif
-- 
1.6.2.5