>________________________________________ >From: Alexander Graf [agraf@xxxxxxx] >Sent: Wednesday, July 04, 2012 6:45 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 04.07.2012, at 17:27, Caraman Mihai Claudiu-B02008 wrote: > >>> -----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. > >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" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
