On Wed, Nov 8, 2023 at 4:20 AM Anish Ghulati <[email protected]> wrote:
>
> This series is a rough, PoC-quality RFC to allow (un)loading and running
> multiple KVM modules simultaneously on a single host, e.g. to deploy
> fixes, mitigations, and/or new features without having to drain all VMs
> from the host. Multi-KVM will also allow running the "same" KVM module
> with different params, e.g. to run trusted VMs with different mitigations.
>
> The goal of this RFC is to get feedback on the idea itself and the
> high-level approach. In particular, we're looking for input on:
>
> - Combining kvm_intel.ko and kvm_amd.ko into kvm.ko
> - Exposing multiple /dev/kvmX devices via Kconfig
> - The name and prefix of the new base module
>
> Feedback on individual patches is also welcome, but please keep in mind
> that this is very much a work in-progress
Hello Anish
Scarce effort on multi-KVM can be seen in the mail list albeit many
companies enable multi-KVM internally.
I'm glad that you took a big step in upstreaming it. And I hope it
can be materialized soon.
>
> - Move system-wide virtualization resource management to a new base
> module to avoid collisions between different KVM modules, e.g. VPIDs
> and ASIDs need to be unique per VM, and callbacks from IRQ handlers need
> to be mediated so that things like PMIs get to the right KVM instance.
perf_register_guest_info_callbacks() also accesses to system-wide resources,
but I don't see its relating code including kvm_guest_cbs being moved to AVC.
>
> - Refactor KVM to make all upgradable assets visible only to KVM, i.e.
> make KVM a black box, so that the layout/size of things like "struct
> kvm_vcpu" isn't exposed to the kernel at-large.
>
> - Fold kvm_intel.ko and kvm_amd.ko into kvm.ko to avoid complications
> having to generate unique symbols for every symbol exported by kvm.ko.
The sizes of kvm_intel.ko and kvm_amd.ko are big, and there
is only 1G in the kernel available for modules. So I don't think folding
two vendors' code into kvm.ko is a good idea.
Since the symbols in the new module are invisible outside, I recommend:
new kvm_intel.ko = kvm_intel.ko + kvm.ko
new kvm_amd.ko = kvm_amd.ko + kvm.ko
>
> - Add a Kconfig string to allow defining a device and module postfix at
> build time, e.g. to create kvmX.ko and /dev/kvmX.
>
> The proposed name of the new base module is vac.ko, a.k.a.
> Virtualization Acceleration Code (Unupgradable Units Module). Childish
> humor aside, "vac" is a unique name in the kernel and hopefully in x86
> and hardware terminology, is a unique name in the kernel and hopefully
> in x86 and hardware terminology, e.g. `git grep vac_` yields no hits in
> the kernel. It also has the same number of characters as "kvm", e.g.
> the namespace can be modified without needing whitespace adjustment if
> we want to go that route.
How about the name kvm_base.ko?
And the variable/function name in it can still be kvm_foo (other than
kvm_base_foo).
Thanks
Lai
On Fri, Nov 17, 2023, Lai Jiangshan wrote:
> On Wed, Nov 8, 2023 at 4:20 AM Anish Ghulati <[email protected]> wrote:
> >
> > This series is a rough, PoC-quality RFC to allow (un)loading and running
> > multiple KVM modules simultaneously on a single host, e.g. to deploy
> > fixes, mitigations, and/or new features without having to drain all VMs
> > from the host. Multi-KVM will also allow running the "same" KVM module
> > with different params, e.g. to run trusted VMs with different mitigations.
> >
> > The goal of this RFC is to get feedback on the idea itself and the
> > high-level approach. In particular, we're looking for input on:
> >
> > - Combining kvm_intel.ko and kvm_amd.ko into kvm.ko
> > - Exposing multiple /dev/kvmX devices via Kconfig
> > - The name and prefix of the new base module
> >
> > Feedback on individual patches is also welcome, but please keep in mind
> > that this is very much a work in-progress
>
> Hello Anish
>
> Scarce effort on multi-KVM can be seen in the mail list albeit many
> companies enable multi-KVM internally.
>
> I'm glad that you took a big step in upstreaming it. And I hope it
> can be materialized soon.
>
>
> >
> > - Move system-wide virtualization resource management to a new base
> > module to avoid collisions between different KVM modules, e.g. VPIDs
> > and ASIDs need to be unique per VM, and callbacks from IRQ handlers need
> > to be mediated so that things like PMIs get to the right KVM instance.
>
> perf_register_guest_info_callbacks() also accesses to system-wide resources,
> but I don't see its relating code including kvm_guest_cbs being moved to AVC.
Yeah, that's on the TODO list. IIRC, the plan is to have VAC register a single
callback with perf, and then have VAC deal with invoking the callback(s) for the
correct KVM instance.
> > - Refactor KVM to make all upgradable assets visible only to KVM, i.e.
> > make KVM a black box, so that the layout/size of things like "struct
> > kvm_vcpu" isn't exposed to the kernel at-large.
> >
> > - Fold kvm_intel.ko and kvm_amd.ko into kvm.ko to avoid complications
> > having to generate unique symbols for every symbol exported by kvm.ko.
>
> The sizes of kvm_intel.ko and kvm_amd.ko are big, and there
> is only 1G in the kernel available for modules. So I don't think folding
> two vendors' code into kvm.ko is a good idea.
>
> Since the symbols in the new module are invisible outside, I recommend:
> new kvm_intel.ko = kvm_intel.ko + kvm.ko
> new kvm_amd.ko = kvm_amd.ko + kvm.ko
Yeah, Paolo also suggested this at LPC.
> > - Add a Kconfig string to allow defining a device and module postfix at
> > build time, e.g. to create kvmX.ko and /dev/kvmX.
> >
> > The proposed name of the new base module is vac.ko, a.k.a.
> > Virtualization Acceleration Code (Unupgradable Units Module). Childish
> > humor aside, "vac" is a unique name in the kernel and hopefully in x86
> > and hardware terminology, is a unique name in the kernel and hopefully
> > in x86 and hardware terminology, e.g. `git grep vac_` yields no hits in
> > the kernel. It also has the same number of characters as "kvm", e.g.
> > the namespace can be modified without needing whitespace adjustment if
> > we want to go that route.
>
> How about the name kvm_base.ko?
>
> And the variable/function name in it can still be kvm_foo (other than
> kvm_base_foo).
My preference is to have a unique name that allows us to differentitate between
the "base" module/code and KVM code. Verbal conversations about all of this get
quite confusing because it's not always clear whether "base KVM" refers to what
is currently kvm.ko, or what would become kvm_base.ko/vac.ko.