| Message ID | 20211110220731.2396491-11-brijesh.singh@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add AMD Secure Nested Paging (SEV-SNP) Guest Support | expand |
On Wed, Nov 10, 2021 at 04:06:56PM -0600, Brijesh Singh wrote: > +/* > + * The hypervisor features are available from GHCB version 2 onward. > + */ > +static bool get_hv_features(void) > +{ > + u64 val; > + > + sev_hv_features = 0; > + > + if (ghcb_version < 2) > + return false; > + > + sev_es_wr_ghcb_msr(GHCB_MSR_HV_FT_REQ); > + VMGEXIT(); > + > + val = sev_es_rd_ghcb_msr(); > + if (GHCB_RESP_CODE(val) != GHCB_MSR_HV_FT_RESP) > + return false; > + > + sev_hv_features = GHCB_MSR_HV_FT_RESP_VAL(val); > + > + return true; > +} I still don't like this. This is more of that run-me-in-the-exception-handler thing while this is purely feature detection stuff which needs to be done exactly once on init. IOW, that stanza if (!sev_es_negotiate_protocol()) sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SEV_ES_PROT_UNSUPPORTED); should be called once in sev_enable() for the decompressor kernel and once in sev_es_init_vc_handling() for kernel proper. Then you don't need to do any of that sev_hv_features = 0 thing but detect them exactly once and query them as much as you can. Thx.
On 12/2/21 11:52 AM, Borislav Petkov wrote: > On Wed, Nov 10, 2021 at 04:06:56PM -0600, Brijesh Singh wrote: >> +/* >> + * The hypervisor features are available from GHCB version 2 onward. >> + */ >> +static bool get_hv_features(void) >> +{ >> + u64 val; >> + >> + sev_hv_features = 0; >> + >> + if (ghcb_version < 2) >> + return false; >> + >> + sev_es_wr_ghcb_msr(GHCB_MSR_HV_FT_REQ); >> + VMGEXIT(); >> + >> + val = sev_es_rd_ghcb_msr(); >> + if (GHCB_RESP_CODE(val) != GHCB_MSR_HV_FT_RESP) >> + return false; >> + >> + sev_hv_features = GHCB_MSR_HV_FT_RESP_VAL(val); >> + >> + return true; >> +} > > I still don't like this. > > This is more of that run-me-in-the-exception-handler thing while this is > purely feature detection stuff which needs to be done exactly once on > init. > > IOW, that stanza > > if (!sev_es_negotiate_protocol()) > sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SEV_ES_PROT_UNSUPPORTED); > > should be called once in sev_enable() for the decompressor kernel and > once in sev_es_init_vc_handling() for kernel proper. > I will look into it, I will improve the sev_enable() to call GHCB MSR protocol query the version and feature. > Then you don't need to do any of that sev_hv_features = 0 thing but > detect them exactly once and query them as much as you can. > > Thx. >
diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h index 3278ee578937..891569c07ed7 100644 --- a/arch/x86/include/asm/sev-common.h +++ b/arch/x86/include/asm/sev-common.h @@ -60,6 +60,9 @@ /* GHCB Hypervisor Feature Request/Response */ #define GHCB_MSR_HV_FT_REQ 0x080 #define GHCB_MSR_HV_FT_RESP 0x081 +#define GHCB_MSR_HV_FT_RESP_VAL(v) \ + /* GHCBData[63:12] */ \ + (((u64)(v) & GENMASK_ULL(63, 12)) >> 12) #define GHCB_MSR_TERM_REQ 0x100 #define GHCB_MSR_TERM_REASON_SET_POS 12 diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h index 9b9c190e8c3b..17b75f6ee11a 100644 --- a/arch/x86/include/asm/sev.h +++ b/arch/x86/include/asm/sev.h @@ -13,7 +13,7 @@ #include <asm/sev-common.h> #define GHCB_PROTOCOL_MIN 1ULL -#define GHCB_PROTOCOL_MAX 1ULL +#define GHCB_PROTOCOL_MAX 2ULL #define GHCB_DEFAULT_USAGE 0ULL #define VMGEXIT() { asm volatile("rep; vmmcall\n\r"); } diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h index efa969325ede..b0ad00f4c1e1 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_HV_FEATURES 0x8000fffd #define SVM_VMGEXIT_UNSUPPORTED_EVENT 0x8000ffff /* Exit code reserved for hypervisor/software use */ @@ -218,6 +219,7 @@ { SVM_VMGEXIT_NMI_COMPLETE, "vmgexit_nmi_complete" }, \ { SVM_VMGEXIT_AP_HLT_LOOP, "vmgexit_ap_hlt_loop" }, \ { SVM_VMGEXIT_AP_JUMP_TABLE, "vmgexit_ap_jump_table" }, \ + { SVM_VMGEXIT_HV_FEATURES, "vmgexit_hypervisor_feature" }, \ { SVM_EXIT_ERR, "invalid_guest_state" } diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c index 91105f5a02a8..85b549f3ee1a 100644 --- a/arch/x86/kernel/sev-shared.c +++ b/arch/x86/kernel/sev-shared.c @@ -23,6 +23,9 @@ */ static u16 ghcb_version __ro_after_init; +/* Bitmap of SEV features supported by the hypervisor */ +static u64 sev_hv_features __ro_after_init; + static bool __init sev_es_check_cpu_features(void) { if (!has_cpuflag(X86_FEATURE_RDRAND)) { @@ -48,6 +51,30 @@ static void __noreturn sev_es_terminate(unsigned int set, unsigned int reason) asm volatile("hlt\n" : : : "memory"); } +/* + * The hypervisor features are available from GHCB version 2 onward. + */ +static bool get_hv_features(void) +{ + u64 val; + + sev_hv_features = 0; + + if (ghcb_version < 2) + return false; + + sev_es_wr_ghcb_msr(GHCB_MSR_HV_FT_REQ); + VMGEXIT(); + + val = sev_es_rd_ghcb_msr(); + if (GHCB_RESP_CODE(val) != GHCB_MSR_HV_FT_RESP) + return false; + + sev_hv_features = GHCB_MSR_HV_FT_RESP_VAL(val); + + return true; +} + static bool sev_es_negotiate_protocol(void) { u64 val; @@ -66,6 +93,9 @@ static bool sev_es_negotiate_protocol(void) ghcb_version = min_t(size_t, GHCB_MSR_PROTO_MAX(val), GHCB_PROTOCOL_MAX); + if (!get_hv_features()) + return false; + return true; }
Version 2 of GHCB specification introduced advertisement of a features that are supported by the hypervisor. Add support to query the HV features on boot. Version 2 of GHCB specification adds several new NAEs, most of them are optional except the hypervisor feature. Now that hypervisor feature NAE is implemented, so bump the GHCB maximum support protocol version. Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> --- arch/x86/include/asm/sev-common.h | 3 +++ arch/x86/include/asm/sev.h | 2 +- arch/x86/include/uapi/asm/svm.h | 2 ++ arch/x86/kernel/sev-shared.c | 30 ++++++++++++++++++++++++++++++ 4 files changed, 36 insertions(+), 1 deletion(-)