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 X-Spam-Level: X-Spam-Status: No, score=-13.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E45E4C636C9 for ; Wed, 21 Jul 2021 17:53:35 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 89C256121E for ; Wed, 21 Jul 2021 17:53:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 89C256121E Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 296266B005D; Wed, 21 Jul 2021 13:53:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 245D16B006C; Wed, 21 Jul 2021 13:53:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 10D926B0070; Wed, 21 Jul 2021 13:53:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0154.hostedemail.com [216.40.44.154]) by kanga.kvack.org (Postfix) with ESMTP id E63CB6B005D for ; Wed, 21 Jul 2021 13:53:34 -0400 (EDT) Received: from smtpin33.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 972DB181EFF5B for ; Wed, 21 Jul 2021 17:53:34 +0000 (UTC) X-FDA: 78387342348.33.5DBBC43 Received: from mail-qk1-f175.google.com (mail-qk1-f175.google.com [209.85.222.175]) by imf15.hostedemail.com (Postfix) with ESMTP id 5C985D006257 for ; Wed, 21 Jul 2021 17:53:34 +0000 (UTC) Received: by mail-qk1-f175.google.com with SMTP id bm6so2898468qkb.1 for ; Wed, 21 Jul 2021 10:53:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=6yFvLCK83EARCEJW2USL66XPk2Snwg9rOn4V8eQnKq4=; b=LmYH4vBpP8tsrP/2QQX/a4c4LvuM1+b4/24JikG+Eg3ffRRWq1sLdvVWqmdZSpffFs zYMHn1k1Y7JSxueTKglktRFPafjcur+BGgT8Ux3LXzZB1CNjBqPF55WMoTS3UHR/naUy 5HN4fHlgHkJlsYXgo1YafStP4kmIs1l/2txFBajcHQd5DgvTLQTHypzSg1u2dEsyWtNa 1Gnt5R/qqXAZ4zGACvt2zRiwgMwtDzYJEII9SZ5Ev6ztWv9+tg0z29/yQFcibOFahdej SPDfQhV5wiSgG/WdTlqfqtvyXLh0cVDvfnKvx7ZZJF7FSQsGy43aezRKof8MJZkcTPGM o0VQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=6yFvLCK83EARCEJW2USL66XPk2Snwg9rOn4V8eQnKq4=; b=qQim18haXYDgQuK1i5ZwsD/JAiiflD6hdAFf3MX2jz7sT7SEic2bUxyTaU04fOJ4Mh Mr9meix33l4X9qjZ5OAFFfA/RoA8YC8UgHzq9WX95QDKA8frfaSo8u/K+F0mPymWVjiW 1VH9T8w34CwhOs1x9K055XuVamLuoID+yJuMSPY5rknFC+oRdEkL6LzxDG4ZgYyU8qSc goGySNnkMxrh1BOkYhhusLQlrRLudYknIzuxGQAYCZgC0O6smWYIPPoNMSHubl/3Rn9D /Vv2X6XAr7f7QUXmIBNPnTmtZbM4GKbdgGenpwvj4fdvX/Bl8upKLYMymVuuk7iFcFYh E9Wg== X-Gm-Message-State: AOAM530lz5y9apBBQ+Y41XZfgI/6gn0yxy/CrotXvBDo/eW5oNFNCWCh n6m2ea0gBrPRai6ma361i2X5gN8KdjC9sF/W7kOu4g== X-Google-Smtp-Source: ABdhPJzkyyOAFQakNLKPpJl5joOFjae/AcNTO5l2I8TPcjJJXSsAIKXc+xGIz33Gsbm9HQbKLCjKhxqfB9nnHVogi3Y= X-Received: by 2002:a37:8044:: with SMTP id b65mr22312539qkd.150.1626890013396; Wed, 21 Jul 2021 10:53:33 -0700 (PDT) MIME-Version: 1.0 References: <20210707183616.5620-1-brijesh.singh@amd.com> <20210707183616.5620-27-brijesh.singh@amd.com> <9ee5a991-3e43-3489-5ee1-ff8c66cfabc1@amd.com> In-Reply-To: From: Marc Orr Date: Wed, 21 Jul 2021 10:53:22 -0700 Message-ID: Subject: Re: [PATCH Part2 RFC v4 26/40] KVM: SVM: Add KVM_SEV_SNP_LAUNCH_FINISH command To: Sean Christopherson Cc: Brijesh Singh , x86@kernel.org, linux-kernel@vger.kernel.org, kvm list , linux-efi@vger.kernel.org, platform-driver-x86@vger.kernel.org, linux-coco@lists.linux.dev, linux-mm@kvack.org, linux-crypto@vger.kernel.org, Thomas Gleixner , Ingo Molnar , Joerg Roedel , Tom Lendacky , "H. Peter Anvin" , Ard Biesheuvel , Paolo Bonzini , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Andy Lutomirski , Dave Hansen , Sergio Lopez , Peter Gonda , Peter Zijlstra , Srinivas Pandruvada , David Rientjes , Dov Murik , Tobin Feldman-Fitzthum , Borislav Petkov , Michael Roth , Vlastimil Babka , tony.luck@intel.com, npmccallum@redhat.com, brijesh.ksingh@gmail.com Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=LmYH4vBp; spf=pass (imf15.hostedemail.com: domain of marcorr@google.com designates 209.85.222.175 as permitted sender) smtp.mailfrom=marcorr@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 5C985D006257 X-Stat-Signature: gk43hxc9st16izz4tx5f9answy36n3t9 X-HE-Tag: 1626890014-235311 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 Mon, Jul 19, 2021 at 9:54 AM Sean Christopherson wrote: > > On Fri, Jul 16, 2021, Brijesh Singh wrote: > > > > On 7/16/21 3:18 PM, Sean Christopherson wrote: > > > On Wed, Jul 07, 2021, Brijesh Singh wrote: > > >> + data->gctx_paddr = __psp_pa(sev->snp_context); > > >> + ret = sev_issue_cmd(kvm, SEV_CMD_SNP_LAUNCH_FINISH, data, &argp->error); > > > Shouldn't KVM unwind everything it did if LAUNCH_FINISH fails? And if that's > > > not possible, take steps to make the VM unusable? > > > > Well, I am not sure if VM need to unwind. If the command fail but VMM decide > > to ignore the error then VMRUN will probably fail and user will get the KVM > > shutdown event. The LAUNCH_FINISH command finalizes the VM launch process, > > the firmware will probably not load the memory encryption keys until it moves > > to the running state. > > Within reason, KVM needs to provide consistent, deterministic behavior. Yes, more > than likely failure at this point will be fatal to the VM, but that doesn't justify > leaving the VM in a random/bogus state. In addition to being a poor ABI, it also > makes it more difficult to reason about what is/isn't possible in KVM. +1 to Sean's feedback to unwind everything here properly here. Comments of the nature of "XYZ should happen" -- without a test (e.g., selftest or kvm-unit-test) to ensure the XYZ _does_ happen -- are a time bomb waiting to happen. Also, I wonder if we leave pages, RMPUPDATE'd to immutable in previous loop iterations, is it possible for them to remain as immutable and be reused later on (after this guest is destroyed)? And if this happens, will we get an RMP violation? Even if the answer is no -- go read this code 2,000 lines away -- it handles this case. That's still not a very satisfying answer. I'd rather see things cleaned up ASAP as soon as the code starts to go off the rails.