diff mbox

[v2] pc_sysfw: Always use alias for ISA BIOS region

Message ID 5094172D.8010303@siemens.com
State New
Headers show

Commit Message

Jan Kiszka Nov. 2, 2012, 6:55 p.m. UTC 
This is no technical reason (anymore) for copying the ISA BIOS from the
original region. Instead, refactor pc_isa_bios_init to serve both pflash
and old-style BIOS setup.

Unfortunately, the previous RAM-backed version created an additional
vmstate section, content-wise redundant to the BIOS, but we still need
to process it when working in compat mode.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Changes in v2:
 - create dummy vmstate section to enable migration from 1.1/1.2

 hw/pc_piix.c  |    4 ++++
 hw/pc_sysfw.c |   55 +++++++++++++++++++++----------------------------------
 2 files changed, 25 insertions(+), 34 deletions(-)

Comments

Jordan Justen Nov. 2, 2012, 9:17 p.m. UTC | #1 
I tested that flash device still works and the alias works with the
flash device.

I *did not* test vm state save/restore/migration.

Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

On Fri, Nov 2, 2012 at 11:55 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> This is no technical reason (anymore) for copying the ISA BIOS from the
> original region. Instead, refactor pc_isa_bios_init to serve both pflash
> and old-style BIOS setup.
>
> Unfortunately, the previous RAM-backed version created an additional
> vmstate section, content-wise redundant to the BIOS, but we still need
> to process it when working in compat mode.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>
> Changes in v2:
>  - create dummy vmstate section to enable migration from 1.1/1.2
>
>  hw/pc_piix.c  |    4 ++++
>  hw/pc_sysfw.c |   55 +++++++++++++++++++++----------------------------------
>  2 files changed, 25 insertions(+), 34 deletions(-)
>
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index cfa839c..0051b2a 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -386,6 +386,10 @@ static QEMUMachine pc_machine_v1_3 = {
>              .driver   = "VGA",\
>              .property = "mmio",\
>              .value    = "off",\
> +        },{\
> +            .driver   = "pc-sysfw",\
> +            .property = "compat_vmsection",\
> +            .value    = "on",\
>          }
>
>  static QEMUMachine pc_machine_v1_2 = {
> diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
> index 9d7c5f4..a60f453 100644
> --- a/hw/pc_sysfw.c
> +++ b/hw/pc_sysfw.c
> @@ -38,40 +38,36 @@
>  typedef struct PcSysFwDevice {
>      SysBusDevice busdev;
>      uint8_t rom_only;
> +    uint32_t compat_vmsection;
>  } PcSysFwDevice;
>
>  static void pc_isa_bios_init(MemoryRegion *rom_memory,
> -                             MemoryRegion *flash_mem,
> -                             int ram_size)
> +                             MemoryRegion *bios, bool compat_vmsection)
>  {
> +    uint64_t bios_size = memory_region_size(bios);
>      int isa_bios_size;
>      MemoryRegion *isa_bios;
> -    uint64_t flash_size;
> -    void *flash_ptr, *isa_bios_ptr;
> -
> -    flash_size = memory_region_size(flash_mem);
>
>      /* map the last 128KB of the BIOS in ISA space */
> -    isa_bios_size = flash_size;
> +    isa_bios_size = bios_size;
>      if (isa_bios_size > (128 * 1024)) {
>          isa_bios_size = 128 * 1024;
>      }
>      isa_bios = g_malloc(sizeof(*isa_bios));
> -    memory_region_init_ram(isa_bios, "isa-bios", isa_bios_size);
> -    vmstate_register_ram_global(isa_bios);
> +    memory_region_init_alias(isa_bios, "isa-bios", bios,
> +                             bios_size - isa_bios_size, isa_bios_size);
>      memory_region_add_subregion_overlap(rom_memory,
>                                          0x100000 - isa_bios_size,
>                                          isa_bios,
>                                          1);
> +    memory_region_set_readonly(isa_bios, true);
>
> -    /* copy ISA rom image from top of flash memory */
> -    flash_ptr = memory_region_get_ram_ptr(flash_mem);
> -    isa_bios_ptr = memory_region_get_ram_ptr(isa_bios);
> -    memcpy(isa_bios_ptr,
> -           ((uint8_t*)flash_ptr) + (flash_size - isa_bios_size),
> -           isa_bios_size);
> +    if (compat_vmsection) {
> +        MemoryRegion *dummy_region = g_new(MemoryRegion, 1);
>
> -    memory_region_set_readonly(isa_bios, true);
> +        memory_region_init_ram(dummy_region, "isa-bios", isa_bios_size);
> +        vmstate_register_ram_global(dummy_region);
> +    }
>  }
>
>  static void pc_fw_add_pflash_drv(void)
> @@ -102,7 +98,7 @@ static void pc_fw_add_pflash_drv(void)
>  }
>
>  static void pc_system_flash_init(MemoryRegion *rom_memory,
> -                                 DriveInfo *pflash_drv)
> +                                 DriveInfo *pflash_drv, bool compat_vmsection)
>  {
>      BlockDriverState *bdrv;
>      int64_t size;
> @@ -129,14 +125,14 @@ static void pc_system_flash_init(MemoryRegion *rom_memory,
>                                           1, 0x0000, 0x0000, 0x0000, 0x0000, 0);
>      flash_mem = pflash_cfi01_get_memory(system_flash);
>
> -    pc_isa_bios_init(rom_memory, flash_mem, size);
> +    pc_isa_bios_init(rom_memory, flash_mem, compat_vmsection);
>  }
>
>  static void old_pc_system_rom_init(MemoryRegion *rom_memory)
>  {
>      char *filename;
> -    MemoryRegion *bios, *isa_bios;
> -    int bios_size, isa_bios_size;
> +    MemoryRegion *bios;
> +    int bios_size;
>      int ret;
>
>      /* BIOS load */
> @@ -167,19 +163,7 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory)
>          g_free(filename);
>      }
>
> -    /* map the last 128KB of the BIOS in ISA space */
> -    isa_bios_size = bios_size;
> -    if (isa_bios_size > (128 * 1024)) {
> -        isa_bios_size = 128 * 1024;
> -    }
> -    isa_bios = g_malloc(sizeof(*isa_bios));
> -    memory_region_init_alias(isa_bios, "isa-bios", bios,
> -                             bios_size - isa_bios_size, isa_bios_size);
> -    memory_region_add_subregion_overlap(rom_memory,
> -                                        0x100000 - isa_bios_size,
> -                                        isa_bios,
> -                                        1);
> -    memory_region_set_readonly(isa_bios, true);
> +    pc_isa_bios_init(rom_memory, bios, false);
>
>      /* map all the bios at the top of memory */
>      memory_region_add_subregion(rom_memory,
> @@ -224,7 +208,8 @@ void pc_system_firmware_init(MemoryRegion *rom_memory)
>      }
>
>      if (pflash_drv != NULL) {
> -        pc_system_flash_init(rom_memory, pflash_drv);
> +        pc_system_flash_init(rom_memory, pflash_drv,
> +                             sysfw_dev->compat_vmsection);
>      } else {
>          fprintf(stderr, "qemu: PC system firmware (pflash) not available\n");
>          exit(1);
> @@ -233,6 +218,8 @@ void pc_system_firmware_init(MemoryRegion *rom_memory)
>
>  static Property pcsysfw_properties[] = {
>      DEFINE_PROP_UINT8("rom_only", PcSysFwDevice, rom_only, 0),
> +    DEFINE_PROP_BIT("compat_vmsection", PcSysFwDevice, compat_vmsection, 0,
> +                    false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> --
> 1.7.3.4
Jan Kiszka Nov. 3, 2012, 7:48 a.m. UTC | #2 
On 2012-11-02 22:17, Jordan Justen wrote:
> I tested that flash device still works and the alias works with the
> flash device.

Thanks!

> 
> I *did not* test vm state save/restore/migration.

I tested this, and it worked for me when migrating to pc-1.1/pc-1.2.

Jan

> 
> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
> 
> On Fri, Nov 2, 2012 at 11:55 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> This is no technical reason (anymore) for copying the ISA BIOS from the
>> original region. Instead, refactor pc_isa_bios_init to serve both pflash
>> and old-style BIOS setup.
>>
>> Unfortunately, the previous RAM-backed version created an additional
>> vmstate section, content-wise redundant to the BIOS, but we still need
>> to process it when working in compat mode.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> Changes in v2:
>>  - create dummy vmstate section to enable migration from 1.1/1.2
>>
>>  hw/pc_piix.c  |    4 ++++
>>  hw/pc_sysfw.c |   55 +++++++++++++++++++++----------------------------------
>>  2 files changed, 25 insertions(+), 34 deletions(-)
>>
>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
>> index cfa839c..0051b2a 100644
>> --- a/hw/pc_piix.c
>> +++ b/hw/pc_piix.c
>> @@ -386,6 +386,10 @@ static QEMUMachine pc_machine_v1_3 = {
>>              .driver   = "VGA",\
>>              .property = "mmio",\
>>              .value    = "off",\
>> +        },{\
>> +            .driver   = "pc-sysfw",\
>> +            .property = "compat_vmsection",\
>> +            .value    = "on",\
>>          }
>>
>>  static QEMUMachine pc_machine_v1_2 = {
>> diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
>> index 9d7c5f4..a60f453 100644
>> --- a/hw/pc_sysfw.c
>> +++ b/hw/pc_sysfw.c
>> @@ -38,40 +38,36 @@
>>  typedef struct PcSysFwDevice {
>>      SysBusDevice busdev;
>>      uint8_t rom_only;
>> +    uint32_t compat_vmsection;
>>  } PcSysFwDevice;
>>
>>  static void pc_isa_bios_init(MemoryRegion *rom_memory,
>> -                             MemoryRegion *flash_mem,
>> -                             int ram_size)
>> +                             MemoryRegion *bios, bool compat_vmsection)
>>  {
>> +    uint64_t bios_size = memory_region_size(bios);
>>      int isa_bios_size;
>>      MemoryRegion *isa_bios;
>> -    uint64_t flash_size;
>> -    void *flash_ptr, *isa_bios_ptr;
>> -
>> -    flash_size = memory_region_size(flash_mem);
>>
>>      /* map the last 128KB of the BIOS in ISA space */
>> -    isa_bios_size = flash_size;
>> +    isa_bios_size = bios_size;
>>      if (isa_bios_size > (128 * 1024)) {
>>          isa_bios_size = 128 * 1024;
>>      }
>>      isa_bios = g_malloc(sizeof(*isa_bios));
>> -    memory_region_init_ram(isa_bios, "isa-bios", isa_bios_size);
>> -    vmstate_register_ram_global(isa_bios);
>> +    memory_region_init_alias(isa_bios, "isa-bios", bios,
>> +                             bios_size - isa_bios_size, isa_bios_size);
>>      memory_region_add_subregion_overlap(rom_memory,
>>                                          0x100000 - isa_bios_size,
>>                                          isa_bios,
>>                                          1);
>> +    memory_region_set_readonly(isa_bios, true);
>>
>> -    /* copy ISA rom image from top of flash memory */
>> -    flash_ptr = memory_region_get_ram_ptr(flash_mem);
>> -    isa_bios_ptr = memory_region_get_ram_ptr(isa_bios);
>> -    memcpy(isa_bios_ptr,
>> -           ((uint8_t*)flash_ptr) + (flash_size - isa_bios_size),
>> -           isa_bios_size);
>> +    if (compat_vmsection) {
>> +        MemoryRegion *dummy_region = g_new(MemoryRegion, 1);
>>
>> -    memory_region_set_readonly(isa_bios, true);
>> +        memory_region_init_ram(dummy_region, "isa-bios", isa_bios_size);
>> +        vmstate_register_ram_global(dummy_region);
>> +    }
>>  }
>>
>>  static void pc_fw_add_pflash_drv(void)
>> @@ -102,7 +98,7 @@ static void pc_fw_add_pflash_drv(void)
>>  }
>>
>>  static void pc_system_flash_init(MemoryRegion *rom_memory,
>> -                                 DriveInfo *pflash_drv)
>> +                                 DriveInfo *pflash_drv, bool compat_vmsection)
>>  {
>>      BlockDriverState *bdrv;
>>      int64_t size;
>> @@ -129,14 +125,14 @@ static void pc_system_flash_init(MemoryRegion *rom_memory,
>>                                           1, 0x0000, 0x0000, 0x0000, 0x0000, 0);
>>      flash_mem = pflash_cfi01_get_memory(system_flash);
>>
>> -    pc_isa_bios_init(rom_memory, flash_mem, size);
>> +    pc_isa_bios_init(rom_memory, flash_mem, compat_vmsection);
>>  }
>>
>>  static void old_pc_system_rom_init(MemoryRegion *rom_memory)
>>  {
>>      char *filename;
>> -    MemoryRegion *bios, *isa_bios;
>> -    int bios_size, isa_bios_size;
>> +    MemoryRegion *bios;
>> +    int bios_size;
>>      int ret;
>>
>>      /* BIOS load */
>> @@ -167,19 +163,7 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory)
>>          g_free(filename);
>>      }
>>
>> -    /* map the last 128KB of the BIOS in ISA space */
>> -    isa_bios_size = bios_size;
>> -    if (isa_bios_size > (128 * 1024)) {
>> -        isa_bios_size = 128 * 1024;
>> -    }
>> -    isa_bios = g_malloc(sizeof(*isa_bios));
>> -    memory_region_init_alias(isa_bios, "isa-bios", bios,
>> -                             bios_size - isa_bios_size, isa_bios_size);
>> -    memory_region_add_subregion_overlap(rom_memory,
>> -                                        0x100000 - isa_bios_size,
>> -                                        isa_bios,
>> -                                        1);
>> -    memory_region_set_readonly(isa_bios, true);
>> +    pc_isa_bios_init(rom_memory, bios, false);
>>
>>      /* map all the bios at the top of memory */
>>      memory_region_add_subregion(rom_memory,
>> @@ -224,7 +208,8 @@ void pc_system_firmware_init(MemoryRegion *rom_memory)
>>      }
>>
>>      if (pflash_drv != NULL) {
>> -        pc_system_flash_init(rom_memory, pflash_drv);
>> +        pc_system_flash_init(rom_memory, pflash_drv,
>> +                             sysfw_dev->compat_vmsection);
>>      } else {
>>          fprintf(stderr, "qemu: PC system firmware (pflash) not available\n");
>>          exit(1);
>> @@ -233,6 +218,8 @@ void pc_system_firmware_init(MemoryRegion *rom_memory)
>>
>>  static Property pcsysfw_properties[] = {
>>      DEFINE_PROP_UINT8("rom_only", PcSysFwDevice, rom_only, 0),
>> +    DEFINE_PROP_BIT("compat_vmsection", PcSysFwDevice, compat_vmsection, 0,
>> +                    false),
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>
>> --
>> 1.7.3.4
> 
>
diff mbox

Patch

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index cfa839c..0051b2a 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -386,6 +386,10 @@  static QEMUMachine pc_machine_v1_3 = {
             .driver   = "VGA",\
             .property = "mmio",\
             .value    = "off",\
+        },{\
+            .driver   = "pc-sysfw",\
+            .property = "compat_vmsection",\
+            .value    = "on",\
         }
 
 static QEMUMachine pc_machine_v1_2 = {
diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
index 9d7c5f4..a60f453 100644
--- a/hw/pc_sysfw.c
+++ b/hw/pc_sysfw.c
@@ -38,40 +38,36 @@ 
 typedef struct PcSysFwDevice {
     SysBusDevice busdev;
     uint8_t rom_only;
+    uint32_t compat_vmsection;
 } PcSysFwDevice;
 
 static void pc_isa_bios_init(MemoryRegion *rom_memory,
-                             MemoryRegion *flash_mem,
-                             int ram_size)
+                             MemoryRegion *bios, bool compat_vmsection)
 {
+    uint64_t bios_size = memory_region_size(bios);
     int isa_bios_size;
     MemoryRegion *isa_bios;
-    uint64_t flash_size;
-    void *flash_ptr, *isa_bios_ptr;
-
-    flash_size = memory_region_size(flash_mem);
 
     /* map the last 128KB of the BIOS in ISA space */
-    isa_bios_size = flash_size;
+    isa_bios_size = bios_size;
     if (isa_bios_size > (128 * 1024)) {
         isa_bios_size = 128 * 1024;
     }
     isa_bios = g_malloc(sizeof(*isa_bios));
-    memory_region_init_ram(isa_bios, "isa-bios", isa_bios_size);
-    vmstate_register_ram_global(isa_bios);
+    memory_region_init_alias(isa_bios, "isa-bios", bios,
+                             bios_size - isa_bios_size, isa_bios_size);
     memory_region_add_subregion_overlap(rom_memory,
                                         0x100000 - isa_bios_size,
                                         isa_bios,
                                         1);
+    memory_region_set_readonly(isa_bios, true);
 
-    /* copy ISA rom image from top of flash memory */
-    flash_ptr = memory_region_get_ram_ptr(flash_mem);
-    isa_bios_ptr = memory_region_get_ram_ptr(isa_bios);
-    memcpy(isa_bios_ptr,
-           ((uint8_t*)flash_ptr) + (flash_size - isa_bios_size),
-           isa_bios_size);
+    if (compat_vmsection) {
+        MemoryRegion *dummy_region = g_new(MemoryRegion, 1);
 
-    memory_region_set_readonly(isa_bios, true);
+        memory_region_init_ram(dummy_region, "isa-bios", isa_bios_size);
+        vmstate_register_ram_global(dummy_region);
+    }
 }
 
 static void pc_fw_add_pflash_drv(void)
@@ -102,7 +98,7 @@  static void pc_fw_add_pflash_drv(void)
 }
 
 static void pc_system_flash_init(MemoryRegion *rom_memory,
-                                 DriveInfo *pflash_drv)
+                                 DriveInfo *pflash_drv, bool compat_vmsection)
 {
     BlockDriverState *bdrv;
     int64_t size;
@@ -129,14 +125,14 @@  static void pc_system_flash_init(MemoryRegion *rom_memory,
                                          1, 0x0000, 0x0000, 0x0000, 0x0000, 0);
     flash_mem = pflash_cfi01_get_memory(system_flash);
 
-    pc_isa_bios_init(rom_memory, flash_mem, size);
+    pc_isa_bios_init(rom_memory, flash_mem, compat_vmsection);
 }
 
 static void old_pc_system_rom_init(MemoryRegion *rom_memory)
 {
     char *filename;
-    MemoryRegion *bios, *isa_bios;
-    int bios_size, isa_bios_size;
+    MemoryRegion *bios;
+    int bios_size;
     int ret;
 
     /* BIOS load */
@@ -167,19 +163,7 @@  static void old_pc_system_rom_init(MemoryRegion *rom_memory)
         g_free(filename);
     }
 
-    /* map the last 128KB of the BIOS in ISA space */
-    isa_bios_size = bios_size;
-    if (isa_bios_size > (128 * 1024)) {
-        isa_bios_size = 128 * 1024;
-    }
-    isa_bios = g_malloc(sizeof(*isa_bios));
-    memory_region_init_alias(isa_bios, "isa-bios", bios,
-                             bios_size - isa_bios_size, isa_bios_size);
-    memory_region_add_subregion_overlap(rom_memory,
-                                        0x100000 - isa_bios_size,
-                                        isa_bios,
-                                        1);
-    memory_region_set_readonly(isa_bios, true);
+    pc_isa_bios_init(rom_memory, bios, false);
 
     /* map all the bios at the top of memory */
     memory_region_add_subregion(rom_memory,
@@ -224,7 +208,8 @@  void pc_system_firmware_init(MemoryRegion *rom_memory)
     }
 
     if (pflash_drv != NULL) {
-        pc_system_flash_init(rom_memory, pflash_drv);
+        pc_system_flash_init(rom_memory, pflash_drv,
+                             sysfw_dev->compat_vmsection);
     } else {
         fprintf(stderr, "qemu: PC system firmware (pflash) not available\n");
         exit(1);
@@ -233,6 +218,8 @@  void pc_system_firmware_init(MemoryRegion *rom_memory)
 
 static Property pcsysfw_properties[] = {
     DEFINE_PROP_UINT8("rom_only", PcSysFwDevice, rom_only, 0),
+    DEFINE_PROP_BIT("compat_vmsection", PcSysFwDevice, compat_vmsection, 0,
+                    false),
     DEFINE_PROP_END_OF_LIST(),
 };