From: Joerg Roedel <[email protected]>
Hi,
here is a small set of patches which I took from the pending SEV-SNP
patch-sets to enable basic support for GHCB protocol version 2.
When SEV-SNP is not supported, only two new MSR protocol VMGEXIT calls
need to be supported:
- MSR-based AP-reset-hold
- MSR-based HV-feature-request
These calls are implemented here and then the protocol is lifted to
version 2.
This is submitted separately because the MSR-based AP-reset-hold call
is required to support kexec/kdump in SEV-ES guests.
Regards,
Joerg
Changes v2->v3:
- Rebased to v5.15-rc1
- Reworked GHCB_MSR access interfaces as suggested by Sean
Christopherson.
Brijesh Singh (2):
KVM: SVM: Add support for Hypervisor Feature support MSR protocol
KVM: SVM: Increase supported GHCB protocol version
Joerg Roedel (1):
KVM: SVM: Get rid of set_ghcb_msr() and *ghcb_msr_bits() functions
Tom Lendacky (1):
KVM: SVM: Add support to handle AP reset MSR protocol
arch/x86/include/asm/kvm_host.h | 10 ++-
arch/x86/include/asm/sev-common.h | 9 +++
arch/x86/include/uapi/asm/svm.h | 1 +
arch/x86/kvm/svm/sev.c | 115 +++++++++++++++++++-----------
arch/x86/kvm/svm/svm.h | 3 +-
arch/x86/kvm/x86.c | 5 +-
6 files changed, 98 insertions(+), 45 deletions(-)
base-commit: 6880fa6c56601bb8ed59df6c30fd390cc5f6dd8f
--
2.33.0
From: Tom Lendacky <[email protected]>
Add support for AP Reset Hold being invoked using the GHCB MSR protocol,
available in version 2 of the GHCB specification.
Signed-off-by: Tom Lendacky <[email protected]>
Signed-off-by: Brijesh Singh <[email protected]>
Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/include/asm/kvm_host.h | 10 ++++++-
arch/x86/kvm/svm/sev.c | 48 ++++++++++++++++++++++++++-------
arch/x86/kvm/x86.c | 5 +++-
3 files changed, 51 insertions(+), 12 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index f8f48a7ec577..2f84cda48cdf 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -237,6 +237,11 @@ enum x86_intercept_stage;
KVM_GUESTDBG_INJECT_DB | \
KVM_GUESTDBG_BLOCKIRQ)
+enum ap_reset_hold_type {
+ AP_RESET_HOLD_NONE,
+ AP_RESET_HOLD_NAE_EVENT,
+ AP_RESET_HOLD_MSR_PROTO,
+};
#define PFERR_PRESENT_BIT 0
#define PFERR_WRITE_BIT 1
@@ -909,6 +914,8 @@ struct kvm_vcpu_arch {
#if IS_ENABLED(CONFIG_HYPERV)
hpa_t hv_root_tdp;
#endif
+
+ enum ap_reset_hold_type reset_hold_type;
};
struct kvm_lpage_info {
@@ -1675,7 +1682,8 @@ int kvm_fast_pio(struct kvm_vcpu *vcpu, int size, unsigned short port, int in);
int kvm_emulate_cpuid(struct kvm_vcpu *vcpu);
int kvm_emulate_halt(struct kvm_vcpu *vcpu);
int kvm_vcpu_halt(struct kvm_vcpu *vcpu);
-int kvm_emulate_ap_reset_hold(struct kvm_vcpu *vcpu);
+int kvm_emulate_ap_reset_hold(struct kvm_vcpu *vcpu,
+ enum ap_reset_hold_type type);
int kvm_emulate_wbinvd(struct kvm_vcpu *vcpu);
void kvm_get_segment(struct kvm_vcpu *vcpu, struct kvm_segment *var, int seg);
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index f368390a5563..8cd1e4ebb4a8 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2214,6 +2214,9 @@ static int sev_es_validate_vmgexit(struct vcpu_svm *svm)
void sev_es_unmap_ghcb(struct vcpu_svm *svm)
{
+ /* Clear any indication that the vCPU is in a type of AP Reset Hold */
+ svm->vcpu.arch.reset_hold_type = AP_RESET_HOLD_NONE;
+
if (!svm->ghcb)
return;
@@ -2357,6 +2360,11 @@ static u64 ghcb_msr_cpuid_resp(u64 reg, u64 value)
return msr;
}
+static u64 ghcb_msr_ap_rst_resp(u64 value)
+{
+ return (u64)GHCB_MSR_AP_RESET_HOLD_RESP | (value << GHCB_DATA_LOW);
+}
+
static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
{
struct vmcb_control_area *control = &svm->vmcb->control;
@@ -2405,6 +2413,16 @@ static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
break;
}
+ case GHCB_MSR_AP_RESET_HOLD_REQ:
+ ret = kvm_emulate_ap_reset_hold(&svm->vcpu, AP_RESET_HOLD_MSR_PROTO);
+
+ /*
+ * Preset the result to a non-SIPI return and then only set
+ * the result to non-zero when delivering a SIPI.
+ */
+ svm->vmcb->control.ghcb_gpa = ghcb_msr_ap_rst_resp(0);
+
+ break;
case GHCB_MSR_TERM_REQ: {
u64 reason_set, reason_code;
@@ -2490,7 +2508,7 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
ret = svm_invoke_exit_handler(vcpu, SVM_EXIT_IRET);
break;
case SVM_VMGEXIT_AP_HLT_LOOP:
- ret = kvm_emulate_ap_reset_hold(vcpu);
+ ret = kvm_emulate_ap_reset_hold(vcpu, AP_RESET_HOLD_NAE_EVENT);
break;
case SVM_VMGEXIT_AP_JUMP_TABLE: {
struct kvm_sev_info *sev = &to_kvm_svm(vcpu->kvm)->sev_info;
@@ -2627,13 +2645,23 @@ void sev_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector)
return;
}
- /*
- * Subsequent SIPI: Return from an AP Reset Hold VMGEXIT, where
- * the guest will set the CS and RIP. Set SW_EXIT_INFO_2 to a
- * non-zero value.
- */
- if (!svm->ghcb)
- return;
-
- ghcb_set_sw_exit_info_2(svm->ghcb, 1);
+ /* Subsequent SIPI */
+ switch (vcpu->arch.reset_hold_type) {
+ case AP_RESET_HOLD_NAE_EVENT:
+ /*
+ * Return from an AP Reset Hold VMGEXIT, where the guest will
+ * set the CS and RIP. Set SW_EXIT_INFO_2 to a non-zero value.
+ */
+ ghcb_set_sw_exit_info_2(svm->ghcb, 1);
+ break;
+ case AP_RESET_HOLD_MSR_PROTO:
+ /*
+ * Return from an AP Reset Hold VMGEXIT, where the guest will
+ * set the CS and RIP. Set GHCB data field to a non-zero value.
+ */
+ svm->vmcb->control.ghcb_gpa = ghcb_msr_ap_rst_resp(1);
+ break;
+ default:
+ break;
+ }
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 28ef14155726..412a195b07af 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8516,10 +8516,13 @@ int kvm_emulate_halt(struct kvm_vcpu *vcpu)
}
EXPORT_SYMBOL_GPL(kvm_emulate_halt);
-int kvm_emulate_ap_reset_hold(struct kvm_vcpu *vcpu)
+int kvm_emulate_ap_reset_hold(struct kvm_vcpu *vcpu,
+ enum ap_reset_hold_type type)
{
int ret = kvm_skip_emulated_instruction(vcpu);
+ vcpu->arch.reset_hold_type = type;
+
return __kvm_vcpu_halt(vcpu, KVM_MP_STATE_AP_RESET_HOLD, KVM_EXIT_AP_RESET_HOLD) && ret;
}
EXPORT_SYMBOL_GPL(kvm_emulate_ap_reset_hold);
--
2.33.0
From: Joerg Roedel <[email protected]>
Replace the get_ghcb_msr_bits() function with macros and open code the
GHCB MSR setters with hypercall specific helper macros and functions.
This will avoid preserving any previous bits in the GHCB-MSR and
improves code readability.
Also get rid of the set_ghcb_msr() function and open-code it at its
call-sites for better code readability.
Suggested-by: Sean Christopherson <[email protected]>
Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/include/asm/sev-common.h | 9 +++++
arch/x86/kvm/svm/sev.c | 56 +++++++++++--------------------
2 files changed, 29 insertions(+), 36 deletions(-)
diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index 2cef6c5a52c2..8540972cad04 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -50,6 +50,10 @@
(GHCB_MSR_CPUID_REQ | \
(((unsigned long)reg & GHCB_MSR_CPUID_REG_MASK) << GHCB_MSR_CPUID_REG_POS) | \
(((unsigned long)fn) << GHCB_MSR_CPUID_FUNC_POS))
+#define GHCB_MSR_CPUID_FN(msr) \
+ (((msr) >> GHCB_MSR_CPUID_FUNC_POS) & GHCB_MSR_CPUID_FUNC_MASK)
+#define GHCB_MSR_CPUID_REG(msr) \
+ (((msr) >> GHCB_MSR_CPUID_REG_POS) & GHCB_MSR_CPUID_REG_MASK)
/* AP Reset Hold */
#define GHCB_MSR_AP_RESET_HOLD_REQ 0x006
@@ -67,6 +71,11 @@
#define GHCB_SEV_TERM_REASON(reason_set, reason_val) \
(((((u64)reason_set) & GHCB_MSR_TERM_REASON_SET_MASK) << GHCB_MSR_TERM_REASON_SET_POS) | \
((((u64)reason_val) & GHCB_MSR_TERM_REASON_MASK) << GHCB_MSR_TERM_REASON_POS))
+#define GHCB_MSR_TERM_REASON_SET(msr) \
+ (((msr) >> GHCB_MSR_TERM_REASON_SET_POS) & GHCB_MSR_TERM_REASON_SET_MASK)
+#define GHCB_MSR_TERM_REASON(msr) \
+ (((msr) >> GHCB_MSR_TERM_REASON_POS) & GHCB_MSR_TERM_REASON_MASK)
+
#define GHCB_SEV_ES_REASON_GENERAL_REQUEST 0
#define GHCB_SEV_ES_REASON_PROTOCOL_UNSUPPORTED 1
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 75e0b21ad07c..f368390a5563 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2346,21 +2346,15 @@ static bool setup_vmgexit_scratch(struct vcpu_svm *svm, bool sync, u64 len)
return true;
}
-static void set_ghcb_msr_bits(struct vcpu_svm *svm, u64 value, u64 mask,
- unsigned int pos)
+static u64 ghcb_msr_cpuid_resp(u64 reg, u64 value)
{
- svm->vmcb->control.ghcb_gpa &= ~(mask << pos);
- svm->vmcb->control.ghcb_gpa |= (value & mask) << pos;
-}
+ u64 msr;
-static u64 get_ghcb_msr_bits(struct vcpu_svm *svm, u64 mask, unsigned int pos)
-{
- return (svm->vmcb->control.ghcb_gpa >> pos) & mask;
-}
+ msr = GHCB_MSR_CPUID_RESP;
+ msr |= (reg & GHCB_MSR_CPUID_REG_MASK) << GHCB_MSR_CPUID_REG_POS;
+ msr |= (value & GHCB_MSR_CPUID_VALUE_MASK) << GHCB_MSR_CPUID_VALUE_POS;
-static void set_ghcb_msr(struct vcpu_svm *svm, u64 value)
-{
- svm->vmcb->control.ghcb_gpa = value;
+ return msr;
}
static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
@@ -2377,16 +2371,14 @@ static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
switch (ghcb_info) {
case GHCB_MSR_SEV_INFO_REQ:
- set_ghcb_msr(svm, GHCB_MSR_SEV_INFO(GHCB_VERSION_MAX,
- GHCB_VERSION_MIN,
- sev_enc_bit));
+ svm->vmcb->control.ghcb_gpa = GHCB_MSR_SEV_INFO(GHCB_VERSION_MAX,
+ GHCB_VERSION_MIN,
+ sev_enc_bit);
break;
case GHCB_MSR_CPUID_REQ: {
u64 cpuid_fn, cpuid_reg, cpuid_value;
- cpuid_fn = get_ghcb_msr_bits(svm,
- GHCB_MSR_CPUID_FUNC_MASK,
- GHCB_MSR_CPUID_FUNC_POS);
+ cpuid_fn = GHCB_MSR_CPUID_FN(control->ghcb_gpa);
/* Initialize the registers needed by the CPUID intercept */
vcpu->arch.regs[VCPU_REGS_RAX] = cpuid_fn;
@@ -2398,9 +2390,8 @@ static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
break;
}
- cpuid_reg = get_ghcb_msr_bits(svm,
- GHCB_MSR_CPUID_REG_MASK,
- GHCB_MSR_CPUID_REG_POS);
+ cpuid_reg = GHCB_MSR_CPUID_REG(control->ghcb_gpa);
+
if (cpuid_reg == 0)
cpuid_value = vcpu->arch.regs[VCPU_REGS_RAX];
else if (cpuid_reg == 1)
@@ -2410,26 +2401,19 @@ static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
else
cpuid_value = vcpu->arch.regs[VCPU_REGS_RDX];
- set_ghcb_msr_bits(svm, cpuid_value,
- GHCB_MSR_CPUID_VALUE_MASK,
- GHCB_MSR_CPUID_VALUE_POS);
+ svm->vmcb->control.ghcb_gpa = ghcb_msr_cpuid_resp(cpuid_reg, cpuid_value);
- set_ghcb_msr_bits(svm, GHCB_MSR_CPUID_RESP,
- GHCB_MSR_INFO_MASK,
- GHCB_MSR_INFO_POS);
break;
}
case GHCB_MSR_TERM_REQ: {
u64 reason_set, reason_code;
- reason_set = get_ghcb_msr_bits(svm,
- GHCB_MSR_TERM_REASON_SET_MASK,
- GHCB_MSR_TERM_REASON_SET_POS);
- reason_code = get_ghcb_msr_bits(svm,
- GHCB_MSR_TERM_REASON_MASK,
- GHCB_MSR_TERM_REASON_POS);
+ reason_set = GHCB_MSR_TERM_REASON_SET(control->ghcb_gpa);
+ reason_code = GHCB_MSR_TERM_REASON(control->ghcb_gpa);
+
pr_info("SEV-ES guest requested termination: %#llx:%#llx\n",
reason_set, reason_code);
+
fallthrough;
}
default:
@@ -2605,9 +2589,9 @@ void sev_es_create_vcpu(struct vcpu_svm *svm)
* Set the GHCB MSR value as per the GHCB specification when creating
* a vCPU for an SEV-ES guest.
*/
- set_ghcb_msr(svm, GHCB_MSR_SEV_INFO(GHCB_VERSION_MAX,
- GHCB_VERSION_MIN,
- sev_enc_bit));
+ svm->vmcb->control.ghcb_gpa = GHCB_MSR_SEV_INFO(GHCB_VERSION_MAX,
+ GHCB_VERSION_MIN,
+ sev_enc_bit);
}
void sev_es_prepare_guest_switch(struct vcpu_svm *svm, unsigned int cpu)
--
2.33.0
From: Brijesh Singh <[email protected]>
Now that KVM has basic support for version 2 of the GHCB specification,
bump the maximum supported protocol version. The SNP specific functions
are still missing, but those are only required when the Hypervisor
supports running SNP guests.
Signed-off-by: Brijesh Singh <[email protected]>
Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/kvm/svm/svm.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index f019da28ff06..0650f51f7de0 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -539,7 +539,7 @@ void svm_vcpu_unblocking(struct kvm_vcpu *vcpu);
/* sev.c */
-#define GHCB_VERSION_MAX 1ULL
+#define GHCB_VERSION_MAX 2ULL
#define GHCB_VERSION_MIN 1ULL
#define GHCB_HV_FT_SUPPORTED 0
--
2.33.0
From: Brijesh Singh <[email protected]>
Version 2 of the GHCB specification introduced advertisement of
supported Hypervisor SEV features. This request is required to support
a the GHCB version 2 protocol.
Signed-off-by: Brijesh Singh <[email protected]>
Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/include/uapi/asm/svm.h | 1 +
arch/x86/kvm/svm/sev.c | 19 +++++++++++++++++++
arch/x86/kvm/svm/svm.h | 1 +
3 files changed, 21 insertions(+)
diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h
index efa969325ede..67cf153fe580 100644
--- a/arch/x86/include/uapi/asm/svm.h
+++ b/arch/x86/include/uapi/asm/svm.h
@@ -108,6 +108,7 @@
#define SVM_VMGEXIT_AP_JUMP_TABLE 0x80000005
#define SVM_VMGEXIT_SET_AP_JUMP_TABLE 0
#define SVM_VMGEXIT_GET_AP_JUMP_TABLE 1
+#define SVM_VMGEXIT_HYPERVISOR_FEATURES 0x8000fffd
#define SVM_VMGEXIT_UNSUPPORTED_EVENT 0x8000ffff
/* Exit code reserved for hypervisor/software use */
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 8cd1e4ebb4a8..b3fdfccf0499 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2184,6 +2184,7 @@ static int sev_es_validate_vmgexit(struct vcpu_svm *svm)
case SVM_VMGEXIT_AP_HLT_LOOP:
case SVM_VMGEXIT_AP_JUMP_TABLE:
case SVM_VMGEXIT_UNSUPPORTED_EVENT:
+ case SVM_VMGEXIT_HYPERVISOR_FEATURES:
break;
default:
goto vmgexit_err;
@@ -2365,6 +2366,16 @@ static u64 ghcb_msr_ap_rst_resp(u64 value)
return (u64)GHCB_MSR_AP_RESET_HOLD_RESP | (value << GHCB_DATA_LOW);
}
+static u64 ghcb_msr_hv_feat_resp(u64 value)
+{
+ u64 msr;
+
+ msr = GHCB_MSR_HV_FT_RESP;
+ msr |= (value << GHCB_DATA_LOW);
+
+ return msr;
+}
+
static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
{
struct vmcb_control_area *control = &svm->vmcb->control;
@@ -2422,6 +2433,9 @@ static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
*/
svm->vmcb->control.ghcb_gpa = ghcb_msr_ap_rst_resp(0);
+ break;
+ case GHCB_MSR_HV_FT_REQ:
+ svm->vmcb->control.ghcb_gpa = ghcb_msr_hv_feat_resp(GHCB_HV_FT_SUPPORTED);
break;
case GHCB_MSR_TERM_REQ: {
u64 reason_set, reason_code;
@@ -2535,6 +2549,11 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
ret = 1;
break;
}
+ case SVM_VMGEXIT_HYPERVISOR_FEATURES:
+ ghcb_set_sw_exit_info_2(ghcb, GHCB_HV_FT_SUPPORTED);
+
+ ret = 1;
+ break;
case SVM_VMGEXIT_UNSUPPORTED_EVENT:
vcpu_unimpl(vcpu,
"vmgexit: unsupported event - exit_info_1=%#llx, exit_info_2=%#llx\n",
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 524d943f3efc..f019da28ff06 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -542,6 +542,7 @@ void svm_vcpu_unblocking(struct kvm_vcpu *vcpu);
#define GHCB_VERSION_MAX 1ULL
#define GHCB_VERSION_MIN 1ULL
+#define GHCB_HV_FT_SUPPORTED 0
extern unsigned int max_sev_asid;
--
2.33.0
On Mon, Sep 13, 2021, Joerg Roedel wrote:
> static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
> @@ -2377,16 +2371,14 @@ static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
>
> switch (ghcb_info) {
> case GHCB_MSR_SEV_INFO_REQ:
> - set_ghcb_msr(svm, GHCB_MSR_SEV_INFO(GHCB_VERSION_MAX,
> - GHCB_VERSION_MIN,
> - sev_enc_bit));
> + svm->vmcb->control.ghcb_gpa = GHCB_MSR_SEV_INFO(GHCB_VERSION_MAX,
> + GHCB_VERSION_MIN,
> + sev_enc_bit);
Nit, "svm->vmcb->control." can be "control->".
With that fixed,
Reviewed-by: Sean Christopherson <[email protected]>
> break;
> case GHCB_MSR_CPUID_REQ: {
> u64 cpuid_fn, cpuid_reg, cpuid_value;
>
> - cpuid_fn = get_ghcb_msr_bits(svm,
> - GHCB_MSR_CPUID_FUNC_MASK,
> - GHCB_MSR_CPUID_FUNC_POS);
> + cpuid_fn = GHCB_MSR_CPUID_FN(control->ghcb_gpa);
>
> /* Initialize the registers needed by the CPUID intercept */
> vcpu->arch.regs[VCPU_REGS_RAX] = cpuid_fn;
> @@ -2398,9 +2390,8 @@ static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
> break;
> }
>
> - cpuid_reg = get_ghcb_msr_bits(svm,
> - GHCB_MSR_CPUID_REG_MASK,
> - GHCB_MSR_CPUID_REG_POS);
> + cpuid_reg = GHCB_MSR_CPUID_REG(control->ghcb_gpa);
> +
> if (cpuid_reg == 0)
> cpuid_value = vcpu->arch.regs[VCPU_REGS_RAX];
> else if (cpuid_reg == 1)
> @@ -2410,26 +2401,19 @@ static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
> else
> cpuid_value = vcpu->arch.regs[VCPU_REGS_RDX];
>
> - set_ghcb_msr_bits(svm, cpuid_value,
> - GHCB_MSR_CPUID_VALUE_MASK,
> - GHCB_MSR_CPUID_VALUE_POS);
> + svm->vmcb->control.ghcb_gpa = ghcb_msr_cpuid_resp(cpuid_reg, cpuid_value);
>
> - set_ghcb_msr_bits(svm, GHCB_MSR_CPUID_RESP,
> - GHCB_MSR_INFO_MASK,
> - GHCB_MSR_INFO_POS);
I don't mind using a helper instead of a macro (I completely agree the helper is
easier to read), but we should be consistent, i.e. get rid of GHCB_MSR_SEV_INFO.
And IMO that helper should handle the min/max version, not the callers. Add this
after patch 01 (modulo the above nit)?
From 5c329f9e7156948697955a2745f32df024aa5ee6 Mon Sep 17 00:00:00 2001
From: Sean Christopherson <[email protected]>
Date: Fri, 24 Sep 2021 12:58:31 -0700
Subject: [PATCH] KVM: SVM: Add helper to generate GHCB MSR verson info, and
drop macro
Convert the GHCB_MSR_SEV_INFO macro into a helper function, and have the
helper hardcode the min/max versions instead of relying on the caller to
do the same. Under no circumstance should different pieces of KVM define
different min/max versions.
No functional change intended.
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/include/asm/sev-common.h | 5 -----
arch/x86/kvm/svm/sev.c | 24 ++++++++++++++++++------
arch/x86/kvm/svm/svm.h | 5 -----
3 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index 8540972cad04..886c36f0cb16 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -24,11 +24,6 @@
#define GHCB_MSR_VER_MIN_MASK 0xffff
#define GHCB_MSR_CBIT_POS 24
#define GHCB_MSR_CBIT_MASK 0xff
-#define GHCB_MSR_SEV_INFO(_max, _min, _cbit) \
- ((((_max) & GHCB_MSR_VER_MAX_MASK) << GHCB_MSR_VER_MAX_POS) | \
- (((_min) & GHCB_MSR_VER_MIN_MASK) << GHCB_MSR_VER_MIN_POS) | \
- (((_cbit) & GHCB_MSR_CBIT_MASK) << GHCB_MSR_CBIT_POS) | \
- GHCB_MSR_SEV_INFO_RESP)
#define GHCB_MSR_INFO(v) ((v) & 0xfffUL)
#define GHCB_MSR_PROTO_MAX(v) (((v) >> GHCB_MSR_VER_MAX_POS) & GHCB_MSR_VER_MAX_MASK)
#define GHCB_MSR_PROTO_MIN(v) (((v) >> GHCB_MSR_VER_MIN_POS) & GHCB_MSR_VER_MIN_MASK)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 7aa6ad4c3118..159b22bb74e4 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2389,6 +2389,22 @@ static u64 ghcb_msr_cpuid_resp(u64 reg, u64 value)
return msr;
}
+/* The min/max GHCB version supported by KVM. */
+#define GHCB_VERSION_MAX 1ULL
+#define GHCB_VERSION_MIN 1ULL
+
+static u64 ghcb_msr_version_info(void)
+{
+ u64 msr;
+
+ msr = GHCB_MSR_SEV_INFO_RESP;
+ msr |= GHCB_VERSION_MAX << GHCB_MSR_VER_MAX_POS;
+ msr |= GHCB_VERSION_MIN << GHCB_MSR_VER_MIN_POS;
+ msr |= sev_enc_bit << GHCB_MSR_CBIT_POS;
+
+ return msr;
+}
+
static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
{
struct vmcb_control_area *control = &svm->vmcb->control;
@@ -2403,9 +2419,7 @@ static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
switch (ghcb_info) {
case GHCB_MSR_SEV_INFO_REQ:
- svm->vmcb->control.ghcb_gpa = GHCB_MSR_SEV_INFO(GHCB_VERSION_MAX,
- GHCB_VERSION_MIN,
- sev_enc_bit);
+ control->ghcb_gpa = ghcb_msr_version_info();
break;
case GHCB_MSR_CPUID_REQ: {
u64 cpuid_fn, cpuid_reg, cpuid_value;
@@ -2621,9 +2635,7 @@ void sev_es_vcpu_reset(struct vcpu_svm *svm)
* Set the GHCB MSR value as per the GHCB specification when emulating
* vCPU RESET for an SEV-ES guest.
*/
- svm->vmcb->control.ghcb_gpa = GHCB_MSR_SEV_INFO(GHCB_VERSION_MAX,
- GHCB_VERSION_MIN,
- sev_enc_bit);
+ svm->vmcb->control.ghcb_gpa = ghcb_msr_version_info();
}
void sev_es_prepare_guest_switch(struct vcpu_svm *svm, unsigned int cpu)
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 0d7bbe548ac3..68e5f16a0554 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -544,11 +544,6 @@ void svm_vcpu_blocking(struct kvm_vcpu *vcpu);
void svm_vcpu_unblocking(struct kvm_vcpu *vcpu);
/* sev.c */
-
-#define GHCB_VERSION_MAX 1ULL
-#define GHCB_VERSION_MIN 1ULL
-
-
extern unsigned int max_sev_asid;
void sev_vm_destroy(struct kvm *kvm);
--
2.33.0.685.g46640cef36-goog
On Mon, Sep 13, 2021, Joerg Roedel wrote:
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 524d943f3efc..f019da28ff06 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -542,6 +542,7 @@ void svm_vcpu_unblocking(struct kvm_vcpu *vcpu);
> #define GHCB_VERSION_MAX 1ULL
> #define GHCB_VERSION_MIN 1ULL
>
> +#define GHCB_HV_FT_SUPPORTED 0
I'd prefer to keep these SEV-only defines in sev.c. I also think it should have
KVM somewhere in the name, a la KVM_SUPPORTED_XCR0, e.g. KVM_SUPPORTED_GHCB_FEATURES?