> -----Original Message----- > From: Alexander Graf [mailto:agraf@xxxxxxx] > Sent: Wednesday, July 04, 2012 5:30 PM > To: Caraman Mihai Claudiu-B02008 > Cc: <kvm-ppc@xxxxxxxxxxxxxxx>; KVM list; linuxppc-dev; qemu- > ppc@xxxxxxxxxx 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@xxxxxxxxxxxxx> > > --- > > 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" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html