Re: [Qemu-devel] [PATCH] Fix guest OS hangs on boot when 64bit PCI BAR present

[Michael S. Tsirkin]

On Wed, Feb 13, 2013 at 06:14:33PM +1300, Alexey Korolev wrote:
> At the moment may_overlap flag of MemoryRegion structure
> is ignored by the address range assignment process.
> This may lead to guest OS hangs if critical qemu
> resources are overlapped by PCI BARs. For example
> ivshmem 64bit PCI BAR may overlap kvm-apic-msi under
> certain conditions. This patch adds a rule that the
> regions which should not be overlapped are added to the
> view first (i.e. having highest priority). The patch
> also corrects ivshmem bar resource to be overlapable
> which is the default for PCI BARs
> 
> Signed-off-by: Alexey Korolev <address@hidden>

Since overlap is currently used inconsistently, it's hard to
know what this will do. Maybe we should just drop the overlap
flag and use priorities instead?

> ---
>  hw/ivshmem.c |    2 +-
>  memory.c     |   15 ++++++++++-----
>  2 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/ivshmem.c b/hw/ivshmem.c
> index afaf9b3..1770fa3 100644
> --- a/hw/ivshmem.c
> +++ b/hw/ivshmem.c
> @@ -341,7 +341,7 @@ static void create_shared_memory_BAR(IVShmemState *s, int 
> fd) {
>      memory_region_init_ram_ptr(&s->ivshmem, "ivshmem.bar2",
>                                 s->ivshmem_size, ptr);
>      vmstate_register_ram(&s->ivshmem, &s->dev.qdev);
> -    memory_region_add_subregion(&s->bar, 0, &s->ivshmem);
> +    memory_region_add_subregion_overlap(&s->bar, 0, &s->ivshmem, 1);
>  
>      /* region for shared memory */
>      pci_register_bar(&s->dev, 2, s->ivshmem_attr, &s->bar);

So why this change, exactly?

> diff --git a/memory.c b/memory.c
> index cd7d5e0..f1119e7 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -475,7 +475,8 @@ static void render_memory_region(FlatView *view,
>                                   MemoryRegion *mr,
>                                   Int128 base,
>                                   AddrRange clip,
> -                                 bool readonly)
> +                                 bool readonly,
> +                                 bool overlap)
>  {
>      MemoryRegion *subregion;
>      unsigned i;
> @@ -503,16 +504,16 @@ static void render_memory_region(FlatView *view,
>      if (mr->alias) {
>          int128_subfrom(&base, int128_make64(mr->alias->addr));
>          int128_subfrom(&base, int128_make64(mr->alias_offset));
> -        render_memory_region(view, mr->alias, base, clip, readonly);
> +        render_memory_region(view, mr->alias, base, clip, readonly, overlap);
>          return;
>      }
>  
>      /* Render subregions in priority order. */
>      QTAILQ_FOREACH(subregion, &mr->subregions, subregions_link) {
> -        render_memory_region(view, subregion, base, clip, readonly);
> +        render_memory_region(view, subregion, base, clip, readonly, overlap);
>      }
>  
> -    if (!mr->terminates) {
> +    if (mr->may_overlap != overlap || !mr->terminates) {
>          return;
>      }
>  
> @@ -567,7 +568,11 @@ static FlatView generate_memory_topology(MemoryRegion 
> *mr)
>  
>      if (mr) {
>          render_memory_region(&view, mr, int128_zero(),
> -                             addrrange_make(int128_zero(), int128_2_64()), 
> false);
> +                             addrrange_make(int128_zero(), int128_2_64()),
> +                             false, false);
> +        render_memory_region(&view, mr, int128_zero(),
> +                             addrrange_make(int128_zero(), int128_2_64()),
> +                             false, true);
>      }
>      flatview_simplify(&view);
>  
> -- 
> 1.7.9.5