On Wed, Mar 15, 2023, Yan Zhao wrote: > On Fri, Mar 10, 2023 at 04:22:50PM -0800, Sean Christopherson wrote: > > Bury the declaration of the page-track helpers that are intended only for > > internal KVM use in a "private" header. In addition to guarding against > > unwanted usage of the internal-only helpers, dropping their definitions > > avoids exposing other structures that should be KVM-internal, e.g. for > > memslots. This is a baby step toward making kvm_host.h a KVM-internal > > header in the very distant future. > > > > Signed-off-by: Sean Christopherson <sea...@google.com> > > --- > > arch/x86/include/asm/kvm_page_track.h | 26 ++++----------------- > > arch/x86/kvm/mmu/mmu.c | 3 ++- > > arch/x86/kvm/mmu/page_track.c | 8 +------ > > arch/x86/kvm/mmu/page_track.h | 33 +++++++++++++++++++++++++++ > > arch/x86/kvm/x86.c | 1 + > > 5 files changed, 42 insertions(+), 29 deletions(-) > > create mode 100644 arch/x86/kvm/mmu/page_track.h > > > > diff --git a/arch/x86/include/asm/kvm_page_track.h > > b/arch/x86/include/asm/kvm_page_track.h > > index e5eb98ca4fce..deece45936a5 100644 > > --- a/arch/x86/include/asm/kvm_page_track.h > > +++ b/arch/x86/include/asm/kvm_page_track.h > > A curious question: > are arch/x86/include/asm/kvm_*.h all expected to be external accessible?
Depends on what you mean by "expected". Currently, yes, everything in there is globally visible. But the vast majority of structs, defines, functions, etc. aren't intended for external non-KVM consumption, things ended up being globally visible largely through carelessness and/or a lack of a forcing function. E.g. there is absolutely no reason anything outside of KVM should need arch/x86/include/asm/kvm-x86-ops.h, but it landed in asm/ because, at the time it was added, nothing would be harmed by making kvm-x86-ops.h "public" and we didn't scrutinize the patches well enough. My primary motivation for this series is to (eventually) get to a state where only select symbols/defines/etc. are exposed by KVM to the outside world, and everything else is internal only. The end goal of tightly restricting KVM's global API is to allow concurrently loading multiple instances of kvm.ko so that userspace can upgrade/rollback KVM without needed to move VMs off the host, i.e. by performing intrahost migration between differenate instances of KVM on the same host. To do that safely, anything that is visible outside of KVM needs to be compatible across different instances of KVM, e.g. if kvm_vcpu is "public" then a KVM upgrade/rollback wouldn't be able to touch "struct kvm_vcpu" in any way. We'll definitely want to be able to modify things like the vCPU structures, thus the push to restrict the API. But even if we never realize that end goal, IMO drastically reducing KVM's "public" API surface is worthy goal in and of itself.
