On 11/05/2014 05:17 PM, Samuel Mendoza-Jonas wrote:
> If a TCG guest reboots during a running migration HTAB entries are not
> marked dirty, and the destination boots with an invalid HTAB.
>
> When a reboot occurs reset the state of HTAB migration, and explicitly
> inform the destination of invalid entries.
>
> Signed-off-by: Samuel Mendoza-Jonas <sam...@au1.ibm.com>
> ---
> hw/ppc/spapr.c | 59
> +++++++++++++++++++++++++++++++++++---------------
> include/hw/ppc/spapr.h | 1 +
> 2 files changed, 42 insertions(+), 18 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 1610c28..9f419e8 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -829,26 +829,30 @@ static void spapr_reset_htab(sPAPREnvironment *spapr)
>
> shift = kvmppc_reset_htab(spapr->htab_shift);
>
> + pthread_mutex_lock(&spapr->htab_mutex);
> if (shift > 0) {
> /* Kernel handles htab, we don't need to allocate one */
> spapr->htab_shift = shift;
> kvmppc_kern_htab = true;
>
> /* Tell readers to update their file descriptor */
> - pthread_mutex_lock(&spapr->htab_mutex);
> if (spapr->htab_fd > 0) {
> spapr->htab_fd_stale = true;
> }
> - pthread_mutex_unlock(&spapr->htab_mutex);
> } else {
> if (!spapr->htab) {
> /* Allocate an htab if we don't yet have one */
> spapr->htab = qemu_memalign(HTAB_SIZE(spapr), HTAB_SIZE(spapr));
> + } else {
> + spapr->htab_mig_full = true;
> + spapr->htab_first_pass = true;
> + spapr->htab_save_index = 0;
> }
>
> /* And clear it */
> memset(spapr->htab, 0, HTAB_SIZE(spapr));
> }
> + pthread_mutex_unlock(&spapr->htab_mutex);
The pthread_mutex_(un)lock things from the chunk above should go to the
previous patch.
>
> /* Update the RMA size if necessary */
> if (spapr->vrma_adjust) {
> @@ -1019,6 +1023,7 @@ static int htab_save_setup(QEMUFile *f, void *opaque)
> pthread_mutex_lock(&spapr->htab_mutex);
> spapr->htab_fd = kvmppc_get_htab_fd(false);
> spapr->htab_fd_stale = false;
> + spapr->htab_mig_full = false;
> pthread_mutex_unlock(&spapr->htab_mutex);
> if (spapr->htab_fd < 0) {
> fprintf(stderr, "Unable to open fd for reading hash table from
> KVM: %s\n",
> @@ -1034,6 +1039,7 @@ static int htab_save_setup(QEMUFile *f, void *opaque)
> static void htab_save_first_pass(QEMUFile *f, sPAPREnvironment *spapr,
> int64_t max_ns)
> {
> + bool final = max_ns < 0;
> int htabslots = HTAB_SIZE(spapr) / HASH_PTE_SIZE_64;
> int index = spapr->htab_save_index;
> int64_t starttime = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> @@ -1041,33 +1047,40 @@ static void htab_save_first_pass(QEMUFile *f,
> sPAPREnvironment *spapr,
> assert(spapr->htab_first_pass);
>
> do {
> - int chunkstart;
> + int chunkstart, invalidstart;
>
> - /* Consume invalid HPTEs */
> - while ((index < htabslots)
> - && !HPTE_VALID(HPTE(spapr->htab, index))) {
Nit: in most places in qemu "&&" would be in the end of the previos line :)
> + chunkstart = index;
> + /* Consume valid HPTEs */
> + while ((index < htabslots && (index - chunkstart < USHRT_MAX))
> + && HPTE_VALID(HPTE(spapr->htab, index))) {
> index++;
> CLEAN_HPTE(HPTE(spapr->htab, index));
> }
>
> - /* Consume valid HPTEs */
> - chunkstart = index;
> - while ((index < htabslots)
> - && HPTE_VALID(HPTE(spapr->htab, index))) {
> + invalidstart = index;
> + /* Consume invalid HPTEs */
> + while ((index < htabslots && (index - invalidstart < USHRT_MAX))
> + && !HPTE_VALID(HPTE(spapr->htab, index))) {
> index++;
> CLEAN_HPTE(HPTE(spapr->htab, index));
> }
>
> - if (index > chunkstart) {
> - int n_valid = index - chunkstart;
> + /* Avoid writing an end marker (0,0,0) */
> + if (index > chunkstart
> + && !(chunkstart == invalidstart && !spapr->htab_mig_full)) {
> + int n_valid = invalidstart - chunkstart;
> + /* If a reset has occured we must explicitly overwrite the HTAB
> + * of the destination */
> + int n_invalid = spapr->htab_mig_full ? index - invalidstart : 0;
>
> qemu_put_be32(f, chunkstart);
> qemu_put_be16(f, n_valid);
> - qemu_put_be16(f, 0);
> + qemu_put_be16(f, n_invalid);
> qemu_put_buffer(f, HPTE(spapr->htab, chunkstart),
> HASH_PTE_SIZE_64 * n_valid);
>
> - if ((qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - starttime) >
> max_ns) {
> + if ((qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - starttime) > max_ns
> + && !final) {
> break;
> }
> }
> @@ -1182,10 +1195,14 @@ static int htab_save_iterate(QEMUFile *f, void
> *opaque)
> if (rc < 0) {
> return rc;
> }
> - } else if (spapr->htab_first_pass) {
> - htab_save_first_pass(f, spapr, MAX_ITERATION_NS);
> } else {
> - rc = htab_save_later_pass(f, spapr, MAX_ITERATION_NS);
> + pthread_mutex_lock(&spapr->htab_mutex);
> + if (spapr->htab_first_pass) {
> + htab_save_first_pass(f, spapr, MAX_ITERATION_NS);
> + } else {
> + rc = htab_save_later_pass(f, spapr, MAX_ITERATION_NS);
> + }
> + pthread_mutex_unlock(&spapr->htab_mutex);
> }
>
> /* End marker */
> @@ -1220,7 +1237,13 @@ static int htab_save_complete(QEMUFile *f, void
> *opaque)
> close(spapr->htab_fd);
> spapr->htab_fd = -1;
> } else {
> - htab_save_later_pass(f, spapr, -1);
> + pthread_mutex_lock(&spapr->htab_mutex);
> + if (spapr->htab_first_pass) {
> + htab_save_first_pass(f, spapr, -1);
> + } else {
> + htab_save_later_pass(f, spapr, -1);
> + }
> + pthread_mutex_unlock(&spapr->htab_mutex);
> }
>
> /* End marker */
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 5e29bec..ee95459 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -38,6 +38,7 @@ typedef struct sPAPREnvironment {
> bool htab_first_pass;
> int htab_fd;
> bool htab_fd_stale;
> + bool htab_mig_full;
Not sure what @htab_mig_full stands for exactly. Please explain it in the
commit log.
> pthread_mutex_t htab_mutex;
> } sPAPREnvironment;
>
>
--
Alexey
