On Fri, Jan 28, 2022 at 10:19 AM Brijesh Singh <brijesh.singh@xxxxxxx> 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 <brijesh.singh@xxxxxxx>
> ---
> 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?
> +
> + ret = -EIO;
> + }
> +
> +e_put:
> + __sev_put_ghcb(&state);
> +e_restore_irq:
> + local_irq_restore(flags);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(snp_issue_guest_request);
> --
> 2.25.1
>
