On Tue, Aug 13, 2024 at 04:45:46PM +0100, Marc Zyngier wrote:
> On Tue, 13 Aug 2024 16:24:52 +0100,
> Joey Gouly <joey.gouly@xxxxxxx> wrote:
> >
> > On Tue, Aug 13, 2024 at 03:47:37PM +0100, Marc Zyngier wrote:
> > > Add the FEAT_S1PIE EL2 registers the sysreg descriptor array so that
> > > they can be handled as a trap.
> > >
> > > Access to these registers is conditionned on ID_AA64MMFR3_EL1.S1PIE
> > > being advertised.
> > >
> > > Similarly to other other changes, PIRE0_EL2 is guaranteed to trap
> > > thanks to the D22677 update to the architecture..
> > >
> > > Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> > > ---
> > > arch/arm64/kvm/sys_regs.c | 14 ++++++++++++++
> > > 1 file changed, 14 insertions(+)
> > >
> > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > > index 52250db3c122..a5f604e24e05 100644
> > > --- a/arch/arm64/kvm/sys_regs.c
> > > +++ b/arch/arm64/kvm/sys_regs.c
> > > @@ -346,6 +346,18 @@ static bool access_rw(struct kvm_vcpu *vcpu,
> > > return true;
> > > }
> > >
> > > +static bool check_s1pie_access_rw(struct kvm_vcpu *vcpu,
> > > + struct sys_reg_params *p,
> > > + const struct sys_reg_desc *r)
> > > +{
> > > + if (!kvm_has_feat(vcpu->kvm, ID_AA64MMFR3_EL1, S1PIE, IMP)) {
> > > + kvm_inject_undefined(vcpu);
> > > + return false;
> > > + }
> > > +
> > > + return access_rw(vcpu, p, r);
> > > +}
> > > +
> > > /*
> > > * See note at ARMv7 ARM B1.14.4 (TL;DR: S/W ops are not easily virtualized).
> > > */
> > > @@ -2827,6 +2839,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> > > EL2_REG(HPFAR_EL2, access_rw, reset_val, 0),
> > >
> > > EL2_REG(MAIR_EL2, access_rw, reset_val, 0),
> > > + EL2_REG(PIRE0_EL2, check_s1pie_access_rw, reset_val, 0),
> > > + EL2_REG(PIR_EL2, check_s1pie_access_rw, reset_val, 0),
> > > EL2_REG(AMAIR_EL2, access_rw, reset_val, 0),
> > >
> > > EL2_REG(VBAR_EL2, access_rw, reset_val, 0),
> >
> > I think we should also use this for PIR_EL1 / PIRE0_EL1? We have NULL for their access field.
> >
> > { SYS_DESC(SYS_PIR_EL1), NULL, reset_unknown, PIR_EL1 },
>
> I don't think we need this. In general, the EL1 FEAT_S1PIE registers
> are directly accessed by the VM, and do not trap.
>
> However, if the VM has been configured to not expose S1PIE, then we
> set the corresponding FGU bits in kvm_calculate_traps():
>
> if (!kvm_has_feat(kvm, ID_AA64MMFR3_EL1, S1PIE, IMP))
> kvm->arch.fgu[HFGxTR_GROUP] |= (HFGxTR_EL2_nPIRE0_EL1 |
> HFGxTR_EL2_nPIR_EL1);
>
> The effect of this is that we don't even make to the sysreg array, and
> inject an UNDEF directly from the point of decoding the trap (see the
> beginning of triage_sysreg_trap()).
>
> For EL2 registers, there is no concept of FGT since they always trap,
> so no architectural trick we can play to shortcut the handling.
> Therefore we make it to the handler and have to triage things there.
>
> Does it make sense?
Ah yes, forgot how that worked, thanks for the reminder!
There's another 'conditionned' typo in the commit message, but otherwise:
Reviewed-by: Joey Gouly <joey.gouly@xxxxxxx>
Thanks,
Joey
