| Message ID | 20240412084309.1733783-2-steven.price@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,01/43] KVM: Prepare for handling only shared mappings in mmu_notifier events | expand |
Hi, On Fri, Apr 12, 2024 at 9:43 AM Steven Price <steven.price@arm.com> wrote: > > From: Sean Christopherson <seanjc@google.com> > > Add flags to "struct kvm_gfn_range" to let notifier events target only > shared and only private mappings, and write up the existing mmu_notifier > events to be shared-only (private memory is never associated with a > userspace virtual address, i.e. can't be reached via mmu_notifiers). > > Add two flags so that KVM can handle the three possibilities (shared, > private, and shared+private) without needing something like a tri-state > enum. > > Link: https://lore.kernel.org/all/ZJX0hk+KpQP0KUyB@google.com > Signed-off-by: Sean Christopherson <seanjc@google.com> > Signed-off-by: Steven Price <steven.price@arm.com> > --- > include/linux/kvm_host.h | 2 ++ > virt/kvm/kvm_main.c | 7 +++++++ > 2 files changed, 9 insertions(+) > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 48f31dcd318a..c7581360fd88 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -268,6 +268,8 @@ struct kvm_gfn_range { > gfn_t start; > gfn_t end; > union kvm_mmu_notifier_arg arg; > + bool only_private; > + bool only_shared; > bool may_block; > }; > bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range); > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index fb49c2a60200..3486ceef6f4e 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -633,6 +633,13 @@ static __always_inline kvm_mn_ret_t __kvm_handle_hva_range(struct kvm *kvm, > * the second or later invocation of the handler). > */ > gfn_range.arg = range->arg; > + > + /* > + * HVA-based notifications aren't relevant to private > + * mappings as they don't have a userspace mapping. > + */ > + gfn_range.only_private = false; > + gfn_range.only_shared = true; > gfn_range.may_block = range->may_block; I'd discussed this with Sean when he posted this earlier. Having two booleans to encode three valid states could be confusing. In response, Sean suggested using an enum instead: https://lore.kernel.org/all/ZUO1Giju0GkUdF0o@google.com/ Cheers, /fuad > > /* > -- > 2.34.1 >
On 25/04/2024 10:48, Fuad Tabba wrote: > Hi, > > On Fri, Apr 12, 2024 at 9:43 AM Steven Price <steven.price@arm.com> wrote: >> >> From: Sean Christopherson <seanjc@google.com> >> >> Add flags to "struct kvm_gfn_range" to let notifier events target only >> shared and only private mappings, and write up the existing mmu_notifier >> events to be shared-only (private memory is never associated with a >> userspace virtual address, i.e. can't be reached via mmu_notifiers). >> >> Add two flags so that KVM can handle the three possibilities (shared, >> private, and shared+private) without needing something like a tri-state >> enum. >> >> Link: https://lore.kernel.org/all/ZJX0hk+KpQP0KUyB@google.com >> Signed-off-by: Sean Christopherson <seanjc@google.com> >> Signed-off-by: Steven Price <steven.price@arm.com> >> --- >> include/linux/kvm_host.h | 2 ++ >> virt/kvm/kvm_main.c | 7 +++++++ >> 2 files changed, 9 insertions(+) >> >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> index 48f31dcd318a..c7581360fd88 100644 >> --- a/include/linux/kvm_host.h >> +++ b/include/linux/kvm_host.h >> @@ -268,6 +268,8 @@ struct kvm_gfn_range { >> gfn_t start; >> gfn_t end; >> union kvm_mmu_notifier_arg arg; >> + bool only_private; >> + bool only_shared; >> bool may_block; >> }; >> bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range); >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >> index fb49c2a60200..3486ceef6f4e 100644 >> --- a/virt/kvm/kvm_main.c >> +++ b/virt/kvm/kvm_main.c >> @@ -633,6 +633,13 @@ static __always_inline kvm_mn_ret_t __kvm_handle_hva_range(struct kvm *kvm, >> * the second or later invocation of the handler). >> */ >> gfn_range.arg = range->arg; >> + >> + /* >> + * HVA-based notifications aren't relevant to private >> + * mappings as they don't have a userspace mapping. >> + */ >> + gfn_range.only_private = false; >> + gfn_range.only_shared = true; >> gfn_range.may_block = range->may_block; > > I'd discussed this with Sean when he posted this earlier. Having two > booleans to encode three valid states could be confusing. In response, > Sean suggested using an enum instead: > https://lore.kernel.org/all/ZUO1Giju0GkUdF0o@google.com/ That would work fine too! Unless I've missed it Sean hasn't posted an updated patch. My assumption is that this will get merged (in whatever form) before the rest of the series as part of that other series. It shouldn't be too hard to adapt. Thanks, Steve > Cheers, > /fuad > >> >> /* > > >> -- >> 2.34.1 >> >
On Thu, Apr 25, 2024, Steven Price wrote: > On 25/04/2024 10:48, Fuad Tabba wrote: > > On Fri, Apr 12, 2024 at 9:43 AM Steven Price <steven.price@arm.com> wrote: > >> bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range); > >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > >> index fb49c2a60200..3486ceef6f4e 100644 > >> --- a/virt/kvm/kvm_main.c > >> +++ b/virt/kvm/kvm_main.c > >> @@ -633,6 +633,13 @@ static __always_inline kvm_mn_ret_t __kvm_handle_hva_range(struct kvm *kvm, > >> * the second or later invocation of the handler). > >> */ > >> gfn_range.arg = range->arg; > >> + > >> + /* > >> + * HVA-based notifications aren't relevant to private > >> + * mappings as they don't have a userspace mapping. > >> + */ > >> + gfn_range.only_private = false; > >> + gfn_range.only_shared = true; > >> gfn_range.may_block = range->may_block; > > > > I'd discussed this with Sean when he posted this earlier. Having two > > booleans to encode three valid states could be confusing. In response, > > Sean suggested using an enum instead: > > https://lore.kernel.org/all/ZUO1Giju0GkUdF0o@google.com/ > > That would work fine too! Unless I've missed it Sean hasn't posted an > updated patch. My assumption is that this will get merged (in whatever > form) before the rest of the series as part of that other series. It > shouldn't be too hard to adapt. Yeah, there's no updated patch. Fuad, if you have a strong preference, I recommend chiming in on the TDX series[*], as that is the series that's likely going to be the first user, and I don't have a strong preference on bools versus an enum. [*] https://lore.kernel.org/all/e324ff5e47e07505648c0092a5370ac9ddd72f0b.1708933498.git.isaku.yamahata@intel.com
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 48f31dcd318a..c7581360fd88 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -268,6 +268,8 @@ struct kvm_gfn_range { gfn_t start; gfn_t end; union kvm_mmu_notifier_arg arg; + bool only_private; + bool only_shared; bool may_block; }; bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range); diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index fb49c2a60200..3486ceef6f4e 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -633,6 +633,13 @@ static __always_inline kvm_mn_ret_t __kvm_handle_hva_range(struct kvm *kvm, * the second or later invocation of the handler). */ gfn_range.arg = range->arg; + + /* + * HVA-based notifications aren't relevant to private + * mappings as they don't have a userspace mapping. + */ + gfn_range.only_private = false; + gfn_range.only_shared = true; gfn_range.may_block = range->may_block; /*