[CRIU] [PATCH 3/3] compel: Save thread registers before executing parasite code

Alice Frosi alice at linux.vnet.ibm.com
Thu Aug 10 19:04:52 MSK 2017


For dumping threads we execute parasite code before we collect the
floating point registers with get_task_regs().

The following describes the code path were this is done:

 dump_task_threads()
     for (i = 0; i < item->nr_threads; i++)
         dump_task_thread()
           parasite_dump_thread_seized()
             compel_prepare_thread()
                prepare_thread()

                   ### Get general purpose registers ###
                   ptrace_get_regs(ctx->regs)

             ### parasite code is executed in thread ###
             compel_run_in_thread(tctl, PARASITE_CMD_DUMP_THREAD)

             compel_get_thread_regs()

                 ### Get FP and VX registers ###
                 get_task_regs()

Since on s390 gcc uses floating point registers to cache general
purpose without the -msoft-float option the floating point
registers would have been clobbered after compel_run_in_thread().

With this patch we first save all thread registers with task_get_regs() and
then call the parasite code.

We introduce a new function arch_set_thread_regs() that restores the saved
registers for all threads. This is necessary for criu dump --leave-running,
--check-only, and error handling.

Pre-dump does not require to save the registers for the threads because
because only the main thread (task) is used for parsite code execution.

The above changes allow us to use all register sets in the parasite
code - therefore we can remove -msoft-float on s390.

Reviewed-by: Michael Holzheu <holzheu at linux.vnet.ibm.com>
Signed-off-by: Alice Frosi <alice at linux.vnet.ibm.com>
---
 Makefile                             |   8 +--
 compel/arch/s390/src/lib/infect.c    |  26 ---------
 criu/arch/s390/crtools.c             | 109 +++++++++++++++++++++++++++++++++++
 criu/arch/s390/include/asm/restore.h |   2 -
 criu/cr-dump.c                       |  10 ++++
 criu/parasite-syscall.c              |  11 ++--
 6 files changed, 126 insertions(+), 40 deletions(-)

diff --git a/Makefile b/Makefile
index 80b9af8..796cad8 100644
--- a/Makefile
+++ b/Makefile
@@ -68,11 +68,7 @@ endif
 #
 # CFLAGS_PIE:
 #
-# We assume that compel code does not change floating point registers.
-# On s390 gcc uses fprs to cache gprs. Therefore disable floating point
-# with -msoft-float.
-#
-# Also ensure with -fno-optimize-sibling-calls that we don't create GOT
+# Ensure with -fno-optimize-sibling-calls that we don't create GOT
 # (Global Offset Table) relocations with gcc compilers that don't have
 # commit "S/390: Fix 64 bit sibcall".
 ifeq ($(ARCH),s390)
@@ -80,7 +76,7 @@ ifeq ($(ARCH),s390)
         SRCARCH		:= s390
         VDSO		:= y
         DEFINES		:= -DCONFIG_S390
-        CFLAGS_PIE	:= -msoft-float -fno-optimize-sibling-calls
+        CFLAGS_PIE	:= -fno-optimize-sibling-calls
 endif
 export CFLAGS_PIE
 
diff --git a/compel/arch/s390/src/lib/infect.c b/compel/arch/s390/src/lib/infect.c
index 1614fa6..3c1fff6 100644
--- a/compel/arch/s390/src/lib/infect.c
+++ b/compel/arch/s390/src/lib/infect.c
@@ -148,32 +148,6 @@ int get_vx_regs(pid_t pid, user_fpregs_struct_t *fpregs)
 	return 0;
 }
 
-/*
- * Set vector registers
- */
-int set_vx_regs(pid_t pid, user_fpregs_struct_t *fpregs)
-{
-	struct iovec iov;
-	int rc;
-
-	if (!(fpregs->flags & USER_FPREGS_VXRS))
-		return 0;
-
-	iov.iov_base = &fpregs->vxrs_low;
-	iov.iov_len = sizeof(fpregs->vxrs_low);
-	rc = ptrace(PTRACE_SETREGSET, pid, NT_S390_VXRS_LOW, &iov);
-	if (rc) {
-		pr_perror("Couldn't set VXRS_LOW registers\n");
-		return rc;
-	}
-
-	iov.iov_base = &fpregs->vxrs_high;
-	iov.iov_len = sizeof(fpregs->vxrs_high);
-	rc = ptrace(PTRACE_SETREGSET, pid, NT_S390_VXRS_HIGH, &iov);
-	if (rc)
-		pr_perror("Couldn't set VXRS_HIGH registers\n");
-	return rc;
-}
 
 /*
  * Prepare task registers for restart
diff --git a/criu/arch/s390/crtools.c b/criu/arch/s390/crtools.c
index 4bd21ec..cfab508 100644
--- a/criu/arch/s390/crtools.c
+++ b/criu/arch/s390/crtools.c
@@ -22,6 +22,12 @@
 #include "protobuf.h"
 #include "images/core.pb-c.h"
 #include "images/creds.pb-c.h"
+#include "ptrace.h"
+#include "pstree.h"
+
+#define NT_PRFPREG		2
+#define NT_S390_VXRS_LOW	0x309
+#define NT_S390_VXRS_HIGH	0x30a
 
 /*
  * Print general purpose and access registers
@@ -339,3 +345,106 @@ void arch_free_thread_info(CoreEntry *core)
 	xfree(CORE_THREAD_ARCH_INFO(core));
 	CORE_THREAD_ARCH_INFO(core) = NULL;
 }
+
+/*
+ * Set regset for pid
+ */
+static int setregset(int pid, int set, const char *set_str, struct iovec *iov)
+{
+	if (ptrace(PTRACE_SETREGSET, pid, set, iov) == 0)
+		return 0;
+	pr_perror("Couldn't set %s registers for pid %d", set_str, pid);
+	return -1;
+}
+
+/*
+ * Set floating point registers for pid from fpregs
+ */
+static int set_fp_regs(pid_t pid, user_fpregs_struct_t *fpregs)
+{
+	struct iovec iov;
+
+	iov.iov_base = &fpregs->prfpreg;
+	iov.iov_len = sizeof(fpregs->prfpreg);
+	return setregset(pid, NT_PRFPREG, "PRFPREG", &iov);
+}
+
+/*
+ * Set vector registers
+ */
+static int set_vx_regs(pid_t pid, user_fpregs_struct_t *fpregs)
+{
+	struct iovec iov;
+
+	if (!(fpregs->flags & USER_FPREGS_VXRS))
+		return 0;
+
+	iov.iov_base = &fpregs->vxrs_low;
+	iov.iov_len = sizeof(fpregs->vxrs_low);
+	if (setregset(pid, NT_S390_VXRS_LOW, "S390_VXRS_LOW", &iov))
+		return -1;
+
+	iov.iov_base = &fpregs->vxrs_high;
+	iov.iov_len = sizeof(fpregs->vxrs_high);
+	return setregset(pid, NT_S390_VXRS_HIGH, "S390_VXRS_HIGH", &iov);
+}
+
+/*
+ * Restore registers for pid from core
+ */
+static int set_task_regs(pid_t pid, CoreEntry *core)
+{
+	UserS390VxrsHighEntry *cvxrs_high;
+	UserS390VxrsLowEntry *cvxrs_low;
+	UserS390FpregsEntry *cfpregs;
+	user_fpregs_struct_t fpregs;
+
+	memset(&fpregs, 0, sizeof(fpregs));
+	/* Floating point registers */
+	cfpregs = CORE_THREAD_ARCH_INFO(core)->fpregs;
+	if (!cfpregs)
+		return -1;
+	fpregs.prfpreg.fpc = cfpregs->fpc;
+	memcpy(fpregs.prfpreg.fprs, cfpregs->fprs, sizeof(fpregs.prfpreg.fprs));
+	if (set_fp_regs(pid, &fpregs) < 0)
+		return -1;
+	/* Vector registers (optional) */
+	cvxrs_low = CORE_THREAD_ARCH_INFO(core)->vxrs_low;
+	if (!cvxrs_low)
+		return 0;
+	cvxrs_high = CORE_THREAD_ARCH_INFO(core)->vxrs_high;
+	if (!cvxrs_high)
+		return -1;
+	fpregs.flags |= USER_FPREGS_VXRS;
+	memcpy(&fpregs.vxrs_low, cvxrs_low->regs, sizeof(fpregs.vxrs_low));
+	memcpy(&fpregs.vxrs_high, cvxrs_high->regs, sizeof(fpregs.vxrs_high));
+
+	return set_vx_regs(pid, &fpregs);
+}
+
+/*
+ * Restore vector and floating point registers for all threads
+ */
+int arch_set_thread_regs(struct pstree_item *item)
+{
+	int i;
+
+	for_each_pstree_item(item) {
+		if (item->pid->state == TASK_DEAD ||
+		    item->pid->state == TASK_ZOMBIE ||
+		    item->pid->state == TASK_HELPER)
+			continue;
+		for (i = 0; i < item->nr_threads; i++) {
+			if (item->threads[i]->state == TASK_DEAD ||
+			    item->threads[i]->state == TASK_ZOMBIE)
+				continue;
+			if (set_task_regs(item->threads[i]->real,
+					  item->core[i])) {
+				pr_perror("Not set registers for task %d",
+					  item->threads[i]->real);
+				return -1;
+			}
+		}
+	}
+	return 0;
+}
diff --git a/criu/arch/s390/include/asm/restore.h b/criu/arch/s390/include/asm/restore.h
index 96358ff..6463d8e 100644
--- a/criu/arch/s390/include/asm/restore.h
+++ b/criu/arch/s390/include/asm/restore.h
@@ -4,7 +4,6 @@
 #include "asm/restorer.h"
 
 #include "images/core.pb-c.h"
-
 /*
  * Load stack to %r15, return address in %r14 and argument 1 into %r2
  */
@@ -25,5 +24,4 @@
 #define core_get_tls(pcore, ptls)
 
 int restore_fpu(struct rt_sigframe *sigframe, CoreEntry *core);
-
 #endif
diff --git a/criu/cr-dump.c b/criu/cr-dump.c
index 06f3966..b730504 100644
--- a/criu/cr-dump.c
+++ b/criu/cr-dump.c
@@ -84,6 +84,15 @@
 #include "dump.h"
 #include "img-remote.h"
 
+/*
+ * Architectures can overwrite this function to restore register sets that
+ * are not covered by ptrace_set/get_regs().
+ */
+int __attribute__((weak)) arch_set_thread_regs(struct pstree_item *item)
+{
+	return 0;
+}
+
 static char loc_buf[PAGE_SIZE];
 
 void free_mappings(struct vm_area_list *vma_area_list)
@@ -1779,6 +1788,7 @@ static int cr_dump_finish(int ret)
 	if (!ret && opts.lazy_pages)
 		ret = cr_lazy_mem_dump();
 
+	arch_set_thread_regs(root_item);
 	pstree_switch_state(root_item,
 			    (ret || post_dump_ret) ?
 			    TASK_ALIVE : opts.final_state);
diff --git a/criu/parasite-syscall.c b/criu/parasite-syscall.c
index 89ec06f..2650d76 100644
--- a/criu/parasite-syscall.c
+++ b/criu/parasite-syscall.c
@@ -198,6 +198,11 @@ int parasite_dump_thread_seized(struct parasite_ctl *ctl, const struct pstree_it
 
 	tc->has_blk_sigset = true;
 	memcpy(&tc->blk_sigset, compel_thread_sigmask(tctl), sizeof(k_rtsigset_t));
+	ret = compel_get_thread_regs(tctl, save_task_regs, core);
+	if (ret) {
+		pr_err("Can't obtain regs for thread %d\n", pid);
+		goto err_rth;
+	}
 
 	ret = compel_run_in_thread(tctl, PARASITE_CMD_DUMP_THREAD);
 	if (ret) {
@@ -211,12 +216,6 @@ int parasite_dump_thread_seized(struct parasite_ctl *ctl, const struct pstree_it
 		goto err_rth;
 	}
 
-	ret = compel_get_thread_regs(tctl, save_task_regs, core);
-	if (ret) {
-		pr_err("Can't obtain regs for thread %d\n", pid);
-		goto err_rth;
-	}
-
 	compel_release_thread(tctl);
 
 	*parasite_tid = args->tid;
-- 
2.9.3



More information about the CRIU mailing list