On 10.08.2012, at 12:41, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: kvm-owner@xxxxxxxxxxxxxxx [mailto:kvm-owner@xxxxxxxxxxxxxxx] On Behalf Of >> Alexander Graf >> Sent: Friday, August 10, 2012 2:49 PM >> To: Bhushan Bharat-R65777 >> Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx >> Subject: Re: [PATCH 2/3] KVM: PPC: booke: Allow multiple exception types >> >> >> On 10.08.2012, at 08:38, Bhushan Bharat-R65777 wrote: >> >>> >>> >>>> -----Original Message----- >>>> From: kvm-ppc-owner@xxxxxxxxxxxxxxx >>>> [mailto:kvm-ppc-owner@xxxxxxxxxxxxxxx] On Behalf Of Alexander Graf >>>> Sent: Tuesday, August 07, 2012 4:16 PM >>>> To: Bhushan Bharat-R65777 >>>> Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; Bhushan >>>> Bharat-R65777 >>>> Subject: Re: [PATCH 2/3] KVM: PPC: booke: Allow multiple exception >>>> types >>>> >>>> >>>> On 03.08.2012, at 09:08, Bharat Bhushan wrote: >>>> >>>>> Current kvmppc_booke_handlers uses the same macro (KVM_HANDLER) and >>>>> all handlers are considered to be the same size. This will not be >>>>> the case if we want to use different macros for different handlers. >>>>> >>>>> This patch improves the kvmppc_booke_handler so that it can support >>>>> different macros for different handlers. >>>>> >>>>> Signed-off-by: Liu Yu <yu.liu@xxxxxxxxxxxxx> >>>>> [bharat.bhushan@xxxxxxxxxxxxx: Substantial changes] >>>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@xxxxxxxxxxxxx> >>>>> --- >>>>> arch/powerpc/include/asm/kvm_ppc.h | 2 - >>>>> arch/powerpc/kvm/booke.c | 9 ++++--- >>>>> arch/powerpc/kvm/booke.h | 1 + >>>>> arch/powerpc/kvm/booke_interrupts.S | 37 ++++++++++++++++++++++++++++++++- >> - >>>>> arch/powerpc/kvm/e500.c | 13 +++++++---- >>>>> 5 files changed, 48 insertions(+), 14 deletions(-) >>>>> >>>>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h >>>> b/arch/powerpc/include/asm/kvm_ppc.h >>>>> index 1438a5e..deb55bd 100644 >>>>> --- a/arch/powerpc/include/asm/kvm_ppc.h >>>>> +++ b/arch/powerpc/include/asm/kvm_ppc.h >>>>> @@ -47,8 +47,6 @@ enum emulation_result { >>>>> >>>>> extern int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu >>>>> *vcpu); extern int __kvmppc_vcpu_run(struct kvm_run *kvm_run, struct >>>>> kvm_vcpu *vcpu); -extern char kvmppc_handlers_start[]; -extern >>>>> unsigned long kvmppc_handler_len; extern void >>>>> kvmppc_handler_highmem(void); >>>>> >>>>> extern void kvmppc_dump_vcpu(struct kvm_vcpu *vcpu); diff --git >>>>> a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index >>>>> aebbb8b..1338233 100644 >>>>> --- a/arch/powerpc/kvm/booke.c >>>>> +++ b/arch/powerpc/kvm/booke.c >>>>> @@ -1541,6 +1541,7 @@ int __init kvmppc_booke_init(void) { #ifndef >>>>> CONFIG_KVM_BOOKE_HV >>>>> unsigned long ivor[16]; >>>>> + unsigned long *handler = kvmppc_booke_handler_addr; >>>>> unsigned long max_ivor = 0; >>>>> int i; >>>>> >>>>> @@ -1574,14 +1575,14 @@ int __init kvmppc_booke_init(void) >>>>> >>>>> for (i = 0; i < 16; i++) { >>>>> if (ivor[i] > max_ivor) >>>>> - max_ivor = ivor[i]; >>>>> + max_ivor = i; >>>>> >>>>> memcpy((void *)kvmppc_booke_handlers + ivor[i], >>>>> - kvmppc_handlers_start + i * kvmppc_handler_len, >>>>> - kvmppc_handler_len); >>>>> + (void *)handler[i], handler[i + 1] - handler[i]); >>>>> } >>>>> flush_icache_range(kvmppc_booke_handlers, >>>>> - kvmppc_booke_handlers + max_ivor + kvmppc_handler_len); >>>>> + kvmppc_booke_handlers + ivor[max_ivor] + >>>>> + handler[max_ivor + 1] - >>>>> +handler[max_ivor]); >>>>> #endif /* !BOOKE_HV */ >>>>> return 0; >>>>> } >>>>> diff --git a/arch/powerpc/kvm/booke.h b/arch/powerpc/kvm/booke.h >>>>> index ba61974..de9e526 100644 >>>>> --- a/arch/powerpc/kvm/booke.h >>>>> +++ b/arch/powerpc/kvm/booke.h >>>>> @@ -65,6 +65,7 @@ >>>>> (1 << BOOKE_IRQPRIO_CRITICAL)) >>>>> >>>>> extern unsigned long kvmppc_booke_handlers; >>>>> +extern unsigned long kvmppc_booke_handler_addr[]; >>>>> >>>>> void kvmppc_set_msr(struct kvm_vcpu *vcpu, u32 new_msr); void >>>>> kvmppc_mmu_msr_notify(struct kvm_vcpu *vcpu, u32 old_msr); diff >>>>> --git a/arch/powerpc/kvm/booke_interrupts.S >>>> b/arch/powerpc/kvm/booke_interrupts.S >>>>> index bb46b32..3539805 100644 >>>>> --- a/arch/powerpc/kvm/booke_interrupts.S >>>>> +++ b/arch/powerpc/kvm/booke_interrupts.S >>>>> @@ -73,6 +73,14 @@ _GLOBAL(kvmppc_handler_\ivor_nr) >>>>> bctr >>>>> .endm >>>>> >>>>> +.macro KVM_HANDLER_ADDR ivor_nr >>>>> + .long kvmppc_handler_\ivor_nr >>>>> +.endm >>>>> + >>>>> +.macro KVM_HANDLER_END >>>>> + .long kvmppc_handlers_end >>>>> +.endm >>>>> + >>>>> _GLOBAL(kvmppc_handlers_start) >>>>> KVM_HANDLER BOOKE_INTERRUPT_CRITICAL SPRN_SPRG_RSCRATCH_CRIT >>>>> SPRN_CSRR0 KVM_HANDLER BOOKE_INTERRUPT_MACHINE_CHECK >>>>> SPRN_SPRG_RSCRATCH_MC SPRN_MCSRR0 @@ -93,9 +101,7 @@ KVM_HANDLER >>>>> BOOKE_INTERRUPT_DEBUG SPRN_SPRG_RSCRATCH_CRIT >>>> SPRN_CSRR0 >>>>> KVM_HANDLER BOOKE_INTERRUPT_SPE_UNAVAIL SPRN_SPRG_RSCRATCH0 >>>>> SPRN_SRR0 KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_DATA >>>>> SPRN_SPRG_RSCRATCH0 SPRN_SRR0 KVM_HANDLER >>>>> BOOKE_INTERRUPT_SPE_FP_ROUND SPRN_SPRG_RSCRATCH0 SPRN_SRR0 >>>>> - >>>>> -_GLOBAL(kvmppc_handler_len) >>>>> - .long kvmppc_handler_1 - kvmppc_handler_0 >>>>> +_GLOBAL(kvmppc_handlers_end) >>>>> >>>>> /* Registers: >>>>> * SPRG_SCRATCH0: guest r4 >>>>> @@ -463,6 +469,31 @@ lightweight_exit: >>>>> lwz r4, VCPU_GPR(R4)(r4) >>>>> rfi >>>>> >>>>> + .data >>>>> + .align 4 >>>>> + .globl kvmppc_booke_handler_addr >>>>> +kvmppc_booke_handler_addr: >>>>> +KVM_HANDLER_ADDR BOOKE_INTERRUPT_CRITICAL KVM_HANDLER_ADDR >>>>> +BOOKE_INTERRUPT_MACHINE_CHECK KVM_HANDLER_ADDR >>>>> +BOOKE_INTERRUPT_DATA_STORAGE KVM_HANDLER_ADDR >>>>> +BOOKE_INTERRUPT_INST_STORAGE KVM_HANDLER_ADDR >>>>> +BOOKE_INTERRUPT_EXTERNAL KVM_HANDLER_ADDR BOOKE_INTERRUPT_ALIGNMENT >>>>> +KVM_HANDLER_ADDR BOOKE_INTERRUPT_PROGRAM KVM_HANDLER_ADDR >>>>> +BOOKE_INTERRUPT_FP_UNAVAIL KVM_HANDLER_ADDR BOOKE_INTERRUPT_SYSCALL >>>>> +KVM_HANDLER_ADDR BOOKE_INTERRUPT_AP_UNAVAIL KVM_HANDLER_ADDR >>>>> +BOOKE_INTERRUPT_DECREMENTER KVM_HANDLER_ADDR BOOKE_INTERRUPT_FIT >>>>> +KVM_HANDLER_ADDR BOOKE_INTERRUPT_WATCHDOG KVM_HANDLER_ADDR >>>>> +BOOKE_INTERRUPT_DTLB_MISS KVM_HANDLER_ADDR >>>>> +BOOKE_INTERRUPT_ITLB_MISS KVM_HANDLER_ADDR BOOKE_INTERRUPT_DEBUG >>>>> +KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_UNAVAIL KVM_HANDLER_ADDR >>>>> +BOOKE_INTERRUPT_SPE_FP_DATA KVM_HANDLER_ADDR >>>>> +BOOKE_INTERRUPT_SPE_FP_ROUND KVM_HANDLER_END /*Always keep this in >>>>> +end*/ >>>>> + >>>>> #ifdef CONFIG_SPE >>>>> _GLOBAL(kvmppc_save_guest_spe) >>>>> cmpi 0,r3,0 >>>>> diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c index >>>>> b479ed7..cb7a5e7 100644 >>>>> --- a/arch/powerpc/kvm/e500.c >>>>> +++ b/arch/powerpc/kvm/e500.c >>>>> @@ -491,12 +491,15 @@ static int __init kvmppc_e500_init(void) { >>>>> int r, i; >>>>> unsigned long ivor[3]; >>>>> + unsigned long *handler = kvmppc_booke_handler_addr; >>>> >>>> /* Process remaining handlers above the generic first 16 */ >>>> unsigned long *handler = &kvmppc_booke_handler_addr[16]; >>>> >>>>> unsigned long max_ivor = 0; >>>>> >>>>> r = kvmppc_core_check_processor_compat(); >>>>> if (r) >>>>> return r; >>>>> >>>>> + handler += 16; >>>>> + >>>>> r = kvmppc_booke_init(); >>>>> if (r) >>>>> return r; >>>>> @@ -506,15 +509,15 @@ static int __init kvmppc_e500_init(void) >>>>> ivor[1] = mfspr(SPRN_IVOR33); >>>>> ivor[2] = mfspr(SPRN_IVOR34); >>>>> for (i = 0; i < 3; i++) { >>>>> - if (ivor[i] > max_ivor) >>>>> - max_ivor = ivor[i]; >>>>> + if (ivor[i] > ivor[max_ivor]) >>>>> + max_ivor = i; >>>>> >>>>> memcpy((void *)kvmppc_booke_handlers + ivor[i], >>>>> - kvmppc_handlers_start + (i + 16) * kvmppc_handler_len, >>>>> - kvmppc_handler_len); >>>>> + (void *)handler[i], handler[i + 1] - handler[i]); >>>> >>>> handler_len = handler[i + 1] - handler[i]; >>>> handler_end = max(handler_end, handler[i] + handler_len); >>> >>> Ok . >>> >>>> >>>>> } >>>>> flush_icache_range(kvmppc_booke_handlers, >>>>> - kvmppc_booke_handlers + max_ivor + kvmppc_handler_len); >>>>> + kvmppc_booke_handlers + ivor[max_ivor] + >>>>> + handler[max_ivor + 1] - handler[max_ivor]); >>>> >>>> handler_end >>> >>> I do not get how this logic will work and simple. Can you please explain ? >>> >>> The end address here is calculated by IVOR offset (same as host ivor offset) >> where KVM handler[last] is copied in kvmppc_booke_handlers and length by >> handler[last + 1] - handler[last]; >> >> Hrm. You want to flush from [ kvmppc_booke_handlers ; kvmppc_booke_handlers + >> handlers_len ]. >> >> handlers_len = handler[0] - handler_end; > > You do not know that handler[0] is placed at lowest memory or some other? I'm mostly concerned about readability here. The icache flush call should make it obvious what we are flushing, without having to trace 5 different variables on their semantics. Alex -- 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
