From: Paolo Bonzini <pbonzini@redhat.com>
To: Ben Gardon <bgardon@google.com>, KVM list <kvm@vger.kernel.org>
Subject: Re: [PATCH v2 18/28] KVM: x86/mmu: Use an rwlock for the x86 MMU
Date: Wed, 3 Feb 2021 19:14:17 +0100 [thread overview]
Message-ID: <106f4dff-703e-762e-d923-909d66b8fd0b@redhat.com> (raw)
In-Reply-To: <CANgfPd_RxhBwM95MQQmGOdtmeH8c6=zPqUnXXHNV5Ta0R5R=iw@mail.gmail.com>
[Sent offlist by mistake]
On 03/02/21 19:03, Ben Gardon wrote:
> On Wed, Feb 3, 2021 at 3:08 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 02/02/21 19:57, Ben Gardon wrote:
>>> Add a read / write lock to be used in place of the MMU spinlock on x86.
>>> The rwlock will enable the TDP MMU to handle page faults, and other
>>> operations in parallel in future commits.
>>>
>>> Reviewed-by: Peter Feiner <pfeiner@google.com>
>>> Signed-off-by: Ben Gardon <bgardon@google.com>
>>>
>>> ---
>>>
>>> v1 -> v2
>>> - Removed MMU lock wrappers
>>> - Completely replaced the MMU spinlock with an rwlock for x86
>>>
>>> arch/x86/include/asm/kvm_host.h | 2 +
>>> arch/x86/kvm/mmu/mmu.c | 90 ++++++++++++++++-----------------
>>> arch/x86/kvm/mmu/page_track.c | 8 +--
>>> arch/x86/kvm/mmu/paging_tmpl.h | 8 +--
>>> arch/x86/kvm/mmu/tdp_mmu.c | 20 ++++----
>>> arch/x86/kvm/x86.c | 4 +-
>>> include/linux/kvm_host.h | 5 ++
>>> virt/kvm/dirty_ring.c | 10 ++++
>>> virt/kvm/kvm_main.c | 46 +++++++++++------
>>> 9 files changed, 112 insertions(+), 81 deletions(-)
>>
>> Let's create a new header file, to abstract this even more.
>>
>> diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c
>> index 70ba572f6e5c..3eeb8c0e9590 100644
>> --- a/virt/kvm/dirty_ring.c
>> +++ b/virt/kvm/dirty_ring.c
>> @@ -9,6 +9,7 @@
>> #include <linux/vmalloc.h>
>> #include <linux/kvm_dirty_ring.h>
>> #include <trace/events/kvm.h>
>> +#include "mmu_lock.h"
>>
>> int __weak kvm_cpu_dirty_log_size(void)
>> {
>> @@ -60,19 +61,9 @@ static void kvm_reset_dirty_gfn(struct kvm *kvm, u32
>> slot, u64 offset, u64 mask)
>> if (!memslot || (offset + __fls(mask)) >= memslot->npages)
>> return;
>>
>> -#ifdef KVM_HAVE_MMU_RWLOCK
>> - write_lock(&kvm->mmu_lock);
>> -#else
>> - spin_lock(&kvm->mmu_lock);
>> -#endif /* KVM_HAVE_MMU_RWLOCK */
>> -
>> + KVM_MMU_LOCK(kvm);
>> kvm_arch_mmu_enable_log_dirty_pt_masked(kvm, memslot, offset, mask);
>> -
>> -#ifdef KVM_HAVE_MMU_RWLOCK
>> - write_unlock(&kvm->mmu_lock);
>> -#else
>> - spin_unlock(&kvm->mmu_lock);
>> -#endif /* KVM_HAVE_MMU_RWLOCK */
>> + KVM_MMU_UNLOCK(kvm);
>> }
>>
>> int kvm_dirty_ring_alloc(struct kvm_dirty_ring *ring, int index, u32 size)
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index 6f0c1473b474..356068103f8a 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -58,6 +58,7 @@
>>
>> #include "coalesced_mmio.h"
>> #include "async_pf.h"
>> +#include "mmu_lock.h"
>> #include "vfio.h"
>>
>> #define CREATE_TRACE_POINTS
>> @@ -450,14 +451,6 @@ static void
>> kvm_mmu_notifier_invalidate_range(struct mmu_notifier *mn,
>> srcu_read_unlock(&kvm->srcu, idx);
>> }
>>
>> -#ifdef KVM_HAVE_MMU_RWLOCK
>> -#define KVM_MMU_LOCK(kvm) write_lock(&kvm->mmu_lock)
>> -#define KVM_MMU_UNLOCK(kvm) write_unlock(&kvm->mmu_lock)
>> -#else
>> -#define KVM_MMU_LOCK(kvm) spin_lock(&kvm->mmu_lock)
>> -#define KVM_MMU_UNLOCK(kvm) spin_unlock(&kvm->mmu_lock)
>> -#endif /* KVM_HAVE_MMU_RWLOCK */
>> -
>> static void kvm_mmu_notifier_change_pte(struct mmu_notifier *mn,
>> struct mm_struct *mm,
>> unsigned long address,
>> @@ -755,11 +748,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
>> if (!kvm)
>> return ERR_PTR(-ENOMEM);
>>
>> -#ifdef KVM_HAVE_MMU_RWLOCK
>> - rwlock_init(&kvm->mmu_lock);
>> -#else
>> - spin_lock_init(&kvm->mmu_lock);
>> -#endif /* KVM_HAVE_MMU_RWLOCK */
>> + KVM_MMU_LOCK_INIT(kvm);
>> mmgrab(current->mm);
>> kvm->mm = current->mm;
>> kvm_eventfd_init(kvm);
>> diff --git a/virt/kvm/mmu_lock.h b/virt/kvm/mmu_lock.h
>> new file mode 100644
>> index 000000000000..1dd1ca2cdc77
>> --- /dev/null
>> +++ b/virt/kvm/mmu_lock.h
>> @@ -0,0 +1,23 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +#ifndef KVM_MMU_LOCK_H
>> +#define KVM_MMU_LOCK_H 1
>> +
>> +/*
>> + * Architectures can choose whether to use an rwlock or spinlock
>> + * for the mmu_lock. These macros, for use in common code
>> + * only, avoids using #ifdefs in places that must deal with
>> + * multiple architectures.
>> + */
>> +
>> +#ifdef KVM_HAVE_MMU_RWLOCK
>> +#define KVM_MMU_LOCK_INIT(kvm) rwlock_init(&(kvm)->mmu_lock)
>> +#define KVM_MMU_LOCK(kvm) write_lock(&(kvm)->mmu_lock)
>> +#define KVM_MMU_UNLOCK(kvm) write_unlock(&(kvm)->mmu_lock)
>> +#else
>> +#define KVM_MMU_LOCK_INIT(kvm) spin_lock_init(&(kvm)->mmu_lock)
>> +#define KVM_MMU_LOCK(kvm) spin_lock(&(kvm)->mmu_lock)
>> +#define KVM_MMU_UNLOCK(kvm) spin_unlock(&(kvm)->mmu_lock)
>> +#endif /* KVM_HAVE_MMU_RWLOCK */
>> +
>> +#endif
>>
>
> That sounds good to me. I don't know if you meant to send that
> off-list, but I'm happy to make that change in a v3.
No, I didn't. At this point I'm crossing fingers that there's no v3
(except for the couple patches at the end).
next prev parent reply other threads:[~2021-02-03 18:16 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-02 18:57 [PATCH v2 00/28] Allow parallel MMU operations with TDP MMU Ben Gardon
2021-02-02 18:57 ` [PATCH v2 01/28] KVM: x86/mmu: change TDP MMU yield function returns to match cond_resched Ben Gardon
2021-02-02 18:57 ` [PATCH v2 02/28] KVM: x86/mmu: Add comment on __tdp_mmu_set_spte Ben Gardon
2021-02-02 18:57 ` [PATCH v2 03/28] KVM: x86/mmu: Add lockdep when setting a TDP MMU SPTE Ben Gardon
2021-02-02 18:57 ` [PATCH v2 04/28] KVM: x86/mmu: Don't redundantly clear TDP MMU pt memory Ben Gardon
2021-02-02 18:57 ` [PATCH v2 05/28] KVM: x86/mmu: Factor out handling of removed page tables Ben Gardon
2021-02-02 18:57 ` [PATCH v2 06/28] locking/rwlocks: Add contention detection for rwlocks Ben Gardon
2021-02-09 20:39 ` Guenter Roeck
2021-02-09 21:46 ` Waiman Long
2021-02-09 22:25 ` Guenter Roeck
2021-02-10 0:27 ` Waiman Long
2021-02-10 0:41 ` Waiman Long
2021-02-10 6:04 ` Guenter Roeck
2021-02-10 14:57 ` Waiman Long
2021-02-10 3:32 ` Waiman Long
2021-02-10 15:15 ` Waiman Long
2021-02-02 18:57 ` [PATCH v2 07/28] sched: Add needbreak " Ben Gardon
2021-02-02 18:57 ` [PATCH v2 08/28] sched: Add cond_resched_rwlock Ben Gardon
2021-02-02 18:57 ` [PATCH v2 09/28] KVM: x86/mmu: Fix braces in kvm_recover_nx_lpages Ben Gardon
2021-02-02 18:57 ` [PATCH v2 10/28] KVM: x86/mmu: Fix TDP MMU zap collapsible SPTEs Ben Gardon
2021-02-03 9:43 ` Paolo Bonzini
2021-02-02 18:57 ` [PATCH v2 11/28] KVM: x86/mmu: Merge flush and non-flush tdp_mmu_iter_cond_resched Ben Gardon
2021-02-02 18:57 ` [PATCH v2 12/28] KVM: x86/mmu: Rename goal_gfn to next_last_level_gfn Ben Gardon
2021-02-02 18:57 ` [PATCH v2 13/28] KVM: x86/mmu: Ensure forward progress when yielding in TDP MMU iter Ben Gardon
2021-02-05 23:42 ` Sean Christopherson
2021-02-02 18:57 ` [PATCH v2 14/28] KVM: x86/mmu: Yield in TDU MMU iter even if no SPTES changed Ben Gardon
2021-02-02 18:57 ` [PATCH v2 15/28] KVM: x86/mmu: Skip no-op changes in TDP MMU functions Ben Gardon
2021-02-02 18:57 ` [PATCH v2 16/28] KVM: x86/mmu: Clear dirtied pages mask bit before early break Ben Gardon
2021-02-02 18:57 ` [PATCH v2 17/28] KVM: x86/mmu: Protect TDP MMU page table memory with RCU Ben Gardon
2021-02-02 18:57 ` [PATCH v2 18/28] KVM: x86/mmu: Use an rwlock for the x86 MMU Ben Gardon
[not found] ` <c8aa8f9c-2305-5d58-3b48-261663524ad5@redhat.com>
[not found] ` <CANgfPd_RxhBwM95MQQmGOdtmeH8c6=zPqUnXXHNV5Ta0R5R=iw@mail.gmail.com>
2021-02-03 18:14 ` Paolo Bonzini [this message]
2021-02-02 18:57 ` [PATCH v2 19/28] KVM: x86/mmu: Factor out functions to add/remove TDP MMU pages Ben Gardon
2021-02-02 18:57 ` [PATCH v2 20/28] KVM: x86/mmu: Use atomic ops to set SPTEs in TDP MMU map Ben Gardon
2021-02-03 2:48 ` kernel test robot
2021-02-03 11:14 ` Paolo Bonzini
2021-02-06 0:26 ` Sean Christopherson
2021-02-08 10:32 ` Paolo Bonzini
2021-04-01 10:32 ` Paolo Bonzini
2021-04-01 16:50 ` Ben Gardon
2021-04-01 17:32 ` Paolo Bonzini
2021-04-01 18:09 ` Sean Christopherson
2021-02-02 18:57 ` [PATCH v2 21/28] KVM: x86/mmu: Flush TLBs after zap in TDP MMU PF handler Ben Gardon
2021-02-06 0:29 ` Sean Christopherson
2021-02-02 18:57 ` [PATCH v2 22/28] KVM: x86/mmu: Mark SPTEs in disconnected pages as removed Ben Gardon
2021-02-03 11:17 ` Paolo Bonzini
2021-02-02 18:57 ` [PATCH v2 23/28] KVM: x86/mmu: Allow parallel page faults for the TDP MMU Ben Gardon
2021-02-03 12:39 ` Paolo Bonzini
2021-02-03 17:46 ` Ben Gardon
2021-02-03 18:30 ` Paolo Bonzini
2021-02-06 0:12 ` Sean Christopherson
2021-02-02 18:57 ` [PATCH v2 24/28] KVM: x86/mmu: Allow zap gfn range to operate under the mmu read lock Ben Gardon
2021-02-03 11:25 ` Paolo Bonzini
2021-02-03 11:26 ` Paolo Bonzini
2021-02-03 18:31 ` Ben Gardon
2021-02-03 18:32 ` Paolo Bonzini
2021-02-02 18:57 ` [PATCH v2 25/28] KVM: x86/mmu: Allow zapping collapsible SPTEs to use MMU " Ben Gardon
2021-02-03 11:34 ` Paolo Bonzini
2021-02-03 18:51 ` Ben Gardon
2021-02-02 18:57 ` [PATCH v2 26/28] KVM: x86/mmu: Allow enabling / disabling dirty logging under " Ben Gardon
2021-02-03 11:38 ` Paolo Bonzini
2021-02-02 18:57 ` [PATCH v2 27/28] KVM: selftests: Add backing src parameter to dirty_log_perf_test Ben Gardon
2021-02-02 18:57 ` [PATCH v2 28/28] KVM: selftests: Disable dirty logging with vCPUs running Ben Gardon
2021-02-03 10:07 ` Paolo Bonzini
2021-02-03 11:00 ` [PATCH v2 00/28] Allow parallel MMU operations with TDP MMU Paolo Bonzini
2021-02-03 17:54 ` Sean Christopherson
2021-02-03 18:13 ` Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=106f4dff-703e-762e-d923-909d66b8fd0b@redhat.com \
--to=pbonzini@redhat.com \
--cc=bgardon@google.com \
--cc=kvm@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).