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>
