On Tue, Aug 28, 2012 at 4:46 PM, Rusty Russell <rusty.russell@xxxxxxxxxx> wrote:
> Instead of overloading x86's KVM_GET_MSRS/KVM_SET_MSRS. The only basic
> difference is that the ids are 64 bit.
>
> Signed-off-by: Rusty Russell <rusty.russell@xxxxxxxxxx>
>
> diff --git a/arch/arm/include/asm/kvm.h b/arch/arm/include/asm/kvm.h
> index d040a2a..9838456 100644
> --- a/arch/arm/include/asm/kvm.h
> +++ b/arch/arm/include/asm/kvm.h
> @@ -85,35 +85,21 @@ struct kvm_sync_regs {
> struct kvm_arch_memory_slot {
> };
>
> -/* Based on x86, but we use KVM_GET_VCPU_MSR_INDEX_LIST. */
> -struct kvm_msr_entry {
> - __u32 index;
> - __u32 reserved;
> - __u64 data;
> -};
> -
> -/* for KVM_GET_MSRS and KVM_SET_MSRS */
> -struct kvm_msrs {
> - __u32 nmsrs; /* number of msrs in entries */
> - __u32 pad;
> -
> - struct kvm_msr_entry entries[0];
> -};
> -
> /* for KVM_VCPU_GET_MSR_INDEX_LIST */
> struct kvm_msr_list {
> - __u32 nmsrs; /* number of msrs in entries */
> - __u32 indices[0];
> + __u64 nmsrs; /* number of msrs in entries */
> + __u64 indices[0];
> };
>
> -/* If you need to interpret the index values, here's the key. */
> -#define KVM_ARM_MSR_COPROC_MASK 0xFFFF0000
> -#define KVM_ARM_MSR_64_BIT_MASK 0x00008000
> -#define KVM_ARM_MSR_64_OPC1_MASK 0x000000F0
> -#define KVM_ARM_MSR_64_CRM_MASK 0x0000000F
> -#define KVM_ARM_MSR_32_CRM_MASK 0x0000000F
> -#define KVM_ARM_MSR_32_OPC2_MASK 0x00000070
> -#define KVM_ARM_MSR_32_CRN_MASK 0x00000780
> -#define KVM_ARM_MSR_32_OPC1_MASK 0x00003800
> +/* If you need to interpret the index values, here are the bit offsets. */
> +#define KVM_REG_ARM_COPROC_START 16 /* Mask: 0xFFFF0000 */
> +#define KVM_REG_ARM_32_OPC2_START 0 /* Mask: 0x00000007 */
> +#define KVM_REG_ARM_32_OPC2_LEN 3
> +#define KVM_REG_ARM_OPC1_START 3 /* Mask: 0x00000078 */
> +#define KVM_REG_ARM_OPC1_LEN 4
> +#define KVM_REG_ARM_CRM_START 7 /* Mask: 0x00000780 */
> +#define KVM_REG_ARM_CRM_LEN 4
> +#define KVM_REG_ARM_32_CRN_START 11 /* Mask: 0x00007800 */
> +#define KVM_REG_ARM_32_CRN_LEN 4
>
> #endif /* __ARM_KVM_H__ */
> diff --git a/arch/arm/include/asm/kvm_coproc.h b/arch/arm/include/asm/kvm_coproc.h
> index 894574c..899b3d3 100644
> --- a/arch/arm/include/asm/kvm_coproc.h
> +++ b/arch/arm/include/asm/kvm_coproc.h
> @@ -28,11 +28,7 @@ int kvm_handle_cp14_access(struct kvm_vcpu *vcpu, struct kvm_run *run);
> int kvm_handle_cp15_32(struct kvm_vcpu *vcpu, struct kvm_run *run);
> int kvm_handle_cp15_64(struct kvm_vcpu *vcpu, struct kvm_run *run);
>
> -int kvm_arm_get_msrs(struct kvm_vcpu *vcpu,
> - struct kvm_msr_entry __user *entries, u32 num);
> -int kvm_arm_set_msrs(struct kvm_vcpu *vcpu,
> - struct kvm_msr_entry __user *entries, u32 num);
> unsigned long kvm_arm_num_guest_msrs(struct kvm_vcpu *vcpu);
> -int kvm_arm_copy_msrindices(struct kvm_vcpu *vcpu, u32 __user *uindices);
> +int kvm_arm_copy_msrindices(struct kvm_vcpu *vcpu, u64 __user *uindices);
> void kvm_coproc_table_init(void);
> #endif /* __ARM_KVM_COPROC_H__ */
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 1c0fa75..7548c95 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -25,6 +25,7 @@
> #define KVM_MEMORY_SLOTS 32
> #define KVM_PRIVATE_MEM_SLOTS 4
> #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
> +#define KVM_HAVE_ONE_REG
>
> #define NUM_FEATURES 0
>
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 4248aa1..55a2995 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -726,20 +726,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> return -E2BIG;
> return kvm_arm_copy_msrindices(vcpu, user_msr_list->indices);
> }
> - case KVM_GET_MSRS: {
> - struct kvm_msrs msrs;
> - struct kvm_msrs __user *umsrs = argp;
> - if (copy_from_user(&msrs, umsrs, sizeof(msrs)) != 0)
> - return -EFAULT;
> - return kvm_arm_get_msrs(vcpu, umsrs->entries, msrs.nmsrs);
> - }
> - case KVM_SET_MSRS: {
> - struct kvm_msrs msrs;
> - struct kvm_msrs __user *umsrs = argp;
> - if (copy_from_user(&msrs, umsrs, sizeof(msrs)) != 0)
> - return -EFAULT;
> - return kvm_arm_set_msrs(vcpu, umsrs->entries, msrs.nmsrs);
> - }
> default:
> return -EINVAL;
> }
> diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
> index 824e5a3..99de71b 100644
> --- a/arch/arm/kvm/coproc.c
> +++ b/arch/arm/kvm/coproc.c
> @@ -568,41 +568,53 @@ int kvm_handle_cp15_32(struct kvm_vcpu *vcpu, struct kvm_run *run)
> *****************************************************************************/
>
> /* Given a simple mask, get those bits. */
> -static inline u32 get_bits(u32 index, u32 mask)
> +static inline u32 get_bits(u32 index, u32 start, u32 len)
> {
> - return (index & mask) >> (ffs(mask) - 1);
> + return (index >> start) & ((1 << len)-1);
> }
>
> -static void index_to_params(u32 index, struct coproc_params *params)
> +static bool index_to_params(u64 index, struct coproc_params *params)
> {
> - if (get_bits(index, KVM_ARM_MSR_64_BIT_MASK)) {
> + switch (index & KVM_REG_SIZE_MASK) {
> + case KVM_REG_SIZE_U32:
> + params->is_64bit = false;
> + params->CRn = get_bits(index, KVM_REG_ARM_32_CRN_START,
> + KVM_REG_ARM_32_CRN_LEN);
> + params->CRm = get_bits(index, KVM_REG_ARM_CRM_START,
> + KVM_REG_ARM_CRM_LEN);
> + params->Op1 = get_bits(index, KVM_REG_ARM_OPC1_START,
> + KVM_REG_ARM_OPC1_LEN);
> + params->Op2 = get_bits(index, KVM_REG_ARM_32_OPC2_START,
> + KVM_REG_ARM_32_OPC2_LEN);
> + return true;
> + case KVM_REG_SIZE_U64:
> params->is_64bit = true;
> - params->CRm = get_bits(index, KVM_ARM_MSR_64_CRM_MASK);
> - params->Op1 = get_bits(index, KVM_ARM_MSR_64_OPC1_MASK);
> + params->CRm = get_bits(index, KVM_REG_ARM_CRM_START,
> + KVM_REG_ARM_CRM_LEN);
> + params->Op1 = get_bits(index, KVM_REG_ARM_OPC1_START,
> + KVM_REG_ARM_OPC1_LEN);
> params->Op2 = 0;
> params->CRn = 0;
> - } else {
> - params->is_64bit = false;
> - params->CRn = get_bits(index, KVM_ARM_MSR_32_CRN_MASK);
> - params->CRm = get_bits(index, KVM_ARM_MSR_32_CRM_MASK);
> - params->Op1 = get_bits(index, KVM_ARM_MSR_32_OPC1_MASK);
> - params->Op2 = get_bits(index, KVM_ARM_MSR_32_OPC2_MASK);
> + return true;
> + default:
> + return false;
> }
> }
>
> /* Decode an index value, and find the cp15 coproc_reg entry. */
> static const struct coproc_reg *index_to_coproc_reg(struct kvm_vcpu *vcpu,
> - u32 index)
> + u64 index)
> {
> size_t num;
> const struct coproc_reg *table, *r;
> struct coproc_params params;
>
> /* We only do cp15 for now. */
> - if (get_bits(index, KVM_ARM_MSR_COPROC_MASK != 15))
> + if (get_bits(index, KVM_REG_ARM_COPROC_START, 16) != 15)
> return NULL;
>
> - index_to_params(index, ¶ms);
> + if (!index_to_params(index, ¶ms))
> + return NULL;
>
> table = get_target_table(vcpu->arch.target, &num);
> r = find_reg(¶ms, table, num);
> @@ -689,30 +701,54 @@ static struct coproc_reg invariant_cp15[] = {
> { CRn( 0), CRm( 0), Op1( 1), Op2( 7), is32, NULL, get_AIDR },
> };
>
> -static int get_invariant_cp15(u32 index, u64 *val)
> +static int reg_from_user(void *val, const void __user *uaddr, u64 index)
> +{
> + /* This Just Works because we are little endian. */
> + if (copy_from_user(val, uaddr, KVM_REG_LEN(index)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static int reg_to_user(void __user *uaddr, const void *val, u64 index)
> +{
> + /* This Just Works because we are little endian. */
> + if (copy_to_user(uaddr, val, KVM_REG_LEN(index)) != 0)
> + return -EFAULT;
> + return 0;
> +}
> +
> +static int get_invariant_cp15(u64 index, void __user *uaddr)
> {
> struct coproc_params params;
> const struct coproc_reg *r;
>
> - index_to_params(index, ¶ms);
> + if (!index_to_params(index, ¶ms))
> + return -ENOENT;
> +
> r = find_reg(¶ms, invariant_cp15, ARRAY_SIZE(invariant_cp15));
> if (!r)
> return -ENOENT;
>
> - *val = r->val;
> - return 0;
> + return reg_to_user(uaddr, &r->val, index);
> }
>
> -static int set_invariant_cp15(u32 index, u64 val)
> +static int set_invariant_cp15(u64 index, void __user *uaddr)
> {
> struct coproc_params params;
> const struct coproc_reg *r;
> + int err;
> + u64 val = 0; /* Make sure high bits are 0 for 32-bit regs */
>
> - index_to_params(index, ¶ms);
> + if (!index_to_params(index, ¶ms))
> + return -ENOENT;
> r = find_reg(¶ms, invariant_cp15, ARRAY_SIZE(invariant_cp15));
> if (!r)
> return -ENOENT;
>
> + err = reg_from_user(&val, uaddr, index);
> + if (err)
> + return err;
> +
> /* This is what we mean by invariant: you can't change it. */
> if (r->val != val)
> return -EINVAL;
> @@ -720,95 +756,36 @@ static int set_invariant_cp15(u32 index, u64 val)
> return 0;
> }
>
> -static int get_msr(struct kvm_vcpu *vcpu, u32 index, u64 *val)
> +int kvm_arch_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> {
> const struct coproc_reg *r;
> + void __user *uaddr = (void __user *)(long)reg->addr;
>
> - r = index_to_coproc_reg(vcpu, index);
> - if (!r)
> - return get_invariant_cp15(index, val);
> -
> - *val = vcpu->arch.cp15[r->reg];
> - if (r->is_64)
> - *val |= ((u64)vcpu->arch.cp15[r->reg+1]) << 32;
> - return 0;
> -}
> -
> -static int set_msr(struct kvm_vcpu *vcpu, u32 index, u64 val)
> -{
> - const struct coproc_reg *r;
> + if ((reg->id & KVM_REG_ARCH_MASK) != KVM_REG_ARM)
> + return -EINVAL;
>
> - r = index_to_coproc_reg(vcpu, index);
> + r = index_to_coproc_reg(vcpu, reg->id);
> if (!r)
> - return set_invariant_cp15(index, val);
> + return get_invariant_cp15(reg->id, uaddr);
>
> - vcpu->arch.cp15[r->reg] = val;
> - if (r->is_64)
> - vcpu->arch.cp15[r->reg+1] = (val >> 32);
> - return 0;
> -}
> -
> -/* Return user adddress to get/set value from. */
> -static u64 __user *get_umsr(struct kvm_msr_entry __user *uentry, u32 *idx)
> -{
> - struct kvm_msr_entry entry;
> -
> - if (copy_from_user(&entry, uentry, sizeof(entry)))
> - return NULL;
> - *idx = entry.index;
> - return &uentry->data;
> + /* Note: copies two regs if size is 64 bit. */
> + return reg_to_user(uaddr, &vcpu->arch.cp15[r->reg], reg->id);
> }
>
> -/**
> - * kvm_arm_get_msrs - copy one or more special registers to userspace.
> - * @vcpu: the vcpu
> - * @entries: the array of entries
> - * @num: the number of entries
> - */
> -int kvm_arm_get_msrs(struct kvm_vcpu *vcpu,
> - struct kvm_msr_entry __user *entries, u32 num)
> +int kvm_arch_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> {
> - u32 i, index;
> - u64 val;
> - u64 __user *uval;
> - int ret;
> + const struct coproc_reg *r;
> + void __user *uaddr = (void __user *)(long)reg->addr;
>
> - for (i = 0; i < num; i++) {
> - uval = get_umsr(&entries[i], &index);
> - if (!uval)
> - return -EFAULT;
> - if ((ret = get_msr(vcpu, index, &val)) != 0)
> - return ret;
> - if (put_user(val, uval))
> - return -EFAULT;
> - }
> - return 0;
> -}
> + if ((reg->id & KVM_REG_ARCH_MASK) != KVM_REG_ARM)
> + return -EINVAL;
>
> -/**
> - * kvm_arm_set_msrs - copy one or more special registers from userspace.
> - * @vcpu: the vcpu
> - * @entries: the array of entries
> - * @num: the number of entries
> - */
> -int kvm_arm_set_msrs(struct kvm_vcpu *vcpu,
> - struct kvm_msr_entry __user *entries, u32 num)
> -{
> - u32 i, index;
> - u64 val;
> - u64 __user *uval;
> - int ret;
> + r = index_to_coproc_reg(vcpu, reg->id);
> + if (!r)
> + return set_invariant_cp15(reg->id, uaddr);
>
> - for (i = 0; i < num; i++) {
> - uval = get_umsr(&entries[i], &index);
> - if (!uval)
> - return -EFAULT;
> - if (copy_from_user(&val, uval, sizeof(val)) != 0)
> - return -EFAULT;
> - if ((ret = set_msr(vcpu, index, val)) != 0)
> - return ret;
> - }
> - return 0;
> + /* Note: copies two regs if size is 64 bit */
> + return copy_from_user(&vcpu->arch.cp15[r->reg], uaddr, reg->id);
> }
>
> static int cmp_reg(const struct coproc_reg *i1, const struct coproc_reg *i2)
> @@ -827,29 +804,24 @@ static int cmp_reg(const struct coproc_reg *i1, const struct coproc_reg *i2)
> return i1->Op2 - i2->Op2;
> }
>
> -/* Puts in the position indicated by mask (assumes val fits in mask) */
> -static inline u32 set_bits(u32 val, u32 mask)
> -{
> - return val << (ffs(mask)-1);
> -}
> -
> -static u32 cp15_to_index(const struct coproc_reg *reg)
> +static u64 cp15_to_index(const struct coproc_reg *reg)
> {
> - u32 val = set_bits(15, KVM_ARM_MSR_COPROC_MASK);
> + u64 val = (15 << KVM_REG_ARM_COPROC_START);
> if (reg->is_64) {
> - val |= set_bits(1, KVM_ARM_MSR_64_BIT_MASK);
> - val |= set_bits(reg->Op1, KVM_ARM_MSR_64_OPC1_MASK);
> - val |= set_bits(reg->CRm, KVM_ARM_MSR_64_CRM_MASK);
> + val |= KVM_REG_SIZE_U64;
> + val |= (reg->Op1 << KVM_REG_ARM_OPC1_START);
> + val |= (reg->CRm << KVM_REG_ARM_CRM_START);
> } else {
> - val |= set_bits(reg->Op1, KVM_ARM_MSR_32_OPC1_MASK);
> - val |= set_bits(reg->Op2, KVM_ARM_MSR_32_OPC2_MASK);
> - val |= set_bits(reg->CRm, KVM_ARM_MSR_32_CRM_MASK);
> - val |= set_bits(reg->CRn, KVM_ARM_MSR_32_CRN_MASK);
> + val |= KVM_REG_SIZE_U32;
> + val |= (reg->Op1 << KVM_REG_ARM_OPC1_START);
> + val |= (reg->Op2 << KVM_REG_ARM_32_OPC2_START);
> + val |= (reg->CRm << KVM_REG_ARM_CRM_START);
> + val |= (reg->CRn << KVM_REG_ARM_32_CRN_START);
> }
> return val;
> }
>
> -static bool copy_reg_to_user(const struct coproc_reg *reg, u32 __user **uind)
> +static bool copy_reg_to_user(const struct coproc_reg *reg, u64 __user **uind)
> {
> if (!*uind)
> return true;
> @@ -862,7 +834,7 @@ static bool copy_reg_to_user(const struct coproc_reg *reg, u32 __user **uind)
> }
>
> /* Assumed ordered tables, see kvm_coproc_table_init. */
> -static int walk_msrs(struct kvm_vcpu *vcpu, u32 __user *uind)
> +static int walk_msrs(struct kvm_vcpu *vcpu, u64 __user *uind)
> {
> const struct coproc_reg *i1, *i2, *end1, *end2;
> unsigned int total = 0;
> @@ -911,7 +883,7 @@ static int walk_msrs(struct kvm_vcpu *vcpu, u32 __user *uind)
> */
> unsigned long kvm_arm_num_guest_msrs(struct kvm_vcpu *vcpu)
> {
> - return ARRAY_SIZE(invariant_cp15) + walk_msrs(vcpu, (u32 __user *)NULL);
> + return ARRAY_SIZE(invariant_cp15) + walk_msrs(vcpu, (u64 __user *)NULL);
> }
>
> /**
> @@ -919,7 +891,7 @@ unsigned long kvm_arm_num_guest_msrs(struct kvm_vcpu *vcpu)
> *
> * This is for special registers, particularly cp15.
> */
> -int kvm_arm_copy_msrindices(struct kvm_vcpu *vcpu, u32 __user *uindices)
> +int kvm_arm_copy_msrindices(struct kvm_vcpu *vcpu, u64 __user *uindices)
> {
> unsigned int i;
> int err;
why do we still call this stuff MSR? Shouldn't we move away from that
term to something more generic?
-Christoffer
--
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
