Re: [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits

[kernel test robot <lkp@intel.com>]

Hi Sean,

I love your patch! Perhaps something to improve:

[auto build test WARNING on 26291c54e111ff6ba87a164d85d4a4e134b7315c]

url:    https://github.com/0day-ci/linux/commits/Sean-Christopherson/x86-uaccess-CMPXCHG-KVM-bug-fixes/20220201-091001
base:   26291c54e111ff6ba87a164d85d4a4e134b7315c
config: x86_64-rhel-8.3-kselftests (https://download.01.org/0day-ci/archive/20220201/202202012104.eSvVUhWh-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/c880d7a9df876f20dc3acdd893c5c71f3cda5029
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sean-Christopherson/x86-uaccess-CMPXCHG-KVM-bug-fixes/20220201-091001
        git checkout c880d7a9df876f20dc3acdd893c5c71f3cda5029
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   arch/x86/kvm/mmu/mmu.c:695:9: sparse: sparse: context imbalance in 'walk_shadow_page_lockless_begin' - different lock contexts for basic block
   arch/x86/kvm/mmu/mmu.c: note: in included file (through include/linux/rbtree.h, include/linux/mm_types.h, arch/x86/kvm/irq.h):
   include/linux/rcupdate.h:725:9: sparse: sparse: context imbalance in 'walk_shadow_page_lockless_end' - unexpected unlock
   arch/x86/kvm/mmu/mmu.c:2595:9: sparse: sparse: context imbalance in 'mmu_try_to_unsync_pages' - different lock contexts for basic block
   arch/x86/kvm/mmu/mmu.c: note: in included file:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
   arch/x86/kvm/mmu/mmu.c: note: in included file:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
   arch/x86/kvm/mmu/mmu.c: note: in included file:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
   arch/x86/kvm/mmu/mmu.c:4549:57: sparse: sparse: cast truncates bits from constant value (ffffff33 becomes 33)
   arch/x86/kvm/mmu/mmu.c:4551:56: sparse: sparse: cast truncates bits from constant value (ffffff0f becomes f)
   arch/x86/kvm/mmu/mmu.c:4553:57: sparse: sparse: cast truncates bits from constant value (ffffff55 becomes 55)

vim +244 arch/x86/kvm/mmu/paging_tmpl.h

   191	
   192	static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
   193						     struct kvm_mmu *mmu,
   194						     struct guest_walker *walker,
   195						     gpa_t addr, int write_fault)
   196	{
   197		unsigned level, index;
   198		pt_element_t pte, orig_pte;
   199		pt_element_t __user *ptep_user;
   200		gfn_t table_gfn;
   201		int ret;
   202	
   203		/* dirty/accessed bits are not supported, so no need to update them */
   204		if (!PT_HAVE_ACCESSED_DIRTY(mmu))
   205			return 0;
   206	
   207		for (level = walker->max_level; level >= walker->level; --level) {
   208			pte = orig_pte = walker->ptes[level - 1];
   209			table_gfn = walker->table_gfn[level - 1];
   210			ptep_user = walker->ptep_user[level - 1];
   211			index = offset_in_page(ptep_user) / sizeof(pt_element_t);
   212			if (!(pte & PT_GUEST_ACCESSED_MASK)) {
   213				trace_kvm_mmu_set_accessed_bit(table_gfn, index, sizeof(pte));
   214				pte |= PT_GUEST_ACCESSED_MASK;
   215			}
   216			if (level == walker->level && write_fault &&
   217					!(pte & PT_GUEST_DIRTY_MASK)) {
   218				trace_kvm_mmu_set_dirty_bit(table_gfn, index, sizeof(pte));
   219	#if PTTYPE == PTTYPE_EPT
   220				if (kvm_x86_ops.nested_ops->write_log_dirty(vcpu, addr))
   221					return -EINVAL;
   222	#endif
   223				pte |= PT_GUEST_DIRTY_MASK;
   224			}
   225			if (pte == orig_pte)
   226				continue;
   227	
   228			/*
   229			 * If the slot is read-only, simply do not process the accessed
   230			 * and dirty bits.  This is the correct thing to do if the slot
   231			 * is ROM, and page tables in read-as-ROM/write-as-MMIO slots
   232			 * are only supported if the accessed and dirty bits are already
   233			 * set in the ROM (so that MMIO writes are never needed).
   234			 *
   235			 * Note that NPT does not allow this at all and faults, since
   236			 * it always wants nested page table entries for the guest
   237			 * page tables to be writable.  And EPT works but will simply
   238			 * overwrite the read-only memory to set the accessed and dirty
   239			 * bits.
   240			 */
   241			if (unlikely(!walker->pte_writable[level - 1]))
   242				continue;
   243	
 > 244			ret = __try_cmpxchg_user(ptep_user, &orig_pte, pte, fault);
   245			if (ret)
   246				return ret;
   247	
   248			kvm_vcpu_mark_page_dirty(vcpu, table_gfn);
   249			walker->ptes[level - 1] = pte;
   250		}
   251		return 0;
   252	}
   253	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org