Re: [kvm-ppc-devel] [PATCH 1/5] Remove KVM_HANDLER macro.

[Hollis Blanchard <hollisb@us.ibm.com>]

On Fri, 2008-01-25 at 15:12 +0800, Zhang Wei wrote:
> Replace KVM_HANDLER macro by kvm_trampoline_handler() function.
> Codes define by KVM_HANDLER marco will be extracted by
> kvm_trampoline_handler install loop.
> 
> Signed-off-by: Zhang Wei <wei.zhang@freescale.com>
> ---
>  arch/powerpc/kvm/booke_interrupts.S |   32 ++++++--------------------------
>  arch/powerpc/kvm/powerpc.c          |    6 +++++-
>  include/asm-powerpc/kvm_host.h      |    1 +
>  3 files changed, 12 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S
> index 45131cc..ec5f071 100644
> --- a/arch/powerpc/kvm/booke_interrupts.S
> +++ b/arch/powerpc/kvm/booke_interrupts.S
> @@ -39,36 +39,19 @@
>  #define HOST_STACK_SIZE (((HOST_MIN_STACK_SIZE + 15) / 16) * 16) /* Align. */
>  #define HOST_STACK_LR   (HOST_STACK_SIZE + 4) /* In caller stack frame. */
> 
> -.macro KVM_HANDLER ivor_nr
> -_GLOBAL(kvm_trampoline_handler_\ivor_nr)
> +_GLOBAL(kvm_trampoline_handler)
>  	/* Get pointer to vcpu and record exit number. */
>  	mtspr	SPRN_SPRG0, r4
>  	mfspr	r4, SPRN_SPRG1
>  	stw	r5, VCPU_GPR(r5)(r4)
> -	li	r5, \ivor_nr
> +	li	r5, 0   /* The SIMM field of this instruction will be replaced
> +                         * at install time by ivor_nr.
> +                         */
>  	/* This branch is fixed up at install time to jump to
>  	 * kvm_trampoline_resume_host(). */
>  	b	.
> -.endm
> -
> -_GLOBAL(kvm_trampoline_start)
> -
> -KVM_HANDLER PPC44x_INTERRUPT_CRITICAL
> -KVM_HANDLER PPC44x_INTERRUPT_MACHINE_CHECK
> -KVM_HANDLER PPC44x_INTERRUPT_DATA_STORAGE
> -KVM_HANDLER PPC44x_INTERRUPT_INST_STORAGE
> -KVM_HANDLER PPC44x_INTERRUPT_EXTERNAL
> -KVM_HANDLER PPC44x_INTERRUPT_ALIGNMENT
> -KVM_HANDLER PPC44x_INTERRUPT_PROGRAM
> -KVM_HANDLER PPC44x_INTERRUPT_FP_UNAVAIL
> -KVM_HANDLER PPC44x_INTERRUPT_SYSCALL
> -KVM_HANDLER PPC44x_INTERRUPT_AP_UNAVAIL
> -KVM_HANDLER PPC44x_INTERRUPT_DECREMENTER
> -KVM_HANDLER PPC44x_INTERRUPT_FIT
> -KVM_HANDLER PPC44x_INTERRUPT_WATCHDOG
> -KVM_HANDLER PPC44x_INTERRUPT_DTLB_MISS
> -KVM_HANDLER PPC44x_INTERRUPT_ITLB_MISS
> -KVM_HANDLER PPC44x_INTERRUPT_DEBUG
> +_GLOBAL(kvm_trampoline_handler_len)
> +	.long	. - kvm_trampoline_handler
> 
>  #define NEED_INST_MASK ((1<<PPC44x_INTERRUPT_PROGRAM) | \
>                          (1<<PPC44x_INTERRUPT_DTLB_MISS))
> @@ -208,9 +191,6 @@ _GLOBAL(kvm_trampoline_resume_guest)
>  _GLOBAL(kvm_trampoline_resume_guest_len)
>  	.long	. - kvm_trampoline_resume_guest
> 
> -_GLOBAL(kvm_trampoline_handler_len)
> -	.long	kvm_trampoline_handler_1 - kvm_trampoline_handler_0
> -
> 
>  /* Registers:
>   * SPRG0: guest r4
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index cbdacd5..0839cfc 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -466,7 +466,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
>  	ivorlist[15] = mfspr(SPRN_IVOR15);
>  	for (i = 0; i < 16; i++) {
>  		memcpy(handlers + ivorlist[i],
> -		       kvm_trampoline_start + i * kvm_trampoline_handler_len,
> +		       &kvm_trampoline_handler,
>  		       kvm_trampoline_handler_len);
>  	}
> 
> @@ -484,9 +484,13 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
>  	/* Manually fix up the handler branches, since we moved the code away
>  	 * from its link address. */
>  	for (i = 0; i < 16; i++) {
> +		unsigned long *lir5;
>  		unsigned long *branch;
> +		lir5 = handlers + ivorlist[i] + kvm_trampoline_handler_len
> +			 - 8;
>  		branch = handlers + ivorlist[i] + kvm_trampoline_handler_len
>  		         - 4;
> +		*lir5 |= i & 0xffff;
>  		*branch |= resume_host - (void *)branch;
>  	}
> 
> diff --git a/include/asm-powerpc/kvm_host.h b/include/asm-powerpc/kvm_host.h
> index 088780f..ef57aec 100644
> --- a/include/asm-powerpc/kvm_host.h
> +++ b/include/asm-powerpc/kvm_host.h
> @@ -143,6 +143,7 @@ enum emulation_result {
> 
>  extern int __kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu);
> 
> +extern unsigned long *kvm_trampoline_handler;
>  extern char kvm_trampoline_start[];
>  extern void kvm_trampoline_resume_host(void);
>  extern unsigned long kvm_trampoline_resume_host_len;

Actually, this patch could still apply with some renaming.

However, now that we've gotten away from binary patching the branch
instruction, I'm not eager to go back to that with the "li" instruction.
Each handler is only 24 bytes, so I'm not that worried about duplicating
them, and the readability and simplicity are better with the
duplication.

-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel