Xen devel - Aug 2013 - [PATCH] hvmloader: fix SeaBIOS interface

The SeaBIOS ROM image may validly exceed 128k in size, it''s only our
interface code that so far assumed that it wouldn''t. Remove that
restriction by setting the base address depending on image size.

Add a check to HVM loader so that too big images won''t result in silent
guest failure anymore.

Uncomment the intended build-time size check for rombios, moving it
into a function so that it would actually compile.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/firmware/hvmloader/config-seabios.h
+++ b/tools/firmware/hvmloader/config-seabios.h
@@ -3,8 +3,6 @@
 
 #define BIOS_INFO_PHYSICAL_ADDRESS  0x00001000
 
-#define SEABIOS_PHYSICAL_ADDRESS    0x000E0000
-
 #endif /* __HVMLOADER_CONFIG_SEABIOS_H__ */
 
 /*
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -292,8 +292,12 @@ int main(void)
     if ( bios->bios_load )
         bios->bios_load(bios);
     else
+    {
+        BUG_ON(bios->bios_address + bios->image_size >
+               HVMLOADER_PHYSICAL_ADDRESS);
         memcpy((void *)bios->bios_address, bios->image,
                bios->image_size);
+    }
 
     if ( (hvm_info->nr_vcpus > 1) || hvm_info->apic_mode )
     {
--- a/tools/firmware/hvmloader/rombios.c
+++ b/tools/firmware/hvmloader/rombios.c
@@ -127,6 +127,8 @@ static void rombios_load(const struct bi
     uint32_t bioshigh;
     struct rombios_info *info;
 
+    BUILD_BUG_ON(sizeof(rombios) > 0x100000 - ROMBIOS_PHYSICAL_ADDRESS);
+
     memcpy((void *)config->bios_address, config->image,
            config->image_size);
 
@@ -206,8 +208,6 @@ static void rombios_create_smbios_tables
         SMBIOS_PHYSICAL_END);
 }
 
-//BUILD_BUG_ON(sizeof(rombios) > (0x00100000U - ROMBIOS_PHYSICAL_ADDRESS));
-
 struct bios_config rombios_config =  {
     .name = "ROMBIOS",
 
--- a/tools/firmware/hvmloader/seabios.c
+++ b/tools/firmware/hvmloader/seabios.c
@@ -133,15 +133,13 @@ static void seabios_setup_e820(void)
     dump_e820_table(e820, info->e820_nr);
 }
 
-//BUILD_BUG_ON(sizeof(seabios) > (0x00100000U - SEABIOS_PHYSICAL_ADDRESS));
-
 struct bios_config seabios_config = {
     .name = "SeaBIOS",
 
     .image = seabios,
     .image_size = sizeof(seabios),
 
-    .bios_address = SEABIOS_PHYSICAL_ADDRESS,
+    .bios_address = 0x100000 - sizeof(seabios),
 
     .load_roms = NULL,
 





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

On Fri, 2013-08-23 at 13:42 +0100, Jan Beulich wrote:
> The SeaBIOS ROM image may validly exceed 128k in size, it''s only
our
> interface code that so far assumed that it wouldn''t. Remove that
> restriction by setting the base address depending on image size.
> 
> Add a check to HVM loader so that too big images won''t result in
silent
> guest failure anymore.
> 
> Uncomment the intended build-time size check for rombios, moving it
> into a function so that it would actually compile.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
 
> -    .bios_address = SEABIOS_PHYSICAL_ADDRESS,
> +    .bios_address = 0x100000 - sizeof(seabios),
Do we rely on the seabios build to pad this, or are arbitrary sizes
aloud? (Doesn''t invalidate my Ack, just curious)

Ian.

>>> On 23.08.13 at 14:46, Ian Campbell <Ian.Campbell@citrix.com>
wrote:
> On Fri, 2013-08-23 at 13:42 +0100, Jan Beulich wrote:
>> The SeaBIOS ROM image may validly exceed 128k in size, it''s
only our
>> interface code that so far assumed that it wouldn''t. Remove
that
>> restriction by setting the base address depending on image size.
>> 
>> Add a check to HVM loader so that too big images won''t result
in silent
>> guest failure anymore.
>> 
>> Uncomment the intended build-time size check for rombios, moving it
>> into a function so that it would actually compile.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>  
>> -    .bios_address = SEABIOS_PHYSICAL_ADDRESS,
>> +    .bios_address = 0x100000 - sizeof(seabios),
> 
> Do we rely on the seabios build to pad this, or are arbitrary sizes
> aloud? (Doesn''t invalidate my Ack, just curious)
Since the padding happens at the front, we don''t really care.

Jan