[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b01a4948-0632-f899-2357-3bba6bc358ae@amd.com>
Date: Tue, 1 Aug 2023 10:49:38 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Nikunj A Dadhania <nikunj@....com>, linux-kernel@...r.kernel.org,
x86@...nel.org
Cc: bp@...en8.de, dionnaglaze@...gle.com, pgonda@...gle.com,
seanjc@...gle.com, pbonzini@...hat.com
Subject: Re: [PATCH v3 04/14] virt: sev-guest: Add SNP guest request structure
On 7/22/23 06:18, Nikunj A Dadhania wrote:
> Add a snp_guest_req structure to simplify the function arguments. The
> structure will be used to call the SNP Guest message request API
> instead of passing a long list of parameters.
>
> Add two helper functions for filling up the parameters:
> handle_guest_request() and handle_guest_request_ext(). GET_EXT_REPORT
> queries for certs_data from the AMD Security processor.
> handle_guest_request_ext() provides those extra parameters for
> receiving certs_data from the AMD security processor.
>
> Signed-off-by: Nikunj A Dadhania <nikunj@....com>
> ---
> .../x86/include/asm}/sev-guest.h | 11 ++
> arch/x86/include/asm/sev.h | 7 --
> arch/x86/kernel/sev.c | 15 ++-
> drivers/virt/coco/sev-guest/sev-guest.c | 107 ++++++++++++------
> 4 files changed, 93 insertions(+), 47 deletions(-)
> rename {drivers/virt/coco/sev-guest => arch/x86/include/asm}/sev-guest.h (80%)
>
> @@ -398,6 +393,46 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code,
> return 0;
> }
>
> +static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code,
> + struct snp_guest_request_ioctl *rio, u8 type,
> + void *req_buf, size_t req_sz, void *resp_buf,
> + u32 resp_sz)
> +{
> + struct snp_guest_req guest_req = {
> + .msg_version = rio->msg_version,
> + .msg_type = type,
> + .vmpck_id = vmpck_id,
> + .req_buf = req_buf,
> + .req_sz = req_sz,
> + .resp_buf = resp_buf,
> + .resp_sz = resp_sz,
> + .exit_code = exit_code,
> + };
> +
> + return snp_send_guest_request(snp_dev, &guest_req, rio);
> +}
> +
> +static int handle_guest_request_ext(struct snp_guest_dev *snp_dev, u64 exit_code,
> + struct snp_guest_request_ioctl *rio, u8 type,
> + void *req_buf, size_t req_sz, void *resp_buf,
> + u32 resp_sz, void *certs_data, size_t *npages)
> +{
> + struct snp_guest_req guest_req = {
> + .msg_version = rio->msg_version,
> + .msg_type = type,
> + .vmpck_id = vmpck_id,
> + .req_buf = req_buf,
> + .req_sz = req_sz,
> + .resp_buf = resp_buf,
> + .resp_sz = resp_sz,
> + .exit_code = exit_code,
> + .data = certs_data,
> + .data_npages = npages,
> + };
> +
> + return snp_send_guest_request(snp_dev, &guest_req, rio);
> +}
I'm not sure these intermediate funcitons are really necessary. Can't you
create/build the struct in get_report() and get_ext_report() and then just
call snp_send_guest_request() directly from those functions?
Thanks,
Tom
> +
> static int get_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_ioctl *arg)
> {
> struct snp_report_resp *resp;
> @@ -480,7 +515,8 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
> {
> struct snp_ext_report_req req;
> struct snp_report_resp *resp;
> - int ret, npages = 0, resp_len;
> + int ret, resp_len;
> + size_t npages = 0;
>
> lockdep_assert_held(&snp_dev->cmd_mutex);
>
> @@ -520,14 +556,14 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
> if (!resp)
> return -ENOMEM;
>
> - snp_dev->input.data_npages = npages;
> - ret = handle_guest_request(snp_dev, SVM_VMGEXIT_EXT_GUEST_REQUEST, arg,
> - SNP_MSG_REPORT_REQ, &req.data,
> - sizeof(req.data), resp->data, resp_len);
> + ret = handle_guest_request_ext(snp_dev, SVM_VMGEXIT_EXT_GUEST_REQUEST,
> + arg, SNP_MSG_REPORT_REQ, &req.data,
> + sizeof(req.data), resp->data, resp_len,
> + snp_dev->certs_data, &npages);
>
> /* If certs length is invalid then copy the returned length */
> if (arg->vmm_error == SNP_GUEST_VMM_ERR_INVALID_LEN) {
> - req.certs_len = snp_dev->input.data_npages << PAGE_SHIFT;
> + req.certs_len = npages << PAGE_SHIFT;
>
> if (copy_to_user((void __user *)arg->req_data, &req, sizeof(req)))
> ret = -EFAULT;
Powered by blists - more mailing lists