On 11/15/21 17:47, David Woodhouse wrote:
So... a user of this must check the validity after setting its mode to
IN_GUEST_MODE, and the invalidation must make a request and wake any
vCPU(s) which might be using it.
Yes, though the check is implicit in the existing call to
kvm_vcpu_exit_request(vcpu).
I moved the invalidation to the invalidate_range MMU notifier, as
discussed. But that's where the plan falls down a little bit because
IIUC, that one can't sleep at all.
Which is a problem in the existing code, too. It hasn't broken yet
because invalidate_range() is _usually_ called with no spinlocks taken
(the only caller that does call with a spinlock taken seems to be
hugetlb_cow).
Once the dust settles, we need to add non_block_start/end around calls
to ops->invalidate_range.
I need to move it *back* to
invalidate_range_start() where I had it before, if I want to let it
wait for vCPUs to exit. Which means... that the cache 'refresh' call
must wait until the mmu_notifier_count reaches zero? Am I allowed to do > that, and make the "There can be only one waiter" comment in
kvm_mmu_notifier_invalidate_range_end() no longer true?
You can also update the cache while taking the mmu_lock for read, and
retry if mmu_notifier_retry_hva tells you to do so. Looking at the
scenario from commit e649b3f0188 you would have:
(Invalidator) kvm_mmu_notifier_invalidate_range_start()
(Invalidator) write_lock(mmu_lock)
(Invalidator) increment mmu_notifier_count
(Invalidator) write_unlock(mmu_lock)
(Invalidator) request KVM_REQ_APIC_PAGE_RELOAD
(KVM VCPU) vcpu_enter_guest()
(KVM VCPU) kvm_vcpu_reload_apic_access_page()
+ (KVM VCPU) read_lock(mmu_lock)
+ (KVM VCPU) mmu_notifier_retry_hva()
+ (KVM VCPU) read_unlock(mmu_lock)
+ (KVM VCPU) retry! (mmu_notifier_count>1)
(Invalidator) actually unmap page
+ (Invalidator) kvm_mmu_notifier_invalidate_range_end()
+ (Invalidator) write_lock(mmu_lock)
+ (Invalidator) decrement mmu_notifier_count
+ (Invalidator) write_unlock(mmu_lock)
+ (KVM VCPU) vcpu_enter_guest()
+ (KVM VCPU) kvm_vcpu_reload_apic_access_page()
+ (KVM VCPU) mmu_notifier_retry_hva()
Changing mn_memslots_update_rcuwait to a waitq (and renaming it to
mn_invalidate_waitq) is of course also a possibility.
Also, for the small requests: since you are at it, can you add the code
in a new file under virt/kvm/?
Paolo
I was also pondering whether to introduce a new arch-independent
KVM_REQ_GPC_INVALIDATE, or let it be arch-dependent and make it a field
of the cache, so that users can raise whatever requests they like?
