Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <27a90573-53cd-846a-e09f-c22c649db60a@amd.com>
Date:   Fri, 21 Oct 2022 14:06:36 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH Part2 v6 42/49] KVM: SVM: Provide support for
 SNP_GUEST_REQUEST NAE event
Content-Language: en-US
To:     Ashish Kalra <Ashish.Kalra@amd.com>, x86@kernel.org,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        linux-coco@lists.linux.dev, linux-mm@kvack.org,
        linux-crypto@vger.kernel.org
Cc:     tglx@linutronix.de, mingo@redhat.com, jroedel@suse.de,
        hpa@zytor.com, ardb@kernel.org, pbonzini@redhat.com,
        seanjc@google.com, vkuznets@redhat.com, jmattson@google.com,
        luto@kernel.org, dave.hansen@linux.intel.com, slp@redhat.com,
        pgonda@google.com, peterz@infradead.org,
        srinivas.pandruvada@linux.intel.com, rientjes@google.com,
        dovmurik@linux.ibm.com, tobin@ibm.com, bp@alien8.de,
        michael.roth@amd.com, vbabka@suse.cz, kirill@shutemov.name,
        ak@linux.intel.com, tony.luck@intel.com, marcorr@google.com,
        sathyanarayanan.kuppuswamy@linux.intel.com, alpergun@google.com,
        dgilbert@redhat.com, jarkko@kernel.org
References: <cover.1655761627.git.ashish.kalra@amd.com>
 <5d05799fc61994684aa2b2ddb8c5b326a3279e25.1655761627.git.ashish.kalra@amd.com>
From:   Tom Lendacky <thomas.lendacky@amd.com>
In-Reply-To: <5d05799fc61994684aa2b2ddb8c5b326a3279e25.1655761627.git.ashish.kalra@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cXFlNVVhaDJJb09mck83WEJpSjdxeU4ySW90Sk81SGJ4ZWRLZG9wUU9SNHpu?=
 =?utf-8?B?ak8vQnEzelgyaE5hb005Y1A5aHU2U1huVEl2SkhMRkZYbmRyYkN5a2ZGMzNX?=
 =?utf-8?B?VmJ6RUN1dEZYdWxBMk5MLzQrcTNpZXNIUUpTNFlTUDVuMElpUW9Dbm9HQms2?=
 =?utf-8?B?YlE0cWowQ1JIeUh1VEl5dVdNKzlKTkorYWdvVDVHWEUvcG5FRU9jQWZQN20x?=
 =?utf-8?B?eHFCTDA4bHRTUmJZcEF2QUlXZXRSc2IvdTluUkN6eUU0QVlSdFpBYjVick1H?=
 =?utf-8?B?UlRheWlxSnZLb3Z2WWRHMC9xTERWSkV1NEoyaHBhQTlRbEVaKzFaOHBibXE5?=
 =?utf-8?B?dGt5eFRSNjEyWXhQTi9iSEhiNXE0TDlxOFU4WGcwZEszbVNhQ1I0cEtkNzhV?=
 =?utf-8?B?V1gyOXUrekFiQVpmSlhuMU5wbDI1WXpWZXEzVFcybUllQ0JpUXVnVXI4TEJt?=
 =?utf-8?B?NExrMzcydVlJUVlYVW1CL3JoQTBsZkxyU1hWb3Y4OUNnbS93ak9hK0Q5cVlM?=
 =?utf-8?B?RFRjQSsra2RHQWpSMTRTL0ZGMGlBWjBsTW93Ny84RUhEampXbzk1Tml5T0J2?=
 =?utf-8?B?VGZydjJGd1ZNZmhKSjZyY1VzbXkwenhKd0NSZmpsVUhzdzBiQ2ZrRm9lT21T?=
 =?utf-8?B?VFc4anBNTHdWRFJnWE9ydnJWKytERFYvSEpuNHoydy9PRDlqaVRUSHRvQjlx?=
 =?utf-8?B?cHFSdDNkWVNnMnA1cVZGT29KQjR2RWQyTTlCSzIvUXM3WU9Jd1ZORTUxbFZU?=
 =?utf-8?B?S3ovUTllbjNNN2tnNGwyY1B1VnRlcWZzMmU4bFF5NkdPK01FUGZZUmNZMzNs?=
 =?utf-8?B?V1A5cjd2QnJ6NGF4NVB4Mjk1T0w4SFFEYXFHakFGcUtIemMxMzFaMVRYaHhm?=
 =?utf-8?B?Q1pyWHErbmhjNVY2TVFIYTl2ZE1GOVFsTmFJVTdkVWtVdFU0d05TaEtUTWtZ?=
 =?utf-8?B?dHAwRGpabkhSVlJkQlJhbHkraUJLN0xyczRFaGFyNlBiSTBERUJjNXpXMjBU?=
 =?utf-8?B?azh1MVhEWnRkRVpLRnBTSHhxZlpaeDZvVGxHamg0UVlOaG43VVBSdVlnV3ht?=
 =?utf-8?B?b2FZWkdIQXNuTk9CZzQrNmxNempLRjllY3FZSzVCeHNZY2ZnSDBNUlNmS0ht?=
 =?utf-8?B?eGs1QWgxVytEZkd5L3pKUlI4QzEyc1I1d3NyZmpGb3hTRHMvNGl1VDhQVHhr?=
 =?utf-8?B?Y2p3N0xGM3pxU2FpRDlkcTEvUFpldmpJdDBrWE5SeHhBaGxEaVI3cURlRG13?=
 =?utf-8?B?MCtoM1hHbVF6VXo1UVR4bGV1QVpPaWRGc2gxUk15RWk4UkdqZDU4ZWFGSXJm?=
 =?utf-8?B?WnQ1TDRSNUdaL3RQVHdnMlRYRm1vc1NtUjlyNDhneFNXZnBYUFk3a1djSzVR?=
 =?utf-8?B?eE15SXlUYjJaSEhDT1U0MElwdjl3eHcrZmxEM2VkOUlnMkk3eEQ5N1Rpb3VE?=
 =?utf-8?B?WVRkL2IrZmdIYjI3NW1SS25Mam9mT2RpZmxReHRTdjVkQzRZZ3I5WitVaHgr?=
 =?utf-8?B?YXpXSEdjQ0lwNmlpcWF2ZnR5Ni9XYXpOQ3hlbmhsN3U1dXluN2M2bFcvZ3Y5?=
 =?utf-8?B?a0JXUWFPd2FpcXBUQUlGNCtGeGUxS2xJN2ZLNzkxVUVDdWJrb2hlejljckZS?=
 =?utf-8?B?cFlWdGpuNTdidGE3YmVOY0xhVnUyVHFzSi9OZWRHTHNRbjVCSlhMTWNSazFC?=
 =?utf-8?B?YXFvUDB5V1p1WENVa0FqZEpTVk45NUp5dGIyUEVlWFQxWEpJanBiWjg1S1Zu?=
 =?utf-8?B?V0FLTzNCWVlIL0FQZDVDWEdCNVFLNGJZaWhuRXZHOXNOYTFxa3V2cXdLT25I?=
 =?utf-8?B?b1Z3MFFBNXJLdmg2NEZLR29Wdm01UEppTkFSaHZNemprMFF6MGxja3RwR1VW?=
 =?utf-8?B?RnVoZEVDNVk1dFFFblBncXRobmNVbzBsWUZVcjIwZUJMK3V1ckcrMDdMdmlk?=
 =?utf-8?B?dUIwZytqZ0ZESjgySWRUWXpyRndwM1BVZVFKSCtTYzBxSXJYTmhIUTJ0NG9B?=
 =?utf-8?B?ZTFEa3FHR1dlZHAxYUMzY2EzWkVoMXlBOFFvV0hpeksyWllubVdHSmQ5QjBx?=
 =?utf-8?B?eUlOTStlU3VjRkw0RHgrckpVSEVXR2RBc0pHNC9GbFJua24rTG5FekFXU2lo?=
 =?utf-8?Q?xNvr5V7ome3phCVKRA0mIGrE9?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5036518a-0e3c-495d-3a93-08dab39762c2
X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5229.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Oct 2022 19:06:40.7175
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: /UcRZi6mKs7BT5pefgngkPcKibCsKgwBe8ObGdOPfz9p8e5zU5R28AXtsFhnaZS00PZ0tBKbONJmsr0+SMvI3w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4565
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,URIBL_BLOCKED autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

On 6/20/22 18:13, Ashish Kalra wrote:
> From: Brijesh Singh <brijesh.singh@amd.com>
> 
> Version 2 of GHCB specification added the support for two SNP Guest
> Request Message NAE events. The events allows for an SEV-SNP guest to
> make request to the SEV-SNP firmware through hypervisor using the
> SNP_GUEST_REQUEST API define in the SEV-SNP firmware specification.
> 
> The SNP_EXT_GUEST_REQUEST is similar to SNP_GUEST_REQUEST with the
> difference of an additional certificate blob that can be passed through
> the SNP_SET_CONFIG ioctl defined in the CCP driver. The CCP driver
> provides snp_guest_ext_guest_request() that is used by the KVM to get
> both the report and certificate data at once.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>   arch/x86/kvm/svm/sev.c | 196 +++++++++++++++++++++++++++++++++++++++--
>   arch/x86/kvm/svm/svm.h |   2 +
>   2 files changed, 192 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 7fc0fad87054..089af21a4efe 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c

> +static void snp_handle_ext_guest_request(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_gpa)
> +{
> +	struct sev_data_snp_guest_request req = {0};
> +	struct kvm_vcpu *vcpu = &svm->vcpu;
> +	struct kvm *kvm = vcpu->kvm;
> +	unsigned long data_npages;
> +	struct kvm_sev_info *sev;
> +	unsigned long rc, err;
> +	u64 data_gpa;
> +
> +	if (!sev_snp_guest(vcpu->kvm)) {
> +		rc = SEV_RET_INVALID_GUEST;
> +		goto e_fail;
> +	}
> +
> +	sev = &to_kvm_svm(kvm)->sev_info;
> +
> +	data_gpa = vcpu->arch.regs[VCPU_REGS_RAX];
> +	data_npages = vcpu->arch.regs[VCPU_REGS_RBX];
> +
> +	if (!IS_ALIGNED(data_gpa, PAGE_SIZE)) {
> +		rc = SEV_RET_INVALID_ADDRESS;
> +		goto e_fail;
> +	}
> +
> +	/* Verify that requested blob will fit in certificate buffer */
> +	if ((data_npages << PAGE_SHIFT) > SEV_FW_BLOB_MAX_SIZE) {

Not sure this is a valid check...  Isn't it OK if the guest has supplied 
more room than is required? If the guest supplies 8 pages and the 
hypervisor only needs to copy 1 page of data (or the SEV_FW_BLOB_MAX_SIZE 
number of pages) that shouldn't be an error. I think this check can go, right?

Thanks,
Tom

> +		rc = SEV_RET_INVALID_PARAM;
> +		goto e_fail;
> +	}
> +
> +	mutex_lock(&sev->guest_req_lock);
> +
> +	rc = snp_setup_guest_buf(svm, &req, req_gpa, resp_gpa);
> +	if (rc)
> +		goto unlock;
> +
> +	rc = snp_guest_ext_guest_request(&req, (unsigned long)sev->snp_certs_data,
> +					 &data_npages, &err);
> +	if (rc) {
> +		/*
> +		 * If buffer length is small then return the expected
> +		 * length in rbx.
> +		 */
> +		if (err == SNP_GUEST_REQ_INVALID_LEN)
> +			vcpu->arch.regs[VCPU_REGS_RBX] = data_npages;
> +
> +		/* pass the firmware error code */
> +		rc = err;
> +		goto cleanup;
> +	}
> +
> +	/* Copy the certificate blob in the guest memory */
> +	if (data_npages &&
> +	    kvm_write_guest(kvm, data_gpa, sev->snp_certs_data, data_npages << PAGE_SHIFT))
> +		rc = SEV_RET_INVALID_ADDRESS;
> +
> +cleanup:
> +	snp_cleanup_guest_buf(&req, &rc);
> +
> +unlock:
> +	mutex_unlock(&sev->guest_req_lock);
> +
> +e_fail:
> +	svm_set_ghcb_sw_exit_info_2(vcpu, rc);
> +}
> +