From mboxrd@z Thu Jan  1 00:00:00 1970
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH v2 1/2] kvm-unit-tests : Add setjmp/longjmp to libcflat
Date: Tue, 16 Jul 2013 17:19:43 +0200
Message-ID: <51E5648F.6050307@redhat.com>
References: <1373987466-2216-1-git-send-email-yzt356@gmail.com> <1373987466-2216-2-git-send-email-yzt356@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org, jan.kiszka@web.de, gleb@redhat.com
To: Arthur Chunqi Li <yzt356@gmail.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:29186 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754902Ab3GPPT5 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 16 Jul 2013 11:19:57 -0400
In-Reply-To: <1373987466-2216-2-git-send-email-yzt356@gmail.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Il 16/07/2013 17:11, Arthur Chunqi Li ha scritto:
> Add setjmp and longjmp functions to libcflat. Now these two functions
> are only supported in X86_64 arch.
> 
> New files added:
> 	lib/x86/setjmp64.S
> 	lib/x86/setjmp64.c
> 
> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> ---
>  config-x86_64.mak  |    2 ++
>  lib/x86/setjmp64.S |   27 +++++++++++++++++++++++++++
>  lib/x86/setjmp64.h |   15 +++++++++++++++
>  3 files changed, 44 insertions(+)
>  create mode 100644 lib/x86/setjmp64.S
>  create mode 100644 lib/x86/setjmp64.h
> 
> diff --git a/config-x86_64.mak b/config-x86_64.mak
> index 4e525f5..91ffcce 100644
> --- a/config-x86_64.mak
> +++ b/config-x86_64.mak
> @@ -4,6 +4,8 @@ bits = 64
>  ldarch = elf64-x86-64
>  CFLAGS += -D__x86_64__
>  
> +cflatobjs += lib/x86/setjmp64.o
> +
>  tests = $(TEST_DIR)/access.flat $(TEST_DIR)/apic.flat \
>  	  $(TEST_DIR)/emulator.flat $(TEST_DIR)/idt_test.flat \
>  	  $(TEST_DIR)/xsave.flat $(TEST_DIR)/rmap_chain.flat \
> diff --git a/lib/x86/setjmp64.S b/lib/x86/setjmp64.S
> new file mode 100644
> index 0000000..c8ae790
> --- /dev/null
> +++ b/lib/x86/setjmp64.S
> @@ -0,0 +1,27 @@
> +.globl setjmp
> +setjmp:
> +	mov (%rsp), %rsi
> +	mov %rsi, (%rdi)
> +	mov %rsp, 0x8(%rdi)
> +	mov %rbp, 0x10(%rdi)
> +	mov %rbx, 0x18(%rdi)
> +	mov %r12, 0x20(%rdi)
> +	mov %r13, 0x28(%rdi)
> +	mov %r14, 0x30(%rdi)
> +	mov %r15, 0x38(%rdi)
> +	xor %eax, %eax
> +	ret
> +
> +.globl longjmp
> +longjmp:
> +	mov %esi, %eax
> +	mov 0x38(%rdi), %r15
> +	mov 0x30(%rdi), %r14
> +	mov 0x28(%rdi), %r13
> +	mov 0x20(%rdi), %r12
> +	mov 0x18(%rdi), %rbx
> +	mov 0x10(%rdi), %rbp
> +	mov 0x8(%rdi), %rsp
> +	mov (%rdi), %rsi
> +	mov %rsi, (%rsp)
> +	ret

Nice!

> diff --git a/lib/x86/setjmp64.h b/lib/x86/setjmp64.h
> new file mode 100644
> index 0000000..98ff16a
> --- /dev/null
> +++ b/lib/x86/setjmp64.h
> @@ -0,0 +1,15 @@
> +#ifndef LIBCFLAT_SETJMP64_H
> +#define LIBCFLAT_SETJMP64_H
> +
> +#include "libcflat.h"
> +
> +struct jmp_buf {
> +	u64 calling_rip;
> +	u64 rsp, rbp, rbx;
> +	u64 r12, r13, r14, r15;
> +};

Please make it a typedef instead.  In fact, jmp_buf is typically an
array so that longjmp(env, 1) works without the ampersand.  A
"struct-like" declaration can go in setjmp64.S as a comment.

With this change, you can put it in lib/setjmp.h so that implementation
for other arches can be added later.

Otherwise looks good, thanks!

Paolo

> +void longjmp(struct jmp_buf *env, int val);
> +int setjmp(struct jmp_buf *env);
> +
> +#endif
>