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@xxxxxxx>
> > ---
> > 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
>
