| Message ID | 20240320083945.991426-26-michael.roth@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add AMD Secure Nested Paging (SEV-SNP) support | expand |
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). Paolo
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 >
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);
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(-)