Re: [PATCH v2 01/12] KVM: arm64: nv: Add handling of EL2-specific timer registers

On Sat, Dec 21, 2024 at 09:57:44AM +0000, Marc Zyngier wrote:

On Sat, 21 Dec 2024 01:38:28 +0000,
Oliver Upton [off-list ref] wrote:

quoted

On Tue, Dec 17, 2024 at 02:23:09PM +0000, Marc Zyngier wrote:

quoted

@@ -3879,9 +4020,11 @@ static const struct sys_reg_desc cp15_64_regs[] = {
 	{ SYS_DESC(SYS_AARCH32_CNTPCT),	      access_arch_timer },
 	{ Op1( 1), CRn( 0), CRm( 2), Op2( 0), access_vm_reg, NULL, TTBR1_EL1 },
 	{ Op1( 1), CRn( 0), CRm(12), Op2( 0), access_gic_sgi }, /* ICC_ASGI1R */
+	{ SYS_DESC(SYS_AARCH32_CNTVCT),	      access_arch_timer },
 	{ Op1( 2), CRn( 0), CRm(12), Op2( 0), access_gic_sgi }, /* ICC_SGI0R */
 	{ SYS_DESC(SYS_AARCH32_CNTP_CVAL),    access_arch_timer },
 	{ SYS_DESC(SYS_AARCH32_CNTPCTSS),     access_arch_timer },
+	{ SYS_DESC(SYS_AARCH32_CNTVCTSS),     access_arch_timer },
 };

Huh. You know, I had always thought we hid 32-bit EL0 from nested
guests, but I now realize that isn't the case. Of course, we don't have
the necessary trap reflection for exits that came out of a 32-bit EL0,
nor should we bother.

Of the 4 NV2 implementations I'm aware of (Neoverse-V1, Neoverse-V2,
AmpereOne, M2) only Neoverse-V1 supports 32-bit userspace. And even
then, a lot of deployments of V1 have a broken NV2 implementation.

What do you think about advertising a 64-bit only EL0 for nested VMs?

I'm completely OK with that.

Actually, we already nuke the guest if exiting from 32bit context, no
matter the EL (vcpu_mode_is_bad_32bit() is where this happens).  But
we're missing the ID_AA64PFR0_EL1.EL0 sanitising, which is a bug. I'll
send a patch shortly.

Now, for this particular patch, I still think we should gracefully
handle access to the EL1 timer from a 32bit capable, non-NV guest.
Just in case we end-up with a CPU with a broken CNTVOFF_EL2 *and*
32bit capability.

In the end, it doesn't cost us much to support this case, and it helps
that we can verify that we handle all registers without exception.

Thoughts?

Absolutely. The only reason I made the comment is because the 32-bit
changes had nudged me into thinking about it. Happy with the patch as
is.

-- 
Thanks,
Oliver