[Qemu-devel] [PATCH v1 0/2] Handle KVM_PPC_ALLOCATE_HTAB ioctl failure

[Qemu-devel] [PATCH v1 0/2] Handle KVM_PPC_ALLOCATE_HTAB ioctl failure

[Bharata B Rao]

KVM_PPC_ALLOCATE_HTAB ioctl can now return errors and handle that
correctly in QEMU.

Also fix !CONFIG_KVM implementation of kvmppc_reset_htab() to return
0 instead of -1.

Changes in v1
------------
- Add a patch to fix !CONFIG_KVM implementation of kvmppc_reset_htab()
so that make check passes for ppc64 target on x86 host.
- Explicitly check for shift < 0 error condition in spapr_alloc/reset_htab.

v0: https://lists.gnu.org/archive/html/qemu-ppc/2015-11/msg00012.html

Bharata B Rao (2):
  ppc: Let kvmppc_reset_htab() return 0 for !CONFIG_KVM
  spapr: Handle failure of KVM_PPC_ALLOCATE_HTAB ioctl

 hw/ppc/spapr.c       | 20 ++++++++++++++++----
 target-ppc/kvm_ppc.h |  2 +-
 2 files changed, 17 insertions(+), 5 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v1 1/2] ppc: Let kvmppc_reset_htab() return 0 for !CONFIG_KVM

[Bharata B Rao]

The !CONFIG_KVM implementation of kvmppc_reset_htab() returns -1
by default. Change this to return 0 so that we fall back to user space
HTAB allocation for emulated guests.

This fixes the make check failures for ppc64 emulated target.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 target-ppc/kvm_ppc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 309cbe0..5e1333d 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -180,7 +180,7 @@ static inline int kvmppc_remove_spapr_tce(void *table, int pfd,
 
 static inline int kvmppc_reset_htab(int shift_hint)
 {
-    return -1;
+    return 0;
 }
 
 static inline uint64_t kvmppc_rma_size(uint64_t current_size,
-- 
2.1.0

[Qemu-devel] [PATCH v1 2/2] spapr: Handle failure of KVM_PPC_ALLOCATE_HTAB ioctl

[Bharata B Rao]

KVM_PPC_ALLOCATE_HTAB ioctl can return -ENOMEM for KVM guests and QEMU
never handled this correctly. But this didn't cause any problems till
now as KVM_PPC_ALLOCATE_HTAB ioctl returned with smaller than requested
HTAB when enough contiguous memory wasn't available in the host.
After the proposed kernel change: https://patchwork.ozlabs.org/patch/530501/,
KVM_PPC_ALLOCATE_HTAB ioctl will not fallback to lower sized HTAB
allocation and will fail if requested HTAB size can't be met.

Check for such failures in QEMU and abort appropriately. This will
prevent guest kernel from hanging/freezing during early boot by doing
graceful exit when host is unable to allocate requested HTAB.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e1202ce..a64a1b5 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1021,9 +1021,19 @@ static void spapr_alloc_htab(sPAPRMachineState *spapr)
      * RAM */
 
     shift = kvmppc_reset_htab(spapr->htab_shift);
-
-    if (shift > 0) {
-        /* Kernel handles htab, we don't need to allocate one */
+    if (shift < 0) {
+        /*
+         * For HV KVM, host kernel will return -ENOMEM when requested
+         * HTAB size can't be allocated.
+         */
+        error_setg(&error_abort, "Failed to allocate HTAB of requested size, try with smaller maxmem");
+    } else if (shift > 0) {
+        /*
+         * Kernel handles htab, we don't need to allocate one
+         *
+         * Older kernels can fall back to lower HTAB shift values,
+         * but we don't allow booting of such guests.
+         */
         if (shift != spapr->htab_shift) {
             error_setg(&error_abort, "Failed to allocate HTAB of requested size, try with smaller maxmem");
         }
@@ -1055,7 +1065,9 @@ static void spapr_reset_htab(sPAPRMachineState *spapr)
     int index;
 
     shift = kvmppc_reset_htab(spapr->htab_shift);
-    if (shift > 0) {
+    if (shift < 0) {
+        error_setg(&error_abort, "Failed to reset HTAB");
+    } else if (shift > 0) {
         if (shift != spapr->htab_shift) {
             error_setg(&error_abort, "Requested HTAB allocation failed during reset");
         }
-- 
2.1.0

Re: [Qemu-devel] [PATCH v1 2/2] spapr: Handle failure of KVM_PPC_ALLOCATE_HTAB ioctl

[Michael Roth]

Quoting Bharata B Rao (2015-11-09 23:24:54)
> KVM_PPC_ALLOCATE_HTAB ioctl can return -ENOMEM for KVM guests and QEMU
> never handled this correctly. But this didn't cause any problems till
> now as KVM_PPC_ALLOCATE_HTAB ioctl returned with smaller than requested
> HTAB when enough contiguous memory wasn't available in the host.
> After the proposed kernel change: https://patchwork.ozlabs.org/patch/530501/,
> KVM_PPC_ALLOCATE_HTAB ioctl will not fallback to lower sized HTAB
> allocation and will fail if requested HTAB size can't be met.
> 
> Check for such failures in QEMU and abort appropriately. This will
> prevent guest kernel from hanging/freezing during early boot by doing
> graceful exit when host is unable to allocate requested HTAB.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
>  hw/ppc/spapr.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e1202ce..a64a1b5 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1021,9 +1021,19 @@ static void spapr_alloc_htab(sPAPRMachineState *spapr)
>       * RAM */
> 
>      shift = kvmppc_reset_htab(spapr->htab_shift);
> -
> -    if (shift > 0) {
> -        /* Kernel handles htab, we don't need to allocate one */
> +    if (shift < 0) {
> +        /*
> +         * For HV KVM, host kernel will return -ENOMEM when requested
> +         * HTAB size can't be allocated.
> +         */
> +        error_setg(&error_abort, "Failed to allocate HTAB of requested size, try with smaller maxmem");

Wording is appropriate, but maybe just "Failed to allocate HTAB, try with
smaller maxmem" to distinguish it from the error below where the call succeeds
but there's a size mismatch?

> +    } else if (shift > 0) {
> +        /*
> +         * Kernel handles htab, we don't need to allocate one
> +         *
> +         * Older kernels can fall back to lower HTAB shift values,
> +         * but we don't allow booting of such guests.
> +         */
>          if (shift != spapr->htab_shift) {
>              error_setg(&error_abort, "Failed to allocate HTAB of requested size, try with smaller maxmem");
>          }
> @@ -1055,7 +1065,9 @@ static void spapr_reset_htab(sPAPRMachineState *spapr)
>      int index;
> 
>      shift = kvmppc_reset_htab(spapr->htab_shift);
> -    if (shift > 0) {
> +    if (shift < 0) {
> +        error_setg(&error_abort, "Failed to reset HTAB");
> +    } else if (shift > 0) {
>          if (shift != spapr->htab_shift) {
>              error_setg(&error_abort, "Requested HTAB allocation failed during reset");
>          }
> -- 
> 2.1.0
> 

Re: [Qemu-devel] [PATCH v1 2/2] spapr: Handle failure of KVM_PPC_ALLOCATE_HTAB ioctl

[David Gibson]

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

On Tue, Nov 10, 2015 at 08:38:57AM -0600, Michael Roth wrote:
> Quoting Bharata B Rao (2015-11-09 23:24:54)
> > KVM_PPC_ALLOCATE_HTAB ioctl can return -ENOMEM for KVM guests and QEMU
> > never handled this correctly. But this didn't cause any problems till
> > now as KVM_PPC_ALLOCATE_HTAB ioctl returned with smaller than requested
> > HTAB when enough contiguous memory wasn't available in the host.
> > After the proposed kernel change: https://patchwork.ozlabs.org/patch/530501/,
> > KVM_PPC_ALLOCATE_HTAB ioctl will not fallback to lower sized HTAB
> > allocation and will fail if requested HTAB size can't be met.
> > 
> > Check for such failures in QEMU and abort appropriately. This will
> > prevent guest kernel from hanging/freezing during early boot by doing
> > graceful exit when host is unable to allocate requested HTAB.
> > 
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > ---
> >  hw/ppc/spapr.c | 20 ++++++++++++++++----
> >  1 file changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index e1202ce..a64a1b5 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1021,9 +1021,19 @@ static void spapr_alloc_htab(sPAPRMachineState *spapr)
> >       * RAM */
> > 
> >      shift = kvmppc_reset_htab(spapr->htab_shift);
> > -
> > -    if (shift > 0) {
> > -        /* Kernel handles htab, we don't need to allocate one */
> > +    if (shift < 0) {
> > +        /*
> > +         * For HV KVM, host kernel will return -ENOMEM when requested
> > +         * HTAB size can't be allocated.
> > +         */
> > +        error_setg(&error_abort, "Failed to allocate HTAB of requested size, try with smaller maxmem");
> 
> Wording is appropriate, but maybe just "Failed to allocate HTAB, try with
> smaller maxmem" to distinguish it from the error below where the call succeeds
> but there's a size mismatch?

Possibly the message could be a little better, but I'm not going to
hold up the patches just for that.  I've applied tentatively to
spapr-next, and I hope to send a pull req with it soon, assuming it
survives my tests.

-- 
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: 819 bytes --]