[CRIU] [PATCH 14/22] restorer: introduced the multiarch support into the restorer.

[Alexander Kartashov]

On 12/28/2012 12:57 PM, Pavel Emelyanov wrote:
> On 12/27/2012 10:51 AM, Alexander Kartashov wrote:
>> * Introduced the macro RT_SIGFRAME_UC to access the user context of a sigframe.
>>
>> * Introduced TLS restoration for ARM.
>>
>> * Don't touch a VMA that lies outside the virtual address space area 0..TASK_SIZE,
>>    because VM above TASK_SIZE is read-only but some areas are mapped on ARM
>>    into the process address space.
>>
>> * Introduced the macro SIGFRAME_OFFSET to tune the beginning of a sigframe
>>    on different architectures.
>>
>> Signed-off-by: Alexander Kartashov <alekskartashov at parallels.com>
>> ---
>>   arch/x86/include/asm/restorer.h |    4 ++++
>>   include/restorer.h              |   14 +++++++++-----
>>   pie/restorer.c                  |   23 +++++++++++++++++++----
>>   3 files changed, 32 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/restorer.h b/arch/x86/include/asm/restorer.h
>> index 1ccc93c..4518064 100644
>> --- a/arch/x86/include/asm/restorer.h
>> +++ b/arch/x86/include/asm/restorer.h
>> @@ -129,9 +129,13 @@ struct rt_sigframe {
>>   		     : "r"(ret)					\
>>   		     : "memory")
>>   
>> +#define RT_SIGFRAME_UC(rt_sigframe) rt_sigframe->uc
>> +
>>   
>>   int restore_gpregs(struct rt_sigframe *f, UserX86RegsEntry *r);
>>   
>>   int restore_fpu(struct rt_sigframe *sigframe, struct thread_restore_args *args);
>>   
>> +static void __attribute__ ((unused)) restore_tls(u32 tls) { }
> Why unused?
This file may be included into the source that doesn't use this function
that results in a compiler warning.
>
>> +
>>   #endif
>> diff --git a/include/restorer.h b/include/restorer.h
>> index 7548ae1..3bb40b9 100644
>> --- a/include/restorer.h
>> +++ b/include/restorer.h
>> @@ -17,10 +17,6 @@
>>   #include "../protobuf/creds.pb-c.h"
>>   #include "../protobuf/core.pb-c.h"
>>   
>> -#ifndef CONFIG_X86_64
>> -# error Only x86-64 is supported
>> -#endif
>> -
>>   struct task_restore_core_args;
>>   struct thread_restore_args;
>>   
>> @@ -67,7 +63,10 @@ struct thread_restore_args {
>>   	struct restore_mem_zone		mem_zone;
>>   
>>   	int				pid;
>> -	UserX86RegsEntry		gpregs;
>> +	UserRegsEntry		        gpregs;
>> +
>> +	u32				tls;
>> +
>>   	u64				clear_tid_addr;
>>   
>>   	bool				has_futex;
>> @@ -126,6 +125,7 @@ struct task_restore_core_args {
>>   	u64				mm_saved_auxv[AT_VECTOR_SIZE];
>>   	u32				mm_saved_auxv_size;
>>   	char				comm[TASK_COMM_LEN];
>> +	uint32_t                        tls;
> Master thread's tls should be on one of the thread_restore_args.
>
>>   
>>   	int				*rst_tcp_socks;
>>   	int				rst_tcp_socks_size;
>> @@ -199,4 +199,8 @@ find_shmem(struct shmems *shmems, unsigned long shmid)
>>   			(vma_entry_is(vma, VMA_ANON_PRIVATE) || \
>>   			vma_entry_is(vma, VMA_FILE_PRIVATE)))
>>   
>> +#define SIGFRAME_OFFSET 8
>> +
>> +#include "asm/memcpy_64.h"
>> +
>>   #endif /* __CR_RESTORER_H__ */
>> diff --git a/pie/restorer.c b/pie/restorer.c
>> index 04e7ab5..a9b4448 100644
>> --- a/pie/restorer.c
>> +++ b/pie/restorer.c
>> @@ -156,7 +156,7 @@ static int restore_thread_common(struct rt_sigframe *sigframe,
>>   	}
>>   
>>   	if (args->has_blk_sigset)
>> -		sigframe->uc.uc_sigmask.sig[0] = args->blk_sigset;
>> +		RT_SIGFRAME_UC(sigframe).uc_sigmask.sig[0] = args->blk_sigset;
>>   
>>   	restore_sched_info(&args->sp);
>>   	if (restore_fpu(sigframe, args))
>> @@ -189,6 +189,7 @@ long __export_restore_thread(struct thread_restore_args *args)
>>   
>>   	restore_creds(&args->ta->creds);
>>   
>> +	restore_tls(args->tls);
>>   
>>   	pr_info("%ld: Restored\n", sys_gettid());
>>   
>> @@ -197,7 +198,7 @@ long __export_restore_thread(struct thread_restore_args *args)
>>   
>>   	futex_dec_and_wake(&thread_inprogress);
>>   
>> -	new_sp = (long)rt_sigframe + 8;
>> +	new_sp = (long)rt_sigframe + SIGFRAME_OFFSET;
>>   	ARCH_RT_SIGRETURN(new_sp);
>>   
>>   core_restore_end:
>> @@ -399,6 +400,10 @@ long __export_restore_task(struct task_restore_core_args *args)
>>   			}
>>   		}
>>   
>> +		if (vma_entry->end >= TASK_SIZE) {
>> +			continue;
>> +		}
>> +
>>   		if (vma_entry->end > premmapped_end) {
>>   			if (vma_entry->start < premmapped_end)
>>   				addr = premmapped_end;
>> @@ -421,6 +426,10 @@ long __export_restore_task(struct task_restore_core_args *args)
>>   		if (!vma_priv(vma_entry))
>>   			continue;
>>   
>> +		if (vma_entry->end >= TASK_SIZE) {
>> +			continue;
>> +		}
>> +
>>   		if (vma_entry->start > vma_entry->shmid)
>>   			break;
>>   
>> @@ -438,6 +447,10 @@ long __export_restore_task(struct task_restore_core_args *args)
>>   		if (!vma_priv(vma_entry))
>>   			continue;
>>   
>> +		if (vma_entry->start > TASK_SIZE) {
>> +			continue;
>> +		}
>> +
>>   		if (vma_entry->start < vma_entry->shmid)
>>   			break;
>>   
>> @@ -684,14 +697,16 @@ long __export_restore_task(struct task_restore_core_args *args)
>>   
>>   	ret = sys_munmap(args->task_entries, TASK_ENTRIES_SIZE);
>>   	if (ret < 0) {
>> -		ret = ((long)__LINE__ << 32) | -ret;
>> +		ret = ((long)__LINE__ << 16) | ((-ret) & 0xffff);
> This hunk is not described in patch comment.
This is an eternal problem of integer types mismatch. long is 32-bit 
long in ARM
that results in a compiler warning here. So I had to reduce the shift value.
Is it better to make ret 64 bit long and specify it's type explicitly (u64)?
>
>>   		goto core_restore_failed;
>>   	}
>>   
>> +	restore_tls(args->tls);
>> +
>>   	/*
>>   	 * Sigframe stack.
>>   	 */
>> -	new_sp = (long)rt_sigframe + 8;
>> +	new_sp = (long)rt_sigframe + SIGFRAME_OFFSET;
>>   
>>   	/*
>>   	 * Prepare the stack and call for sigreturn,
>>
>


-- 
Sincerely yours,
Alexander Kartashov

Intern
Core team

www.parallels.com

Skype: aleksandr.kartashov
Email: alekskartashov at parallels.com