Re: [PATCH 2/9] backends/confidential-guest-support: Add IGVM file parameter

[Roy Hopkins]

On Tue, 2024-03-19 at 16:10 +0100, Stefano Garzarella wrote:
> On Tue, Feb 27, 2024 at 02:50:08PM +0000, Roy Hopkins wrote:
> > In order to add support for parsing IGVM files for secure virtual
> > machines, a the path to an IGVM file needs to be specified as
> > part of the guest configuration. It makes sense to add this to
> > the ConfidentialGuestSupport object as this is common to all secure
> > virtual machines that potentially could support IGVM based
> > configuration.
> > 
> > This patch allows the filename to be configured via the QEMU
> > object model in preparation for subsequent patches that will read and
> > parse the IGVM file.
> > 
> > Signed-off-by: Roy Hopkins <roy.hopkins@suse.com>
> > ---
> > backends/confidential-guest-support.c     | 21 +++++++++++++++++++++
> > include/exec/confidential-guest-support.h |  9 +++++++++
> > qapi/qom.json                             | 13 +++++++++++++
> > qemu-options.hx                           |  8 +++++++-
> > 4 files changed, 50 insertions(+), 1 deletion(-)
> > 
> > diff --git a/backends/confidential-guest-support.c b/backends/confidential-
> > guest-support.c
> > index 052fde8db0..da436fb736 100644
> > --- a/backends/confidential-guest-support.c
> > +++ b/backends/confidential-guest-support.c
> > @@ -20,8 +20,29 @@ OBJECT_DEFINE_ABSTRACT_TYPE(ConfidentialGuestSupport,
> >                             CONFIDENTIAL_GUEST_SUPPORT,
> >                             OBJECT)
> > 
> > +#if defined(CONFIG_IGVM)
> > +static char *get_igvm(Object *obj, Error **errp)
> > +{
> > +    ConfidentialGuestSupport *cgs = CONFIDENTIAL_GUEST_SUPPORT(obj);
> > +    return g_strdup(cgs->igvm_filename);
> > +}
> > +
> > +static void set_igvm(Object *obj, const char *value, Error **errp)
> > +{
> > +    ConfidentialGuestSupport *cgs = CONFIDENTIAL_GUEST_SUPPORT(obj);
> > +    g_free(cgs->igvm_filename);
> > +    cgs->igvm_filename = g_strdup(value);
> > +}
> > +#endif
> > +
> > static void confidential_guest_support_class_init(ObjectClass *oc, void
> > *data)
> > {
> > +#if defined(CONFIG_IGVM)
> > +    object_class_property_add_str(oc, "igvm-file",
> > +        get_igvm, set_igvm);
> > +    object_class_property_set_description(oc, "igvm-file",
> > +        "Set the IGVM filename to use");
> > +#endif
> > }
> > 
> > static void confidential_guest_support_init(Object *obj)
> > diff --git a/include/exec/confidential-guest-support.h
> > b/include/exec/confidential-guest-support.h
> > index ba2dd4b5df..b08ad8de4d 100644
> > --- a/include/exec/confidential-guest-support.h
> > +++ b/include/exec/confidential-guest-support.h
> > @@ -51,6 +51,15 @@ struct ConfidentialGuestSupport {
> >      * so 'ready' is not set, we'll abort.
> >      */
> >     bool ready;
> > +
> > +#if defined(CONFIG_IGVM)
> > +    /*
> > +     * igvm_filename: Optional filename that specifies a file that contains
> > +     *                the configuration of the guest in Isolated Guest
> > +     *                Virtual Machine (IGVM) format.
> > +     */
> > +    char *igvm_filename;
> > +#endif
> > };
> > 
> > typedef struct ConfidentialGuestSupportClass {
> > diff --git a/qapi/qom.json b/qapi/qom.json
> > index 2a6e49365a..570bdd7d55 100644
> > --- a/qapi/qom.json
> > +++ b/qapi/qom.json
> > @@ -859,6 +859,18 @@
> >   'base': 'RngProperties',
> >   'data': { '*filename': 'str' } }
> > 
> > +##
> > +# @ConfidentialGuestProperties:
> > +#
> > +# Properties common to objects that are derivatives of confidential-guest-
> > support.
> > +#
> > +# @igvm-file: IGVM file to use to configure guest (default: none)
> > +#
> > +# Since: 8.2
> 
> Should it be 9.0 or maybe 9.1 ?

Good question. Obviously it is hard to predict which version this will
potentially land in. I can update it to 9.1 because it is unlikely to be in any
version prior to this, but what is the normal convention for choosing a version
number here?
> 
> > +##
> > +{ 'struct': 'ConfidentialGuestProperties',
> > +  'data': { '*igvm-file': 'str' } }
> > +
> > ##
> > # @SevGuestProperties:
> > #
> > @@ -886,6 +898,7 @@
> > # Since: 2.12
> > ##
> > { 'struct': 'SevGuestProperties',
> > +  'base': 'ConfidentialGuestProperties',
> >   'data': { '*sev-device': 'str',
> >             '*dh-cert-file': 'str',
> >             '*session-file': 'str',
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 9be1e5817c..49d9226e35 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -5640,7 +5640,7 @@ SRST
> >                  -object secret,id=sec0,keyid=secmaster0,format=base64,\\
> >                      data=$SECRET,iv=$(<iv.b64)
> > 
> > -    ``-object sev-guest,id=id,cbitpos=cbitpos,reduced-phys-bits=val,[sev-
> > device=string,policy=policy,handle=handle,dh-cert-file=file,session-
> > file=file,kernel-hashes=on|off]``
> > +    ``-object sev-guest,id=id,cbitpos=cbitpos,reduced-phys-bits=val,[sev-
> > device=string,policy=policy,handle=handle,dh-cert-file=file,session-
> > file=file,kernel-hashes=on|off,igvm-file=file]``
> >         Create a Secure Encrypted Virtualization (SEV) guest object,
> >         which can be used to provide the guest memory encryption support
> >         on AMD processors.
> > @@ -5684,6 +5684,12 @@ SRST
> >         cmdline to a designated guest firmware page for measured Linux
> >         boot with -kernel. The default is off. (Since 6.2)
> > 
> > +        The ``igvm-file`` is an optional parameter that, when specified,
> > +        allows an Independent Guest Virtual Machine (IGVM) file to be
> > +        specified that configures the secure virtual machine and can
> > +        include, for example, an SVSM module, system firmware, initial
> > +        boot state, etc.
> > +
> >         e.g to launch a SEV guest
> > 
> >         .. parsed-literal::
> > -- 
> > 2.43.0
> > 
> > 
>