Re: [PATCH 02/20] KVM/MIPS32: Arch specific KVM data structures.

[David Daney <ddaney.cavm@gmail.com>]

On 11/01/2012 08:04 AM, Avi Kivity wrote:
> On 10/31/2012 05:18 PM, Sanjay Lal wrote:
>>
>> Signed-off-by: Sanjay Lal <sanjayl@kymasys.com>
>> ---
>>   arch/mips/include/asm/kvm.h      |  58 ++++
>>   arch/mips/include/asm/kvm_host.h | 672 +++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 730 insertions(+)
>>   create mode 100644 arch/mips/include/asm/kvm.h
>>   create mode 100644 arch/mips/include/asm/kvm_host.h
>>
>> diff --git a/arch/mips/include/asm/kvm.h b/arch/mips/include/asm/kvm.h
>> new file mode 100644
>> index 0000000..39bb715
>> --- /dev/null
>> +++ b/arch/mips/include/asm/kvm.h
>> @@ -0,0 +1,58 @@
>> +/*
>> +* This file is subject to the terms and conditions of the GNU General Public
>> +* License.  See the file "COPYING" in the main directory of this archive
>> +* for more details.
>> +*
>> +*
>> +* Copyright (C) 2012  MIPS Technologies, Inc.  All rights reserved.
>> +* Authors: Sanjay Lal <sanjayl@kymasys.com>
>> +*/
>> +
>> +
>> +#ifndef __LINUX_KVM_MIPS_H
>> +#define __LINUX_KVM_MIPS_H
>> +
>> +#include <linux/types.h>
>> +
>> +#define __KVM_MIPS
>> +
>> +#define N_MIPS_COPROC_REGS      32
>> +#define N_MIPS_COPROC_SEL   	8
>> +
>> +/* for KVM_GET_REGS and KVM_SET_REGS */
>> +struct kvm_regs {
>> +    __u32 gprs[32];
>> +    __u32 hi;
>> +    __u32 lo;
>> +    __u32 pc;
>> +
>> +    ulong cp0reg[N_MIPS_COPROC_REGS][N_MIPS_COPROC_SEL];
>> +};
>
> ulong changes size in 64-bit archs, requiring compat translations when
> issuing 32-bit syscalls on a 64-bit kernel.  I don't know MIPS enough to
> know whether that's a useful scenario.

For a 32-bit machine all the registers are 32-bits wide.

For a 64-bit machine, they are ... yes 64-bits wide.

Since this entire structure seems to be useless for a 64-bit 
implementation just use __u32 for everything.

This does beg the question though, how will you handle 64-bit machines?

Why not name the entire structure something like struct kvm_regs_mips32? 
  Then for the 64-bit implementation use struct kvm_regs_mips64.

I haven't studied it enough to know if you can do this.  if it needs to 
be called exactly 'struct kvm_regs', then you have to make everything 
__u64 and only populate the lower halves of the fields for mips32. 
Watch out for endian issues in this case.

David Daney