Re: [PATCH v3 04/14] virt: sev-guest: Add SNP guest request structure

[Nikunj A. Dadhania]

On 8/1/2023 9:19 PM, Tom Lendacky wrote:
> 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@xxxxxxx>
>> ---
>>   .../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?

No particular preference, for tsc_info in sev.c I have build the structure directly and called snp_send_guest_request() directly. I will remove the above helper functions.

Regards,
Nikunj