[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <e99be091-8671-0ffd-ee87-1952d8302e43@amd.com>
Date: Fri, 14 Apr 2023 10:40:54 +0530
From: "Nikunj A. Dadhania" <nikunj@....com>
To: Peter Gonda <pgonda@...gle.com>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org, bp@...en8.de,
thomas.lendacky@....com, dionnaglaze@...gle.com, seanjc@...gle.com,
pbonzini@...hat.com, michael.roth@....com, ketanch@...k.ac.in
Subject: Re: [PATCH v2 08/11] x86/sev: Add Secure TSC support for SNP guests
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
Powered by blists - more mailing lists