On Wed, Sep 14, 2016 at 09:30:51AM +0100, Daniel P. Berrange wrote: > On Tue, Sep 13, 2016 at 07:00:44PM -0300, Eduardo Habkost wrote: > > (CCing Daniel Berrange in case he has feedback on the > > nonce/dh_pub_qx/dh_pub_qy loading/parsing at the end of this > > message) > > > > On Tue, Sep 13, 2016 at 02:54:40PM -0500, Brijesh Singh wrote: > > > Hi Eduardo, > > > > > > On 09/13/2016 10:58 AM, Eduardo Habkost wrote: > > > > > > > > > > A typical SEV config file looks like this: > > > > > > > > > > > > > Are those config options documented somewhere? > > > > > > > > > > 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? > > * Is bit 0 (KS) the only bit that can be set on flags? If so, why > > not a boolean "ks" option? > > * 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) > > * If vcpu_mask is a bitmap for each VCPU, should we represent it > > as a list of VCPU indexes? > > > > 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. > > It needs to use QOM really, not QAPI, since it has to be user > creatable on the CLI and we don't want to invent new command > line arguments.
As much as I don't like not being able to use the QAPI schema to document -object, this is true. [...] > > > > > > > > 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. > > The secrets object is used for information that has to be kept > private from eavesdroppers. Based on the param names here > 'dh_pub_qx' it sounds like this is non-sensitive "public" > data, so would not need to use the secrets object, but it > is hard to say for sure without close look at the technical > details. They are a public key and nonce for ECDH key agreement, so they are public (AFAICS). So let's forget about "-object secret". I just want to ensure we either reuse existing parsing code, or put the new parser in common code that can be reused. -- Eduardo
