From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brijesh Singh Subject: Re: [PATCH v10 11/28] sev/i386: add command to initialize the memory encryption context Date: Wed, 7 Mar 2018 07:19:02 -0600 Message-ID: References: <20180228211028.83970-1-brijesh.singh@amd.com> <20180228211028.83970-12-brijesh.singh@amd.com> <6a76cb62-99ec-f442-28cd-ba9daf478a05@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: "Edgar E. Iglesias" , Peter Maydell , Borislav Petkov , brijesh.singh@amd.com, Eduardo Habkost , kvm@vger.kernel.org, "Michael S. Tsirkin" , Stefan Hajnoczi , Alistair Francis , Peter Crosthwaite , Richard Henderson , Alexander Graf , Markus Armbruster , Christian Borntraeger , Marcel Apfelbaum , Paolo Bonzini , Thomas Lendacky , Bruce Rogers , Cornelia Huck , "Dr. David Alan Gilbert" , Richard Henderson To: Laszlo Ersek , qemu-devel@nongnu.org Return-path: In-Reply-To: <6a76cb62-99ec-f442-28cd-ba9daf478a05@redhat.com> Content-Language: en-US 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 Hi Laszlo, On 3/5/18 7:37 AM, Laszlo Ersek wrote: > Hi Brijesh, > > I triggered an assertion failure here: > > On 02/28/18 22:10, Brijesh Singh wrote: >> When memory encryption is enabled, KVM_SEV_INIT command is used to >> initialize the platform. The command loads the SEV related persistent >> data from non-volatile storage and initializes the platform context. >> This command should be first issued before invoking any other guest >> commands provided by the SEV firmware. >> >> Cc: Paolo Bonzini >> Cc: Richard Henderson >> Cc: Eduardo Habkost >> Signed-off-by: Brijesh Singh >> --- >> accel/kvm/kvm-all.c | 15 ++++ >> include/sysemu/sev.h | 22 +++++ >> stubs/Makefile.objs | 1 + >> stubs/sev.c | 21 +++++ >> target/i386/Makefile.objs | 2 +- >> target/i386/monitor.c | 11 ++- >> target/i386/sev-stub.c | 41 +++++++++ >> target/i386/sev.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++ >> target/i386/sev_i386.h | 29 ++++++ >> target/i386/trace-events | 3 + >> 10 files changed, 364 insertions(+), 3 deletions(-) >> create mode 100644 include/sysemu/sev.h >> create mode 100644 stubs/sev.c >> create mode 100644 target/i386/sev-stub.c > [...] > >> @@ -1636,6 +1640,17 @@ static int kvm_init(MachineState *ms) >> >> kvm_state = s; >> >> + /* >> + * if memory encryption object is specified then initialize the memory >> + * encryption context. >> + */ >> + if (ms->memory_encryption) { >> + kvm_state->memcrypt_handle = sev_guest_init(ms->memory_encryption); >> + if (!kvm_state->memcrypt_handle) { >> + goto err; >> + } >> + } >> + > You should set the "err" variable to a negative error code; otherwise > the assertion at the "err" label will trigger: > > err: > assert(ret < 0); Noted, will fix in v11. thanks for reporting this. From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32855) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1etYyU-0005MJ-SD for qemu-devel@nongnu.org; Wed, 07 Mar 2018 08:19:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1etYyR-0001XI-Lw for qemu-devel@nongnu.org; Wed, 07 Mar 2018 08:19:18 -0500 Received: from mail-by2nam01on0089.outbound.protection.outlook.com ([104.47.34.89]:11264 helo=NAM01-BY2-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1etYyR-0001Wr-9E for qemu-devel@nongnu.org; Wed, 07 Mar 2018 08:19:15 -0500 References: <20180228211028.83970-1-brijesh.singh@amd.com> <20180228211028.83970-12-brijesh.singh@amd.com> <6a76cb62-99ec-f442-28cd-ba9daf478a05@redhat.com> From: Brijesh Singh Message-ID: Date: Wed, 7 Mar 2018 07:19:02 -0600 MIME-Version: 1.0 In-Reply-To: <6a76cb62-99ec-f442-28cd-ba9daf478a05@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [Qemu-devel] [PATCH v10 11/28] sev/i386: add command to initialize the memory encryption context List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek , qemu-devel@nongnu.org Cc: brijesh.singh@amd.com, Peter Maydell , kvm@vger.kernel.org, "Michael S. Tsirkin" , Stefan Hajnoczi , Alexander Graf , "Edgar E. Iglesias" , Markus Armbruster , Bruce Rogers , Christian Borntraeger , Marcel Apfelbaum , Borislav Petkov , Thomas Lendacky , Eduardo Habkost , Richard Henderson , "Dr. David Alan Gilbert" , Alistair Francis , Cornelia Huck , Richard Henderson , Peter Crosthwaite , Paolo Bonzini Hi Laszlo, On 3/5/18 7:37 AM, Laszlo Ersek wrote: > Hi Brijesh, > > I triggered an assertion failure here: > > On 02/28/18 22:10, Brijesh Singh wrote: >> When memory encryption is enabled, KVM_SEV_INIT command is used to >> initialize the platform. The command loads the SEV related persistent >> data from non-volatile storage and initializes the platform context. >> This command should be first issued before invoking any other guest >> commands provided by the SEV firmware. >> >> Cc: Paolo Bonzini >> Cc: Richard Henderson >> Cc: Eduardo Habkost >> Signed-off-by: Brijesh Singh >> --- >> accel/kvm/kvm-all.c | 15 ++++ >> include/sysemu/sev.h | 22 +++++ >> stubs/Makefile.objs | 1 + >> stubs/sev.c | 21 +++++ >> target/i386/Makefile.objs | 2 +- >> target/i386/monitor.c | 11 ++- >> target/i386/sev-stub.c | 41 +++++++++ >> target/i386/sev.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++ >> target/i386/sev_i386.h | 29 ++++++ >> target/i386/trace-events | 3 + >> 10 files changed, 364 insertions(+), 3 deletions(-) >> create mode 100644 include/sysemu/sev.h >> create mode 100644 stubs/sev.c >> create mode 100644 target/i386/sev-stub.c > [...] > >> @@ -1636,6 +1640,17 @@ static int kvm_init(MachineState *ms) >> >> kvm_state = s; >> >> + /* >> + * if memory encryption object is specified then initialize the memory >> + * encryption context. >> + */ >> + if (ms->memory_encryption) { >> + kvm_state->memcrypt_handle = sev_guest_init(ms->memory_encryption); >> + if (!kvm_state->memcrypt_handle) { >> + goto err; >> + } >> + } >> + > You should set the "err" variable to a negative error code; otherwise > the assertion at the "err" label will trigger: > > err: > assert(ret < 0); Noted, will fix in v11. thanks for reporting this.