From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lf1-f43.google.com (mail-lf1-f43.google.com [209.85.167.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 37FD828E2 for ; Fri, 8 Jul 2022 15:54:38 +0000 (UTC) Received: by mail-lf1-f43.google.com with SMTP id i18so37007897lfu.8 for ; Fri, 08 Jul 2022 08:54:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=/os5t3S4cPsz3I/S9Mj4qyigjvCHgQQtkEVeDgJ05RM=; b=PsBVmJ6R6GowUlEMeEe8ZRY8jb52v2WfXzscfT5jLoCOQsbZ36cbawSyHAtGiHnafj HaFtIhvPwrjTsy3gBfiNpXk6CLJYS++w+8CYIEoISvqmxuuVetqxorhjsydgFa23mZKQ 5/cnrNL72sWZGEGfjILngANqq9MIg3cWbdiTWVR/h5IQrycCXuvQ4YGYcrr2ICC+JjoL DyDSxQSfjivojcSr1c9UT0BUbasRhFYlNTGM0VY5acbNpT07M1F5BIlW4gQlVvfNsmya l77+tZgU/wKnuxcPYeZ/vqIStbc7H1V+E0CACaXVs84rluFvh1bSyYJPimKqQ++dB94y p0rw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=/os5t3S4cPsz3I/S9Mj4qyigjvCHgQQtkEVeDgJ05RM=; b=ZccTtpQP8DCpeRFj/GxaJTJ9+onvuSITj06ZBk4mXeNu3TsHA6rUoA291JAFNp1ozD NXInzn4kYSnpAQkvMjMxy66pmAfL1zae9H3zuh+wDTvocYYITd7NxmX0pqNcI8NKnG5Y C410Vyn1nGbzu6t9httF5CBeKz/CqombP+9liw88XFKvRwJ8fh9axaFfOl5z/ntJ1mbd TXsAzzxfhGCSQxmHicDXEJj20kLHFjyX6ItBYcEn1LyYXzQgveTUdWsDioxhMTvx4zcQ WlI1QXHeb4uNTIC4SlJm0wUUWuq1dADOIvwhOkI8Ho+TLrjRnAm2QlZ21XRFgW15yhAz vhag== X-Gm-Message-State: AJIora/EC8fmMxGI9qOdkdozHl79MfSGXLMIxJvmGd7OZJ4bT2WQcMfm McxjU7mHzE7AmgwVX3xarmRSSzQrscx9vp3Nl5AryQ== X-Google-Smtp-Source: AGRyM1vakIVeDaj9C17a1elb+iyUeaYTA/kg2lyv8/76nZi9IF95NVCr4t6HVeEbFti0eBxcXliDfOCnMI7CCXuWRVI= X-Received: by 2002:a05:6512:a8c:b0:484:73f8:704d with SMTP id m12-20020a0565120a8c00b0048473f8704dmr3076083lfu.193.1657295675981; Fri, 08 Jul 2022 08:54:35 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-coco@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <7845d453af6344d0b156493eb4555399aad78615.1655761627.git.ashish.kalra@amd.com> In-Reply-To: From: Peter Gonda Date: Fri, 8 Jul 2022 09:54:24 -0600 Message-ID: Subject: Re: [PATCH Part2 v6 35/49] KVM: SVM: Remove the long-lived GHCB host map To: "Kalra, Ashish" Cc: "the arch/x86 maintainers" , LKML , kvm list , "linux-coco@lists.linux.dev" , "linux-mm@kvack.org" , Linux Crypto Mailing List , Thomas Gleixner , Ingo Molnar , Joerg Roedel , "Lendacky, Thomas" , "H. Peter Anvin" , Ard Biesheuvel , Paolo Bonzini , Sean Christopherson , Vitaly Kuznetsov , Jim Mattson , Andy Lutomirski , Dave Hansen , Sergio Lopez , Peter Zijlstra , Srinivas Pandruvada , David Rientjes , Dov Murik , Tobin Feldman-Fitzthum , Borislav Petkov , "Roth, Michael" , Vlastimil Babka , "Kirill A . Shutemov" , Andi Kleen , Tony Luck , Marc Orr , Sathyanarayanan Kuppuswamy , Alper Gun , "Dr. David Alan Gilbert" , "jarkko@kernel.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Jul 7, 2022 at 2:31 PM Kalra, Ashish wrote: > > [AMD Official Use Only - General] > > Hello Peter, > > >> >There is a perf cost to this suggestion but it might make accessing > >> >the GHCB safer for KVM. Have you thought about just using > >> >kvm_read_guest() or copy_from_user() to fully copy out the GCHB into = a KVM owned buffer, then copying it back before the VMRUN. That way the KVM= doesn't need to guard against page_state_changes on the GHCBs, that could = be a perf ?>>improvement in a follow up. > >> > >> Along with the performance costs you mentioned, the main concern here > >> will be the GHCB write-back path (copying it back) before VMRUN: this > >> will again hit the issue we have currently with > >> kvm_write_guest() / copy_to_user(), when we use it to sync the scratch > >> buffer back to GHCB. This can fail if guest RAM is mapped using huge-p= age(s) and RMP is 4K. Please refer to the patch/fix mentioned below, kvm_wr= ite_guest() potentially can fail before VMRUN in case of SNP : > >> > >> commit 94ed878c2669532ebae8eb9b4503f19aa33cd7aa > >> Author: Ashish Kalra > >> Date: Mon Jun 6 22:28:01 2022 +0000 > >> > >> KVM: SVM: Sync the GHCB scratch buffer using already mapped ghcb > >> > >> Using kvm_write_guest() to sync the GHCB scratch buffer can fail > >> due to host mapping being 2M, but RMP being 4K. The page fault han= dling > >> in do_user_addr_fault() fails to split the 2M page to handle RMP f= ault due > >> to it being called here in a non-preemptible context. Instead use > >> the already kernel mapped ghcb to sync the scratch buffer when the > >> scratch buffer is contained within the GHCB. > > >Ah I didn't see that issue thanks for the pointer. > > >The patch description says "When SEV-SNP is enabled the mapped GPA needs= to be protected against a page state change." since if the guest were to c= onvert the GHCB page to private when the host is using the GHCB the host co= uld get an RMP violation right? > > Right. > > >That RMP violation would cause the host to crash unless we use some copy= _to_user() type protections. > > As such copy_to_user() will only swallow the RMP violation and return fai= lure, so the host can retry the write. > > > I don't see anything mechanism for this patch to add the page state cha= nge protection discussed. Can't another vCPU still convert the GHCB to priv= ate? > > We do have the protections for GHCB getting mapped to private specificall= y, there are new post_{map|unmap}_gfn functions added to verify if it is sa= fe to map > GHCB pages. There is a PSC spinlock added which protects again page state= change for these mapped pages. > Below is the reference to this patch: > https://lore.kernel.org/lkml/cover.1655761627.git.ashish.kalra@amd.com/T/= #mafcaac7296eb9a92c0ea58730dbd3ca47a8e0756 > > But do note that there is protection only for GHCB pages and there is a n= eed to add generic post_{map,unmap}_gfn() ops that can be used to verify > that it's safe to map a given guest page in the hypervisor. This is a TOD= O right now and probably this is something which UPM can address more clean= ly. Thank you Ashish. I had missed that. Can you help me understand why its OK to use kvm_write_guest() for the |snp_certs_data| inside of snp_handle_ext_guest_request() in patch 42/49? I would have thought we'd have the same 2M vs 4K mapping issues. > > >I was wrong about the importance of this though seanjc@ walked me throug= h how UPM will solve this issue so no worries about this until the series i= s rebased on to UPM. > > Thanks, > Ashish