Re: [PATCH v3 01/10] KVM: x86/mmu: Change tdp_mmu to a read-only parameter

[David Matlack <dmatlack@google.com>]

On Mon, Oct 03, 2022 at 11:58:34AM -0700, Isaku Yamahata wrote:
> On Tue, Sep 27, 2022 at 09:10:43PM +0000, "Huang, Kai" <kai.huang@intel.com> wrote:
> > On Tue, 2022-09-27 at 09:14 -0700, David Matlack wrote:
> > > On Tue, Sep 27, 2022 at 2:19 AM Huang, Kai <kai.huang@intel.com> wrote:
> > > > 
> > > > Sorry last time I didn't review deeply, but I am wondering why do we need
> > > > 'tdp_mmu_allowed' at all?  The purpose of having 'allow_mmio_caching' is because
> > > > kvm_mmu_set_mmio_spte_mask() is called twice, and 'enable_mmio_caching' can be
> > > > disabled in the first call, so it can be against user's desire in the second
> > > > call.  However it appears for 'tdp_mmu_enabled' we don't need 'tdp_mmu_allowed',
> > > > as kvm_configure_mmu() is only called once by VMX or SVM, if I read correctly.
> > > 
> > > tdp_mmu_allowed is needed because kvm_intel and kvm_amd are separate
> > > modules from kvm. So kvm_configure_mmu() can be called multiple times
> > > (each time kvm_intel or kvm_amd is loaded).
> > > 
> > > 
> > 
> > Indeed. :)
> > 
> > Reviewed-by: Kai Huang <kai.huang@intel.com>
> 
> kvm_arch_init() which is called early during the module initialization before
> kvm_configure_mmu() via kvm_arch_hardware_setup() checks if the vendor module
> (kvm_intel or kvm_amd) was already loaded.  If yes, it results in -EEXIST.
> 
> So kvm_configure_mmu() won't be called twice.

kvm_configure_mmu() can be called multiple times if the vendor module is
unloaded without unloading the kvm module. For example:

 $ modprobe kvm
 $ modprobe kvm_intel ept=Y  # kvm_configure_mmu(true, ...)
 $ modprobe -r kvm_intel
 $ modprobe kvm_intel ept=N  # kvm_configure_mmu(false, ...)