From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 35D4FC54EBC for ; Tue, 10 Jan 2023 15:23:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BB31A900002; Tue, 10 Jan 2023 10:23:18 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B8A498E0001; Tue, 10 Jan 2023 10:23:18 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A2C39900002; Tue, 10 Jan 2023 10:23:18 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 92EB88E0001 for ; Tue, 10 Jan 2023 10:23:18 -0500 (EST) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 690D6C0D29 for ; Tue, 10 Jan 2023 15:23:18 +0000 (UTC) X-FDA: 80339258076.25.B141EFD Received: from mail-lf1-f41.google.com (mail-lf1-f41.google.com [209.85.167.41]) by imf27.hostedemail.com (Postfix) with ESMTP id AE5FE40012 for ; Tue, 10 Jan 2023 15:23:15 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=ksjpe3eP; spf=pass (imf27.hostedemail.com: domain of pgonda@google.com designates 209.85.167.41 as permitted sender) smtp.mailfrom=pgonda@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1673364195; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Xei/D1xFFLE5WpxaBE/Tg4Sb4JRRguY1a3omXhNRdNQ=; b=DftQB3kbCURUNwyqt2MKn8hBJ0kT9sO34O1EgmDZfvoOAug7eJZhx2NtmTclWtZotPJ8YP 6NYj3haLjJ8jyoC0KCVSUsfBUEcA9cI0EA11RVBavH9+RermsnX9zPJb+7kw2pgaB7Srcz 6acgKgKbydXcbMF9vAPFZeOflV6xATc= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=ksjpe3eP; spf=pass (imf27.hostedemail.com: domain of pgonda@google.com designates 209.85.167.41 as permitted sender) smtp.mailfrom=pgonda@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1673364195; a=rsa-sha256; cv=none; b=djbJbV/+QevNIjzkJajV9Kvx+Q+rIzpZWM13SkD9cCkpXsCrEoGc57f2HQTvO2dq6+XR5u cTTmdm+j7kp+JIarYY4KOsACX2T+EPnlSa3yAFo2ufZ2j/SIdYqixkoYts2O+ZV8KSUnMZ EhFzOBeyIZ5fLcs+XBcJjJfgBJNmUXs= Received: by mail-lf1-f41.google.com with SMTP id bq39so19030486lfb.0 for ; Tue, 10 Jan 2023 07:23:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=Xei/D1xFFLE5WpxaBE/Tg4Sb4JRRguY1a3omXhNRdNQ=; b=ksjpe3ePskHcWnV5GTQFpREDCBhu/f/GUrq/ztAX0yjA5OuQ+DutOkLZMxTHo/I+oT gSOsKR2CYx0y6FaPPb135ztWWumo2ihZo6Ahtz3tgU5mTzh1iUVqE/qC696Px6TF03zB HJ2wfcTzzjh0RCQoPte6n7r2FXr0+ml9tuu2hdvhnf1kixXGHUUotiiqqOagwsRCu4tH IfshrT0GjPV1fvD03OZ9Iy/w/vDEEyqakIvbgzh/c02+8JdjVRht0JGh1ozXSnsYeKhF ci6m6qdmPoXCnxYdZT2PubbzQgh5LkWDPkj0xHHait+gqehjaPIYZrb0xA5uQaMFqCZD GQXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Xei/D1xFFLE5WpxaBE/Tg4Sb4JRRguY1a3omXhNRdNQ=; b=w2q+nLGQKh8S1W60f95U/+RHGAUvAr5zkQ2J290cBhbquydCWiODQDeemk2OlYkjXo ZrRTVbHV67PgSs+uQC/+QzYvILqBkBueWIcM8Ak0tAw6Q6rRi14Y23wdX9mVw4TfqUTN qVnlqguOfBW6wiUgXUJSI2SCz6jYvswT4O2wzbhkTzQZkv+KXkqqfNA4TFPogiMIN98S DXNFFbuKOty6PDp/GvmLqUE4ZUf4Core1Ujnq23kwdyA75CC+SeD2b83czQYOKkh5QZr pDOqVEBCZzYpGug3kCrIrTDs4dHVtTLBIboSA+7KTaD3n5GZCHbm2XlZf576vqaVdKZw /U7A== X-Gm-Message-State: AFqh2kr2xp+7kiDpr9iSY7+Ad2Rruy5w27enZ3uWngQ/X3nI/WiiEE2A aCZ3Mo3UNoguvl2/ty8TpLCX5ELXXx6F8KKNlRIU3A== X-Google-Smtp-Source: AMrXdXutwwZyshi7xoMMAtLcru6vJQ0zecDlsCbhc0a61fPAh5s83V9KF6oInnQ7VfNNGFDxOOjHvS4HTidsTLJf/lM= X-Received: by 2002:ac2:5e7c:0:b0:4c3:d803:4427 with SMTP id a28-20020ac25e7c000000b004c3d8034427mr2883098lfr.170.1673364193464; Tue, 10 Jan 2023 07:23:13 -0800 (PST) MIME-Version: 1.0 References: <20221214194056.161492-1-michael.roth@amd.com> <20221214194056.161492-63-michael.roth@amd.com> <1c02cc0d-9f0c-cf4a-b012-9932f551dd83@linux.ibm.com> <54ff7326-e3a4-945f-1f60-e73dd8865527@amd.com> <1047996c-309b-6839-fdd7-265fc51eb07a@amd.com> In-Reply-To: <1047996c-309b-6839-fdd7-265fc51eb07a@amd.com> From: Peter Gonda Date: Tue, 10 Jan 2023 08:23:01 -0700 Message-ID: Subject: Re: [PATCH RFC v7 62/64] x86/sev: Add KVM commands for instance certs To: Tom Lendacky Cc: Dov Murik , Dionna Amalie Glaze , Michael Roth , kvm@vger.kernel.org, linux-coco@lists.linux.dev, linux-mm@kvack.org, linux-crypto@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, tglx@linutronix.de, mingo@redhat.com, jroedel@suse.de, hpa@zytor.com, ardb@kernel.org, pbonzini@redhat.com, seanjc@google.com, vkuznets@redhat.com, wanpengli@tencent.com, jmattson@google.com, luto@kernel.org, dave.hansen@linux.intel.com, slp@redhat.com, peterz@infradead.org, srinivas.pandruvada@linux.intel.com, rientjes@google.com, tobin@ibm.com, bp@alien8.de, 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, ashish.kalra@amd.com, harald@profian.com Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: fujt7ypgcbp6k53go31yyngc3tsen46o X-Rspam-User: X-Rspamd-Queue-Id: AE5FE40012 X-Rspamd-Server: rspam06 X-HE-Tag: 1673364195-264644 X-HE-Meta: U2FsdGVkX1/sGE6b5X/XgYuP7ZVp3eM5B/zZVFhcn9PsfkZuOaj6+MxJqbtguRJZb5XK71gONr1E8JaQtTaM+a1OmGcRwAsSjLHBLbeaGelTTPV/itVkciP+gZpWigcuSnQFhsWdrKJoyKogqrmp6Hb6/tgm8Cy1jfHTKO3vu7al+aJrcvC1Z/zVL/Itu5v2Zsd44dzvUMJzg5qmXHFPDQeXW6zdpsFgJbBWSRN8I1siyNJkgNOhSVxkIVnbqoHCezv5tQaD7J7rDWdzyX/a2355c/TyLpgrLHDRoC5IAqJMySZymZVJFVAAiLS9Ojrr82+entw34/0PkcXyhMMheS774FlI5p1jRzKc6v4RQyQhQRl6zPMdbPrTHL9YjMZ/xxtbvbqknBQ+BtnF2dlB2xCrMHG+Ei+NrkHuvnu1lhXMXxGsQXYAHKltsP3uH6SCnxVd9NjAvmVHRzimvV9qzLilq2Or3p2KR/RoW3aXKGjzHxkMbN/LwhZeOiq+omfQrIaEg4nV4d4ZVgkxuVCGIIDrX0PClWrDlPSR60fqPvuKkmdMb60O4xIb1M0Ef9qo1nzyvd/nXNq9IIaXJwV+MIrQ1hHICbR9pQ84LacxoHa+ssyT1N0+RlrAlEKm13W304xz2ID434e28MTSeKjHyVbubvI3+raiomiYILBEU6+xOMVYhAJ6o38am2kJFS40fZ/a8q4mN+2zFdbVyMvmS8EZFcusu1niJ/fUK0aDTH6vTsY/ZG0NPlcl6VETJ/jE9pL3l/KNBAMdbPycZEwalLwG0ySYxdI/fivZvUGcr6+5ug6i5TTuwWt1lv188HyrmB4clj2QcPBAHsj1IDcGv06F3xBavn6pobeEqGVHkVglLpxu0zWtBJbvxzD+KzzxOM/H5rHrEy5FJ9nQwoAvKG14rJCJxcwOYwJr/pOtzUkKqLgLne2Qp8XOJXy8JNVBmXwp6t97my1Fdhm1hbZ Q7Ui2GPb jDkefwpuff7r8AgY= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Jan 10, 2023 at 8:10 AM Tom Lendacky wrote: > > On 1/10/23 01:10, Dov Murik wrote: > > Hi Tom, > > > > On 10/01/2023 0:27, Tom Lendacky wrote: > >> On 1/9/23 10:55, Dionna Amalie Glaze wrote: > >>>>> + > >>>>> +static int snp_set_instance_certs(struct kvm *kvm, struct > >>>>> kvm_sev_cmd *argp) > >>>>> +{ > >>>> [...] > >>>> > >>>> Here we set the length to the page-aligned value, but we copy only > >>>> params.cert_len bytes. If there are two subsequent > >>>> snp_set_instance_certs() calls where the second one has a shorter > >>>> length, we might "keep" some leftover bytes from the first call. > >>>> > >>>> Consider: > >>>> 1. snp_set_instance_certs(certs_addr point to "AAA...", certs_len=8192) > >>>> 2. snp_set_instance_certs(certs_addr point to "BBB...", certs_len=4097) > >>>> > >>>> If I understand correctly, on the second call we'll copy 4097 "BBB..." > >>>> bytes into the to_certs buffer, but length will be (4096 + PAGE_SIZE - > >>>> 1) & PAGE_MASK which will be 8192. > >>>> > >>>> Later when fetching the certs (for the extended report or in > >>>> snp_get_instance_certs()) the user will get a buffer of 8192 bytes > >>>> filled with 4097 BBBs and 4095 leftover AAAs. > >>>> > >>>> Maybe zero sev->snp_certs_data entirely before writing to it? > >>>> > >>> > >>> Yes, I agree it should be zeroed, at least if the previous length is > >>> greater than the new length. Good catch. > >>> > >>> > >>>> Related question (not only for this patch) regarding snp_certs_data > >>>> (host or per-instance): why is its size page-aligned at all? why is it > >>>> limited by 16KB or 20KB? If I understand correctly, for SNP, this buffer > >>>> is never sent to the PSP. > >>>> > >>> > >>> The buffer is meant to be copied into the guest driver following the > >>> GHCB extended guest request protocol. The data to copy back are > >>> expected to be in 4K page granularity. > >> > >> I don't think the data has to be in 4K page granularity. Why do you > >> think it does? > >> > > > > I looked at AMD publication 56421 SEV-ES Guest-Hypervisor Communication > > Block Standardization (July 2022), page 37. The table says: > > > > -------------- > > > > NAE Event: SNP Extended Guest Request > > > > Notes: > > > > RAX will have the guest physical address of the page(s) to hold returned > > data > > > > RBX > > State to Hypervisor: will contain the number of guest contiguous > > pages supplied to hold returned data > > State from Hypervisor: on error will contain the number of guest > > contiguous pages required to hold the data to be returned > > > > ... > > > > The request page, response page and data page(s) must be assigned to the > > hypervisor (shared). > > > > -------------- > > > > > > According to this spec, it looks like the sizes are communicated as > > number of pages in RBX. So the data should start at a 4KB alignment > > (this is verified in snp_handle_ext_guest_request()) and its length > > should be 4KB-aligned, as Dionna noted. > > That only indicates how many pages are required to hold the data, but the > hypervisor only has to copy however much data is present. If the data is > 20 bytes, then you only have to copy 20 bytes. If the user supplied 0 for > the number of pages, then the code returns 1 in RBX to indicate that one > page is required to hold the 20 bytes. > > > > > I see no reason (in the spec and in the kernel code) for the data length > > to be limited to 16KB (SEV_FW_BLOB_MAX_SIZE) but I might be missing some > > flow because Dionna ran into this limit. > > Correct, there is no limit. I believe that SEV_FW_BLOB_MAX_SIZE is a way > to keep the memory usage controlled because data is coming from userspace > and it isn't expected that the data would be larger than that. > > I'm not sure if that was in from the start or as a result of a review > comment. Not sure what is the best approach is. This was discussed a bit in the guest driver changes recently too that SEV_FW_BLOB_MAX_SIZE is used in the guest driver code for the max cert length. We discussed increasing the limit there after fixing the IV reuse issue. Maybe we could introduce SEV_CERT_BLOB_MAX_SIZE here to be more clear there is no firmware based limit? Then we could switch the guest driver to use that too. Dionna confirmed 4 pages is enough for our current usecase, Dov would you recommend something larger to start? > > Thanks, > Tom > > > > > > > -Dov > > > > > > > >> Thanks, > >> Tom > >> > >>> > >>>> [...] > >>>>> > >>>>> -#define SEV_FW_BLOB_MAX_SIZE 0x4000 /* 16KB */ > >>>>> +#define SEV_FW_BLOB_MAX_SIZE 0x5000 /* 20KB */ > >>>>> > >>>> > >>>> This has effects in drivers/crypto/ccp/sev-dev.c > >>>> (for > >>>> example in alloc_snp_host_map). Is that OK? > >>>> > >>> > >>> No, this was a mistake of mine because I was using a bloated data > >>> encoding that needed 5 pages for the GUID table plus 4 small > >>> certificates. I've since fixed that in our user space code. > >>> We shouldn't change this size and instead wait for a better size > >>> negotiation protocol between the guest and host to avoid this awkward > >>> hard-coding. > >>> > >>>