diff mbox series

KVM: x86/mmu: Don't save mmu_invalidate_seq after checking private attr

Message ID 20240528102234.2162763-1-tao1.su@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series KVM: x86/mmu: Don't save mmu_invalidate_seq after checking private attr | expand

Commit Message

Tao Su May 28, 2024, 10:22 a.m. UTC
Drop the second snapshot of mmu_invalidate_seq in kvm_faultin_pfn().
Before checking the mismatch of private vs. shared, mmu_invalidate_seq is
saved to fault->mmu_seq, which can be used to detect an invalidation
related to the gfn occurred, i.e. KVM will not install a mapping in page
table if fault->mmu_seq != mmu_invalidate_seq.

Currently there is a second snapshot of mmu_invalidate_seq, which may not
be same as the first snapshot in kvm_faultin_pfn(), i.e. the gfn attribute
may be changed between the two snapshots, but the gfn may be mapped in
page table without hindrance. Therefore, drop the second snapshot as it
has no obvious benefits.

Fixes: f6adeae81f35 ("KVM: x86/mmu: Handle no-slot faults at the beginning of kvm_faultin_pfn()")
Signed-off-by: Tao Su <tao1.su@linux.intel.com>
---
 arch/x86/kvm/mmu/mmu.c | 3 ---
 1 file changed, 3 deletions(-)


base-commit: 2bfcfd584ff5ccc8bb7acde19b42570414bf880b

Comments

Chao Gao May 29, 2024, 7:21 a.m. UTC | #1
On Tue, May 28, 2024 at 06:22:34PM +0800, Tao Su wrote:
>Drop the second snapshot of mmu_invalidate_seq in kvm_faultin_pfn().
>Before checking the mismatch of private vs. shared, mmu_invalidate_seq is
>saved to fault->mmu_seq, which can be used to detect an invalidation
>related to the gfn occurred, i.e. KVM will not install a mapping in page
>table if fault->mmu_seq != mmu_invalidate_seq.
>
>Currently there is a second snapshot of mmu_invalidate_seq, which may not
>be same as the first snapshot in kvm_faultin_pfn(), i.e. the gfn attribute
>may be changed between the two snapshots, but the gfn may be mapped in
>page table without hindrance. Therefore, drop the second snapshot as it
>has no obvious benefits.
>
>Fixes: f6adeae81f35 ("KVM: x86/mmu: Handle no-slot faults at the beginning of kvm_faultin_pfn()")
>Signed-off-by: Tao Su <tao1.su@linux.intel.com>

Reviewed-by: Chao Gao <chao.gao@intel.com>
Sean Christopherson June 4, 2024, 11:29 p.m. UTC | #2
On Tue, 28 May 2024 18:22:34 +0800, Tao Su wrote:
> Drop the second snapshot of mmu_invalidate_seq in kvm_faultin_pfn().
> Before checking the mismatch of private vs. shared, mmu_invalidate_seq is
> saved to fault->mmu_seq, which can be used to detect an invalidation
> related to the gfn occurred, i.e. KVM will not install a mapping in page
> table if fault->mmu_seq != mmu_invalidate_seq.
> 
> Currently there is a second snapshot of mmu_invalidate_seq, which may not
> be same as the first snapshot in kvm_faultin_pfn(), i.e. the gfn attribute
> may be changed between the two snapshots, but the gfn may be mapped in
> page table without hindrance. Therefore, drop the second snapshot as it
> has no obvious benefits.
> 
> [...]

Applied to kvm-x86 fixes, thanks!

[1/1] KVM: x86/mmu: Don't save mmu_invalidate_seq after checking private attr
      https://github.com/kvm-x86/linux/commit/f66e50ed09b3

--
https://github.com/kvm-x86/linux/tree/next
Paolo Bonzini June 5, 2024, 10:46 a.m. UTC | #3
On 6/5/24 01:29, Sean Christopherson wrote:
> On Tue, 28 May 2024 18:22:34 +0800, Tao Su wrote:
>> Drop the second snapshot of mmu_invalidate_seq in kvm_faultin_pfn().
>> Before checking the mismatch of private vs. shared, mmu_invalidate_seq is
>> saved to fault->mmu_seq, which can be used to detect an invalidation
>> related to the gfn occurred, i.e. KVM will not install a mapping in page
>> table if fault->mmu_seq != mmu_invalidate_seq.
>>
>> Currently there is a second snapshot of mmu_invalidate_seq, which may not
>> be same as the first snapshot in kvm_faultin_pfn(), i.e. the gfn attribute
>> may be changed between the two snapshots, but the gfn may be mapped in
>> page table without hindrance. Therefore, drop the second snapshot as it
>> has no obvious benefits.
>>
>> [...]
> 
> Applied to kvm-x86 fixes, thanks!
> 
> [1/1] KVM: x86/mmu: Don't save mmu_invalidate_seq after checking private attr
>        https://github.com/kvm-x86/linux/commit/f66e50ed09b3

Since I'm already sending a much larger pull request for -rc3, I guess 
you don't mind if I also queue this one. :)

Paolo
Sean Christopherson June 5, 2024, 1:19 p.m. UTC | #4
On Wed, Jun 05, 2024, Paolo Bonzini wrote:
> On 6/5/24 01:29, Sean Christopherson wrote:
> > On Tue, 28 May 2024 18:22:34 +0800, Tao Su wrote:
> > > Drop the second snapshot of mmu_invalidate_seq in kvm_faultin_pfn().
> > > Before checking the mismatch of private vs. shared, mmu_invalidate_seq is
> > > saved to fault->mmu_seq, which can be used to detect an invalidation
> > > related to the gfn occurred, i.e. KVM will not install a mapping in page
> > > table if fault->mmu_seq != mmu_invalidate_seq.
> > > 
> > > Currently there is a second snapshot of mmu_invalidate_seq, which may not
> > > be same as the first snapshot in kvm_faultin_pfn(), i.e. the gfn attribute
> > > may be changed between the two snapshots, but the gfn may be mapped in
> > > page table without hindrance. Therefore, drop the second snapshot as it
> > > has no obvious benefits.
> > > 
> > > [...]
> > 
> > Applied to kvm-x86 fixes, thanks!
> > 
> > [1/1] KVM: x86/mmu: Don't save mmu_invalidate_seq after checking private attr
> >        https://github.com/kvm-x86/linux/commit/f66e50ed09b3
> 
> Since I'm already sending a much larger pull request for -rc3, I guess you
> don't mind if I also queue this one. :)

Not at all, dropped from kvm-x86.
diff mbox series

Patch

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 662f62dfb2aa..4372df109aff 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4400,9 +4400,6 @@  static int kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault,
 			return RET_PF_EMULATE;
 	}
 
-	fault->mmu_seq = vcpu->kvm->mmu_invalidate_seq;
-	smp_rmb();
-
 	/*
 	 * Check for a relevant mmu_notifier invalidation event before getting
 	 * the pfn from the primary MMU, and before acquiring mmu_lock.