On 09/02/21 04:33, Yu Zhang wrote:
On Mon, Feb 08, 2021 at 05:47:22PM +0100, Paolo Bonzini wrote:
On 08/02/21 14:49, Yu Zhang wrote:
On Mon, Feb 08, 2021 at 12:36:57PM +0100, Paolo Bonzini wrote:
On 07/02/21 13:22, Yu Zhang wrote:
In shadow page table, only leaf SPs may be marked as unsync.
And for non-leaf SPs, we use unsync_children to keep the number
of the unsynced children. In kvm_mmu_sync_root(), sp->unsync
shall always be zero for the root SP, , hence no need to check
it. Instead, a warning inside mmu_sync_children() is added, in
case someone incorrectly used it.
Also, clarify the mmu_need_write_protect(), by moving the warning
into kvm_unsync_page().
Signed-off-by: Yu Zhang <yu.c.zhang@xxxxxxxxxxxxxxx>
Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
This should really be more of a Co-developed-by, and there are a couple
adjustments that could be made in the commit message. I've queued the patch
and I'll fix it up later.
Indeed. Thanks for the remind, and I'll pay attention in the future. :)
Also:
arch/x86/kvm/mmu/mmu.c: In function ‘mmu_sync_children’:
arch/x86/kvm/mmu/mmu.c:2002:17: error: ‘sp’ is used uninitialized in this
function [-Werror=uninitialized]
WARN_ON_ONCE(sp->unsync);
Oops. This is wrong. Should be WARN_ON_ONCE(parent->unsync);
so how was this tested?
I ran access test in kvm-unit-test for previous version, which hasn't
this code(also in my local repo "enable_ept" was explicitly set to
0 in order to test the shadow mode). But I did not test this one. I'm
truely sorry for the negligence - even trying to compile should make
this happen!
Should we submit another version? Any suggestions on the test cases?
Yes, please send v3.
The commit message can be:
In shadow page table, only leaf SPs may be marked as unsync; instead,
for non-leaf SPs, we store the number of unsynced children in
unsync_children. Therefore, in kvm_mmu_sync_root(), sp->unsync
shall always be zero for the root SP and there is no need to check
it. Remove the check, and add a warning inside mmu_sync_children() to
assert that the flags are used properly.
While at it, move the warning from mmu_need_write_protect() to
kvm_unsync_page().
Paolo
