[Qemu-devel] [RFC PATCH v1 0/2] spapr: Abort when HTAB size requirement can't be met

[Qemu-devel] [RFC PATCH v1 0/2] spapr: Abort when HTAB size requirement can't be met

[Bharata B Rao]

HTAB size is a factor of maximum memory size that is specified by maxmem=
command line option. In cases where there is shortage of host memory, host
will not be able to allocate contiguous memory for guest HTAB and will
instead allocate a smaller HTAB. This usually is not a problem but when
user starts hotplugging memory to the guest, we can run out of HTAB entries
and hence memory hotplug fails. This failure should have been handled
gracefully by the guest kernel, but currently it leads to guest kernel OOPS.
This will eventually get fixed when the handling of memory hotplug is
completely moved to kernel for PowerKVM.

Prevent such kernel failure by refusing to boot the guest when requested
HTAB size can't be allocated. However HTAB allocation happens in the
reset path from where it is too late to abort. Hence this patchset
moves the HTAB allocation to machine init and aborts if HTAB size
requirement isn't met.

This patchset applies against David Gibson's spapr-next. With this
patchset, simple boot, reboot and migration tests pass with HV KVM guest.
For PR KVM guest, boot and reboot tests were done since migration already
appears broken for PR KVM.

Changes in v1
-------------
- Correctly handle HTAB for PR KVM guests that use QEMU allocated HTAB.
- Verbose error message when aborting.
- Check for returned shift value in reset path too and abort when
  there is mismatch in htab shift values.
- Move the marking of htab_fd as stale from spapr_alloc_htab() to
  spapr_reset_htab() where it originally belonged.

v0: http://lists.nongnu.org/archive/html/qemu-devel/2015-09/msg05491.html

Bharata B Rao (2):
  spapr: Allocate HTAB from machine init
  spapr: Abort when HTAB of requested size isn't allocated

 hw/ppc/spapr.c | 42 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 7 deletions(-)

-- 
2.1.0

[Qemu-devel] [RFC PATCH v1 1/2] spapr: Allocate HTAB from machine init

[Bharata B Rao]

Allocate HTAB from ppc_spapr_init() so that we can abort the guest
if requested HTAB size is't allocated by the host. However retain the
htab reset call in spapr_reset_htab() so that HTAB gets reset (and
not allocated) during machine reset.

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

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7f4f196..f6a5c29 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -979,7 +979,7 @@ static void emulate_spapr_hypercall(PowerPCCPU *cpu)
 #define CLEAN_HPTE(_hpte)  ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY))
 #define DIRTY_HPTE(_hpte)  ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY))
 
-static void spapr_reset_htab(sPAPRMachineState *spapr)
+static void spapr_alloc_htab(sPAPRMachineState *spapr)
 {
     long shift;
     int index;
@@ -994,18 +994,37 @@ static void spapr_reset_htab(sPAPRMachineState *spapr)
         /* Kernel handles htab, we don't need to allocate one */
         spapr->htab_shift = shift;
         kvmppc_kern_htab = true;
+    } else {
+        /* Allocate htab */
+        spapr->htab = qemu_memalign(HTAB_SIZE(spapr), HTAB_SIZE(spapr));
+
+        /* And clear it */
+        memset(spapr->htab, 0, HTAB_SIZE(spapr));
+
+        for (index = 0; index < HTAB_SIZE(spapr) / HASH_PTE_SIZE_64; index++) {
+            DIRTY_HPTE(HPTE(spapr->htab, index));
+        }
+    }
+}
+
+/*
+ * Clear HTAB entries during reset.
+ *
+ * If host kernel has allocated HTAB, KVM_PPC_ALLOCATE_HTAB ioctl is
+ * used to clear HTAB. Otherwise QEMU-allocated HTAB is cleared manually.
+ */
+static void spapr_reset_htab(sPAPRMachineState *spapr)
+{
+    long shift;
+    int index;
 
+    shift = kvmppc_reset_htab(spapr->htab_shift);
+    if (shift > 0) {
         /* Tell readers to update their file descriptor */
         if (spapr->htab_fd >= 0) {
             spapr->htab_fd_stale = true;
         }
     } else {
-        if (!spapr->htab) {
-            /* Allocate an htab if we don't yet have one */
-            spapr->htab = qemu_memalign(HTAB_SIZE(spapr), HTAB_SIZE(spapr));
-        }
-
-        /* And clear it */
         memset(spapr->htab, 0, HTAB_SIZE(spapr));
 
         for (index = 0; index < HTAB_SIZE(spapr) / HASH_PTE_SIZE_64; index++) {
@@ -1709,6 +1728,7 @@ static void ppc_spapr_init(MachineState *machine)
         }
         spapr->htab_shift++;
     }
+    spapr_alloc_htab(spapr);
 
     /* Set up Interrupt Controller before we create the VCPUs */
     spapr->icp = xics_system_init(machine,
-- 
2.1.0

[Qemu-devel] [RFC PATCH v1 2/2] spapr: Abort when HTAB of requested size isn't allocated

[Bharata B Rao]

Terminate the guest when HTAB of requested size isn't allocated by
the host.

When memory hotplug is attempted on a guest that has booted with
less than requested HTAB size, the guest kernel will not be able
to gracefully fail the hotplug request. This patch will ensure that
we never end up in a situation where memory hotplug fails due to
less than requested HTAB size.

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

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f6a5c29..3c60df2 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -992,6 +992,10 @@ static void spapr_alloc_htab(sPAPRMachineState *spapr)
 
     if (shift > 0) {
         /* Kernel handles htab, we don't need to allocate one */
+        if (shift != spapr->htab_shift) {
+            error_setg(&error_abort, "Failed to allocate HTAB of requested size, try with smaller maxmem");
+        }
+
         spapr->htab_shift = shift;
         kvmppc_kern_htab = true;
     } else {
@@ -1020,6 +1024,10 @@ static void spapr_reset_htab(sPAPRMachineState *spapr)
 
     shift = kvmppc_reset_htab(spapr->htab_shift);
     if (shift > 0) {
+        if (shift != spapr->htab_shift) {
+            error_setg(&error_abort, "Requested HTAB allocation failed during reset");
+        }
+
         /* Tell readers to update their file descriptor */
         if (spapr->htab_fd >= 0) {
             spapr->htab_fd_stale = true;
-- 
2.1.0

Re: [Qemu-devel] [RFC PATCH v1 0/2] spapr: Abort when HTAB size requirement can't be met

[David Gibson]

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

On Thu, Sep 24, 2015 at 01:52:46PM +0530, Bharata B Rao wrote:
> HTAB size is a factor of maximum memory size that is specified by maxmem=
> command line option. In cases where there is shortage of host memory, host
> will not be able to allocate contiguous memory for guest HTAB and will
> instead allocate a smaller HTAB. This usually is not a problem but when
> user starts hotplugging memory to the guest, we can run out of HTAB entries
> and hence memory hotplug fails. This failure should have been handled
> gracefully by the guest kernel, but currently it leads to guest kernel OOPS.
> This will eventually get fixed when the handling of memory hotplug is
> completely moved to kernel for PowerKVM.
> 
> Prevent such kernel failure by refusing to boot the guest when requested
> HTAB size can't be allocated. However HTAB allocation happens in the
> reset path from where it is too late to abort. Hence this patchset
> moves the HTAB allocation to machine init and aborts if HTAB size
> requirement isn't met.
> 
> This patchset applies against David Gibson's spapr-next. With this
> patchset, simple boot, reboot and migration tests pass with HV KVM guest.
> For PR KVM guest, boot and reboot tests were done since migration already
> appears broken for PR KVM.

This looks good.  Applied to spapr-next

-- 
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: Type: application/pgp-signature, Size: 819 bytes --]