On 2/24/20 9:55 PM, Philippe Mathieu-Daudé wrote:
> The scripts/coccinelle/memory-region-housekeeping.cocci reported:
> * TODO [[view:hw/i386/pc_sysfw.c::face=ovl-face1::linb=67::colb=4::cole=26][potential use of memory_region_init_rom*() in hw/i386/pc_sysfw.c::67]]
>
> pc_isa_bios_init() does a manual copy of a part of the BIOS,
> from a read-only region. We can simplify by directly aliasing
> the same part.
>
> Before:
>
> (qemu) info mtree
> memory-region: system
> 0000000000000000-ffffffffffffffff (prio 0, i/o): system
> 0000000000000000-0000000007ffffff (prio 0, ram): alias ram-below-4g @pc.ram 0000000000000000-0000000007ffffff
> 0000000000000000-ffffffffffffffff (prio -1, i/o): pci
> 00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
> 00000000000c0000-00000000000dffff (prio 1, rom): pc.rom
> 00000000000e0000-00000000000fffff (prio 1, rom): isa-bios
> ...
> 00000000fff00000-00000000ffffffff (prio 0, romd): system.flash0
>
> After:
>
> (qemu) info mtree
> memory-region: system
> 0000000000000000-ffffffffffffffff (prio 0, i/o): system
> 0000000000000000-0000000007ffffff (prio 0, ram): alias ram-below-4g @pc.ram 0000000000000000-0000000007ffffff
> 0000000000000000-ffffffffffffffff (prio -1, i/o): pci
> 00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
> 00000000000c0000-00000000000dffff (prio 1, rom): pc.rom
> 00000000000e0000-00000000000fffff (prio 1, romd): alias isa-bios @system.flash0 00000000000e0000-00000000000fffff
> ...
> 00000000fff00000-00000000ffffffff (prio 0, romd): system.flash0
IIUC migrating old -> new is OK, the previous ROM copy is discarded.
What about new -> old, does it require specific handling? Do we care?
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/i386/pc_sysfw.c | 24 ++++++------------------
> 1 file changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
> index f5f3f466b0..e864c09ea8 100644
> --- a/hw/i386/pc_sysfw.c
> +++ b/hw/i386/pc_sysfw.c
> @@ -54,31 +54,19 @@ static void pc_isa_bios_init(MemoryRegion *rom_memory,
> MemoryRegion *flash_mem,
> int ram_size)
> {
> - int isa_bios_size;
> - MemoryRegion *isa_bios;
> - uint64_t flash_size;
> - void *flash_ptr, *isa_bios_ptr;
> -
> - flash_size = memory_region_size(flash_mem);
> + uint64_t isa_bios_size;
> + MemoryRegion *isa_bios = g_new(MemoryRegion, 1);
> + uint64_t flash_size = memory_region_size(flash_mem);
>
> /* map the last 128KB of the BIOS in ISA space */
> isa_bios_size = MIN(flash_size, 128 * KiB);
> - isa_bios = g_malloc(sizeof(*isa_bios));
> - memory_region_init_ram(isa_bios, NULL, "isa-bios", isa_bios_size,
> - &error_fatal);
> + memory_region_init_alias(isa_bios, NULL, "isa-bios", flash_mem,
> + flash_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);
> }
>
> static PFlashCFI01 *pc_pflash_create(PCMachineState *pcms,
>