Re: [PATCH v2 0/9] Move initializing SEV/SNP functionality to KVM

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Dec 17, 2024 at 05:16:01PM -0600, Kalra, Ashish wrote:
> 
> 
> On 12/17/2024 3:37 PM, Sean Christopherson wrote:
> > On Tue, Dec 17, 2024, Ashish Kalra wrote:
> >>
> >>
> >> On 12/17/2024 10:00 AM, Dionna Amalie Glaze wrote:
> >>> On Mon, Dec 16, 2024 at 3:57 PM Ashish Kalra <Ashish.Kalra@amd.com> wrote:
> >>>>
> >>>> From: Ashish Kalra <ashish.kalra@amd.com>
> >>>
> >>>> The on-demand SEV initialization support requires a fix in QEMU to
> >>>> remove check for SEV initialization to be done prior to launching
> >>>> SEV/SEV-ES VMs.
> >>>> NOTE: With the above fix for QEMU, older QEMU versions will be broken
> >>>> with respect to launching SEV/SEV-ES VMs with the newer kernel/KVM as
> >>>> older QEMU versions require SEV initialization to be done before
> >>>> launching SEV/SEV-ES VMs.
> >>>>
> >>>
> >>> I don't think this is okay. I think you need to introduce a KVM
> >>> capability to switch over to the new way of initializing SEV VMs and
> >>> deprecate the old way so it doesn't need to be supported for any new
> >>> additions to the interface.
> >>>
> >>
> >> But that means KVM will need to support both mechanisms of doing SEV
> >> initialization - during KVM module load time and the deferred/lazy
> >> (on-demand) SEV INIT during VM launch.
> > 
> > What's the QEMU change?  Dionna is right, we can't break userspace, but maybe
> > there's an alternative to supporting both models.
> 
> Here is the QEMU fix : (makes a SEV PLATFORM STATUS firmware call via PSP driver ioctl
> to check if SEV is in INIT state)
>  
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 1a4eb1ada6..4fa8665395 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -1503,15 +1503,6 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>          }
>      }
> 
> -    if (sev_es_enabled() && !sev_snp_enabled()) {
> -        if (!(status.flags & SEV_STATUS_FLAGS_CONFIG_ES)) {
> -            error_setg(errp, "%s: guest policy requires SEV-ES, but "
> -                         "host SEV-ES support unavailable",
> -                         __func__);
> -            return -1;
> -        }
> -    }

Sigh, that code exists in all versions of QEMU that shipped with SEV-ES
support. IOW the proposed kernel change is not limited to breaking
"older QEMU versions". Every QEMU for the last 3 years will break,
including the newest version released last week. Please don't do that.

If the kvm-svm  kmod supports both load time init and lazy init, then
the QEMU incompatibility still exists, and will likely get pushed on
users by the OS distro forcing use of the lazy-load option :-(

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