Re: [PATCH v1 1/3] KVM: arm64: Initialize HCRX_EL2 traps in pKVM

[Marc Zyngier <maz@kernel.org>]

On Wed, 26 Feb 2025 12:44:49 +0000,
Fuad Tabba <tabba@google.com> wrote:
> 
> Hi Marc,
> 
> On Wed, 26 Feb 2025 at 02:45, Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Wed, 26 Feb 2025 10:07:56 +0000,
> > Oliver Upton <oliver.upton@linux.dev> wrote:
> > >
> > > Hi Fuad,
> > >
> > > Series LGTM overall, one comment:
> > >
> > > On Fri, Feb 14, 2025 at 03:02:56PM +0000, Fuad Tabba wrote:
> > > > Initialize and set the traps controlled by the HCRX_EL2 in pKVM
> > > > when the register is supported by the system.
> > > >
> > > > Signed-off-by: Fuad Tabba <tabba@google.com>
> > > > ---
> > > >  arch/arm64/kvm/hyp/nvhe/pkvm.c | 46 ++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 46 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > > > index 3927fe52a3dd..668ebec27f1b 100644
> > > > --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > > > +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > > > @@ -58,6 +58,30 @@ static void pkvm_vcpu_reset_hcr(struct kvm_vcpu *vcpu)
> > > >             vcpu->arch.hcr_el2 |= HCR_ATA;
> > > >  }
> > > >
> > > > +static void pkvm_vcpu_reset_hcrx(struct pkvm_hyp_vcpu *hyp_vcpu)
> > > > +{
> > > > +   struct kvm_vcpu *host_vcpu = hyp_vcpu->host_vcpu;
> > > > +   struct kvm_vcpu *vcpu = &hyp_vcpu->vcpu;
> > > > +
> > > > +   if (!cpus_have_final_cap(ARM64_HAS_HCX))
> > > > +           return;
> > > > +
> > > > +   /*
> > > > +    * In general, all HCRX_EL2 bits are gated by a feature.
> > > > +    * The only reason we can set SMPME without checking any
> > > > +    * feature is that its effects are not directly observable
> > > > +    * from the guest.
> > > > +    */
> > > > +   vcpu->arch.hcrx_el2 = HCRX_EL2_SMPME;
> > > > +
> > >
> > > The comment isn't wrong, but we don't support SME at all in KVM at this
> > > point.
> >
> > This is a copy/paste of what we have in kvm_calculate_traps(), and the
> > result of the removal of the dreaded HCRX_GUEST_FLAGS.
> >
> > > Any objection to dropping this bit? I can fix it when applying the
> > > series, no need to respin.
> >
> > Whatever we do, I think we should keep the two side of the trap
> > configuration in sync, as this is otherwise a cause of bugs.
> 
> I could take the initialization of hcrx part from
> kvm_calculate_traps() and place it in an inline function in
> kvm_emulate.h. This then would be called by kvm_calculate_traps() and
> pkvm_vcpu_init_traps().
> 
> What do you think?

That could be a good option indeed, irrespective of the fate of SMPME.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.