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

[Oliver Upton <oliver.upton@linux.dev>]

On Sat, Dec 21, 2024 at 09:57:44AM +0000, Marc Zyngier wrote:
> On Sat, 21 Dec 2024 01:38:28 +0000,
> Oliver Upton <oliver.upton@linux.dev> wrote:
> > 
> > On Tue, Dec 17, 2024 at 02:23:09PM +0000, Marc Zyngier wrote:
> > > @@ -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