From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brijesh Singh Subject: Re: [PATCH v8 05/28] target/i386: add memory encryption feature cpuid support Date: Tue, 13 Feb 2018 09:39:01 -0600 Message-ID: References: <20180212153715.87555-1-brijesh.singh@amd.com> <20180212153715.87555-6-brijesh.singh@amd.com> <20180212183803.GR13981@localhost.localdomain> <4a1f22d9-da2e-618b-1423-629817389948@amd.com> <20180212211948.GK14422@pd.tnic> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: "Edgar E. Iglesias" , Peter Maydell , Cornelia Huck , Brijesh Singh , Eduardo Habkost , kvm@vger.kernel.org, "Michael S. Tsirkin" , Stefan Hajnoczi , Peter Crosthwaite , Richard Henderson , Markus Armbruster , qemu-devel@nongnu.org, Christian Borntraeger , Alexander Graf , Marcel Apfelbaum , Paolo Bonzini , Thomas Lendacky , Alistair Francis , Bruce Rogers , "Dr. David Alan Gilbert" , Richard Henderson To: Borislav Petkov Return-path: In-Reply-To: <20180212211948.GK14422@pd.tnic> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel2=m.gmane.org@nongnu.org Sender: "Qemu-devel" List-Id: kvm.vger.kernel.org On Mon, Feb 12, 2018 at 3:19 PM, Borislav Petkov wrote: > On Mon, Feb 12, 2018 at 03:07:26PM -0600, Brijesh Singh wrote: > > In current implementation, when -cpu ...,+sev is passed without > > appropriate SEV configuration then we populate the Fn8000_001F CPUID bu= t > > VMCB will not have SEV bit set hence a guest will be launched as > > non-SEV. > > I think we should fail the guest init if sev is not really supported by > the host. Otherwise people might get mislead. > > Sure I will do that. We can simplify this patch if we don't fill the CPUID for non SEV guest. I am thinking of doing something like this: "If SEV configration is provided in QEMU command line then automatically increase xlevel to 0x8000_001F and populate the EAX and EBX registers" > > > Changing existing CPU models is possible only on very specific > > > circumstances (where VMs that are currently runnable would always > > > stay runnable), and would require compat_props entries to keep > > > compatibility on existing machine-types. > > > > Ah I didn't consider that case. What is recommendation, should we creat= e > > a new CPU Model (EPYC-SEV) ? > > Can we please stop creating a new CPU model with every new CPUID feature > support added? This is just ridiculous. > > If this is about live migration, then by all means, fail the migration > if the feature bits are not compatible. But replicating CPU models and > then adding one new differing feature doesn't make any sense. > > Yes, I think we should be able to avoid creating new CPU model to handle this case. I am leaning towards dropping this patch and implement logic to populate th= e CPUID 0x8000_001F only when SEV is enabled. This should not require any changes in existing CPU model feature flag and live migration of existing guest should work fine. > -- > Regards/Gruss, > Boris. > > SUSE Linux GmbH, GF: Felix Imend=C3=B6rffer, Jane Smithard, Graham Norton= , HRB > 21284 (AG N=C3=BCrnberg) > -- > > From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53654) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1elcfg-0000OC-P8 for qemu-devel@nongnu.org; Tue, 13 Feb 2018 10:39:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1elcff-0003ev-LI for qemu-devel@nongnu.org; Tue, 13 Feb 2018 10:39:04 -0500 Received: from mail-qk0-x243.google.com ([2607:f8b0:400d:c09::243]:46938) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1elcff-0003ek-FB for qemu-devel@nongnu.org; Tue, 13 Feb 2018 10:39:03 -0500 Received: by mail-qk0-x243.google.com with SMTP id g129so8383969qkb.13 for ; Tue, 13 Feb 2018 07:39:03 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20180212211948.GK14422@pd.tnic> References: <20180212153715.87555-1-brijesh.singh@amd.com> <20180212153715.87555-6-brijesh.singh@amd.com> <20180212183803.GR13981@localhost.localdomain> <4a1f22d9-da2e-618b-1423-629817389948@amd.com> <20180212211948.GK14422@pd.tnic> From: Brijesh Singh Date: Tue, 13 Feb 2018 09:39:01 -0600 Message-ID: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v8 05/28] target/i386: add memory encryption feature cpuid support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Borislav Petkov Cc: Brijesh Singh , Eduardo Habkost , "Edgar E. Iglesias" , Peter Maydell , kvm@vger.kernel.org, "Michael S. Tsirkin" , Marcel Apfelbaum , Markus Armbruster , Peter Crosthwaite , Richard Henderson , qemu-devel@nongnu.org, "Dr. David Alan Gilbert" , Christian Borntraeger , Alexander Graf , Stefan Hajnoczi , Cornelia Huck , Paolo Bonzini , Thomas Lendacky , Alistair Francis , Bruce Rogers , Richard Henderson On Mon, Feb 12, 2018 at 3:19 PM, Borislav Petkov wrote: > On Mon, Feb 12, 2018 at 03:07:26PM -0600, Brijesh Singh wrote: > > In current implementation, when -cpu ...,+sev is passed without > > appropriate SEV configuration then we populate the Fn8000_001F CPUID bu= t > > VMCB will not have SEV bit set hence a guest will be launched as > > non-SEV. > > I think we should fail the guest init if sev is not really supported by > the host. Otherwise people might get mislead. > > Sure I will do that. We can simplify this patch if we don't fill the CPUID for non SEV guest. I am thinking of doing something like this: "If SEV configration is provided in QEMU command line then automatically increase xlevel to 0x8000_001F and populate the EAX and EBX registers" > > > Changing existing CPU models is possible only on very specific > > > circumstances (where VMs that are currently runnable would always > > > stay runnable), and would require compat_props entries to keep > > > compatibility on existing machine-types. > > > > Ah I didn't consider that case. What is recommendation, should we creat= e > > a new CPU Model (EPYC-SEV) ? > > Can we please stop creating a new CPU model with every new CPUID feature > support added? This is just ridiculous. > > If this is about live migration, then by all means, fail the migration > if the feature bits are not compatible. But replicating CPU models and > then adding one new differing feature doesn't make any sense. > > Yes, I think we should be able to avoid creating new CPU model to handle this case. I am leaning towards dropping this patch and implement logic to populate th= e CPUID 0x8000_001F only when SEV is enabled. This should not require any changes in existing CPU model feature flag and live migration of existing guest should work fine. > -- > Regards/Gruss, > Boris. > > SUSE Linux GmbH, GF: Felix Imend=C3=B6rffer, Jane Smithard, Graham Norton= , HRB > 21284 (AG N=C3=BCrnberg) > -- > >