[RFC PATCH 51/55] KVM: SVM: Keep TPR[3:0] across exits without a guest CR8 write

[David Woodhouse]

From: David Woodhouse <dwmw@xxxxxxxxxxxx>

V_TPR holds only TPR[7:4], and sync_cr8_to_lapic() pushed it back into
the APIC after every VMRUN.  A guest that sets TASKPRI through the
APIC page or MSR loses the low nibble at the next exit: VTL1 in the
hyperv_vsm KUT writes 0xff and reads back 0xf0 ("VTL1 TASKPRI write
sticks"), and interrupts in the sub-class the guest meant to mask are
let through.  VMX keeps all eight TPR bits in the virtual-APIC page
and does not have this problem.

Hardware changes V_TPR only on a guest CR8 write.  Record the value
sync_lapic_to_cr8() loads on entry and push V_TPR back into the APIC
only when it differs, i.e. only when the guest actually wrote CR8.  A
CR8 write whose value matches the loaded TPR[7:4] is indistinguishable
from no write and now preserves TPR[3:0], where architecturally it
would clear it; the previous behaviour cleared TPR[3:0] on every exit
regardless.

Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
Assisted-by: LLM
---
 arch/x86/kvm/svm/svm.c | 11 ++++++++++-
 arch/x86/kvm/svm/svm.h |  9 +++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 42a12f2ad612..46ab1f8e330f 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4320,7 +4320,15 @@ static inline void sync_cr8_to_lapic(struct kvm_vcpu *vcpu)
 
 	if (!svm_is_intercept(svm, INTERCEPT_CR8_WRITE)) {
 		int cr8 = svm->vmcb->control.int_ctl & V_TPR_MASK;
-		kvm_set_cr8(vcpu, cr8);
+
+		/*
+		 * Hardware changes V_TPR only on a guest CR8 write.  Pushing
+		 * an unchanged value into the APIC would zero TPR[3:0] (CR8
+		 * maps TPR[7:4] alone), wiping the low nibble of a TASKPRI
+		 * value the guest set through the APIC page or MSR.
+		 */
+		if (cr8 != svm->v_tpr_entry)
+			kvm_set_cr8(vcpu, cr8);
 	}
 }
 
@@ -4335,6 +4343,7 @@ static inline void sync_lapic_to_cr8(struct kvm_vcpu *vcpu)
 	cr8 = kvm_get_cr8(vcpu);
 	svm->vmcb->control.int_ctl &= ~V_TPR_MASK;
 	svm->vmcb->control.int_ctl |= cr8 & V_TPR_MASK;
+	svm->v_tpr_entry = cr8 & V_TPR_MASK;
 }
 
 static void svm_complete_soft_interrupt(struct kvm_vcpu *vcpu, u8 vector,
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 64438317d148..746a2ae98c4c 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -316,6 +316,15 @@ struct vcpu_svm {
 
 	void *msrpm;
 
+	/*
+	 * V_TPR value written by sync_lapic_to_cr8() on the last entry.
+	 * sync_cr8_to_lapic() pushes V_TPR back into the APIC only when it
+	 * differs, i.e. when the guest wrote CR8 in guest mode: V_TPR holds
+	 * TPR[7:4] only, so an unconditional back-sync would zero TPR[3:0]
+	 * after every exit, losing the low nibble of an APIC TASKPRI write.
+	 */
+	u8 v_tpr_entry;
+
 	ulong nmi_iret_rip;
 
 	struct svm_nested_state nested;
-- 
2.50.1