Messages in this thread | | | Date | Fri, 14 Apr 2023 10:40:54 +0530 | Subject | Re: [PATCH v2 08/11] x86/sev: Add Secure TSC support for SNP guests | From | "Nikunj A. Dadhania" <> |
| |
On 4/10/2023 10:44 PM, Peter Gonda wrote: >> + >> /* #VC handler runtime per-CPU data */ >> struct sev_es_runtime_data { >> struct ghcb ghcb_page; >> @@ -1107,7 +1111,7 @@ static void *alloc_shared_pages(size_t sz) >> return page_address(page); >> } >> >> -static int snp_setup_psp_messaging(struct sev_guest_platform_data *pdata) >> +static int __init snp_setup_psp_messaging(struct sev_guest_platform_data *pdata) >> { >> u64 gpa; >> int ret; >> @@ -1406,6 +1410,80 @@ bool snp_assign_vmpck(struct snp_guest_dev *dev, int vmpck_id) >> } >> EXPORT_SYMBOL_GPL(snp_assign_vmpck); >> >> +static int __init snp_get_tsc_info(void) >> +{ >> + u8 buf[SNP_TSC_INFO_REQ_SZ + AUTHTAG_LEN]; >> + struct snp_tsc_info_resp tsc_resp = {0}; >> + struct snp_tsc_info_req tsc_req; >> + struct snp_guest_req req; >> + struct snp_guest_dev dev; >> + int rc, resp_len; >> + >> + /* >> + * The intermediate response buffer is used while decrypting the >> + * response payload. Make sure that it has enough space to cover the >> + * authtag. >> + */ >> + resp_len = sizeof(tsc_resp) + AUTHTAG_LEN; >> + if (sizeof(buf) < resp_len) >> + return -EINVAL; >> + >> + /* Zero the tsc_info_req */ >> + memzero_explicit(&tsc_req, sizeof(tsc_req)); >> + memzero_explicit(&req, sizeof(req)); > > Whats the guidance on when we should use memzero_explicit() vs just > something like: `snp_tsc_info_resp tsc_resp = {0};`?
Going over the history of memzero_explicit, it seems it was introduce to explicitly zero sensitive information before the variable goes out of scope. GCC was optimizing out the memset in these cases:
d4c5efdb9777 ("random: add and use memzero_explicit() for clearing data")
https://bugzilla.kernel.org/show_bug.cgi?id=82041
With the above detail, IMHO, we do not need the memzero_explicit() for both case.
> >> + >> + dev.pdata = platform_data; >> + if (!snp_assign_vmpck(&dev, 0)) >> + return -EINVAL; >> + >> + req.msg_version = MSG_HDR_VER; >> + req.msg_type = SNP_MSG_TSC_INFO_REQ; >> + req.req_buf = &tsc_req; >> + req.req_sz = sizeof(tsc_req); >> + req.resp_buf = buf; >> + req.resp_sz = resp_len; >> + req.fw_err = NULL; > > Why do we not want the FW error code?
I will add the FW error code.
Regards Nikunj
| |