[PATCH 3/5 v4] KVM: PPC: Book3s: Remove kvmppc_read_inst() function

[Alexander Graf]

Am 03.07.2014 um 18:18 schrieb "mihai.caraman at freescale.com" <mihai.caraman at freescale.com>:

>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf at suse.de]
>> Sent: Thursday, July 03, 2014 4:37 PM
>> To: Caraman Mihai Claudiu-B02008; kvm-ppc at vger.kernel.org
>> Cc: kvm at vger.kernel.org; linuxppc-dev at lists.ozlabs.org
>> 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 at freescale.com>
>>> ---
>>> 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