Hi Marc, On 7/12/23 16:58, Marc Zyngier wrote: > Describe the CNTHCTL_EL2 register, and associate it with all the sysregs > it allows to trap. > > Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx> > --- > arch/arm64/kvm/emulate-nested.c | 37 ++++++++++++++++++++++++++++++++- > 1 file changed, 36 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c > index 25e4842ac334..c07c0f3361d7 100644 > --- a/arch/arm64/kvm/emulate-nested.c > +++ b/arch/arm64/kvm/emulate-nested.c > @@ -98,9 +98,11 @@ enum coarse_grain_trap_id { > > /* > * Anything after this point requires a callback evaluating a > - * complex trap condition. Hopefully we'll never need this... > + * complex trap condition. Ugly stuff. > */ > __COMPLEX_CONDITIONS__, > + CGT_CNTHCTL_EL1PCTEN = __COMPLEX_CONDITIONS__, > + CGT_CNTHCTL_EL1PTEN, > }; > > static const struct trap_bits coarse_trap_bits[] = { > @@ -358,9 +360,37 @@ static const enum coarse_grain_trap_id *coarse_control_combo[] = { > > typedef enum trap_behaviour (*complex_condition_check)(struct kvm_vcpu *); > > +static u64 get_sanitized_cnthctl(struct kvm_vcpu *vcpu) > +{ > + u64 val = __vcpu_sys_reg(vcpu, CNTHCTL_EL2); > + > + if (!vcpu_el2_e2h_is_set(vcpu)) > + val = (val & (CNTHCTL_EL1PCEN | CNTHCTL_EL1PCTEN)) << 10; > + > + return val; don't you want to return only bits 10 & 11 to match the other condition? I would add a comment saying that When FEAT_VHE is implemented and HCR_EL2.E2H == 1: sanitized_cnthctl[11:10] = [EL1PTEN, EL1PCTEN] otherwise sanitized_cnthctl[11:10] = [EL1PCEN, EL1PCTEN] > +} > + > +static enum trap_behaviour check_cnthctl_el1pcten(struct kvm_vcpu *vcpu) > +{ > + if (get_sanitized_cnthctl(vcpu) & (CNTHCTL_EL1PCTEN << 10)) > + return BEHAVE_HANDLE_LOCALLY; > + > + return BEHAVE_FORWARD_ANY; > +} > + > +static enum trap_behaviour check_cnthctl_el1pten(struct kvm_vcpu *vcpu) or pcen. This is a bit confusing to see EL1PCEN below. But this is due to above sanitized CNTHCTL. Worth a comment to me. > +{ > + if (get_sanitized_cnthctl(vcpu) & (CNTHCTL_EL1PCEN << 10)) > + return BEHAVE_HANDLE_LOCALLY; > + > + return BEHAVE_FORWARD_ANY; > +} > + > #define CCC(id, fn) [id - __COMPLEX_CONDITIONS__] = fn > > static const complex_condition_check ccc[] = { > + CCC(CGT_CNTHCTL_EL1PCTEN, check_cnthctl_el1pcten), > + CCC(CGT_CNTHCTL_EL1PTEN, check_cnthctl_el1pten), > }; > > /* > @@ -855,6 +885,11 @@ static const struct encoding_to_trap_config encoding_to_cgt[] __initdata = { > SR_TRAP(SYS_TRBPTR_EL1, CGT_MDCR_E2TB), > SR_TRAP(SYS_TRBSR_EL1, CGT_MDCR_E2TB), > SR_TRAP(SYS_TRBTRG_EL1, CGT_MDCR_E2TB), > + SR_TRAP(SYS_CNTP_TVAL_EL0, CGT_CNTHCTL_EL1PTEN), > + SR_TRAP(SYS_CNTP_CVAL_EL0, CGT_CNTHCTL_EL1PTEN), > + SR_TRAP(SYS_CNTP_CTL_EL0, CGT_CNTHCTL_EL1PTEN), > + SR_TRAP(SYS_CNTPCT_EL0, CGT_CNTHCTL_EL1PCTEN), > + SR_TRAP(SYS_CNTPCTSS_EL0, CGT_CNTHCTL_EL1PCTEN), > }; > > static DEFINE_XARRAY(sr_forward_xa); Otherwise looks good to me Reviewed-by: Eric Auger <eric.auger@xxxxxxxxxx> Eric