Re: [PATCH v3 3/3] KVM: PPC: Book3S: MMIO emulation support for little endian guests

[Cedric Le Goater <clg@fr.ibm.com>]

On 10/08/2013 05:36 PM, Alexander Graf wrote:
> 
> On 08.10.2013, at 17:31, Cédric Le Goater <clg@fr.ibm.com> wrote:
> 
>> MMIO emulation reads the last instruction executed by the guest
>> and then emulates. If the guest is running in Little Endian mode,
>> the instruction needs to be byte-swapped before being emulated.
>>
>> This patch stores the last instruction in the endian order of the
>> host, primarily doing a byte-swap if needed. The common code
>> which fetches 'last_inst' uses a helper routine kvmppc_need_byteswap().
>> and the exit paths for the Book3S PV and HR guests use their own
>> version in assembly.
>>
>> Finally, the meaning of the 'is_bigendian' argument of the
>> routines kvmppc_handle_load() of kvmppc_handle_store() is
>> slightly changed to represent an eventual reverse operation. This
>> is used in conjunction with kvmppc_is_bigendian() to determine if
>> the instruction being emulated should be byte-swapped.
>>
>> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
>> ---
>>
>> Changes in v3:
>>
>> - moved kvmppc_need_byteswap() in kvmppc_ld32. It previously was in
>>   kvmppc_ld_inst(). (Alexander Graf)
>>
>> Changes in v2:
>>
>> - replaced rldicl. by andi. to test the MSR_LE bit in the guest
>>   exit paths. (Paul Mackerras)
>>
>> - moved the byte swapping logic to kvmppc_handle_load() and 
>>   kvmppc_handle_load() by changing the is_bigendian parameter
>>   meaning. (Paul Mackerras)
>>
>> arch/powerpc/include/asm/kvm_book3s.h   |    9 ++++++++-
>> arch/powerpc/include/asm/kvm_ppc.h      |   10 +++++-----
>> arch/powerpc/kvm/book3s_hv_rmhandlers.S |   11 +++++++++++
>> arch/powerpc/kvm/book3s_segment.S       |   10 ++++++++++
>> arch/powerpc/kvm/emulate.c              |    1 -
>> arch/powerpc/kvm/powerpc.c              |   16 ++++++++++++----
>> 6 files changed, 46 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
>> index 4ee6c66..9403042 100644
>> --- a/arch/powerpc/include/asm/kvm_book3s.h
>> +++ b/arch/powerpc/include/asm/kvm_book3s.h
>> @@ -283,7 +283,14 @@ static inline bool kvmppc_is_bigendian(struct kvm_vcpu *vcpu)
>> static inline int kvmppc_ld32(struct kvm_vcpu *vcpu, ulong *eaddr,
>> 			      u32 *ptr, bool data)
>> {
>> -	return kvmppc_ld(vcpu, eaddr, sizeof(u32), ptr, data);
>> +	int ret;
>> +
>> +	ret = kvmppc_ld(vcpu, eaddr, sizeof(u32), ptr, data);
>> +
>> +	if (kvmppc_need_byteswap(vcpu))
>> +		*ptr = swab32(*ptr);
>> +
>> +	return ret;
>> }
>>
>> static inline int kvmppc_ld_inst(struct kvm_vcpu *vcpu, ulong *eaddr,
> 
> ... at which point this helper is pretty useless ;).

ok. This sounds like a request for a v4 ... :)

C.