Re: [PATCH 19/27] KVM: arm64: nv: Add trap forwarding for CNTHCTL_EL2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023-07-25 18:37, Eric Auger wrote:
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.

I'm adding the following:

/*
 * Warning, maximum confusion ahead.
 *
 * When E2H=0, CNTHCTL_EL2[1:0] are defined as EL1PCEN:EL1PCTEN
 * When E2H=1, CNTHCTL_EL2[11:10] are defined as EL1PTEN:EL1PCTEN
 *
 * Note the single letter difference? Yet, the bits have the same
 * function despite a different layout and a different name.
 *
 * We don't try to reconcile this mess. We just use the E2H=0 bits
 * to generate something that is in the E2H=1 format, and live with
 * it. You're welcome.
 */

Hopefully this will make things clearer. Not completely sure though.

+{
+	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>

Thanks!

        M.
--
Jazz is not dead. It just smells funny...



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux