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

Brijesh Singh
2021-10-13T11:37:21Z

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@amd.com>
>> ---
>> 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