qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: benh@kernel.crashing.org, aik@ozlabs.ru, agraf@suse.de
Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org,
	David Gibson <david@gibson.dropbear.id.au>
Subject: [Qemu-devel] [PATCHv2 3/7] pseries: Simplify handling of the hash page table fd
Date: Tue,  9 Feb 2016 12:12:22 +1000	[thread overview]
Message-ID: <1454983946-32073-4-git-send-email-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <1454983946-32073-1-git-send-email-david@gibson.dropbear.id.au>

When migrating the 'pseries' machine type with KVM, we use a special fd
to access the hash page table stored within KVM.  Usually, this fd is
opened at the beginning of migration, and kept open until the migration
is complete.

However, if there is a guest reset during the migration, the fd can become
stale and we need to re-open it.  At the moment we use an 'htab_fd_stale'
flag in sPAPRMachineState to signal this, which is checked in the migration
iterators.

But that's rather ugly.  It's simpler to just close and invalidate the
fd on reset, and lazily re-open it in migration if necessary.  This patch
implements that change.

This requires a small addition to the machine state's instance_init,
so that htab_fd is initialized to -1 (telling the migration code it
needs to open it) instead of 0, which could be a valid fd.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr.c         | 86 ++++++++++++++++++++++++--------------------------
 include/hw/ppc/spapr.h |  1 -
 2 files changed, 41 insertions(+), 46 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5bd8fd3..c315715 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1024,6 +1024,32 @@ 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))
 
+/*
+ * Get the fd to access the kernel htab, re-opening it if necessary
+ */
+static int get_htab_fd(sPAPRMachineState *spapr)
+{
+    if (spapr->htab_fd >= 0) {
+        return spapr->htab_fd;
+    }
+
+    spapr->htab_fd = kvmppc_get_htab_fd(false);
+    if (spapr->htab_fd < 0) {
+        error_report("Unable to open fd for reading hash table from KVM: %s",
+                     strerror(errno));
+    }
+
+    return spapr->htab_fd;
+}
+
+static void close_htab_fd(sPAPRMachineState *spapr)
+{
+    if (spapr->htab_fd >= 0) {
+        close(spapr->htab_fd);
+    }
+    spapr->htab_fd = -1;
+}
+
 static void spapr_alloc_htab(sPAPRMachineState *spapr)
 {
     long shift;
@@ -1085,10 +1111,7 @@ static void spapr_reset_htab(sPAPRMachineState *spapr)
             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;
-        }
+        close_htab_fd(spapr);
     } else {
         memset(spapr->htab, 0, HTAB_SIZE(spapr));
 
@@ -1121,28 +1144,6 @@ static int find_unknown_sysbus_device(SysBusDevice *sbdev, void *opaque)
     return 0;
 }
 
-/*
- * A guest reset will cause spapr->htab_fd to become stale if being used.
- * Reopen the file descriptor to make sure the whole HTAB is properly read.
- */
-static int spapr_check_htab_fd(sPAPRMachineState *spapr)
-{
-    int rc = 0;
-
-    if (spapr->htab_fd_stale) {
-        close(spapr->htab_fd);
-        spapr->htab_fd = kvmppc_get_htab_fd(false);
-        if (spapr->htab_fd < 0) {
-            error_report("Unable to open fd for reading hash table from KVM: "
-                         "%s", strerror(errno));
-            rc = -1;
-        }
-        spapr->htab_fd_stale = false;
-    }
-
-    return rc;
-}
-
 static void ppc_spapr_reset(void)
 {
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
@@ -1313,14 +1314,6 @@ static int htab_save_setup(QEMUFile *f, void *opaque)
         spapr->htab_first_pass = true;
     } else {
         assert(kvm_enabled());
-
-        spapr->htab_fd = kvmppc_get_htab_fd(false);
-        spapr->htab_fd_stale = false;
-        if (spapr->htab_fd < 0) {
-            fprintf(stderr, "Unable to open fd for reading hash table from KVM: %s\n",
-                    strerror(errno));
-            return -1;
-        }
     }
 
 
@@ -1460,6 +1453,7 @@ static int htab_save_later_pass(QEMUFile *f, sPAPRMachineState *spapr,
 static int htab_save_iterate(QEMUFile *f, void *opaque)
 {
     sPAPRMachineState *spapr = opaque;
+    int fd;
     int rc = 0;
 
     /* Iteration header */
@@ -1468,13 +1462,12 @@ static int htab_save_iterate(QEMUFile *f, void *opaque)
     if (!spapr->htab) {
         assert(kvm_enabled());
 
-        rc = spapr_check_htab_fd(spapr);
-        if (rc < 0) {
-            return rc;
+        fd = get_htab_fd(spapr);
+        if (fd < 0) {
+            return fd;
         }
 
-        rc = kvmppc_save_htab(f, spapr->htab_fd,
-                              MAX_KVM_BUF_SIZE, MAX_ITERATION_NS);
+        rc = kvmppc_save_htab(f, fd, MAX_KVM_BUF_SIZE, MAX_ITERATION_NS);
         if (rc < 0) {
             return rc;
         }
@@ -1495,6 +1488,7 @@ static int htab_save_iterate(QEMUFile *f, void *opaque)
 static int htab_save_complete(QEMUFile *f, void *opaque)
 {
     sPAPRMachineState *spapr = opaque;
+    int fd;
 
     /* Iteration header */
     qemu_put_be32(f, 0);
@@ -1504,17 +1498,16 @@ static int htab_save_complete(QEMUFile *f, void *opaque)
 
         assert(kvm_enabled());
 
-        rc = spapr_check_htab_fd(spapr);
-        if (rc < 0) {
-            return rc;
+        fd = get_htab_fd(spapr);
+        if (fd < 0) {
+            return fd;
         }
 
-        rc = kvmppc_save_htab(f, spapr->htab_fd, MAX_KVM_BUF_SIZE, -1);
+        rc = kvmppc_save_htab(f, fd, MAX_KVM_BUF_SIZE, -1);
         if (rc < 0) {
             return rc;
         }
-        close(spapr->htab_fd);
-        spapr->htab_fd = -1;
+        close_htab_fd(spapr);
     } else {
         htab_save_later_pass(f, spapr, -1);
     }
@@ -2125,6 +2118,9 @@ static void spapr_set_kvm_type(Object *obj, const char *value, Error **errp)
 
 static void spapr_machine_initfn(Object *obj)
 {
+    sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
+
+    spapr->htab_fd = -1;
     object_property_add_str(obj, "kvm-type",
                             spapr_get_kvm_type, spapr_set_kvm_type, NULL);
     object_property_set_description(obj, "kvm-type",
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 1f9e722..098d85d 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -72,7 +72,6 @@ struct sPAPRMachineState {
     int htab_save_index;
     bool htab_first_pass;
     int htab_fd;
-    bool htab_fd_stale;
 
     /* RTAS state */
     QTAILQ_HEAD(, sPAPRConfigureConnectorState) ccs_list;
-- 
2.5.0

  parent reply	other threads:[~2016-02-09  2:11 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-09  2:12 [Qemu-devel] [PATCHv2 0/7] Cleanups to Hash Page Table handling David Gibson
2016-02-09  2:12 ` [Qemu-devel] [PATCHv2 1/7] target-ppc: Remove unused kvmppc_update_sdr1() stub David Gibson
2016-02-09  2:12 ` [Qemu-devel] [PATCHv2 2/7] target-ppc: Include missing MMU models for SDR1 in info registers David Gibson
2016-02-09  2:12 ` David Gibson [this message]
2016-02-09  2:12 ` [Qemu-devel] [PATCHv2 4/7] pseries: Add helper to calculate recommended hash page table size David Gibson
2016-02-09  2:12 ` [Qemu-devel] [PATCHv2 5/7] pseries: Move hash page table allocation to reset time David Gibson
2016-02-09  2:59   ` Alexey Kardashevskiy
2016-02-09  2:12 ` [Qemu-devel] [PATCHv2 6/7] target-ppc: Remove hack for ppc_hash64_load_hpte*() with HV KVM David Gibson
2016-02-09  2:12 ` [Qemu-devel] [PATCHv2 7/7] target-ppc: Add helpers for updating a CPU's SDR1 and external HPT David Gibson
2016-02-11  0:51 ` [Qemu-devel] [PATCHv2 0/7] Cleanups to Hash Page Table handling David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1454983946-32073-4-git-send-email-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=agraf@suse.de \
    --cc=aik@ozlabs.ru \
    --cc=benh@kernel.crashing.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).