Re: [PATCH v4 10/11] KVM: x86/xen: Add KVM_IRQ_ROUTING_XEN_EVTCHN and event channel delivery

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

[-- Attachment #1: Type: text/plain, Size: 7743 bytes --]

Hi David,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on next-20211118]
[cannot apply to kvm/queue kvms390/next powerpc/topic/ppc-kvm kvmarm/next mst-vhost/linux-next v5.16-rc1]
[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/David-Woodhouse/KVM-Introduce-CONFIG_HAVE_KVM_DIRTY_RING/20211120-192837
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a90af8f15bdc9449ee2d24e1d73fa3f7e8633f81
config: i386-randconfig-s001-20211118 (attached as .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/a9a90c7ab5f10064f2153f60e2410222c1b00700
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review David-Woodhouse/KVM-Introduce-CONFIG_HAVE_KVM_DIRTY_RING/20211120-192837
        git checkout a9a90c7ab5f10064f2153f60e2410222c1b00700
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 

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/xen.c:272:22: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const [noderef] __user *ptr @@     got void * @@
   arch/x86/kvm/xen.c:272:22: sparse:     expected void const [noderef] __user *ptr
   arch/x86/kvm/xen.c:272:22: sparse:     got void *
>> arch/x86/kvm/xen.c:276:56: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct vcpu_info [noderef] __user *vi @@     got void * @@
   arch/x86/kvm/xen.c:276:56: sparse:     expected struct vcpu_info [noderef] __user *vi
   arch/x86/kvm/xen.c:276:56: sparse:     got void *
>> arch/x86/kvm/xen.c:294:63: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct compat_vcpu_info [noderef] __user *vi @@     got void * @@
   arch/x86/kvm/xen.c:294:63: sparse:     expected struct compat_vcpu_info [noderef] __user *vi
   arch/x86/kvm/xen.c:294:63: sparse:     got void *

vim +272 arch/x86/kvm/xen.c

   196	
   197	int __kvm_xen_has_interrupt(struct kvm_vcpu *v)
   198	{
   199		unsigned long evtchn_pending_sel = READ_ONCE(v->arch.xen.evtchn_pending_sel);
   200		bool atomic = in_atomic() || !task_is_running(current);
   201		int err;
   202		u8 rc = 0;
   203	
   204		/*
   205		 * If the global upcall vector (HVMIRQ_callback_vector) is set and
   206		 * the vCPU's evtchn_upcall_pending flag is set, the IRQ is pending.
   207		 */
   208		struct gfn_to_hva_cache *ghc = &v->arch.xen.vcpu_info_cache;
   209		struct kvm_memslots *slots = kvm_memslots(v->kvm);
   210		bool ghc_valid = slots->generation == ghc->generation &&
   211			!kvm_is_error_hva(ghc->hva) && ghc->memslot;
   212	
   213		unsigned int offset = offsetof(struct vcpu_info, evtchn_upcall_pending);
   214	
   215		/* No need for compat handling here */
   216		BUILD_BUG_ON(offsetof(struct vcpu_info, evtchn_upcall_pending) !=
   217			     offsetof(struct compat_vcpu_info, evtchn_upcall_pending));
   218		BUILD_BUG_ON(sizeof(rc) !=
   219			     sizeof_field(struct vcpu_info, evtchn_upcall_pending));
   220		BUILD_BUG_ON(sizeof(rc) !=
   221			     sizeof_field(struct compat_vcpu_info, evtchn_upcall_pending));
   222	
   223		/*
   224		 * For efficiency, this mirrors the checks for using the valid
   225		 * cache in kvm_read_guest_offset_cached(), but just uses
   226		 * __get_user() instead. And falls back to the slow path.
   227		 */
   228		if (!evtchn_pending_sel && ghc_valid) {
   229			/* Fast path */
   230			pagefault_disable();
   231			err = __get_user(rc, (u8 __user *)ghc->hva + offset);
   232			pagefault_enable();
   233			if (!err)
   234				return rc;
   235		}
   236	
   237		/* Slow path */
   238	
   239		/*
   240		 * This function gets called from kvm_vcpu_block() after setting the
   241		 * task to TASK_INTERRUPTIBLE, to see if it needs to wake immediately
   242		 * from a HLT. So we really mustn't sleep. If the page ended up absent
   243		 * at that point, just return 1 in order to trigger an immediate wake,
   244		 * and we'll end up getting called again from a context where we *can*
   245		 * fault in the page and wait for it.
   246		 */
   247		if (atomic)
   248			return 1;
   249	
   250		if (!ghc_valid) {
   251			err = kvm_gfn_to_hva_cache_init(v->kvm, ghc, ghc->gpa, ghc->len);
   252			if (err || !ghc->memslot) {
   253				/*
   254				 * If this failed, userspace has screwed up the
   255				 * vcpu_info mapping. No interrupts for you.
   256				 */
   257				return 0;
   258			}
   259		}
   260	
   261		/*
   262		 * Now we have a valid (protected by srcu) userspace HVA in
   263		 * ghc->hva which points to the struct vcpu_info. If there
   264		 * are any bits in the in-kernel evtchn_pending_sel then
   265		 * we need to write those to the guest vcpu_info and set
   266		 * its evtchn_upcall_pending flag. If there aren't any bits
   267		 * to add, we only want to *check* evtchn_upcall_pending.
   268		 */
   269		if (evtchn_pending_sel) {
   270			bool long_mode = v->kvm->arch.xen.long_mode;
   271	
 > 272			if (!user_access_begin((void *)ghc->hva, sizeof(struct vcpu_info)))
   273				return 0;
   274	
   275			if (IS_ENABLED(CONFIG_64BIT) && long_mode) {
 > 276				struct vcpu_info __user *vi = (void *)ghc->hva;
   277	
   278				/* Attempt to set the evtchn_pending_sel bits in the
   279				 * guest, and if that succeeds then clear the same
   280				 * bits in the in-kernel version. */
   281				asm volatile("1:\t" LOCK_PREFIX "orq %0, %1\n"
   282					     "\tnotq %0\n"
   283					     "\t" LOCK_PREFIX "andq %0, %2\n"
   284					     "2:\n"
   285					     "\t.section .fixup,\"ax\"\n"
   286					     "3:\tjmp\t2b\n"
   287					     "\t.previous\n"
   288					     _ASM_EXTABLE_UA(1b, 3b)
   289					     : "=r" (evtchn_pending_sel),
   290					       "+m" (vi->evtchn_pending_sel),
   291					       "+m" (v->arch.xen.evtchn_pending_sel)
   292					     : "0" (evtchn_pending_sel));
   293			} else {
 > 294				struct compat_vcpu_info __user *vi = (void *)ghc->hva;
   295				u32 evtchn_pending_sel32 = evtchn_pending_sel;
   296	
   297				/* Attempt to set the evtchn_pending_sel bits in the
   298				 * guest, and if that succeeds then clear the same
   299				 * bits in the in-kernel version. */
   300				asm volatile("1:\t" LOCK_PREFIX "orl %0, %1\n"
   301					     "\tnotl %0\n"
   302					     "\t" LOCK_PREFIX "andl %0, %2\n"
   303					     "2:\n"
   304					     "\t.section .fixup,\"ax\"\n"
   305					     "3:\tjmp\t2b\n"
   306					     "\t.previous\n"
   307					     _ASM_EXTABLE_UA(1b, 3b)
   308					     : "=r" (evtchn_pending_sel32),
   309					       "+m" (vi->evtchn_pending_sel),
   310					       "+m" (v->arch.xen.evtchn_pending_sel)
   311					     : "0" (evtchn_pending_sel32));
   312			}
   313			rc = 1;
   314			unsafe_put_user(rc, (u8 __user *)ghc->hva + offset, err);
   315	
   316		err:
   317			user_access_end();
   318	
   319			mark_page_dirty_in_slot(v->kvm, ghc->memslot, ghc->gpa >> PAGE_SHIFT);
   320		} else {
   321			__get_user(rc, (u8 __user *)ghc->hva + offset);
   322		}
   323	
   324		return rc;
   325	}
   326	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37665 bytes --]