* [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).