On Wed, Mar 20, 2024 at 11:47:28AM +0000, Daniel P. Berrangé wrote:
> On Wed, Mar 20, 2024 at 03:39:17AM -0500, Michael Roth wrote:
> > 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 are
> > 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@xxxxxxx>
> > ---
> > qapi/qom.json | 32 ++--
> > target/i386/sev.c | 457 ++++++++++++++++++++++++++--------------------
> > target/i386/sev.h | 3 +
> > 3 files changed, 281 insertions(+), 211 deletions(-)
> >
>
> > static SevInfo *sev_get_info(void)
> > {
> > SevInfo *info;
> > + SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
> > + SevGuestState *sev_guest =
> > + (SevGuestState *)object_dynamic_cast(OBJECT(sev_common),
> > + TYPE_SEV_GUEST);
> >
> > info = g_new0(SevInfo, 1);
> > info->enabled = sev_enabled();
> >
> > if (info->enabled) {
> > - info->api_major = sev_guest->api_major;
> > - info->api_minor = sev_guest->api_minor;
> > - info->build_id = sev_guest->build_id;
> > - info->policy = sev_guest->policy;
> > - info->state = sev_guest->state;
> > - info->handle = sev_guest->handle;
> > + if (sev_guest) {
> > + info->handle = sev_guest->handle;
> > + }
>
> If we're not going to provide a value for 'handle', then
> we should update the QAPI for this to mark the property
> as optional, which would then require doing
>
> info->has_handle = true;
>
> inside this 'if' block.
I think this is another temporarily-awkward case that gets resolved
with:
i386/sev: Update query-sev QAPI format to handle SEV-SNP
With that patch 'handle' is always available for SEV guests, and never
available for SNP, and that's managed through a discriminated union
type. I think that info->handle should be treated the same as the
other fields as part of this patch and any changes in how they are
reported should be kept in the above-mentioned patch.
This might be another artifact from v2's handling. Will get this fixed
up.
-Mike
> > + }
>
> > + info->api_major = sev_common->api_major;
> > + info->api_minor = sev_common->api_minor;
> > + info->build_id = sev_common->build_id;
> > + info->state = sev_common->state;
> > + /* we only report the lower 32-bits of policy for SNP, ok for now... */
> > + info->policy =
> > + (uint32_t)object_property_get_uint(OBJECT(sev_common),
> > + "policy", NULL);
> > }
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>
