Re: [PATCH v3 23/49] i386/sev: Add a sev_snp_enabled() helper

[Michael Roth]

On Wed, Mar 20, 2024 at 12:35:09PM +0000, Daniel P. Berrangé wrote:
> On Wed, Mar 20, 2024 at 03:39:19AM -0500, Michael Roth wrote:
> > Add a simple helper to check if the current guest type is SNP. Also have
> > SNP-enabled imply that SEV-ES is enabled as well, and fix up any places
> > where the sev_es_enabled() check is expecting a pure/non-SNP guest.
> > 
> > Signed-off-by: Michael Roth <michael.roth@amd.com>
> > ---
> >  target/i386/sev.c | 13 ++++++++++++-
> >  target/i386/sev.h |  2 ++
> >  2 files changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target/i386/sev.c b/target/i386/sev.c
> > index 7e6dab642a..2eb13ba639 100644
> > --- a/target/i386/sev.c
> > +++ b/target/i386/sev.c
> 
> 
> > @@ -933,7 +942,9 @@ static int sev_kvm_init(ConfidentialGuestSupport *cgs, 
> > Error **errp)
> >                           __func__);
> >              goto err;
> >          }
> > +    }
> >  
> > +    if (sev_es_enabled() && !sev_snp_enabled()) {
> >          if (!(status.flags & SEV_STATUS_FLAGS_CONFIG_ES)) {
> >              error_report("%s: guest policy requires SEV-ES, but "
> >                           "host SEV-ES support unavailable",
> 
> Opps, pre-existing bug here - this method has an 'Error **errp'
> parameter, so should be using 'error_report'.
> 
> There are several more examples of this in this method that
> predate your patch series.  Can you put a patch at the start
> of this series that fixes them before introducing SNP.

Sure, will add a pre-patch to fix up all the pre-existing issues
you've noted.

-Mike

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