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

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

[Tao Su]

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(-)

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.

base-commit: 2bfcfd584ff5ccc8bb7acde19b42570414bf880b
-- 
2.34.1

Re: [PATCH] KVM: x86/mmu: Don't save mmu_invalidate_seq after checking private attr

[Chao Gao]

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>

Re: [PATCH] KVM: x86/mmu: Don't save mmu_invalidate_seq after checking private attr

[Sean Christopherson]

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

Re: [PATCH] KVM: x86/mmu: Don't save mmu_invalidate_seq after checking private attr

[Paolo Bonzini]

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

Re: [PATCH] KVM: x86/mmu: Don't save mmu_invalidate_seq after checking private attr

[Sean Christopherson]

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.