On Tue, Feb 1, 2022 at 2:17 PM Peter Gonda wrote: > On Fri, Jan 28, 2022 at 10:19 AM Brijesh Singh > wrote: > > > > Version 2 of GHCB specification provides SNP_GUEST_REQUEST and > > SNP_EXT_GUEST_REQUEST NAE that can be used by the SNP guest to > communicate > > with the PSP. > > > > While at it, add a snp_issue_guest_request() helper that will be used by > > driver or other subsystem to issue the request to PSP. > > > > See SEV-SNP firmware and GHCB spec for more details. > > > > Signed-off-by: Brijesh Singh > > --- > > arch/x86/include/asm/sev-common.h | 3 ++ > > arch/x86/include/asm/sev.h | 14 ++++++++ > > arch/x86/include/uapi/asm/svm.h | 4 +++ > > arch/x86/kernel/sev.c | 55 +++++++++++++++++++++++++++++++ > > 4 files changed, 76 insertions(+) > > > > diff --git a/arch/x86/include/asm/sev-common.h > b/arch/x86/include/asm/sev-common.h > > index cd769984e929..442614879dad 100644 > > --- a/arch/x86/include/asm/sev-common.h > > +++ b/arch/x86/include/asm/sev-common.h > > @@ -128,6 +128,9 @@ struct snp_psc_desc { > > struct psc_entry entries[VMGEXIT_PSC_MAX_ENTRY]; > > } __packed; > > > > +/* Guest message request error code */ > > +#define SNP_GUEST_REQ_INVALID_LEN BIT_ULL(32) > > + > > #define GHCB_MSR_TERM_REQ 0x100 > > #define GHCB_MSR_TERM_REASON_SET_POS 12 > > #define GHCB_MSR_TERM_REASON_SET_MASK 0xf > > diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h > > index 219abb4590f2..9830ee1d6ef0 100644 > > --- a/arch/x86/include/asm/sev.h > > +++ b/arch/x86/include/asm/sev.h > > @@ -87,6 +87,14 @@ extern bool handle_vc_boot_ghcb(struct pt_regs *regs); > > > > #define RMPADJUST_VMSA_PAGE_BIT BIT(16) > > > > +/* SNP Guest message request */ > > +struct snp_req_data { > > + unsigned long req_gpa; > > + unsigned long resp_gpa; > > + unsigned long data_gpa; > > + unsigned int data_npages; > > +}; > > + > > #ifdef CONFIG_AMD_MEM_ENCRYPT > > extern struct static_key_false sev_es_enable_key; > > extern void __sev_es_ist_enter(struct pt_regs *regs); > > @@ -154,6 +162,7 @@ void snp_set_memory_private(unsigned long vaddr, > unsigned int npages); > > void snp_set_wakeup_secondary_cpu(void); > > bool snp_init(struct boot_params *bp); > > void snp_abort(void); > > +int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, > unsigned long *fw_err); > > #else > > static inline void sev_es_ist_enter(struct pt_regs *regs) { } > > static inline void sev_es_ist_exit(void) { } > > @@ -173,6 +182,11 @@ static inline void snp_set_memory_private(unsigned > long vaddr, unsigned int npag > > static inline void snp_set_wakeup_secondary_cpu(void) { } > > static inline bool snp_init(struct boot_params *bp) { return false; } > > static inline void snp_abort(void) { } > > +static inline int snp_issue_guest_request(u64 exit_code, struct > snp_req_data *input, > > + unsigned long *fw_err) > > +{ > > + return -ENOTTY; > > +} > > #endif > > > > #endif > > diff --git a/arch/x86/include/uapi/asm/svm.h > b/arch/x86/include/uapi/asm/svm.h > > index 8b4c57baec52..5b8bc2b65a5e 100644 > > --- a/arch/x86/include/uapi/asm/svm.h > > +++ b/arch/x86/include/uapi/asm/svm.h > > @@ -109,6 +109,8 @@ > > #define SVM_VMGEXIT_SET_AP_JUMP_TABLE 0 > > #define SVM_VMGEXIT_GET_AP_JUMP_TABLE 1 > > #define SVM_VMGEXIT_PSC 0x80000010 > > +#define SVM_VMGEXIT_GUEST_REQUEST 0x80000011 > > +#define SVM_VMGEXIT_EXT_GUEST_REQUEST 0x80000012 > > #define SVM_VMGEXIT_AP_CREATION 0x80000013 > > #define SVM_VMGEXIT_AP_CREATE_ON_INIT 0 > > #define SVM_VMGEXIT_AP_CREATE 1 > > @@ -225,6 +227,8 @@ > > { SVM_VMGEXIT_AP_HLT_LOOP, "vmgexit_ap_hlt_loop" }, \ > > { SVM_VMGEXIT_AP_JUMP_TABLE, "vmgexit_ap_jump_table" }, \ > > { SVM_VMGEXIT_PSC, "vmgexit_page_state_change" }, \ > > + { SVM_VMGEXIT_GUEST_REQUEST, "vmgexit_guest_request" > }, \ > > + { SVM_VMGEXIT_EXT_GUEST_REQUEST, > "vmgexit_ext_guest_request" }, \ > > { SVM_VMGEXIT_AP_CREATION, "vmgexit_ap_creation" }, \ > > { SVM_VMGEXIT_HV_FEATURES, "vmgexit_hypervisor_feature" }, \ > > { SVM_EXIT_ERR, "invalid_guest_state" } > > diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c > > index cb97200bfda7..1d3ac83226fc 100644 > > --- a/arch/x86/kernel/sev.c > > +++ b/arch/x86/kernel/sev.c > > @@ -2122,3 +2122,58 @@ static int __init snp_check_cpuid_table(void) > > } > > > > arch_initcall(snp_check_cpuid_table); > > + > > +int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, > unsigned long *fw_err) > > +{ > > + struct ghcb_state state; > > + struct es_em_ctxt ctxt; > > + unsigned long flags; > > + struct ghcb *ghcb; > > + int ret; > > + > > + if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) > > + return -ENODEV; > > + > > + /* > > + * __sev_get_ghcb() needs to run with IRQs disabled because it > is using > > + * a per-CPU GHCB. > > + */ > > + local_irq_save(flags); > > + > > + ghcb = __sev_get_ghcb(&state); > > + if (!ghcb) { > > + ret = -EIO; > > + goto e_restore_irq; > > + } > > + > > + vc_ghcb_invalidate(ghcb); > > + > > + if (exit_code == SVM_VMGEXIT_EXT_GUEST_REQUEST) { > > + ghcb_set_rax(ghcb, input->data_gpa); > > + ghcb_set_rbx(ghcb, input->data_npages); > > + } > > + > > + ret = sev_es_ghcb_hv_call(ghcb, true, &ctxt, exit_code, > input->req_gpa, input->resp_gpa); > > + if (ret) > > + goto e_put; > > + > > + if (ghcb->save.sw_exit_info_2) { > > + /* Number of expected pages are returned in RBX */ > > + if (exit_code == SVM_VMGEXIT_EXT_GUEST_REQUEST && > > + ghcb->save.sw_exit_info_2 == > SNP_GUEST_REQ_INVALID_LEN) > > + input->data_npages = ghcb_get_rbx(ghcb); > > + > > + if (fw_err) > > + *fw_err = ghcb->save.sw_exit_info_2; > > In the PSP driver we've had a bit of discussion around the fw_err and > the return code and that it would be preferable to have fw_err be a > required parameter. And then we can easily make sure fw_err is always > non-zero when the return code is non-zero. Thoughts about doing the > same inside the guest? > > As per the GHCB spec, we will always have a non-zero error code. So, yes, I can drop the if() check. FYI, somehow this email did not show up on my @amd.com so I did not address it in v10. thanks