CC: [email protected] In-Reply-To: <[email protected]> References: <[email protected]> TO: Sean Christopherson <[email protected]> TO: Paolo Bonzini <[email protected]> CC: Sean Christopherson <[email protected]> CC: Vitaly Kuznetsov <[email protected]> CC: Wanpeng Li <[email protected]> CC: Jim Mattson <[email protected]> CC: Joerg Roedel <[email protected]> CC: [email protected] CC: [email protected] CC: Ben Gardon <[email protected]>
Hi Sean, I love your patch! Perhaps something to improve: [auto build test WARNING on kvm/queue] [also build test WARNING on v5.14-rc5 next-20210812] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Sean-Christopherson/KVM-x86-mmu-Protect-marking-SPs-unsync-when-using-TDP-MMU-with-spinlock/20210813-022023 base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git queue :::::: branch date: 7 hours ago :::::: commit date: 7 hours ago config: i386-randconfig-c001-20210812 (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <[email protected]> Reported-by: Julia Lawall <[email protected]> cocci warnings: (new ones prefixed by >>) >> arch/x86/kvm/mmu/mmu.c:2631:3-12: second lock on line 2631 vim +2631 arch/x86/kvm/mmu/mmu.c 9cf5cf5ad43b29 arch/x86/kvm/mmu.c Xiao Guangrong 2010-05-24 2589 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2590 /* 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2591 * Attempt to unsync any shadow pages that can be reached by the specified gfn, 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2592 * KVM is creating a writable mapping for said gfn. Returns 0 if all pages 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2593 * were marked unsync (or if there is no shadow page), -EPERM if the SPTE must 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2594 * be write-protected. 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2595 */ 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2596 int mmu_try_to_unsync_pages(struct kvm_vcpu *vcpu, gfn_t gfn, bool can_unsync) 4731d4c7a07769 arch/x86/kvm/mmu.c Marcelo Tosatti 2008-09-23 2597 { 5c520e90af3ad5 arch/x86/kvm/mmu.c Xiao Guangrong 2016-02-24 2598 struct kvm_mmu_page *sp; 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2599 bool locked = false; 9cf5cf5ad43b29 arch/x86/kvm/mmu.c Xiao Guangrong 2010-05-24 2600 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2601 /* 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2602 * Force write-protection if the page is being tracked. Note, the page 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2603 * track machinery is used to write-protect upper-level shadow pages, 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2604 * i.e. this guards the role.level == 4K assertion below! 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2605 */ 3d0c27ad6ee465 arch/x86/kvm/mmu.c Xiao Guangrong 2016-02-24 2606 if (kvm_page_track_is_active(vcpu, gfn, KVM_PAGE_TRACK_WRITE)) 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2607 return -EPERM; 3d0c27ad6ee465 arch/x86/kvm/mmu.c Xiao Guangrong 2016-02-24 2608 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2609 /* 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2610 * The page is not write-tracked, mark existing shadow pages unsync 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2611 * unless KVM is synchronizing an unsync SP (can_unsync = false). In 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2612 * that case, KVM must complete emulation of the guest TLB flush before 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2613 * allowing shadow pages to become unsync (writable by the guest). 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2614 */ 5c520e90af3ad5 arch/x86/kvm/mmu.c Xiao Guangrong 2016-02-24 2615 for_each_gfn_indirect_valid_sp(vcpu->kvm, sp, gfn) { 36a2e6774bfb5f arch/x86/kvm/mmu.c Xiao Guangrong 2010-06-30 2616 if (!can_unsync) 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2617 return -EPERM; 36a2e6774bfb5f arch/x86/kvm/mmu.c Xiao Guangrong 2010-06-30 2618 5c520e90af3ad5 arch/x86/kvm/mmu.c Xiao Guangrong 2016-02-24 2619 if (sp->unsync) 5c520e90af3ad5 arch/x86/kvm/mmu.c Xiao Guangrong 2016-02-24 2620 continue; 9cf5cf5ad43b29 arch/x86/kvm/mmu.c Xiao Guangrong 2010-05-24 2621 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2622 /* 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2623 * TDP MMU page faults require an additional spinlock as they 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2624 * run with mmu_lock held for read, not write, and the unsync 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2625 * logic is not thread safe. Take the spinklock regardless of 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2626 * the MMU type to avoid extra conditionals/parameters, there's 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2627 * no meaningful penalty if mmu_lock is held for write. 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2628 */ 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2629 if (!locked) { 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2630 locked = true; 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 @2631 spin_lock(&vcpu->kvm->arch.mmu_unsync_pages_lock); 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2632 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2633 /* 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2634 * Recheck after taking the spinlock, a different vCPU 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2635 * may have since marked the page unsync. A false 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2636 * positive on the unprotected check above is not 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2637 * possible as clearing sp->unsync _must_ hold mmu_lock 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2638 * for write, i.e. unsync cannot transition from 0->1 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2639 * while this CPU holds mmu_lock for read (or write). 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2640 */ 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2641 if (READ_ONCE(sp->unsync)) 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2642 continue; 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2643 } 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2644 3bae0459bcd559 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2020-04-27 2645 WARN_ON(sp->role.level != PG_LEVEL_4K); 5c520e90af3ad5 arch/x86/kvm/mmu.c Xiao Guangrong 2016-02-24 2646 kvm_unsync_page(vcpu, sp); 9cf5cf5ad43b29 arch/x86/kvm/mmu.c Xiao Guangrong 2010-05-24 2647 } 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2648 if (locked) 985bc2aab52bde arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-08-12 2649 spin_unlock(&vcpu->kvm->arch.mmu_unsync_pages_lock); 3d0c27ad6ee465 arch/x86/kvm/mmu.c Xiao Guangrong 2016-02-24 2650 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2651 /* 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2652 * We need to ensure that the marking of unsync pages is visible 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2653 * before the SPTE is updated to allow writes because 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2654 * kvm_mmu_sync_roots() checks the unsync flags without holding 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2655 * the MMU lock and so can race with this. If the SPTE was updated 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2656 * before the page had been marked as unsync-ed, something like the 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2657 * following could happen: 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2658 * 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2659 * CPU 1 CPU 2 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2660 * --------------------------------------------------------------------- 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2661 * 1.2 Host updates SPTE 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2662 * to be writable 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2663 * 2.1 Guest writes a GPTE for GVA X. 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2664 * (GPTE being in the guest page table shadowed 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2665 * by the SP from CPU 1.) 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2666 * This reads SPTE during the page table walk. 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2667 * Since SPTE.W is read as 1, there is no 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2668 * fault. 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2669 * 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2670 * 2.2 Guest issues TLB flush. 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2671 * That causes a VM Exit. 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2672 * 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2673 * 2.3 Walking of unsync pages sees sp->unsync is 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2674 * false and skips the page. 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2675 * 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2676 * 2.4 Guest accesses GVA X. 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2677 * Since the mapping in the SP was not updated, 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2678 * so the old mapping for GVA X incorrectly 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2679 * gets used. 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2680 * 1.1 Host marks SP 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2681 * as unsync 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2682 * (sp->unsync = true) 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2683 * 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2684 * The write barrier below ensures that 1.1 happens before 1.2 and thus 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2685 * the situation in 2.4 does not arise. The implicit barrier in 2.2 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2686 * pairs with this write barrier. 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2687 */ 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2688 smp_wmb(); 578e1c4db22135 arch/x86/kvm/mmu.c Junaid Shahid 2018-06-27 2689 0337f585f57fc8 arch/x86/kvm/mmu/mmu.c Sean Christopherson 2021-06-22 2690 return 0; 4731d4c7a07769 arch/x86/kvm/mmu.c Marcelo Tosatti 2008-09-23 2691 } 4731d4c7a07769 arch/x86/kvm/mmu.c Marcelo Tosatti 2008-09-23 2692 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/[email protected]
.config.gz
Description: application/gzip
_______________________________________________ kbuild mailing list -- [email protected] To unsubscribe send an email to [email protected]
