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

[David Gibson]

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 <address@hidden>
> Signed-off-by: Daniel Henrique Barboza <address@hidden>

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

signature.asc
Description: PGP signature