[Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present

[Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present

[Kevin O'Connor]

Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms
directory.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
---
Changes v2->v3:
   Fix coding style (missing braces).
Changes v1->v2:
   Add signed-off-by line.
---
 hw/fw_cfg.c |    9 +++++++--
 hw/pc.c     |    8 ++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index 72866ae..5a2c0aa 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -304,8 +304,13 @@ int fw_cfg_add_file(FWCfgState *s,  const char *dir, const char *filename,
         basename = filename;
     }
 
-    snprintf(s->files->f[index].name, sizeof(s->files->f[index].name),
-             "%s/%s", dir, basename);
+    if (dir && dir[0]) {
+        snprintf(s->files->f[index].name, sizeof(s->files->f[index].name),
+                 "%s/%s", dir, basename);
+    } else {
+        strncpy(s->files->f[index].name, basename,
+                sizeof(s->files->f[index].name));
+    }
     for (i = 0; i < index; i++) {
         if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) {
             FW_CFG_DPRINTF("%s: skip duplicate: %s\n", __FUNCTION__,
diff --git a/hw/pc.c b/hw/pc.c
index 58dea57..6893799 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -54,6 +54,7 @@
 #endif
 
 #define BIOS_FILENAME "bios.bin"
+#define BOOTSPLASH_FILENAME "bootsplash.jpg"
 
 #define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
 
@@ -963,6 +964,13 @@ void pc_memory_init(ram_addr_t ram_size,
     fw_cfg = bochs_bios_init();
     rom_set_fw(fw_cfg);
 
+    /* Optional bootsplash file */
+    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BOOTSPLASH_FILENAME);
+    if (filename) {
+        qemu_free(filename);
+        rom_add_file(BOOTSPLASH_FILENAME, "", 0);
+    }
+
     if (linux_boot) {
         load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, below_4g_mem_size);
     }
-- 
1.7.2

Re: [Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present

[Anthony Liguori]

On 08/02/2010 11:11 AM, Kevin O'Connor wrote:
> Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms
> directory.
>    

Sorry, I should have provided this in the first response.

Does the bootsplash cause a delay in startup time?  If so, we'll want to 
be able to disable this at boot time via the -boot option.  If it 
doesn't, this patch is probably fine as is.

Regards,

Anthony Liguori

> Signed-off-by: Kevin O'Connor<kevin@koconnor.net>
> ---
> Changes v2->v3:
>     Fix coding style (missing braces).
> Changes v1->v2:
>     Add signed-off-by line.
> ---
>   hw/fw_cfg.c |    9 +++++++--
>   hw/pc.c     |    8 ++++++++
>   2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
> index 72866ae..5a2c0aa 100644
> --- a/hw/fw_cfg.c
> +++ b/hw/fw_cfg.c
> @@ -304,8 +304,13 @@ int fw_cfg_add_file(FWCfgState *s,  const char *dir, const char *filename,
>           basename = filename;
>       }
>
> -    snprintf(s->files->f[index].name, sizeof(s->files->f[index].name),
> -             "%s/%s", dir, basename);
> +    if (dir&&  dir[0]) {
> +        snprintf(s->files->f[index].name, sizeof(s->files->f[index].name),
> +                 "%s/%s", dir, basename);
> +    } else {
> +        strncpy(s->files->f[index].name, basename,
> +                sizeof(s->files->f[index].name));
> +    }
>       for (i = 0; i<  index; i++) {
>           if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) {
>               FW_CFG_DPRINTF("%s: skip duplicate: %s\n", __FUNCTION__,
> diff --git a/hw/pc.c b/hw/pc.c
> index 58dea57..6893799 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -54,6 +54,7 @@
>   #endif
>
>   #define BIOS_FILENAME "bios.bin"
> +#define BOOTSPLASH_FILENAME "bootsplash.jpg"
>
>   #define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
>
> @@ -963,6 +964,13 @@ void pc_memory_init(ram_addr_t ram_size,
>       fw_cfg = bochs_bios_init();
>       rom_set_fw(fw_cfg);
>
> +    /* Optional bootsplash file */
> +    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BOOTSPLASH_FILENAME);
> +    if (filename) {
> +        qemu_free(filename);
> +        rom_add_file(BOOTSPLASH_FILENAME, "", 0);
> +    }
> +
>       if (linux_boot) {
>           load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, below_4g_mem_size);
>       }
>    

Re: [Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present

[Kevin O'Connor]

On Mon, Aug 02, 2010 at 11:33:33AM -0500, Anthony Liguori wrote:
> On 08/02/2010 11:11 AM, Kevin O'Connor wrote:
> >Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms
> >directory.
> 
> Sorry, I should have provided this in the first response.
> 
> Does the bootsplash cause a delay in startup time?  If so, we'll
> want to be able to disable this at boot time via the -boot option.
> If it doesn't, this patch is probably fine as is.

That's okay - the patch is intended to start discussion anyway.

There is no added delay for the bootsplash.  However, it does take
time to decompress it.  On my machine it can take between 200-300ms
for the jpeg code.

It may make more sense for SeaBIOS to only bother with the bootsplash
if the boot menu is enabled (or some new option) - as otherwise the
picture is basically cleared immiediately after it's shown.

-Kevin

Re: [Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present

[Anthony Liguori]

On 08/02/2010 11:47 AM, Kevin O'Connor wrote:
> On Mon, Aug 02, 2010 at 11:33:33AM -0500, Anthony Liguori wrote:
>    
>> On 08/02/2010 11:11 AM, Kevin O'Connor wrote:
>>      
>>> Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms
>>> directory.
>>>        
>> Sorry, I should have provided this in the first response.
>>
>> Does the bootsplash cause a delay in startup time?  If so, we'll
>> want to be able to disable this at boot time via the -boot option.
>> If it doesn't, this patch is probably fine as is.
>>      
> That's okay - the patch is intended to start discussion anyway.
>
> There is no added delay for the bootsplash.  However, it does take
> time to decompress it.  On my machine it can take between 200-300ms
> for the jpeg code.
>
> It may make more sense for SeaBIOS to only bother with the bootsplash
> if the boot menu is enabled (or some new option) - as otherwise the
> picture is basically cleared immiediately after it's shown.
>    

Yeah, if there's no boot menu and no boot menu delay, I imagine the boot 
splash ends up just being a blip on the screen, right?

We definitely want a "boot as fast as you possibly can" mode for the 
BIOS although for non-developer environments, I think a boot splash is a 
nice touch.

BTW, we need to document somewhere any assumptions SeaBIOS has about the 
JPEG.  I see that it expects a 1024x768 image.  Any additional 
restrictions on the jpeg image?

Regards,

Anthony Liguori

> -Kevin
>    

Re: [Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present

[Kevin O'Connor]

On Mon, Aug 02, 2010 at 11:52:22AM -0500, Anthony Liguori wrote:
> BTW, we need to document somewhere any assumptions SeaBIOS has about
> the JPEG.  I see that it expects a 1024x768 image.  Any additional
> restrictions on the jpeg image?

I listed some notes in a previous email:

>> Some notes:
>> 
>> This uses the qemu "rom" interface for loading the jpeg file.  It
>> seems to work, but I'm not sure if this is strictly correct.
>> 
>> The jpeg viewer in SeaBIOS will look at the image size and try to find
>> a vesa graphics mode that supports that size.  So, choose images that
>> are exactly 640x480, 1024x768, etc.  Also, the SeaBIOS viewer has
>> stripped down support for jpegs - not all valid jpegs will work.  Some
>> quick tests with the netpbm tools worked okay for me.
>> 
>> SeaBIOS only shows the bootsplash during the interval between vgabios
>> init and OS execution.  This is generally too short a time to be seen.
>> Add "-menu boot=on" to the qemu command line to have it shown longer.
>> 
>> Unfortunately, the vgabios doesn't support writing text to the screen
>> while in a vesa video mode.  So, this means that if a user selects F12
>> for the boot menu, they can't actually see the boot menu.  This will
>> need to be fixed in SeaBIOS in a follow up patch.

-Kevin

Re: [Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present

[Anthony Liguori]

On 08/02/2010 01:10 PM, Kevin O'Connor wrote:
> On Mon, Aug 02, 2010 at 11:52:22AM -0500, Anthony Liguori wrote:
>    
>> BTW, we need to document somewhere any assumptions SeaBIOS has about
>> the JPEG.  I see that it expects a 1024x768 image.  Any additional
>> restrictions on the jpeg image?
>>      
> I listed some notes in a previous email:
>    

Right, we need this in either docs/seabios.txt in qemu.git or in a file 
in seabios.git I think.

Regards,

Anthony Liguori

>>> Some notes:
>>>
>>> This uses the qemu "rom" interface for loading the jpeg file.  It
>>> seems to work, but I'm not sure if this is strictly correct.
>>>
>>> The jpeg viewer in SeaBIOS will look at the image size and try to find
>>> a vesa graphics mode that supports that size.  So, choose images that
>>> are exactly 640x480, 1024x768, etc.  Also, the SeaBIOS viewer has
>>> stripped down support for jpegs - not all valid jpegs will work.  Some
>>> quick tests with the netpbm tools worked okay for me.
>>>
>>> SeaBIOS only shows the bootsplash during the interval between vgabios
>>> init and OS execution.  This is generally too short a time to be seen.
>>> Add "-menu boot=on" to the qemu command line to have it shown longer.
>>>
>>> Unfortunately, the vgabios doesn't support writing text to the screen
>>> while in a vesa video mode.  So, this means that if a user selects F12
>>> for the boot menu, they can't actually see the boot menu.  This will
>>> need to be fixed in SeaBIOS in a follow up patch.
>>>        
> -Kevin
>    

Re: [Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present

[Richard W.M. Jones]

On Mon, Aug 02, 2010 at 12:47:53PM -0400, Kevin O'Connor wrote:
> On Mon, Aug 02, 2010 at 11:33:33AM -0500, Anthony Liguori wrote:
> > On 08/02/2010 11:11 AM, Kevin O'Connor wrote:
> > >Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms
> > >directory.
> > 
> > Sorry, I should have provided this in the first response.
> > 
> > Does the bootsplash cause a delay in startup time?  If so, we'll
> > want to be able to disable this at boot time via the -boot option.
> > If it doesn't, this patch is probably fine as is.
> 
> That's okay - the patch is intended to start discussion anyway.
> 
> There is no added delay for the bootsplash.  However, it does take
> time to decompress it.  On my machine it can take between 200-300ms
> for the jpeg code.

Does it detect a "headless" boot and disable itself completely?

300ms is 5% of libguestfs boot time ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw

Re: [Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present

[Anthony Liguori]

On 08/03/2010 04:52 AM, Richard W.M. Jones wrote:
> On Mon, Aug 02, 2010 at 12:47:53PM -0400, Kevin O'Connor wrote:
>    
>> On Mon, Aug 02, 2010 at 11:33:33AM -0500, Anthony Liguori wrote:
>>      
>>> On 08/02/2010 11:11 AM, Kevin O'Connor wrote:
>>>        
>>>> Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms
>>>> directory.
>>>>          
>>> Sorry, I should have provided this in the first response.
>>>
>>> Does the bootsplash cause a delay in startup time?  If so, we'll
>>> want to be able to disable this at boot time via the -boot option.
>>> If it doesn't, this patch is probably fine as is.
>>>        
>> That's okay - the patch is intended to start discussion anyway.
>>
>> There is no added delay for the bootsplash.  However, it does take
>> time to decompress it.  On my machine it can take between 200-300ms
>> for the jpeg code.
>>      
> Does it detect a "headless" boot and disable itself completely?
>
> 300ms is 5% of libguestfs boot time ...
>    

If menu=off, it probably just shouldn't display.  I assume libguestfs is 
passing menu=off...

Regards,

Anthony Liguori

> Rich.
>
>    

Re: [Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present

[Richard W.M. Jones]

On Tue, Aug 03, 2010 at 07:51:02AM -0500, Anthony Liguori wrote:
> If menu=off, it probably just shouldn't display.  I assume
> libguestfs is passing menu=off...

We don't actually, but it is a tremendously good idea so I'll add it,
thanks :-)

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

Re: [Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present

[Richard W.M. Jones]

On Tue, Aug 03, 2010 at 01:54:12PM +0100, Richard W.M. Jones wrote:
> On Tue, Aug 03, 2010 at 07:51:02AM -0500, Anthony Liguori wrote:
> > If menu=off, it probably just shouldn't display.  I assume
> > libguestfs is passing menu=off...
> 
> We don't actually, but it is a tremendously good idea so I'll add it,
> thanks :-)

Or maybe I won't.  The default is boot_menu = 0 (off), confirmed
by examining the code.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw

Re: [Qemu-devel] [PATCHv3] Load "bootsplash.jpg" if present

[Anthony Liguori]

On 08/03/2010 08:04 AM, Richard W.M. Jones wrote:
> On Tue, Aug 03, 2010 at 01:54:12PM +0100, Richard W.M. Jones wrote:
>    
>> On Tue, Aug 03, 2010 at 07:51:02AM -0500, Anthony Liguori wrote:
>>      
>>> If menu=off, it probably just shouldn't display.  I assume
>>> libguestfs is passing menu=off...
>>>        
>> We don't actually, but it is a tremendously good idea so I'll add it,
>> thanks :-)
>>      
> Or maybe I won't.  The default is boot_menu = 0 (off), confirmed
> by examining the code.
>    

You shouldn't rely on defaults though if you want to be future proof.  
The defaults are "whatever we think is the best for the user" and as 
such, may change from version to version.

Regards,

Anthony Liguori

> Rich.
>
>    

[Qemu-devel] Re: [PATCHv3] Load "bootsplash.jpg" if present

[Paolo Bonzini]

On 08/02/2010 06:47 PM, Kevin O'Connor wrote:
> On Mon, Aug 02, 2010 at 11:33:33AM -0500, Anthony Liguori wrote:
>> On 08/02/2010 11:11 AM, Kevin O'Connor wrote:
>>> Load the "bootsplash.jpg" file into fw_cfg if it is found in the roms
>>> directory.
>>
>> Sorry, I should have provided this in the first response.
>>
>> Does the bootsplash cause a delay in startup time?  If so, we'll
>> want to be able to disable this at boot time via the -boot option.
>> If it doesn't, this patch is probably fine as is.
>
> That's okay - the patch is intended to start discussion anyway.
>
> There is no added delay for the bootsplash.  However, it does take
> time to decompress it.  On my machine it can take between 200-300ms
> for the jpeg code.

KVM or TCG?

Also, what's the time for a completely black bootsplash?

Paolo

[Qemu-devel] Re: [PATCHv3] Load "bootsplash.jpg" if present

[Kevin O'Connor]

On Tue, Aug 03, 2010 at 04:38:48PM +0200, Paolo Bonzini wrote:
> On 08/02/2010 06:47 PM, Kevin O'Connor wrote:
> >There is no added delay for the bootsplash.  However, it does take
> >time to decompress it.  On my machine it can take between 200-300ms
> >for the jpeg code.
> 
> KVM or TCG?

TCG.

> Also, what's the time for a completely black bootsplash?

Here are the timings (times are in seconds measured on the host) for a
completely black 640x480 jpeg on my machine:

00.135: Checking for bootsplash
00.139: VESA 2.0
00.140: VENDOR: VGABIOS Cirrus extension
00.143: PRODUCT: VGABIOS Cirrus extension
00.145: Copying bootsplash.jpg
00.146: Decoding bootsplash.jpg
00.154: Finding vesa mode with dimensions 640/480
00.160: mode: 0111
00.162: framebuffer: 0xf0000000
00.164: bytes per scanline: 1280
00.165: bits per pixel: 16
00.166: Decompressing bootsplash.jpg
00.283: Switching to graphics mode
00.322: Showing bootsplash.jpg
00.327: Bootsplash copy complete

One can compare this output with the dprintf() calls in the code at:

http://git.linuxtogo.org/?p=kevin/seabios.git;a=blob;f=src/bootsplash.c;h=14bdd4cfaa6b931032e30236039b0fd4934f75ac;hb=eb6dc785475e4676de728f99a0fcd638d81c5b68#l144

The times weren't notably different with a real picture instead of
just a black picture.

As before, SeaBIOS should probably be changed so that it only shows
the image if the boot menu has been enabled.

-Kevin