Hi Fuad,
On Mon, Jan 24, 2022 at 9:17 AM Fuad Tabba <tabba@xxxxxxxxxx> wrote:
>
> Hi Reiji,
>
> On Thu, Jan 6, 2022 at 4:29 AM Reiji Watanabe <reijiw@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
