Re: [PATCH] sev: allow capabilities to check for SEV-ES support

[Tyler Fanelli]

On 11/16/21 4:17 AM, Daniel P. Berrangé wrote:
> On Mon, Nov 15, 2021 at 02:38:04PM -0500, Tyler Fanelli wrote:
> > Probe for SEV-ES and SEV-SNP capabilities to distinguish between Rome,
> > Naples, and Milan processors. Use the CPUID function to probe if a
> > processor is capable of running SEV-ES or SEV-SNP, rather than if it
> > actually is running SEV-ES or SEV-SNP.
> >
> > Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
> > ---
> >   qapi/misc-target.json | 11 +++++++++--
> >   target/i386/sev.c     |  6 ++++--
> >   2 files changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > index 5aa2b95b7d..c3e9bce12b 100644
> > --- a/qapi/misc-target.json
> > +++ b/qapi/misc-target.json
> > @@ -182,13 +182,19 @@
> >   # @reduced-phys-bits: Number of physical Address bit reduction when SEV is
> >   #                     enabled
> >   #
> > +# @es: SEV-ES capability of the machine.
> > +#
> > +# @snp: SEV-SNP capability of the machine.
> > +#
> >   # Since: 2.12
> >   ##
> >   { 'struct': 'SevCapability',
> >     'data': { 'pdh': 'str',
> >               'cert-chain': 'str',
> >               'cbitpos': 'int',
> > -            'reduced-phys-bits': 'int'},
> > +            'reduced-phys-bits': 'int',
> > +            'es': 'bool',
> > +            'snp': 'bool'},
> >     'if': 'TARGET_I386' }
> >   
> >   ##
> > @@ -205,7 +211,8 @@
> >   #
> >   # -> { "execute": "query-sev-capabilities" }
> >   # <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
> > -#                  "cbitpos": 47, "reduced-phys-bits": 5}}
> > +#                  "cbitpos": 47, "reduced-phys-bits": 5
> > +#                  "es": false, "snp": false}}
> We've previously had patches posted to support SNP in QEMU
>
>    https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg04761.html
>
> and this included an update to query-sev for reporting info
> about the VM instance.
>
> Your patch is updating query-sev-capabilities, which is a
> counterpart for detecting host capabilities separate from
> a guest instance.

Yes, that's because with this patch, I'm more interested in determining 
which AMD processor is running on a host, and less if ES or SNP is 
actually running on a guest instance or not.

> None the less I wonder if the same design questions from
> query-sev apply. ie do we need to have the ability to
> report any SNP specific information fields, if so we need
> to use a discriminated union of structs, not just bool
> flags.
>
> More generally I'm some what wary of adding this to
> query-sev-capabilities at all, unless it is part of the
> main SEV-SNP series.
>
> Also what's the intended usage for the mgmt app from just
> having these boolean fields ? Are they other more explicit
> feature flags we should be reporting, instead of what are
> essentially SEV generation codenames.

If by "mgmt app" you're referring to sevctl, in order to determine which 
certificate chain to use (Naples vs Rome vs Milan ARK/ASK) we must query 
which processor we are running on. Although sevctl has a feature which 
can do this already, we cannot guarantee that sevctl is running on the 
same host that a VM is running on, so we must query this capability from 
QEMU. My logic was determining the processor would have been the following:

!es && !snp --> Naples

es && !snp --> Rome

es && snp --> Milan

> Regards,
> Daniel

Tyler.

--
Tyler Fanelli (tfanelli)