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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 91147CCA47A for ; Tue, 14 Jun 2022 20:30:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1358234AbiFNUaD (ORCPT ); Tue, 14 Jun 2022 16:30:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49966 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1358162AbiFNU35 (ORCPT ); Tue, 14 Jun 2022 16:29:57 -0400 Received: from mail-lf1-x12d.google.com (mail-lf1-x12d.google.com [IPv6:2a00:1450:4864:20::12d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8BC544EDE5 for ; Tue, 14 Jun 2022 13:29:53 -0700 (PDT) Received: by mail-lf1-x12d.google.com with SMTP id h23so15744894lfe.4 for ; Tue, 14 Jun 2022 13:29:53 -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=i1JbdaTpMJydG2La5gQ81SLd4rzBeviDTlLKc1Eh/88=; b=GeEzs3PaW1MdOrbgFZ/otLfl3Zbs0O9rwiW5jR3wEk7DxOJJMNNaAAd9x3vM+ls+vP SYoWgeoNHoBptIJJOojShXU13QiM5QUde3iyhfQnGylp3dfOJv1fd9e/kqkjVyDsO8Ak VGmpHA6oteHXGscOvQXivDkJlXHLf+JVvDS9MfT9/GEguVMCR0iw3TFO1dHFfJjQdgel dBmPayr75a2wpNqnj2UDwqzxBz7W+DedtpboLm1h6z+y3tXbGBua9JMlFEW2dL7oKJdj GChzCb3cE3z0NztobHO7Cc7W8M+GVhNKCvicnUoH5y9Sz2wExD7QaUw7d1WhJejCLPgo KCAQ== 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=i1JbdaTpMJydG2La5gQ81SLd4rzBeviDTlLKc1Eh/88=; b=rQB7bAQzp8NQFnWc5L8tLCtIbU15/Bp3HHVZpPE46zgWeNvxA5n2Z89N6fRvGV5nkk 8p0mthGvbK+kqc+DZ/YyFAf4JcooI3I19SwAeu71zDwYfgSMqrSLG2Z2G+wlOZh6jf4J s6k6C7xs+XnxHV4wLwg4KfZlJZgXBfF5S41iOSGefM0xI1eAZ+ZklTIFoxFXmi2HcdmY r3MEoexresRYUm2EpQ92ntpB7yL/9wj2hMNMQNHIqqDMW5Fl+HMW6qr2f0NKhNX9URTR cXNBESYqpws3xI5FxbmbsAtAL8GYMrguGN4YcYU6YgV7DUEFoMrLVuij5Rkvhu4sxpZb ZR/w== X-Gm-Message-State: AOAM530B7KZg1CZies7MJkmlJdakl40TqVH9drO91YEvaGo+Y7NkuaV0 zdT6vrrdGs5TtLUyt8Anc4u57yhvWOnxO919bjoJ2A== X-Google-Smtp-Source: AGRyM1uB8vovM2S7g0G+YvxGib/s21Ys3d7Mu/guzm6mTRKwWFEw+NzAwM1UtOZyS5AKzwwfYb7eQjcwM7Mj92nnSGc= X-Received: by 2002:a05:6512:238c:b0:479:8c3:e11b with SMTP id c12-20020a056512238c00b0047908c3e11bmr4044322lfv.456.1655238591503; Tue, 14 Jun 2022 13:29:51 -0700 (PDT) MIME-Version: 1.0 References: <20210820155918.7518-1-brijesh.singh@amd.com> <20210820155918.7518-24-brijesh.singh@amd.com> <1cadca0d-c3dc-68ed-075f-f88ccb0ccc0a@amd.com> In-Reply-To: From: Peter Gonda Date: Tue, 14 Jun 2022 14:29:39 -0600 Message-ID: Subject: Re: [PATCH Part2 v5 23/45] KVM: SVM: Add KVM_SNP_INIT command To: "Kalra, Ashish" Cc: Alper Gun , "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 , Wanpeng Li , 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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jun 14, 2022 at 2:23 PM Kalra, Ashish wrote: > > [AMD Official Use Only - General] > > Hello Alper, > > Here is the feedback from the SEV/SNP firmware team: > > The SNP spec has this line in SNP_INIT_EX: > > =E2=80=9CThe firmware marks all encryption capable ASIDs as unusable for = encrypted virtualization.=E2=80=9D > > This is a back-handed way of saying that after SNP_INIT, all of the ASIDs= are considered =E2=80=9Cdirty=E2=80=9D. None are in use, but the FW can=E2= =80=99t know if there=E2=80=99s any data in the caches left over from some = prior guest. So after doing an SNP_INIT (or SNP_INIT_EX), a WBINVD (on all = threads) and DF_FLUSH sequence is required. It doesn=E2=80=99t matter wheth= er it=E2=80=99s an SEV DF_FLUSH or an SNP_DF_FLUSH=E2=80=A6 they both do EX= ACTLY the same thing. > > I don=E2=80=99t understand off hand why SNP_LAUNCH_START would require a = DF_FLUSH=E2=80=A6 Usually, it=E2=80=99s only an =E2=80=9Cactivate=E2=80=9D = command that requires the DF_FLUSH. ACTIVATE/ACTIVATE_EX/SNP_ACTIVATE/SNP_A= CTIVATE_EX > > [Ashish] That's why I asked if you are getting the DLFLUSH_REQUIRED error= after the SNP activate command ? > > It=E2=80=99s when you attempt to (re-)use >an ASID that=E2=80=99s =E2=80= =9Cdirty=E2=80=9D that you should get the DF_FLUSH_REQUIRED error. > > [Ashish] And we do SNP_DF_FLUSH/SEV_DF_FLUSH whenever ASIDs are reused/re= -cycled. > > If the host only wants to run SNP guests, there=E2=80=99s no need to do a= n SEV INIT or any other SEV operations. If the host DOES want to run SEV AN= D SNP guests, then the required sequence is to do the SNP_INIT before the S= EV INIT. Doing the WBINVD/DF_FLUSH any time between the SNP_INIT and any AC= TIVATE command should be sufficient. Thanks for the follow up Ashish! I was assuming there was some issue with the ASIDs here but didn't have time to experiment yet. Is there any downside to enabling SEV? If not these patches make sense but if there is some cost should we make KVM capable of running SNP VMs without enabling SEV? > > Thanks, > Ashish > > -----Original Message----- > From: Alper Gun > Sent: Tuesday, June 14, 2022 1:58 PM > To: Kalra, Ashish > Cc: Peter Gonda ; 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 ; Wanpeng Li ; Jim Mattson ; Andy Lutomirski <= luto@kernel.org>; Dave Hansen ; Sergio Lopez <= slp@redhat.com>; Peter Zijlstra ; Srinivas Pandruvada= ; David Rientjes ; Dov Murik ; Tobin Feldman-Fitzthum ; Borislav Petkov ; Roth, Michael ; V= lastimil Babka ; Kirill A . Shutemov = ; Andi Kleen ; Tony Luck ; Marc Or= r ; Sathyanarayanan Kuppuswamy > Subject: Re: [PATCH Part2 v5 23/45] KVM: SVM: Add KVM_SNP_INIT command > > Let me summarize what I tried. > > 1- when using psp_init_probe false, the SNP VM fails in SNP_LAUNCH_START = step with error SEV_RET_DFFLUSH_REQUIRED(15). > 2- added SEV_DF_FLUSH just after SNP platform init and it didn't fail in = launch start but failed later during SNP_LAUNCH_UPDATE with > SEV_RET_INVALID_PARAM(22) > 3- added SNP_DF_FLUSH just after SNP platform init and it failed again du= ring SNP_LAUNCH_UPDATE with SEV_RET_INVALID_PARAM(22) > 4- added sev_platform_init for SNP VMs and it worked. > > For me DF_FLUSH alone didn' help boot a VM. I don't know yet why sev plat= form status impacts the SNP VM, but sev_platform_init fixes the problem. > > > On Tue, Jun 14, 2022 at 10:16 AM Kalra, Ashish wro= te: > > > > [AMD Official Use Only - General] > > > > Hello Alper, Peter, > > > > -----Original Message----- > > From: Peter Gonda > > Sent: Tuesday, June 14, 2022 11:30 AM > > To: Kalra, Ashish > > Cc: Alper Gun ; Brijesh Singh > > ; 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 ; Wanpeng > > Li ; 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 > > ; Pavan Kumar Paluri > > > > Subject: Re: [PATCH Part2 v5 23/45] KVM: SVM: Add KVM_SNP_INIT command > > > > On Tue, Jun 14, 2022 at 10:11 AM Kalra, Ashish w= rote: > > > > > > [AMD Official Use Only - General] > > > > > > > > > -----Original Message----- > > > From: Peter Gonda > > > Sent: Tuesday, June 14, 2022 10:38 AM > > > To: Kalra, Ashish > > > Cc: Alper Gun ; Brijesh Singh > > > ; Kalra, Ashish ; 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 ; Wanpeng > > > Li ; 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 > > > ; Pavan Kumar Paluri > > > > > > Subject: Re: [PATCH Part2 v5 23/45] KVM: SVM: Add KVM_SNP_INIT > > > command > > > > > > On Mon, Jun 13, 2022 at 6:21 PM Ashish Kalra wrote= : > > > > > > > > > > > > On 6/13/22 23:33, Alper Gun wrote: > > > > > On Mon, Jun 13, 2022 at 4:15 PM Ashish Kalra w= rote: > > > > >> Hello Alper, > > > > >> > > > > >> On 6/13/22 20:58, Alper Gun wrote: > > > > >>> static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd > > > > >>> *argp) > > > > >>>> { > > > > >>>> + bool es_active =3D (argp->id =3D=3D KVM_SEV_ES_INIT || > > > > >>>> + argp->id =3D=3D KVM_SEV_SNP_INIT); > > > > >>>> struct kvm_sev_info *sev =3D &to_kvm_svm(kvm)->sev_i= nfo; > > > > >>>> - bool es_active =3D argp->id =3D=3D KVM_SEV_ES_INIT; > > > > >>>> + bool snp_active =3D argp->id =3D=3D KVM_SEV_SNP_INIT; > > > > >>>> int asid, ret; > > > > >>>> > > > > >>>> if (kvm->created_vcpus) @@ -249,12 +269,22 @@ > > > > >>>> static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd = *argp) > > > > >>>> return ret; > > > > >>>> > > > > >>>> sev->es_active =3D es_active; > > > > >>>> + sev->snp_active =3D snp_active; > > > > >>>> asid =3D sev_asid_new(sev); > > > > >>>> if (asid < 0) > > > > >>>> goto e_no_asid; > > > > >>>> sev->asid =3D asid; > > > > >>>> > > > > >>>> - ret =3D sev_platform_init(&argp->error); > > > > >>>> + if (snp_active) { > > > > >>>> + ret =3D verify_snp_init_flags(kvm, argp); > > > > >>>> + if (ret) > > > > >>>> + goto e_free; > > > > >>>> + > > > > >>>> + ret =3D sev_snp_init(&argp->error); > > > > >>>> + } else { > > > > >>>> + ret =3D sev_platform_init(&argp->error); > > > > >>> After SEV INIT_EX support patches, SEV may be initialized in th= e platform late. > > > > >>> In my tests, if SEV has not been initialized in the platform > > > > >>> yet, SNP VMs fail with SEV_DF_FLUSH required error. I tried > > > > >>> calling SEV_DF_FLUSH right after the SNP platform init but > > > > >>> this time it failed later on the SNP launch update command > > > > >>> with SEV_RET_INVALID_PARAM error. Looks like there is another > > > > >>> dependency on SEV platform initialization. > > > > >>> > > > > >>> Calling sev_platform_init for SNP VMs fixes the problem in our = tests. > > > > >> Trying to get some more context for this issue. > > > > >> > > > > >> When you say after SEV_INIT_EX support patches, SEV may be > > > > >> initialized in the platform late, do you mean sev_pci_init()->se= v_snp_init() ... > > > > >> sev_platform_init() code path has still not executed on the host= BSP ? > > > > >> > > > > > Correct, INIT_EX requires the file system to be ready and there > > > > > is a ccp module param to call it only when needed. > > > > > > > > > > MODULE_PARM_DESC(psp_init_on_probe, " if true, the PSP will be > > > > > initialized on module init. Else the PSP will be initialized on > > > > > the first command requiring it"); > > > > > > > > > > If this module param is false, it won't initialize SEV on the > > > > > platform until the first SEV VM. > > > > > > > > > Ok, that makes sense. > > > > > > > > So the fix will be to call sev_platform_init() unconditionally > > > > here in sev_guest_init(), and both sev_snp_init() and > > > > sev_platform_init() are protected from being called again, so > > > > there won't be any issues if these functions are invoked again at > > > > SNP/SEV VM launch if they have been invoked earlier during module i= nit. > > > > > > >That's one solution. I don't know if there is a downside to the syst= em for enabling SEV if SNP is being enabled but another solution could be t= o just directly place a DF_FLUSH command instead of calling sev_platform_in= it(). > > > > > > Actually sev_platform_init() is already called on module init if psp_= init_on_probe is not false. Only need to ensure that SNP firmware is initia= lized first with SNP_INIT command. > > > > > But if psp_init_on_probe is false, sev_platform_init() isn't called d= own this path. Alper has suggested we always call sev_platform_init() but w= e could just place an SEV_DF_FLUSH command instead. Or am I still missing s= omething? > > > > >After SEV INIT_EX support patches, SEV may be initialized in the platf= orm late. > > > In my tests, if SEV has not been initialized in the platform yet, > > >SNP VMs fail with SEV_DF_FLUSH required error. I tried calling > > >SEV_DF_FLUSH right after the SNP platform init. > > > > Are you getting the DLFLUSH_REQUIRED error after the SNP activate comma= nd ? > > > > Also did you use the SEV_DF_FLUSH command or the SNP_DF_FLUSH command ? > > > > With SNP you need to use SNP_DF_FLUSH command. > > > > Thanks, > > Ashish