Hi Reiji,

On 11/17/21 7:43 AM, Reiji Watanabe wrote:
> Extend sys_regs[] of kvm_cpu_context for ID registers and save ID
> registers' sanitized value in the array for the vCPU at the first
> vCPU reset. Use the saved ones when ID registers are read by
> userspace (via KVM_GET_ONE_REG) or the guest.
> 
> Signed-off-by: Reiji Watanabe <rei...@google.com>
> ---
>  arch/arm64/include/asm/kvm_host.h | 10 +++++++
>  arch/arm64/kvm/sys_regs.c         | 43 +++++++++++++++++++------------
>  2 files changed, 37 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h 
> b/arch/arm64/include/asm/kvm_host.h
> index edbe2cb21947..72db73c79403 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -146,6 +146,14 @@ struct kvm_vcpu_fault_info {
>       u64 disr_el1;           /* Deferred [SError] Status Register */
>  };
>  
> +/*
> + * (Op0, Op1, CRn, CRm, Op2) of ID registers is (3, 0, 0, crm, op2),
> + * where 0<=crm<8, 0<=op2<8.
> + */
> +#define KVM_ARM_ID_REG_MAX_NUM 64
> +#define IDREG_IDX(id)                ((sys_reg_CRm(id) << 3) | 
> sys_reg_Op2(id))
> +#define IDREG_SYS_IDX(id)    (ID_REG_BASE + IDREG_IDX(id))
> +
>  enum vcpu_sysreg {
>       __INVALID_SYSREG__,   /* 0 is reserved as an invalid value */
>       MPIDR_EL1,      /* MultiProcessor Affinity Register */
> @@ -210,6 +218,8 @@ enum vcpu_sysreg {
>       CNTP_CVAL_EL0,
>       CNTP_CTL_EL0,
>  
> +     ID_REG_BASE,
> +     ID_REG_END = ID_REG_BASE + KVM_ARM_ID_REG_MAX_NUM - 1,
>       /* Memory Tagging Extension registers */
>       RGSR_EL1,       /* Random Allocation Tag Seed Register */
>       GCR_EL1,        /* Tag Control Register */
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index e3ec1a44f94d..5608d3410660 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -33,6 +33,8 @@
>  
>  #include "trace.h"
>  
> +static u64 __read_id_reg(const struct kvm_vcpu *vcpu, u32 id);
> +
>  /*
>   * All of this file is extremely similar to the ARM coproc.c, but the
>   * types are different. My gut feeling is that it should be pretty
> @@ -273,7 +275,7 @@ static bool trap_loregion(struct kvm_vcpu *vcpu,
>                         struct sys_reg_params *p,
>                         const struct sys_reg_desc *r)
>  {
> -     u64 val = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
> +     u64 val = __read_id_reg(vcpu, SYS_ID_AA64MMFR1_EL1);
>       u32 sr = reg_to_encoding(r);
>  
>       if (!(val & (0xfUL << ID_AA64MMFR1_LOR_SHIFT))) {
> @@ -1059,17 +1061,9 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu,
>       return true;
>  }
>  
> -/* Read a sanitised cpufeature ID register by sys_reg_desc */
> -static u64 read_id_reg(const struct kvm_vcpu *vcpu,
> -             struct sys_reg_desc const *r, bool raz)
> +static u64 __read_id_reg(const struct kvm_vcpu *vcpu, u32 id)
>  {
> -     u32 id = reg_to_encoding(r);
> -     u64 val;
> -
> -     if (raz)
> -             return 0;
> -
> -     val = read_sanitised_ftr_reg(id);
> +     u64 val = __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id));
>  
>       switch (id) {
>       case SYS_ID_AA64PFR0_EL1:
> @@ -1119,6 +1113,14 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
>       return val;
>  }
>  
> +static u64 read_id_reg(const struct kvm_vcpu *vcpu,
> +                    struct sys_reg_desc const *r, bool raz)
> +{
> +     u32 id = reg_to_encoding(r);
> +
> +     return raz ? 0 : __read_id_reg(vcpu, id);
> +}
> +
>  static unsigned int id_visibility(const struct kvm_vcpu *vcpu,
>                                 const struct sys_reg_desc *r)
>  {
> @@ -1178,6 +1180,16 @@ static unsigned int sve_visibility(const struct 
> kvm_vcpu *vcpu,
>       return REG_HIDDEN;
>  }
>  
> +static void reset_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc 
> *rd)
> +{
> +     u32 id = reg_to_encoding(rd);
> +
> +     if (vcpu_has_reset_once(vcpu))
> +             return;
> +
> +     __vcpu_sys_reg(vcpu, IDREG_SYS_IDX(id)) = read_sanitised_ftr_reg(id);
> +}
> +
>  static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
>                              const struct sys_reg_desc *rd,
>                              const struct kvm_one_reg *reg, void __user 
> *uaddr)
> @@ -1223,9 +1235,7 @@ static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
>  /*
>   * cpufeature ID register user accessors
>   *
> - * For now, these registers are immutable for userspace, so no values
> - * are stored, and for set_id_reg() we don't allow the effective value
> - * to be changed.
> + * We don't allow the effective value to be changed.
This change may be moved to a subsequent patch as this patch does not
change the behavior yet.
>   */
>  static int __get_id_reg(const struct kvm_vcpu *vcpu,
>                       const struct sys_reg_desc *rd, void __user *uaddr,
> @@ -1382,6 +1392,7 @@ static unsigned int mte_visibility(const struct 
> kvm_vcpu *vcpu,
>  #define ID_SANITISED(name) {                 \
>       SYS_DESC(SYS_##name),                   \
>       .access = access_id_reg,                \
> +     .reset  = reset_id_reg,                 \
>       .get_user = get_id_reg,                 \
>       .set_user = set_id_reg,                 \
>       .visibility = id_visibility,            \
> @@ -1837,8 +1848,8 @@ static bool trap_dbgdidr(struct kvm_vcpu *vcpu,
>       if (p->is_write) {
>               return ignore_write(vcpu, p);
>       } else {
> -             u64 dfr = read_sanitised_ftr_reg(SYS_ID_AA64DFR0_EL1);
> -             u64 pfr = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);
> +             u64 dfr = __read_id_reg(vcpu, SYS_ID_AA64DFR0_EL1);
> +             u64 pfr = __read_id_reg(vcpu, SYS_ID_AA64PFR0_EL1);
>               u32 el3 = !!cpuid_feature_extract_unsigned_field(pfr, 
> ID_AA64PFR0_EL3_SHIFT);
>  
>               p->regval = ((((dfr >> ID_AA64DFR0_WRPS_SHIFT) & 0xf) << 28) |
> 
Thanks

Eric

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to