From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34052) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkCmG-0007iS-P5 for qemu-devel@nongnu.org; Wed, 14 Sep 2016 12:11:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bkCmB-0005nl-6V for qemu-devel@nongnu.org; Wed, 14 Sep 2016 12:11:11 -0400 Received: from mail-dm3nam03on0046.outbound.protection.outlook.com ([104.47.41.46]:32205 helo=NAM03-DM3-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkCmA-0005nC-TF for qemu-devel@nongnu.org; Wed, 14 Sep 2016 12:11:07 -0400 References: <147377800565.11859.4411044563640180545.stgit@brijesh-build-machine> <147377806784.11859.11149856529336910514.stgit@brijesh-build-machine> <20160913155807.GA2850@thinpad.lan.raisama.net> <6411b07f-4edd-390c-acca-5342ab1187ba@amd.com> <20160913220044.GY24695@thinpad.lan.raisama.net> From: Brijesh Singh Message-ID: Date: Wed, 14 Sep 2016 11:10:54 -0500 MIME-Version: 1.0 In-Reply-To: <20160913220044.GY24695@thinpad.lan.raisama.net> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH v1 06/22] sev: add initial SEV support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: brijesh.singh@amd.com, crosthwaite.peter@gmail.com, armbru@redhat.com, mst@redhat.com, p.fedin@samsung.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, pbonzini@redhat.com, rth@twiddle.net, "Daniel P. Berrange" >> Various commands and parameters are documented [1] >> >> [1] http://support.amd.com/TechDocs/55766_SEV-KM%20API_Spec.pdf > > If I understand correctly, the docs describe the firmware > interface. The interface provided by QEMU is not the same thing, > and needs to be documented as well (even if it contains pointers > to sections or tables in the firmware interface docs). > > Some of the questions I have about the fields are: > * Do we really need the user to provide all the options below? > * Can't QEMU or KVM calculate vcpu_count/vcpu_length/vcpu_mask, > for example? Good question, I don't think we need to get this information from guest owner and it can be calculated from KVM. I will check with security folks on how this information is used in measurement generation and make the changes accordingly. > * Is bit 0 (KS) the only bit that can be set on flags? If so, why > not a boolean "ks" option? Agreed. I will fix in v2. > * Is "policy" the guest policy structure described at page 23? If > so, why exposing the raw value instead of separate fields for > each bit/field in the structure? (and only for the ones that > are supposed to be set by the user) Yes policy is described in chapter 3, page 23. I am open to separate the fields. Let me know if something like this works sev-launch-rule,flags.ks=0,policy.dbg=0,policy.ks=0,policy.nosend=0,... > * If vcpu_mask is a bitmap for each VCPU, should we represent it > as a list of VCPU indexes? > I will check on this one. > A good way to model this data and document it more properly is > through a QAPI schema. grep for "opts_visitor_new()" in the code > for examples where QEMU options are parsed according to a QAPI > schema. The downside is that using a QAPI visitor is (AFAIK) not > possible if using -object like I suggest below. > >> >>>> [sev-launch] >>>> flags = "00000000" >>>> policy = "000000" >>>> dh_pub_qx = "0123456789abcdef0123456789abcdef" >>>> dh_pub_qy = "0123456789abcdef0123456789abcdef" >>>> nonce = "0123456789abcdef" >>>> vcpu_count = "1" >>>> vcpu_length = "30" >>>> vcpu_mask = "00ab" >>> >> >> All these config parameters should be provided by the guest owner before >> launching or migrating a guest. I believe we need to make changes in >> libvirt, virsh and other upper layer software stack to communicate with >> guest owner to get these input parameters. For development purposes I choose >> a simple config file to get these parameters. I am not sure if we will able >> to "add new option to a existing objects/device" but we can look into >> creating a "new type for -object" or we can simply accept a fd from upper >> layer and read the fd to get these parameters. > > I was thinking of something like: > > -object sev-launch-rule,flags=0,policy=0,dh_pub_qx=XXXXX,dh_pub_qy=YYYYY,nonce=ZZZZ,vcpu_count=1,vcpu_length=30,vcpu_mask=00ab \ > -machine pc,accel=kvm,sev=on # see note below[1] > > With this, you won't need separate code for command-line, config > files, and QMP commands. They will all be able to use the same > mechanisms. > > ...but this conflicts with the idea of using QAPI. So I am not > sure which way to go. (But either way we go, we need a clearer > and better documented set of parameters). > I am open to idea and need direction on which way to go. I will work on documenting the parameters and usages. Should I consider implementing your below approach in v2 ? -object sev-launch-rule,flags=0,policy=0,dh_pub_qx=XXXXX,dh_pub_qy=YYYYY,nonce=ZZZZ,vcpu_count=1,vcpu_length=30,vcpu_mask=00ab \ -machine pc,accel=kvm,sev=on Any tips on which qemu file i can use as reference during implementation. Thanks. > [1] I would go even further and separate the accel object options > from the machine options, but this would require reworking > the accelerator configuration inside QEMU. e.g.: > > -object kvm-accel,sev=on,id=kvm0 > -machine pc,accel=kvm0 > > [...] >>> Do we really need to write our own parser? I wonder if we can >>> reuse crypto/secret.c for loading the keys. >>> >> I just looked at crypto/secret.c for loading the keys but not sure if will >> able to reuse the secret_load routines, this is mainly because the SEV >> inputs parameters are different compare to what we have in crypto/secrets.c. >> I will still look more closely and see if we can find some common code. > > There are other parameters, sure, but maybe it would be > appropriate to just load nonce/dh_pub_qx/dh_pub_qy as > TTYPE_QCRYPTO_SECRET object(s) (-object secret,...)? I am not > sure because I don't understand the crypto part fully. > > Daniel, what do you think? >