On Wed, Jun 10, 2026 at 09:43:39AM +0100, Daniel P. Berrangé wrote:
> On Wed, Jun 10, 2026 at 01:43:52PM +0530, Naveen N Rao wrote:
> > Hi Daniel,
> > 
> > On Tue, Jun 09, 2026 at 10:14:11AM +0100, Daniel P. Berrangé wrote:
> > > On Wed, May 20, 2026 at 06:57:55PM +0530, Naveen N Rao (AMD) wrote:
> > > > In preparation for qemu being able to set SEV features through the cli,
> > > > add a check to ensure that SEV features are not also set if using IGVM
> > > > files.
> > > > 
> > > > Reviewed-by: Tom Lendacky <[email protected]>
> > > > Signed-off-by: Naveen N Rao (AMD) <[email protected]>
> > > > ---
> > > >  target/i386/sev.c | 10 ++++++++++
> > > >  1 file changed, 10 insertions(+)
> > > > 
> > > > diff --git a/target/i386/sev.c b/target/i386/sev.c
> > > > index 22c350fe14b7..641a295c42b7 100644
> > > > --- a/target/i386/sev.c
> > > > +++ b/target/i386/sev.c
> > > > @@ -1908,6 +1908,16 @@ static int 
> > > > sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
> > > >           * as SEV_STATE_UNINIT.
> > > >           */
> > > >          if (x86machine->igvm) {
> > > > +            /*
> > > > +             * Test only the user-set SEV features by masking out
> > > > +             * SVM_SEV_FEAT_SNP_ACTIVE which is set by default.
> > > > +             */
> > > > +            if (sev_common->sev_features & ~SVM_SEV_FEAT_SNP_ACTIVE) {
> > > > +                error_setg(errp,
> > > > +                           "%s: SEV features can't be specified when 
> > > > using IGVM files",
> > > > +                           __func__);
> > > 
> > > IMHO, including __func__ in error messages is redundant / undesirable.
> > > The error msg  alone should be sufficient to diagnose problems.
> > 
> > Understood. Markus had a similar concern, though this is just following 
> > the pattern in other messages in this function.
> > 
> > In fact, I checked and there are 40-odd uses of __func__ in 
> > target/i386/sev.c, so this looks to be a common pattern used for SEV.  I 
> > can submit a separate cleanup patch (independent of this series) if you 
> > think we should make this change throughout.
> 
> If you want to do it a separate cleanup of __func__ in sev.c later
> that's fine with me.

I have posted a patch which does this (includes your below suggested 
change on top of this patchset):
https://lore.kernel.org/qemu-devel/[email protected]/T/#u


- Naveen

> 
> > > Perhaps we should include the actual feature values though
> > > 
> > >                            "SEV features 0x%x can't be specified when 
> > > using IGVM files",
> > >                            sev_common->sev_features & 
> > > ~SVM_SEV_FEAT_SNP_ACTIVE);
> > > 
> > > this case, however, you should report which features are not permitted
> > 
> > Sure, I can incorporate your suggested change above unless you wanted 
> > the individual SEV features to be called out/named?
> 
> Naming them individually is probably overkill for a scenario that
> should hopefully be rare.
> 
> With regards,
> Daniel
> -- 
> |: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
> |: https://libvirt.org          ~~          https://entangle-photo.org :|
> |: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|
> 

Reply via email to