> -----Original Message----- > From: Alexander Graf [mailto:ag...@suse.de] > Sent: Friday, August 10, 2012 4:27 PM > To: Bhushan Bharat-R65777 > Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org > Subject: Re: [PATCH 2/3] KVM: PPC: booke: Allow multiple exception types > > > On 10.08.2012, at 12:41, Bhushan Bharat-R65777 wrote: > > > > > > >> -----Original Message----- > >> From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On > >> Behalf Of Alexander Graf > >> Sent: Friday, August 10, 2012 2:49 PM > >> To: Bhushan Bharat-R65777 > >> Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org > >> 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-ow...@vger.kernel.org > >>>> [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf > >>>> Sent: Tuesday, August 07, 2012 4:16 PM > >>>> To: Bhushan Bharat-R65777 > >>>> Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; 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....@freescale.com> > >>>>> [bharat.bhus...@freescale.com: Substantial changes] > >>>>> Signed-off-by: Bharat Bhushan <bharat.bhus...@freescale.com> > >>>>> --- > >>>>> 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.
I will try to remove some variable or at least add comment to support readability . Thanks -Bharat -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
