Re: [PATCH v3 25/49] i386/sev: Skip RAMBlock notifiers for SNP

[Michael Roth]

On Wed, Mar 20, 2024 at 10:46:29AM +0100, Paolo Bonzini wrote:
> On 3/20/24 09:39, Michael Roth wrote:
> > SEV uses these notifiers to register/pin pages prior to guest use, since
> > they could potentially be used for private memory where page migration
> > is not supported. But SNP only uses guest_memfd-provided pages for
> > private memory, which has its own kernel-internal mechanisms for
> > registering/pinning memory.
> > 
> > Signed-off-by: Michael Roth <michael.roth@amd.com>
> > ---
> >   target/i386/sev.c | 10 +++++++++-
> >   1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target/i386/sev.c b/target/i386/sev.c
> > index 61af312a11..774262d834 100644
> > --- a/target/i386/sev.c
> > +++ b/target/i386/sev.c
> > @@ -982,7 +982,15 @@ static int sev_kvm_init(ConfidentialGuestSupport *cgs, 
> > Error **errp)
> >           goto err;
> >       }
> > -    ram_block_notifier_add(&sev_ram_notifier);
> > +    if (!sev_snp_enabled()) {
> > +        /*
> > +         * SEV uses these notifiers to register/pin pages prior to guest 
> > use,
> > +         * but SNP relies on guest_memfd for private pages, which has it's
> > +         * own internal mechanisms for registering/pinning private memory.
> > +         */
> > +        ram_block_notifier_add(&sev_ram_notifier);
> > +    }
> > +
> >       qemu_add_machine_init_done_notifier(&sev_machine_done_notify);
> >       qemu_add_vm_change_state_handler(sev_vm_state_change, sev_common);
> 
> These three lines can be done in any order, so I suggest removing
> ram_block_notifier_add + qemu_add_machine_init_done_notifier from the
> sev-common implementation of kvm_init (let's call it sev_common_kvm_init);
> and add an override in sev-guest that calls them if sev_common_kvm_init()
> succeeds.
> 
> (treat this as a review for 25/26/29).

Makes sense. Will split out the common bits of sev_kvm_init() and use
class methods for initialization specific to sev-guest and
sev-snp-guest.

-Mike

> 
> Paolo
>