Re: [PATCH 06/10] KVM: x86/xen: Use guest's copy of pvclock when starting timer

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

Hi Sean,

kernel test robot noticed the following build warnings:

[auto build test WARNING on eb723766b1030a23c38adf2348b7c3d1409d11f0]

url:    https://github.com/intel-lab-lkp/linux/commits/Sean-Christopherson/KVM-x86-Don-t-take-kvm-lock-when-iterating-over-vCPUs-in-suspend-notifier/20250118-085939
base:   eb723766b1030a23c38adf2348b7c3d1409d11f0
patch link:    https://lore.kernel.org/r/20250118005552.2626804-7-seanjc%40google.com
patch subject: [PATCH 06/10] KVM: x86/xen: Use guest's copy of pvclock when starting timer
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20250119/202501191041.ew4CA984-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250119/202501191041.ew4CA984-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501191041.ew4CA984-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from arch/x86/kvm/xen.c:10:
   In file included from arch/x86/kvm/x86.h:5:
   In file included from include/linux/kvm_host.h:16:
   In file included from include/linux/mm.h:2223:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> arch/x86/kvm/xen.c:219:7: warning: variable 'xen' is uninitialized when used here [-Wuninitialized]
     219 |                 if (xen->vcpu_info_cache.active)
         |                     ^~~
   arch/x86/kvm/xen.c:189:26: note: initialize the variable 'xen' to silence this warning
     189 |         struct kvm_vcpu_xen *xen;
         |                                 ^
         |                                  = NULL
   5 warnings generated.


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

   185	
   186	static void kvm_xen_start_timer(struct kvm_vcpu *vcpu, u64 guest_abs,
   187					bool linux_wa)
   188	{
   189		struct kvm_vcpu_xen *xen;
   190		int64_t kernel_now, delta;
   191		uint64_t guest_now;
   192		int r = -EOPNOTSUPP;
   193	
   194		/*
   195		 * The guest provides the requested timeout in absolute nanoseconds
   196		 * of the KVM clock — as *it* sees it, based on the scaled TSC and
   197		 * the pvclock information provided by KVM.
   198		 *
   199		 * The kernel doesn't support hrtimers based on CLOCK_MONOTONIC_RAW
   200		 * so use CLOCK_MONOTONIC. In the timescales covered by timers, the
   201		 * difference won't matter much as there is no cumulative effect.
   202		 *
   203		 * Calculate the time for some arbitrary point in time around "now"
   204		 * in terms of both kvmclock and CLOCK_MONOTONIC. Calculate the
   205		 * delta between the kvmclock "now" value and the guest's requested
   206		 * timeout, apply the "Linux workaround" described below, and add
   207		 * the resulting delta to the CLOCK_MONOTONIC "now" value, to get
   208		 * the absolute CLOCK_MONOTONIC time at which the timer should
   209		 * fire.
   210		 */
   211		do {
   212			struct pvclock_vcpu_time_info hv_clock;
   213			uint64_t host_tsc, guest_tsc;
   214	
   215			if (!static_cpu_has(X86_FEATURE_CONSTANT_TSC) ||
   216			    !vcpu->kvm->arch.use_master_clock)
   217				break;
   218	
 > 219			if (xen->vcpu_info_cache.active)
   220				r = xen_get_guest_pvclock(vcpu, &hv_clock, &xen->vcpu_info_cache,
   221							offsetof(struct compat_vcpu_info, time));
   222			else if (xen->vcpu_time_info_cache.active)
   223				r = xen_get_guest_pvclock(vcpu, &hv_clock, &xen->vcpu_time_info_cache, 0);
   224			if (r)
   225				break;
   226	
   227			if (!IS_ENABLED(CONFIG_64BIT) ||
   228			    !kvm_get_monotonic_and_clockread(&kernel_now, &host_tsc)) {
   229				/*
   230				 * Don't fall back to get_kvmclock_ns() because it's
   231				 * broken; it has a systemic error in its results
   232				 * because it scales directly from host TSC to
   233				 * nanoseconds, and doesn't scale first to guest TSC
   234				 * and *then* to nanoseconds as the guest does.
   235				 *
   236				 * There is a small error introduced here because time
   237				 * continues to elapse between the ktime_get() and the
   238				 * subsequent rdtsc(). But not the systemic drift due
   239				 * to get_kvmclock_ns().
   240				 */
   241				kernel_now = ktime_get(); /* This is CLOCK_MONOTONIC */
   242				host_tsc = rdtsc();
   243			}
   244	
   245			/* Calculate the guest kvmclock as the guest would do it. */
   246			guest_tsc = kvm_read_l1_tsc(vcpu, host_tsc);
   247			guest_now = __pvclock_read_cycles(&hv_clock, guest_tsc);
   248		} while (0);
   249	
   250		if (r) {
   251			/*
   252			 * Without CONSTANT_TSC, get_kvmclock_ns() is the only option.
   253			 *
   254			 * Also if the guest PV clock hasn't been set up yet, as is
   255			 * likely to be the case during migration when the vCPU has
   256			 * not been run yet. It would be possible to calculate the
   257			 * scaling factors properly in that case but there's not much
   258			 * point in doing so. The get_kvmclock_ns() drift accumulates
   259			 * over time, so it's OK to use it at startup. Besides, on
   260			 * migration there's going to be a little bit of skew in the
   261			 * precise moment at which timers fire anyway. Often they'll
   262			 * be in the "past" by the time the VM is running again after
   263			 * migration.
   264			 */
   265			guest_now = get_kvmclock_ns(vcpu->kvm);
   266			kernel_now = ktime_get();
   267		}
   268	
   269		delta = guest_abs - guest_now;
   270	
   271		/*
   272		 * Xen has a 'Linux workaround' in do_set_timer_op() which checks for
   273		 * negative absolute timeout values (caused by integer overflow), and
   274		 * for values about 13 days in the future (2^50ns) which would be
   275		 * caused by jiffies overflow. For those cases, Xen sets the timeout
   276		 * 100ms in the future (not *too* soon, since if a guest really did
   277		 * set a long timeout on purpose we don't want to keep churning CPU
   278		 * time by waking it up).  Emulate Xen's workaround when starting the
   279		 * timer in response to __HYPERVISOR_set_timer_op.
   280		 */
   281		if (linux_wa &&
   282		    unlikely((int64_t)guest_abs < 0 ||
   283			     (delta > 0 && (uint32_t) (delta >> 50) != 0))) {
   284			delta = 100 * NSEC_PER_MSEC;
   285			guest_abs = guest_now + delta;
   286		}
   287	
   288		/*
   289		 * Avoid races with the old timer firing. Checking timer_expires
   290		 * to avoid calling hrtimer_cancel() will only have false positives
   291		 * so is fine.
   292		 */
   293		if (vcpu->arch.xen.timer_expires)
   294			hrtimer_cancel(&vcpu->arch.xen.timer);
   295	
   296		atomic_set(&vcpu->arch.xen.timer_pending, 0);
   297		vcpu->arch.xen.timer_expires = guest_abs;
   298	
   299		if (delta <= 0)
   300			xen_timer_callback(&vcpu->arch.xen.timer);
   301		else
   302			hrtimer_start(&vcpu->arch.xen.timer,
   303				      ktime_add_ns(kernel_now, delta),
   304				      HRTIMER_MODE_ABS_HARD);
   305	}
   306	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki