[PATCH v8 6/6] KVM: arm64: Refactor writings for PMUVer/CSV2/CSV3

[Suraj Jitindar Singh]

On Wed, 2023-05-03 at 17:16 +0000, Jing Zhang wrote:
> Refactor writings for ID_AA64PFR0_EL1.[CSV2|CSV3],
> ID_AA64DFR0_EL1.PMUVer and ID_DFR0_ELF.PerfMon based on utilities
> introduced by ID register descriptor array.
> 
> Signed-off-by: Jing Zhang <jingzhangos at google.com>

Hi,

> ---
>  arch/arm64/include/asm/cpufeature.h |   1 +
>  arch/arm64/kernel/cpufeature.c      |   2 +-
>  arch/arm64/kvm/id_regs.c            | 361 ++++++++++++++++++--------
> --
>  3 files changed, 242 insertions(+), 122 deletions(-)
> 
> 

[ snip ]

> diff --git a/arch/arm64/kvm/id_regs.c b/arch/arm64/kvm/id_regs.c
> index 64d40aa395be..6cd56c9e6428 100644
> --- a/arch/arm64/kvm/id_regs.c
> +++ b/arch/arm64/kvm/id_regs.c
> @@ -18,6 +18,86 @@
>  
>  #include "sys_regs.h"
>  
> [ snip ]
>  
> +static u64 read_sanitised_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
> +                                         const struct sys_reg_desc
> *rd)
> +{
> +       u64 val;
> +       u32 id = reg_to_encoding(rd);
> +
> +       val = read_sanitised_ftr_reg(id);
> +       /*
> +        * The default is to expose CSV2 == 1 if the HW isn't
> affected.
> +        * Although this is a per-CPU feature, we make it global
> because
> +        * asymmetric systems are just a nuisance.
> +        *
> +        * Userspace can override this as long as it doesn't promise
> +        * the impossible.
> +        */
> +       if (arm64_get_spectre_v2_state() == SPECTRE_UNAFFECTED) {
> +               val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2);
> +               val |=
> FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2), 1);
> +       }
> +       if (arm64_get_meltdown_state() == SPECTRE_UNAFFECTED) {
> +               val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3);
> +               val |=
> FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3), 1);
> +       }
> +

I think the virtual GIC check also needs to be moved here from
kvm_arm_read_id_reg()

        if (kvm_vgic_global_state.type == VGIC_V3) {
                val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_GIC);
                val |=
FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_GIC), 1);
        }
 
Since the host supports GICv4.1 ID_AA64PFR0_EL1_GIC == 3 when qemu
tries to read this register then overwrite it with the same value it
read previously an error occurs. ID_AA64PFR0_EL1_GIC == 3 is the
"limit" value however this field will read as ID_AA64PFR0_EL1_GIC == 1
when a virtual GICv3 is in use. Thus when qemu tries to set
ID_AA64PFR0_EL1_GIC == 1, arm64_check_features() fails as those bits
aren't set in id_reg.val meaning that modifications aren't allowed.

Additionally this means it's possible to set ID_AA64PFR0_EL1_GIC == 3
from userspace however any reads will see this field as
ID_AA64PFR0_EL1_GIC == 1.

This all means the smp kvm_unit_tests failed. With the above change
they pass.

Thanks,
Suraj

> +       val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_AMU);
> +
> +       return val;
> +}
> +
>  static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
>                                const struct sys_reg_desc *rd,
>                                u64 val)
[ snip ]