Re: [Qemu-devel] [PATCH v4 32/47] postcopy: ram_enable_notify to switch on userfault

[David Gibson]

On Fri, Oct 03, 2014 at 06:47:38PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
> 
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> ---
>  include/migration/migration.h    |  2 ++
>  include/migration/postcopy-ram.h |  6 +++++
>  postcopy-ram.c                   | 49 
> +++++++++++++++++++++++++++++++++++++++-
>  savevm.c                         |  9 ++++++++
>  4 files changed, 65 insertions(+), 1 deletion(-)
> 
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index be63c89..b01cc17 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -87,6 +87,8 @@ struct MigrationIncomingState {
>          POSTCOPY_RAM_INCOMING_END
>      } postcopy_ram_state;
>  
> +    /* For the kernel to send us notifications */
> +    int            userfault_fd;
>      QEMUFile *return_path;
>      QemuMutex      rp_mutex;    /* We send replies from multiple threads */
>      PostcopyPMI    postcopy_pmi;
> diff --git a/include/migration/postcopy-ram.h 
> b/include/migration/postcopy-ram.h
> index 8f237a2..413b670 100644
> --- a/include/migration/postcopy-ram.h
> +++ b/include/migration/postcopy-ram.h
> @@ -19,6 +19,12 @@
>  int postcopy_ram_hosttest(void);
>  
>  /*
> + * Make all of RAM sensitive to accesses to areas that haven't yet been 
> written
> + * and wire up anything necessary to deal with it.
> + */
> +int postcopy_ram_enable_notify(MigrationIncomingState *mis);
> +
> +/*
>   * Initialise postcopy-ram, setting the RAM to a state where we can go into
>   * postcopy later; must be called prior to any precopy.
>   * called from arch_init's similarly named ram_postcopy_incoming_init
> diff --git a/postcopy-ram.c b/postcopy-ram.c
> index 8eccf26..925ac77 100644
> --- a/postcopy-ram.c
> +++ b/postcopy-ram.c
> @@ -485,9 +485,51 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState 
> *mis)
>      return 0;
>  }
>  
> +/*
> + * Mark the given area of RAM as requiring notification to unwritten areas
> + * Used as a  callback on qemu_ram_foreach_block.
> + *   host_addr: Base of area to mark
> + *   offset: Offset in the whole ram arena
> + *   length: Length of the section
> + *   opaque: Unused

                ^^^^^^
This appears to be wrong - opaque is used to find the MIS.

> + * Returns 0 on success
> + */
> +static int postcopy_ram_sensitise_area(const char *block_name, void 
> *host_addr,
> +                                       ram_addr_t offset, ram_addr_t length,
> +                                       void *opaque)
> +{
> +    MigrationIncomingState *mis = opaque;
> +    uint64_t tokern[2];

"tokern"?

> +
> +    if (madvise(host_addr, length, MADV_USERFAULT)) {
> +        perror("postcopy_ram_sensitise_area madvise");
> +        return -1;
> +    }
> +
> +    /* Now tell our userfault_fd that it's responsible for this area */
> +    tokern[0] = (uint64_t)(uintptr_t)host_addr | 1; /* 1 means register area 
> */
> +    tokern[1] = (uint64_t)(uintptr_t)host_addr + length;
> +    if (write(mis->userfault_fd, tokern, 16) != 16) {
> +        perror("postcopy_ram_sensitise_area write");
> +        madvise(host_addr, length, MADV_NOUSERFAULT);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +int postcopy_ram_enable_notify(MigrationIncomingState *mis)
> +{
> +    /* Mark so that we get notified of accesses to unwritten areas */
> +    if (qemu_ram_foreach_block(postcopy_ram_sensitise_area, mis)) {
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
>  #else
>  /* No target OS support, stubs just fail */
> -
>  int postcopy_ram_hosttest(void)
>  {
>      error_report("postcopy_ram_hosttest: No OS support");
> @@ -528,6 +570,11 @@ int postcopy_ram_discard_range(MigrationIncomingState 
> *mis, uint8_t *start,
>  {
>      assert(0);
>  }
> +
> +int postcopy_ram_enable_notify(MigrationIncomingState *mis)
> +{
> +    assert(0);
> +}
>  #endif
>  
>  /* ------------------------------------------------------------------------- 
> */
> diff --git a/savevm.c b/savevm.c
> index 54bdb26..859c96f 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1304,6 +1304,15 @@ static int 
> loadvm_postcopy_ram_handle_listen(MigrationIncomingState *mis)
>  
>      mis->postcopy_ram_state = POSTCOPY_RAM_INCOMING_LISTENING;
>  
> +    /*
> +     * Sensitise RAM - can now generate requests for blocks that don't exist
> +     * However, at this point the CPU shouldn't be running, and the IO
> +     * shouldn't be doing anything yet so don't actually expect requests
> +     */
> +    if (postcopy_ram_enable_notify(mis)) {
> +        return -1;
> +    }
> +
>      /* TODO start up the postcopy listening thread */
>      return 0;
>  }

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

pgpibBF4BiFYy.pgp
Description: PGP signature