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

[Daniel P . Berrangé]

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.


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