[Qemu-devel] [PATCH] get roms more room.

[Qemu-devel] [PATCH] get roms more room.

[Glauber Costa]

This patch increases by 50 % the size available for option roms.
The main motivator is that some roms grew bigger than the 64k we
currently allocate for them (Hey, it's 2009!)

One example is the gpxe project, that produces some roms with 69k,
70k, etc. The space proposed by this patch actually makes it as
big as 84k. Probably still a fit for some time.

But there is no free lunch. This space must come from somewhere,
and we take it from vga rom space. Currently, our vga roms are
around 35k in size. With this patch, option rom space will begin
just after vga ends, aligned to the next 2k boundary.

Technicaly, we could do the same with the uper space (the bios itself),
but since bochs bios is already 128 k in size, I don't see an
urgent need to do it.

[ fix case for vgabioses smaller than 30k, by Carl-Daniel Hailfinger ]

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 hw/pc.c |   29 +++++++++++++++++++----------
 1 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 69f25f3..f9cfd1f 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -763,7 +763,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
 {
     char buf[1024];
     int ret, linux_boot, i;
-    ram_addr_t ram_addr, vga_ram_addr, bios_offset, vga_bios_offset;
+    ram_addr_t ram_addr, vga_ram_addr, bios_offset, vga_bios_offset, option_rom_start = 0;
     ram_addr_t below_4g_mem_size, above_4g_mem_size = 0;
     int bios_size, isa_bios_size, vga_bios_size;
     PCIBus *pci_bus;
@@ -774,6 +774,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
     int index;
     BlockDriverState *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     BlockDriverState *fd[MAX_FD];
+    int using_vga = cirrus_vga_enabled || std_vga_enabled || vmsvga_enabled;
 
     if (ram_size >= 0xe0000000 ) {
         above_4g_mem_size = ram_size - 0xe0000000;
@@ -856,7 +857,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
         exit(1);
     }
 
-    if (cirrus_vga_enabled || std_vga_enabled || vmsvga_enabled) {
+    if (using_vga) {
         /* VGA BIOS load */
         if (cirrus_vga_enabled) {
             snprintf(buf, sizeof(buf), "%s/%s", bios_dir, VGABIOS_CIRRUS_FILENAME);
@@ -874,12 +875,21 @@ vga_bios_error:
             fprintf(stderr, "qemu: could not load VGA BIOS '%s'\n", buf);
             exit(1);
         }
+	/* Round up vga bios size to the next 2k boundary */
+	vga_bios_size = (vga_bios_size + 2047) & ~2047;
+	option_rom_start = 0xc0000 + vga_bios_size;
 
         /* setup basic memory access */
-        cpu_register_physical_memory(0xc0000, 0x10000,
+        cpu_register_physical_memory(0xc0000, vga_bios_size,
                                      vga_bios_offset | IO_MEM_ROM);
     }
 
+    /* No point in placing option roms before this address, since bochs bios
+     * will only start looking for it at 0xc8000 */
+    if (option_rom_start < 0xc8000)
+	    option_rom_start = 0xc8000;
+
+
     /* map the last 128KB of the BIOS in ISA space */
     isa_bios_size = bios_size;
     if (isa_bios_size > (128 * 1024))
@@ -892,14 +902,14 @@ vga_bios_error:
         ram_addr_t option_rom_offset;
         int size, offset;
 
-        offset = 0;
+        offset = option_rom_start;
         if (linux_boot) {
             option_rom_offset = qemu_ram_alloc(TARGET_PAGE_SIZE);
             load_linux(phys_ram_base + option_rom_offset,
                        kernel_filename, initrd_filename, kernel_cmdline);
-            cpu_register_physical_memory(0xd0000, TARGET_PAGE_SIZE,
+            cpu_register_physical_memory(option_rom_start, TARGET_PAGE_SIZE,
                                          option_rom_offset | IO_MEM_ROM);
-            offset = TARGET_PAGE_SIZE;
+            offset += TARGET_PAGE_SIZE;
         }
 
         for (i = 0; i < nb_option_roms; i++) {
@@ -909,18 +919,17 @@ vga_bios_error:
                         option_rom[i]);
                 exit(1);
             }
-            if (size > (0x10000 - offset))
+            if (size > (0xe0000  - offset))
                 goto option_rom_error;
             option_rom_offset = qemu_ram_alloc(size);
             ret = load_image(option_rom[i], phys_ram_base + option_rom_offset);
             if (ret != size) {
             option_rom_error:
-                fprintf(stderr, "Too many option ROMS\n");
+                fprintf(stderr, "Could not fit %soption roms in available space\n", using_vga ? "VGA bios and " : "");
                 exit(1);
             }
             size = (size + 4095) & ~4095;
-            cpu_register_physical_memory(0xd0000 + offset,
-                                         size, option_rom_offset | IO_MEM_ROM);
+            cpu_register_physical_memory(offset, size, option_rom_offset | IO_MEM_ROM);
             offset += size;
         }
     }
-- 
1.6.2

Re: [Qemu-devel] [PATCH] get roms more room.

[Carl-Daniel Hailfinger]

On 28.03.2009 05:25, Glauber Costa wrote:
> This patch increases by 50 % the size available for option roms.
> The main motivator is that some roms grew bigger than the 64k we
> currently allocate for them (Hey, it's 2009!)
>
> One example is the gpxe project, that produces some roms with 69k,
> 70k, etc. The space proposed by this patch actually makes it as
> big as 84k. Probably still a fit for some time.
>
> But there is no free lunch. This space must come from somewhere,
> and we take it from vga rom space. Currently, our vga roms are
> around 35k in size. With this patch, option rom space will begin
> just after vga ends, aligned to the next 2k boundary.
>
> Technicaly, we could do the same with the uper space (the bios itself),
> but since bochs bios is already 128 k in size, I don't see an
> urgent need to do it.
>
> [ fix case for vgabioses smaller than 30k, by Carl-Daniel Hailfinger ]
>
> Signed-off-by: Glauber Costa <glommer@redhat.com>
>   

Thanks for your patience with my comments!

Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/

Re: [Qemu-devel] [PATCH] get roms more room.

[Anthony Liguori]

Glauber Costa wrote:
> This patch increases by 50 % the size available for option roms.
> The main motivator is that some roms grew bigger than the 64k we
> currently allocate for them (Hey, it's 2009!)
>
> One example is the gpxe project, that produces some roms with 69k,
> 70k, etc. The space proposed by this patch actually makes it as
> big as 84k. Probably still a fit for some time.
>
> But there is no free lunch. This space must come from somewhere,
> and we take it from vga rom space. Currently, our vga roms are
> around 35k in size. With this patch, option rom space will begin
> just after vga ends, aligned to the next 2k boundary.
>
> Technicaly, we could do the same with the uper space (the bios itself),
> but since bochs bios is already 128 k in size, I don't see an
> urgent need to do it.
>
> [ fix case for vgabioses smaller than 30k, by Carl-Daniel Hailfinger ]
>
> Signed-off-by: Glauber Costa <glommer@redhat.com>
>   

Applied.  Thanks.

Sounds like it's time to switch to gpxe roms.  I wonder what ever 
happened to the gpxe virtio-blk patches...

Regards,

Anthony Liguori

> ---
>  hw/pc.c |   29 +++++++++++++++++++----------
>  1 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 69f25f3..f9cfd1f 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -763,7 +763,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
>  {
>      char buf[1024];
>      int ret, linux_boot, i;
> -    ram_addr_t ram_addr, vga_ram_addr, bios_offset, vga_bios_offset;
> +    ram_addr_t ram_addr, vga_ram_addr, bios_offset, vga_bios_offset, option_rom_start = 0;
>      ram_addr_t below_4g_mem_size, above_4g_mem_size = 0;
>      int bios_size, isa_bios_size, vga_bios_size;
>      PCIBus *pci_bus;
> @@ -774,6 +774,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
>      int index;
>      BlockDriverState *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>      BlockDriverState *fd[MAX_FD];
> +    int using_vga = cirrus_vga_enabled || std_vga_enabled || vmsvga_enabled;
>  
>      if (ram_size >= 0xe0000000 ) {
>          above_4g_mem_size = ram_size - 0xe0000000;
> @@ -856,7 +857,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
>          exit(1);
>      }
>  
> -    if (cirrus_vga_enabled || std_vga_enabled || vmsvga_enabled) {
> +    if (using_vga) {
>          /* VGA BIOS load */
>          if (cirrus_vga_enabled) {
>              snprintf(buf, sizeof(buf), "%s/%s", bios_dir, VGABIOS_CIRRUS_FILENAME);
> @@ -874,12 +875,21 @@ vga_bios_error:
>              fprintf(stderr, "qemu: could not load VGA BIOS '%s'\n", buf);
>              exit(1);
>          }
> +	/* Round up vga bios size to the next 2k boundary */
> +	vga_bios_size = (vga_bios_size + 2047) & ~2047;
> +	option_rom_start = 0xc0000 + vga_bios_size;
>  
>          /* setup basic memory access */
> -        cpu_register_physical_memory(0xc0000, 0x10000,
> +        cpu_register_physical_memory(0xc0000, vga_bios_size,
>                                       vga_bios_offset | IO_MEM_ROM);
>      }
>  
> +    /* No point in placing option roms before this address, since bochs bios
> +     * will only start looking for it at 0xc8000 */
> +    if (option_rom_start < 0xc8000)
> +	    option_rom_start = 0xc8000;
> +
> +
>      /* map the last 128KB of the BIOS in ISA space */
>      isa_bios_size = bios_size;
>      if (isa_bios_size > (128 * 1024))
> @@ -892,14 +902,14 @@ vga_bios_error:
>          ram_addr_t option_rom_offset;
>          int size, offset;
>  
> -        offset = 0;
> +        offset = option_rom_start;
>          if (linux_boot) {
>              option_rom_offset = qemu_ram_alloc(TARGET_PAGE_SIZE);
>              load_linux(phys_ram_base + option_rom_offset,
>                         kernel_filename, initrd_filename, kernel_cmdline);
> -            cpu_register_physical_memory(0xd0000, TARGET_PAGE_SIZE,
> +            cpu_register_physical_memory(option_rom_start, TARGET_PAGE_SIZE,
>                                           option_rom_offset | IO_MEM_ROM);
> -            offset = TARGET_PAGE_SIZE;
> +            offset += TARGET_PAGE_SIZE;
>          }
>  
>          for (i = 0; i < nb_option_roms; i++) {
> @@ -909,18 +919,17 @@ vga_bios_error:
>                          option_rom[i]);
>                  exit(1);
>              }
> -            if (size > (0x10000 - offset))
> +            if (size > (0xe0000  - offset))
>                  goto option_rom_error;
>              option_rom_offset = qemu_ram_alloc(size);
>              ret = load_image(option_rom[i], phys_ram_base + option_rom_offset);
>              if (ret != size) {
>              option_rom_error:
> -                fprintf(stderr, "Too many option ROMS\n");
> +                fprintf(stderr, "Could not fit %soption roms in available space\n", using_vga ? "VGA bios and " : "");
>                  exit(1);
>              }
>              size = (size + 4095) & ~4095;
> -            cpu_register_physical_memory(0xd0000 + offset,
> -                                         size, option_rom_offset | IO_MEM_ROM);
> +            cpu_register_physical_memory(offset, size, option_rom_offset | IO_MEM_ROM);
>              offset += size;
>          }
>      }
>   

Re: [Qemu-devel] [PATCH] get roms more room.

[Glauber Costa]

On Sat, Mar 28, 2009 at 2:30 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Glauber Costa wrote:
>>
>> This patch increases by 50 % the size available for option roms.
>> The main motivator is that some roms grew bigger than the 64k we
>> currently allocate for them (Hey, it's 2009!)
>>
>> One example is the gpxe project, that produces some roms with 69k,
>> 70k, etc. The space proposed by this patch actually makes it as
>> big as 84k. Probably still a fit for some time.
>>
>> But there is no free lunch. This space must come from somewhere,
>> and we take it from vga rom space. Currently, our vga roms are
>> around 35k in size. With this patch, option rom space will begin
>> just after vga ends, aligned to the next 2k boundary.
>>
>> Technicaly, we could do the same with the uper space (the bios itself),
>> but since bochs bios is already 128 k in size, I don't see an
>> urgent need to do it.
>>
>> [ fix case for vgabioses smaller than 30k, by Carl-Daniel Hailfinger ]
>>
>> Signed-off-by: Glauber Costa <glommer@redhat.com>
>>
>
> Applied.  Thanks.
>
> Sounds like it's time to switch to gpxe roms.  I wonder what ever happened
> to the gpxe virtio-blk patches...
>
> Regards,
Not yet.

There's still a patch that needs to be applied to the bios. I've
already sent it to bochs-developers list, I believe it should go
there, rather than here.

Also, currently gpxe does not work with kvm, just qemu. Stiil unsure
why, but something to do with interrupts being disabled when they
shouldn't. I'm still on this,
expect news soon.


-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

[Qemu-devel] [PATCH] get roms more room.

[Glauber Costa]

This patch increases by 50 % the size available for option roms.
The main motivator is that some roms grew bigger than the 64k we
currently allocate for them (Hey, it's 2009!)

One example is the gpxe project, that produces some roms with 69k,
70k, etc. The space proposed by this patch actually makes it as
big as 84k. Probably still a fit for some time.

But there is no free lunch. This space must come from somewhere,
and we take it from vga rom space. Currently, our vga roms are
around 35k in size. With this patch, option rom space will begin
just after vga ends, aligned to the next 2k boundary.

Technicaly, we could do the same with the uper space (the bios itself),
but since bochs bios is already 128 k in size, I don't see an
urgent need to do it.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 hw/pc.c |   27 +++++++++++++++++----------
 1 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 69f25f3..07a9791 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -763,7 +763,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
 {
     char buf[1024];
     int ret, linux_boot, i;
-    ram_addr_t ram_addr, vga_ram_addr, bios_offset, vga_bios_offset;
+    ram_addr_t ram_addr, vga_ram_addr, bios_offset, vga_bios_offset, option_rom_start;
     ram_addr_t below_4g_mem_size, above_4g_mem_size = 0;
     int bios_size, isa_bios_size, vga_bios_size;
     PCIBus *pci_bus;
@@ -774,6 +774,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
     int index;
     BlockDriverState *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     BlockDriverState *fd[MAX_FD];
+    int using_vga = cirrus_vga_enabled || std_vga_enabled || vmsvga_enabled;
 
     if (ram_size >= 0xe0000000 ) {
         above_4g_mem_size = ram_size - 0xe0000000;
@@ -856,7 +857,11 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
         exit(1);
     }
 
-    if (cirrus_vga_enabled || std_vga_enabled || vmsvga_enabled) {
+    /* No point in placing option roms before this address, since bochs bios
+     * will only start looking for it at 0xc8000 */
+    option_rom_start = 0xc8000;
+
+    if (using_vga) {
         /* VGA BIOS load */
         if (cirrus_vga_enabled) {
             snprintf(buf, sizeof(buf), "%s/%s", bios_dir, VGABIOS_CIRRUS_FILENAME);
@@ -874,9 +879,12 @@ vga_bios_error:
             fprintf(stderr, "qemu: could not load VGA BIOS '%s'\n", buf);
             exit(1);
         }
+	/* Round up vga bios size to the next 2k boundary */
+	vga_bios_size = (vga_bios_size + 2047) & ~2047;
+	option_rom_start = 0xc0000 + vga_bios_size;
 
         /* setup basic memory access */
-        cpu_register_physical_memory(0xc0000, 0x10000,
+        cpu_register_physical_memory(0xc0000, vga_bios_size,
                                      vga_bios_offset | IO_MEM_ROM);
     }
 
@@ -892,14 +900,14 @@ vga_bios_error:
         ram_addr_t option_rom_offset;
         int size, offset;
 
-        offset = 0;
+        offset = option_rom_start;
         if (linux_boot) {
             option_rom_offset = qemu_ram_alloc(TARGET_PAGE_SIZE);
             load_linux(phys_ram_base + option_rom_offset,
                        kernel_filename, initrd_filename, kernel_cmdline);
-            cpu_register_physical_memory(0xd0000, TARGET_PAGE_SIZE,
+            cpu_register_physical_memory(option_rom_start, TARGET_PAGE_SIZE,
                                          option_rom_offset | IO_MEM_ROM);
-            offset = TARGET_PAGE_SIZE;
+            offset += TARGET_PAGE_SIZE;
         }
 
         for (i = 0; i < nb_option_roms; i++) {
@@ -909,18 +917,17 @@ vga_bios_error:
                         option_rom[i]);
                 exit(1);
             }
-            if (size > (0x10000 - offset))
+            if (size > (0xe0000  - offset))
                 goto option_rom_error;
             option_rom_offset = qemu_ram_alloc(size);
             ret = load_image(option_rom[i], phys_ram_base + option_rom_offset);
             if (ret != size) {
             option_rom_error:
-                fprintf(stderr, "Too many option ROMS\n");
+                fprintf(stderr, "Could not fit %soption roms in available space\n", using_vga ? "VGA bios and " : "");
                 exit(1);
             }
             size = (size + 4095) & ~4095;
-            cpu_register_physical_memory(0xd0000 + offset,
-                                         size, option_rom_offset | IO_MEM_ROM);
+            cpu_register_physical_memory(offset, size, option_rom_offset | IO_MEM_ROM);
             offset += size;
         }
     }
-- 
1.6.2

Re: [Qemu-devel] [PATCH] get roms more room.

[Carl-Daniel Hailfinger]

On 28.03.2009 00:48, Glauber Costa wrote:
> This patch increases by 50 % the size available for option roms.
> The main motivator is that some roms grew bigger than the 64k we
> currently allocate for them (Hey, it's 2009!)
>
> One example is the gpxe project, that produces some roms with 69k,
> 70k, etc. The space proposed by this patch actually makes it as
> big as 84k. Probably still a fit for some time.
>
> But there is no free lunch. This space must come from somewhere,
> and we take it from vga rom space. Currently, our vga roms are
> around 35k in size. With this patch, option rom space will begin
> just after vga ends, aligned to the next 2k boundary.
>
> Technicaly, we could do the same with the uper space (the bios itself),
> but since bochs bios is already 128 k in size, I don't see an
> urgent need to do it.
>
> Signed-off-by: Glauber Costa <glommer@redhat.com>
>   

I really like the patch, but I have one questions I can't answer because
I'm not familiar enough with the Qemu and Bochs BIOS codebase. You can
find it as inline comment.

Since the patch seems to do what I want, it is
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

If a Signed-off-by line is preferred, I can send that instead.

> ---
>  hw/pc.c |   27 +++++++++++++++++----------
>  1 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 69f25f3..07a9791 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -763,7 +763,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
>  {
>      char buf[1024];
>      int ret, linux_boot, i;
> -    ram_addr_t ram_addr, vga_ram_addr, bios_offset, vga_bios_offset;
> +    ram_addr_t ram_addr, vga_ram_addr, bios_offset, vga_bios_offset, option_rom_start;
>      ram_addr_t below_4g_mem_size, above_4g_mem_size = 0;
>      int bios_size, isa_bios_size, vga_bios_size;
>      PCIBus *pci_bus;
> @@ -774,6 +774,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
>      int index;
>      BlockDriverState *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>      BlockDriverState *fd[MAX_FD];
> +    int using_vga = cirrus_vga_enabled || std_vga_enabled || vmsvga_enabled;
>  
>      if (ram_size >= 0xe0000000 ) {
>          above_4g_mem_size = ram_size - 0xe0000000;
> @@ -856,7 +857,11 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
>          exit(1);
>      }
>  
> -    if (cirrus_vga_enabled || std_vga_enabled || vmsvga_enabled) {
> +    /* No point in placing option roms before this address, since bochs bios
> +     * will only start looking for it at 0xc8000 */
> +    option_rom_start = 0xc8000;
> +
> +    if (using_vga) {
>          /* VGA BIOS load */
>          if (cirrus_vga_enabled) {
>              snprintf(buf, sizeof(buf), "%s/%s", bios_dir, VGABIOS_CIRRUS_FILENAME);
> @@ -874,9 +879,12 @@ vga_bios_error:
>              fprintf(stderr, "qemu: could not load VGA BIOS '%s'\n", buf);
>              exit(1);
>          }
> +	/* Round up vga bios size to the next 2k boundary */
> +	vga_bios_size = (vga_bios_size + 2047) & ~2047;
>   

If vga_bios_size ever shrinks below 30k, the option ROMs will start
below 0xc8000 and the Bochs BIOS won't find them. Is that right?

> +	option_rom_start = 0xc0000 + vga_bios_size;
>  
>          /* setup basic memory access */
> -        cpu_register_physical_memory(0xc0000, 0x10000,
> +        cpu_register_physical_memory(0xc0000, vga_bios_size,
>                                       vga_bios_offset | IO_MEM_ROM);
>      }
>  
> @@ -892,14 +900,14 @@ vga_bios_error:
>          ram_addr_t option_rom_offset;
>          int size, offset;
>  
> -        offset = 0;
> +        offset = option_rom_start;
>          if (linux_boot) {
>              option_rom_offset = qemu_ram_alloc(TARGET_PAGE_SIZE);
>              load_linux(phys_ram_base + option_rom_offset,
>                         kernel_filename, initrd_filename, kernel_cmdline);
> -            cpu_register_physical_memory(0xd0000, TARGET_PAGE_SIZE,
> +            cpu_register_physical_memory(option_rom_start, TARGET_PAGE_SIZE,
>                                           option_rom_offset | IO_MEM_ROM);
> -            offset = TARGET_PAGE_SIZE;
> +            offset += TARGET_PAGE_SIZE;
>          }
>  
>          for (i = 0; i < nb_option_roms; i++) {
> @@ -909,18 +917,17 @@ vga_bios_error:
>                          option_rom[i]);
>                  exit(1);
>              }
> -            if (size > (0x10000 - offset))
> +            if (size > (0xe0000  - offset))
>                  goto option_rom_error;
>              option_rom_offset = qemu_ram_alloc(size);
>              ret = load_image(option_rom[i], phys_ram_base + option_rom_offset);
>              if (ret != size) {
>              option_rom_error:
> -                fprintf(stderr, "Too many option ROMS\n");
> +                fprintf(stderr, "Could not fit %soption roms in available space\n", using_vga ? "VGA bios and " : "");
>                  exit(1);
>              }
>              size = (size + 4095) & ~4095;
> -            cpu_register_physical_memory(0xd0000 + offset,
> -                                         size, option_rom_offset | IO_MEM_ROM);
> +            cpu_register_physical_memory(offset, size, option_rom_offset | IO_MEM_ROM);
>              offset += size;
>          }
>      }
>   

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/

Re: [Qemu-devel] [PATCH] get roms more room.

[Glauber Costa]

>
> I really like the patch, but I have one questions I can't answer because
> I'm not familiar enough with the Qemu and Bochs BIOS codebase. You can
> find it as inline comment.
>
> Since the patch seems to do what I want, it is
> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
>
> If a Signed-off-by line is preferred, I can send that instead.
>
>> ---
>>  hw/pc.c |   27 +++++++++++++++++----------
>>  1 files changed, 17 insertions(+), 10 deletions(-)
>>
>> +     /* Round up vga bios size to the next 2k boundary */
>> +     vga_bios_size = (vga_bios_size + 2047) & ~2047;
>>
>
> If vga_bios_size ever shrinks below 30k, the option ROMs will start
> below 0xc8000 and the Bochs BIOS won't find them. Is that right?
>

Hummm, humm.... /me thinks....

You're actually right. Although it won't happen if our current codebase, it's
a very valid concern. I will send an updated patch soon, hopefully 100 % ready
for inclusion.

Good catch!

-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

[Qemu-devel] [PATCH] get roms more room.

[Glauber Costa]

This patch increases by 50 % the size available for option roms.
The main motivator is that some roms grew bigger than the 64k we
currently allocate for them (Hey, it's 2009!)

One example is the gpxe project, that produces some roms with 69k,
70k, etc. The space proposed by this patch actually makes it as
big as 84k. Probably still a fit for some time.

But there is no free lunch. This space must come from somewhere,
and we take it from vga rom space. Currently, our vga roms are
around 35k in size. With this patch, they will be limited to
44k, instead of 64k, which seems reasonable to me.
---
 hw/pc.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 69f25f3..f66bfdd 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -876,7 +876,7 @@ vga_bios_error:
         }
 
         /* setup basic memory access */
-        cpu_register_physical_memory(0xc0000, 0x10000,
+        cpu_register_physical_memory(0xc0000, 0xb000,
                                      vga_bios_offset | IO_MEM_ROM);
     }
 
@@ -897,7 +897,7 @@ vga_bios_error:
             option_rom_offset = qemu_ram_alloc(TARGET_PAGE_SIZE);
             load_linux(phys_ram_base + option_rom_offset,
                        kernel_filename, initrd_filename, kernel_cmdline);
-            cpu_register_physical_memory(0xd0000, TARGET_PAGE_SIZE,
+            cpu_register_physical_memory(0xcb000, TARGET_PAGE_SIZE,
                                          option_rom_offset | IO_MEM_ROM);
             offset = TARGET_PAGE_SIZE;
         }
@@ -909,7 +909,7 @@ vga_bios_error:
                         option_rom[i]);
                 exit(1);
             }
-            if (size > (0x10000 - offset))
+            if (size > (0x15000 - offset))
                 goto option_rom_error;
             option_rom_offset = qemu_ram_alloc(size);
             ret = load_image(option_rom[i], phys_ram_base + option_rom_offset);
@@ -919,7 +919,7 @@ vga_bios_error:
                 exit(1);
             }
             size = (size + 4095) & ~4095;
-            cpu_register_physical_memory(0xd0000 + offset,
+            cpu_register_physical_memory(0xcb000 + offset,
                                          size, option_rom_offset | IO_MEM_ROM);
             offset += size;
         }
-- 
1.6.2

Re: [Qemu-devel] [PATCH] get roms more room.

[Carl-Daniel Hailfinger]

On 27.03.2009 18:37, Glauber Costa wrote:
> This patch increases by 50 % the size available for option roms.
> The main motivator is that some roms grew bigger than the 64k we
> currently allocate for them (Hey, it's 2009!)
>
> One example is the gpxe project, that produces some roms with 69k,
> 70k, etc. The space proposed by this patch actually makes it as
> big as 84k. Probably still a fit for some time.
>
> But there is no free lunch. This space must come from somewhere,
> and we take it from vga rom space. Currently, our vga roms are
> around 35k in size. With this patch, they will be limited to
> 44k, instead of 64k, which seems reasonable to me.
>   

As a firmware developer, I have to speak up here. Although I agree that
option ROMs may be bigger than 64k, replacing one hardcoded magic limit
with another hardcoded magic limit seems to be a bit questionable.

If you really want to hardcode this instead of calculating it
dynamically, please use at least some symbolic constants and tell the
user to change the symbolic constants and recompile Qemu if he hits the
size limit.

This patch reduces the room for the VGA option ROM without checking for
its size. If we perform option ROM size checks, we may as well not
exclude the VGA option ROM from that check.

AFAICS resuming an image created with the old memory layout will change
option ROM location and may lead to crashes, but I'm not so sure about that.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/

Re: [Qemu-devel] [PATCH] get roms more room.

[Glauber Costa]

On Fri, Mar 27, 2009 at 5:29 PM, Carl-Daniel Hailfinger
<c-d.hailfinger.devel.2006@gmx.net> wrote:
> On 27.03.2009 18:37, Glauber Costa wrote:
>> This patch increases by 50 % the size available for option roms.
>> The main motivator is that some roms grew bigger than the 64k we
>> currently allocate for them (Hey, it's 2009!)
>>
>> One example is the gpxe project, that produces some roms with 69k,
>> 70k, etc. The space proposed by this patch actually makes it as
>> big as 84k. Probably still a fit for some time.
>>
>> But there is no free lunch. This space must come from somewhere,
>> and we take it from vga rom space. Currently, our vga roms are
>> around 35k in size. With this patch, they will be limited to
>> 44k, instead of 64k, which seems reasonable to me.
>>
>
> As a firmware developer, I have to speak up here. Although I agree that
> option ROMs may be bigger than 64k, replacing one hardcoded magic limit
> with another hardcoded magic limit seems to be a bit questionable.
>
> If you really want to hardcode this instead of calculating it
> dynamically, please use at least some symbolic constants and tell the
> user to change the symbolic constants and recompile Qemu if he hits the
> size limit.
>
> This patch reduces the room for the VGA option ROM without checking for
> its size. If we perform option ROM size checks, we may as well not
> exclude the VGA option ROM from that check.

We load the VGA rom before loading any option roms. Would you be
okay with an approach that loads the vga rom, round up its size to the
next boundary, and assign the remaining space to option roms?
>
> AFAICS resuming an image created with the old memory layout will change
> option ROM location and may lead to crashes, but I'm not so sure about that.
>
> Regards,
> Carl-Daniel
>
> --
> http://www.hailfinger.org/
>
>
>
>



-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

Re: [Qemu-devel] [PATCH] get roms more room.

[Carl-Daniel Hailfinger]

On 27.03.2009 21:45, Glauber Costa wrote:
> On Fri, Mar 27, 2009 at 5:29 PM, Carl-Daniel Hailfinger
> <c-d.hailfinger.devel.2006@gmx.net> wrote:
>   
>> On 27.03.2009 18:37, Glauber Costa wrote:
>>     
>>> This patch increases by 50 % the size available for option roms.
>>> The main motivator is that some roms grew bigger than the 64k we
>>> currently allocate for them (Hey, it's 2009!)
>>>
>>> One example is the gpxe project, that produces some roms with 69k,
>>> 70k, etc. The space proposed by this patch actually makes it as
>>> big as 84k. Probably still a fit for some time.
>>>
>>> But there is no free lunch. This space must come from somewhere,
>>> and we take it from vga rom space. Currently, our vga roms are
>>> around 35k in size. With this patch, they will be limited to
>>> 44k, instead of 64k, which seems reasonable to me.
>>>
>>>       
>> As a firmware developer, I have to speak up here. Although I agree that
>> option ROMs may be bigger than 64k, replacing one hardcoded magic limit
>> with another hardcoded magic limit seems to be a bit questionable.
>>
>> If you really want to hardcode this instead of calculating it
>> dynamically, please use at least some symbolic constants and tell the
>> user to change the symbolic constants and recompile Qemu if he hits the
>> size limit.
>>
>> This patch reduces the room for the VGA option ROM without checking for
>> its size. If we perform option ROM size checks, we may as well not
>> exclude the VGA option ROM from that check.
>>     
>
> We load the VGA rom before loading any option roms. Would you be
> okay with an approach that loads the vga rom, round up its size to the
> next boundary, and assign the remaining space to option roms?
>   

Excellent idea.
Could you please change the error message to tell the user that
VGA+other option ROMs may not exceed a total size?
A check for VGA ROM size to be less than the total allowed size may be
useful to prevent underflows in calculations of the remaining space for
other option ROMs.


Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/

Re: [Qemu-devel] [PATCH] get roms more room.

[Glauber Costa]

On Fri, Mar 27, 2009 at 8:12 PM, Carl-Daniel Hailfinger
<c-d.hailfinger.devel.2006@gmx.net> wrote:
> On 27.03.2009 21:45, Glauber Costa wrote:
>> On Fri, Mar 27, 2009 at 5:29 PM, Carl-Daniel Hailfinger
>> <c-d.hailfinger.devel.2006@gmx.net> wrote:
>>
>>> On 27.03.2009 18:37, Glauber Costa wrote:
>>>
>>>> This patch increases by 50 % the size available for option roms.
>>>> The main motivator is that some roms grew bigger than the 64k we
>>>> currently allocate for them (Hey, it's 2009!)
>>>>
>>>> One example is the gpxe project, that produces some roms with 69k,
>>>> 70k, etc. The space proposed by this patch actually makes it as
>>>> big as 84k. Probably still a fit for some time.
>>>>
>>>> But there is no free lunch. This space must come from somewhere,
>>>> and we take it from vga rom space. Currently, our vga roms are
>>>> around 35k in size. With this patch, they will be limited to
>>>> 44k, instead of 64k, which seems reasonable to me.
>>>>
>>>>
>>> As a firmware developer, I have to speak up here. Although I agree that
>>> option ROMs may be bigger than 64k, replacing one hardcoded magic limit
>>> with another hardcoded magic limit seems to be a bit questionable.
>>>
>>> If you really want to hardcode this instead of calculating it
>>> dynamically, please use at least some symbolic constants and tell the
>>> user to change the symbolic constants and recompile Qemu if he hits the
>>> size limit.
>>>
>>> This patch reduces the room for the VGA option ROM without checking for
>>> its size. If we perform option ROM size checks, we may as well not
>>> exclude the VGA option ROM from that check.
>>>
>>
>> We load the VGA rom before loading any option roms. Would you be
>> okay with an approach that loads the vga rom, round up its size to the
>> next boundary, and assign the remaining space to option roms?
>>
>
> Excellent idea.
> Could you please change the error message to tell the user that
> VGA+other option ROMs may not exceed a total size?
> A check for VGA ROM size to be less than the total allowed size may be
> useful to prevent underflows in calculations of the remaining space for
> other option ROMs.

Such a check already exists.

-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."