On 13/01/23 17:23, Zhi Wang wrote: > On Thu, 12 Jan 2023 14:11:39 +0530 > Nikunj A Dadhania <nikunj@xxxxxxx> wrote: > >> The hypervisor can enable various new features (SEV_FEATURES[1:63]) >> and start the SNP guest. Some of these features need guest side >> implementation. If any of these features are enabled without guest >> side implementation, the behavior of the SNP guest will be undefined. >> The SNP guest boot may fail in a non-obvious way making it difficult >> to debug. >> >> Instead of allowing the guest to continue and have it fail randomly >> later, detect this early and fail gracefully. >> >> SEV_STATUS MSR indicates features which the hypervisor has enabled. >> While booting, SNP guests should ascertain that all the enabled >> features have guest side implementation. In case any feature is not >> implemented in the guest, the guest terminates booting with GHCB >> protocol Non-Automatic Exit(NAE) termination request event[1]. Populate >> SW_EXITINFO2 with mask of unsupported features that the hypervisor >> can easily report to the user. >> >> More details in AMD64 APM[2] Vol 2: 15.34.10 SEV_STATUS MSR >> >> [1] https://developer.amd.com/wp-content/resources/56421.pdf >> 4.1.13 Termination Request >> >> [2] https://www.amd.com/system/files/TechDocs/40332_4.05.pdf >> > > The link of [2] is broken. Better update it. That is strange, I will fix that. >> + >> +void snp_check_features(void) >> +{ >> + u64 unsupported_features; >> + >> + if (!(sev_status & MSR_AMD64_SEV_SNP_ENABLED)) >> + return; >> + >> + /* >> + * Terminate the boot if hypervisor has enabled any feature >> + * lacking guest side implementation. >> + */ >> + unsupported_features = sev_status & SNP_FEATURES_IMPL_REQ & >> ~SNP_FEATURES_PRESENT; >> + if (unsupported_features) { >> + if (sev_es_get_ghcb_version() < 2 || >> + (!boot_ghcb && !early_setup_ghcb())) >> + sev_es_terminate(SEV_TERM_SET_GEN, >> GHCB_SNP_UNSUPPORTED); + > > === >> + u64 exit_info_1 = >> SVM_VMGEXIT_TERM_REASON(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED); + >> + vc_ghcb_invalidate(boot_ghcb); >> + ghcb_set_sw_exit_code(boot_ghcb, >> SVM_VMGEXIT_TERM_REQUEST); >> + ghcb_set_sw_exit_info_1(boot_ghcb, exit_info_1); >> + ghcb_set_sw_exit_info_2(boot_ghcb, >> unsupported_features); + >> + sev_es_wr_ghcb_msr(__pa(boot_ghcb)); >> + VMGEXIT(); >> + >> + while (true) >> + asm volatile("hlt\n" : : : "memory"); > === > > This seems another approach to terminate the guest which can bring extra > reason info compared to sev_es_terminate(). It would be better to wrap > the above snippet into a function and call it here. Right, I will add it as part of the new function in my next revision: static void __noreturn sev_es_ghcb_terminate(struct ghcb *ghcb, u64 exit_info_1, u64 exit_info_2) Regards Nikunj
