Am 03.07.2014 um 18:18 schrieb "mihai.caraman@xxxxxxxxxxxxx" <mihai.caraman@xxxxxxxxxxxxx>: >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@xxxxxxx] >> Sent: Thursday, July 03, 2014 4:37 PM >> To: Caraman Mihai Claudiu-B02008; kvm-ppc@xxxxxxxxxxxxxxx >> Cc: kvm@xxxxxxxxxxxxxxx; linuxppc-dev@xxxxxxxxxxxxxxxx >> Subject: Re: [PATCH 3/5 v4] KVM: PPC: Book3s: Remove kvmppc_read_inst() >> function >> >> >>> On 28.06.14 00:49, Mihai Caraman wrote: >>> In the context of replacing kvmppc_ld() function calls with a version >> of >>> kvmppc_get_last_inst() which allow to fail, Alex Graf suggested this: >>> >>> "If we get EMULATE_AGAIN, we just have to make sure we go back into the >> guest. >>> No need to inject an ISI into the guest - it'll do that all by itself. >>> With an error returning kvmppc_get_last_inst we can just use completely >>> get rid of kvmppc_read_inst() and only use kvmppc_get_last_inst() >> instead." >>> >>> As a intermediate step get rid of kvmppc_read_inst() and only use >> kvmppc_ld() >>> instead. >>> >>> Signed-off-by: Mihai Caraman <mihai.caraman@xxxxxxxxxxxxx> >>> --- >>> v4: >>> - new patch >>> >>> arch/powerpc/kvm/book3s_pr.c | 85 ++++++++++++++++++----------------- >> --------- >>> 1 file changed, 35 insertions(+), 50 deletions(-) >>> >>> diff --git a/arch/powerpc/kvm/book3s_pr.c >> b/arch/powerpc/kvm/book3s_pr.c >>> index 15fd6c2..d247d88 100644 >>> --- a/arch/powerpc/kvm/book3s_pr.c >>> +++ b/arch/powerpc/kvm/book3s_pr.c >>> @@ -665,42 +665,6 @@ static void kvmppc_giveup_fac(struct kvm_vcpu >> *vcpu, ulong fac) >>> #endif >>> } >>> >>> -static int kvmppc_read_inst(struct kvm_vcpu *vcpu) >>> -{ >>> - ulong srr0 = kvmppc_get_pc(vcpu); >>> - u32 last_inst = kvmppc_get_last_inst(vcpu); >>> - int ret; >>> - >>> - ret = kvmppc_ld(vcpu, &srr0, sizeof(u32), &last_inst, false); >>> - if (ret == -ENOENT) { >>> - ulong msr = kvmppc_get_msr(vcpu); >>> - >>> - msr = kvmppc_set_field(msr, 33, 33, 1); >>> - msr = kvmppc_set_field(msr, 34, 36, 0); >>> - msr = kvmppc_set_field(msr, 42, 47, 0); >>> - kvmppc_set_msr_fast(vcpu, msr); >>> - kvmppc_book3s_queue_irqprio(vcpu, >> BOOK3S_INTERRUPT_INST_STORAGE); >>> - return EMULATE_AGAIN; >>> - } >>> - >>> - return EMULATE_DONE; >>> -} >>> - >>> -static int kvmppc_check_ext(struct kvm_vcpu *vcpu, unsigned int >> exit_nr) >>> -{ >>> - >>> - /* Need to do paired single emulation? */ >>> - if (!(vcpu->arch.hflags & BOOK3S_HFLAG_PAIRED_SINGLE)) >>> - return EMULATE_DONE; >>> - >>> - /* Read out the instruction */ >>> - if (kvmppc_read_inst(vcpu) == EMULATE_DONE) >>> - /* Need to emulate */ >>> - return EMULATE_FAIL; >>> - >>> - return EMULATE_AGAIN; >>> -} >>> - >>> /* Handle external providers (FPU, Altivec, VSX) */ >>> static int kvmppc_handle_ext(struct kvm_vcpu *vcpu, unsigned int >> exit_nr, >>> ulong msr) >>> @@ -1101,31 +1065,51 @@ program_interrupt: >>> case BOOK3S_INTERRUPT_VSX: >>> { >>> int ext_msr = 0; >>> + int emul; >>> + ulong pc; >>> + u32 last_inst; >>> >>> - switch (exit_nr) { >>> - case BOOK3S_INTERRUPT_FP_UNAVAIL: ext_msr = MSR_FP; break; >>> - case BOOK3S_INTERRUPT_ALTIVEC: ext_msr = MSR_VEC; break; >>> - case BOOK3S_INTERRUPT_VSX: ext_msr = MSR_VSX; break; >>> - } >>> + if (!(vcpu->arch.hflags & BOOK3S_HFLAG_PAIRED_SINGLE)) { >> >> Please make paired single emulation the unusual, if()'ed case, not the >> normal exit path :). > > Huh ... do you have more Book3s specific requests, it will be strange if > it will still work after all this blind changes :) Heh :). All I'm saying is that rather than if (no emulation) { foo(); break; ) ps_emulation(); break; We should do if (ps emulation) { ps_emulation(); break; } foo(); break; 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