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=-11.6 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FSL_HELO_FAKE, 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 BC381C07E9B for ; Mon, 19 Jul 2021 20:52:07 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 52CA3610F7 for ; Mon, 19 Jul 2021 20:52:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 52CA3610F7 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 F24918D00F4; Mon, 19 Jul 2021 16:52:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id ED41D8D00EC; Mon, 19 Jul 2021 16:52:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D4F068D00F4; Mon, 19 Jul 2021 16:52:06 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0129.hostedemail.com [216.40.44.129]) by kanga.kvack.org (Postfix) with ESMTP id ABB028D00EC for ; Mon, 19 Jul 2021 16:52:06 -0400 (EDT) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 4634A250D7 for ; Mon, 19 Jul 2021 20:52:05 +0000 (UTC) X-FDA: 78380534610.03.B017ACC Received: from mail-pf1-f179.google.com (mail-pf1-f179.google.com [209.85.210.179]) by imf16.hostedemail.com (Postfix) with ESMTP id EEC90F000091 for ; Mon, 19 Jul 2021 20:52:04 +0000 (UTC) Received: by mail-pf1-f179.google.com with SMTP id i14so5626595pfd.5 for ; Mon, 19 Jul 2021 13:52:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=AFTM2GRWufz5H7ZF2I53fhdcy31mPSD1WmaoCuSp/kE=; b=EaCLfVRUeWm1VySqrv8i2wNaa1wkPklvEK+xtMHdsSwgX2wFqb1Td6Qxx/Gb5l+cVb iFDBBPj7D4Vc1AccEll1JyWJ1bUw6f6E5fWHeRlt33bQVWVeosWbQYPcJTxJJzXLzM9O kIducZm41jdtN6sUFDlUG707INiMICk4cgWapIwGO3HfFDCHIJXCNZP4gXRP+tRtGBAe upzRB6mNR4POD1tE5HNErufrnwLonZnXJR1npy0SArVe6sk5bxak769zdSKot2JBoe2R 8I+m0tueFJM91D9WIUbZQDxA3BDZrk+shYWYLpiKWRmq9uaqtm4emtrPiHXDahsjmVPG OV2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=AFTM2GRWufz5H7ZF2I53fhdcy31mPSD1WmaoCuSp/kE=; b=ubUsTa8lJDBDNyaZpaf+VaTVKYWbwNIKJNP1kLlF1vnlKG0nhY1JWA8P3Ve+igHuUy NKjeP2IWsyj59MoS10wSTQYW3yasEkAfCJPRkC1o1fiG5h8g81A/+NUqTUWV4ZMtE6WI kPhEUJNXGY3kL3KDLBmPEEveTzLlnYtjoQIrmpCC4RKInt3osGH1eza7B3eqXJSZN2Dg X+d7TQ5h7KoTmUlefsspuaNebIwGrmiEBMIXXdJsWJtmQvH6bodi1E8cpvwcb4e5CwLy r5kGdWanoTXxn4y8ShWV2R3msjpmAOXRGVleaCZshcRv3Q0G9RTLa7dddJn3TJKwAc4M JO6w== X-Gm-Message-State: AOAM531nmFwfkW2BQzKKcP4ygKTPRRhoOZLZXmm0iQ0tApe/dJueB3h8 tMIyYO+6syKYz03OgcGV3EAGSg== X-Google-Smtp-Source: ABdhPJyjItrVjb+a0pFhuKYnpkTfNH72AHGpeo/P7u6yE1PaSgsExPO2J1JwGyGZTKpxOFB6QaRDuw== X-Received: by 2002:a62:3045:0:b029:32b:880f:c03a with SMTP id w66-20020a6230450000b029032b880fc03amr27819158pfw.22.1626727923390; Mon, 19 Jul 2021 13:52:03 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id v10sm381814pjd.29.2021.07.19.13.52.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Jul 2021 13:52:02 -0700 (PDT) Date: Mon, 19 Jul 2021 20:51:58 +0000 From: Sean Christopherson To: Brijesh Singh Cc: x86@kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, 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 Subject: Re: [PATCH Part2 RFC v4 24/40] KVM: SVM: Add KVM_SEV_SNP_LAUNCH_UPDATE command Message-ID: References: <20210707183616.5620-1-brijesh.singh@amd.com> <20210707183616.5620-25-brijesh.singh@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=EaCLfVRU; spf=pass (imf16.hostedemail.com: domain of seanjc@google.com designates 209.85.210.179 as permitted sender) smtp.mailfrom=seanjc@google.com; dmarc=pass (policy=reject) header.from=google.com X-Stat-Signature: 6nhwmmei6is8f4bwmf6moms3osb63ftk X-Rspamd-Queue-Id: EEC90F000091 X-Rspamd-Server: rspam01 X-HE-Tag: 1626727924-22303 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 Fri, Jul 16, 2021, Brijesh Singh wrote: > > On 7/16/21 3:01 PM, Sean Christopherson wrote: > > I'm having a bit of deja vu... This flow needs to hold kvm->srcu to do a memslot > > lookup. > > > > That said, IMO having KVM do the hva->gpa is not a great ABI. The memslots are > > completely arbitrary (from a certain point of view) and have no impact on the > > validity of the memory pinning or PSP command. E.g. a memslot update while this > > code is in-flight would be all kinds of weird. > > > > In other words, make userspace provide both the hva (because it's sadly needed > > to pin memory) as well as the target gpa. That prevents KVM from having to deal > > with memslot lookups and also means that userspace can issue the command before > > configuring the memslots (though I've no idea if that's actually feasible for > > any userspace VMM). > > The operation happen during the guest creation time so I was not sure if > memslot will be updated while we are executing this command. But I guess > its possible that a VMM may run different thread which may update > memslot while another thread calls the encryption. I'll let userspace > provide both the HVA and GPA as you recommended. I'm not worried about a well-behaved userspace VMM, I'm worried about the code KVM has to carry to guard against a misbehaving VMM. > >> + ret = -EINVAL; > >> + goto e_unpin; > >> + } > >> + > >> + psize = page_level_size(level); > >> + pmask = page_level_mask(level); > > Is there any hope of this path supporting 2mb/1gb pages in the not-too-distant > > future? If not, then I vote to do away with the indirection and just hardcode > > 4kg sizes in the flow. I.e. if this works on 4kb chunks, make that obvious. > > No plans to do 1g/2mb in this path. I will make that obvious by > hardcoding it. > > > >> + gpa = gpa & pmask; > >> + > >> + /* Transition the page state to pre-guest */ > >> + memset(&e, 0, sizeof(e)); > >> + e.assigned = 1; > >> + e.gpa = gpa; > >> + e.asid = sev_get_asid(kvm); > >> + e.immutable = true; > >> + e.pagesize = X86_TO_RMP_PG_LEVEL(level); > >> + ret = rmpupdate(inpages[i], &e); > > What happens if userspace pulls a stupid and assigns the same page to multiple > > SNP guests? Does RMPUPDATE fail? Can one RMPUPDATE overwrite another? > > The RMPUPDATE is available to the hv and it can call anytime with > whatever it want. The important thing is the RMPUPDATE + PVALIDATE > combination is what locks the page. In this case, PSP firmware updates > the RMP table and also validates the page. > > If someone else attempts to issue another RMPUPDATE then Validated bit > will be cleared and page is no longer used as a private. Access to > unvalidated page will cause #VC. Hmm, and there's no indication on success that the previous entry was assigned? Adding a tracepoint in rmpupdate() to allow tracking transitions is probably a good idea, otherwise debugging RMP violations and/or unexpected #VC is going to be painful. And/or if the kernel/KVM behavior is to never reassign directly and reading an RMP entry isn't prohibitively expensive, then we could add a sanity check that the RMP is unassigned and reject rmpupdate() if the page is already assigned. Probably not worth it if the overhead is noticeable, but it could be nice to have if things go sideways. > >> +e_unpin: > >> + /* Content of memory is updated, mark pages dirty */ > >> + memset(&e, 0, sizeof(e)); > >> + for (i = 0; i < npages; i++) { > >> + set_page_dirty_lock(inpages[i]); > >> + mark_page_accessed(inpages[i]); > >> + > >> + /* > >> + * If its an error, then update RMP entry to change page ownership > >> + * to the hypervisor. > >> + */ > >> + if (ret) > >> + rmpupdate(inpages[i], &e); > > This feels wrong since it's purging _all_ RMP entries, not just those that were > > successfully modified. And maybe add a RMP "reset" helper, e.g. why is zeroing > > the RMP entry the correct behavior? > > By default all the pages are hypervior owned (i.e zero). If the > LAUNCH_UPDATE was successful then page should have transition from the > hypervisor owned to guest valid. By zero'ing it are reverting it back to > hypevisor owned. > > I agree that I optimize it to clear the modified entries only and leave > everything else as a default. To be clear, it's not just an optimization. Pages that haven't yet been touched may be already owned by a different VM (or even this VM). I.e. "reverting" those pages would actually result in a form of corruption. It's somewhat of a moot point because assigning a single page to multiple guests is going to be fatal anyways, but potentially making a bug worse by introducing even more noise/confusion is not good.