* [Qemu-devel] [PATCH] pc_sysfw: Always use alias for ISA BIOS region
@ 2012-11-01 16:32 Jan Kiszka
2012-11-01 17:15 ` Jordan Justen
2012-11-02 18:55 ` [Qemu-devel] [PATCH v2] " Jan Kiszka
0 siblings, 2 replies; 9+ messages in thread
From: Jan Kiszka @ 2012-11-01 16:32 UTC (permalink / raw)
To: qemu-devel; +Cc: Jordan Justen
From: Jan Kiszka <jan.kiszka@siemens.com>
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.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/pc_sysfw.c | 42 +++++++++---------------------------------
1 files changed, 9 insertions(+), 33 deletions(-)
diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
index 9d7c5f4..07627df 100644
--- a/hw/pc_sysfw.c
+++ b/hw/pc_sysfw.c
@@ -41,36 +41,24 @@ typedef struct PcSysFwDevice {
} PcSysFwDevice;
static void pc_isa_bios_init(MemoryRegion *rom_memory,
- MemoryRegion *flash_mem,
- int ram_size)
+ MemoryRegion *bios)
{
+ 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);
-
- /* 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);
-
memory_region_set_readonly(isa_bios, true);
}
@@ -129,14 +117,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);
}
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 +155,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);
/* map all the bios at the top of memory */
memory_region_add_subregion(rom_memory,
--
1.7.3.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] pc_sysfw: Always use alias for ISA BIOS region
2012-11-01 16:32 [Qemu-devel] [PATCH] pc_sysfw: Always use alias for ISA BIOS region Jan Kiszka
@ 2012-11-01 17:15 ` Jordan Justen
2012-11-01 17:17 ` Jan Kiszka
2012-11-02 18:55 ` [Qemu-devel] [PATCH v2] " Jan Kiszka
1 sibling, 1 reply; 9+ messages in thread
From: Jordan Justen @ 2012-11-01 17:15 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel
Would the old behavior need to be preserved for pc-1.1 & pc-1.2?
-Jordan
On Thu, Nov 1, 2012 at 9:32 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> 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.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> hw/pc_sysfw.c | 42 +++++++++---------------------------------
> 1 files changed, 9 insertions(+), 33 deletions(-)
>
> diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
> index 9d7c5f4..07627df 100644
> --- a/hw/pc_sysfw.c
> +++ b/hw/pc_sysfw.c
> @@ -41,36 +41,24 @@ typedef struct PcSysFwDevice {
> } PcSysFwDevice;
>
> static void pc_isa_bios_init(MemoryRegion *rom_memory,
> - MemoryRegion *flash_mem,
> - int ram_size)
> + MemoryRegion *bios)
> {
> + 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);
> -
> - /* 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);
> -
> memory_region_set_readonly(isa_bios, true);
> }
>
> @@ -129,14 +117,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);
> }
>
> 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 +155,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);
>
> /* map all the bios at the top of memory */
> memory_region_add_subregion(rom_memory,
> --
> 1.7.3.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] pc_sysfw: Always use alias for ISA BIOS region
2012-11-01 17:15 ` Jordan Justen
@ 2012-11-01 17:17 ` Jan Kiszka
2012-11-01 17:21 ` Jan Kiszka
0 siblings, 1 reply; 9+ messages in thread
From: Jan Kiszka @ 2012-11-01 17:17 UTC (permalink / raw)
To: Jordan Justen; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 4142 bytes --]
On 2012-11-01 18:15, Jordan Justen wrote:
> Would the old behavior need to be preserved for pc-1.1 & pc-1.2?
Why? This is just restoring the older, correct behavior.
Jan
>
> -Jordan
>
> On Thu, Nov 1, 2012 at 9:32 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> 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.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>> hw/pc_sysfw.c | 42 +++++++++---------------------------------
>> 1 files changed, 9 insertions(+), 33 deletions(-)
>>
>> diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
>> index 9d7c5f4..07627df 100644
>> --- a/hw/pc_sysfw.c
>> +++ b/hw/pc_sysfw.c
>> @@ -41,36 +41,24 @@ typedef struct PcSysFwDevice {
>> } PcSysFwDevice;
>>
>> static void pc_isa_bios_init(MemoryRegion *rom_memory,
>> - MemoryRegion *flash_mem,
>> - int ram_size)
>> + MemoryRegion *bios)
>> {
>> + 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);
>> -
>> - /* 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);
>> -
>> memory_region_set_readonly(isa_bios, true);
>> }
>>
>> @@ -129,14 +117,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);
>> }
>>
>> 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 +155,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);
>>
>> /* map all the bios at the top of memory */
>> memory_region_add_subregion(rom_memory,
>> --
>> 1.7.3.4
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] pc_sysfw: Always use alias for ISA BIOS region
2012-11-01 17:17 ` Jan Kiszka
@ 2012-11-01 17:21 ` Jan Kiszka
2012-11-01 18:03 ` Jordan Justen
0 siblings, 1 reply; 9+ messages in thread
From: Jan Kiszka @ 2012-11-01 17:21 UTC (permalink / raw)
To: Jordan Justen; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 525 bytes --]
On 2012-11-01 18:17, Jan Kiszka wrote:
> On 2012-11-01 18:15, Jordan Justen wrote:
>> Would the old behavior need to be preserved for pc-1.1 & pc-1.2?
>
> Why? This is just restoring the older, correct behavior.
Err, sorry, there was no difference to the behavior before pflash
(unless flash was changed by the guest).
Still, I see no point in preserving the current behavior even for compat
machine. Which (sane) guest should rely on an inconsistency between the
two BIOS mappings after an update?
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] pc_sysfw: Always use alias for ISA BIOS region
2012-11-01 17:21 ` Jan Kiszka
@ 2012-11-01 18:03 ` Jordan Justen
2012-11-01 18:23 ` Jan Kiszka
0 siblings, 1 reply; 9+ messages in thread
From: Jordan Justen @ 2012-11-01 18:03 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel
On Thu, Nov 1, 2012 at 10:21 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
> On 2012-11-01 18:17, Jan Kiszka wrote:
>> On 2012-11-01 18:15, Jordan Justen wrote:
>>> Would the old behavior need to be preserved for pc-1.1 & pc-1.2?
>>
>> Why? This is just restoring the older, correct behavior.
>
> Err, sorry, there was no difference to the behavior before pflash
> (unless flash was changed by the guest).
>
> Still, I see no point in preserving the current behavior even for compat
> machine. Which (sane) guest should rely on an inconsistency between the
> two BIOS mappings after an update?
I will not claim to know much about this, but I thought the purpose
was to allow qemu to properly restore old saved VMs.
I agree that the alias in an improvement in machine emulation, and I
don't think any guest software will rely upon the pc-1.1/pc-1.2
behavior.
It is probably worth verifying that the 440 chipset PAM registers are
still working after this change.
-Jordan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] pc_sysfw: Always use alias for ISA BIOS region
2012-11-01 18:03 ` Jordan Justen
@ 2012-11-01 18:23 ` Jan Kiszka
0 siblings, 0 replies; 9+ messages in thread
From: Jan Kiszka @ 2012-11-01 18:23 UTC (permalink / raw)
To: Jordan Justen; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]
On 2012-11-01 19:03, Jordan Justen wrote:
> On Thu, Nov 1, 2012 at 10:21 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> On 2012-11-01 18:17, Jan Kiszka wrote:
>>> On 2012-11-01 18:15, Jordan Justen wrote:
>>>> Would the old behavior need to be preserved for pc-1.1 & pc-1.2?
>>>
>>> Why? This is just restoring the older, correct behavior.
>>
>> Err, sorry, there was no difference to the behavior before pflash
>> (unless flash was changed by the guest).
>>
>> Still, I see no point in preserving the current behavior even for compat
>> machine. Which (sane) guest should rely on an inconsistency between the
>> two BIOS mappings after an update?
>
> I will not claim to know much about this, but I thought the purpose
> was to allow qemu to properly restore old saved VMs.
Ah, I'm getting the problem: the old version created additional RAM,
outside the main memory, and that caused an additional vmsection to be
written. Unfortunate. But I guess we can address this by registering a
dummy vmstate for compat machine types. The content is redundant anyway.
>
> I agree that the alias in an improvement in machine emulation, and I
> don't think any guest software will rely upon the pc-1.1/pc-1.2
> behavior.
>
> It is probably worth verifying that the 440 chipset PAM registers are
> still working after this change.
Seabios relies on PAM, so they are apparently still fine. More testing
always welcome, of course.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v2] pc_sysfw: Always use alias for ISA BIOS region
2012-11-01 16:32 [Qemu-devel] [PATCH] pc_sysfw: Always use alias for ISA BIOS region Jan Kiszka
2012-11-01 17:15 ` Jordan Justen
@ 2012-11-02 18:55 ` Jan Kiszka
2012-11-02 21:17 ` Jordan Justen
1 sibling, 1 reply; 9+ messages in thread
From: Jan Kiszka @ 2012-11-02 18:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Jordan Justen
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2] pc_sysfw: Always use alias for ISA BIOS region
2012-11-02 18:55 ` [Qemu-devel] [PATCH v2] " Jan Kiszka
@ 2012-11-02 21:17 ` Jordan Justen
2012-11-03 7:48 ` Jan Kiszka
0 siblings, 1 reply; 9+ messages in thread
From: Jordan Justen @ 2012-11-02 21:17 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v2] pc_sysfw: Always use alias for ISA BIOS region
2012-11-02 21:17 ` Jordan Justen
@ 2012-11-03 7:48 ` Jan Kiszka
0 siblings, 0 replies; 9+ messages in thread
From: Jan Kiszka @ 2012-11-03 7:48 UTC (permalink / raw)
To: Jordan Justen; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 6725 bytes --]
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
>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-11-03 7:48 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-01 16:32 [Qemu-devel] [PATCH] pc_sysfw: Always use alias for ISA BIOS region Jan Kiszka
2012-11-01 17:15 ` Jordan Justen
2012-11-01 17:17 ` Jan Kiszka
2012-11-01 17:21 ` Jan Kiszka
2012-11-01 18:03 ` Jordan Justen
2012-11-01 18:23 ` Jan Kiszka
2012-11-02 18:55 ` [Qemu-devel] [PATCH v2] " Jan Kiszka
2012-11-02 21:17 ` Jordan Justen
2012-11-03 7:48 ` Jan Kiszka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).