[RFC PATCH v4 20/26] KVM: arm64: Trap disabled features of ID_AA64PFR0_EL1

[Reiji Watanabe]

Hi Fuad,

On Mon, Jan 24, 2022 at 9:17 AM Fuad Tabba <tabba at google.com> wrote:
>
> Hi Reiji,
>
> On Thu, Jan 6, 2022 at 4:29 AM Reiji Watanabe <reijiw at google.com> wrote:
> >
> > Add feature_config_ctrl for RAS and AMU, which are indicated in
> > ID_AA64PFR0_EL1, to program configuration registers to trap
> > guest's using those features when they are not exposed to the guest.
> >
> > Introduce trap_ras_regs() to change a behavior of guest's access to
> > the registers, which is currently raz/wi, depending on the feature's
> > availability for the guest (and inject undefined instruction
> > exception when guest's RAS register access are trapped and RAS is
> > not exposed to the guest).  In order to keep the current visibility
> > of the RAS registers from userspace (always visible), a visibility
> > function for RAS registers is not added.
> >
> > No code is added for AMU's access/visibility handler because the
> > current code already injects the exception for Guest's AMU register
> > access unconditionally because AMU is never exposed to the guest.
>
> I think it might be code to trap it anyway, in case AMU guest support
> is added in the future.

Yes, I will fix it.
(I forgot to update the comment above...)


> >
> > Signed-off-by: Reiji Watanabe <reijiw at google.com>
> > ---
> >  arch/arm64/kvm/sys_regs.c | 90 +++++++++++++++++++++++++++++++++++----
> >  1 file changed, 82 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 33893a501475..015d67092d5e 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -304,6 +304,63 @@ struct feature_config_ctrl {
> >         void    (*trap_activate)(struct kvm_vcpu *vcpu);
> >  };
> >
> > +enum vcpu_config_reg {
> > +       VCPU_HCR_EL2 = 1,
> > +       VCPU_MDCR_EL2,
> > +       VCPU_CPTR_EL2,
> > +};
> > +
> > +static void feature_trap_activate(struct kvm_vcpu *vcpu,
> > +                                 enum vcpu_config_reg cfg_reg,
> > +                                 u64 cfg_set, u64 cfg_clear)
> > +{
> > +       u64 *reg_ptr, reg_val;
> > +
> > +       switch (cfg_reg) {
> > +       case VCPU_HCR_EL2:
> > +               reg_ptr = &vcpu->arch.hcr_el2;
> > +               break;
> > +       case VCPU_MDCR_EL2:
> > +               reg_ptr = &vcpu->arch.mdcr_el2;
> > +               break;
> > +       case VCPU_CPTR_EL2:
> > +               reg_ptr = &vcpu->arch.cptr_el2;
> > +               break;
> > +       }
> > +
> > +       /* Clear/Set fields that are indicated by cfg_clear/cfg_set. */
> > +       reg_val = (*reg_ptr & ~cfg_clear);
> > +       reg_val |= cfg_set;
> > +       *reg_ptr = reg_val;
> > +}
> > +
> > +static void feature_ras_trap_activate(struct kvm_vcpu *vcpu)
> > +{
> > +       feature_trap_activate(vcpu, VCPU_HCR_EL2, HCR_TERR | HCR_TEA, HCR_FIEN);
>
> Covers all the flags for ras.
>
> > +}
> > +
> > +static void feature_amu_trap_activate(struct kvm_vcpu *vcpu)
> > +{
> > +       feature_trap_activate(vcpu, VCPU_CPTR_EL2, CPTR_EL2_TAM, 0);
>
> Covers the CPTR flags for AMU, but as you mentioned, does not
> explicitly clear HCR_AMVOFFEN.

In my understanding, clearing HCR_EL2.AMVOFFEN is not necessary as
CPTR_EL2.TAM == 1 traps the guest's accessing AMEVCNTR0<n>_EL0 and
AMEVCNTR1<n>_EL0 anyway (HCR_EL2.AMVOFFEN doesn't matter).
(Or is my understanding wrong ??)

Thanks,
Reiji