From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f200.google.com (mail-pg1-f200.google.com [209.85.215.200]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0C0C63C5833 for ; Fri, 24 Jul 2026 21:29:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.200 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784928585; cv=none; b=E4794M+F4aOsQ+pvQpUYH8Pb3yheWWi9lTXbNIqt0wPhEtUyrz7pQa2h2PRfnpiis2cWUwRucYPn87PdA1XZFrQwptOS/odm3RKtVWcYC739Wxt9mMuk2C4CJZxrKTTYQ/1iugwTTJLVh5QCJynTMSEQ5sPgvhR2vJ95bgfN+xg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784928585; c=relaxed/simple; bh=R8kivS+PAT8BTHF7UsSksEuqQUzOURbJU+P2fAuFGUg=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=PyOSocRCvr5U2uV6h92Wb9iAK7Yd4vd3NeG/9tOH8mZop46x2NFUXtyow3LJVLi3Uw4PQVu4cmidtoA5Fqyu48KfRamL3bjR+qis/gcjI23Kt9JVFglmY48zJt/3ecV8jsHbkyQ3Esf3Y17E2nLQClGAc6SXHo5gp6Pyd2z/HTI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=euRxa9UZ; arc=none smtp.client-ip=209.85.215.200 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="euRxa9UZ" Received: by mail-pg1-f200.google.com with SMTP id 41be03b00d2f7-cb835525b10so940441a12.2 for ; Fri, 24 Jul 2026 14:29:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1784928583; x=1785533383; darn=vger.kernel.org; h=content-type:cc:to:from:subject:message-id:references:mime-version :in-reply-to:date:from:to:cc:subject:date:message-id:reply-to :content-type; bh=LeEjIRradrtHITbFsKRpe0yq9gWXW7YqA8y6Nt7MYfg=; b=euRxa9UZYByAEZ8NdwDRanjH/Q38iYh/zlccg4xx+7YrkpFQl8yBEhVFdXsldxsXnT U7yabo0bk8AD1vqMNGXo4w7u3WNkvX/MwOiQ9J5xqptvq/4oSD850Yo1HxDvJF6Nh16D XWgR4Z7NVglSm6LE6GnAaUmAlzZNfg1rbS0IeeumkgXYLpjDYjJyaW5PBpxKapShS1xA NAtNNNpzZBnteKPTal8dPQ/qsxOSkTgBVhE0pg1wZYkoW4Fitcu32BYM7wgvCtaDdY6b dLL9B4L5Gj1d9ysHZ9b8TrVEQrwZyMpWkxdjL8DZgKj1t9hFbKJof+90xVpztDYZ9eAI yaVw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784928583; x=1785533383; h=content-type:cc:to:from:subject:message-id:references:mime-version :in-reply-to:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=LeEjIRradrtHITbFsKRpe0yq9gWXW7YqA8y6Nt7MYfg=; b=dSd5XDstZ0Ev2n1FNDLd7y03fg1Jg6QsvG2bB2ILsjfBBQKDTpdDWUos7moGNwpfGn 68l7Tx5YkVISg9pPVGXmDbGyk5t4t1AMxpfs7GHZ2VLh1jtO3ioOgqGiqpENE526cO6k fhBthHKiCJj9Wmqz42M7A1/PJm9R+Rj9wH6muLPLO7N4JDbgvwAaBRS8aAf/EN95pviu JFqS7N6d1ibus0QGsic1cnqRQbM8yRJ+/E4Y9jV/HZKURKbakziJY6pdJHyc8F1k8j6G aTQ/Kp5NA/wyhRt/o64d00qc5jJ9XB4hPCXkd/uUEbyczMH+d50TqFScZHTYeEmHpFCs 9xQw== X-Forwarded-Encrypted: i=1; AHgh+RrSMeJNwq1JbZdQu9buV5zwwFK9fyn76fdY9NVBG3R/bpqbJ5QNIz0mV7elQ4FvIZnsEk+qjGqoAD0=@vger.kernel.org X-Gm-Message-State: AOJu0Yy/udnboey6L/IgUTGw3MugP1hmE9CPCj70xwWzTE2Wav2IqmPj uCU4XSiuHdz5BxxYq5SqK5ZRIBhsNtPysnLpleGc9HAy5t+LUUzWIBu5bFYanF7Vgx08+ZpgMEp RxrKjhg== X-Received: from pgbcm9.prod.google.com ([2002:a05:6a02:a09:b0:c85:8035:8b21]) (user=seanjc job=prod-delivery.src-stubby-dispatcher) by 2002:a05:6a20:d52b:b0:3c3:e411:418a with SMTP id adf61e73a8af0-3c67e164e76mr39234637.70.1784928583222; Fri, 24 Jul 2026 14:29:43 -0700 (PDT) Date: Fri, 24 Jul 2026 14:29:42 -0700 In-Reply-To: <20260703212145.343527-13-dwmw2@infradead.org> Precedence: bulk X-Mailing-List: linux-doc@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20260703212145.343527-1-dwmw2@infradead.org> <20260703212145.343527-13-dwmw2@infradead.org> Message-ID: Subject: Re: [PATCH v6 12/36] KVM: x86: Restructure get_kvmclock() From: Sean Christopherson To: David Woodhouse Cc: Paolo Bonzini , Jonathan Corbet , Shuah Khan , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , Vitaly Kuznetsov , Juergen Gross , Boris Ostrovsky , Paul Durrant , Jonathan Cameron , Sascha Bischoff , Marc Zyngier , Joey Gouly , Jack Allister , Dongli Zhang , joe.jin@oracle.com, kvm@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org, linux-kselftest@vger.kernel.org Content-Type: text/plain; charset="us-ascii" On Fri, Jul 03, 2026, David Woodhouse wrote: > From: David Woodhouse > > Simplify the use_master_clock condition: the open-coded CONSTANT_TSC || > cpu_tsc_khz check is unnecessary since use_master_clock can only be true > when the host clocksource is TSC based, which in turn requires a stable, > constant and synchronised TSC across all CPUs. > > Given that, the get_cpu()/put_cpu() pinning is not needed either: both > the TSC read and get_cpu_tsc_khz() are CPU-independent when the master > clock is in use, so drop them. > > Wrap the entire use_master_clock block in #ifdef CONFIG_X86_64, since > use_master_clock is never true on 32-bit (host_tsc_clocksource is only > set under CONFIG_X86_64), and declare hv_clock inside the block so it is > not left as an unused variable on 32-bit. > > Use 'continue' on the master-clock success path so the non-master-clock > computation becomes the common tail, avoiding a goto and label. When the > clock read fails (e.g. clocksource transitioning away from TSC), fall > back to that path rather than proceeding with uninitialised data or > spinning in the seqcount loop. Please split this up. The changelog suggests there are at least three logical changes here. Yeah, the series is big, but smaller patches helps with review, even if it results in more total patches. > Signed-off-by: David Woodhouse > --- > arch/x86/kvm/x86.c | 42 ++++++++++++++++++++---------------------- > 1 file changed, 20 insertions(+), 22 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 50f088570dab..37b1f8192842 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3203,40 +3203,38 @@ static unsigned long get_cpu_tsc_khz(void) > static void get_kvmclock(struct kvm *kvm, struct kvm_clock_data *data) > { > struct kvm_arch *ka = &kvm->arch; > - struct pvclock_vcpu_time_info hv_clock; > unsigned int seq; > > do { > seq = read_seqcount_begin(&ka->pvclock_sc); > > - /* both __this_cpu_read() and rdtsc() should be on the same cpu */ > - get_cpu(); > - > data->flags = 0; > - if (ka->use_master_clock && > - (static_cpu_has(X86_FEATURE_CONSTANT_TSC) || __this_cpu_read(cpu_tsc_khz))) { > #ifdef CONFIG_X86_64 > + if (ka->use_master_clock) { > + struct pvclock_vcpu_time_info hv_clock; > struct timespec64 ts; > > if (kvm_get_walltime_and_clockread(&ts, &data->host_tsc)) { > data->realtime = ts.tv_nsec + NSEC_PER_SEC * ts.tv_sec; > - data->flags |= KVM_CLOCK_REALTIME | KVM_CLOCK_HOST_TSC; > - } else > -#endif > - data->host_tsc = rdtsc(); > - > - data->flags |= KVM_CLOCK_TSC_STABLE; > - hv_clock.tsc_timestamp = ka->master_cycle_now; > - hv_clock.system_time = ka->master_kernel_ns + ka->kvmclock_offset; > - kvm_get_time_scale(NSEC_PER_SEC, get_cpu_tsc_khz() * 1000LL, > - &hv_clock.tsc_shift, > - &hv_clock.tsc_to_system_mul); > - data->clock = __pvclock_read_cycles(&hv_clock, data->host_tsc); > - } else { > - data->clock = get_kvmclock_base_ns() + ka->kvmclock_offset; > - } > + data->flags |= KVM_CLOCK_REALTIME | KVM_CLOCK_HOST_TSC | KVM_CLOCK_TSC_STABLE; > + > + hv_clock.tsc_timestamp = ka->master_cycle_now; > + hv_clock.system_time = ka->master_kernel_ns + ka->kvmclock_offset; > + kvm_get_time_scale(NSEC_PER_SEC, get_cpu_tsc_khz() * 1000LL, > + &hv_clock.tsc_shift, > + &hv_clock.tsc_to_system_mul); > + data->clock = __pvclock_read_cycles(&hv_clock, data->host_tsc); > + continue; > + } > > - put_cpu(); > + /* > + * Clock read failed (e.g. clocksource is transitioning > + * away from TSC). Fall back to the non-master-clock path > + * rather than spinning. > + */ > + } > +#endif > + data->clock = get_kvmclock_base_ns() + ka->kvmclock_offset; > } while (read_seqcount_retry(&ka->pvclock_sc, seq)); > } > > -- > 2.54.0 >