On Mon, Oct 03, 2022 at 01:02:42PM -0700, David Matlack <dmatlack@xxxxxxxxxx> wrote: > 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@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx> > > > > 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, ...) Oh, yes, you're right. -- Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
