On 09/03/2016 08:18, Lan Tianyu wrote:
How about the following comments.
Log for kvm_mmu_commit_zap_page()
* We need to make sure everyone sees our modifications to
* the page tables and see changes to vcpu->mode here.
Please mention that this pairs with vcpu_enter_guest and
* barrier in the kvm_flush_remote_tlbs() helps us to achieve
* these. Otherwise, wait for all vcpus to exit guest mode
* and/or lockless shadow page table walks.
The rest of the comment is okay, but please replace "Otherwise" with "In
addition, we need to".
Log for kvm_flush_remote_tlbs()
* We want to publish modifications to the page tables before
* reading mode. Pairs with a memory barrier in arch-specific
* - x86: smp_mb__after_srcu_read_unlock in vcpu_enter_guest.
... and smp_mb in walk_shadow_page_lockless_begin/end.
* - powerpc: smp_mb in kvmppc_prepare_to_enter.
The comment looks good, but the smp_mb__before_atomic() is not needed.
As mentioned in the reply to Guangrong, only a smp_load_acquire is required.
So the comment should say something like "There is already an smp_mb()
before kvm_make_all_cpus_request reads vcpu->mode. We reuse that
On top of this there is:
- the change to paging_tmpl.h that Guangrong posted, adding smp_wmb()
before each increment of vcpu->kvm->tlbs_dirty
- the change to smp_mb__after_atomic() in kvm_make_all_cpus_request
- if you want :) you can also replace the store+mb in
walk_shadow_page_lockless_begin with smp_store_mb, and the mb+store in
walk_shadow_page_lockless_end with smp_store_release.