Re: [PATCH 02/10] qapi: expand docs for SEV commands

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, May 13, 2025 at 02:06:40PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > This gives some more context about the behaviour of the commands in
> > unsupported guest configuration or platform scenarios.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  qapi/misc-target.json | 43 ++++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 36 insertions(+), 7 deletions(-)
> >
> > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > index 5d0ffb0164..ae55e437a5 100644
> > --- a/qapi/misc-target.json
> > +++ b/qapi/misc-target.json
> > @@ -110,7 +110,11 @@
> >  ##
> >  # @query-sev:
> >  #
> > -# Returns information about SEV
> > +# Returns information about SEV/SEV-ES/SEV-SNP.
> > +#
> > +# If unavailable due to an incompatible configuration the
> > +# returned @enabled field will be set to 'false' and the
> > +# state of all other fields is undefined.
> 
> That's awful.  Not this patch's fault.

Yep, IMHO, all the fields except 'enabled' should have been
optional, and omitted when @enabled==false. Probably too
later 


> What's "incompatible configuration"?

Essentially it'll only set values for the extra fields
beyond @enabled when a configuration includes the
following:

  '-object sev-guest,id=sev -machine ...,confidential-guest-support=sev"

(or sev-snp-guest object)

> Actual behavior as far as I can tell:
> 
> * If !CONFIG_SEV: GenericError "SEV is not available in this QEMU".
> 
> * If CONFIG_SEV and !sev_enabled(): SevInfo filled with zero bytes

Having these two scenarios be different feels wrong to me - they
are both "SEV not enabled" scenarios IMHO, and whether or not
SEV is enabled should be irrelevant.

A difference is justified in query-sev-capabilities as that's
a feature probing method, where as this one is a runtime state
query method.

> * If CONFIG_SEV and sev_enabled(): SevInfo filled properly
> 
> sev_enabled() is true when the machine's cgs member is an instance of
> "sev-common".

Yep.

> > @@ -185,8 +198,9 @@
> >  ##
> >  # @query-sev-capabilities:
> >  #
> > -# This command is used to get the SEV capabilities, and is supported
> > -# on AMD X86 platforms only.
> > +# This command is used to get the SEV capabilities, and is only
> > +# supported on AMD X86 platforms with KVM enabled. If SEV is not
> > +# available on the platform an error will be returned.
> 
> What does "not supported" mean here?

Any of at least:

 * Not x86 system target
 * Not KVM accelerator
 * No SEV in host kernel
 * No SEV in host CPUs
 * SEV not enabled in host UEFI
 * /dev/sev device not accessible / not present



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 :|