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

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

[Daniel Henrique Barboza]

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>
---
 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) {
-- 
2.14.3

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

[David Gibson]

[-- 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 --]