* Peter Maydell (peter.mayd...@linaro.org) wrote: > On 3 October 2014 at 18:47, Dr. David Alan Gilbert (git) > <dgilb...@redhat.com> wrote: > > Just noticed after writing most of this email that this is an > old patch with new review comments. I'm sending the below > anyway in case some of it is still valid...
Still useful; I'm just looking at doing a new post soon. > > /* > > + * Utility for the outgoing postcopy code. > > + * > > + * Discard any partially sent host-page size chunks, mark any partially > > + * dirty host-page size chunks as all dirty. > > + * > > + * Returns: 0 on success > > + */ > > +static int postcopy_chunk_hostpages(MigrationState *ms) > > +{ > > + struct RAMBlock *block; > > + unsigned int host_bits = sysconf(_SC_PAGESIZE) / TARGET_PAGE_SIZE; > > I'm guessing this won't build on Win32. Can you use getpagesize() ? > We provide a compat wrapper for that in util/ as necessary. I'd used sysconf since the manpage of getpagesize() says 'Portable applications should employ sysconf(_SC_PAGESIZE) instead of getpagesize()' and I can see there are a couple of other places in qemu that use the same sysconf; however if it's not on Win32 then yes I'm happy to change over. > What happens if the TARGET_PAGE_SIZE is larger than the > host page size? (If you want MIN(host page size, TARGET_PAGE_SIZE) > try qemu_host_page_size, see page_size_init()). Thanks; I hadn't realised that was possible - but yes I should probably just use qemu_host_page_size instead of my sysconf in most places. What happens where the target wants to map a RAMBlock with Target-page-size alignment? > > + uint32_t host_mask; > > + > > + /* Should be a power of 2 */ > > + assert(host_bits && !(host_bits & (host_bits - 1))); > > assert(is_power_of_2(host_bits)); Thanks; fixed. > > + /* > > + * If the host_bits isn't a division of 32 (the minimum long size) > > + * then the code gets a lot more complex; disallow for now > > + * (I'm not aware of a system where it's true anyway) > > + */ > > + assert((32 % host_bits) == 0); > > + > > + /* A mask, starting at bit 0, containing host_bits continuous set bits > > */ > > + host_mask = (1u << host_bits) - 1; > > If the host has 64K pages and the guest TARGET_PAGE_SIZE is 1K > (eg ARM) then this will shift off the end of your uint32_t. Gah! That's going to make things a lot hairier; OK, that's going to take some rework, I'll have a think how. Note, keep an eye out for the RAM_SAVE_FLAG definitions in arch_init, they're one-bit-per-type of message (for no good reason) and with a TPS of 1K there are only a couple spare. Thanks for the comments, Dave > > thanks > -- PMM -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK