Re: [PATCH 16/17] ARM: KVM: add the guest-visible host cp15 registers to the API.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jul 12, 2012 at 7:55 AM, Rusty Russell <rusty.russell@xxxxxxxxxx> wrote:
> For futureproofing, we need to tell QEMU about the CP15 registers the
> host lets the guest access.
>
> It will need this information to restore a current guest on a future
> CPU or perhaps a future KVM which allow some of these to be changed.
>
> We use a separate table for these, as they're only for the userspace API.
>
> Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> ---
>  arch/arm/include/asm/kvm_coproc.h |    2 +
>  arch/arm/kvm/arm.c                |    1 +
>  arch/arm/kvm/coproc.c             |  125 +++++++++++++++++++++++++++++++++++--
>  3 files changed, 124 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_coproc.h b/arch/arm/include/asm/kvm_coproc.h
> index 31e0b12..c950bb1 100644
> --- a/arch/arm/include/asm/kvm_coproc.h
> +++ b/arch/arm/include/asm/kvm_coproc.h
> @@ -32,4 +32,6 @@ 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);
> +
> +void kvm_fixed_coproc_table_init(void);
>  #endif /* __ARM_KVM_COPROC_H__ */
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 7ac8381..15283c9 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -941,6 +941,7 @@ int kvm_arch_init(void *opaque)
>         if (err)
>                 goto out_err;
>
> +       kvm_fixed_coproc_table_init();
>         return 0;
>  out_err:
>         return err;
> diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
> index 297b3a3..e58d712 100644
> --- a/arch/arm/kvm/coproc.c
> +++ b/arch/arm/kvm/coproc.c
> @@ -605,6 +605,107 @@ static const struct coproc_reg *index_to_cp15(struct kvm_vcpu *vcpu, u32 index)
>         return r;
>  }
>
> +/*
> + * These are the fixed cp15 registers: we let the guest see the host
> + * versions of these, so they're part of the guest state.
> + *

took me the better part of 10 minutes to finally figure out what you
meant by fixed, as in, read-only host state, that never changes, at
all. (right?).

I think the comment could add some motivation about why user space
would want to know about the underlying hardware to know how to adapt
for e.g. migration scenarios, if that is in fact the point.

> + * A future chip or future kvm may allow these to be changed.

hmm, this confuses me :-/

> + */
> +/* Unfortunately, there's no register-argument for mrc, so generate. */
> +#define FUNCTION_FOR32(crn, crm, op1, op2, name)                       \
> +       static void get_##name(struct kvm_vcpu *v,                      \
> +                              const struct coproc_reg *r)              \
> +       {                                                               \
> +               u32 val;                                                \
> +                                                                       \
> +               asm volatile("mrc p15, " __stringify(op1)               \
> +                            ", %0, c" __stringify(crn)                 \
> +                            ", c" __stringify(crm)                     \
> +                            ", " __stringify(op2) "\n" : "=r" (val));  \
> +               ((struct coproc_reg *)r)->val = val;                    \
> +       }
> +
> +FUNCTION_FOR32(0, 0, 0, 1, CTR)
> +FUNCTION_FOR32(0, 0, 0, 2, TCMTR)
> +FUNCTION_FOR32(0, 0, 0, 3, TLBTR)
> +FUNCTION_FOR32(0, 0, 0, 6, REVIDR)
> +FUNCTION_FOR32(0, 1, 0, 0, ID_PFR0)
> +FUNCTION_FOR32(0, 1, 0, 1, ID_PFR1)
> +FUNCTION_FOR32(0, 1, 0, 2, ID_DFR0)
> +FUNCTION_FOR32(0, 1, 0, 3, ID_AFR0)
> +FUNCTION_FOR32(0, 1, 0, 4, ID_MMFR0)
> +FUNCTION_FOR32(0, 1, 0, 5, ID_MMFR1)
> +FUNCTION_FOR32(0, 1, 0, 6, ID_MMFR2)
> +FUNCTION_FOR32(0, 1, 0, 7, ID_MMFR3)
> +FUNCTION_FOR32(0, 2, 0, 0, ID_ISAR0)
> +FUNCTION_FOR32(0, 2, 0, 1, ID_ISAR1)
> +FUNCTION_FOR32(0, 2, 0, 2, ID_ISAR2)
> +FUNCTION_FOR32(0, 2, 0, 3, ID_ISAR3)
> +FUNCTION_FOR32(0, 2, 0, 4, ID_ISAR4)
> +FUNCTION_FOR32(0, 2, 0, 5, ID_ISAR5)
> +FUNCTION_FOR32(0, 0, 1, 0, CSSIDR)
> +FUNCTION_FOR32(0, 0, 1, 1, CLIDR)
> +FUNCTION_FOR32(0, 0, 1, 7, AIDR)
> +
> +/* ->val is filled in by kvm_fixed_coproc_table_init() */
> +static struct coproc_reg fixed_cp15[] = {
> +       { CRn( 0), CRm( 0), Op1( 0), Op2( 1), is32, NULL, get_CTR },
> +       { CRn( 0), CRm( 0), Op1( 0), Op2( 2), is32, NULL, get_TCMTR },
> +       { CRn( 0), CRm( 0), Op1( 0), Op2( 3), is32, NULL, get_TLBTR },
> +       { CRn( 0), CRm( 0), Op1( 0), Op2( 6), is32, NULL, get_REVIDR },
> +
> +       { CRn( 0), CRm( 1), Op1( 0), Op2( 0), is32, NULL, get_ID_PFR0 },
> +       { CRn( 0), CRm( 1), Op1( 0), Op2( 1), is32, NULL, get_ID_PFR1 },
> +       { CRn( 0), CRm( 1), Op1( 0), Op2( 2), is32, NULL, get_ID_DFR0 },
> +       { CRn( 0), CRm( 1), Op1( 0), Op2( 3), is32, NULL, get_ID_AFR0 },
> +       { CRn( 0), CRm( 1), Op1( 0), Op2( 4), is32, NULL, get_ID_MMFR0 },
> +       { CRn( 0), CRm( 1), Op1( 0), Op2( 5), is32, NULL, get_ID_MMFR1 },
> +       { CRn( 0), CRm( 1), Op1( 0), Op2( 6), is32, NULL, get_ID_MMFR2 },
> +       { CRn( 0), CRm( 1), Op1( 0), Op2( 7), is32, NULL, get_ID_MMFR3 },
> +
> +       { CRn( 0), CRm( 2), Op1( 0), Op2( 0), is32, NULL, get_ID_ISAR0 },
> +       { CRn( 0), CRm( 2), Op1( 0), Op2( 1), is32, NULL, get_ID_ISAR1 },
> +       { CRn( 0), CRm( 2), Op1( 0), Op2( 2), is32, NULL, get_ID_ISAR2 },
> +       { CRn( 0), CRm( 2), Op1( 0), Op2( 3), is32, NULL, get_ID_ISAR3 },
> +       { CRn( 0), CRm( 2), Op1( 0), Op2( 4), is32, NULL, get_ID_ISAR4 },
> +       { CRn( 0), CRm( 2), Op1( 0), Op2( 5), is32, NULL, get_ID_ISAR5 },
> +
> +       { CRn( 0), CRm( 0), Op1( 1), Op2( 0), is32, NULL, get_CSSIDR },
> +       { CRn( 0), CRm( 0), Op1( 1), Op2( 1), is32, NULL, get_CLIDR },
> +       { CRn( 0), CRm( 0), Op1( 1), Op2( 7), is32, NULL, get_AIDR },
> +};
> +
> +static int get_fixed_cp15(u32 index, u64 *val)
> +{
> +       struct coproc_params params;
> +       const struct coproc_reg *r;
> +
> +       index_to_params(index, &params);
> +       r = find_reg(&params, fixed_cp15, ARRAY_SIZE(fixed_cp15));
> +       if (!r)
> +               return -EINVAL;
> +
> +       *val = r->val;
> +       return 0;
> +}
> +
> +static int set_fixed_cp15(u32 index, u64 val)
> +{
> +       struct coproc_params params;
> +       const struct coproc_reg *r;
> +
> +       index_to_params(index, &params);
> +       r = find_reg(&params, fixed_cp15, ARRAY_SIZE(fixed_cp15));
> +       if (!r)
> +               return -EINVAL;
> +
> +       /* This is what we mean by fixed: you can't change it. */
> +       if (r->val != val)
> +               return -EINVAL;
> +
> +       return 0;
> +}
> +
>  static int get_msr(struct kvm_vcpu *vcpu, u32 index, u64 *val)
>  {
>         const struct coproc_reg *r;
> @@ -616,7 +717,7 @@ static int get_msr(struct kvm_vcpu *vcpu, u32 index, u64 *val)
>
>         r = index_to_cp15(vcpu, index);
>         if (!r)
> -               return -EINVAL;
> +               return get_fixed_cp15(index, val);
>
>         *val = vcpu->arch.cp15[r->reg];
>         if (r->is_64)
> @@ -635,7 +736,7 @@ static int set_msr(struct kvm_vcpu *vcpu, u32 index, u64 val)
>
>         r = index_to_cp15(vcpu, index);
>         if (!r)
> -               return -EINVAL;
> +               return set_fixed_cp15(index, val);
>
>         vcpu->arch.cp15[r->reg] = val;
>         if (r->is_64)
> @@ -714,7 +815,7 @@ int kvm_arm_set_msrs(struct kvm_vcpu *vcpu,
>  unsigned long kvm_arm_num_guest_msrs(struct kvm_vcpu *vcpu)
>  {
>         const struct coproc_reg *table;
> -       unsigned int i, total = 0;
> +       unsigned int i, total = ARRAY_SIZE(fixed_cp15);
>         size_t num;
>
>         /* Count registers in arch-specific table. */
> @@ -768,7 +869,14 @@ int kvm_copy_msrindices(struct kvm_vcpu *vcpu, u32 __user *uindices)
>         unsigned int i;
>         size_t num;
>
> -       /* First give the target-specific registers' indices. */
> +       /* First give them all the fixed registers' indices. */
> +       for (i = 0; i < ARRAY_SIZE(fixed_cp15); i++) {
> +               if (put_user(cp15_to_index(&fixed_cp15[i]), uindices))
> +                       return -EFAULT;
> +               uindices++;
> +       }
> +
> +       /* Next 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. */
> @@ -792,3 +900,12 @@ int kvm_copy_msrindices(struct kvm_vcpu *vcpu, u32 __user *uindices)
>         }
>         return 0;
>  }
> +
> +void kvm_fixed_coproc_table_init(void)
> +{
> +       unsigned int i;
> +
> +       /* We abuse the reset function to overwrite the table itself. */
> +       for (i = 0; i < ARRAY_SIZE(fixed_cp15); i++)
> +               fixed_cp15[i].reset(NULL, &fixed_cp15[i]);
> +}


looks good to me otherwise.

-Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm


[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux