Re: [Qemu-devel] [PULL 49/57] Round up RAMBlock sizes to host page sizes

[Dr. David Alan Gilbert]

* Peter Crosthwaite (address@hidden) wrote:
> On Tue, Nov 10, 2015 at 6:25 AM, Juan Quintela <address@hidden> wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> >
> > RAMBlocks that are not a multiple of host pages in length
> > cause problems for postcopy (I've seen an ACPI table on aarch64
> > be 5k in length - i.e. 5x target-page), so round RAMBlock sizes
> > up to a host-page.
> >
> > This potentially breaks migration compatibility due to changes
> > in RAMBlock sizes; however:
> >    1) x86 and s390 I think always have host=target page size
> >    2) When I've tried on Power the block sizes already seem aligned.
> >    3) I don't think there's anything else that maintains per-version
> >       machine-types for compatibility.
> >
> 
> Is there any reason this shouldn't be converted to
> REAL_HOST_PAGE_ALIGN given these restrictions?

If I remember correctly the only time host_page_size and real_host_page_size
are different is the odd case where host_page_size < target_page_size;
since we need to ensure alignment to the larger of the two, I don't
think it's right to use real_host_page_size, since you might be aligning
to something smaller than target_page_size.
(I don't know if migration already works in that case anyway)

> I'm thinking about multi-arch, where HOST_PAGE_ALIGN is inaccessible
> from exec.c as it is target-arch specific. My previous workaround was
> to define the target page size for multi-arch so that TARGET_PAGE_SIZE
> was usable but not sure that should go viral to these other defs.

Would it help to move qemu_ram_resize elsewhere?

> The other approach, is to calculate the lowest common denominator for
> the page size accross the multi-arch targets (+ the host) for the
> implementation of HOST_PAGE_ALIGN and friends.

I'm not sure I understand what happens to the migration bitmaps in multi-arch;
the bitmaps are one bit per target-page.

Dave

> 
> Regards,
> Peter
> 
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > Reviewed-by: Juan Quintela <address@hidden>
> > Signed-off-by: Juan Quintela <address@hidden>
> > ---
> >  exec.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/exec.c b/exec.c
> > index 36886ee..b09f18b 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -1452,7 +1452,7 @@ int qemu_ram_resize(ram_addr_t base, ram_addr_t 
> > newsize, Error **errp)
> >
> >      assert(block);
> >
> > -    newsize = TARGET_PAGE_ALIGN(newsize);
> > +    newsize = HOST_PAGE_ALIGN(newsize);
> >
> >      if (block->used_length == newsize) {
> >          return 0;
> > @@ -1596,7 +1596,7 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, 
> > MemoryRegion *mr,
> >          return -1;
> >      }
> >
> > -    size = TARGET_PAGE_ALIGN(size);
> > +    size = HOST_PAGE_ALIGN(size);
> >      new_block = g_malloc0(sizeof(*new_block));
> >      new_block->mr = mr;
> >      new_block->used_length = size;
> > @@ -1632,8 +1632,8 @@ ram_addr_t qemu_ram_alloc_internal(ram_addr_t size, 
> > ram_addr_t max_size,
> >      ram_addr_t addr;
> >      Error *local_err = NULL;
> >
> > -    size = TARGET_PAGE_ALIGN(size);
> > -    max_size = TARGET_PAGE_ALIGN(max_size);
> > +    size = HOST_PAGE_ALIGN(size);
> > +    max_size = HOST_PAGE_ALIGN(max_size);
> >      new_block = g_malloc0(sizeof(*new_block));
> >      new_block->mr = mr;
> >      new_block->resized = resized;
> > --
> > 2.5.0
> >
> >
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK