Re: [PATCH] slow_map: minor improvements to ROM BAR handling

[Alexander Graf <agraf@suse.de>]

Michael S. Tsirkin wrote:
> ROM BAR can be handled same as regular BAR:
> load_option_roms utility will take care of
> copying it to RAM as appropriate.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> This patch applies on top of agraf's one,
> it takes care of non-page aligned ROM BARs as well:
> they mostly are taken care of, we just do not
> need to warn user about them.
>
>  hw/device-assignment.c |   20 +++++++++-----------
>  1 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 000fa61..066fdb6 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -486,25 +486,23 @@ static int assigned_dev_register_regions(PCIRegion *io_regions,
>                  : PCI_BASE_ADDRESS_SPACE_MEMORY;
>  
>              if (cur_region->size & 0xFFF) {
> -                fprintf(stderr, "PCI region %d at address 0x%llx "
> -                        "has size 0x%x, which is not a multiple of 4K. "
> -                        "You might experience some performance hit due to that.\n",
> -                        i, (unsigned long long)cur_region->base_addr,
> -                        cur_region->size);
> +                if (i != PCI_ROM_SLOT) {
> +                    fprintf(stderr, "PCI region %d at address 0x%llx "
> +                            "has size 0x%x, which is not a multiple of 4K. "
> +                            "You might experience some performance hit "
> +                            "due to that.\n",
> +                            i, (unsigned long long)cur_region->base_addr,
> +                            cur_region->size);
> +                }
>                  slow_map = 1;
>   

This is wrong. You're setting slow_map = 1 on code that is very likely
to be executed inside the guest. That doesn't work.

Better pad the ROM size to page boundary and use the shadow mapping we
have in place already.


Alex