> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]] On Behalf Of Alexander Graf
> Sent: Friday, January 22, 2010 7:13 PM
> To: Liu Yu-B13201
> Cc: [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH] kvmppc/booke: Set ESR and DEAR when
> inject interrupt to guest
>
>
> On 22.01.2010, at 11:54, Liu Yu wrote:
>
> > Old method prematurely sets ESR and DEAR.
> > Move this part after we decide to inject interrupt,
> > and make it more like hardware behave.
> >
> > Signed-off-by: Liu Yu <[email protected]>
> > ---
> > arch/powerpc/kvm/booke.c | 24 ++++++++++++++----------
> > arch/powerpc/kvm/emulate.c | 2 --
> > 2 files changed, 14 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> > index e283e44..a8ee148 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -84,7 +84,8 @@ static void
> kvmppc_booke_queue_irqprio(struct kvm_vcpu *vcpu,
> >
> > void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
> > {
> > - /* BookE does flags in ESR, so ignore those we get here */
> > + if (flags == SRR1_PROGTRAP)
> > + vcpu->arch.fault_esr = ESR_PTR;
>
> This looks odd. I was more thinking of "flags" being ESR in
> this context. So you'd save off flags to a field in the vcpu
> here and restore it later when the fault actually gets injected.
See the end.
>
> > kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_PROGRAM);
> > }
> >
> > @@ -115,14 +116,19 @@ static int
> kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
> > {
> > int allowed = 0;
> > ulong msr_mask;
> > + bool update_esr = false, update_dear = false;
> >
> > switch (priority) {
> > - case BOOKE_IRQPRIO_PROGRAM:
> > case BOOKE_IRQPRIO_DTLB_MISS:
> > - case BOOKE_IRQPRIO_ITLB_MISS:
> > - case BOOKE_IRQPRIO_SYSCALL:
>
> Is this correct? Was the previous order wrong according to the spec?
what order? just make switch items share the code.
>
> > case BOOKE_IRQPRIO_DATA_STORAGE:
> > + update_dear = true;
> > + /* fall through */
> > case BOOKE_IRQPRIO_INST_STORAGE:
> > + case BOOKE_IRQPRIO_PROGRAM:
> > + update_esr = true;
> > + /* fall through */
> > + case BOOKE_IRQPRIO_ITLB_MISS:
> > + case BOOKE_IRQPRIO_SYSCALL:
> > case BOOKE_IRQPRIO_FP_UNAVAIL:
> > case BOOKE_IRQPRIO_SPE_UNAVAIL:
> > case BOOKE_IRQPRIO_SPE_FP_DATA:
> > @@ -157,6 +163,10 @@ static int
> kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
> > vcpu->arch.srr0 = vcpu->arch.pc;
> > vcpu->arch.srr1 = vcpu->arch.msr;
> > vcpu->arch.pc = vcpu->arch.ivpr |
> vcpu->arch.ivor[priority];
> > + if (update_esr == true)
> > + vcpu->arch.esr = vcpu->arch.fault_esr;
>
> I'd rather like to see new fields used for that.
> > @@ -286,15 +295,12 @@ int kvmppc_handle_exit(struct kvm_run
> *run, struct kvm_vcpu *vcpu,
> > break;
> >
> > case BOOKE_INTERRUPT_DATA_STORAGE:
> > - vcpu->arch.dear = vcpu->arch.fault_dear;
> > - vcpu->arch.esr = vcpu->arch.fault_esr;
> > kvmppc_booke_queue_irqprio(vcpu,
> BOOKE_IRQPRIO_DATA_STORAGE);
>
> kvmppc_booke_queue_data_storage(vcpu, vcpu->arch.fault_esr,
> vcpu->arch.fault_dear);
>
> > kvmppc_account_exit(vcpu, DSI_EXITS);
> > r = RESUME_GUEST;
> > break;
> >
> > case BOOKE_INTERRUPT_INST_STORAGE:
> > - vcpu->arch.esr = vcpu->arch.fault_esr;
> > kvmppc_booke_queue_irqprio(vcpu,
> BOOKE_IRQPRIO_INST_STORAGE);
>
> kvmppc_booke_queue_inst_storage(vcpu, vcpu->arch.fault_esr);
>
Not sure if this is redundant, as we already have fault_esr.
Or should we ignore what hareware is and create a new esr to guest?
>
> > - vcpu->arch.dear = vcpu->arch.fault_dear;
> > - vcpu->arch.esr = vcpu->arch.fault_esr;
> > kvmppc_mmu_dtlb_miss(vcpu);
> > kvmppc_account_exit(vcpu, DTLB_REAL_MISS_EXITS);
> > r = RESUME_GUEST;
> > diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
> > index b905623..4f6b9df 100644
> > --- a/arch/powerpc/kvm/emulate.c
> > +++ b/arch/powerpc/kvm/emulate.c
> > @@ -151,8 +151,6 @@ int kvmppc_emulate_instruction(struct
> kvm_run *run, struct kvm_vcpu *vcpu)
> > case OP_TRAP:
> > #ifdef CONFIG_PPC64
> > case OP_TRAP_64:
> > -#else
> > - vcpu->arch.esr |= ESR_PTR;
> > #endif
> > kvmppc_core_queue_program(vcpu, SRR1_PROGTRAP);
>
> kvmppc_core_queue_program(vcpu, vcpu->arch.esr | ESR_PTR);
>
This breaks book3s code.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
