On Thu, Sep 01, 2022 at 02:12:56PM +0000, Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > On Thu, Sep 01, 2022, Chao Gao wrote: > > On Tue, Aug 30, 2022 at 05:01:16AM -0700, isaku.yamahata@xxxxxxxxx wrote: > > >From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx> > > > > > >KVM/X86 uses user return notifier to switch MSR for guest or user space. > > >Snapshot host values on CPU online, change MSR values for guest, and > > >restore them on returning to user space. The current code abuses > > >kvm_arch_hardware_enable() which is called on kvm module initialization or > > >CPU online. > > > > > >Remove such the abuse of kvm_arch_hardware_enable by capturing the host > > >value on the first change of the MSR value to guest VM instead of CPU > > >online. > > > > > >Suggested-by: Sean Christopherson <seanjc@xxxxxxxxxx> > > >Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx> > > >--- > > > arch/x86/kvm/x86.c | 43 ++++++++++++++++++++++++------------------- > > > 1 file changed, 24 insertions(+), 19 deletions(-) > > > > > >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > >index 205ebdc2b11b..16104a2f7d8e 100644 > > >--- a/arch/x86/kvm/x86.c > > >+++ b/arch/x86/kvm/x86.c > > >@@ -200,6 +200,7 @@ struct kvm_user_return_msrs { > > > struct kvm_user_return_msr_values { > > > u64 host; > > > u64 curr; > > >+ bool initialized; > > > } values[KVM_MAX_NR_USER_RETURN_MSRS]; > > > > The benefit of having an "initialized" state for each user return MSR on > > each CPU is small. A per-cpu state looks suffice. With it, you can keep > > kvm_user_return_msr_cpu_online() and simply call the function from > > kvm_set_user_return_msr() if initialized is false on current CPU. > > Yep, a per-CPU flag is I intended. This is the completely untested patch that's > sitting in a development branch of mine. With the following fix, it worked. I'll replace this patch with yours. diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 205ebdc2b11b..0e200fe44b35 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -9212,7 +9217,12 @@ int kvm_arch_init(void *opaque) return -ENOMEM; } - user_return_msrs = alloc_percpu(struct kvm_user_return_msrs); + /* + * __GFP_ZERO to ensure user_return_msrs.values[].initialized = false. + * See kvm_user_return_msr_init_cpu(). + */ + user_return_msrs = alloc_percpu_gfp(struct kvm_user_return_msrs, + GFP_KERNEL | __GFP_ZERO); if (!user_return_msrs) { printk(KERN_ERR "kvm: failed to allocate percpu kvm_user_return_msrs\n"); r = -ENOMEM; -- Isaku Yamahata <isaku.yamahata@xxxxxxxxx>