[PATCH 1/4] KVM: arm64: add emulation for CTR_EL0 register
Marc Zyngier
maz at kernel.org
Mon Mar 18 04:45:14 PDT 2024
Please add all the reviewers and the relevant mailing lists.
On Mon, 18 Mar 2024 11:16:33 +0000,
Sebastian Ott <sebott at redhat.com> wrote:
>
> CTR_EL0 is currently handled as an invariant register, thus
> guests will be presented with the host value of that register.
> Add emulation for CTR_EL0 and maintain a per vcpu value. The
> only thing that is allowed to be changed compared to the host
> value is to switch off the DIC bit which describes Icache
> invalidation requirements.
>
> Signed-off-by: Sebastian Ott <sebott at redhat.com>
> ---
> arch/arm64/kvm/sys_regs.c | 44 +++++++++++++++++++++++++++++++--------
> 1 file changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 30253bd19917..b2019faa9d73 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1871,10 +1871,42 @@ static bool access_ctr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> if (p->is_write)
> return write_to_read_only(vcpu, p, r);
>
> - p->regval = read_sanitised_ftr_reg(SYS_CTR_EL0);
> + p->regval = __vcpu_sys_reg(vcpu, r->reg);
> return true;
> }
>
> +static u64 reset_ctr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd)
> +{
> + u64 val = read_sanitised_ftr_reg(SYS_CTR_EL0);
> +
> + __vcpu_sys_reg(vcpu, rd->reg) = val;
> + return __vcpu_sys_reg(vcpu, rd->reg);
I really don't think we should make this a per-CPU value, and instead
keep it a VM-wide value, just like any other ID register.
Also, rd->reg isn't set to anything useful, leading to memory
corruption (hint, there is no CTR_EL0 in the vcpu sysreg file).
> +}
> +
> +static int get_ctr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + u64 *val)
> +{
> + *val = __vcpu_sys_reg(vcpu, rd->reg);
> + return 0;
> +}
> +
> +static int set_ctr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> + u64 val)
> +{
> + u64 ctr_el0 = read_sanitised_ftr_reg(SYS_CTR_EL0);
> +
> + if (kvm_vm_has_ran_once(vcpu->kvm) &&
> + val != __vcpu_sys_reg(vcpu, rd->reg))
> + return -EBUSY;
> +
> + if (((ctr_el0 & ~CTR_EL0_DIC_MASK) != (val & ~CTR_EL0_DIC_MASK)) ||
> + ((ctr_el0 & CTR_EL0_DIC_MASK) < (val & CTR_EL0_DIC_MASK)))
> + return -EINVAL;
Why limit this to DIC only? Anything that advertises something
"weaker" than what the HW has, such as a smaller cache line size, is
equally valid.
> +
> + __vcpu_sys_reg(vcpu, rd->reg) = val;
> + return 0;
> +}
> +
> static bool access_clidr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> const struct sys_reg_desc *r)
> {
> @@ -2461,7 +2493,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> { SYS_DESC(SYS_CCSIDR2_EL1), undef_access },
> { SYS_DESC(SYS_SMIDR_EL1), undef_access },
> { SYS_DESC(SYS_CSSELR_EL1), access_csselr, reset_unknown, CSSELR_EL1 },
> - { SYS_DESC(SYS_CTR_EL0), access_ctr },
> + { SYS_DESC(SYS_CTR_EL0), access_ctr, .reset = reset_ctr,
> + .get_user = get_ctr, .set_user = set_ctr},
Now, who traps this? Since c876c3f182a5 ("KVM: arm64: Relax trapping
of CTR_EL0 when FEAT_EVT is available"), we don't trap it anymore when
FEAT_EVT is present. Surely you should account for this.
Also, we really shouldn't trap it unless the guest view is different,
as this has a very visible impact on any userspace.
M.
--
Without deviation from the norm, progress is not possible.
More information about the linux-arm-kernel
mailing list