Re: [Qemu-devel] [PATCH v4 1/2] arch_init: align MR size to target page size

[Michael S. Tsirkin]

On Mon, Aug 19, 2013 at 06:45:10PM +0100, Peter Maydell wrote:
> On 19 August 2013 18:37, Laszlo Ersek <address@hidden> wrote:
> > ((3) memory_region_size() is slightly different from
> > int128_get64(mr->size); it has a special case for int128_2_64() -- and I
> > don't understand that.
> 
> The special case is because valid memory region sizes range
> from 0 to 2^64, *inclusive*. [2^64-sized regions tend to be
> containers representing address spaces like PCI or the system
> memory space.] Inside memory.c we store the size
> in an int128 in the way you'd expect. However some of the
> memory region APIs take or return the size of the region
> as a uint64_t, with the convention that UINT64_MAX means
> "actually 2^64" (with a size of really 2^64 - 1 not being
> valid). So the special case here is doing the conversion
> from an int128 representation of the size to the uint64_t
> representation. You can see the same thing going the other
> way in memory_region_init(), where we take the size as a
> uint64_t and convert it to an int128 with
> 
>     mr->size = int128_make64(size);
>     if (size == UINT64_MAX) {
>         mr->size = int128_2_64();
>     }
> 
> (Note that int128_get64() of an int128 which == 2^64 or
> more will assert, so int128_get64(mr->size) is a bit of a
> code smell; it happens to be OK here because we know that a
> RAM-backed MR will never be a 2^64-sized region.)
> 
> -- PMM

Note code smell is in the original code -
this patch uses the API.
We could add memory_region_get_target_pages
as that will fit in uint64_t.
But other migration code shifts it right back by target page size,
so I don't think it's worth it.