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@xxxxxxxxxx>
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 ]
