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

[Pavel Emelyanov]

On 12/28/2012 01:07 PM, Alexander Kartashov wrote:
> 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.

This declare it as static inline.

>>
>>> +
>>>   #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)?

No, just describe this in comment.

>>
>>>   		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,
>>>
>>
> 
>