RE: [PATCH 2/3] KVM: PPC: booke: Allow multiple exception types

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Alexander Graf [mailto:agraf@xxxxxxx]
> Sent: Friday, August 10, 2012 4:27 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 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.

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" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux