Re: [PATCH 2/9] KVM: x86: Fix recording of guest steal time / preempted status

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Oct 14, 2022 at 10:26:36PM +0000, Rishabh Bhatnagar wrote:
> From: David Woodhouse <dwmw2@xxxxxxxxxxxxx>
> 
> commit 7e2175ebd695f17860c5bd4ad7616cce12ed4591 upstream.
> 
> In commit b043138246a4 ("x86/KVM: Make sure KVM_VCPU_FLUSH_TLB flag is
> not missed") we switched to using a gfn_to_pfn_cache for accessing the
> guest steal time structure in order to allow for an atomic xchg of the
> preempted field. This has a couple of problems.
> 
> Firstly, kvm_map_gfn() doesn't work at all for IOMEM pages when the
> atomic flag is set, which it is in kvm_steal_time_set_preempted(). So a
> guest vCPU using an IOMEM page for its steal time would never have its
> preempted field set.
> 
> Secondly, the gfn_to_pfn_cache is not invalidated in all cases where it
> should have been. There are two stages to the GFN->PFN conversion;
> first the GFN is converted to a userspace HVA, and then that HVA is
> looked up in the process page tables to find the underlying host PFN.
> Correct invalidation of the latter would require being hooked up to the
> MMU notifiers, but that doesn't happen---so it just keeps mapping and
> unmapping the *wrong* PFN after the userspace page tables change.
> 
> In the !IOMEM case at least the stale page *is* pinned all the time it's
> cached, so it won't be freed and reused by anyone else while still
> receiving the steal time updates. The map/unmap dance only takes care
> of the KVM administrivia such as marking the page dirty.
> 
> Until the gfn_to_pfn cache handles the remapping automatically by
> integrating with the MMU notifiers, we might as well not get a
> kernel mapping of it, and use the perfectly serviceable userspace HVA
> that we already have.  We just need to implement the atomic xchg on
> the userspace address with appropriate exception handling, which is
> fairly trivial.
> 
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: b043138246a4 ("x86/KVM: Make sure KVM_VCPU_FLUSH_TLB flag is not missed")
> Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
> Message-Id: <3645b9b889dac6438394194bb5586a46b68d581f.camel@xxxxxxxxxxxxx>
> [I didn't entirely agree with David's assessment of the
>  usefulness of the gfn_to_pfn cache, and integrated the outcome
>  of the discussion in the above commit message. - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> [risbhat@xxxxxxxxxx: Use the older mark_page_dirty_in_slot api without
> kvm argument]
> Signed-off-by: Rishabh Bhatnagar <risbhat@xxxxxxxxxx>
> ---
>  arch/x86/include/asm/kvm_host.h |   2 +-
>  arch/x86/kvm/x86.c              | 105 +++++++++++++++++++++++---------
>  2 files changed, 76 insertions(+), 31 deletions(-)

This is already in some stable kernels, why send it again and what
tree(s) are you asking for it to be applied to?

Same for the other backports you sent.

confused,

greg k-h



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux