Re: [PATCH v5 2/6] KVM: PPC: Book3S: add helper routines to detect endian

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

Hi Alex,

On 01/02/2014 09:05 PM, Alexander Graf wrote:
> 
> On 05.11.2013, at 18:22, Cédric Le Goater <clg@fr.ibm.com> wrote:
> 
>> They will be used to decide whether to byte-swap or not. When Little
>> Endian host kernels come, these routines will need to be changed
>> accordingly.
>>
>> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
>> ---
>> arch/powerpc/include/asm/kvm_book3s.h |   10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
>> index d11c089..22ec875 100644
>> --- a/arch/powerpc/include/asm/kvm_book3s.h
>> +++ b/arch/powerpc/include/asm/kvm_book3s.h
>> @@ -270,6 +270,16 @@ static inline ulong kvmppc_get_pc(struct kvm_vcpu *vcpu)
>> 	return vcpu->arch.pc;
>> }
>>
>> +static inline bool kvmppc_need_byteswap(struct kvm_vcpu *vcpu)
>> +{
>> +	return vcpu->arch.shared->msr & MSR_LE;
>> +}
>> +
>> +static inline bool kvmppc_is_bigendian(struct kvm_vcpu *vcpu)
>> +{
>> +	return !kvmppc_need_byteswap(vcpu);
> 
> This is logically reversed. kvmppc_need_byteswap should check kvmppc_is_bigendian(), 
> not the other way around.
> 
 
I think we should get rid of kvmppc_is_bigendian(). 

As you noted in a subsequent email, it ends up returning true when run 
for "little endian guests on little endian hosts", which is awkward and 
the way it is used in kvmppc_handle_load() and kvmppc_handle_store()
can be improved.

I will give it a try taking into account the other comments you made. 

C.