Re: [RFC PATCH v2 01/12] i386/sev: introduce "sev-common" type to encapsulate common SEV state

[Michael Roth]

On Wed, Sep 01, 2021 at 04:18:21PM +0200, Markus Armbruster wrote:
> Michael Roth <michael.roth@amd.com> writes:
> 
> > Currently all SEV/SEV-ES functionality is managed through a single
> > 'sev-guest' QOM type. With upcoming support for SEV-SNP, taking this
> > same approach won't work well since some of the properties/state
> > managed by 'sev-guest' is not applicable to SEV-SNP, which will instead
> > rely on a new QOM type with its own set of properties/state.
> >
> > To prepare for this, this patch moves common state into an abstract
> > 'sev-common' parent type to encapsulate properties/state that is
> > common to both SEV/SEV-ES and SEV-SNP, leaving only SEV/SEV-ES-specific
> > properties/state in the current 'sev-guest' type. This should not
> > affect current behavior or command-line options.
> >
> > As part of this patch, some related changes are also made:
> >
> >   - a static 'sev_guest' variable is currently used to keep track of
> >     the 'sev-guest' instance. SEV-SNP would similarly introduce an
> >     'sev_snp_guest' static variable. But these instances are now
> >     available via qdev_get_machine()->cgs, so switch to using that
> >     instead and drop the static variable.
> >
> >   - 'sev_guest' is currently used as the name for the static variable
> >     holding a pointer to the 'sev-guest' instance. Re-purpose the name
> >     as a local variable referring the 'sev-guest' instance, and use
> >     that consistently throughout the code so it can be easily
> >     distinguished from sev-common/sev-snp-guest instances.
> >
> >   - 'sev' is generally used as the name for local variables holding a
> >     pointer to the 'sev-guest' instance. In cases where that now points
> >     to common state, use the name 'sev_common'; in cases where that now
> >     points to state specific to 'sev-guest' instance, use the name
> >     'sev_guest'
> >
> > Signed-off-by: Michael Roth <michael.roth@amd.com>
> > ---
> >  qapi/qom.json     |  34 +++--
> >  target/i386/sev.c | 329 +++++++++++++++++++++++++++-------------------
> >  2 files changed, 214 insertions(+), 149 deletions(-)
> >
> > diff --git a/qapi/qom.json b/qapi/qom.json
> > index a25616bc7a..211e083727 100644
> > --- a/qapi/qom.json
> > +++ b/qapi/qom.json
> > @@ -735,12 +735,29 @@
> >    'data': { '*filename': 'str' } }
> >  
> >  ##
> > -# @SevGuestProperties:
> > +# @SevCommonProperties:
> >  #
> > -# Properties for sev-guest objects.
> > +# Properties common to objects that are derivatives of sev-common.
> >  #
> >  # @sev-device: SEV device to use (default: "/dev/sev")
> >  #
> > +# @cbitpos: C-bit location in page table entry (default: 0)
> > +#
> > +# @reduced-phys-bits: number of bits in physical addresses that become
> > +#                     unavailable when SEV is enabled
> > +#
> > +# Since: 2.12
> > +##
> > +{ 'struct': 'SevCommonProperties',
> > +  'data': { '*sev-device': 'str',
> > +            '*cbitpos': 'uint32',
> > +            'reduced-phys-bits': 'uint32' } }
> > +
> > +##
> > +# @SevGuestProperties:
> > +#
> > +# Properties for sev-guest objects.
> > +#
> >  # @dh-cert-file: guest owners DH certificate (encoded with base64)
> >  #
> >  # @session-file: guest owners session parameters (encoded with base64)
> > @@ -749,21 +766,14 @@
> >  #
> >  # @handle: SEV firmware handle (default: 0)
> >  #
> > -# @cbitpos: C-bit location in page table entry (default: 0)
> > -#
> > -# @reduced-phys-bits: number of bits in physical addresses that become
> > -#                     unavailable when SEV is enabled
> > -#
> >  # Since: 2.12
> >  ##
> >  { 'struct': 'SevGuestProperties',
> > -  'data': { '*sev-device': 'str',
> > -            '*dh-cert-file': 'str',
> > +  'base': 'SevCommonProperties',
> > +  'data': { '*dh-cert-file': 'str',
> >              '*session-file': 'str',
> >              '*policy': 'uint32',
> > -            '*handle': 'uint32',
> > -            '*cbitpos': 'uint32',
> > -            'reduced-phys-bits': 'uint32' } }
> > +            '*handle': 'uint32' } }
> >  
> >  ##
> >  # @ObjectType:
> 
> External interface remains unchanged, as far as I can tell.
> 
> For the QAPI schema:
> Acked-by: Markus Armbruster <armbru@redhat.com>

Thanks!