[PATCH v4 02/14] KVM: arm64: Group setting traps for protected VMs by control register

[Fuad Tabba]

Hi Quentin,

On Fri, 6 Dec 2024 at 16:44, Quentin Perret <qperret at google.com> wrote:
>
> On Monday 02 Dec 2024 at 15:47:29 (+0000), Fuad Tabba wrote:
> > +static void pvm_init_traps_cptr(struct kvm_vcpu *vcpu)
> >  {
> > -     const u64 feature_ids = pvm_read_id_reg(vcpu, SYS_ID_AA64DFR0_EL1);
> > -     u64 mdcr_set = 0;
> > -     u64 mdcr_clear = 0;
> > -     u64 cptr_set = 0;
> > +     const u64 id_aa64pfr0 = pvm_read_id_reg(vcpu, SYS_ID_AA64PFR0_EL1);
> > +     const u64 id_aa64pfr1 = pvm_read_id_reg(vcpu, SYS_ID_AA64PFR1_EL1);
> > +     const u64 id_aa64dfr0 = pvm_read_id_reg(vcpu, SYS_ID_AA64DFR0_EL1);
> > +     u64 val = vcpu->arch.cptr_el2;
> >
> > -     /* Trap/constrain PMU */
> > -     if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer), feature_ids)) {
> > -             mdcr_set |= MDCR_EL2_TPM | MDCR_EL2_TPMCR;
> > -             mdcr_clear |= MDCR_EL2_HPME | MDCR_EL2_MTPME |
> > -                           MDCR_EL2_HPMN_MASK;
> > +     if (!has_hvhe()) {
> > +             val |= CPTR_NVHE_EL2_RES1;
> > +             val &= ~(CPTR_NVHE_EL2_RES0);
>
> It's a bit odd to do that at the beginning of the function no? Perhaps
> do it last to be sure to overwrite all the changes below?
>
> Also, not sure we need the RES1 part here, kvm_get_reset_cptr_el2()
> should have set those for us.

I agree on both, and by the end of this series they end up being as
you suggest. The purpose of this particular patch is just the grouping
of the traps, while keeping everything else the way it is.

Cheers,
/fuad

>
> >       }
> >
> > -     /* Trap Debug */
> > -     if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DebugVer), feature_ids))
> > -             mdcr_set |= MDCR_EL2_TDRA | MDCR_EL2_TDA | MDCR_EL2_TDE;
> > -
> > -     /* Trap OS Double Lock */
> > -     if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DoubleLock), feature_ids))
> > -             mdcr_set |= MDCR_EL2_TDOSA;
> > +     /* Trap AMU */
> > +     if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_AMU), id_aa64pfr0))
> > +             val |= CPTR_EL2_TAM;
> >
> > -     /* Trap SPE */
> > -     if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMSVer), feature_ids)) {
> > -             mdcr_set |= MDCR_EL2_TPMS;
> > -             mdcr_clear |= MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT;
> > +     /* Trap SVE */
> > +     if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_SVE), id_aa64pfr0)) {
> > +             if (has_hvhe())
> > +                     val &= ~(CPACR_ELx_ZEN);
> > +             else
> > +                     val |= CPTR_EL2_TZ;
> >       }
> >
> > -     /* Trap Trace Filter */
> > -     if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_TraceFilt), feature_ids))
> > -             mdcr_set |= MDCR_EL2_TTRF;
> > +     /* No SME support in KVM. */
> > +     BUG_ON(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR1_EL1_SME), id_aa64pfr1));
> > +     if (has_hvhe())
> > +             val &= ~(CPACR_ELx_SMEN);
> > +     else
> > +             val |= CPTR_EL2_TSM;
> >
> >       /* Trap Trace */
> > -     if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_TraceVer), feature_ids)) {
> > +     if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_TraceVer), id_aa64dfr0)) {
> >               if (has_hvhe())
> > -                     cptr_set |= CPACR_EL1_TTA;
> > +                     val |= CPACR_EL1_TTA;
> >               else
> > -                     cptr_set |= CPTR_EL2_TTA;
> > +                     val |= CPTR_EL2_TTA;
> >       }
> >
> > -     /* Trap External Trace */
> > -     if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_ExtTrcBuff), feature_ids))
> > -             mdcr_clear |= MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT;
> > -
> > -     vcpu->arch.mdcr_el2 |= mdcr_set;
> > -     vcpu->arch.mdcr_el2 &= ~mdcr_clear;
> > -     vcpu->arch.cptr_el2 |= cptr_set;
> > -}
> > -
> > -/*
> > - * Set trap register values based on features in ID_AA64MMFR0.
> > - */
> > -static void pvm_init_traps_aa64mmfr0(struct kvm_vcpu *vcpu)
> > -{
> > -     const u64 feature_ids = pvm_read_id_reg(vcpu, SYS_ID_AA64MMFR0_EL1);
> > -     u64 mdcr_set = 0;
> > -
> > -     /* Trap Debug Communications Channel registers */
> > -     if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64MMFR0_EL1_FGT), feature_ids))
> > -             mdcr_set |= MDCR_EL2_TDCC;
> > -
> > -     vcpu->arch.mdcr_el2 |= mdcr_set;
> > +     vcpu->arch.cptr_el2 = val;
> >  }