Re: [Qemu-devel] [PATCH for 2.10] pc: make 'pc.rom' readonly when machine has PCI enabled

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Igor Mammedov (imammedo@redhat.com) wrote:
> looking at bios ROM mapping in QEMU it seems that only isapc
> (i.e. not PCI enabled machine) requires ROM being mapped as
> RW in other cases BIOS is mapped as RO. Do the same for option
> ROM 'pc.rom' when machine has PCI enabled.

Does this need to be machine-type linked?

Dave

> As useful side-effect pc.rom MemoryRegion stops being
> put in vhost memory map (filtered out by vhost_section()),
> which reduces number of entries by 1.
> 
> Coincidentally it fixes migration failure reported in
> 
> "[PATCH V2]  vhost: fix a migration failed because of vhost region merge"
> 
> where following destination CLI with /sys/module/vhost/parameters/max_mem_regions = 8
> 
> export DIMMSCOUNT=6
> QEMU -enable-kvm \
>      -netdev type=tap,id=guest0,vhost=on,script=no,vhostforce \
>      -device virtio-net-pci,netdev=guest0 \
>      -m 256,slots=256,maxmem=2G \
>      `i=0; while [ $i -lt $DIMMSCOUNT ]; do echo \
>          "-object memory-backend-ram,id=m$i,size=128M \
>           -device pc-dimm,id=d$i,memdev=m$i"; i=$(($i + 1)); \
>      done`
> 
> will fail to startup with error:
> 
>  "-device pc-dimm,id=d5,memdev=m5: a used vhost backend has no free memory slots left"
> 
> while it's possible to add the 6th DIMM during hotplug
> on source.
> 
> Issue is caused by the fact that number of entries in vhost map
> is bigger on 1 entry, when -device is processed, than
> after guest boots up, and that offending entry belongs to
> 'pc.rom', it's not like vhost intends to do IO in ROM range
> so making it RO hides region from vhost and makes number
> of entries in vhost memory map at -device/machine_done time
> match number of entries after guest boots.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reported-by: Peng Hao <peng.hao2@zte.com.cn>
> ---
>  hw/i386/pc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index e3fcd51..de459e2 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1449,6 +1449,9 @@ void pc_memory_init(PCMachineState *pcms,
>      option_rom_mr = g_malloc(sizeof(*option_rom_mr));
>      memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE,
>                             &error_fatal);
> +    if (pcmc->pci_enabled) {
> +        memory_region_set_readonly(option_rom_mr, true);
> +    }
>      vmstate_register_ram_global(option_rom_mr);
>      memory_region_add_subregion_overlap(rom_memory,
>                                          PC_ROM_MIN_VGA,
> -- 
> 2.7.4
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK