[PATCH kernel 0/9] KVM: SEV: Enable AMD SEV-ES DebugSwap

[PATCH kernel 0/9] KVM: SEV: Enable AMD SEV-ES DebugSwap

[Alexey Kardashevskiy]

This is to use another AMD SEV-ES hardware assisted register swap,
more detail in 6/9. In the process it's been suggested to fix other
things, here is the attempt, with the great help of amders.

The previous conversation is here:
https://lore.kernel.org/r/20230411125718.2297768-1-aik@amd.com 

This is based on sha1
6e2e1e779912 Ingo Molnar "Merge branch into tip/master: 'x86/sgx'".

The tree is here: https://github.com/aik/linux/tree/debugswap

Please comment. Thanks.


Alexey Kardashevskiy (6):
  KVM: SEV: move set_dr_intercepts/clr_dr_intercepts from the header
  KVM: SEV: Move SEV's GP_VECTOR intercept setup to SEV
  KVM: SEV-ES: explicitly disable debug
  KVM: SVM/SEV/SEV-ES: Rework intercepts
  KVM: SEV: Enable data breakpoints in SEV-ES
  KVM: SEV-ES: Eliminate #DB intercept when DebugSwap enabled

Sean Christopherson (3):
  KVM: SVM: Rewrite sev_es_prepare_switch_to_guest()'s comment about
    swap types
  KVM: SVM: Don't defer NMI unblocking until next exit for SEV-ES guests
  KVM: SVM: Don't try to pointlessly single-step SEV-ES guests for NMI
    window

 arch/x86/include/asm/cpufeatures.h       |  1 +
 arch/x86/include/asm/svm.h               |  1 +
 arch/x86/kvm/svm/svm.h                   | 42 ----------
 tools/arch/x86/include/asm/cpufeatures.h |  1 +
 arch/x86/kvm/svm/sev.c                   | 86 +++++++++++++++++---
 arch/x86/kvm/svm/svm.c                   | 70 ++++++++++++++--
 6 files changed, 137 insertions(+), 64 deletions(-)

-- 
2.40.1

[PATCH kernel 1/9] KVM: SEV: move set_dr_intercepts/clr_dr_intercepts from the header

[Alexey Kardashevskiy]

Static functions set_dr_intercepts() and clr_dr_intercepts() are only
called from SVM so move them to .c.

No functional change intended.

Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
Reviewed-by: Carlos Bilbao <carlos.bilbao@amd.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Santosh Shukla <santosh.shukla@amd.com>
---
Changes:
v5:
* new in the series
---
 arch/x86/kvm/svm/svm.h | 42 --------------------
 arch/x86/kvm/svm/svm.c | 42 ++++++++++++++++++++
 2 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index f44751dd8d5d..a99f97a86c59 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -405,48 +405,6 @@ static inline bool vmcb12_is_intercept(struct vmcb_ctrl_area_cached *control, u3
 	return test_bit(bit, (unsigned long *)&control->intercepts);
 }
 
-static inline void set_dr_intercepts(struct vcpu_svm *svm)
-{
-	struct vmcb *vmcb = svm->vmcb01.ptr;
-
-	if (!sev_es_guest(svm->vcpu.kvm)) {
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR0_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR1_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR2_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR3_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR4_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR5_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR6_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR0_WRITE);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR1_WRITE);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR2_WRITE);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR3_WRITE);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR4_WRITE);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR5_WRITE);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR6_WRITE);
-	}
-
-	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_READ);
-	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_WRITE);
-
-	recalc_intercepts(svm);
-}
-
-static inline void clr_dr_intercepts(struct vcpu_svm *svm)
-{
-	struct vmcb *vmcb = svm->vmcb01.ptr;
-
-	vmcb->control.intercepts[INTERCEPT_DR] = 0;
-
-	/* DR7 access must remain intercepted for an SEV-ES guest */
-	if (sev_es_guest(svm->vcpu.kvm)) {
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_WRITE);
-	}
-
-	recalc_intercepts(svm);
-}
-
 static inline void set_exception_intercept(struct vcpu_svm *svm, u32 bit)
 {
 	struct vmcb *vmcb = svm->vmcb01.ptr;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 54089f990c8f..980faf460bfe 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -690,6 +690,48 @@ static int svm_cpu_init(int cpu)
 
 }
 
+static void set_dr_intercepts(struct vcpu_svm *svm)
+{
+	struct vmcb *vmcb = svm->vmcb01.ptr;
+
+	if (!sev_es_guest(svm->vcpu.kvm)) {
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR0_READ);
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR1_READ);
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR2_READ);
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR3_READ);
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR4_READ);
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR5_READ);
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR6_READ);
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR0_WRITE);
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR1_WRITE);
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR2_WRITE);
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR3_WRITE);
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR4_WRITE);
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR5_WRITE);
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR6_WRITE);
+	}
+
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_READ);
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_WRITE);
+
+	recalc_intercepts(svm);
+}
+
+static void clr_dr_intercepts(struct vcpu_svm *svm)
+{
+	struct vmcb *vmcb = svm->vmcb01.ptr;
+
+	vmcb->control.intercepts[INTERCEPT_DR] = 0;
+
+	/* DR7 access must remain intercepted for an SEV-ES guest */
+	if (sev_es_guest(svm->vcpu.kvm)) {
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_READ);
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_WRITE);
+	}
+
+	recalc_intercepts(svm);
+}
+
 static int direct_access_msr_slot(u32 msr)
 {
 	u32 i;
-- 
2.40.1

[PATCH kernel 2/9] KVM: SEV: Move SEV's GP_VECTOR intercept setup to SEV

[Alexey Kardashevskiy]

Currently SVM setup is done sequentially in
init_vmcb() -> sev_init_vmcb() -> sev_es_init_vmcb() and tries
keeping SVM/SEV/SEV-ES bits separated. One of the exceptions
is #GP intercept which init_vmcb() skips setting for SEV guests and
then sev_es_init_vmcb() needlessly clears it.

Remove the SEV check from init_vmcb(). Clear the #GP intercept in
sev_init_vmcb(). SEV-ES will use the SEV setting.

No functional change intended.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
Reviewed-by: Carlos Bilbao <carlos.bilbao@amd.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Santosh Shukla <santosh.shukla@amd.com>
---
Changes:
v5:
* new in the series
---
 arch/x86/kvm/svm/sev.c | 9 ++++++---
 arch/x86/kvm/svm/svm.c | 5 ++---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 69ae5e1b3120..c03bd063aecf 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2971,9 +2971,6 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
 	svm_set_intercept(svm, TRAP_CR4_WRITE);
 	svm_set_intercept(svm, TRAP_CR8_WRITE);
 
-	/* No support for enable_vmware_backdoor */
-	clr_exception_intercept(svm, GP_VECTOR);
-
 	/* Can't intercept XSETBV, HV can't modify XCR0 directly */
 	svm_clr_intercept(svm, INTERCEPT_XSETBV);
 
@@ -2999,6 +2996,12 @@ void sev_init_vmcb(struct vcpu_svm *svm)
 	svm->vmcb->control.nested_ctl |= SVM_NESTED_CTL_SEV_ENABLE;
 	clr_exception_intercept(svm, UD_VECTOR);
 
+	/*
+	 * Don't intercept #GP for SEV guests, e.g. for the VMware backdoor, as
+	 * KVM can't decrypt guest memory to decode the faulting instruction.
+	 */
+	clr_exception_intercept(svm, GP_VECTOR);
+
 	if (sev_es_guest(svm->vcpu.kvm))
 		sev_es_init_vmcb(svm);
 }
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 980faf460bfe..9c1b191aed4b 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1256,10 +1256,9 @@ static void init_vmcb(struct kvm_vcpu *vcpu)
 	 * Guest access to VMware backdoor ports could legitimately
 	 * trigger #GP because of TSS I/O permission bitmap.
 	 * We intercept those #GP and allow access to them anyway
-	 * as VMware does.  Don't intercept #GP for SEV guests as KVM can't
-	 * decrypt guest memory to decode the faulting instruction.
+	 * as VMware does.
 	 */
-	if (enable_vmware_backdoor && !sev_guest(vcpu->kvm))
+	if (enable_vmware_backdoor)
 		set_exception_intercept(svm, GP_VECTOR);
 
 	svm_set_intercept(svm, INTERCEPT_INTR);
-- 
2.40.1

[PATCH kernel 3/9] KVM: SVM: Rewrite sev_es_prepare_switch_to_guest()'s comment about swap types

[Alexey Kardashevskiy]

From: Sean Christopherson <seanjc@google.com>

Rewrite the comment(s) in sev_es_prepare_switch_to_guest() to explain the
swap types employed by the CPU for SEV-ES guests, i.e. to explain why KVM
needs to save a seemingly random subset of host state, and to provide a
decoder for the APM's Type-A/B/C terminology.

Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
Changes:
v6:
* new to the series
---
 arch/x86/kvm/svm/sev.c | 25 ++++++++++++--------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index c03bd063aecf..36fe2fcb4698 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3020,19 +3020,24 @@ void sev_es_vcpu_reset(struct vcpu_svm *svm)
 void sev_es_prepare_switch_to_guest(struct sev_es_save_area *hostsa)
 {
 	/*
-	 * As an SEV-ES guest, hardware will restore the host state on VMEXIT,
-	 * of which one step is to perform a VMLOAD.  KVM performs the
-	 * corresponding VMSAVE in svm_prepare_guest_switch for both
-	 * traditional and SEV-ES guests.
+	 * All host state for SEV-ES guests is categorized into three swap types
+	 * based on how it is handled by hardware during a world switch:
+	 *
+	 * A: VMRUN:   Host state saved in host save area
+	 *    VMEXIT:  Host state loaded from host save area
+	 *
+	 * B: VMRUN:   Host state _NOT_ saved in host save area
+	 *    VMEXIT:  Host state loaded from host save area
+	 *
+	 * C: VMRUN:   Host state _NOT_ saved in host save area
+	 *    VMEXIT:  Host state initialized to default(reset) values
+	 *
+	 * Manually save type-B state, i.e. state that is loaded by VMEXIT but
+	 * isn't saved by VMRUN, that isn't already saved by VMSAVE (performed
+	 * by common SVM code).
 	 */
-
-	/* XCR0 is restored on VMEXIT, save the current host value */
 	hostsa->xcr0 = xgetbv(XCR_XFEATURE_ENABLED_MASK);
-
-	/* PKRU is restored on VMEXIT, save the current host value */
 	hostsa->pkru = read_pkru();
-
-	/* MSR_IA32_XSS is restored on VMEXIT, save the currnet host value */
 	hostsa->xss = host_xss;
 }
 
-- 
2.40.1

[PATCH kernel 4/9] KVM: SEV-ES: explicitly disable debug

[Alexey Kardashevskiy]

SVM/SEV enable debug registers intercepts to skip swapping DRs
on entering/exiting the guest. When the guest is in control of
debug registers (vcpu->guest_debug == 0), there is an optimisation to
reduce the number of context switches: intercepts are cleared and
the KVM_DEBUGREG_WONT_EXIT flag is set to tell KVM to do swapping
on guest enter/exit.

The same code also executes for SEV-ES, however it has no effect as
- it always takes (vcpu->guest_debug == 0) branch;
- KVM_DEBUGREG_WONT_EXIT is set but DR7 intercept is not cleared;
- vcpu_enter_guest() writes DRs but VMRUN for SEV-ES swaps them
with the values from _encrypted_ VMSA.

Be explicit about SEV-ES not supporting debug:
- return right away from dr_interception() and skip unnecessary processing;
- return an error right away from the KVM_SEV_LAUNCH_UPDATE_VMSA handler
if debugging was already enabled.
KVM_SET_GUEST_DEBUG are failing already after KVM_SEV_LAUNCH_UPDATE_VMSA
is finished due to vcpu->arch.guest_state_protected set to true.

Add WARN_ON to kvm_x86::sync_dirty_debug_regs() (saves guest DRs on
guest exit) to signify that SEV-ES won't hit that path.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
Changes:
v6:
* fail in LAUNCH_UPDATE_VMSA instead of clearing the flag
* pr_warn_ratelimited -> pr_warn_once
* due to the rework, removed Tom's "rb"

v5:
* new in the series
---
 arch/x86/kvm/svm/sev.c | 5 +++++
 arch/x86/kvm/svm/svm.c | 9 ++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 36fe2fcb4698..981286359b72 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -619,6 +619,11 @@ static int __sev_launch_update_vmsa(struct kvm *kvm, struct kvm_vcpu *vcpu,
 	struct vcpu_svm *svm = to_svm(vcpu);
 	int ret;
 
+	if (vcpu->guest_debug) {
+		pr_warn_once("KVM_SET_GUEST_DEBUG for SEV-ES guest is not supported");
+		return -EINVAL;
+	}
+
 	/* Perform some pre-encryption checks against the VMSA */
 	ret = sev_es_sync_vmsa(svm);
 	if (ret)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 9c1b191aed4b..bec6fb82f494 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1996,7 +1996,7 @@ static void svm_sync_dirty_debug_regs(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
-	if (vcpu->arch.guest_state_protected)
+	if (WARN_ON_ONCE(sev_es_guest(vcpu->kvm)))
 		return;
 
 	get_debugreg(vcpu->arch.db[0], 0);
@@ -2727,6 +2727,13 @@ static int dr_interception(struct kvm_vcpu *vcpu)
 	unsigned long val;
 	int err = 0;
 
+	/*
+	 * SEV-ES intercepts DR7 only to disable guest debugging and the guest issues a VMGEXIT
+	 * for DR7 write only. KVM cannot change DR7 (always swapped as type 'A') so return early.
+	 */
+	if (sev_es_guest(vcpu->kvm))
+		return 1;
+
 	if (vcpu->guest_debug == 0) {
 		/*
 		 * No more DR vmexits; force a reload of the debug registers
-- 
2.40.1

[PATCH kernel 5/9] KVM: SVM/SEV/SEV-ES: Rework intercepts

[Alexey Kardashevskiy]

Currently SVM setup is done sequentially in
init_vmcb() -> sev_init_vmcb() -> sev_es_init_vmcb()
and tries keeping SVM/SEV/SEV-ES bits separated. One of the exceptions
is DR intercepts which is for SEV-ES before sev_es_init_vmcb() runs.

Move the SEV-ES intercept setup to sev_es_init_vmcb(). From now on
set_dr_intercepts()/clr_dr_intercepts() handle SVM/SEV only.

Extend the comment about intercepting DR7 which is to prevent the CPU
from getting stuck in an infinite #DB loop as described in
https://bugzilla.redhat.com/show_bug.cgi?id=1278496

No functional change intended.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
Reviewed-by: Santosh Shukla <santosh.shukla@amd.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
---
Changes:
v6:
* updated the commit log
* updated the DR7 intercept comment in the code

v5:
* updated the comments
* removed sev_es_guest() checks from set_dr_intercepts()/clr_dr_intercepts()
* removed remaining intercepts from clr_dr_intercepts()
---
 arch/x86/kvm/svm/sev.c | 11 ++++++
 arch/x86/kvm/svm/svm.c | 37 ++++++++------------
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 981286359b72..744bcc2e6a05 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2948,6 +2948,7 @@ int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in)
 
 static void sev_es_init_vmcb(struct vcpu_svm *svm)
 {
+	struct vmcb *vmcb = svm->vmcb01.ptr;
 	struct kvm_vcpu *vcpu = &svm->vcpu;
 
 	svm->vmcb->control.nested_ctl |= SVM_NESTED_CTL_SEV_ES_ENABLE;
@@ -2976,6 +2977,16 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
 	svm_set_intercept(svm, TRAP_CR4_WRITE);
 	svm_set_intercept(svm, TRAP_CR8_WRITE);
 
+	/*
+	 * DR7 access must remain intercepted for an SEV-ES guest to disallow
+	 * the guest kernel set up a #DB on memory that's needed to vector a #DB
+	 * as otherwise the CPU gets stuck in an infinite #DB loop.
+	 */
+	vmcb->control.intercepts[INTERCEPT_DR] = 0;
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_READ);
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_WRITE);
+	recalc_intercepts(svm);
+
 	/* Can't intercept XSETBV, HV can't modify XCR0 directly */
 	svm_clr_intercept(svm, INTERCEPT_XSETBV);
 
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index bec6fb82f494..1df99e9f8655 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -694,23 +694,20 @@ static void set_dr_intercepts(struct vcpu_svm *svm)
 {
 	struct vmcb *vmcb = svm->vmcb01.ptr;
 
-	if (!sev_es_guest(svm->vcpu.kvm)) {
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR0_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR1_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR2_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR3_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR4_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR5_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR6_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR0_WRITE);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR1_WRITE);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR2_WRITE);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR3_WRITE);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR4_WRITE);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR5_WRITE);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR6_WRITE);
-	}
-
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR0_READ);
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR1_READ);
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR2_READ);
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR3_READ);
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR4_READ);
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR5_READ);
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR6_READ);
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR0_WRITE);
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR1_WRITE);
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR2_WRITE);
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR3_WRITE);
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR4_WRITE);
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR5_WRITE);
+	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR6_WRITE);
 	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_READ);
 	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_WRITE);
 
@@ -723,12 +720,6 @@ static void clr_dr_intercepts(struct vcpu_svm *svm)
 
 	vmcb->control.intercepts[INTERCEPT_DR] = 0;
 
-	/* DR7 access must remain intercepted for an SEV-ES guest */
-	if (sev_es_guest(svm->vcpu.kvm)) {
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_READ);
-		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_WRITE);
-	}
-
 	recalc_intercepts(svm);
 }
 
-- 
2.40.1

[PATCH kernel 6/9] KVM: SEV: Enable data breakpoints in SEV-ES

[Alexey Kardashevskiy]

Add support for "DebugSwap for SEV-ES guests", which provides support
for swapping DR[0-3] and DR[0-3]_ADDR_MASK on VMRUN and VMEXIT, i.e.
allows KVM to expose debug capabilities to SEV-ES guests. Without
DebugSwap support, the CPU doesn't save/load most _guest_ debug
registers (except DR6/7), and KVM cannot manually context switch guest
DRs due the VMSA being encrypted.

Enable DebugSwap if and only if the CPU also supports NoNestedDataBp,
which causes the CPU to ignore nested #DBs, i.e. #DBs that occur when
vectoring a #DB.  Without NoNestedDataBp, a malicious guest can DoS
the host by putting the CPU into an infinite loop of vectoring #DBs
(see https://bugzilla.redhat.com/show_bug.cgi?id=1278496)

Set the features bit in sev_es_sync_vmsa() which is the last point
when VMSA is not encrypted yet as sev_(es_)init_vmcb() (where the most
init happens) is called not only when VCPU is initialised but also on
intrahost migration when VMSA is encrypted.

Eliminate DR7 intercepts as KVM can't modify guest DR7, and intercepting
DR7 would completely defeat the purpose of enabling DebugSwap.

Make X86_FEATURE_DEBUG_SWAP appear in /proc/cpuinfo (by not adding "") to
let the operator know if the VM can debug.

Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
Changes:
v6:
* rewrote the commit log as suggested by Sean
* clr_exception_intercept(#DB) moved to a separate patch (next to this)
* updated tools/arch/x86/include/asm/cpufeatures.h

(old versions from when this was a single patch, ignore?)
v9:
* changed the commit log to one from Sean
* moved #DB intercept handling later in the series

v5:
* added CPUID's DebugSwap feature
* commit log, comments updated
* redid the whole thing

v4:
* removed sev_es_is_debug_swap_enabled() helper
* made sev_es_debug_swap_enabled (module param) static
* set sev_feature early in sev_es_init_vmcb() and made intercepts
  dependend on it vs. module param
* move set_/clr_dr_intercepts to .c

v3:
* rewrote the commit log again
* rebased on tip/master to use recently defined X86_FEATURE_NO_NESTED_DATA_BP
* s/boot_cpu_has/cpu_feature_enabled/

v2:
* debug_swap moved from vcpu to module_param
* rewrote commit log

---
Tested with:
===
int x;
int main(int argc, char *argv[])
{
        x = 1;
        return 0;
}
===
gcc -g a.c
rsync a.out ruby-954vm:~/
ssh -t ruby-954vm 'gdb -ex "file a.out" -ex "watch x" -ex r'

where ruby-954vm is a VM.

With "/sys/module/kvm_amd/parameters/debug_swap = 0", gdb does not stop
on the watchpoint, with "= 1" - gdb does.
---
 arch/x86/include/asm/cpufeatures.h       |  1 +
 arch/x86/include/asm/svm.h               |  1 +
 tools/arch/x86/include/asm/cpufeatures.h |  1 +
 arch/x86/kvm/svm/sev.c                   | 37 ++++++++++++++++++--
 4 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index cb8ca46213be..31c862d79fae 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -434,6 +434,7 @@
 #define X86_FEATURE_SEV_ES		(19*32+ 3) /* AMD Secure Encrypted Virtualization - Encrypted State */
 #define X86_FEATURE_V_TSC_AUX		(19*32+ 9) /* "" Virtual TSC_AUX */
 #define X86_FEATURE_SME_COHERENT	(19*32+10) /* "" AMD hardware-enforced cache coherency */
+#define X86_FEATURE_DEBUG_SWAP		(19*32+14) /* AMD SEV-ES full debug state swap support */
 
 /* AMD-defined Extended Feature 2 EAX, CPUID level 0x80000021 (EAX), word 20 */
 #define X86_FEATURE_NO_NESTED_DATA_BP	(20*32+ 0) /* "" No Nested Data Breakpoints */
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index e7c7379d6ac7..72ebd5e4e975 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -288,6 +288,7 @@ static_assert((X2AVIC_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AVIC_
 
 #define AVIC_HPA_MASK	~((0xFFFULL << 52) | 0xFFF)
 
+#define SVM_SEV_FEAT_DEBUG_SWAP                        BIT(5)
 
 struct vmcb_seg {
 	u16 selector;
diff --git a/tools/arch/x86/include/asm/cpufeatures.h b/tools/arch/x86/include/asm/cpufeatures.h
index cb8ca46213be..31c862d79fae 100644
--- a/tools/arch/x86/include/asm/cpufeatures.h
+++ b/tools/arch/x86/include/asm/cpufeatures.h
@@ -434,6 +434,7 @@
 #define X86_FEATURE_SEV_ES		(19*32+ 3) /* AMD Secure Encrypted Virtualization - Encrypted State */
 #define X86_FEATURE_V_TSC_AUX		(19*32+ 9) /* "" Virtual TSC_AUX */
 #define X86_FEATURE_SME_COHERENT	(19*32+10) /* "" AMD hardware-enforced cache coherency */
+#define X86_FEATURE_DEBUG_SWAP		(19*32+14) /* AMD SEV-ES full debug state swap support */
 
 /* AMD-defined Extended Feature 2 EAX, CPUID level 0x80000021 (EAX), word 20 */
 #define X86_FEATURE_NO_NESTED_DATA_BP	(20*32+ 0) /* "" No Nested Data Breakpoints */
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 744bcc2e6a05..abc502ce7871 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -23,6 +23,7 @@
 #include <asm/pkru.h>
 #include <asm/trapnr.h>
 #include <asm/fpu/xcr.h>
+#include <asm/debugreg.h>
 
 #include "mmu.h"
 #include "x86.h"
@@ -54,9 +55,14 @@ module_param_named(sev, sev_enabled, bool, 0444);
 /* enable/disable SEV-ES support */
 static bool sev_es_enabled = true;
 module_param_named(sev_es, sev_es_enabled, bool, 0444);
+
+/* enable/disable SEV-ES DebugSwap support */
+static bool sev_es_debug_swap_enabled = true;
+module_param_named(debug_swap, sev_es_debug_swap_enabled, bool, 0444);
 #else
 #define sev_enabled false
 #define sev_es_enabled false
+#define sev_es_debug_swap_enabled false
 #endif /* CONFIG_KVM_AMD_SEV */
 
 static u8 sev_enc_bit;
@@ -606,6 +612,9 @@ static int sev_es_sync_vmsa(struct vcpu_svm *svm)
 	save->xss  = svm->vcpu.arch.ia32_xss;
 	save->dr6  = svm->vcpu.arch.dr6;
 
+	if (sev_es_debug_swap_enabled)
+		save->sev_features |= SVM_SEV_FEAT_DEBUG_SWAP;
+
 	pr_debug("Virtual Machine Save Area (VMSA):\n");
 	print_hex_dump_debug("", DUMP_PREFIX_NONE, 16, 1, save, sizeof(*save), false);
 
@@ -2258,6 +2267,9 @@ void __init sev_hardware_setup(void)
 out:
 	sev_enabled = sev_supported;
 	sev_es_enabled = sev_es_supported;
+	if (!sev_es_enabled || !cpu_feature_enabled(X86_FEATURE_DEBUG_SWAP) ||
+	    !cpu_feature_enabled(X86_FEATURE_NO_NESTED_DATA_BP))
+		sev_es_debug_swap_enabled = false;
 #endif
 }
 
@@ -2978,14 +2990,17 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
 	svm_set_intercept(svm, TRAP_CR8_WRITE);
 
 	/*
+	 * Unless DebugSwap (depends on X86_FEATURE_NO_NESTED_DATA_BP) is enabled,
 	 * DR7 access must remain intercepted for an SEV-ES guest to disallow
 	 * the guest kernel set up a #DB on memory that's needed to vector a #DB
 	 * as otherwise the CPU gets stuck in an infinite #DB loop.
 	 */
 	vmcb->control.intercepts[INTERCEPT_DR] = 0;
-	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_READ);
-	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_WRITE);
-	recalc_intercepts(svm);
+	if (!sev_es_debug_swap_enabled) {
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_READ);
+		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_WRITE);
+		recalc_intercepts(svm);
+	}
 
 	/* Can't intercept XSETBV, HV can't modify XCR0 directly */
 	svm_clr_intercept(svm, INTERCEPT_XSETBV);
@@ -3055,6 +3070,22 @@ void sev_es_prepare_switch_to_guest(struct sev_es_save_area *hostsa)
 	hostsa->xcr0 = xgetbv(XCR_XFEATURE_ENABLED_MASK);
 	hostsa->pkru = read_pkru();
 	hostsa->xss = host_xss;
+
+	/*
+	 * If DebugSwap is enabled, debug registers are loaded but NOT saved by
+	 * the CPU (Type-B). If DebugSwap is disabled/unsupported, the CPU both
+	 * saves and loads debug registers (Type-A).
+	 */
+	if (sev_es_debug_swap_enabled) {
+		hostsa->dr0 = native_get_debugreg(0);
+		hostsa->dr1 = native_get_debugreg(1);
+		hostsa->dr2 = native_get_debugreg(2);
+		hostsa->dr3 = native_get_debugreg(3);
+		hostsa->dr0_addr_mask = amd_get_dr_addr_mask(0);
+		hostsa->dr1_addr_mask = amd_get_dr_addr_mask(1);
+		hostsa->dr2_addr_mask = amd_get_dr_addr_mask(2);
+		hostsa->dr3_addr_mask = amd_get_dr_addr_mask(3);
+	}
 }
 
 void sev_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector)
-- 
2.40.1

[PATCH kernel 7/9] KVM: SEV-ES: Eliminate #DB intercept when DebugSwap enabled

[Alexey Kardashevskiy]

Disable #DB for SEV-ES guests when DebugSwap is enabled. There is no point
in such intercept as KVM does not allow guest debug for SEV-ES guests.

Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
Changes:
v6:
* new to the series
---
 arch/x86/kvm/svm/sev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index abc502ce7871..9c43cbdab022 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3000,6 +3000,8 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
 		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_READ);
 		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_WRITE);
 		recalc_intercepts(svm);
+	} else {
+		clr_exception_intercept(svm, DB_VECTOR);
 	}
 
 	/* Can't intercept XSETBV, HV can't modify XCR0 directly */
-- 
2.40.1

[PATCH kernel 8/9] KVM: SVM: Don't defer NMI unblocking until next exit for SEV-ES guests

[Alexey Kardashevskiy]

From: Sean Christopherson <seanjc@google.com>

Immediately mark NMIs as unmasked in response to #VMGEXIT(NMI complete)
instead of setting awaiting_iret_completion and waiting until the *next*
VM-Exit to unmask NMIs.  The whole point of "NMI complete" is that the
guest is responsible for telling the hypervisor when it's safe to inject
an NMI, i.e. there's no need to wait.  And because there's no IRET to
single-step, the next VM-Exit could be a long time coming, i.e. KVM could
incorrectly hold an NMI pending for far longer than what is required and
expected.

Opportunistically fix a stale reference to HF_IRET_MASK.

Fixes: 916b54a7688b ("KVM: x86: Move HF_NMI_MASK and HF_IRET_MASK into "struct vcpu_svm"")
Fixes: 4444dfe4050b ("KVM: SVM: Add NMI support for an SEV-ES guest")
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---

May be 916b54a7688b is not really necessary to mention to avoid triggering
the stable kernel backporting bot?

---
Changes:
v6:
* new to the series
---
 arch/x86/kvm/svm/sev.c |  5 ++++-
 arch/x86/kvm/svm/svm.c | 10 +++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 9c43cbdab022..4a426feab1b8 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2897,7 +2897,10 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
 					    svm->sev_es.ghcb_sa);
 		break;
 	case SVM_VMGEXIT_NMI_COMPLETE:
-		ret = svm_invoke_exit_handler(vcpu, SVM_EXIT_IRET);
+		++vcpu->stat.nmi_window_exits;
+		svm->nmi_masked = false;
+		kvm_make_request(KVM_REQ_EVENT, vcpu);
+		ret = 1;
 		break;
 	case SVM_VMGEXIT_AP_HLT_LOOP:
 		ret = kvm_emulate_ap_reset_hold(vcpu);
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 1df99e9f8655..52f1d88e82a0 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2548,12 +2548,13 @@ static int iret_interception(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
+	WARN_ON_ONCE(sev_es_guest(vcpu->kvm));
+
 	++vcpu->stat.nmi_window_exits;
 	svm->awaiting_iret_completion = true;
 
 	svm_clr_iret_intercept(svm);
-	if (!sev_es_guest(vcpu->kvm))
-		svm->nmi_iret_rip = kvm_rip_read(vcpu);
+	svm->nmi_iret_rip = kvm_rip_read(vcpu);
 
 	kvm_make_request(KVM_REQ_EVENT, vcpu);
 	return 1;
@@ -3972,12 +3973,11 @@ static void svm_complete_interrupts(struct kvm_vcpu *vcpu)
 	svm->soft_int_injected = false;
 
 	/*
-	 * If we've made progress since setting HF_IRET_MASK, we've
+	 * If we've made progress since setting awaiting_iret_completion, we've
 	 * executed an IRET and can allow NMI injection.
 	 */
 	if (svm->awaiting_iret_completion &&
-	    (sev_es_guest(vcpu->kvm) ||
-	     kvm_rip_read(vcpu) != svm->nmi_iret_rip)) {
+	    kvm_rip_read(vcpu) != svm->nmi_iret_rip) {
 		svm->awaiting_iret_completion = false;
 		svm->nmi_masked = false;
 		kvm_make_request(KVM_REQ_EVENT, vcpu);
-- 
2.40.1

[PATCH kernel 9/9] KVM: SVM: Don't try to pointlessly single-step SEV-ES guests for NMI window

[Alexey Kardashevskiy]

From: Sean Christopherson <seanjc@google.com>

Bail early from svm_enable_nmi_window() for SEV-ES guests without trying
to enable single-step of the guest, as single-stepping an SEV-ES guest is
impossible and the guest is responsible for *telling* KVM when it is ready
for an new NMI to be injected.

Functionally, setting TF and RF in svm->vmcb->save.rflags is benign as the
field is ignored by hardware, but it's all kinds of confusing.

Signed-off-by: Sean Christopherson <seanjc@google.com>
[aik: removed the clause about "KVM suppresses EFER.SVME (see efer_trap())"]
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
Changes:
v6:
* new to the series
---
 arch/x86/kvm/svm/svm.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 52f1d88e82a0..c9837a8667b7 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3824,6 +3824,19 @@ static void svm_enable_nmi_window(struct kvm_vcpu *vcpu)
 	if (svm_get_nmi_mask(vcpu) && !svm->awaiting_iret_completion)
 		return; /* IRET will cause a vm exit */
 
+	/*
+	 * SEV-ES guests are responsible for signaling when a vCPU is ready to
+	 * receive a new NMI, as SEV-ES guests can't be single-stepped, i.e.
+	 * KVM can't intercept and single-step IRET to detect when NMIs are
+	 * unblocked (architecturally speaking).  See SVM_VMGEXIT_NMI_COMPLETE.
+	 *
+	 * Note, GIF is guaranteed to be '1' for SEV-ES guests as hardware
+	 * ignores SEV-ES guest writes to EFER.SVME *and* CLGI/STGI are not
+	 * supported NAEs in the GHCB protocol.
+	 */
+	if (sev_es_guest(vcpu->kvm))
+		return;
+
 	if (!gif_set(svm)) {
 		if (vgif)
 			svm_set_intercept(svm, INTERCEPT_STGI);
-- 
2.40.1

Re: [PATCH kernel 0/9 v6] KVM: SEV: Enable AMD SEV-ES DebugSwap

[Alexey Kardashevskiy]

Ouch, forgot to mark this as "v6". Sorry...



On 15/6/23 16:37, Alexey Kardashevskiy wrote:
> This is to use another AMD SEV-ES hardware assisted register swap,
> more detail in 6/9. In the process it's been suggested to fix other
> things, here is the attempt, with the great help of amders.
> 
> The previous conversation is here:
> https://lore.kernel.org/r/20230411125718.2297768-1-aik@amd.com
> 
> This is based on sha1
> 6e2e1e779912 Ingo Molnar "Merge branch into tip/master: 'x86/sgx'".
> 
> The tree is here: https://github.com/aik/linux/tree/debugswap
> 
> Please comment. Thanks.
> 
> 
> Alexey Kardashevskiy (6):
>    KVM: SEV: move set_dr_intercepts/clr_dr_intercepts from the header
>    KVM: SEV: Move SEV's GP_VECTOR intercept setup to SEV
>    KVM: SEV-ES: explicitly disable debug
>    KVM: SVM/SEV/SEV-ES: Rework intercepts
>    KVM: SEV: Enable data breakpoints in SEV-ES
>    KVM: SEV-ES: Eliminate #DB intercept when DebugSwap enabled
> 
> Sean Christopherson (3):
>    KVM: SVM: Rewrite sev_es_prepare_switch_to_guest()'s comment about
>      swap types
>    KVM: SVM: Don't defer NMI unblocking until next exit for SEV-ES guests
>    KVM: SVM: Don't try to pointlessly single-step SEV-ES guests for NMI
>      window
> 
>   arch/x86/include/asm/cpufeatures.h       |  1 +
>   arch/x86/include/asm/svm.h               |  1 +
>   arch/x86/kvm/svm/svm.h                   | 42 ----------
>   tools/arch/x86/include/asm/cpufeatures.h |  1 +
>   arch/x86/kvm/svm/sev.c                   | 86 +++++++++++++++++---
>   arch/x86/kvm/svm/svm.c                   | 70 ++++++++++++++--
>   6 files changed, 137 insertions(+), 64 deletions(-)
> 

-- 
Alexey

Re: [PATCH kernel 0/9 v6] KVM: SEV: Enable AMD SEV-ES DebugSwap

[Alexey Kardashevskiy]

Sean, do you want me to repost with "v6" in all patches or this will do? 
Thanks,


On 15/6/23 17:13, Alexey Kardashevskiy wrote:
> Ouch, forgot to mark this as "v6". Sorry...
> 
> 
> 
> On 15/6/23 16:37, Alexey Kardashevskiy wrote:
>> This is to use another AMD SEV-ES hardware assisted register swap,
>> more detail in 6/9. In the process it's been suggested to fix other
>> things, here is the attempt, with the great help of amders.
>>
>> The previous conversation is here:
>> https://lore.kernel.org/r/20230411125718.2297768-1-aik@amd.com
>>
>> This is based on sha1
>> 6e2e1e779912 Ingo Molnar "Merge branch into tip/master: 'x86/sgx'".
>>
>> The tree is here: https://github.com/aik/linux/tree/debugswap
>>
>> Please comment. Thanks.
>>
>>
>> Alexey Kardashevskiy (6):
>>    KVM: SEV: move set_dr_intercepts/clr_dr_intercepts from the header
>>    KVM: SEV: Move SEV's GP_VECTOR intercept setup to SEV
>>    KVM: SEV-ES: explicitly disable debug
>>    KVM: SVM/SEV/SEV-ES: Rework intercepts
>>    KVM: SEV: Enable data breakpoints in SEV-ES
>>    KVM: SEV-ES: Eliminate #DB intercept when DebugSwap enabled
>>
>> Sean Christopherson (3):
>>    KVM: SVM: Rewrite sev_es_prepare_switch_to_guest()'s comment about
>>      swap types
>>    KVM: SVM: Don't defer NMI unblocking until next exit for SEV-ES guests
>>    KVM: SVM: Don't try to pointlessly single-step SEV-ES guests for NMI
>>      window
>>
>>   arch/x86/include/asm/cpufeatures.h       |  1 +
>>   arch/x86/include/asm/svm.h               |  1 +
>>   arch/x86/kvm/svm/svm.h                   | 42 ----------
>>   tools/arch/x86/include/asm/cpufeatures.h |  1 +
>>   arch/x86/kvm/svm/sev.c                   | 86 +++++++++++++++++---
>>   arch/x86/kvm/svm/svm.c                   | 70 ++++++++++++++--
>>   6 files changed, 137 insertions(+), 64 deletions(-)
>>
> 

-- 
Alexey

Re: [PATCH kernel 0/9 v6] KVM: SEV: Enable AMD SEV-ES DebugSwap

[Sean Christopherson]

On Fri, Jun 23, 2023, Alexey Kardashevskiy wrote:
> Sean, do you want me to repost with "v6" in all patches or this will do?

No need on my end.

Re: [PATCH kernel 0/9 v6] KVM: SEV: Enable AMD SEV-ES DebugSwap

[Alexey Kardashevskiy]

On 24/6/23 00:19, Sean Christopherson wrote:
> On Fri, Jun 23, 2023, Alexey Kardashevskiy wrote:
>> Sean, do you want me to repost with "v6" in all patches or this will do?
> 
> No need on my end.

Cool. My colleagues are gently asking if this is any closer to getting 
pulled or not just yet? :) Thanks,

-- 
Alexey

Re: [PATCH kernel 5/9] KVM: SVM/SEV/SEV-ES: Rework intercepts

[Sean Christopherson]

On Thu, Jun 15, 2023, Alexey Kardashevskiy wrote:
> @@ -2976,6 +2977,16 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
>  	svm_set_intercept(svm, TRAP_CR4_WRITE);
>  	svm_set_intercept(svm, TRAP_CR8_WRITE);
>  
> +	/*
> +	 * DR7 access must remain intercepted for an SEV-ES guest to disallow
> +	 * the guest kernel set up a #DB on memory that's needed to vector a #DB
> +	 * as otherwise the CPU gets stuck in an infinite #DB loop.
> +	 */

This isn't correct.  Letting the guest configuring breakpoints would be weird
and nonsensical, but it wouldn't lead to infinite #DBs so long as KVM intercepts
#DB.

KVM intercepts DR7 when DebugSwap isn't enabled because otherwise KVM has no way
of context switching DR[0-3] for the guest.  At least, I assume that's the case,
AFAICT the APM never actually says what happens with DR[0-3] when DebugSwap is
disabled.

Re: [PATCH kernel 0/9 v6] KVM: SEV: Enable AMD SEV-ES DebugSwap

[Sean Christopherson]

On Fri, Jun 30, 2023, Alexey Kardashevskiy wrote:
> 
> 
> On 24/6/23 00:19, Sean Christopherson wrote:
> > On Fri, Jun 23, 2023, Alexey Kardashevskiy wrote:
> > > Sean, do you want me to repost with "v6" in all patches or this will do?
> > 
> > No need on my end.
> 
> Cool. My colleagues are gently asking if this is any closer to getting
> pulled or not just yet? :) Thanks,

Just looked through it.  A few nits, but nothing I can't fix when applying.  I'm
planning on applying it for 6.6 (pending testing, etc.), though I'm out next week
so it'll be a week or three before that actually happens.

Re: [PATCH kernel 5/9] KVM: SVM/SEV/SEV-ES: Rework intercepts

[Alexey Kardashevskiy]

On 1/7/23 07:49, Sean Christopherson wrote:
> On Thu, Jun 15, 2023, Alexey Kardashevskiy wrote:
>> @@ -2976,6 +2977,16 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
>>   	svm_set_intercept(svm, TRAP_CR4_WRITE);
>>   	svm_set_intercept(svm, TRAP_CR8_WRITE);
>>   
>> +	/*
>> +	 * DR7 access must remain intercepted for an SEV-ES guest to disallow
>> +	 * the guest kernel set up a #DB on memory that's needed to vector a #DB
>> +	 * as otherwise the CPU gets stuck in an infinite #DB loop.
>> +	 */
> 
> This isn't correct.  Letting the guest configuring breakpoints would be weird
> and nonsensical, but it wouldn't lead to infinite #DBs so long as KVM intercepts
> #DB.

True. OTOH not intercepting #DB and intercepting only DR7 would do the 
same thing.

> KVM intercepts DR7 when DebugSwap isn't enabled because otherwise KVM has no way
> of context switching DR[0-3] for the guest.  At least, I assume that's the case,
> AFAICT the APM never actually says what happens with DR[0-3] when DebugSwap is
> disabled.

This is the SEV-ES code, no DR[0-3] context switching anyway, is not it?

The actual immediate reason for intercepting DR7 is "SEV-ES GHCB" but 
this does not really explain it to me :-/

4.5 Debug Register Support
Currently, hardware debug traps are not supported for an SEV-ES guest. 
The hypervisor must set the intercept for both read and write of the 
debug control register (DR7). With the intercepts in place, the #VC 
handler will be invoked when the guest accesses DR7. For a write to DR7, 
the #VC handler should perform Standard VMGExit processing. The #VC 
handler must not update the actual DR7 register, but rather it should 
cache the DR7 value being written. For a read of DR7, the #VC handler 
should return the cached value of the DR7 register.


-- 
Alexey

Re: [PATCH kernel 0/9 v6] KVM: SEV: Enable AMD SEV-ES DebugSwap

[Alexey Kardashevskiy]

On 30/6/23 16:52, Sean Christopherson wrote:
> On Fri, Jun 30, 2023, Alexey Kardashevskiy wrote:
>>
>>
>> On 24/6/23 00:19, Sean Christopherson wrote:
>>> On Fri, Jun 23, 2023, Alexey Kardashevskiy wrote:
>>>> Sean, do you want me to repost with "v6" in all patches or this will do?
>>>
>>> No need on my end.
>>
>> Cool. My colleagues are gently asking if this is any closer to getting
>> pulled or not just yet? :) Thanks,
> 
> Just looked through it.  A few nits, but nothing I can't fix when applying.  I'm
> planning on applying it for 6.6 (pending testing, etc.), though I'm out next week
> so it'll be a week or three before that actually happens.

Soo three it is :)


-- 
Alexey

Re: [PATCH kernel 0/9] KVM: SEV: Enable AMD SEV-ES DebugSwap

[Sean Christopherson]

On Thu, 15 Jun 2023 16:37:48 +1000, Alexey Kardashevskiy wrote:
> This is to use another AMD SEV-ES hardware assisted register swap,
> more detail in 6/9. In the process it's been suggested to fix other
> things, here is the attempt, with the great help of amders.
> 
> The previous conversation is here:
> https://lore.kernel.org/r/20230411125718.2297768-1-aik@amd.com
> 
> [...]

Finally applied to kvm-x86 svm, thanks!  Though I was *really* tempted to see
just how snarky the pings would get at week 5+ ;-)

[1/9] KVM: SEV: move set_dr_intercepts/clr_dr_intercepts from the header
      https://github.com/kvm-x86/linux/commit/b265ee7bae11
[2/9] KVM: SEV: Move SEV's GP_VECTOR intercept setup to SEV
      https://github.com/kvm-x86/linux/commit/29de732cc95c
[3/9] KVM: SVM: Rewrite sev_es_prepare_switch_to_guest()'s comment about swap types
      https://github.com/kvm-x86/linux/commit/f8d808ed1ba0
[4/9] KVM: SEV-ES: explicitly disable debug
      https://github.com/kvm-x86/linux/commit/2837dd00f8fc
[5/9] KVM: SVM/SEV/SEV-ES: Rework intercepts
      https://github.com/kvm-x86/linux/commit/5aefd3a05fe1
[6/9] KVM: SEV: Enable data breakpoints in SEV-ES
      https://github.com/kvm-x86/linux/commit/fb71b1298709
[7/9] KVM: SEV-ES: Eliminate #DB intercept when DebugSwap enabled
      https://github.com/kvm-x86/linux/commit/8b54cc7e1817
[8/9] KVM: SVM: Don't defer NMI unblocking until next exit for SEV-ES guests
      https://github.com/kvm-x86/linux/commit/c54268e1036f
[9/9] KVM: SVM: Don't try to pointlessly single-step SEV-ES guests for NMI window
      https://github.com/kvm-x86/linux/commit/e11f81043a12

--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes

Re: [PATCH kernel 0/9] KVM: SEV: Enable AMD SEV-ES DebugSwap

[Alexey Kardashevskiy]

On 29/7/23 09:49, Sean Christopherson wrote:
> On Thu, 15 Jun 2023 16:37:48 +1000, Alexey Kardashevskiy wrote:
>> This is to use another AMD SEV-ES hardware assisted register swap,
>> more detail in 6/9. In the process it's been suggested to fix other
>> things, here is the attempt, with the great help of amders.
>>
>> The previous conversation is here:
>> https://lore.kernel.org/r/20230411125718.2297768-1-aik@amd.com
>>
>> [...]
> 
> Finally applied to kvm-x86 svm, thanks!  Though I was *really* tempted to see
> just how snarky the pings would get at week 5+ ;-)

Thanks!
Here is a gist what it could look like:
https://www.spinics.net/lists/kvm-ppc/msg20903.html :)


> 
> [1/9] KVM: SEV: move set_dr_intercepts/clr_dr_intercepts from the header
>        https://github.com/kvm-x86/linux/commit/b265ee7bae11
> [2/9] KVM: SEV: Move SEV's GP_VECTOR intercept setup to SEV
>        https://github.com/kvm-x86/linux/commit/29de732cc95c
> [3/9] KVM: SVM: Rewrite sev_es_prepare_switch_to_guest()'s comment about swap types
>        https://github.com/kvm-x86/linux/commit/f8d808ed1ba0
> [4/9] KVM: SEV-ES: explicitly disable debug
>        https://github.com/kvm-x86/linux/commit/2837dd00f8fc
> [5/9] KVM: SVM/SEV/SEV-ES: Rework intercepts
>        https://github.com/kvm-x86/linux/commit/5aefd3a05fe1
> [6/9] KVM: SEV: Enable data breakpoints in SEV-ES
>        https://github.com/kvm-x86/linux/commit/fb71b1298709
> [7/9] KVM: SEV-ES: Eliminate #DB intercept when DebugSwap enabled
>        https://github.com/kvm-x86/linux/commit/8b54cc7e1817
> [8/9] KVM: SVM: Don't defer NMI unblocking until next exit for SEV-ES guests
>        https://github.com/kvm-x86/linux/commit/c54268e1036f
> [9/9] KVM: SVM: Don't try to pointlessly single-step SEV-ES guests for NMI window
>        https://github.com/kvm-x86/linux/commit/e11f81043a12
> 
> --
> https://github.com/kvm-x86/linux/tree/next
> https://github.com/kvm-x86/linux/tree/fixes

-- 
Alexey