Re: [Qemu-devel] [PATCH 1/1] hw/ppc/spapr_hcall: set htab_shift after kvmppc_resize_hpt_commit

From: David Gibson <david@gibson.dropbear.id.au>
To: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 1/1] hw/ppc/spapr_hcall: set htab_shift after kvmppc_resize_hpt_commit
Date: Wed, 14 Feb 2018 10:29:35 +1100	[thread overview]
Message-ID: <20180213232935.GA3437@umbus.fritz.box> (raw)
In-Reply-To: <20180213173716.21358-1-danielhb@linux.vnet.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 2456 bytes --]

On Tue, Feb 13, 2018 at 03:37:16PM -0200, Daniel Henrique Barboza wrote:
1;5002;0c> Newer kernels have a htab resize capability when adding or remove
> memory. At these situations, the guest kernel might reallocate its
> htab to a more suitable size based on the resulting memory.
> 
> However, we're not setting the new value back into the machine state
> when a KVM guest resizes its htab. At first this doesn't seem harmful,
> but when migrating or saving the guest state (via virsh managedsave,
> for instance) this mismatch between the htab size of QEMU and the
> kernel makes the guest hangs when trying to load its state.
> 
> Inside h_resize_hpt_commit, the hypercall that commits the hash page
> resize changes, let's set spapr->htab_shift to the new value if we're
> sure that kvmppc_resize_hpt_commit were successful.
> 
> While we're here, add a "not RADIX" sanity check as it is already done
> in the related hypercall h_resize_hpt_prepare.
> 
> Fixes: https://github.com/open-power-host-os/qemu/issues/28
> Reported-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>

Ouch.  Good catch.  I'm kind of astonished this didn't break even
worse than it did.  Applied.

> ---
>  hw/ppc/spapr_hcall.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 76422cfac1..1986560480 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -731,11 +731,21 @@ static target_ulong h_resize_hpt_commit(PowerPCCPU *cpu,
>          return H_AUTHORITY;
>      }
>  
> +    if (!spapr->htab_shift) {
> +        /* Radix guest, no HPT */
> +        return H_NOT_AVAILABLE;
> +    }
> +
>      trace_spapr_h_resize_hpt_commit(flags, shift);
>  
>      rc = kvmppc_resize_hpt_commit(cpu, flags, shift);
>      if (rc != -ENOSYS) {
> -        return resize_hpt_convert_rc(rc);
> +        rc = resize_hpt_convert_rc(rc);
> +        if (rc == H_SUCCESS) {
> +            /* Need to set the new htab_shift in the machine state */
> +            spapr->htab_shift = shift;
> +        }
> +        return rc;
>      }
>  
>      if (flags != 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]