arch, x86: int80 -- Clobber all rX registers

Submitted by Cyrill Gorcunov on Jan. 15, 2019, 11:07 a.m.

Details

Message ID 20190115110713.4655-1-gorcunov@gmail.com
State New
Series "arch, x86: int80 -- Clobber all rX registers"
Headers show

Commit Message

Cyrill Gorcunov Jan. 15, 2019, 11:07 a.m.
While vanilla kernel requires only r8-r11 to be saved
by a caller when doing int80 interrupt we've discovered
some buggy kernels which may not follow the rule, thus
to be able to checkpoint programs on such kernels we
mark all rX registers as clobbered to be on a safe side.

CC: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
CC: Andrew Vagin <avagin@virtuozzo.com>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---

Andrew, ptikhomirov@ has to carry this patch to run our
criu-dev tests on Vz7 kernel. I think it is safe to merge
it upstream.

 compel/arch/x86/src/lib/include/uapi/asm/sigframe.h | 3 ++-
 criu/arch/x86/include/asm/compat.h                  | 2 +-
 criu/arch/x86/include/asm/parasite.h                | 3 ++-
 criu/arch/x86/restorer.c                            | 3 ++-
 4 files changed, 7 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/compel/arch/x86/src/lib/include/uapi/asm/sigframe.h b/compel/arch/x86/src/lib/include/uapi/asm/sigframe.h
index c29de3bd5876..385ff716c5f1 100644
--- a/compel/arch/x86/src/lib/include/uapi/asm/sigframe.h
+++ b/compel/arch/x86/src/lib/include/uapi/asm/sigframe.h
@@ -176,7 +176,8 @@  struct rt_sigframe {
 		".code64					\n"	\
 		:							\
 		: "rdi"(new_sp)						\
-		: "eax", "r8", "r9", "r10", "r11", "memory")
+		: "eax", "r8", "r9", "r10", "r11",			\
+			"r12", "r13", "r14", "r15", "memory")
 
 #define ARCH_RT_SIGRETURN(new_sp, rt_sigframe)				\
 do {									\
diff --git a/criu/arch/x86/include/asm/compat.h b/criu/arch/x86/include/asm/compat.h
index cd1ae472d77e..d0a3b3706222 100644
--- a/criu/arch/x86/include/asm/compat.h
+++ b/criu/arch/x86/include/asm/compat.h
@@ -57,7 +57,7 @@  static inline void do_full_int80(struct syscall_args32 *args)
 		      : "+a" (args->nr),
 			"+b" (args->arg0), "+c" (args->arg1), "+d" (args->arg2),
 			"+S" (args->arg3), "+D" (args->arg4), "+g" (args->arg5)
-			: : "r8", "r9", "r10", "r11");
+			: : "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15");
 }
 
 #ifndef CR_NOGLIBC
diff --git a/criu/arch/x86/include/asm/parasite.h b/criu/arch/x86/include/asm/parasite.h
index 0ef1d9a867bb..43c005026e79 100644
--- a/criu/arch/x86/include/asm/parasite.h
+++ b/criu/arch/x86/include/asm/parasite.h
@@ -34,7 +34,8 @@  static int arch_get_user_desc(user_desc_t *desc)
 	"	mov %%eax,%0			\n"
 	: "+m"(ret)
 	: "m"(desc)
-	: "rax", "rbx", "r8", "r9", "r10", "r11", "memory");
+	: "rax", "rbx", "r8", "r9", "r10", "r11",
+		"r12", "r13", "r14", "r15", "memory");
 
 	if (ret)
 		pr_err("Failed to dump TLS descriptor #%d: %d\n",
diff --git a/criu/arch/x86/restorer.c b/criu/arch/x86/restorer.c
index 2d335d5e1d29..ffee08a82d61 100644
--- a/criu/arch/x86/restorer.c
+++ b/criu/arch/x86/restorer.c
@@ -103,7 +103,8 @@  void restore_tls(tls_t *ptls)
 		"	mov %%eax,%0			\n"
 		: "=g"(ret)
 		: "r"(__NR32_set_thread_area), "r"((uint32_t)(uintptr_t)stack32)
-		: "eax", "ebx", "r8", "r9", "r10", "r11", "memory");
+		: "eax", "ebx", "r8", "r9", "r10", "r11",
+			"r12", "r13", "r14", "r15", "memory");
 
 		if (ret)
 			pr_err("Failed to restore TLS descriptor %u in GDT: %d\n",

Comments

Cyrill Gorcunov Jan. 15, 2019, 11:08 a.m.
On Tue, Jan 15, 2019 at 02:07:13PM +0300, Cyrill Gorcunov wrote:
> While vanilla kernel requires only r8-r11 to be saved
> by a caller when doing int80 interrupt we've discovered
> some buggy kernels which may not follow the rule, thus
> to be able to checkpoint programs on such kernels we
> mark all rX registers as clobbered to be on a safe side.
> 
> CC: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> CC: Andrew Vagin <avagin@virtuozzo.com>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>

Just noticed I'm using your old vz email adress, sorry.
Dmitry Safonov Jan. 15, 2019, 2:06 p.m.
On Tue, 15 Jan 2019 at 11:08, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
>
> While vanilla kernel requires only r8-r11 to be saved
> by a caller when doing int80 interrupt we've discovered
> some buggy kernels which may not follow the rule, thus
> to be able to checkpoint programs on such kernels we
> mark all rX registers as clobbered to be on a safe side.

Some more details would be appreciated.
What kind of kernels is it about?
Is there a redhat patch that changes this ABI?
Worth a reference?

It's not that I'm against - probably performance penalty on
mainstream from pushing a couple of registers more on the stack
apriori syscalls is small(?), but the commit message could
be improved.
Cyrill Gorcunov Jan. 15, 2019, 2:08 p.m.
On Tue, Jan 15, 2019 at 02:06:30PM +0000, Dmitry Safonov wrote:
> 
> Some more details would be appreciated.
> What kind of kernels is it about?
> Is there a redhat patch that changes this ABI?
> Worth a reference?
> 
> It's not that I'm against - probably performance penalty on
> mainstream from pushing a couple of registers more on the stack
> apriori syscalls is small(?), but the commit message could
> be improved.

Pasha has to apply it, so it is vz kernel I think. Pasha?
Pavel Tikhomirov Jan. 15, 2019, 2:49 p.m.
On 1/15/19 5:08 PM, Cyrill Gorcunov wrote:
> On Tue, Jan 15, 2019 at 02:06:30PM +0000, Dmitry Safonov wrote:
>>
>> Some more details would be appreciated.
>> What kind of kernels is it about?
>> Is there a redhat patch that changes this ABI?
>> Worth a reference?
>>
>> It's not that I'm against - probably performance penalty on
>> mainstream from pushing a couple of registers more on the stack
>> apriori syscalls is small(?), but the commit message could
>> be improved.
> 
> Pasha has to apply it, so it is vz kernel I think. Pasha?
> 

I saw these problem running criu-dev zdtm on VZ7 kernel (it was year ago 
so it's hard to find exact version), it showed up as:

========================== Run zdtm/static/env00 in h 
==========================
Start test
./env00 --pidfile=env00.pid --outfile=env00.out --envname=ENV_00_TEST
Run criu dump
=[log]=> dump/zdtm/static/env00/36/1/dump.log
------------------------ grep Error ------------------------
(00.006926) Obtaining personality ...
(00.006953) Sent msg to daemon 64 0 0
(00.006966) Wait for ack 64 on daemon socket
pie: 36: __fetched msg: 64 0 0
(00.007040) Error (criu/parasite-syscall.c:92): si_code=4 si_pid=36 
si_status=11
(00.007052) Error (criu/parasite-syscall.c:100): 36 was stopped by 11 
unexpectedly
------------------------ ERROR OVER ------------------------
################### Test zdtm/static/env00 FAIL at CRIU dump 
###################

And these patch applied on top of criu-dev unblocked the test.

Will try to run without these patch to check if it is still relevant.
Pavel Tikhomirov Jan. 15, 2019, 4:01 p.m.
On 1/15/19 5:49 PM, Pavel Tikhomirov wrote:
> On 1/15/19 5:08 PM, Cyrill Gorcunov wrote:
>> On Tue, Jan 15, 2019 at 02:06:30PM +0000, Dmitry Safonov wrote:
>>>
>>> Some more details would be appreciated.
>>> What kind of kernels is it about?
>>> Is there a redhat patch that changes this ABI?
>>> Worth a reference?
>>>
>>> It's not that I'm against - probably performance penalty on
>>> mainstream from pushing a couple of registers more on the stack
>>> apriori syscalls is small(?), but the commit message could
>>> be improved.
>>
>> Pasha has to apply it, so it is vz kernel I think. Pasha?
>>
> 
> I saw these problem running criu-dev zdtm on VZ7 kernel (it was year ago 
> so it's hard to find exact version), it showed up as:
> 
> ========================== Run zdtm/static/env00 in h 
> ==========================
> Start test
> ./env00 --pidfile=env00.pid --outfile=env00.out --envname=ENV_00_TEST
> Run criu dump
> =[log]=> dump/zdtm/static/env00/36/1/dump.log
> ------------------------ grep Error ------------------------
> (00.006926) Obtaining personality ...
> (00.006953) Sent msg to daemon 64 0 0
> (00.006966) Wait for ack 64 on daemon socket
> pie: 36: __fetched msg: 64 0 0
> (00.007040) Error (criu/parasite-syscall.c:92): si_code=4 si_pid=36 
> si_status=11
> (00.007052) Error (criu/parasite-syscall.c:100): 36 was stopped by 11 
> unexpectedly
> ------------------------ ERROR OVER ------------------------
> ################### Test zdtm/static/env00 FAIL at CRIU dump 
> ###################
> 
> And these patch applied on top of criu-dev unblocked the test.
> 
> Will try to run without these patch to check if it is still relevant.

On latest VZ7 kernel without the patch tests passes with no error: 
https://ci.openvz.org/job/CRIU/job/CRIU-virtuozzo/job/criu-dev/4631/consoleFull
Cyrill Gorcunov Jan. 15, 2019, 4:04 p.m.
On Tue, Jan 15, 2019 at 04:01:09PM +0000, Pavel Tikhomirov wrote:
...
> > 
> > Will try to run without these patch to check if it is still relevant.
> 
> On latest VZ7 kernel without the patch tests passes with no error: 
> https://ci.openvz.org/job/CRIU/job/CRIU-virtuozzo/job/criu-dev/4631/consoleFull

OK then, so ignore the patch.