Re: [Qemu-devel] [PATCH RFC 1/2] memory: make MemoryRegion alias migratable

[Igor Mammedov <imammedo@redhat.com>]

On Mon, 29 Jul 2019 18:53:15 +0100
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:

> * Igor Mammedov (imammedo@redhat.com) wrote:
> > use qemu_ram_alloc_from_ptr() to create aliased RAMBlock
> > to the part of original memory region.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  exec.c   | 7 ++++---
> >  memory.c | 5 +++++
> >  2 files changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/exec.c b/exec.c
> > index 3e78de3b8f..daef0cd54f 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -2313,7 +2313,7 @@ static void ram_block_add(RAMBlock *new_block, Error **errp, bool shared)
> >                                          new_block->used_length,
> >                                          DIRTY_CLIENTS_ALL);
> >  
> > -    if (new_block->host) {
> > +    if (new_block->host && !new_block->mr->alias) {
> >          qemu_ram_setup_dump(new_block->host, new_block->max_length);
> >          qemu_madvise(new_block->host, new_block->max_length, QEMU_MADV_HUGEPAGE);
> >          /* MADV_DONTFORK is also needed by KVM in absence of synchronous MMU */
> > @@ -2671,7 +2671,8 @@ RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
> >  
> >      rcu_read_lock();
> >      block = atomic_rcu_read(&ram_list.mru_block);
> > -    if (block && block->host && host - block->host < block->max_length) {
> > +    if (block && !block->mr->alias && block->host &&
> > +        host - block->host < block->max_length) {
> >          goto found;
> >      }
> >  
> > @@ -2680,7 +2681,7 @@ RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
> >          if (block->host == NULL) {
> >              continue;
> >          }
> > -        if (host - block->host < block->max_length) {
> > +        if (!block->mr->alias && host - block->host < block->max_length) {
> >              goto found;
> >          }
> >      }
> > diff --git a/memory.c b/memory.c
> > index 5d8c9a9234..d710c17a26 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -1678,6 +1678,11 @@ void memory_region_init_alias(MemoryRegion *mr,
> >      memory_region_init(mr, owner, name, size);
> >      mr->alias = orig;
> >      mr->alias_offset = offset;
> > +    if (orig->ram_block && size) {
> > +        mr->ram_block = qemu_ram_alloc_from_ptr(size,
> > +                                                orig->ram_block->host + offset,
> > +                                                mr, &error_fatal);
> > +    }
> >  }
> 
> Doesn't this cause new memory regions to be created in other existing
> machines, e.g. x86's mem-smram, or the various PCI vga hacks?

I'd guess you've meant RAMBlocks instead of memory regions, if that's it
then yes, every alias pointing to RAM backed memory region will have
RAMBlock that's points to aliased part of aliased memory region.

(I didn't not intended to limit it to s390 only as it potentially
 could be used for x86 initial memory refactoring as well)
If it's issue we probably can address that by adding additional API
  memory_region_alias_make_migratable()
and call it on selected aliases, but the less APIs we have the better
so I went for reusing exiting vmstate_register_ram_global().

(I admit faking RAMBlock is a hack (albeit working one),
 but my attempt to teach migration code to accept aliases directly didn't
 work for me well and was very fragile, I don't remember specifics any more
 and probably code as well since it was utter failure)

PS:
 this patch is by no means a complete one (I probably cut corners on cleanup path)
 but should be sufficient to demo idea and see if it's acceptable.


> Dave
> >  
> >  void memory_region_init_rom_nomigrate(MemoryRegion *mr,
> > -- 
> > 2.18.1
> > 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK