diff mbox

[RFC,12/17] PowerPC: booke64: Add DO_KVM kernel hooks

Message ID 1340627195-11544-13-git-send-email-mihai.caraman@freescale.com
State New, archived
Headers show

Commit Message

Mihai Caraman June 25, 2012, 12:26 p.m. UTC
Hook DO_KVM macro to 64-bit booke in a optimal way similar to 32-bit booke
see head_fsl_booke.S file. Extend interrupt handlers' parameter list with
interrupt vector numbers to accomodate the macro. Rework Guest Doorbell
handler to use the proper GSRRx save/restore registers.
Only the bolted version of tlb miss handers is addressed now.

Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
 arch/powerpc/kernel/exceptions-64e.S |  114 ++++++++++++++++++++++++----------
 arch/powerpc/mm/tlb_low_64e.S        |   14 +++-
 2 files changed, 92 insertions(+), 36 deletions(-)

Comments

Alexander Graf July 4, 2012, 2:29 p.m. UTC | #1
On 25.06.2012, at 14:26, Mihai Caraman wrote:

> Hook DO_KVM macro to 64-bit booke in a optimal way similar to 32-bit booke
> see head_fsl_booke.S file. Extend interrupt handlers' parameter list with
> interrupt vector numbers to accomodate the macro. Rework Guest Doorbell
> handler to use the proper GSRRx save/restore registers.
> Only the bolted version of tlb miss handers is addressed now.
> 
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> arch/powerpc/kernel/exceptions-64e.S |  114 ++++++++++++++++++++++++----------
> arch/powerpc/mm/tlb_low_64e.S        |   14 +++-
> 2 files changed, 92 insertions(+), 36 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
> index 06f7aec..a60f81f 100644
> --- a/arch/powerpc/kernel/exceptions-64e.S
> +++ b/arch/powerpc/kernel/exceptions-64e.S
> @@ -25,6 +25,8 @@
> #include <asm/ppc-opcode.h>
> #include <asm/mmu.h>
> #include <asm/hw_irq.h>
> +#include <asm/kvm_asm.h>
> +#include <asm/kvm_booke_hv_asm.h>
> 
> /* XXX This will ultimately add space for a special exception save
>  *     structure used to save things like SRR0/SRR1, SPRGs, MAS, etc...
> @@ -34,13 +36,24 @@
>  */
> #define	SPECIAL_EXC_FRAME_SIZE	INT_FRAME_SIZE
> 
> +#ifdef CONFIG_KVM_BOOKE_HV
> +#define KVM_BOOKE_HV_MFSPR(reg, spr)				\
> +	BEGIN_FTR_SECTION					\
> +		mfspr	reg, spr;			  	\
> +	END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
> +#else
> +#define KVM_BOOKE_HV_MFSPR(reg, spr)
> +#endif

Bleks - this is ugly. Do we really need to open-code the #ifdef here? Can't the feature section code determine that the feature is disabled and just always not include the code?

> +
> /* Exception prolog code for all exceptions */
> -#define EXCEPTION_PROLOG(n, type, srr0, srr1, addition)		     	    \
> +#define EXCEPTION_PROLOG(n, intnum, type, srr0, srr1, addition)		    \
> 	mtspr	SPRN_SPRG_##type##_SCRATCH,r13;	/* get spare registers */   \
> 	mfspr	r13,SPRN_SPRG_PACA;	/* get PACA */			    \
> 	std	r10,PACA_EX##type+EX_R10(r13);				    \
> 	std	r11,PACA_EX##type+EX_R11(r13);				    \
> 	mfcr	r10;			/* save CR */			    \
> +	KVM_BOOKE_HV_MFSPR(r11,srr1);			    		    \
> +	DO_KVM	intnum,srr1;				    		    \

So if DO_KVM already knows srr1, why explicitly do something with it the line above, and not in DO_KVM itself?

> 	addition;			/* additional code for that exc. */ \
> 	std	r1,PACA_EX##type+EX_R1(r13); /* save old r1 in the PACA */  \
> 	stw	r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \
> @@ -69,17 +82,21 @@
> 	ld	r1,PACA_MC_STACK(r13);					    \
> 	subi	r1,r1,SPECIAL_EXC_FRAME_SIZE;
> 
> -#define NORMAL_EXCEPTION_PROLOG(n, addition)				    \
> -	EXCEPTION_PROLOG(n, GEN, SPRN_SRR0, SPRN_SRR1, addition##_GEN(n))
> +#define NORMAL_EXCEPTION_PROLOG(n, intnum, addition)			    \
> +	EXCEPTION_PROLOG(n, intnum, GEN, SPRN_SRR0, SPRN_SRR1,		    \

We would we want to pass in 2 numbers? Let's please confine this onto a single ID per interrupt vector. Either we use the hardcoded ones available here in the KVM code or we use the KVM ones instead of the hardcoded ones here. But not both please. Just because it's like that on 32bit doesn't count as an excuse :).

> +					 addition##_GEN(n))
> 
> -#define CRIT_EXCEPTION_PROLOG(n, addition)				    \
> -	EXCEPTION_PROLOG(n, CRIT, SPRN_CSRR0, SPRN_CSRR1, addition##_CRIT(n))
> +#define CRIT_EXCEPTION_PROLOG(n, intnum, addition)			    \
> +	EXCEPTION_PROLOG(n, intnum, CRIT, SPRN_CSRR0, SPRN_CSRR1, 	    \
> +					 addition##_CRIT(n))
> 
> -#define DBG_EXCEPTION_PROLOG(n, addition)				    \
> -	EXCEPTION_PROLOG(n, DBG, SPRN_DSRR0, SPRN_DSRR1, addition##_DBG(n))
> +#define DBG_EXCEPTION_PROLOG(n, intnum, addition)			    \
> +	EXCEPTION_PROLOG(n, intnum, DBG, SPRN_DSRR0, SPRN_DSRR1, 	    \
> +					 addition##_DBG(n))
> 
> -#define MC_EXCEPTION_PROLOG(n, addition)				    \
> -	EXCEPTION_PROLOG(n, MC, SPRN_MCSRR0, SPRN_MCSRR1, addition##_MC(n))
> +#define MC_EXCEPTION_PROLOG(n, intnum, addition)			    \
> +	EXCEPTION_PROLOG(n, intnum, MC, SPRN_MCSRR0, SPRN_MCSRR1, 	    \
> +					 addition##_MC(n))
> 
> 
> /* Variants of the "addition" argument for the prolog
> @@ -226,9 +243,9 @@ exc_##n##_bad_stack:							    \
> 1:
> 
> 
> -#define MASKABLE_EXCEPTION(trapnum, label, hdlr, ack)			\
> +#define MASKABLE_EXCEPTION(trapnum, intnum, label, hdlr, ack)		\
> 	START_EXCEPTION(label);						\
> -	NORMAL_EXCEPTION_PROLOG(trapnum, PROLOG_ADDITION_MASKABLE)	\
> +	NORMAL_EXCEPTION_PROLOG(trapnum, intnum, PROLOG_ADDITION_MASKABLE)\
> 	EXCEPTION_COMMON(trapnum, PACA_EXGEN, INTS_DISABLE)		\
> 	ack(r8);							\
> 	CHECK_NAPPING();						\
> @@ -279,7 +296,8 @@ interrupt_end_book3e:
> 
> /* Critical Input Interrupt */
> 	START_EXCEPTION(critical_input);
> -	CRIT_EXCEPTION_PROLOG(0x100, PROLOG_ADDITION_NONE)
> +	CRIT_EXCEPTION_PROLOG(0x100, BOOKE_INTERRUPT_CRITICAL,
> +			      PROLOG_ADDITION_NONE)
> //	EXCEPTION_COMMON(0x100, PACA_EXCRIT, INTS_DISABLE)
> //	bl	special_reg_save_crit
> //	CHECK_NAPPING();
> @@ -290,7 +308,8 @@ interrupt_end_book3e:
> 
> /* Machine Check Interrupt */
> 	START_EXCEPTION(machine_check);
> -	MC_EXCEPTION_PROLOG(0x200, PROLOG_ADDITION_NONE)
> +	MC_EXCEPTION_PROLOG(0x200, BOOKE_INTERRUPT_MACHINE_CHECK,
> +			    PROLOG_ADDITION_NONE)
> //	EXCEPTION_COMMON(0x200, PACA_EXMC, INTS_DISABLE)
> //	bl	special_reg_save_mc
> //	addi	r3,r1,STACK_FRAME_OVERHEAD
> @@ -301,7 +320,8 @@ interrupt_end_book3e:
> 
> /* Data Storage Interrupt */
> 	START_EXCEPTION(data_storage)
> -	NORMAL_EXCEPTION_PROLOG(0x300, PROLOG_ADDITION_2REGS)
> +	NORMAL_EXCEPTION_PROLOG(0x300, BOOKE_INTERRUPT_DATA_STORAGE,
> +				PROLOG_ADDITION_2REGS)
> 	mfspr	r14,SPRN_DEAR
> 	mfspr	r15,SPRN_ESR
> 	EXCEPTION_COMMON(0x300, PACA_EXGEN, INTS_DISABLE)
> @@ -309,18 +329,21 @@ interrupt_end_book3e:
> 
> /* Instruction Storage Interrupt */
> 	START_EXCEPTION(instruction_storage);
> -	NORMAL_EXCEPTION_PROLOG(0x400, PROLOG_ADDITION_2REGS)
> +	NORMAL_EXCEPTION_PROLOG(0x400, BOOKE_INTERRUPT_INST_STORAGE,
> +				PROLOG_ADDITION_2REGS)
> 	li	r15,0
> 	mr	r14,r10
> 	EXCEPTION_COMMON(0x400, PACA_EXGEN, INTS_DISABLE)
> 	b	storage_fault_common
> 
> /* External Input Interrupt */
> -	MASKABLE_EXCEPTION(0x500, external_input, .do_IRQ, ACK_NONE)
> +	MASKABLE_EXCEPTION(0x500, BOOKE_INTERRUPT_EXTERNAL,
> +			   external_input, .do_IRQ, ACK_NONE)
> 
> /* Alignment */
> 	START_EXCEPTION(alignment);
> -	NORMAL_EXCEPTION_PROLOG(0x600, PROLOG_ADDITION_2REGS)
> +	NORMAL_EXCEPTION_PROLOG(0x600, BOOKE_INTERRUPT_ALIGNMENT,
> +				PROLOG_ADDITION_2REGS)
> 	mfspr	r14,SPRN_DEAR
> 	mfspr	r15,SPRN_ESR
> 	EXCEPTION_COMMON(0x600, PACA_EXGEN, INTS_KEEP)
> @@ -328,7 +351,8 @@ interrupt_end_book3e:
> 
> /* Program Interrupt */
> 	START_EXCEPTION(program);
> -	NORMAL_EXCEPTION_PROLOG(0x700, PROLOG_ADDITION_1REG)
> +	NORMAL_EXCEPTION_PROLOG(0x700, BOOKE_INTERRUPT_PROGRAM,
> +				PROLOG_ADDITION_1REG)
> 	mfspr	r14,SPRN_ESR
> 	EXCEPTION_COMMON(0x700, PACA_EXGEN, INTS_DISABLE)
> 	std	r14,_DSISR(r1)
> @@ -340,7 +364,8 @@ interrupt_end_book3e:
> 
> /* Floating Point Unavailable Interrupt */
> 	START_EXCEPTION(fp_unavailable);
> -	NORMAL_EXCEPTION_PROLOG(0x800, PROLOG_ADDITION_NONE)
> +	NORMAL_EXCEPTION_PROLOG(0x800, BOOKE_INTERRUPT_FP_UNAVAIL,
> +				PROLOG_ADDITION_NONE)
> 	/* we can probably do a shorter exception entry for that one... */
> 	EXCEPTION_COMMON(0x800, PACA_EXGEN, INTS_KEEP)
> 	ld	r12,_MSR(r1)
> @@ -355,14 +380,17 @@ interrupt_end_book3e:
> 	b	.ret_from_except
> 
> /* Decrementer Interrupt */
> -	MASKABLE_EXCEPTION(0x900, decrementer, .timer_interrupt, ACK_DEC)
> +	MASKABLE_EXCEPTION(0x900, BOOKE_INTERRUPT_DECREMENTER,
> +			   decrementer, .timer_interrupt, ACK_DEC)
> 
> /* Fixed Interval Timer Interrupt */
> -	MASKABLE_EXCEPTION(0x980, fixed_interval, .unknown_exception, ACK_FIT)
> +	MASKABLE_EXCEPTION(0x980, BOOKE_INTERRUPT_FIT,
> +			   fixed_interval, .unknown_exception, ACK_FIT)
> 
> /* Watchdog Timer Interrupt */
> 	START_EXCEPTION(watchdog);
> -	CRIT_EXCEPTION_PROLOG(0x9f0, PROLOG_ADDITION_NONE)
> +	CRIT_EXCEPTION_PROLOG(0x9f0, BOOKE_INTERRUPT_WATCHDOG,
> +			      PROLOG_ADDITION_NONE)
> //	EXCEPTION_COMMON(0x9f0, PACA_EXCRIT, INTS_DISABLE)
> //	bl	special_reg_save_crit
> //	CHECK_NAPPING();
> @@ -381,7 +409,8 @@ interrupt_end_book3e:
> 
> /* Auxiliary Processor Unavailable Interrupt */
> 	START_EXCEPTION(ap_unavailable);
> -	NORMAL_EXCEPTION_PROLOG(0xf20, PROLOG_ADDITION_NONE)
> +	NORMAL_EXCEPTION_PROLOG(0xf20, BOOKE_INTERRUPT_AP_UNAVAIL,
> +				PROLOG_ADDITION_NONE)
> 	EXCEPTION_COMMON(0xf20, PACA_EXGEN, INTS_DISABLE)
> 	bl	.save_nvgprs
> 	addi	r3,r1,STACK_FRAME_OVERHEAD
> @@ -390,7 +419,8 @@ interrupt_end_book3e:
> 
> /* Debug exception as a critical interrupt*/
> 	START_EXCEPTION(debug_crit);
> -	CRIT_EXCEPTION_PROLOG(0xd00, PROLOG_ADDITION_2REGS)
> +	CRIT_EXCEPTION_PROLOG(0xd00, BOOKE_INTERRUPT_DEBUG,
> +			      PROLOG_ADDITION_2REGS)
> 
> 	/*
> 	 * If there is a single step or branch-taken exception in an
> @@ -455,7 +485,8 @@ kernel_dbg_exc:
> 
> /* Debug exception as a debug interrupt*/
> 	START_EXCEPTION(debug_debug);
> -	DBG_EXCEPTION_PROLOG(0xd08, PROLOG_ADDITION_2REGS)
> +	DBG_EXCEPTION_PROLOG(0xd00, BOOKE_INTERRUPT_DEBUG,
> +						 PROLOG_ADDITION_2REGS)
> 
> 	/*
> 	 * If there is a single step or branch-taken exception in an
> @@ -516,18 +547,21 @@ kernel_dbg_exc:
> 	b	.ret_from_except
> 
> 	START_EXCEPTION(perfmon);
> -	NORMAL_EXCEPTION_PROLOG(0x260, PROLOG_ADDITION_NONE)
> +	NORMAL_EXCEPTION_PROLOG(0x260, BOOKE_INTERRUPT_PERFORMANCE_MONITOR,
> +				PROLOG_ADDITION_NONE)
> 	EXCEPTION_COMMON(0x260, PACA_EXGEN, INTS_DISABLE)
> 	addi	r3,r1,STACK_FRAME_OVERHEAD
> 	bl	.performance_monitor_exception
> 	b	.ret_from_except_lite
> 
> /* Doorbell interrupt */
> -	MASKABLE_EXCEPTION(0x280, doorbell, .doorbell_exception, ACK_NONE)
> +	MASKABLE_EXCEPTION(0x280, BOOKE_INTERRUPT_DOORBELL,
> +			   doorbell, .doorbell_exception, ACK_NONE)
> 
> /* Doorbell critical Interrupt */
> 	START_EXCEPTION(doorbell_crit);
> -	CRIT_EXCEPTION_PROLOG(0x2a0, PROLOG_ADDITION_NONE)
> +	CRIT_EXCEPTION_PROLOG(0x2a0, BOOKE_INTERRUPT_DOORBELL_CRITICAL,
> +			      PROLOG_ADDITION_NONE)
> //	EXCEPTION_COMMON(0x2a0, PACA_EXCRIT, INTS_DISABLE)
> //	bl	special_reg_save_crit
> //	CHECK_NAPPING();
> @@ -536,12 +570,24 @@ kernel_dbg_exc:
> //	b	ret_from_crit_except
> 	b	.
> 
> -/* Guest Doorbell */
> -	MASKABLE_EXCEPTION(0x2c0, guest_doorbell, .unknown_exception, ACK_NONE)
> +/*
> + *	Guest doorbell interrupt
> + *	This general exception use GSRRx save/restore registers
> + */
> +	START_EXCEPTION(guest_doorbell);
> +	EXCEPTION_PROLOG(0x2c0, BOOKE_INTERRUPT_GUEST_DBELL, GEN,
> +			 SPRN_GSRR0, SPRN_GSRR1, PROLOG_ADDITION_NONE)
> +	EXCEPTION_COMMON(0x2c0, PACA_EXGEN, INTS_KEEP)
> +	addi	r3,r1,STACK_FRAME_OVERHEAD
> +	bl	.save_nvgprs
> +	INTS_RESTORE_HARD
> +	bl	.unknown_exception
> +	b	.ret_from_except

This is independent of DO_KVM, right?

> 
> /* Guest Doorbell critical Interrupt */
> 	START_EXCEPTION(guest_doorbell_crit);
> -	CRIT_EXCEPTION_PROLOG(0x2e0, PROLOG_ADDITION_NONE)
> +	CRIT_EXCEPTION_PROLOG(0x2e0, BOOKE_INTERRUPT_GUEST_DBELL_CRIT,
> +			      PROLOG_ADDITION_NONE)

Shouldn't this one also use GSRR?

> //	EXCEPTION_COMMON(0x2e0, PACA_EXCRIT, INTS_DISABLE)
> //	bl	special_reg_save_crit
> //	CHECK_NAPPING();
> @@ -552,7 +598,8 @@ kernel_dbg_exc:
> 
> /* Hypervisor call */
> 	START_EXCEPTION(hypercall);
> -	NORMAL_EXCEPTION_PROLOG(0x310, PROLOG_ADDITION_NONE)
> +	NORMAL_EXCEPTION_PROLOG(0x310, BOOKE_INTERRUPT_HV_SYSCALL,
> +			        PROLOG_ADDITION_NONE)
> 	EXCEPTION_COMMON(0x310, PACA_EXGEN, INTS_KEEP)
> 	addi	r3,r1,STACK_FRAME_OVERHEAD
> 	bl	.save_nvgprs
> @@ -562,7 +609,8 @@ kernel_dbg_exc:
> 
> /* Embedded Hypervisor priviledged  */
> 	START_EXCEPTION(ehpriv);
> -	NORMAL_EXCEPTION_PROLOG(0x320, PROLOG_ADDITION_NONE)
> +	NORMAL_EXCEPTION_PROLOG(0x320, BOOKE_INTERRUPT_HV_PRIV,
> +			        PROLOG_ADDITION_NONE)
> 	EXCEPTION_COMMON(0x320, PACA_EXGEN, INTS_KEEP)
> 	addi	r3,r1,STACK_FRAME_OVERHEAD
> 	bl	.save_nvgprs
> diff --git a/arch/powerpc/mm/tlb_low_64e.S b/arch/powerpc/mm/tlb_low_64e.S
> index ff672bd..88feaaa 100644
> --- a/arch/powerpc/mm/tlb_low_64e.S
> +++ b/arch/powerpc/mm/tlb_low_64e.S
> @@ -20,6 +20,8 @@
> #include <asm/pgtable.h>
> #include <asm/exception-64e.h>
> #include <asm/ppc-opcode.h>
> +#include <asm/kvm_asm.h>
> +#include <asm/kvm_booke_hv_asm.h>
> 
> #ifdef CONFIG_PPC_64K_PAGES
> #define VPTE_PMD_SHIFT	(PTE_INDEX_SIZE+1)
> @@ -37,12 +39,18 @@
>  *                                                                    *
>  **********************************************************************/
> 
> -.macro tlb_prolog_bolted addr
> +.macro tlb_prolog_bolted intnum addr
> 	mtspr	SPRN_SPRG_TLB_SCRATCH,r13
> 	mfspr	r13,SPRN_SPRG_PACA
> 	std	r10,PACA_EXTLB+EX_TLB_R10(r13)
> 	mfcr	r10
> 	std	r11,PACA_EXTLB+EX_TLB_R11(r13)
> +#ifdef CONFIG_KVM_BOOKE_HV
> +BEGIN_FTR_SECTION
> +	mfspr	r11, SPRN_SRR1
> +END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
> +#endif

This thing really should vanish behind DO_KVM :)

Alex

> +	DO_KVM	\intnum, SPRN_SRR1
> 	std	r16,PACA_EXTLB+EX_TLB_R16(r13)
> 	mfspr	r16,\addr		/* get faulting address */
> 	std	r14,PACA_EXTLB+EX_TLB_R14(r13)
> @@ -66,7 +74,7 @@
> 
> /* Data TLB miss */
> 	START_EXCEPTION(data_tlb_miss_bolted)
> -	tlb_prolog_bolted SPRN_DEAR
> +	tlb_prolog_bolted BOOKE_INTERRUPT_DTLB_MISS SPRN_DEAR
> 
> 	/* We need _PAGE_PRESENT and  _PAGE_ACCESSED set */
> 
> @@ -214,7 +222,7 @@ itlb_miss_fault_bolted:
> 
> /* Instruction TLB miss */
> 	START_EXCEPTION(instruction_tlb_miss_bolted)
> -	tlb_prolog_bolted SPRN_SRR0
> +	tlb_prolog_bolted BOOKE_INTERRUPT_ITLB_MISS SPRN_SRR0
> 
> 	rldicl.	r10,r16,64-PGTABLE_EADDR_SIZE,PGTABLE_EADDR_SIZE+4
> 	srdi	r15,r16,60		/* get region */
> -- 
> 1.7.4.1
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Caraman Mihai Claudiu-B02008 July 4, 2012, 3:27 p.m. UTC | #2
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Wednesday, July 04, 2012 5:30 PM
> To: Caraman Mihai Claudiu-B02008
> Cc: <kvm-ppc@vger.kernel.org>; KVM list; linuxppc-dev; qemu-
> ppc@nongnu.org List; Benjamin Herrenschmidt
> Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM
> kernel hooks
> 
> 
> On 25.06.2012, at 14:26, Mihai Caraman wrote:
> 
> > Hook DO_KVM macro to 64-bit booke in a optimal way similar to 32-bit
> booke
> > see head_fsl_booke.S file. Extend interrupt handlers' parameter list
> with
> > interrupt vector numbers to accomodate the macro. Rework Guest Doorbell
> > handler to use the proper GSRRx save/restore registers.
> > Only the bolted version of tlb miss handers is addressed now.
> >
> > Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> > ---
> > arch/powerpc/kernel/exceptions-64e.S |  114 ++++++++++++++++++++++++---
> -------
> > arch/powerpc/mm/tlb_low_64e.S        |   14 +++-
> > 2 files changed, 92 insertions(+), 36 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/exceptions-64e.S
> b/arch/powerpc/kernel/exceptions-64e.S
> > index 06f7aec..a60f81f 100644
> > --- a/arch/powerpc/kernel/exceptions-64e.S
> > +++ b/arch/powerpc/kernel/exceptions-64e.S
> > @@ -25,6 +25,8 @@
> > #include <asm/ppc-opcode.h>
> > #include <asm/mmu.h>
> > #include <asm/hw_irq.h>
> > +#include <asm/kvm_asm.h>
> > +#include <asm/kvm_booke_hv_asm.h>
> >
> > /* XXX This will ultimately add space for a special exception save
> >  *     structure used to save things like SRR0/SRR1, SPRGs, MAS, etc...
> > @@ -34,13 +36,24 @@
> >  */
> > #define	SPECIAL_EXC_FRAME_SIZE	INT_FRAME_SIZE
> >
> > +#ifdef CONFIG_KVM_BOOKE_HV
> > +#define KVM_BOOKE_HV_MFSPR(reg, spr)				\
> > +	BEGIN_FTR_SECTION					\
> > +		mfspr	reg, spr;			  	\
> > +	END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
> > +#else
> > +#define KVM_BOOKE_HV_MFSPR(reg, spr)
> > +#endif
> 
> Bleks - this is ugly.

I agree :) But I opted to keep the optimizations done for 32-bit.

> Do we really need to open-code the #ifdef here?

32-bit implementation fortunately use asm macros, we can't nest defines.

> Can't the feature section code determine that the feature is disabled and
> just always not include the code?

CPU_FTR_EMB_HV is set even if KVM is not configured.

> 
> > +
> > /* Exception prolog code for all exceptions */
> > -#define EXCEPTION_PROLOG(n, type, srr0, srr1, addition)
> \
> > +#define EXCEPTION_PROLOG(n, intnum, type, srr0, srr1, addition)
> 	    \
> > 	mtspr	SPRN_SPRG_##type##_SCRATCH,r13;	/* get spare registers */
> \
> > 	mfspr	r13,SPRN_SPRG_PACA;	/* get PACA */			    \
> > 	std	r10,PACA_EX##type+EX_R10(r13);				    \
> > 	std	r11,PACA_EX##type+EX_R11(r13);				    \
> > 	mfcr	r10;			/* save CR */			    \
> > +	KVM_BOOKE_HV_MFSPR(r11,srr1);			    		    \
> > +	DO_KVM	intnum,srr1;				    		    \
> 
> So if DO_KVM already knows srr1, why explicitly do something with it the
> line above, and not in DO_KVM itself?

srr1 is used to expand the interrupt handler symbol name while r11 is used
for the actual MSR[GS] optimal check:
	mtocrf	0x80, r11

> > -/* Guest Doorbell */
> > -	MASKABLE_EXCEPTION(0x2c0, guest_doorbell, .unknown_exception,
> ACK_NONE)
> > +/*
> > + *	Guest doorbell interrupt
> > + *	This general exception use GSRRx save/restore registers
> > + */
> > +	START_EXCEPTION(guest_doorbell);
> > +	EXCEPTION_PROLOG(0x2c0, BOOKE_INTERRUPT_GUEST_DBELL, GEN,
> > +			 SPRN_GSRR0, SPRN_GSRR1, PROLOG_ADDITION_NONE)
> > +	EXCEPTION_COMMON(0x2c0, PACA_EXGEN, INTS_KEEP)
> > +	addi	r3,r1,STACK_FRAME_OVERHEAD
> > +	bl	.save_nvgprs
> > +	INTS_RESTORE_HARD
> > +	bl	.unknown_exception
> > +	b	.ret_from_except
> 
> This is independent of DO_KVM, right?

Yes, just kvm_handler definitions in bookehv_interrupts.S depends on this.

> 
> >
> > /* Guest Doorbell critical Interrupt */
> > 	START_EXCEPTION(guest_doorbell_crit);
> > -	CRIT_EXCEPTION_PROLOG(0x2e0, PROLOG_ADDITION_NONE)
> > +	CRIT_EXCEPTION_PROLOG(0x2e0, BOOKE_INTERRUPT_GUEST_DBELL_CRIT,
> > +			      PROLOG_ADDITION_NONE)
> 
> Shouldn't this one also use GSRR?

No, this is a critical exception.

> >
> > -.macro tlb_prolog_bolted addr
> > +.macro tlb_prolog_bolted intnum addr
> > 	mtspr	SPRN_SPRG_TLB_SCRATCH,r13
> > 	mfspr	r13,SPRN_SPRG_PACA
> > 	std	r10,PACA_EXTLB+EX_TLB_R10(r13)
> > 	mfcr	r10
> > 	std	r11,PACA_EXTLB+EX_TLB_R11(r13)
> > +#ifdef CONFIG_KVM_BOOKE_HV
> > +BEGIN_FTR_SECTION
> > +	mfspr	r11, SPRN_SRR1
> > +END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
> > +#endif
> 
> This thing really should vanish behind DO_KVM :)

Then let's do it first for 32-bit ;)

-Mike

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf July 4, 2012, 3:45 p.m. UTC | #3
On 04.07.2012, at 17:27, Caraman Mihai Claudiu-B02008 wrote:

>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Wednesday, July 04, 2012 5:30 PM
>> To: Caraman Mihai Claudiu-B02008
>> Cc: <kvm-ppc@vger.kernel.org>; KVM list; linuxppc-dev; qemu-
>> ppc@nongnu.org List; Benjamin Herrenschmidt
>> Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM
>> kernel hooks
>> 
>> 
>> On 25.06.2012, at 14:26, Mihai Caraman wrote:
>> 
>>> Hook DO_KVM macro to 64-bit booke in a optimal way similar to 32-bit
>> booke
>>> see head_fsl_booke.S file. Extend interrupt handlers' parameter list
>> with
>>> interrupt vector numbers to accomodate the macro. Rework Guest Doorbell
>>> handler to use the proper GSRRx save/restore registers.
>>> Only the bolted version of tlb miss handers is addressed now.
>>> 
>>> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>>> ---
>>> arch/powerpc/kernel/exceptions-64e.S |  114 ++++++++++++++++++++++++---
>> -------
>>> arch/powerpc/mm/tlb_low_64e.S        |   14 +++-
>>> 2 files changed, 92 insertions(+), 36 deletions(-)
>>> 
>>> diff --git a/arch/powerpc/kernel/exceptions-64e.S
>> b/arch/powerpc/kernel/exceptions-64e.S
>>> index 06f7aec..a60f81f 100644
>>> --- a/arch/powerpc/kernel/exceptions-64e.S
>>> +++ b/arch/powerpc/kernel/exceptions-64e.S
>>> @@ -25,6 +25,8 @@
>>> #include <asm/ppc-opcode.h>
>>> #include <asm/mmu.h>
>>> #include <asm/hw_irq.h>
>>> +#include <asm/kvm_asm.h>
>>> +#include <asm/kvm_booke_hv_asm.h>
>>> 
>>> /* XXX This will ultimately add space for a special exception save
>>> *     structure used to save things like SRR0/SRR1, SPRGs, MAS, etc...
>>> @@ -34,13 +36,24 @@
>>> */
>>> #define	SPECIAL_EXC_FRAME_SIZE	INT_FRAME_SIZE
>>> 
>>> +#ifdef CONFIG_KVM_BOOKE_HV
>>> +#define KVM_BOOKE_HV_MFSPR(reg, spr)				\
>>> +	BEGIN_FTR_SECTION					\
>>> +		mfspr	reg, spr;			  	\
>>> +	END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
>>> +#else
>>> +#define KVM_BOOKE_HV_MFSPR(reg, spr)
>>> +#endif
>> 
>> Bleks - this is ugly.
> 
> I agree :) But I opted to keep the optimizations done for 32-bit.
> 
>> Do we really need to open-code the #ifdef here?
> 
> 32-bit implementation fortunately use asm macros, we can't nest defines.
> 
>> Can't the feature section code determine that the feature is disabled and
>> just always not include the code?
> 
> CPU_FTR_EMB_HV is set even if KVM is not configured.

I don't get the point then. Why not have the whole DO_KVM masked under FTR_SECTION_IFSET(CPU_FTR_EMB_HV)? Are there book3s_64 implementations without HV? Can't we just mfspr unconditionally in DO_KVM?

> 
>> 
>>> +
>>> /* Exception prolog code for all exceptions */
>>> -#define EXCEPTION_PROLOG(n, type, srr0, srr1, addition)
>> \
>>> +#define EXCEPTION_PROLOG(n, intnum, type, srr0, srr1, addition)
>> 	    \
>>> 	mtspr	SPRN_SPRG_##type##_SCRATCH,r13;	/* get spare registers */
>> \
>>> 	mfspr	r13,SPRN_SPRG_PACA;	/* get PACA */			    \
>>> 	std	r10,PACA_EX##type+EX_R10(r13);				    \
>>> 	std	r11,PACA_EX##type+EX_R11(r13);				    \
>>> 	mfcr	r10;			/* save CR */			    \
>>> +	KVM_BOOKE_HV_MFSPR(r11,srr1);			    		    \
>>> +	DO_KVM	intnum,srr1;				    		    \
>> 
>> So if DO_KVM already knows srr1, why explicitly do something with it the
>> line above, and not in DO_KVM itself?
> 
> srr1 is used to expand the interrupt handler symbol name while r11 is used
> for the actual MSR[GS] optimal check:
> 	mtocrf	0x80, r11

Right, so basically we want

#ifdef CONFIG_KVM
mfspr r11, spr
mtocrf 0x80, r11
beq ...
#endif

right?

> 
>>> -/* Guest Doorbell */
>>> -	MASKABLE_EXCEPTION(0x2c0, guest_doorbell, .unknown_exception,
>> ACK_NONE)
>>> +/*
>>> + *	Guest doorbell interrupt
>>> + *	This general exception use GSRRx save/restore registers
>>> + */
>>> +	START_EXCEPTION(guest_doorbell);
>>> +	EXCEPTION_PROLOG(0x2c0, BOOKE_INTERRUPT_GUEST_DBELL, GEN,
>>> +			 SPRN_GSRR0, SPRN_GSRR1, PROLOG_ADDITION_NONE)
>>> +	EXCEPTION_COMMON(0x2c0, PACA_EXGEN, INTS_KEEP)
>>> +	addi	r3,r1,STACK_FRAME_OVERHEAD
>>> +	bl	.save_nvgprs
>>> +	INTS_RESTORE_HARD
>>> +	bl	.unknown_exception
>>> +	b	.ret_from_except
>> 
>> This is independent of DO_KVM, right?
> 
> Yes, just kvm_handler definitions in bookehv_interrupts.S depends on this.

Then please split it out into a separate patch.

> 
>> 
>>> 
>>> /* Guest Doorbell critical Interrupt */
>>> 	START_EXCEPTION(guest_doorbell_crit);
>>> -	CRIT_EXCEPTION_PROLOG(0x2e0, PROLOG_ADDITION_NONE)
>>> +	CRIT_EXCEPTION_PROLOG(0x2e0, BOOKE_INTERRUPT_GUEST_DBELL_CRIT,
>>> +			      PROLOG_ADDITION_NONE)
>> 
>> Shouldn't this one also use GSRR?
> 
> No, this is a critical exception.

Ah, right. Looked at the wrong bit, sorry :).

> 
>>> 
>>> -.macro tlb_prolog_bolted addr
>>> +.macro tlb_prolog_bolted intnum addr
>>> 	mtspr	SPRN_SPRG_TLB_SCRATCH,r13
>>> 	mfspr	r13,SPRN_SPRG_PACA
>>> 	std	r10,PACA_EXTLB+EX_TLB_R10(r13)
>>> 	mfcr	r10
>>> 	std	r11,PACA_EXTLB+EX_TLB_R11(r13)
>>> +#ifdef CONFIG_KVM_BOOKE_HV
>>> +BEGIN_FTR_SECTION
>>> +	mfspr	r11, SPRN_SRR1
>>> +END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
>>> +#endif
>> 
>> This thing really should vanish behind DO_KVM :)
> 
> Then let's do it first for 32-bit ;)

You could #ifdef it in DO_KVM for 64-bit for now. IIRC it's not done on 32-bit because the register value is used even beyond DO_KVM there.


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Caraman Mihai Claudiu-B02008 July 4, 2012, 6:15 p.m. UTC | #4
>________________________________________
>From: Alexander Graf [agraf@suse.de]
>Sent: Wednesday, July 04, 2012 6:45 PM
>To: Caraman Mihai Claudiu-B02008
>Cc: <kvm-ppc@vger.kernel.org>; KVM list; linuxppc-dev; qemu-ppc@nongnu.org List; Benjamin Herrenschmidt
>Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM kernel hooks
>
>On 04.07.2012, at 17:27, Caraman Mihai Claudiu-B02008 wrote:
>
>>> -----Original Message-----
>>> From: Alexander Graf [mailto:agraf@suse.de]
>>> Sent: Wednesday, July 04, 2012 5:30 PM
>>> To: Caraman Mihai Claudiu-B02008
>>> Cc: <kvm-ppc@vger.kernel.org>; KVM list; linuxppc-dev; qemu-
>>> ppc@nongnu.org List; Benjamin Herrenschmidt
>>> Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM
>>> kernel hooks
>>>
>>>
>>> On 25.06.2012, at 14:26, Mihai Caraman wrote:
>>>
>>>> Hook DO_KVM macro to 64-bit booke in a optimal way similar to 32-bit
>>> booke
>>>> see head_fsl_booke.S file. Extend interrupt handlers' parameter list
>>> with
>>>> interrupt vector numbers to accomodate the macro. Rework Guest Doorbell
>>>> handler to use the proper GSRRx save/restore registers.
>>>> Only the bolted version of tlb miss handers is addressed now.
>>>>
>>>> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>>>> ---
>>>> arch/powerpc/kernel/exceptions-64e.S |  114 ++++++++++++++++++++++++---
>>> -------
>>>> arch/powerpc/mm/tlb_low_64e.S        |   14 +++-
>>>> 2 files changed, 92 insertions(+), 36 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/kernel/exceptions-64e.S
>>> b/arch/powerpc/kernel/exceptions-64e.S
>>>> index 06f7aec..a60f81f 100644
>>>> --- a/arch/powerpc/kernel/exceptions-64e.S
>>>> +++ b/arch/powerpc/kernel/exceptions-64e.S
>>>> @@ -25,6 +25,8 @@
>>>> #include <asm/ppc-opcode.h>
>>>> #include <asm/mmu.h>
>>>> #include <asm/hw_irq.h>
>>>> +#include <asm/kvm_asm.h>
>>>> +#include <asm/kvm_booke_hv_asm.h>
>>>>
>>>> /* XXX This will ultimately add space for a special exception save
>>>> *     structure used to save things like SRR0/SRR1, SPRGs, MAS, etc...
>>>> @@ -34,13 +36,24 @@
>>>> */
>>>> #define     SPECIAL_EXC_FRAME_SIZE  INT_FRAME_SIZE
>>>>
>>>> +#ifdef CONFIG_KVM_BOOKE_HV
>>>> +#define KVM_BOOKE_HV_MFSPR(reg, spr)                               \
>>>> +   BEGIN_FTR_SECTION                                       \
>>>> +           mfspr   reg, spr;                               \
>>>> +   END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
>>>> +#else
>>>> +#define KVM_BOOKE_HV_MFSPR(reg, spr)
>>>> +#endif
>>>
>>> Bleks - this is ugly.
>>
>> I agree :) But I opted to keep the optimizations done for 32-bit.
>>
>>> Do we really need to open-code the #ifdef here?
>>
>> 32-bit implementation fortunately use asm macros, we can't nest defines.
>>
>>> Can't the feature section code determine that the feature is disabled and
>>> just always not include the code?
>>
>> CPU_FTR_EMB_HV is set even if KVM is not configured.
>
>I don't get the point then. Why not have the whole DO_KVM masked under FTR_SECTION_IFSET(CPU_FTR_EMB_HV)? Are there book3s_64 implementations without HV? 

I guess you refer to book3e_64. I don't know all implementations but Embedded.HV category is optional.

>Can't we just mfspr unconditionally in DO_KVM?

I think Scott should better answer this question, I don't know why he opted for the other approach.

>>>> -/* Guest Doorbell */
>>>> -   MASKABLE_EXCEPTION(0x2c0, guest_doorbell, .unknown_exception,
>>> ACK_NONE)
>>>> +/*
>>>> + * Guest doorbell interrupt
>>>> + * This general exception use GSRRx save/restore registers
>>>> + */
>>>> +   START_EXCEPTION(guest_doorbell);
>>>> +   EXCEPTION_PROLOG(0x2c0, BOOKE_INTERRUPT_GUEST_DBELL, GEN,
>>>> +                    SPRN_GSRR0, SPRN_GSRR1, PROLOG_ADDITION_NONE)
>>>> +   EXCEPTION_COMMON(0x2c0, PACA_EXGEN, INTS_KEEP)
>>>> +   addi    r3,r1,STACK_FRAME_OVERHEAD
>>>> +   bl      .save_nvgprs
>>>> +   INTS_RESTORE_HARD
>>>> +   bl      .unknown_exception
>>>> +   b       .ret_from_except
>>>
>>> This is independent of DO_KVM, right?
>>
>> Yes, just kvm_handler definitions in bookehv_interrupts.S depends on this.
>
>Then please split it out into a separate patch.

Can you be more precise, are you referring to guest_doorbell exception handler?

>>>> -.macro tlb_prolog_bolted addr
>>>> +.macro tlb_prolog_bolted intnum addr
>>>>     mtspr   SPRN_SPRG_TLB_SCRATCH,r13
>>>>     mfspr   r13,SPRN_SPRG_PACA
>>>>     std     r10,PACA_EXTLB+EX_TLB_R10(r13)
>>>>     mfcr    r10
>>>>     std     r11,PACA_EXTLB+EX_TLB_R11(r13)
>>>> +#ifdef CONFIG_KVM_BOOKE_HV
>>>> +BEGIN_FTR_SECTION
>>>> +   mfspr   r11, SPRN_SRR1
>>>> +END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
>>>> +#endif
>>>
>>> This thing really should vanish behind DO_KVM :)
>>
>> Then let's do it first for 32-bit ;)
>
>You could #ifdef it in DO_KVM for 64-bit for now. IIRC it's not done on 32-bit because the register value is used even beyond DO_KVM there.

Nope, 32-bit code is also guarded by CONFIG_KVM_BOOKE_HV.

-Mike
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benjamin Herrenschmidt July 4, 2012, 10:25 p.m. UTC | #5
On Wed, 2012-07-04 at 16:29 +0200, Alexander Graf wrote:
 
> > +#ifdef CONFIG_KVM_BOOKE_HV
> > +#define KVM_BOOKE_HV_MFSPR(reg, spr)				\
> > +	BEGIN_FTR_SECTION					\
> > +		mfspr	reg, spr;			  	\
> > +	END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
> > +#else
> > +#define KVM_BOOKE_HV_MFSPR(reg, spr)
> > +#endif
> 
> Bleks - this is ugly. Do we really need to open-code the #ifdef here?
> Can't the feature section code determine that the feature is disabled
> and just always not include the code?

You can't but in any case I don't see the point of the conditional here,
we'll eventually have to load srr1 no ? We can move the load up to here
in all cases or can't we ? If really not, we could have it inside DO_KVM
and be done with it no ?

> > +
> > /* Exception prolog code for all exceptions */
> > -#define EXCEPTION_PROLOG(n, type, srr0, srr1, addition)		     	    \
> > +#define EXCEPTION_PROLOG(n, intnum, type, srr0, srr1, addition)		    \
> > 	mtspr	SPRN_SPRG_##type##_SCRATCH,r13;	/* get spare registers */   \
> > 	mfspr	r13,SPRN_SPRG_PACA;	/* get PACA */			    \
> > 	std	r10,PACA_EX##type+EX_R10(r13);				    \
> > 	std	r11,PACA_EX##type+EX_R11(r13);				    \
> > 	mfcr	r10;			/* save CR */			    \
> > +	KVM_BOOKE_HV_MFSPR(r11,srr1);			    		    \
> > +	DO_KVM	intnum,srr1;				    		    \
> 
> So if DO_KVM already knows srr1, why explicitly do something with it
> the line above, and not in DO_KVM itself?

Yeah that or just move things around in the prolog.

> > 	addition;			/* additional code for that exc. */ \
> > 	std	r1,PACA_EX##type+EX_R1(r13); /* save old r1 in the PACA */  \
> > 	stw	r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \
> > @@ -69,17 +82,21 @@
> > 	ld	r1,PACA_MC_STACK(r13);					    \
> > 	subi	r1,r1,SPECIAL_EXC_FRAME_SIZE;
> > 
> > -#define NORMAL_EXCEPTION_PROLOG(n, addition)				    \
> > -	EXCEPTION_PROLOG(n, GEN, SPRN_SRR0, SPRN_SRR1, addition##_GEN(n))
> > +#define NORMAL_EXCEPTION_PROLOG(n, intnum, addition)			    \
> > +	EXCEPTION_PROLOG(n, intnum, GEN, SPRN_SRR0, SPRN_SRR1,		    \
> 
> We would we want to pass in 2 numbers? Let's please confine this onto
> a single ID per interrupt vector. Either we use the hardcoded ones
> available here in the KVM code or we use the KVM ones instead of the
> hardcoded ones here. But not both please. Just because it's like that
> on 32bit doesn't count as an excuse :).

Right. Also I already objected to the explicit passing of the srr's
anyway.

Cheers,
Ben.


--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood July 6, 2012, 12:19 a.m. UTC | #6
On 07/04/2012 01:15 PM, Caraman Mihai Claudiu-B02008 wrote:
>> ________________________________________
>> From: Alexander Graf [agraf@suse.de]
>> Sent: Wednesday, July 04, 2012 6:45 PM
>> To: Caraman Mihai Claudiu-B02008
>> Cc: <kvm-ppc@vger.kernel.org>; KVM list; linuxppc-dev; qemu-ppc@nongnu.org List; Benjamin Herrenschmidt
>> Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM kernel hooks
>>
>> On 04.07.2012, at 17:27, Caraman Mihai Claudiu-B02008 wrote:
>>
>>>> -----Original Message-----
>>>> From: Alexander Graf [mailto:agraf@suse.de]
>>>> Sent: Wednesday, July 04, 2012 5:30 PM
>>>> To: Caraman Mihai Claudiu-B02008
>>>> Cc: <kvm-ppc@vger.kernel.org>; KVM list; linuxppc-dev; qemu-
>>>> ppc@nongnu.org List; Benjamin Herrenschmidt
>>>> Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM
>>>> kernel hooks
>>>>
>>>>
>>>> On 25.06.2012, at 14:26, Mihai Caraman wrote:
>>>>
>>>>> Hook DO_KVM macro to 64-bit booke in a optimal way similar to 32-bit
>>>> booke
>>>>> see head_fsl_booke.S file. Extend interrupt handlers' parameter list
>>>> with
>>>>> interrupt vector numbers to accomodate the macro. Rework Guest Doorbell
>>>>> handler to use the proper GSRRx save/restore registers.
>>>>> Only the bolted version of tlb miss handers is addressed now.
>>>>>
>>>>> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>>>>> ---
>>>>> arch/powerpc/kernel/exceptions-64e.S |  114 ++++++++++++++++++++++++---
>>>> -------
>>>>> arch/powerpc/mm/tlb_low_64e.S        |   14 +++-
>>>>> 2 files changed, 92 insertions(+), 36 deletions(-)
>>>>>
>>>>> diff --git a/arch/powerpc/kernel/exceptions-64e.S
>>>> b/arch/powerpc/kernel/exceptions-64e.S
>>>>> index 06f7aec..a60f81f 100644
>>>>> --- a/arch/powerpc/kernel/exceptions-64e.S
>>>>> +++ b/arch/powerpc/kernel/exceptions-64e.S
>>>>> @@ -25,6 +25,8 @@
>>>>> #include <asm/ppc-opcode.h>
>>>>> #include <asm/mmu.h>
>>>>> #include <asm/hw_irq.h>
>>>>> +#include <asm/kvm_asm.h>
>>>>> +#include <asm/kvm_booke_hv_asm.h>
>>>>>
>>>>> /* XXX This will ultimately add space for a special exception save
>>>>> *     structure used to save things like SRR0/SRR1, SPRGs, MAS, etc...
>>>>> @@ -34,13 +36,24 @@
>>>>> */
>>>>> #define     SPECIAL_EXC_FRAME_SIZE  INT_FRAME_SIZE
>>>>>
>>>>> +#ifdef CONFIG_KVM_BOOKE_HV
>>>>> +#define KVM_BOOKE_HV_MFSPR(reg, spr)                               \
>>>>> +   BEGIN_FTR_SECTION                                       \
>>>>> +           mfspr   reg, spr;                               \
>>>>> +   END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
>>>>> +#else
>>>>> +#define KVM_BOOKE_HV_MFSPR(reg, spr)
>>>>> +#endif
>>>>
>>>> Bleks - this is ugly.
>>>
>>> I agree :) But I opted to keep the optimizations done for 32-bit.
>>>
>>>> Do we really need to open-code the #ifdef here?
>>>
>>> 32-bit implementation fortunately use asm macros, we can't nest defines.
>>>
>>>> Can't the feature section code determine that the feature is disabled and
>>>> just always not include the code?
>>>
>>> CPU_FTR_EMB_HV is set even if KVM is not configured.
>>
>> I don't get the point then. Why not have the whole DO_KVM masked under FTR_SECTION_IFSET(CPU_FTR_EMB_HV)? Are there book3s_64 implementations without HV? 
> 
> I guess you refer to book3e_64. I don't know all implementations but Embedded.HV category is optional.
> 
>> Can't we just mfspr unconditionally in DO_KVM?
> 
> I think Scott should better answer this question, I don't know why he opted for the other approach.

That was on 32-bit, where some of DO_KVM's users want SRR1 for their own
purposes.

>>>>> -.macro tlb_prolog_bolted addr
>>>>> +.macro tlb_prolog_bolted intnum addr
>>>>>     mtspr   SPRN_SPRG_TLB_SCRATCH,r13
>>>>>     mfspr   r13,SPRN_SPRG_PACA
>>>>>     std     r10,PACA_EXTLB+EX_TLB_R10(r13)
>>>>>     mfcr    r10
>>>>>     std     r11,PACA_EXTLB+EX_TLB_R11(r13)
>>>>> +#ifdef CONFIG_KVM_BOOKE_HV
>>>>> +BEGIN_FTR_SECTION
>>>>> +   mfspr   r11, SPRN_SRR1
>>>>> +END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
>>>>> +#endif
>>>>
>>>> This thing really should vanish behind DO_KVM :)
>>>
>>> Then let's do it first for 32-bit ;)
>>
>> You could #ifdef it in DO_KVM for 64-bit for now. IIRC it's not done on 32-bit because the register value is used even beyond DO_KVM there.
> 
> Nope, 32-bit code is also guarded by CONFIG_KVM_BOOKE_HV.

Only in the TLB miss handlers, not the normal exception prolog.

-Scott

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf July 6, 2012, 11:11 p.m. UTC | #7
On 07.07.2012, at 00:33, Caraman Mihai Claudiu-B02008 wrote:

>> -----Original Message-----
>> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
>> Sent: Thursday, July 05, 2012 1:26 AM
>> To: Alexander Graf
>> Cc: Caraman Mihai Claudiu-B02008; <kvm-ppc@vger.kernel.org>; KVM list;
>> linuxppc-dev; qemu-ppc@nongnu.org List
>> Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM
>> kernel hooks
>> 
>> On Wed, 2012-07-04 at 16:29 +0200, Alexander Graf wrote:
>> 
>>>> +#ifdef CONFIG_KVM_BOOKE_HV
>>>> +#define KVM_BOOKE_HV_MFSPR(reg, spr)				\
>>>> +	BEGIN_FTR_SECTION					\
>>>> +		mfspr	reg, spr;			  	\
>>>> +	END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
>>>> +#else
>>>> +#define KVM_BOOKE_HV_MFSPR(reg, spr)
>>>> +#endif
>>> 
>>> Bleks - this is ugly. Do we really need to open-code the #ifdef here?
>>> Can't the feature section code determine that the feature is disabled
>>> and just always not include the code?
>> 
>> You can't but in any case I don't see the point of the conditional here,
>> we'll eventually have to load srr1 no ? We can move the load up to here
>> in all cases or can't we ? 
> 
> I like the idea, but there is a problem with addition macros which may clobber
> r11 and PROLOG_ADDITION_MASKABLE_GEN is such a case.

Mike -v please :)


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Caraman Mihai Claudiu-B02008 July 7, 2012, 8:39 a.m. UTC | #8
>________________________________________
>From: Alexander Graf [agraf@suse.de]
>Sent: Saturday, July 07, 2012 2:11 AM
>To: Caraman Mihai Claudiu-B02008
>Cc: Benjamin Herrenschmidt; <kvm-ppc@vger.kernel.org>; KVM list; linuxppc-dev; qemu-ppc@nongnu.org List
>Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM kernel hooks
>
>On 07.07.2012, at 00:33, Caraman Mihai Claudiu-B02008 wrote:
>
>>> -----Original Message-----
>>> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
>>> Sent: Thursday, July 05, 2012 1:26 AM
>>> To: Alexander Graf
>>> Cc: Caraman Mihai Claudiu-B02008; <kvm-ppc@vger.kernel.org>; KVM list;
>>> linuxppc-dev; qemu-ppc@nongnu.org List
>>> Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM
>>> kernel hooks
>>>
>>> You can't but in any case I don't see the point of the conditional here,
>>> we'll eventually have to load srr1 no ? We can move the load up to here
>>> in all cases or can't we ?
>>
>> I like the idea, but there is a problem with addition macros which may clobber
>> r11 and PROLOG_ADDITION_MASKABLE_GEN is such a case.
>
>Mike -v please :)

Ben suggested something like this:
	
 #define EXCEPTION_PROLOG(n, type, addition) \
 	mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \
 	mfspr r13,SPRN_SPRG_PACA; /* get PACA */ \
 	std r10,PACA_EX##type+EX_R10(r13); \
 	std r11,PACA_EX##type+EX_R11(r13); \
 	mfcr r10; /* save CR */ \	
+	mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */ \
	DO_KVM	intnum,srr1; \
 	addition; /* additional code for that exc. */ \
 	std r1,PACA_EX##type+EX_R1(r13); /* save old r1 in the PACA */ \
 	stw r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \
-	mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */ \
 	type##_SET_KSTACK; /* get special stack if necessary */\
 	andi. r10,r11,MSR_PR; /* save stack pointer */ \

But one of the addition looks like this:
	
 #define PROLOG_ADDITION_MASKABLE_GEN(n) \
 	lbz r11,PACASOFTIRQEN(r13); /* are irqs soft-disabled ? */ \
	cmpwi cr0,r11,0; /* yes -> go out of line */ \
	beq masked_interrupt_book3e_##n	

So for maskable gen we end up with:

 #define EXCEPTION_PROLOG(n, type, addition) \
 	mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \
 	mfspr r13,SPRN_SPRG_PACA; /* get PACA */ \
 	std r10,PACA_EX##type+EX_R10(r13); \
 	std r11,PACA_EX##type+EX_R11(r13); \
 	mfcr r10; /* save CR */ \
	mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */ \
 	DO_KVM	intnum,srr1; \
	lbz r11,PACASOFTIRQEN(r13); /* are irqs soft-disabled ? */ \
	cmpwi cr0,r11,0; /* yes -> go out of line */ \
	beq masked_interrupt_book3e_##n	\
	std r1,PACA_EX##type+EX_R1(r13); /* save old r1 in the PACA */ \
 	stw r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \
 	type##_SET_KSTACK; /* get special stack if necessary */\
 	andi. r10,r11,MSR_PR; /* save stack pointer */ \
	
This affects the last asm line, we load srr1 into r11 but clobber it in-between.
We need a spare register for maskable gen addition. I think we can free r10 sooner
and used it in addition like this:

 #define EXCEPTION_PROLOG(n, type, addition) \
 	mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \
 	mfspr r13,SPRN_SPRG_PACA; /* get PACA */ \		
 	std r10,PACA_EX##type+EX_R10(r13); \
 	std r11,PACA_EX##type+EX_R11(r13); \
+	mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */ \
	mfcr r10; /* save CR */ \
+ 	stw r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \
 	DO_KVM	intnum,srr1; \
-	lbz r11,PACASOFTIRQEN(r13); /* are irqs soft-disabled ? */ \
-	cmpwi cr0,r11,0; /* yes -> go out of line */ \
+	lbz r10,PACASOFTIRQEN(r13); /* are irqs soft-disabled ? */ \
+	cmpwi cr0,r10,0; /* yes -> go out of line */ \
	beq masked_interrupt_book3e_##n	\
 	std r1,PACA_EX##type+EX_R1(r13); /* save old r1 in the PACA */ \
- 	stw r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \
-	mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */ \
 	type##_SET_KSTACK; /* get special stack if necessary */\
 	andi. r10,r11,MSR_PR; /* save stack pointer */ \
	
-Mike
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf July 11, 2012, 10:25 p.m. UTC | #9
On 07.07.2012, at 10:39, Caraman Mihai Claudiu-B02008 wrote:

>> ________________________________________
>> From: Alexander Graf [agraf@suse.de]
>> Sent: Saturday, July 07, 2012 2:11 AM
>> To: Caraman Mihai Claudiu-B02008
>> Cc: Benjamin Herrenschmidt; <kvm-ppc@vger.kernel.org>; KVM list; linuxppc-dev; qemu-ppc@nongnu.org List
>> Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM kernel hooks
>> 
>> On 07.07.2012, at 00:33, Caraman Mihai Claudiu-B02008 wrote:
>> 
>>>> -----Original Message-----
>>>> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
>>>> Sent: Thursday, July 05, 2012 1:26 AM
>>>> To: Alexander Graf
>>>> Cc: Caraman Mihai Claudiu-B02008; <kvm-ppc@vger.kernel.org>; KVM list;
>>>> linuxppc-dev; qemu-ppc@nongnu.org List
>>>> Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM
>>>> kernel hooks
>>>> 
>>>> You can't but in any case I don't see the point of the conditional here,
>>>> we'll eventually have to load srr1 no ? We can move the load up to here
>>>> in all cases or can't we ?
>>> 
>>> I like the idea, but there is a problem with addition macros which may clobber
>>> r11 and PROLOG_ADDITION_MASKABLE_GEN is such a case.
>> 
>> Mike -v please :)
> 
> Ben suggested something like this:
> 	
> #define EXCEPTION_PROLOG(n, type, addition) \
> 	mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \
> 	mfspr r13,SPRN_SPRG_PACA; /* get PACA */ \
> 	std r10,PACA_EX##type+EX_R10(r13); \
> 	std r11,PACA_EX##type+EX_R11(r13); \
> 	mfcr r10; /* save CR */ \	
> +	mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */ \
> 	DO_KVM	intnum,srr1; \
> 	addition; /* additional code for that exc. */ \
> 	std r1,PACA_EX##type+EX_R1(r13); /* save old r1 in the PACA */ \
> 	stw r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \
> -	mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */ \
> 	type##_SET_KSTACK; /* get special stack if necessary */\
> 	andi. r10,r11,MSR_PR; /* save stack pointer */ \
> 
> But one of the addition looks like this:
> 	
> #define PROLOG_ADDITION_MASKABLE_GEN(n) \
> 	lbz r11,PACASOFTIRQEN(r13); /* are irqs soft-disabled ? */ \
> 	cmpwi cr0,r11,0; /* yes -> go out of line */ \
> 	beq masked_interrupt_book3e_##n	
> 
> So for maskable gen we end up with:
> 
> #define EXCEPTION_PROLOG(n, type, addition) \
> 	mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \
> 	mfspr r13,SPRN_SPRG_PACA; /* get PACA */ \
> 	std r10,PACA_EX##type+EX_R10(r13); \
> 	std r11,PACA_EX##type+EX_R11(r13); \
> 	mfcr r10; /* save CR */ \
> 	mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */ \
> 	DO_KVM	intnum,srr1; \
> 	lbz r11,PACASOFTIRQEN(r13); /* are irqs soft-disabled ? */ \
> 	cmpwi cr0,r11,0; /* yes -> go out of line */ \
> 	beq masked_interrupt_book3e_##n	\
> 	std r1,PACA_EX##type+EX_R1(r13); /* save old r1 in the PACA */ \
> 	stw r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \
> 	type##_SET_KSTACK; /* get special stack if necessary */\
> 	andi. r10,r11,MSR_PR; /* save stack pointer */ \
> 	
> This affects the last asm line, we load srr1 into r11 but clobber it in-between.
> We need a spare register for maskable gen addition. I think we can free r10 sooner
> and used it in addition like this:

Ah, makes sense, yes.

> 
> #define EXCEPTION_PROLOG(n, type, addition) \
> 	mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \
> 	mfspr r13,SPRN_SPRG_PACA; /* get PACA */ \		
> 	std r10,PACA_EX##type+EX_R10(r13); \
> 	std r11,PACA_EX##type+EX_R11(r13); \

Or just free up another register early on, like here.


Alex

> +	mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */ \
> 	mfcr r10; /* save CR */ \
> + 	stw r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \
> 	DO_KVM	intnum,srr1; \
> -	lbz r11,PACASOFTIRQEN(r13); /* are irqs soft-disabled ? */ \
> -	cmpwi cr0,r11,0; /* yes -> go out of line */ \
> +	lbz r10,PACASOFTIRQEN(r13); /* are irqs soft-disabled ? */ \
> +	cmpwi cr0,r10,0; /* yes -> go out of line */ \
> 	beq masked_interrupt_book3e_##n	\
> 	std r1,PACA_EX##type+EX_R1(r13); /* save old r1 in the PACA */ \
> - 	stw r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \
> -	mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */ \
> 	type##_SET_KSTACK; /* get special stack if necessary */\
> 	andi. r10,r11,MSR_PR; /* save stack pointer */ \
> 	
> -Mike
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benjamin Herrenschmidt July 11, 2012, 10:28 p.m. UTC | #10
On Thu, 2012-07-12 at 00:25 +0200, Alexander Graf wrote:
> Or just free up another register early on, like here.

If you're going to do that, you want to measure the impact on null
syscall performance though.

Cheers,
Ben.


--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf July 11, 2012, 10:35 p.m. UTC | #11
On 12.07.2012, at 00:28, Benjamin Herrenschmidt wrote:

> On Thu, 2012-07-12 at 00:25 +0200, Alexander Graf wrote:
>> Or just free up another register early on, like here.
> 
> If you're going to do that, you want to measure the impact on null
> syscall performance though.

That should hold true for any change in that code, no?


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benjamin Herrenschmidt July 11, 2012, 10:43 p.m. UTC | #12
On Thu, 2012-07-12 at 00:35 +0200, Alexander Graf wrote:
> > On Thu, 2012-07-12 at 00:25 +0200, Alexander Graf wrote:
> >> Or just free up another register early on, like here.
> > 
> > If you're going to do that, you want to measure the impact on null
> > syscall performance though.
> 
> That should hold true for any change in that code, no?

Yes, but adding a gpr save is more invasive (you have to also load it
back later on & put it on the stack).

Cheers,
Ben.


--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf July 11, 2012, 10:51 p.m. UTC | #13
On 12.07.2012, at 00:43, Benjamin Herrenschmidt wrote:

> On Thu, 2012-07-12 at 00:35 +0200, Alexander Graf wrote:
>>> On Thu, 2012-07-12 at 00:25 +0200, Alexander Graf wrote:
>>>> Or just free up another register early on, like here.
>>> 
>>> If you're going to do that, you want to measure the impact on null
>>> syscall performance though.
>> 
>> That should hold true for any change in that code, no?
> 
> Yes, but adding a gpr save is more invasive (you have to also load it
> back later on & put it on the stack).

What's the usual cache line size like on these boxes? If you align it properly with r10 and r11 on the paca so that all 3 registers are in the same cache line, the load should be almost for free, no?


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 06f7aec..a60f81f 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -25,6 +25,8 @@ 
 #include <asm/ppc-opcode.h>
 #include <asm/mmu.h>
 #include <asm/hw_irq.h>
+#include <asm/kvm_asm.h>
+#include <asm/kvm_booke_hv_asm.h>
 
 /* XXX This will ultimately add space for a special exception save
  *     structure used to save things like SRR0/SRR1, SPRGs, MAS, etc...
@@ -34,13 +36,24 @@ 
  */
 #define	SPECIAL_EXC_FRAME_SIZE	INT_FRAME_SIZE
 
+#ifdef CONFIG_KVM_BOOKE_HV
+#define KVM_BOOKE_HV_MFSPR(reg, spr)				\
+	BEGIN_FTR_SECTION					\
+		mfspr	reg, spr;			  	\
+	END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
+#else
+#define KVM_BOOKE_HV_MFSPR(reg, spr)
+#endif
+
 /* Exception prolog code for all exceptions */
-#define EXCEPTION_PROLOG(n, type, srr0, srr1, addition)		     	    \
+#define EXCEPTION_PROLOG(n, intnum, type, srr0, srr1, addition)		    \
 	mtspr	SPRN_SPRG_##type##_SCRATCH,r13;	/* get spare registers */   \
 	mfspr	r13,SPRN_SPRG_PACA;	/* get PACA */			    \
 	std	r10,PACA_EX##type+EX_R10(r13);				    \
 	std	r11,PACA_EX##type+EX_R11(r13);				    \
 	mfcr	r10;			/* save CR */			    \
+	KVM_BOOKE_HV_MFSPR(r11,srr1);			    		    \
+	DO_KVM	intnum,srr1;				    		    \
 	addition;			/* additional code for that exc. */ \
 	std	r1,PACA_EX##type+EX_R1(r13); /* save old r1 in the PACA */  \
 	stw	r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \
@@ -69,17 +82,21 @@ 
 	ld	r1,PACA_MC_STACK(r13);					    \
 	subi	r1,r1,SPECIAL_EXC_FRAME_SIZE;
 
-#define NORMAL_EXCEPTION_PROLOG(n, addition)				    \
-	EXCEPTION_PROLOG(n, GEN, SPRN_SRR0, SPRN_SRR1, addition##_GEN(n))
+#define NORMAL_EXCEPTION_PROLOG(n, intnum, addition)			    \
+	EXCEPTION_PROLOG(n, intnum, GEN, SPRN_SRR0, SPRN_SRR1,		    \
+					 addition##_GEN(n))
 
-#define CRIT_EXCEPTION_PROLOG(n, addition)				    \
-	EXCEPTION_PROLOG(n, CRIT, SPRN_CSRR0, SPRN_CSRR1, addition##_CRIT(n))
+#define CRIT_EXCEPTION_PROLOG(n, intnum, addition)			    \
+	EXCEPTION_PROLOG(n, intnum, CRIT, SPRN_CSRR0, SPRN_CSRR1, 	    \
+					 addition##_CRIT(n))
 
-#define DBG_EXCEPTION_PROLOG(n, addition)				    \
-	EXCEPTION_PROLOG(n, DBG, SPRN_DSRR0, SPRN_DSRR1, addition##_DBG(n))
+#define DBG_EXCEPTION_PROLOG(n, intnum, addition)			    \
+	EXCEPTION_PROLOG(n, intnum, DBG, SPRN_DSRR0, SPRN_DSRR1, 	    \
+					 addition##_DBG(n))
 
-#define MC_EXCEPTION_PROLOG(n, addition)				    \
-	EXCEPTION_PROLOG(n, MC, SPRN_MCSRR0, SPRN_MCSRR1, addition##_MC(n))
+#define MC_EXCEPTION_PROLOG(n, intnum, addition)			    \
+	EXCEPTION_PROLOG(n, intnum, MC, SPRN_MCSRR0, SPRN_MCSRR1, 	    \
+					 addition##_MC(n))
 
 
 /* Variants of the "addition" argument for the prolog
@@ -226,9 +243,9 @@  exc_##n##_bad_stack:							    \
 1:
 
 
-#define MASKABLE_EXCEPTION(trapnum, label, hdlr, ack)			\
+#define MASKABLE_EXCEPTION(trapnum, intnum, label, hdlr, ack)		\
 	START_EXCEPTION(label);						\
-	NORMAL_EXCEPTION_PROLOG(trapnum, PROLOG_ADDITION_MASKABLE)	\
+	NORMAL_EXCEPTION_PROLOG(trapnum, intnum, PROLOG_ADDITION_MASKABLE)\
 	EXCEPTION_COMMON(trapnum, PACA_EXGEN, INTS_DISABLE)		\
 	ack(r8);							\
 	CHECK_NAPPING();						\
@@ -279,7 +296,8 @@  interrupt_end_book3e:
 
 /* Critical Input Interrupt */
 	START_EXCEPTION(critical_input);
-	CRIT_EXCEPTION_PROLOG(0x100, PROLOG_ADDITION_NONE)
+	CRIT_EXCEPTION_PROLOG(0x100, BOOKE_INTERRUPT_CRITICAL,
+			      PROLOG_ADDITION_NONE)
 //	EXCEPTION_COMMON(0x100, PACA_EXCRIT, INTS_DISABLE)
 //	bl	special_reg_save_crit
 //	CHECK_NAPPING();
@@ -290,7 +308,8 @@  interrupt_end_book3e:
 
 /* Machine Check Interrupt */
 	START_EXCEPTION(machine_check);
-	MC_EXCEPTION_PROLOG(0x200, PROLOG_ADDITION_NONE)
+	MC_EXCEPTION_PROLOG(0x200, BOOKE_INTERRUPT_MACHINE_CHECK,
+			    PROLOG_ADDITION_NONE)
 //	EXCEPTION_COMMON(0x200, PACA_EXMC, INTS_DISABLE)
 //	bl	special_reg_save_mc
 //	addi	r3,r1,STACK_FRAME_OVERHEAD
@@ -301,7 +320,8 @@  interrupt_end_book3e:
 
 /* Data Storage Interrupt */
 	START_EXCEPTION(data_storage)
-	NORMAL_EXCEPTION_PROLOG(0x300, PROLOG_ADDITION_2REGS)
+	NORMAL_EXCEPTION_PROLOG(0x300, BOOKE_INTERRUPT_DATA_STORAGE,
+				PROLOG_ADDITION_2REGS)
 	mfspr	r14,SPRN_DEAR
 	mfspr	r15,SPRN_ESR
 	EXCEPTION_COMMON(0x300, PACA_EXGEN, INTS_DISABLE)
@@ -309,18 +329,21 @@  interrupt_end_book3e:
 
 /* Instruction Storage Interrupt */
 	START_EXCEPTION(instruction_storage);
-	NORMAL_EXCEPTION_PROLOG(0x400, PROLOG_ADDITION_2REGS)
+	NORMAL_EXCEPTION_PROLOG(0x400, BOOKE_INTERRUPT_INST_STORAGE,
+				PROLOG_ADDITION_2REGS)
 	li	r15,0
 	mr	r14,r10
 	EXCEPTION_COMMON(0x400, PACA_EXGEN, INTS_DISABLE)
 	b	storage_fault_common
 
 /* External Input Interrupt */
-	MASKABLE_EXCEPTION(0x500, external_input, .do_IRQ, ACK_NONE)
+	MASKABLE_EXCEPTION(0x500, BOOKE_INTERRUPT_EXTERNAL,
+			   external_input, .do_IRQ, ACK_NONE)
 
 /* Alignment */
 	START_EXCEPTION(alignment);
-	NORMAL_EXCEPTION_PROLOG(0x600, PROLOG_ADDITION_2REGS)
+	NORMAL_EXCEPTION_PROLOG(0x600, BOOKE_INTERRUPT_ALIGNMENT,
+				PROLOG_ADDITION_2REGS)
 	mfspr	r14,SPRN_DEAR
 	mfspr	r15,SPRN_ESR
 	EXCEPTION_COMMON(0x600, PACA_EXGEN, INTS_KEEP)
@@ -328,7 +351,8 @@  interrupt_end_book3e:
 
 /* Program Interrupt */
 	START_EXCEPTION(program);
-	NORMAL_EXCEPTION_PROLOG(0x700, PROLOG_ADDITION_1REG)
+	NORMAL_EXCEPTION_PROLOG(0x700, BOOKE_INTERRUPT_PROGRAM,
+				PROLOG_ADDITION_1REG)
 	mfspr	r14,SPRN_ESR
 	EXCEPTION_COMMON(0x700, PACA_EXGEN, INTS_DISABLE)
 	std	r14,_DSISR(r1)
@@ -340,7 +364,8 @@  interrupt_end_book3e:
 
 /* Floating Point Unavailable Interrupt */
 	START_EXCEPTION(fp_unavailable);
-	NORMAL_EXCEPTION_PROLOG(0x800, PROLOG_ADDITION_NONE)
+	NORMAL_EXCEPTION_PROLOG(0x800, BOOKE_INTERRUPT_FP_UNAVAIL,
+				PROLOG_ADDITION_NONE)
 	/* we can probably do a shorter exception entry for that one... */
 	EXCEPTION_COMMON(0x800, PACA_EXGEN, INTS_KEEP)
 	ld	r12,_MSR(r1)
@@ -355,14 +380,17 @@  interrupt_end_book3e:
 	b	.ret_from_except
 
 /* Decrementer Interrupt */
-	MASKABLE_EXCEPTION(0x900, decrementer, .timer_interrupt, ACK_DEC)
+	MASKABLE_EXCEPTION(0x900, BOOKE_INTERRUPT_DECREMENTER,
+			   decrementer, .timer_interrupt, ACK_DEC)
 
 /* Fixed Interval Timer Interrupt */
-	MASKABLE_EXCEPTION(0x980, fixed_interval, .unknown_exception, ACK_FIT)
+	MASKABLE_EXCEPTION(0x980, BOOKE_INTERRUPT_FIT,
+			   fixed_interval, .unknown_exception, ACK_FIT)
 
 /* Watchdog Timer Interrupt */
 	START_EXCEPTION(watchdog);
-	CRIT_EXCEPTION_PROLOG(0x9f0, PROLOG_ADDITION_NONE)
+	CRIT_EXCEPTION_PROLOG(0x9f0, BOOKE_INTERRUPT_WATCHDOG,
+			      PROLOG_ADDITION_NONE)
 //	EXCEPTION_COMMON(0x9f0, PACA_EXCRIT, INTS_DISABLE)
 //	bl	special_reg_save_crit
 //	CHECK_NAPPING();
@@ -381,7 +409,8 @@  interrupt_end_book3e:
 
 /* Auxiliary Processor Unavailable Interrupt */
 	START_EXCEPTION(ap_unavailable);
-	NORMAL_EXCEPTION_PROLOG(0xf20, PROLOG_ADDITION_NONE)
+	NORMAL_EXCEPTION_PROLOG(0xf20, BOOKE_INTERRUPT_AP_UNAVAIL,
+				PROLOG_ADDITION_NONE)
 	EXCEPTION_COMMON(0xf20, PACA_EXGEN, INTS_DISABLE)
 	bl	.save_nvgprs
 	addi	r3,r1,STACK_FRAME_OVERHEAD
@@ -390,7 +419,8 @@  interrupt_end_book3e:
 
 /* Debug exception as a critical interrupt*/
 	START_EXCEPTION(debug_crit);
-	CRIT_EXCEPTION_PROLOG(0xd00, PROLOG_ADDITION_2REGS)
+	CRIT_EXCEPTION_PROLOG(0xd00, BOOKE_INTERRUPT_DEBUG,
+			      PROLOG_ADDITION_2REGS)
 
 	/*
 	 * If there is a single step or branch-taken exception in an
@@ -455,7 +485,8 @@  kernel_dbg_exc:
 
 /* Debug exception as a debug interrupt*/
 	START_EXCEPTION(debug_debug);
-	DBG_EXCEPTION_PROLOG(0xd08, PROLOG_ADDITION_2REGS)
+	DBG_EXCEPTION_PROLOG(0xd00, BOOKE_INTERRUPT_DEBUG,
+						 PROLOG_ADDITION_2REGS)
 
 	/*
 	 * If there is a single step or branch-taken exception in an
@@ -516,18 +547,21 @@  kernel_dbg_exc:
 	b	.ret_from_except
 
 	START_EXCEPTION(perfmon);
-	NORMAL_EXCEPTION_PROLOG(0x260, PROLOG_ADDITION_NONE)
+	NORMAL_EXCEPTION_PROLOG(0x260, BOOKE_INTERRUPT_PERFORMANCE_MONITOR,
+				PROLOG_ADDITION_NONE)
 	EXCEPTION_COMMON(0x260, PACA_EXGEN, INTS_DISABLE)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	.performance_monitor_exception
 	b	.ret_from_except_lite
 
 /* Doorbell interrupt */
-	MASKABLE_EXCEPTION(0x280, doorbell, .doorbell_exception, ACK_NONE)
+	MASKABLE_EXCEPTION(0x280, BOOKE_INTERRUPT_DOORBELL,
+			   doorbell, .doorbell_exception, ACK_NONE)
 
 /* Doorbell critical Interrupt */
 	START_EXCEPTION(doorbell_crit);
-	CRIT_EXCEPTION_PROLOG(0x2a0, PROLOG_ADDITION_NONE)
+	CRIT_EXCEPTION_PROLOG(0x2a0, BOOKE_INTERRUPT_DOORBELL_CRITICAL,
+			      PROLOG_ADDITION_NONE)
 //	EXCEPTION_COMMON(0x2a0, PACA_EXCRIT, INTS_DISABLE)
 //	bl	special_reg_save_crit
 //	CHECK_NAPPING();
@@ -536,12 +570,24 @@  kernel_dbg_exc:
 //	b	ret_from_crit_except
 	b	.
 
-/* Guest Doorbell */
-	MASKABLE_EXCEPTION(0x2c0, guest_doorbell, .unknown_exception, ACK_NONE)
+/*
+ *	Guest doorbell interrupt
+ *	This general exception use GSRRx save/restore registers
+ */
+	START_EXCEPTION(guest_doorbell);
+	EXCEPTION_PROLOG(0x2c0, BOOKE_INTERRUPT_GUEST_DBELL, GEN,
+			 SPRN_GSRR0, SPRN_GSRR1, PROLOG_ADDITION_NONE)
+	EXCEPTION_COMMON(0x2c0, PACA_EXGEN, INTS_KEEP)
+	addi	r3,r1,STACK_FRAME_OVERHEAD
+	bl	.save_nvgprs
+	INTS_RESTORE_HARD
+	bl	.unknown_exception
+	b	.ret_from_except
 
 /* Guest Doorbell critical Interrupt */
 	START_EXCEPTION(guest_doorbell_crit);
-	CRIT_EXCEPTION_PROLOG(0x2e0, PROLOG_ADDITION_NONE)
+	CRIT_EXCEPTION_PROLOG(0x2e0, BOOKE_INTERRUPT_GUEST_DBELL_CRIT,
+			      PROLOG_ADDITION_NONE)
 //	EXCEPTION_COMMON(0x2e0, PACA_EXCRIT, INTS_DISABLE)
 //	bl	special_reg_save_crit
 //	CHECK_NAPPING();
@@ -552,7 +598,8 @@  kernel_dbg_exc:
 
 /* Hypervisor call */
 	START_EXCEPTION(hypercall);
-	NORMAL_EXCEPTION_PROLOG(0x310, PROLOG_ADDITION_NONE)
+	NORMAL_EXCEPTION_PROLOG(0x310, BOOKE_INTERRUPT_HV_SYSCALL,
+			        PROLOG_ADDITION_NONE)
 	EXCEPTION_COMMON(0x310, PACA_EXGEN, INTS_KEEP)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	.save_nvgprs
@@ -562,7 +609,8 @@  kernel_dbg_exc:
 
 /* Embedded Hypervisor priviledged  */
 	START_EXCEPTION(ehpriv);
-	NORMAL_EXCEPTION_PROLOG(0x320, PROLOG_ADDITION_NONE)
+	NORMAL_EXCEPTION_PROLOG(0x320, BOOKE_INTERRUPT_HV_PRIV,
+			        PROLOG_ADDITION_NONE)
 	EXCEPTION_COMMON(0x320, PACA_EXGEN, INTS_KEEP)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	.save_nvgprs
diff --git a/arch/powerpc/mm/tlb_low_64e.S b/arch/powerpc/mm/tlb_low_64e.S
index ff672bd..88feaaa 100644
--- a/arch/powerpc/mm/tlb_low_64e.S
+++ b/arch/powerpc/mm/tlb_low_64e.S
@@ -20,6 +20,8 @@ 
 #include <asm/pgtable.h>
 #include <asm/exception-64e.h>
 #include <asm/ppc-opcode.h>
+#include <asm/kvm_asm.h>
+#include <asm/kvm_booke_hv_asm.h>
 
 #ifdef CONFIG_PPC_64K_PAGES
 #define VPTE_PMD_SHIFT	(PTE_INDEX_SIZE+1)
@@ -37,12 +39,18 @@ 
  *                                                                    *
  **********************************************************************/
 
-.macro tlb_prolog_bolted addr
+.macro tlb_prolog_bolted intnum addr
 	mtspr	SPRN_SPRG_TLB_SCRATCH,r13
 	mfspr	r13,SPRN_SPRG_PACA
 	std	r10,PACA_EXTLB+EX_TLB_R10(r13)
 	mfcr	r10
 	std	r11,PACA_EXTLB+EX_TLB_R11(r13)
+#ifdef CONFIG_KVM_BOOKE_HV
+BEGIN_FTR_SECTION
+	mfspr	r11, SPRN_SRR1
+END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
+#endif
+	DO_KVM	\intnum, SPRN_SRR1
 	std	r16,PACA_EXTLB+EX_TLB_R16(r13)
 	mfspr	r16,\addr		/* get faulting address */
 	std	r14,PACA_EXTLB+EX_TLB_R14(r13)
@@ -66,7 +74,7 @@ 
 
 /* Data TLB miss */
 	START_EXCEPTION(data_tlb_miss_bolted)
-	tlb_prolog_bolted SPRN_DEAR
+	tlb_prolog_bolted BOOKE_INTERRUPT_DTLB_MISS SPRN_DEAR
 
 	/* We need _PAGE_PRESENT and  _PAGE_ACCESSED set */
 
@@ -214,7 +222,7 @@  itlb_miss_fault_bolted:
 
 /* Instruction TLB miss */
 	START_EXCEPTION(instruction_tlb_miss_bolted)
-	tlb_prolog_bolted SPRN_SRR0
+	tlb_prolog_bolted BOOKE_INTERRUPT_ITLB_MISS SPRN_SRR0
 
 	rldicl.	r10,r16,64-PGTABLE_EADDR_SIZE,PGTABLE_EADDR_SIZE+4
 	srdi	r15,r16,60		/* get region */