| Message ID | 20240320083945.991426-24-michael.roth@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add AMD Secure Nested Paging (SEV-SNP) support | expand |
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
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 :| >
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 @@ -316,12 +316,21 @@ sev_enabled(void) return !!object_dynamic_cast(OBJECT(cgs), TYPE_SEV_COMMON); } +bool +sev_snp_enabled(void) +{ + ConfidentialGuestSupport *cgs = MACHINE(qdev_get_machine())->cgs; + + return !!object_dynamic_cast(OBJECT(cgs), TYPE_SEV_SNP_GUEST); +} + bool sev_es_enabled(void) { ConfidentialGuestSupport *cgs = MACHINE(qdev_get_machine())->cgs; - return sev_enabled() && (SEV_GUEST(cgs)->policy & SEV_POLICY_ES); + return sev_snp_enabled() || + (sev_enabled() && SEV_GUEST(cgs)->policy & SEV_POLICY_ES); } uint32_t @@ -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", diff --git a/target/i386/sev.h b/target/i386/sev.h index bedc667eeb..94295ee74f 100644 --- a/target/i386/sev.h +++ b/target/i386/sev.h @@ -45,9 +45,11 @@ typedef struct SevKernelLoaderContext { #ifdef CONFIG_SEV bool sev_enabled(void); bool sev_es_enabled(void); +bool sev_snp_enabled(void); #else #define sev_enabled() 0 #define sev_es_enabled() 0 +#define sev_snp_enabled() 0 #endif uint32_t sev_get_cbit_position(void);
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(-)