Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Tue, 28 Nov 2023 10:10:20 -0800
In-Reply-To: <CAJhGHyBtis3SkNZP8RSX5nKFcnQ4qvUrfTMD2RPc+w+Rzf30Zw@mail.gmail.com>
Mime-Version: 1.0
References: <20231107202002.667900-1-aghulati@google.com> <CAJhGHyBtis3SkNZP8RSX5nKFcnQ4qvUrfTMD2RPc+w+Rzf30Zw@mail.gmail.com>
Message-ID: <ZWYtDGH5p4RpGYBw@google.com>
Subject: Re: [RFC PATCH 00/14] Support multiple KVM modules on the same host
From:   Sean Christopherson <seanjc@google.com>
To:     Lai Jiangshan <jiangshanlai@gmail.com>
Cc:     Anish Ghulati <aghulati@google.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
        hpa@zytor.com, Vitaly Kuznetsov <vkuznets@redhat.com>,
        peterz@infradead.org, paulmck@kernel.org,
        Mark Rutland <mark.rutland@arm.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Nov 17, 2023, Lai Jiangshan wrote:
> On Wed, Nov 8, 2023 at 4:20=E2=80=AFAM Anish Ghulati <aghulati@google.com=
> wrote:
> >
> > This series is a rough, PoC-quality RFC to allow (un)loading and runnin=
g
> > 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 mitigatio=
ns.
> >
> > 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
>=20
> Hello Anish
>=20
> Scarce effort on multi-KVM can be seen in the mail list albeit many
> companies enable multi-KVM internally.
>=20
> I'm glad that you took a big step in upstreaming it.  And I hope it
> can be materialized soon.
>=20
>=20
> >
> >  - 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 instanc=
e.
>=20
> perf_register_guest_info_callbacks() also accesses to system-wide resourc=
es,
> 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 si=
ngle
callback with perf, and then have VAC deal with invoking the callback(s) fo=
r 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.k=
o.
>=20
> 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.
>=20
> Since the symbols in the new module are invisible outside, I recommend:
> new kvm_intel.ko =3D kvm_intel.ko + kvm.ko
> new kvm_amd.ko =3D kvm_amd.ko + kvm.ko

Yeah, Paolo also suggested this at LPC.

> >  - Add a Kconfig string to allow defining a device and module postfix a=
t
> >    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.
>=20
> How about the name kvm_base.ko?
>=20
> 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 bet=
ween
the "base" module/code and KVM code.  Verbal conversations about all of thi=
s 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.