On Thu, Jul 12, 2012 at 7:54 AM, Rusty Russell <rusty.russell@xxxxxxxxxx> wrote:
> Now we have a table for all the cp15 registers, we can drive a generic
> API. x86 already has one for sparse-numbered registers, so we simply
> reproduce that. The only difference is that our KVM_GET_MSR_INDEX_LIST
> is a per-vcpu ioctl; we can't know the MSRs until we know the cpu type.
>
> The numbering for the indices for coprocesors is simple, if userspace
> cares (it might not for simple save and restore): the upper 16 bits
> are the coprocessor number. If it's > 15, it's something else, for
> future expansion.
>
> Bit 15 indicates a 64-bit register. For 64 bit registers the bottom 4
> bits are CRm, the next 4 are opc1 (just like the MCRR/MRCC instruction
> encoding). For 32 bit registers, the bottom 4 bits are CRm, the next
> 3 are opc2, the next 4 CRn, and the next 3 opc1 (the same order as the
> MRC/MCR instruction encoding, but not the same bit positions).
>
> 64-bit coprocessor register:
> ...|19 18 17 16|15|14 13 12 11 10 9 8| 7 6 5 4 |3 2 1 0|
> ...0 0 | cp num | 1| 0 0 0 0 0 0 0| opc1 | CRm |
>
> 32-bit coprocessor register:
> ...|19 18 17 16|15|14|13 12 11|10 9 8 7 |6 5 4 |3 2 1 0|
> ...0 0 | cp num | 0| 0| opc1 | CRn | opc2 | CRm |
>
> Non-coprocessor register:
>
> | 32 31 30 29 28 27 26 25 24 23 22 21 20|19 18 17 16 15 ...
> | < some non-zero value > | ...
>
> Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> ---
> arch/arm/include/asm/kvm.h | 31 +++++
> arch/arm/include/asm/kvm_coproc.h | 7 ++
> arch/arm/kvm/arm.c | 29 +++++
> arch/arm/kvm/coproc.c | 237 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 304 insertions(+)
>
> diff --git a/arch/arm/include/asm/kvm.h b/arch/arm/include/asm/kvm.h
> index 09b5be3..d6531e8 100644
> --- a/arch/arm/include/asm/kvm.h
> +++ b/arch/arm/include/asm/kvm.h
> @@ -92,4 +92,35 @@ struct kvm_sync_regs {
> struct kvm_arch_memory_slot {
> };
>
> +/* Exactly like x86. */
> +struct kvm_msr_entry {
> + __u32 index;
> + __u32 reserved;
> + __u64 data;
> +};
makes sense to move this to include/linux/kvm.h ?
> +
> +/* 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_GET_MSR_INDEX_LIST */
> +struct kvm_msr_list {
> + __u32 nmsrs; /* number of msrs in entries */
> + __u32 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
> +
> #endif /* __ARM_KVM_H__ */
> diff --git a/arch/arm/include/asm/kvm_coproc.h b/arch/arm/include/asm/kvm_coproc.h
> index 6bed190..31e0b12 100644
> --- a/arch/arm/include/asm/kvm_coproc.h
> +++ b/arch/arm/include/asm/kvm_coproc.h
> @@ -25,4 +25,11 @@ int kvm_handle_cp14_load_store(struct kvm_vcpu *vcpu, struct kvm_run *run);
> 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_copy_msrindices(struct kvm_vcpu *vcpu, u32 __user *uindices);
> #endif /* __ARM_KVM_COPROC_H__ */
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 4cf8d3d..7ac8381 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -730,6 +730,35 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> return kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id, &irq_event);
> }
> #endif
> + case KVM_GET_MSR_INDEX_LIST: {
> + struct kvm_msr_list __user *user_msr_list = argp;
> + struct kvm_msr_list msr_list;
> + unsigned n;
> +
> + if (copy_from_user(&msr_list, user_msr_list, sizeof msr_list))
> + return -EFAULT;
> + n = msr_list.nmsrs;
> + msr_list.nmsrs = kvm_arm_num_guest_msrs(vcpu);
> + if (copy_to_user(user_msr_list, &msr_list, sizeof msr_list))
> + return -EFAULT;
> + if (n < msr_list.nmsrs)
> + return -E2BIG;
> + return kvm_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 64b4397..297b3a3 100644
> --- a/arch/arm/kvm/coproc.c
> +++ b/arch/arm/kvm/coproc.c
> @@ -16,6 +16,7 @@
> * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
> */
> #include <linux/mm.h>
> +#include <linux/uaccess.h>
> #include <asm/kvm_arm.h>
> #include <asm/kvm_host.h>
> #include <asm/kvm_emulate.h>
> @@ -555,3 +556,239 @@ int kvm_handle_cp15_32(struct kvm_vcpu *vcpu, struct kvm_run *run)
>
> return emulate_cp15(vcpu, ¶ms);
> }
> +
> +/******************************************************************************
> + * Userspace API
> + *****************************************************************************/
> +
> +/* Given a simple mask, get those bits. */
> +static inline u32 get_bits(u32 index, u32 mask)
> +{
> + return (index & mask) >> (ffs(mask)-1);
> +}
> +
> +static void index_to_params(u32 index, struct coproc_params *params)
> +{
> + if (get_bits(index, KVM_ARM_MSR_64_BIT_MASK)) {
> + 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->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);
> + }
> +}
> +
> +/* Decode an index value, and find the cp15 coproc_reg entry. */
> +static const struct coproc_reg *index_to_cp15(struct kvm_vcpu *vcpu, u32 index)
> +{
> + size_t num;
> + const struct coproc_reg *table, *r;
> + struct coproc_params params;
> +
> + index_to_params(index, ¶ms);
> +
> + table = get_target_table(vcpu->arch.target, &num);
> + r = find_reg(¶ms, table, num);
> + if (!r)
> + r = find_reg(¶ms, cp15_regs, ARRAY_SIZE(cp15_regs));
> +
> + /* Not saved in the cp15 array? */
> + if (r && !r->reg)
> + r = NULL;
> +
> + return r;
> +}
> +
> +static int get_msr(struct kvm_vcpu *vcpu, u32 index, u64 *val)
> +{
> + const struct coproc_reg *r;
> + u32 coproc = get_bits(index, KVM_ARM_MSR_COPROC_MASK);
> +
> + /* We only do cp15 for now. */
> + if (coproc != 15)
> + return -EINVAL;
> +
> + r = index_to_cp15(vcpu, index);
> + if (!r)
> + return -EINVAL;
> +
> + *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;
> + u32 coproc = get_bits(index, KVM_ARM_MSR_COPROC_MASK);
> +
> + /* We only do cp15 for now. */
> + if (coproc != 15)
> + return -EINVAL;
> +
> + r = index_to_cp15(vcpu, index);
> + if (!r)
> + return -EINVAL;
> +
> + vcpu->arch.cp15[r->reg] = val;
> + if (r->is_64)
> + vcpu->arch.cp15[r->reg+1] = (val >> 32);
> + return 0;
> +}
worth combining get_msr and set_msr with a 'bool write' parameter?
> +
> +/* 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;
> +}
> +
> +/**
> + * 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)
> +{
> + u32 i, index;
> + u64 val;
> + u64 __user *uval;
> + int ret;
> +
> + 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;
> +}
> +
> +/**
> + * 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;
> +
> + 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;
get_user ?
> + if ((ret = set_msr(vcpu, index, val)) != 0)
> + return ret;
> + }
> + return 0;
> +}
> +
> +/**
> + * kvm_arm_num_guest_msrs - how many registers do we present via KVM_GET_MSR
> + *
> + * This is for special registers, particularly cp15.
> + */
> +unsigned long kvm_arm_num_guest_msrs(struct kvm_vcpu *vcpu)
> +{
> + const struct coproc_reg *table;
> + unsigned int i, total = 0;
> + size_t num;
> +
> + /* Count registers in arch-specific table. */
> + table = get_target_table(vcpu->arch.target, &num);
> + for (i = 0; i < num; i++) {
> + if (table[i].reg)
> + total++;
> + }
> +
> + /* FIXME: Assumes no duplicates! */
If this is a FIXME, shouldn't we just fix it (allocate a table of the
registers, zero it out, set a bit and only count once)? I prefer not
merging new FIXMEs if possible.
> + table = cp15_regs;
> + num = ARRAY_SIZE(cp15_regs);
> + for (i = 0; i < num; i++) {
> + if (table[i].reg)
> + total++;
> + }
> +
> + return total;
> +}
> +
> +/* 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);
spaces around the - operator iirc.
> +}
> +
> +static u32 cp15_to_index(const struct coproc_reg *reg)
> +{
> + u32 val = set_bits(15, KVM_ARM_MSR_COPROC_MASK);
> + 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);
> + } 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);
> + }
> + return val;
> +}
> +
> +/**
> + * kvm_arm_msridx - what's the index value for the nth register.
this is not the name of the function
> + *
> + * This is for special registers, particularly cp15.
> + */
> +int kvm_copy_msrindices(struct kvm_vcpu *vcpu, u32 __user *uindices)
shouldn't this be kvm_arm_copy_msrindicies for consistency?
> +{
> + const struct coproc_reg *table;
> + unsigned int i;
> + size_t num;
> +
> + /* First give the target-specific registers' indices. */
> + table = get_target_table(vcpu->arch.target, &num);
> + for (i = 0; i < num; i++) {
> + /* Ingore registers we trap but don't save. */
> + if (!table[i].reg)
> + continue;
> + if (put_user(cp15_to_index(&table[i]), uindices))
> + return -EFAULT;
> + uindices++;
> + }
> +
> + /* Finally give the architected registers' indices. */
> + table = cp15_regs;
> + num = ARRAY_SIZE(cp15_regs);
> + for (i = 0; i < num; i++) {
> + /* Ingore registers we trap but don't save. */
> + if (!table[i].reg)
> + continue;
> + if (put_user(cp15_to_index(&table[i]), uindices))
> + return -EFAULT;
> + uindices++;
> + }
> + return 0;
> +}
ok, this is pretty damn nice. Good argument for driving the cp15
register reset from here ;)
-Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
