Re: [PATCH v6 40/42] virt: Add SEV-SNP guest driver

[Brijesh Singh]

Hi Dov,

On 10/10/21 10:51 AM, Dov Murik wrote:
> Hi Brijesh,
>
> On 08/10/2021 21:04, Brijesh Singh wrote:
>> SEV-SNP specification provides the guest a mechanisum to communicate with
>> the PSP without risk from a malicious hypervisor who wishes to read, alter,
>> drop or replay the messages sent. The driver uses snp_issue_guest_request()
>> to issue GHCB SNP_GUEST_REQUEST or SNP_EXT_GUEST_REQUEST NAE events to
>> submit the request to PSP.
>>
>> The PSP requires that all communication should be encrypted using key
>> specified through the platform_data.
>>
>> The userspace can use SNP_GET_REPORT ioctl() to query the guest
>> attestation report.
>>
>> See SEV-SNP spec section Guest Messages for more details.
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx>
>> ---
>>  Documentation/virt/coco/sevguest.rst  |  77 ++++
>>  drivers/virt/Kconfig                  |   3 +
>>  drivers/virt/Makefile                 |   1 +
>>  drivers/virt/coco/sevguest/Kconfig    |   9 +
>>  drivers/virt/coco/sevguest/Makefile   |   2 +
>>  drivers/virt/coco/sevguest/sevguest.c | 561 ++++++++++++++++++++++++++
>>  drivers/virt/coco/sevguest/sevguest.h |  98 +++++
>>  include/uapi/linux/sev-guest.h        |  44 ++
>>  8 files changed, 795 insertions(+)
>>  create mode 100644 Documentation/virt/coco/sevguest.rst
>>  create mode 100644 drivers/virt/coco/sevguest/Kconfig
>>  create mode 100644 drivers/virt/coco/sevguest/Makefile
>>  create mode 100644 drivers/virt/coco/sevguest/sevguest.c
>>  create mode 100644 drivers/virt/coco/sevguest/sevguest.h
>>  create mode 100644 include/uapi/linux/sev-guest.h
>>
> [...]
>
>
>> +
>> +static u8 *get_vmpck(int id, struct snp_secrets_page_layout *layout, u32 **seqno)
>> +{
>> +	u8 *key = NULL;
>> +
>> +	switch (id) {
>> +	case 0:
>> +		*seqno = &layout->os_area.msg_seqno_0;
>> +		key = layout->vmpck0;
>> +		break;
>> +	case 1:
>> +		*seqno = &layout->os_area.msg_seqno_1;
>> +		key = layout->vmpck1;
>> +		break;
>> +	case 2:
>> +		*seqno = &layout->os_area.msg_seqno_2;
>> +		key = layout->vmpck2;
>> +		break;
>> +	case 3:
>> +		*seqno = &layout->os_area.msg_seqno_3;
>> +		key = layout->vmpck3;
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +
>> +	return NULL;
> This should be 'return key', right?


Yes, I did caught that during my testing and the hunk to fix it is in
42/42. I missed merging the hunk in this patch and will take care in
next rev. thanks