* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock
From: Hillf Danton @ 2026-07-02 8:25 UTC (permalink / raw)
To: Ryosuke Yasuoka
Cc: Dmitry Osipenko, Dmitry Baryshkov, Javier Martinez Canillas,
dri-devel, virtualization, linux-kernel
In-Reply-To: <18be678e97322160.8f56a0770e5621ab.8d175eba8a62d6be@ryasuoka-thinkpadx1carbongen9.tokyo.csb>
On Thu, 2 Jul 2026 07:12:44 +0000 Ryosuke Yasuoka wrote:
>On 02/07/2026 07:43, Hillf Danton wrote:
>> On Wed, 1 Jul 2026 09:23:05 +0000 Ryosuke Yasuoka wrote:
>>>On 30/06/2026 16:46, Dmitry Osipenko wrote:
>>>> Could you please move drm_kms_helper_hotplug_event() to virtio_gpu_init(),
>>>> placing it after wait_event_timeout(display_info_pending)? This will avoid
>>>> additional work_struct that otherwise needs to be cancelled in
>>>> virtio_gpu_init() on the timeout.
>>>
>>>IIUC, moving the drm_kms_helper_hotplug_event() and _hpd_irq_event()
>>>into virtio_gpu_init() after wait_event_timeout() would not prevent the
>>>issue.
>>>
>>>Looking at the syzbot call traces[1][2], the deadlock occurs during
>>>drm_client_setup(), which runs after virtio_gpu_init() has already
>>>returned. The display_info_cb that triggers the deadlock is called from
>>>the dequeue worker while drm_client_register() holds clientlist_mutex.
>>>
>>>Thread A:
>>>virtio_gpu_probe()
>>> -> virtio_gpu_init() // sends GET_DISPLAY_INFO and waits up to 5s
>>> -> drm_dev_register()
>>> -> drm_client_setup() // deadlock happens HERE
>>> -> drm_client_register() // holds clientlist_mutex
>>> ...
>>> -> virtio_gpu_queue_fenced_ctrl_buffer()
>>> -> wait_event() // waits for free space
>>>
>>>Thread B:
>>>virtio_gpu_dequeue_ctrl_func()
>>> -> reclaim_vbufs() // make free space
>>
>> I wonder if the deadlock could be cured by alternatively adding a wakeup
>> before the responce cb, given that free space is available.
>
>Thanks for your comment. I agree that moving wake_up() before response
>cb would help in most cases, and I think it is a reasonable aproach.
>However, there are a few rare scenarios where the deadlock can still
>occur:
>
>1. If reclaim_vbufs() does not free enough slots to satisfy
>num_free >= elemcnt, wake_up() fires but the wait_event() condition
>evaluates false. Thread A remains asleep holding clientlist_mutex, and
>the dequeue worker proceeds to display_info_cb which blocks on
>clientlist_mutex.
task0 task1
lock clientlist_mutex
wait_event(for enough free slots)
free some slots
lock clientlist_mutex
unlock clientlist_mutex
wakeup
deadlock
The clientlist_mutex will not be released without enough slots freed, so more
free slots and wakeup are needed to cure the deadlock, with nothing to do with
acquiring the mutex in the workqueue context.
Your workqueue approach works by removing the dependence of wakeup on acquiring
the mutex, without enough free slots guaranteed.
In short, wakeup before acquiring the mutex is the right thing to do, I mean
it is not the only one at all.
>2. Even if enough slots are freed, there is a small window between
>wait_event() returning and Thread A acquiring spin_lock(&qlock) at the
>"goto again" path in virtio_gpu_queue_ctrl_sgs(). Another thread can
>consume the freed slots during this window, causing Thread A to loop
>back to wait_event() still holding clientlist_mutex.
Ditto
>3. Reordering wake_up() before resp_cb inverts the natural "process
>response, then signal completion" pattern, which could introduce subtle
>issues if a future resp_cb depends on side effects that should happen
>after wake_up(), I believe.
>
Different taste
>These scenarios are likely rare in practice, but the schedule_work()
>approach avoids all of them entirely by ensuring the dequeue worker
>never touches clientlist_mutex. It also follows the same pattern
>already used by config_changed_work in this driver.
>
>Best regards
>Ryosuke
>
>>
>>> -> resp_cb()
>>> -> virtio_gpu_cmd_get_display_info_cb
>>> -> drm_helper_hpd_irq_event()
>>> -> drm_kms_helper_hotplug_event()
>>> -> drm_client_dev_hotplug() // need to lock clientlist_mutex
>>> -> wake_up() // never reached
>>>
>>>IIUC the hotplug notification in display_info_cb is needed because it
>>>notifies DRM after updating by the callback with fresh data from the host.
>>>
>>>This work_struct ensures display_info_cb never blocks on
>>>clientlist_mutex in the dequeue worker, while preserving the hotplug
>>>notification with fresh data.
>>>
>>>[1] https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
>>>[2] https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
>>>
>>>Best regards,
>>>Ryosuke
^ permalink raw reply
* Re: [PATCH 01/13] mm: introduce vma_flags_can_grow() and vma_can_grow()
From: Lance Yang @ 2026-07-02 7:27 UTC (permalink / raw)
To: ljs
Cc: akpm, tsbogend, maddy, mpe, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, l.stach, inki.dae, sw0312.kim,
kyungmin.park, krzk, peter.griffin, jani.nikula, joonas.lahtinen,
rodrigo.vivi, tursulin, robin.clark, lumag, lyude, dakr,
tomi.valkeinen, hjc, heiko, andy.yan, thierry.reding, mperttunen,
jonathanh, kraxel, dmitry.osipenko, zack.rusin, matthew.brost,
thomas.hellstrom, oleksandr_andrushchenko, deller, bcrl, viro,
brauner, muchun.song, osalvador, david, ziy, baolin.wang, liam,
npache, ryan.roberts, dev.jain, baohua, lance.yang, hughd, vbabka,
rppt, surenb, mhocko, jannh, pfalcato, kees, perex, tiwai,
linux-mips, linux-kernel, linuxppc-dev, dri-devel, etnaviv,
linux-arm-kernel, linux-samsung-soc, intel-gfx, linux-arm-msm,
freedreno, nouveau, linux-rockchip, linux-tegra, virtualization,
intel-xe, xen-devel, linux-fbdev, linux-aio, linux-fsdevel,
linux-mm, linux-sound
In-Reply-To: <f2e8c32515d328db62279cc8bab8398ea278d74f.1782760670.git.ljs@kernel.org>
On Mon, Jun 29, 2026 at 08:25:24PM +0100, Lorenzo Stoakes wrote:
>These test whether the VMA has stack sematics, i.e. is able to grow upwards
>or downwards depending on the architecture.
>
>In order to account for arches which do not support upward-growing stacks,
>introduce VMA_GROWSUP whose definition depends on the architecture
>supporting it, and use vma_flags_test_single_mask() in vma_flags_can_grow()
>to account for this.
>
>Update the VMA userland tests to reflect the changes
>
>No functional change intended.
>
>Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
>---
Nice cleanup! Feel free to add:
Reviewed-by: Lance Yang <lance.yang@linux.dev>
^ permalink raw reply
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock
From: Ryosuke Yasuoka @ 2026-07-02 7:12 UTC (permalink / raw)
To: Hillf Danton
Cc: Dmitry Osipenko, Dmitry Baryshkov, Javier Martinez Canillas,
dri-devel, virtualization, linux-kernel
In-Reply-To: <20260701234301.361-1-hdanton@sina.com>
On 02/07/2026 07:43, Hillf Danton wrote:
> On Wed, 1 Jul 2026 09:23:05 +0000 Ryosuke Yasuoka wrote:
>>On 30/06/2026 16:46, Dmitry Osipenko wrote:
>>> Could you please move drm_kms_helper_hotplug_event() to virtio_gpu_init(),
>>> placing it after wait_event_timeout(display_info_pending)? This will avoid
>>> additional work_struct that otherwise needs to be cancelled in
>>> virtio_gpu_init() on the timeout.
>>
>>IIUC, moving the drm_kms_helper_hotplug_event() and _hpd_irq_event()
>>into virtio_gpu_init() after wait_event_timeout() would not prevent the
>>issue.
>>
>>Looking at the syzbot call traces[1][2], the deadlock occurs during
>>drm_client_setup(), which runs after virtio_gpu_init() has already
>>returned. The display_info_cb that triggers the deadlock is called from
>>the dequeue worker while drm_client_register() holds clientlist_mutex.
>>
>>Thread A:
>>virtio_gpu_probe()
>> -> virtio_gpu_init() // sends GET_DISPLAY_INFO and waits up to 5s
>> -> drm_dev_register()
>> -> drm_client_setup() // deadlock happens HERE
>> -> drm_client_register() // holds clientlist_mutex
>> ...
>> -> virtio_gpu_queue_fenced_ctrl_buffer()
>> -> wait_event() // waits for free space
>>
>>Thread B:
>>virtio_gpu_dequeue_ctrl_func()
>> -> reclaim_vbufs() // make free space
>
> I wonder if the deadlock could be cured by alternatively adding a wakeup
> before the responce cb, given that free space is available.
Thanks for your comment. I agree that moving wake_up() before response
cb would help in most cases, and I think it is a reasonable aproach.
However, there are a few rare scenarios where the deadlock can still
occur:
1. If reclaim_vbufs() does not free enough slots to satisfy
num_free >= elemcnt, wake_up() fires but the wait_event() condition
evaluates false. Thread A remains asleep holding clientlist_mutex, and
the dequeue worker proceeds to display_info_cb which blocks on
clientlist_mutex.
2. Even if enough slots are freed, there is a small window between
wait_event() returning and Thread A acquiring spin_lock(&qlock) at the
"goto again" path in virtio_gpu_queue_ctrl_sgs(). Another thread can
consume the freed slots during this window, causing Thread A to loop
back to wait_event() still holding clientlist_mutex.
3. Reordering wake_up() before resp_cb inverts the natural "process
response, then signal completion" pattern, which could introduce subtle
issues if a future resp_cb depends on side effects that should happen
after wake_up(), I believe.
These scenarios are likely rare in practice, but the schedule_work()
approach avoids all of them entirely by ensuring the dequeue worker
never touches clientlist_mutex. It also follows the same pattern
already used by config_changed_work in this driver.
Best regards
Ryosuke
>
>> -> resp_cb()
>> -> virtio_gpu_cmd_get_display_info_cb
>> -> drm_helper_hpd_irq_event()
>> -> drm_kms_helper_hotplug_event()
>> -> drm_client_dev_hotplug() // need to lock clientlist_mutex
>> -> wake_up() // never reached
>>
>>IIUC the hotplug notification in display_info_cb is needed because it
>>notifies DRM after updating by the callback with fresh data from the host.
>>
>>This work_struct ensures display_info_cb never blocks on
>>clientlist_mutex in the dequeue worker, while preserving the hotplug
>>notification with fresh data.
>>
>>[1] https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
>>[2] https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
>>
>>Best regards,
>>Ryosuke
^ permalink raw reply
* Re: [PATCH v2 2/3] drm/virtio: honor blob_alignment requirements
From: Michael S. Tsirkin @ 2026-07-02 4:03 UTC (permalink / raw)
To: Alyssa Ross
Cc: Sergio Lopez, Chia-I Wu, Jason Wang, Eugenio Pérez,
Xuan Zhuo, linux-kernel, Simona Vetter, Dmitry Osipenko,
Thomas Zimmermann, David Airlie, Gurchetan Singh, Gerd Hoffmann,
virtualization, dri-devel, Maxime Ripard, Maarten Lankhorst
In-Reply-To: <akT1XEW766c7Imst@mbp.qyliss.net>
On Wed, Jul 01, 2026 at 01:10:30PM +0200, Alyssa Ross wrote:
> On Tue, Apr 28, 2026 at 09:44:49PM +0200, Sergio Lopez wrote:
> > If VIRTIO_GPU_F_BLOB_ALIGNMENT has been negotiated, blob size must be
> > aligned to blob_alignment. Validate this in verify_blob() so that
> > invalid requests are rejected early.
> >
> > Signed-off-by: Sergio Lopez <slp@redhat.com>
>
> FYI: this change breaks crosvm, which is squatting the 5 and 6 values
> of VIRTIO_GPU_F_* with different meanings. I've reported it as a
> crosvm bug, so hopefully it can be taken care of there.
>
> https://issuetracker.google.com/issues/529852979
Move fast and beak things, then it takes a wile to fix them)
Do you understand who uses it maybe? Maybe if there's a PR
for both sides it will be handled faster.
> > ---
> > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > index c33c057365f8..d0c4edf1eaf4 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > @@ -489,6 +489,11 @@ static int verify_blob(struct virtio_gpu_device *vgdev,
> > params->size = rc_blob->size;
> > params->blob = true;
> > params->blob_flags = rc_blob->blob_flags;
> > +
> > + if (vgdev->has_blob_alignment &&
> > + !IS_ALIGNED(params->size, vgdev->blob_alignment))
> > + return -EINVAL;
> > +
> > return 0;
> > }
> >
> > --
> > 2.53.0
> >
^ permalink raw reply
* [PATCH v3 1/2] tools/virtio: Add missing compat definitions for vhost_net_test
From: Yichong Chen @ 2026-07-02 3:03 UTC (permalink / raw)
To: mst, jasowang, xuanzhuo, eperezma, akpm
Cc: sj, rppt, ljs, pabeni, chenyichong, virtualization, linux-kernel
In-Reply-To: <20260702030358.308155-1-chenyichong@uniontech.com>
vhost_net_test builds virtio_ring.c in userspace.
Recent virtio headers pull in helper headers that are not provided by
the tools/virtio compatibility layer, including asm/percpu_types.h,
linux/completion.h, linux/mod_devicetable.h and linux/virtio_features.h.
Add the missing compat definitions and the DMA attribute used by the
current virtio ring code.
Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
Tested-by: SJ Park <sj@kernel.org>
---
tools/virtio/asm/percpu_types.h | 7 +++
tools/virtio/linux/completion.h | 9 ++++
tools/virtio/linux/device.h | 1 +
tools/virtio/linux/dma-mapping.h | 1 +
tools/virtio/linux/mod_devicetable.h | 14 +++++
tools/virtio/linux/virtio_features.h | 79 ++++++++++++++++++++++++++++
6 files changed, 111 insertions(+)
create mode 100644 tools/virtio/asm/percpu_types.h
create mode 100644 tools/virtio/linux/completion.h
create mode 100644 tools/virtio/linux/mod_devicetable.h
create mode 100644 tools/virtio/linux/virtio_features.h
diff --git a/tools/virtio/asm/percpu_types.h b/tools/virtio/asm/percpu_types.h
new file mode 100644
index 000000000000..4eb53d93c099
--- /dev/null
+++ b/tools/virtio/asm/percpu_types.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_PERCPU_TYPES_H
+#define _ASM_PERCPU_TYPES_H
+
+#define __percpu_qual
+
+#endif /* _ASM_PERCPU_TYPES_H */
diff --git a/tools/virtio/linux/completion.h b/tools/virtio/linux/completion.h
new file mode 100644
index 000000000000..5e54b679721b
--- /dev/null
+++ b/tools/virtio/linux/completion.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_COMPLETION_H
+#define _LINUX_COMPLETION_H
+
+struct completion {
+ unsigned int done;
+};
+
+#endif /* _LINUX_COMPLETION_H */
diff --git a/tools/virtio/linux/device.h b/tools/virtio/linux/device.h
index 075c2140d975..abf100cb0023 100644
--- a/tools/virtio/linux/device.h
+++ b/tools/virtio/linux/device.h
@@ -1,4 +1,5 @@
#ifndef LINUX_DEVICE_H
+#define LINUX_DEVICE_H
struct device {
void *parent;
diff --git a/tools/virtio/linux/dma-mapping.h b/tools/virtio/linux/dma-mapping.h
index 8d1a16cb20db..b9fc5e8338e3 100644
--- a/tools/virtio/linux/dma-mapping.h
+++ b/tools/virtio/linux/dma-mapping.h
@@ -61,5 +61,6 @@ enum dma_data_direction {
#define DMA_MAPPING_ERROR (~(dma_addr_t)0)
#define DMA_ATTR_CPU_CACHE_CLEAN (1UL << 11)
+#define DMA_ATTR_DEBUGGING_IGNORE_CACHELINES 0
#endif
diff --git a/tools/virtio/linux/mod_devicetable.h b/tools/virtio/linux/mod_devicetable.h
new file mode 100644
index 000000000000..3ba594b8229d
--- /dev/null
+++ b/tools/virtio/linux/mod_devicetable.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_MOD_DEVICETABLE_H
+#define _LINUX_MOD_DEVICETABLE_H
+
+#include <linux/types.h>
+
+struct virtio_device_id {
+ __u32 device;
+ __u32 vendor;
+};
+
+#define VIRTIO_DEV_ANY_ID 0xffffffff
+
+#endif /* _LINUX_MOD_DEVICETABLE_H */
diff --git a/tools/virtio/linux/virtio_features.h b/tools/virtio/linux/virtio_features.h
new file mode 100644
index 000000000000..04cbb9622ec7
--- /dev/null
+++ b/tools/virtio/linux/virtio_features.h
@@ -0,0 +1,79 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_VIRTIO_FEATURES_H
+#define _LINUX_VIRTIO_FEATURES_H
+
+#include <linux/bug.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+#define VIRTIO_FEATURES_U64S 2
+#define VIRTIO_FEATURES_BITS (VIRTIO_FEATURES_U64S * 64)
+
+#define VIRTIO_BIT(b) (1ULL << ((b) & 0x3f))
+#define VIRTIO_U64(b) ((b) >> 6)
+
+#define VIRTIO_DECLARE_FEATURES(name) \
+ union { \
+ u64 name; \
+ u64 name##_array[VIRTIO_FEATURES_U64S];\
+ }
+
+static inline bool virtio_features_chk_bit(unsigned int bit)
+{
+ return bit < VIRTIO_FEATURES_BITS;
+}
+
+static inline bool virtio_features_test_bit(const u64 *features,
+ unsigned int bit)
+{
+ return virtio_features_chk_bit(bit) &&
+ !!(features[VIRTIO_U64(bit)] & VIRTIO_BIT(bit));
+}
+
+static inline void virtio_features_set_bit(u64 *features, unsigned int bit)
+{
+ if (virtio_features_chk_bit(bit))
+ features[VIRTIO_U64(bit)] |= VIRTIO_BIT(bit);
+}
+
+static inline void virtio_features_clear_bit(u64 *features, unsigned int bit)
+{
+ if (virtio_features_chk_bit(bit))
+ features[VIRTIO_U64(bit)] &= ~VIRTIO_BIT(bit);
+}
+
+static inline void virtio_features_zero(u64 *features)
+{
+ memset(features, 0, sizeof(features[0]) * VIRTIO_FEATURES_U64S);
+}
+
+static inline void virtio_features_from_u64(u64 *features, u64 from)
+{
+ virtio_features_zero(features);
+ features[0] = from;
+}
+
+static inline bool virtio_features_equal(const u64 *f1, const u64 *f2)
+{
+ int i;
+
+ for (i = 0; i < VIRTIO_FEATURES_U64S; ++i)
+ if (f1[i] != f2[i])
+ return false;
+ return true;
+}
+
+static inline void virtio_features_copy(u64 *to, const u64 *from)
+{
+ memcpy(to, from, sizeof(to[0]) * VIRTIO_FEATURES_U64S);
+}
+
+static inline void virtio_features_andnot(u64 *to, const u64 *f1, const u64 *f2)
+{
+ int i;
+
+ for (i = 0; i < VIRTIO_FEATURES_U64S; i++)
+ to[i] = f1[i] & ~f2[i];
+}
+
+#endif /* _LINUX_VIRTIO_FEATURES_H */
--
2.51.0
^ permalink raw reply related
* [PATCH v3 2/2] tools/include: Include stdint.h for SIZE_MAX in overflow.h
From: Yichong Chen @ 2026-07-02 3:03 UTC (permalink / raw)
To: mst, jasowang, xuanzhuo, eperezma, akpm
Cc: sj, rppt, ljs, pabeni, chenyichong, virtualization, linux-kernel
In-Reply-To: <20260702030358.308155-1-chenyichong@uniontech.com>
tools/include/linux/overflow.h uses SIZE_MAX in its size helper
functions.
Include stdint.h so tools users that include overflow.h without another
SIZE_MAX provider can build.
Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
---
tools/include/linux/overflow.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/include/linux/overflow.h b/tools/include/linux/overflow.h
index 3427d7880326..9d30ae0dbd1d 100644
--- a/tools/include/linux/overflow.h
+++ b/tools/include/linux/overflow.h
@@ -2,6 +2,8 @@
#ifndef __LINUX_OVERFLOW_H
#define __LINUX_OVERFLOW_H
+#include <stdint.h>
+
#include <linux/compiler.h>
/*
--
2.51.0
^ permalink raw reply related
* [PATCH v3 0/2] tools: Fix tools/virtio test build
From: Yichong Chen @ 2026-07-02 3:03 UTC (permalink / raw)
To: mst, jasowang, xuanzhuo, eperezma, akpm
Cc: sj, rppt, ljs, pabeni, chenyichong, virtualization, linux-kernel
Hi,
This series fixes build failures hit by:
make -C tools/virtio test
It is based on linux-next commit:
commit 3d5670d672ae ("Add linux-next specific files for 20260626")
Patch 1 adds tools/virtio compatibility definitions needed by current
virtio headers when building the tools/virtio tests. Patch 2 makes
tools/include/linux/overflow.h include stdint.h for SIZE_MAX, which is
used by its size helper functions.
With the series applied, make -C tools/virtio test builds virtio_test,
vringh_test and vhost_net_test successfully.
Tested on x86_64 and arm64 with:
make -C tools/virtio clean
make -C tools/virtio test
Changes in v3:
- Move the stdint.h include inside the overflow.h include guard, as SJ
suggested.
- Add Tested-by from SJ Park to patch 1.
Changes in v2:
- Rebase and retest on linux-next.
- Add the missing asm/percpu_types.h compat header reported by Eugenio.
- Keep the tools/virtio compat definitions aligned with current
virtio_features.h helpers.
- Drop the slab.h kmalloc_obj/kmalloc_objs change because linux-next
already defines them in tools/virtio/linux/kernel.h.
Yichong Chen (2):
tools/virtio: Add missing compat definitions for vhost_net_test
tools/include: Include stdint.h for SIZE_MAX in overflow.h
tools/include/linux/overflow.h | 2 +
tools/virtio/asm/percpu_types.h | 7 +++
tools/virtio/linux/completion.h | 9 ++++
tools/virtio/linux/device.h | 1 +
tools/virtio/linux/dma-mapping.h | 1 +
tools/virtio/linux/mod_devicetable.h | 14 +++++
tools/virtio/linux/virtio_features.h | 79 ++++++++++++++++++++++++++++
7 files changed, 113 insertions(+)
create mode 100644 tools/virtio/asm/percpu_types.h
create mode 100644 tools/virtio/linux/completion.h
create mode 100644 tools/virtio/linux/mod_devicetable.h
create mode 100644 tools/virtio/linux/virtio_features.h
--
2.51.0
^ permalink raw reply
* Re: [syzbot] Monthly virt report (Jun 2026)
From: mawupeng @ 2026-07-02 2:55 UTC (permalink / raw)
To: syzbot+listbf7b8eeeb8dda31d6de1, linux-kernel, syzkaller-bugs,
virtualization, mst, jasowang, xuanzhuo, eperezma, stefanha,
sgarzare, davem, edumazet, kuba, pabeni, horms
Cc: mawupeng1, virtualization, kvm, netdev
In-Reply-To: <6a3c3ed6.80e5668d.5d0ef.0001.GAE@google.com>
On 周四 2026-6-25 04:32, syzbot wrote:
> Hello virt maintainers/developers,
>
> This is a 31-day syzbot report for the virt subsystem.
> All related reports/information can be found at:
> https://syzkaller.appspot.com/upstream/s/virt
>
> During the period, 0 new issues were detected and 0 were fixed.
> In total, 5 issues are still open and 61 have already been fixed.
> There are also 2 low-priority issues.
>
> Some of the still happening issues:
>
> Ref Crashes Repro Title
> <1> 24 No WARNING: refcount bug in call_timer_fn (4)
> https://syzkaller.appspot.com/bug?extid=07dcf509f4c013e25dc5
> <2> 3 Yes memory leak in __vsock_create (2)
> https://syzkaller.appspot.com/bug?extid=1b2c9c4a0f8708082678
Hi,
This is regarding the still-open "memory leak in __vsock_create (2)"
bug (#2 in the monthly virt report, extid 1b2c9c4a0f8708082678):
https://syzkaller.appspot.com/bug?extid=1b2c9c4a0f8708082678
I spent some time analyzing the root cause and the previous fix
attempt; below is a summary and a direction that tested out.
== Root cause ==
The leaked object is the child socket created by
virtio_transport_recv_listen() via __vsock_create() — exactly the
allocation site kmemleak points at. The reason it never gets freed is
in the accept() error path, not in the allocation itself.
When vsock_accept() dequeues a child but the listener carries an error
(listener->sk_err, e.g. set by a failed connect() issued on the socket
before listen()), it sets vconnected->rejected = true, skips
sock_graft(), drops the dequeue reference and *relies on
vsock_pending_work()* to clean the child up.
The catch: vsock_pending_work() is never scheduled on the transports
involved here. It is only ever scheduled by vmci_transport
(vmci_transport.c:1130); virtio_transport and vsock_loopback never
schedule it. So the rejected child sits with an unreleased initial
reference (the one from sk_alloc()) plus the connected-table
reference, vsock_sk_destruct() is never reached, and the cascade —
child socket, struct cred, virtio transport, SELinux blob — all leak.
The earlier commit 3a5cc90a4d17 ("vsock/virtio: remove socket from
connected/bound list on shutdown") adds an unconditional
vsock_remove_sock() in virtio_transport_recv_connected() when a
SHUTDOWN arrives, which drops the connected-table reference for a
child that later receives a SHUTDOWN; but it does not release the
sk_alloc() reference. So the leak is not really a regression
introduced there — rejected children have never been cleaned up on
transports that don't schedule pending_work. 3a5cc90a4d17 mainly
changes whether kmemleak can see the leak: on v6.6 it can (the
cascade shows up), on mainline the smaller struct sock layout leaves a
residual pointer inside the child that kmemleak counts as a reachable
reference, so mainline kmemleak stays silent even though
create/destruct accounting confirms the child never reaches
vsock_sk_destruct().
== Why the previous attempt didn't land ==
Divya's patch [1] tried to fix it by re-locking the parent listener
inside virtio_transport_recv_listen() and re-checking the shutdown
state under that lock before vsock_enqueue_accept(). That re-locks an
already-held lock — virtio_transport_recv_pkt() holds lock_sock(sk)
across the call into recv_listen() — and syzbot ci immediately flagged
"possible recursive locking" [2]. So it was backed out and the bug
stayed open.
== A direction that tests out ==
Instead of re-locking in the receive path, handle the cleanup directly
in vsock_accept(): on reject, instead of setting vconnected->rejected
and relying on pending_work, explicitly release the child's references
there:
if (err) {
vsock_remove_connected(vconnected); /* connected-table ref */
connected->sk_state = TCP_CLOSE;
sock_put(connected); /* enqueue_accept ref */
} else {
sock_graft(connected, newsock);
}
...
sock_put(connected); /* the existing, common put — sk_alloc ref */
This drops exactly the three references the child holds at dequeue
time (sk_alloc + __vsock_insert_connected + vsock_enqueue_accept),
lets refcount reach zero and vsock_sk_destruct() run. The `rejected`
flag and its pending_work handling can then be removed. The receive
path is not touched, so there is no re-locking and no deadlock.
I verified this on ARM64 QEMU. On linux v6.6.y (where kmemleak can
see the leak) with the syzbot reproducer:
- before: 6 creates / 4 destructs (2 leaked); kmemleak reports the
cascade;
- after: 6 creates / 6 destructs (0 leaked); kmemleak clean;
- 50-iter normal server and 50-iter same-port-reconnect tests both
pass 50/50 with zero leaks, no double-put warnings.
On mainline, kmemleak stays silent (see above) but create/destruct
accounting confirms the same leak before the fix; the fix is
code-identical across v6.6.y and mainline (same recv_listen/accept
paths).
I'm not subscribed to follow the list at full volume; happy to send a
formal patch (with the af_vsock.h / pending_work changes folded in)
if the direction looks right to the maintainers.
== Trigger, for completeness ==
The reproducer's atypical-but-legal sequence is what sets
listener->sk_err: a socket is connect()ed (leaving sk_err set, since
vsock_connect() only clears it at the start of a new connect) and then
turned into a listener:
fd = socket(AF_VSOCK, SOCK_STREAM, 0);
bind(fd, ...);
connect(fd, &(VMADDR_CID_LOCAL, ...)); /* leaves sk_err set */
listen(fd, 5);
/* a peer connects to fd; the child created is later rejected */
accept4(fd, ...);
Standard servers (listen before any connect on the same fd) don't hit
it, which is why this went ~2.5 years between the offending commit and
the syzbot report.
[1] https://lore.kernel.org/all/20260605191922.12720-1-divyakm@unc.edu/
[2] https://ci.syzbot.org/series/76f40e62-5a21-46d4-a636-10f0ec9c5040
Thanks.
> <3> 3913 Yes INFO: rcu detected stall in do_idle
> https://syzkaller.appspot.com/bug?extid=385468161961cee80c31
>
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> To disable reminders for individual bugs, reply with the following command:
> #syz set <Ref> no-reminders
>
> To change bug's subsystems, reply with:
> #syz set <Ref> subsystems: new-subsystem
>
> You may send multiple commands in a single email message.
^ permalink raw reply
* Re: [PATCH v2 2/2] tools/include: Include stdint.h for SIZE_MAX in overflow.h
From: Yichong Chen @ 2026-07-02 2:20 UTC (permalink / raw)
To: sj
Cc: akpm, chenyichong, eperezma, jasowang, linux-kernel, ljs, mst,
pabeni, rppt, virtualization, xuanzhuo
In-Reply-To: <20260701133503.41823-1-sj@kernel.org>
Hi SJ,
You are right. The include should be inside the header guard, after
#define __LINUX_OVERFLOW_H.
I will fix it in the next version.
Thanks,
Yichong
^ permalink raw reply
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock
From: Hillf Danton @ 2026-07-01 23:43 UTC (permalink / raw)
To: Ryosuke Yasuoka
Cc: Dmitry Osipenko, Dmitry Baryshkov, Javier Martinez Canillas,
dri-devel, virtualization, linux-kernel
In-Reply-To: <18be2016e26ade27.a6e9e2496bc1f978.9bc3a62421115997@ryasuoka-thinkpadx1carbongen9.tokyo.csb>
On Wed, 1 Jul 2026 09:23:05 +0000 Ryosuke Yasuoka wrote:
>On 30/06/2026 16:46, Dmitry Osipenko wrote:
>> Could you please move drm_kms_helper_hotplug_event() to virtio_gpu_init(),
>> placing it after wait_event_timeout(display_info_pending)? This will avoid
>> additional work_struct that otherwise needs to be cancelled in
>> virtio_gpu_init() on the timeout.
>
>IIUC, moving the drm_kms_helper_hotplug_event() and _hpd_irq_event()
>into virtio_gpu_init() after wait_event_timeout() would not prevent the
>issue.
>
>Looking at the syzbot call traces[1][2], the deadlock occurs during
>drm_client_setup(), which runs after virtio_gpu_init() has already
>returned. The display_info_cb that triggers the deadlock is called from
>the dequeue worker while drm_client_register() holds clientlist_mutex.
>
>Thread A:
>virtio_gpu_probe()
> -> virtio_gpu_init() // sends GET_DISPLAY_INFO and waits up to 5s
> -> drm_dev_register()
> -> drm_client_setup() // deadlock happens HERE
> -> drm_client_register() // holds clientlist_mutex
> ...
> -> virtio_gpu_queue_fenced_ctrl_buffer()
> -> wait_event() // waits for free space
>
>Thread B:
>virtio_gpu_dequeue_ctrl_func()
> -> reclaim_vbufs() // make free space
I wonder if the deadlock could be cured by alternatively adding a wakeup
before the responce cb, given that free space is available.
> -> resp_cb()
> -> virtio_gpu_cmd_get_display_info_cb
> -> drm_helper_hpd_irq_event()
> -> drm_kms_helper_hotplug_event()
> -> drm_client_dev_hotplug() // need to lock clientlist_mutex
> -> wake_up() // never reached
>
>IIUC the hotplug notification in display_info_cb is needed because it
>notifies DRM after updating by the callback with fresh data from the host.
>
>This work_struct ensures display_info_cb never blocks on
>clientlist_mutex in the dequeue worker, while preserving the hotplug
>notification with fresh data.
>
>[1] https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
>[2] https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
>
>Best regards,
>Ryosuke
^ permalink raw reply
* [PATCH v5 51/51] x86/kvm: Get local APIC bus frequency from PV CPUID Timing Info
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
When running as a KVM guest with PV timing info provided by the host,
stuff the APIC timer period/frequency with the local APIC bus frequency
reported in CPUID.0x40000010.EBX instead of trying to calibrate/guess the
frequency.
See Documentation/virt/kvm/x86/cpuid.rst for details.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/kvm.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index f9a6346077b0..beea0b6aa78e 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -990,7 +990,7 @@ static void __init kvm_init_platform(void)
.mask_lo = (u32)(~(SZ_4G - tolud - 1)) | MTRR_PHYSMASK_V,
.mask_hi = (BIT_ULL(boot_cpu_data.x86_phys_bits) - 1) >> 32,
};
- u32 timing_info_leaf;
+ u32 timing_info_leaf, apic_khz;
bool tsc_is_reliable;
if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT) &&
@@ -1052,6 +1052,11 @@ static void __init kvm_init_platform(void)
x86_init.hyper.get_tsc_khz = kvm_get_tsc_khz;
x86_init.hyper.get_cpu_khz = kvm_get_tsc_khz;
}
+
+ /* The leaf also includes the local APIC bus/timer frequency.*/
+ apic_khz = cpuid_ebx(timing_info_leaf);
+ if (apic_khz)
+ apic_set_timer_period_khz(apic_khz, "KVM hypervisor");
}
/*
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 50/51] x86/paravirt: Move using_native_sched_clock() stub into timer.h
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Now that timer.h ended up with CONFIG_PARAVIRT #ifdeffery anyways, move the
PARAVIRT=n using_native_sched_clock() stub into timer.h as a "free"
optimization.
No functional change intended.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/timer.h | 6 ++++--
arch/x86/kernel/tsc.c | 2 --
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/timer.h b/arch/x86/include/asm/timer.h
index ca5c95d48c03..a52388af6055 100644
--- a/arch/x86/include/asm/timer.h
+++ b/arch/x86/include/asm/timer.h
@@ -11,9 +11,9 @@ extern void recalibrate_cpu_khz(void);
extern int no_timer_check;
-extern bool using_native_sched_clock(void);
-
#ifdef CONFIG_PARAVIRT
+extern bool using_native_sched_clock(void);
+
int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
void (*save)(void), void (*restore)(void));
@@ -23,6 +23,8 @@ static __always_inline void paravirt_set_sched_clock(u64 (*func)(void),
{
(void)__paravirt_set_sched_clock(func, true, save, restore);
}
+#else
+static inline bool using_native_sched_clock(void) { return true; }
#endif
/*
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index a146fc7b5e74..564be4faa5a0 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -302,8 +302,6 @@ int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
}
#else
u64 sched_clock_noinstr(void) __attribute__((alias("native_sched_clock")));
-
-bool using_native_sched_clock(void) { return true; }
#endif
notrace u64 sched_clock(void)
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 49/51] x86/kvmclock: Plumb in AP-online and BSP-resume to kvmlock, for documentation
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Invoke kvmclock_cpu_action() with AP_ONLINE and BSP_RESUME, even though
kvmclock doesn't need to do anything in either case, so that the asymmetry
of kvmclock is a detail buried in kvmclock, and to explicitly document
that doing nothing during those phases is intentional and correct.
For all intents and purposes, no functional change intended.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/kvm_para.h | 2 ++
arch/x86/kernel/kvm.c | 22 +++++++++++++-------
arch/x86/kernel/kvmclock.c | 37 ++++++++++++++++++++++++++-------
3 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index 08686ff19caa..763ed017738a 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -120,6 +120,8 @@ static inline long kvm_sev_hypercall3(unsigned int nr, unsigned long p1,
#ifdef CONFIG_KVM_GUEST
enum kvm_guest_cpu_action {
KVM_GUEST_BSP_SUSPEND,
+ KVM_GUEST_BSP_RESUME,
+ KVM_GUEST_AP_ONLINE,
KVM_GUEST_AP_OFFLINE,
KVM_GUEST_SHUTDOWN,
};
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 604b52f233aa..f9a6346077b0 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -474,18 +474,24 @@ static void kvm_guest_cpu_offline(enum kvm_guest_cpu_action action)
kvmclock_cpu_action(action);
}
+static void __kvm_cpu_online(unsigned int cpu, enum kvm_guest_cpu_action action)
+{
+ unsigned long flags;
+
+ local_irq_save(flags);
+ kvmclock_cpu_action(action);
+ kvm_guest_cpu_init();
+ local_irq_restore(flags);
+}
+
+#ifdef CONFIG_SMP
+
static int kvm_cpu_online(unsigned int cpu)
{
- unsigned long flags;
-
- local_irq_save(flags);
- kvm_guest_cpu_init();
- local_irq_restore(flags);
+ __kvm_cpu_online(cpu, KVM_GUEST_AP_ONLINE);
return 0;
}
-#ifdef CONFIG_SMP
-
static DEFINE_PER_CPU(cpumask_var_t, __pv_cpu_mask);
static bool pv_tlb_flush_supported(void)
@@ -750,7 +756,7 @@ static int kvm_suspend(void *data)
static void kvm_resume(void *data)
{
- kvm_cpu_online(raw_smp_processor_id());
+ __kvm_cpu_online(raw_smp_processor_id(), KVM_GUEST_BSP_RESUME);
#ifdef CONFIG_ARCH_CPUIDLE_HALTPOLL
if (kvm_para_has_feature(KVM_FEATURE_POLL_CONTROL) && has_guest_poll)
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index bc98ebb8587d..842f38c5f6ca 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -150,7 +150,7 @@ static void kvm_save_sched_clock_state(void)
#ifdef CONFIG_SMP
static void kvm_setup_secondary_clock(void)
{
- kvm_register_clock("secondary cpu clock");
+ kvm_register_clock("secondary cpu, startup");
}
#endif
@@ -174,13 +174,34 @@ static void kvmclock_resume(struct clocksource *cs)
void kvmclock_cpu_action(enum kvm_guest_cpu_action action)
{
- /*
- * Don't disable kvmclock on the BSP during suspend. If kvmclock is
- * being used for sched_clock, then it needs to be kept alive until the
- * last minute, and restored as quickly as possible after resume.
- */
- if (action != KVM_GUEST_BSP_SUSPEND)
+ switch (action) {
+ /*
+ * The BSP's clock is managed via clocksource suspend/resume,
+ * to ensure it's enabled/disabled when timekeeping needs it
+ * to be, e.g. before reading wallclock (which uses kvmclock).
+ */
+ case KVM_GUEST_BSP_SUSPEND:
+ case KVM_GUEST_BSP_RESUME:
+ break;
+ case KVM_GUEST_AP_ONLINE:
+ /*
+ * Secondary CPUs use a dedicated hook to enable kvmclock early
+ * during bringup, there's nothing to be done during CPU online
+ * (which runs at CPUHP_AP_ONLINE_DYN). When kvmclock is being
+ * used as sched_clock, kvmclock must be enabled *very* early,
+ * and even when kvmclock is "only" being used for the main
+ * clocksource, it still needs to be enabled long before the
+ * dynamic CPUHP calls are made.
+ */
+ break;
+ case KVM_GUEST_AP_OFFLINE:
+ case KVM_GUEST_SHUTDOWN:
kvmclock_disable();
+ break;
+ default:
+ WARN_ON_ONCE(1);
+ break;
+ }
}
/*
@@ -382,7 +403,7 @@ void __init kvmclock_init(bool prefer_tsc)
msr_kvm_system_time, msr_kvm_wall_clock);
this_cpu_write(hv_clock_per_cpu, &hv_clock_boot[0]);
- kvm_register_clock("primary cpu clock");
+ kvm_register_clock("primary cpu, online");
pvclock_set_pvti_cpu0_va(hv_clock_boot);
if (kvm_para_has_feature(KVM_FEATURE_CLOCKSOURCE_STABLE_BIT)) {
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 47/51] x86/paravirt: Don't use a PV sched_clock in CoCo guests with trusted TSC
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Silently ignore attempts to switch to a paravirt sched_clock when running
as a CoCo guest with trusted TSC. In hand-wavy theory, a misbehaving
hypervisor could attack the guest by manipulating the PV clock to affect
guest scheduling in some weird and/or predictable way. More importantly,
reading TSC on such platforms is faster than any PV clock, and sched_clock
is all about speed.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/tsc.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 012321fed5e5..a146fc7b5e74 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -283,6 +283,15 @@ bool using_native_sched_clock(void)
int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
void (*save)(void), void (*restore)(void))
{
+ /*
+ * Don't replace TSC with a PV clock when running as a CoCo guest and
+ * the TSC is secure/trusted; PV clocks are emulated by the hypervisor,
+ * which isn't in the guest's TCB.
+ */
+ if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC) ||
+ boot_cpu_has(X86_FEATURE_TDX_GUEST))
+ return -EPERM;
+
if (!stable)
clear_sched_clock_stable();
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 48/51] x86/kvmclock: Use TSC for sched_clock if it's constant and non-stop
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Prefer the TSC over kvmclock for sched_clock if the TSC is constant and
nonstop. I.e. use the same criteria as tweaking the clocksource rating so
that TSC is preferred over kvmclock. Per the below comment from
native_sched_clock(), sched_clock is more tolerant of slop than
clocksource; using TSC for clocksource but not sched_clock makes little to
no sense, especially now that KVM CoCo guests with a trusted TSC use TSC,
not kvmclock.
/*
* Fall back to jiffies if there's no TSC available:
* ( But note that we still use it if the TSC is marked
* unstable. We do this because unlike Time Of Day,
* the scheduler clock tolerates small errors and it's
* very important for it to be as fast as the platform
* can achieve it. )
*/
The only advantage of using kvmclock is that doing so allows for early
and common detection of PVCLOCK_GUEST_STOPPED, but that code has been
broken for over two years with nary a complaint, i.e. it can't be
_that_ valuable. And as above, certain types of KVM guests are losing
the functionality regardless, i.e. acknowledging PVCLOCK_GUEST_STOPPED
needs to be decoupled from sched_clock() no matter what.
Link: https://lore.kernel.org/all/Z4hDK27OV7wK572A@google.com
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/kvmclock.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 22e8855fcd4d..bc98ebb8587d 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -396,7 +396,6 @@ void __init kvmclock_init(bool prefer_tsc)
PVCLOCK_TSC_STABLE_BIT;
}
- kvm_sched_clock_init(stable);
if (!x86_init.hyper.get_tsc_khz)
x86_init.hyper.get_tsc_khz = kvmclock_get_tsc_khz;
@@ -416,6 +415,8 @@ void __init kvmclock_init(bool prefer_tsc)
*/
if (prefer_tsc)
kvm_clock.rating = 299;
+ else
+ kvm_sched_clock_init(stable);
clocksource_register_hz(&kvm_clock, NSEC_PER_SEC);
pv_info.name = "KVM";
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 46/51] x86/paravirt: Plumb a return code into __paravirt_set_sched_clock()
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Add a return code to __paravirt_set_sched_clock() so that the kernel can
reject attempts to use a PV sched_clock without breaking the caller. E.g.
when running as a CoCo VM with a secure TSC, using a PV clock is generally
undesirable.
Note, kvmclock is the only PV clock that does anything "extra" beyond
simply registering itself as sched_clock, i.e. is the only caller that
needs to check the new return value.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/timer.h | 6 +++---
arch/x86/kernel/kvmclock.c | 9 ++++++---
arch/x86/kernel/tsc.c | 5 +++--
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/timer.h b/arch/x86/include/asm/timer.h
index 96ae7feac47c..ca5c95d48c03 100644
--- a/arch/x86/include/asm/timer.h
+++ b/arch/x86/include/asm/timer.h
@@ -14,14 +14,14 @@ extern int no_timer_check;
extern bool using_native_sched_clock(void);
#ifdef CONFIG_PARAVIRT
-void __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
- void (*save)(void), void (*restore)(void));
+int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
+ void (*save)(void), void (*restore)(void));
static __always_inline void paravirt_set_sched_clock(u64 (*func)(void),
void (*save)(void),
void (*restore)(void))
{
- __paravirt_set_sched_clock(func, true, save, restore);
+ (void)__paravirt_set_sched_clock(func, true, save, restore);
}
#endif
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 2cc3dd2ba355..22e8855fcd4d 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -332,10 +332,13 @@ static int kvmclock_setup_percpu(unsigned int cpu)
static __init void kvm_sched_clock_init(bool stable)
{
+ /* Ensure the offset is configured before making kvmclock visible! */
kvm_sched_clock_offset = kvm_clock_read();
- __paravirt_set_sched_clock(kvm_sched_clock_read, stable,
- kvm_save_sched_clock_state,
- kvm_restore_sched_clock_state);
+
+ if (__paravirt_set_sched_clock(kvm_sched_clock_read, stable,
+ kvm_save_sched_clock_state,
+ kvm_restore_sched_clock_state))
+ return;
/*
* The BSP's clock is managed via dedicated sched_clock save/restore
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 0f92b29adecc..012321fed5e5 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -280,8 +280,8 @@ bool using_native_sched_clock(void)
return static_call_query(pv_sched_clock) == native_sched_clock;
}
-void __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
- void (*save)(void), void (*restore)(void))
+int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
+ void (*save)(void), void (*restore)(void))
{
if (!stable)
clear_sched_clock_stable();
@@ -289,6 +289,7 @@ void __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
static_call_update(pv_sched_clock, func);
x86_platform.save_sched_clock_state = save;
x86_platform.restore_sched_clock_state = restore;
+ return 0;
}
#else
u64 sched_clock_noinstr(void) __attribute__((alias("native_sched_clock")));
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 45/51] x86/paravirt: Mark __paravirt_set_sched_clock() as __init
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Annotate __paravirt_set_sched_clock() as __init, and make its wrapper
__always_inline to ensure sanitizers don't result in a non-inline version
hanging around. All callers run during __init, and changing sched_clock
after boot would be all kinds of crazy.
No functional change intended.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/timer.h | 10 +++++-----
arch/x86/kernel/tsc.c | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/timer.h b/arch/x86/include/asm/timer.h
index e97cd1ae03d1..96ae7feac47c 100644
--- a/arch/x86/include/asm/timer.h
+++ b/arch/x86/include/asm/timer.h
@@ -14,12 +14,12 @@ extern int no_timer_check;
extern bool using_native_sched_clock(void);
#ifdef CONFIG_PARAVIRT
-void __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
- void (*save)(void), void (*restore)(void));
+void __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
+ void (*save)(void), void (*restore)(void));
-static inline void paravirt_set_sched_clock(u64 (*func)(void),
- void (*save)(void),
- void (*restore)(void))
+static __always_inline void paravirt_set_sched_clock(u64 (*func)(void),
+ void (*save)(void),
+ void (*restore)(void))
{
__paravirt_set_sched_clock(func, true, save, restore);
}
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 83353d643150..0f92b29adecc 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -280,8 +280,8 @@ bool using_native_sched_clock(void)
return static_call_query(pv_sched_clock) == native_sched_clock;
}
-void __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
- void (*save)(void), void (*restore)(void))
+void __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
+ void (*save)(void), void (*restore)(void))
{
if (!stable)
clear_sched_clock_stable();
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 44/51] x86/kvmclock: WARN if wall clock is read while kvmclock is suspended
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
WARN if kvmclock is still suspended when its wallclock is read, i.e. when
the kernel reads its persistent clock. The wallclock subtly depends on
the BSP's kvmclock being enabled, and returns garbage if kvmclock is
disabled.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/kvmclock.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 41aff709b90a..2cc3dd2ba355 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -53,6 +53,8 @@ static struct pvclock_vsyscall_time_info *hvclock_mem;
DEFINE_PER_CPU(struct pvclock_vsyscall_time_info *, hv_clock_per_cpu);
EXPORT_PER_CPU_SYMBOL_GPL(hv_clock_per_cpu);
+static bool kvmclock_suspended;
+
/*
* The wallclock is the time of day when we booted. Since then, some time may
* have elapsed since the hypervisor wrote the data. So we try to account for
@@ -60,6 +62,7 @@ EXPORT_PER_CPU_SYMBOL_GPL(hv_clock_per_cpu);
*/
static void kvm_get_wallclock(struct timespec64 *now)
{
+ WARN_ON_ONCE(kvmclock_suspended);
wrmsrq(msr_kvm_wall_clock, slow_virt_to_phys(&wall_clock));
preempt_disable();
pvclock_read_wallclock(&wall_clock, this_cpu_pvti(), now);
@@ -140,6 +143,7 @@ static void kvm_save_sched_clock_state(void)
* to the old address prior to reconfiguring kvmclock would clobber
* random memory.
*/
+ kvmclock_suspended = true;
kvmclock_disable();
}
@@ -152,16 +156,19 @@ static void kvm_setup_secondary_clock(void)
static void kvm_restore_sched_clock_state(void)
{
+ kvmclock_suspended = false;
kvm_register_clock("primary cpu, sched_clock resume");
}
static void kvmclock_suspend(struct clocksource *cs)
{
+ kvmclock_suspended = true;
kvmclock_disable();
}
static void kvmclock_resume(struct clocksource *cs)
{
+ kvmclock_suspended = false;
kvm_register_clock("primary cpu, clocksource resume");
}
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 43/51] x86/kvmclock: Hook clocksource.suspend/resume when kvmclock isn't sched_clock
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Save/restore kvmclock across suspend/resume via clocksource hooks when
kvmclock isn't being used for sched_clock. This will allow using kvmclock
as a clocksource (or for wallclock!) without also using it for sched_clock.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/kvmclock.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 61d4d943fe74..41aff709b90a 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -152,7 +152,17 @@ static void kvm_setup_secondary_clock(void)
static void kvm_restore_sched_clock_state(void)
{
- kvm_register_clock("primary cpu clock, resume");
+ kvm_register_clock("primary cpu, sched_clock resume");
+}
+
+static void kvmclock_suspend(struct clocksource *cs)
+{
+ kvmclock_disable();
+}
+
+static void kvmclock_resume(struct clocksource *cs)
+{
+ kvm_register_clock("primary cpu, clocksource resume");
}
void kvmclock_cpu_action(enum kvm_guest_cpu_action action)
@@ -223,6 +233,8 @@ static struct clocksource kvm_clock = {
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
.id = CSID_X86_KVM_CLK,
.enable = kvm_cs_enable,
+ .suspend = kvmclock_suspend,
+ .resume = kvmclock_resume,
};
static void __init kvmclock_init_mem(void)
@@ -318,6 +330,15 @@ static __init void kvm_sched_clock_init(bool stable)
kvm_save_sched_clock_state,
kvm_restore_sched_clock_state);
+ /*
+ * The BSP's clock is managed via dedicated sched_clock save/restore
+ * hooks when kvmclock is used as sched_clock, as sched_clock needs to
+ * be kept alive until the very end of suspend entry, and restored as
+ * quickly as possible after resume.
+ */
+ kvm_clock.suspend = NULL;
+ kvm_clock.resume = NULL;
+
pr_info("kvm-clock: using sched offset of %llu cycles",
kvm_sched_clock_offset);
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 42/51] timekeeping: Resume clocksources before reading persistent clock
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
When resuming timekeeping after suspend, restore clocksources prior to
reading the persistent clock. Paravirt clocks, e.g. kvmclock, tie the
validity of a PV persistent clock to a clocksource, i.e. reading the PV
persistent clock will return garbage if the underlying PV clocksource
hasn't been enabled. The flaw has gone unnoticed because kvmclock is a
mess and uses its own suspend/resume hooks instead of the clocksource
suspend/resume hooks, which happens to work by sheer dumb luck (the
kvmclock resume hook runs before timekeeping_resume()).
Note, there is no evidence that any clocksource supported by the kernel
depends on a persistent clock.
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
kernel/time/timekeeping.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index b1b5ec43c0f2..5bc77d36c7a3 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -2180,11 +2180,16 @@ void timekeeping_resume(void)
u64 cycle_now, nsec;
unsigned long flags;
- read_persistent_clock64(&ts_new);
-
clockevents_resume();
clocksource_resume();
+ /*
+ * Read persistent time after clocksources have been resumed. Paravirt
+ * clocks have a nasty habit of piggybacking a persistent clock on a
+ * system clock, and may return garbage if the system clock is suspended.
+ */
+ read_persistent_clock64(&ts_new);
+
raw_spin_lock_irqsave(&tk_core.lock, flags);
/*
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 41/51] x86/kvmclock: Refactor handling of PVCLOCK_TSC_STABLE_BIT during kvmclock_init()
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Clean up the setting of PVCLOCK_TSC_STABLE_BIT during kvmclock init to
make it somewhat obvious that pvclock_read_flags() must be called *after*
pvclock_set_flags().
Note, in theory, a different PV clock could have set PVCLOCK_TSC_STABLE_BIT
in the supported flags, i.e. reading flags only if
KVM_FEATURE_CLOCKSOURCE_STABLE_BIT is set could very, very theoretically
result in a change in behavior. In practice, the kernel only supports a
single PV clock.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/kvmclock.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 5220d205abc7..61d4d943fe74 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -327,7 +327,7 @@ static __init void kvm_sched_clock_init(bool stable)
void __init kvmclock_init(bool prefer_tsc)
{
- u8 flags;
+ bool stable = false;
if (!kvm_para_available() || !kvmclock)
return;
@@ -354,11 +354,18 @@ void __init kvmclock_init(bool prefer_tsc)
kvm_register_clock("primary cpu clock");
pvclock_set_pvti_cpu0_va(hv_clock_boot);
- if (kvm_para_has_feature(KVM_FEATURE_CLOCKSOURCE_STABLE_BIT))
+ if (kvm_para_has_feature(KVM_FEATURE_CLOCKSOURCE_STABLE_BIT)) {
pvclock_set_flags(PVCLOCK_TSC_STABLE_BIT);
- flags = pvclock_read_flags(&hv_clock_boot[0].pvti);
- kvm_sched_clock_init(flags & PVCLOCK_TSC_STABLE_BIT);
+ /*
+ * Check if the clock is stable *after* marking TSC_STABLE as a
+ * valid flag.
+ */
+ stable = pvclock_read_flags(&hv_clock_boot[0].pvti) &
+ PVCLOCK_TSC_STABLE_BIT;
+ }
+
+ kvm_sched_clock_init(stable);
if (!x86_init.hyper.get_tsc_khz)
x86_init.hyper.get_tsc_khz = kvmclock_get_tsc_khz;
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 40/51] x86/pvclock: WARN if pvclock's valid_flags are overwritten
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
WARN if the common PV clock valid_flags are overwritten; all PV clocks
expect that they are the one and only PV clock, i.e. don't guard against
another PV clock having modified the flags.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/pvclock.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
index a51adce67f92..8d098841a225 100644
--- a/arch/x86/kernel/pvclock.c
+++ b/arch/x86/kernel/pvclock.c
@@ -21,6 +21,7 @@ static struct pvclock_vsyscall_time_info *pvti_cpu0_va __ro_after_init;
void __init pvclock_set_flags(u8 flags)
{
+ WARN_ON(valid_flags);
valid_flags = flags;
}
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 39/51] x86/pvclock: Mark setup helpers and related various as __init/__ro_after_init
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Now that Xen PV clock and kvmclock explicitly do setup only during init,
tag the common PV clock flags/vsyscall variables and their mutators with
__init.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/pvclock.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
index b3f81379c2fc..a51adce67f92 100644
--- a/arch/x86/kernel/pvclock.c
+++ b/arch/x86/kernel/pvclock.c
@@ -16,10 +16,10 @@
#include <asm/pvclock.h>
#include <asm/vgtod.h>
-static u8 valid_flags __read_mostly = 0;
-static struct pvclock_vsyscall_time_info *pvti_cpu0_va __read_mostly;
+static u8 valid_flags __ro_after_init = 0;
+static struct pvclock_vsyscall_time_info *pvti_cpu0_va __ro_after_init;
-void pvclock_set_flags(u8 flags)
+void __init pvclock_set_flags(u8 flags)
{
valid_flags = flags;
}
@@ -153,7 +153,7 @@ void pvclock_read_wallclock(struct pvclock_wall_clock *wall_clock,
set_normalized_timespec64(ts, now.tv_sec, now.tv_nsec);
}
-void pvclock_set_pvti_cpu0_va(struct pvclock_vsyscall_time_info *pvti)
+void __init pvclock_set_pvti_cpu0_va(struct pvclock_vsyscall_time_info *pvti)
{
WARN_ON(vclock_was_used(VDSO_CLOCKMODE_PVCLOCK));
pvti_cpu0_va = pvti;
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 38/51] x86/xen/time: Mark xen_setup_vsyscall_time_info() as __init
From: Sean Christopherson @ 2026-07-01 19:31 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Annotate xen_setup_vsyscall_time_info() as being used only during kernel
initialization; it's called only by xen_time_init(), which is already
tagged __init.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/xen/time.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index 8cd8bfaf1320..bc26f00fc53e 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -443,7 +443,7 @@ void xen_restore_time_memory_area(void)
xen_sched_clock_offset = xen_clocksource_read() - xen_clock_value_saved;
}
-static void xen_setup_vsyscall_time_info(void)
+static void __init xen_setup_vsyscall_time_info(void)
{
struct vcpu_register_time_memory_area t;
struct pvclock_vsyscall_time_info *ti;
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 37/51] x86/kvmclock: Move kvm_sched_clock_init() down in kvmclock.c
From: Sean Christopherson @ 2026-07-01 19:31 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Move kvm_sched_clock_init() "down" so that it can reference the global
kvm_clock structure without needing a forward declaration.
Opportunistically mark the helper as "__init" instead of "inline" to make
its usage more obvious; modern compilers don't need a hint to inline a
single-use function, and an extra CALL+RET pair during boot is a complete
non-issue. And, if the compiler ignores the hint and does NOT inline the
function, the resulting code may not get discarded after boot due lack of
an __init annotation.
No functional change intended.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/kvmclock.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 5b9955343199..5220d205abc7 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -155,20 +155,6 @@ static void kvm_restore_sched_clock_state(void)
kvm_register_clock("primary cpu clock, resume");
}
-static inline void kvm_sched_clock_init(bool stable)
-{
- kvm_sched_clock_offset = kvm_clock_read();
- __paravirt_set_sched_clock(kvm_sched_clock_read, stable,
- kvm_save_sched_clock_state,
- kvm_restore_sched_clock_state);
-
- pr_info("kvm-clock: using sched offset of %llu cycles",
- kvm_sched_clock_offset);
-
- BUILD_BUG_ON(sizeof(kvm_sched_clock_offset) >
- sizeof(((struct pvclock_vcpu_time_info *)NULL)->system_time));
-}
-
void kvmclock_cpu_action(enum kvm_guest_cpu_action action)
{
/*
@@ -325,6 +311,20 @@ static int kvmclock_setup_percpu(unsigned int cpu)
return p ? 0 : -ENOMEM;
}
+static __init void kvm_sched_clock_init(bool stable)
+{
+ kvm_sched_clock_offset = kvm_clock_read();
+ __paravirt_set_sched_clock(kvm_sched_clock_read, stable,
+ kvm_save_sched_clock_state,
+ kvm_restore_sched_clock_state);
+
+ pr_info("kvm-clock: using sched offset of %llu cycles",
+ kvm_sched_clock_offset);
+
+ BUILD_BUG_ON(sizeof(kvm_sched_clock_offset) >
+ sizeof(((struct pvclock_vcpu_time_info *)NULL)->system_time));
+}
+
void __init kvmclock_init(bool prefer_tsc)
{
u8 flags;
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox