Re: [PATCH v2 09/12] KVM: arm64: nv: Propagate CNTHCTL_EL2.EL1NV{P,V}CT bits

Wei-Lin Chang
2025-01-06T02:32:53Z

Hi Marc and other KVM ARM developers,

I have a question while learning about NV and reading the code:

On Tue, Dec 17, 2024 at 02:23:17PM +0000, Marc Zyngier wrote:
> Allow a guest hypervisor to trap accesses to CNT{P,V}CT_EL02 by
> propagating these trap bits to the host trap configuration.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> arch/arm64/kvm/arch_timer.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
> index 6f04f31c0a7f2..e5951e6eaf236 100644
> --- a/arch/arm64/kvm/arch_timer.c
> +++ b/arch/arm64/kvm/arch_timer.c
> @@ -824,6 +824,10 @@ static void timer_set_traps(struct kvm_vcpu *vcpu, struct timer_map *map)
> * Apply the enable bits that the guest hypervisor has requested for
> * its own guest. We can only add traps that wouldn't have been set
> * above.
> + * Implementation choices: we do not support NV when E2H=0 in the
> + * guest, and we don't support configuration where E2H is writable
> + * by the guest (either FEAT_VHE or FEAT_E2H0 is implemented, but
> + * not both). This simplifies the handling of the EL1NV* bits.

Previously I was not aware that KVM ARM has these constraints on guest's
view of NV and E2H, so I appreciate this comment very much. However,
after digging through the code I could not find anywhere where these
constraints are enforced, for example initially I thought I would find
ID_AA64MMFR2_EL1_NV being zeroed in limit_nv_id_regs(), or HCR_NV added
to the res0 mask of HCR_EL2, base on whether FEAT_VHE or FEAT_E2H0 is
available to the guest. But seems like in these places the code applies
constraints looking at the host's capabilities, not the guest's.
Do you mind providing some pointers for me to investigate the code mode?

Thank you so much, I'd appreciate any help.

Wei-Lin Chang

> */
> if (vcpu_has_nv(vcpu) && !is_hyp_ctxt(vcpu)) {
> u64 val = __vcpu_sys_reg(vcpu, CNTHCTL_EL2);
> @@ -834,6 +838,9 @@ static void timer_set_traps(struct kvm_vcpu *vcpu, struct timer_map *map)
>
> tpt |= !(val & (CNTHCTL_EL1PCEN << 10));
> tpc |= !(val & (CNTHCTL_EL1PCTEN << 10));
> +
> + tpt02 |= (val & CNTHCTL_EL1NVPCT);
> + tvt02 |= (val & CNTHCTL_EL1NVVCT);
> }
>
> /*
> --
> 2.39.2
>