* [PATCH v3 03/10] drm/ingenic: Remove calls to drm_atomic_helper_check_plane_damage()
From: Thomas Zimmermann @ 2026-05-27 14:46 UTC (permalink / raw)
To: mripard, maarten.lankhorst, airlied, airlied, simona, admin,
gargaditya08, paul, jani.nikula, mhklinux, zack.rusin,
bcm-kernel-feedback-list
Cc: dri-devel, linux-hyperv, intel-gfx, intel-xe, linux-mips,
virtualization, Thomas Zimmermann
In-Reply-To: <20260527145113.241595-1-tzimmermann@suse.de>
Atomic helpers call drm_atomic_helper_check_plane_damage() after the
atomic_check anyway. See atomic_helper_check_planes(). Remove the calls
from the planes' atomic_check.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 3 ---
drivers/gpu/drm/ingenic/ingenic-ipu.c | 8 ++------
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 42c86f195c66..e99b44e3ac92 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -519,9 +519,6 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane,
old_plane_state->fb->format->format != new_plane_state->fb->format->format))
crtc_state->mode_changed = true;
- if (priv->soc_info->map_noncoherent)
- drm_atomic_helper_check_plane_damage(state, new_plane_state);
-
return 0;
}
diff --git a/drivers/gpu/drm/ingenic/ingenic-ipu.c b/drivers/gpu/drm/ingenic/ingenic-ipu.c
index 56143a191f36..fd17c642c7ac 100644
--- a/drivers/gpu/drm/ingenic/ingenic-ipu.c
+++ b/drivers/gpu/drm/ingenic/ingenic-ipu.c
@@ -594,7 +594,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
if (!new_plane_state->crtc ||
!crtc_state->mode.hdisplay || !crtc_state->mode.vdisplay)
- goto out_check_damage;
+ return 0;
/* Plane must be fully visible */
if (new_plane_state->crtc_x < 0 || new_plane_state->crtc_y < 0 ||
@@ -611,7 +611,7 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
if (!osd_changed(new_plane_state, old_plane_state))
- goto out_check_damage;
+ return 0;
crtc_state->mode_changed = true;
@@ -645,10 +645,6 @@ static int ingenic_ipu_plane_atomic_check(struct drm_plane *plane,
ipu_state->denom_w = denom_w;
ipu_state->denom_h = denom_h;
-out_check_damage:
- if (ingenic_drm_map_noncoherent(ipu->master))
- drm_atomic_helper_check_plane_damage(state, new_plane_state);
-
return 0;
}
--
2.54.0
^ permalink raw reply related
* [PATCH v3 04/10] drm/damage-helper: Test src coord in drm_atomic_helper_check_plane_damage()
From: Thomas Zimmermann @ 2026-05-27 14:46 UTC (permalink / raw)
To: mripard, maarten.lankhorst, airlied, airlied, simona, admin,
gargaditya08, paul, jani.nikula, mhklinux, zack.rusin,
bcm-kernel-feedback-list
Cc: dri-devel, linux-hyperv, intel-gfx, intel-xe, linux-mips,
virtualization, Thomas Zimmermann
In-Reply-To: <20260527145113.241595-1-tzimmermann@suse.de>
Planes require a full update if the source coordinates change across
atomic commits. Evaluate this during the atomic-check and set the flag
ignore_damage_clips in the plane state, if so. Remove the check from
drm_atomic_helper_damage_iter_init().
This will help with removing the old state from the atomic-commit phase
and simplify atomic_update helpers a bit.
Several unit tests check against the change of the src coordinate. Drop
them as they do no longer make sense. If the src coordinate changes across
commits, atomic helpers will set the plane state's ignore_damage_clips
flag, for which a separate unit test exists.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_atomic_helper.c | 4 +-
drivers/gpu/drm/drm_damage_helper.c | 20 ++-
.../gpu/drm/tests/drm_damage_helper_test.c | 151 ------------------
include/drm/drm_damage_helper.h | 3 +-
4 files changed, 16 insertions(+), 162 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 4c37299e8ccb..823ce60d45b4 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1034,7 +1034,7 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
drm_atomic_helper_plane_changed(state, old_plane_state, new_plane_state, plane);
- drm_atomic_helper_check_plane_damage(state, new_plane_state);
+ drm_atomic_helper_check_plane_damage(state, old_plane_state, new_plane_state);
if (!funcs || !funcs->atomic_check)
continue;
@@ -1066,7 +1066,7 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
}
for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
- drm_atomic_helper_check_plane_damage(state, new_plane_state);
+ drm_atomic_helper_check_plane_damage(state, old_plane_state, new_plane_state);
}
return ret;
diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c
index 945fac8dc27b..f492a59edbeb 100644
--- a/drivers/gpu/drm/drm_damage_helper.c
+++ b/drivers/gpu/drm/drm_damage_helper.c
@@ -55,7 +55,8 @@ static void convert_clip_rect_to_rect(const struct drm_clip_rect *src,
/**
* drm_atomic_helper_check_plane_damage - Verify plane damage on atomic_check.
* @state: The driver state object.
- * @plane_state: Plane state for which to verify damage.
+ * @old_plane_state: Old plane state to verify against.
+ * @new_plane_state: Plane state for which to verify damage.
*
* This helper function makes sure that damage from plane state is discarded
* for full modeset. If there are more reasons a driver would want to do a full
@@ -67,19 +68,23 @@ static void convert_clip_rect_to_rect(const struct drm_clip_rect *src,
* &drm_plane_state.src as damage.
*/
void drm_atomic_helper_check_plane_damage(struct drm_atomic_commit *state,
- struct drm_plane_state *plane_state)
+ const struct drm_plane_state *old_plane_state,
+ struct drm_plane_state *new_plane_state)
{
struct drm_crtc_state *crtc_state;
- if (plane_state->crtc) {
+ if (!drm_rect_equals(&new_plane_state->src, &old_plane_state->src))
+ new_plane_state->ignore_damage_clips = true;
+
+ if (new_plane_state->crtc) {
crtc_state = drm_atomic_get_new_crtc_state(state,
- plane_state->crtc);
+ new_plane_state->crtc);
if (WARN_ON(!crtc_state))
return;
if (drm_atomic_crtc_needs_modeset(crtc_state))
- plane_state->ignore_damage_clips = true;
+ new_plane_state->ignore_damage_clips = true;
}
}
EXPORT_SYMBOL(drm_atomic_helper_check_plane_damage);
@@ -204,7 +209,7 @@ EXPORT_SYMBOL(drm_atomic_helper_dirtyfb);
/**
* drm_atomic_helper_damage_iter_init - Initialize the damage iterator.
* @iter: The iterator to initialize.
- * @old_state: Old plane state for validation.
+ * @old_state: Unused, pass NULL.
* @state: Plane state from which to iterate the damage clips.
*
* Initialize an iterator, which clips plane damage
@@ -241,8 +246,7 @@ drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
iter->plane_src.x2 = (src.x2 >> 16) + !!(src.x2 & 0xFFFF);
iter->plane_src.y2 = (src.y2 >> 16) + !!(src.y2 & 0xFFFF);
- if (!iter->clips || state->ignore_damage_clips ||
- !drm_rect_equals(&state->src, &old_state->src)) {
+ if (!iter->clips || state->ignore_damage_clips) {
iter->clips = NULL;
iter->num_clips = 0;
iter->full_update = true;
diff --git a/drivers/gpu/drm/tests/drm_damage_helper_test.c b/drivers/gpu/drm/tests/drm_damage_helper_test.c
index 64f038a62ffe..ef931497baf9 100644
--- a/drivers/gpu/drm/tests/drm_damage_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_damage_helper_test.c
@@ -155,45 +155,6 @@ static void drm_test_damage_iter_no_damage_fractional_src(struct kunit *test)
check_damage_clip(test, &clip, 3, 3, 1028, 772);
}
-static void drm_test_damage_iter_no_damage_src_moved(struct kunit *test)
-{
- struct drm_damage_mock *mock = test->priv;
- struct drm_atomic_helper_damage_iter iter;
- struct drm_rect clip;
- u32 num_hits = 0;
-
- /* Plane src moved since old plane state. */
- set_plane_src(&mock->old_state, 0, 0, 1024 << 16, 768 << 16);
- set_plane_src(&mock->state, 10 << 16, 10 << 16,
- (10 + 1024) << 16, (10 + 768) << 16);
- drm_atomic_helper_damage_iter_init(&iter, &mock->old_state, &mock->state);
- drm_atomic_for_each_plane_damage(&iter, &clip)
- num_hits++;
-
- KUNIT_EXPECT_EQ_MSG(test, num_hits, 1, "Should return plane src as damage.");
- check_damage_clip(test, &clip, 10, 10, 1034, 778);
-}
-
-static void drm_test_damage_iter_no_damage_fractional_src_moved(struct kunit *test)
-{
- struct drm_damage_mock *mock = test->priv;
- struct drm_atomic_helper_damage_iter iter;
- struct drm_rect clip;
- u32 num_hits = 0;
-
- /* Plane src has fractional part and it moved since old plane state. */
- set_plane_src(&mock->old_state, 0x3fffe, 0x3fffe,
- 0x3fffe + (1024 << 16), 0x3fffe + (768 << 16));
- set_plane_src(&mock->state, 0x40002, 0x40002,
- 0x40002 + (1024 << 16), 0x40002 + (768 << 16));
- drm_atomic_helper_damage_iter_init(&iter, &mock->old_state, &mock->state);
- drm_atomic_for_each_plane_damage(&iter, &clip)
- num_hits++;
-
- KUNIT_EXPECT_EQ_MSG(test, num_hits, 1, "Should return plane src as damage.");
- check_damage_clip(test, &clip, 4, 4, 1029, 773);
-}
-
static void drm_test_damage_iter_no_damage_not_visible(struct kunit *test)
{
struct drm_damage_mock *mock = test->priv;
@@ -415,58 +376,6 @@ static void drm_test_damage_iter_single_damage_outside_fractional_src(struct kun
KUNIT_EXPECT_EQ_MSG(test, num_hits, 0, "Should have no damage.");
}
-static void drm_test_damage_iter_single_damage_src_moved(struct kunit *test)
-{
- struct drm_damage_mock *mock = test->priv;
- struct drm_atomic_helper_damage_iter iter;
- struct drm_property_blob damage_blob;
- struct drm_mode_rect damage;
- struct drm_rect clip;
- u32 num_hits = 0;
-
- /* Plane src moved since old plane state. */
- set_plane_src(&mock->old_state, 0, 0, 1024 << 16, 768 << 16);
- set_plane_src(&mock->state, 10 << 16, 10 << 16,
- (10 + 1024) << 16, (10 + 768) << 16);
- set_damage_clip(&damage, 20, 30, 256, 256);
- set_damage_blob(&damage_blob, &damage, sizeof(damage));
- set_plane_damage(&mock->state, &damage_blob);
- drm_atomic_helper_damage_iter_init(&iter, &mock->old_state, &mock->state);
- drm_atomic_for_each_plane_damage(&iter, &clip)
- num_hits++;
-
- KUNIT_EXPECT_EQ_MSG(test, num_hits, 1,
- "Should return plane src as damage.");
- check_damage_clip(test, &clip, 10, 10, 1034, 778);
-}
-
-static void drm_test_damage_iter_single_damage_fractional_src_moved(struct kunit *test)
-{
- struct drm_damage_mock *mock = test->priv;
- struct drm_atomic_helper_damage_iter iter;
- struct drm_property_blob damage_blob;
- struct drm_mode_rect damage;
- struct drm_rect clip;
- u32 num_hits = 0;
-
- /* Plane src with fractional part moved since old plane state. */
- set_plane_src(&mock->old_state, 0x3fffe, 0x3fffe,
- 0x3fffe + (1024 << 16), 0x3fffe + (768 << 16));
- set_plane_src(&mock->state, 0x40002, 0x40002,
- 0x40002 + (1024 << 16), 0x40002 + (768 << 16));
- /* Damage intersect with plane src. */
- set_damage_clip(&damage, 20, 30, 1360, 256);
- set_damage_blob(&damage_blob, &damage, sizeof(damage));
- set_plane_damage(&mock->state, &damage_blob);
- drm_atomic_helper_damage_iter_init(&iter, &mock->old_state, &mock->state);
- drm_atomic_for_each_plane_damage(&iter, &clip)
- num_hits++;
-
- KUNIT_EXPECT_EQ_MSG(test, num_hits, 1,
- "Should return rounded off plane as damage.");
- check_damage_clip(test, &clip, 4, 4, 1029, 773);
-}
-
static void drm_test_damage_iter_damage(struct kunit *test)
{
struct drm_damage_mock *mock = test->priv;
@@ -549,60 +458,6 @@ static void drm_test_damage_iter_damage_one_outside(struct kunit *test)
check_damage_clip(test, &clip, 240, 200, 280, 250);
}
-static void drm_test_damage_iter_damage_src_moved(struct kunit *test)
-{
- struct drm_damage_mock *mock = test->priv;
- struct drm_atomic_helper_damage_iter iter;
- struct drm_property_blob damage_blob;
- struct drm_mode_rect damage[2];
- struct drm_rect clip;
- u32 num_hits = 0;
-
- set_plane_src(&mock->old_state, 0x40002, 0x40002,
- 0x40002 + (1024 << 16), 0x40002 + (768 << 16));
- set_plane_src(&mock->state, 0x3fffe, 0x3fffe,
- 0x3fffe + (1024 << 16), 0x3fffe + (768 << 16));
- /* 2 damage clips, one outside plane src. */
- set_damage_clip(&damage[0], 1360, 1360, 1380, 1380);
- set_damage_clip(&damage[1], 240, 200, 280, 250);
- set_damage_blob(&damage_blob, &damage[0], sizeof(damage));
- set_plane_damage(&mock->state, &damage_blob);
- drm_atomic_helper_damage_iter_init(&iter, &mock->old_state, &mock->state);
- drm_atomic_for_each_plane_damage(&iter, &clip)
- num_hits++;
-
- KUNIT_EXPECT_EQ_MSG(test, num_hits, 1,
- "Should return round off plane src as damage.");
- check_damage_clip(test, &clip, 3, 3, 1028, 772);
-}
-
-static void drm_test_damage_iter_damage_not_visible(struct kunit *test)
-{
- struct drm_damage_mock *mock = test->priv;
- struct drm_atomic_helper_damage_iter iter;
- struct drm_property_blob damage_blob;
- struct drm_mode_rect damage[2];
- struct drm_rect clip;
- u32 num_hits = 0;
-
- mock->state.visible = false;
-
- set_plane_src(&mock->old_state, 0x40002, 0x40002,
- 0x40002 + (1024 << 16), 0x40002 + (768 << 16));
- set_plane_src(&mock->state, 0x3fffe, 0x3fffe,
- 0x3fffe + (1024 << 16), 0x3fffe + (768 << 16));
- /* 2 damage clips, one outside plane src. */
- set_damage_clip(&damage[0], 1360, 1360, 1380, 1380);
- set_damage_clip(&damage[1], 240, 200, 280, 250);
- set_damage_blob(&damage_blob, &damage[0], sizeof(damage));
- set_plane_damage(&mock->state, &damage_blob);
- drm_atomic_helper_damage_iter_init(&iter, &mock->old_state, &mock->state);
- drm_atomic_for_each_plane_damage(&iter, &clip)
- num_hits++;
-
- KUNIT_EXPECT_EQ_MSG(test, num_hits, 0, "Should not return any damage.");
-}
-
static void drm_test_damage_iter_damage_ignore(struct kunit *test)
{
struct drm_damage_mock *mock = test->priv;
@@ -633,8 +488,6 @@ static void drm_test_damage_iter_damage_ignore(struct kunit *test)
static struct kunit_case drm_damage_helper_tests[] = {
KUNIT_CASE(drm_test_damage_iter_no_damage),
KUNIT_CASE(drm_test_damage_iter_no_damage_fractional_src),
- KUNIT_CASE(drm_test_damage_iter_no_damage_src_moved),
- KUNIT_CASE(drm_test_damage_iter_no_damage_fractional_src_moved),
KUNIT_CASE(drm_test_damage_iter_no_damage_not_visible),
KUNIT_CASE(drm_test_damage_iter_no_damage_no_crtc),
KUNIT_CASE(drm_test_damage_iter_no_damage_no_fb),
@@ -645,13 +498,9 @@ static struct kunit_case drm_damage_helper_tests[] = {
KUNIT_CASE(drm_test_damage_iter_single_damage_fractional_src),
KUNIT_CASE(drm_test_damage_iter_single_damage_intersect_fractional_src),
KUNIT_CASE(drm_test_damage_iter_single_damage_outside_fractional_src),
- KUNIT_CASE(drm_test_damage_iter_single_damage_src_moved),
- KUNIT_CASE(drm_test_damage_iter_single_damage_fractional_src_moved),
KUNIT_CASE(drm_test_damage_iter_damage),
KUNIT_CASE(drm_test_damage_iter_damage_one_intersect),
KUNIT_CASE(drm_test_damage_iter_damage_one_outside),
- KUNIT_CASE(drm_test_damage_iter_damage_src_moved),
- KUNIT_CASE(drm_test_damage_iter_damage_not_visible),
KUNIT_CASE(drm_test_damage_iter_damage_ignore),
{ }
};
diff --git a/include/drm/drm_damage_helper.h b/include/drm/drm_damage_helper.h
index 3661aeab2cd3..e93eaa0fbcb6 100644
--- a/include/drm/drm_damage_helper.h
+++ b/include/drm/drm_damage_helper.h
@@ -65,7 +65,8 @@ struct drm_atomic_helper_damage_iter {
};
void drm_atomic_helper_check_plane_damage(struct drm_atomic_commit *state,
- struct drm_plane_state *plane_state);
+ const struct drm_plane_state *old_plane_state,
+ struct drm_plane_state *new_plane_state);
int drm_atomic_helper_dirtyfb(struct drm_framebuffer *fb,
struct drm_file *file_priv, unsigned int flags,
unsigned int color, struct drm_clip_rect *clips,
--
2.54.0
^ permalink raw reply related
* [PATCH v3 02/10] drm/atomic-helpers: Evaluate plane damage after atomic_check
From: Thomas Zimmermann @ 2026-05-27 14:46 UTC (permalink / raw)
To: mripard, maarten.lankhorst, airlied, airlied, simona, admin,
gargaditya08, paul, jani.nikula, mhklinux, zack.rusin,
bcm-kernel-feedback-list
Cc: dri-devel, linux-hyperv, intel-gfx, intel-xe, linux-mips,
virtualization, Thomas Zimmermann
In-Reply-To: <20260527145113.241595-1-tzimmermann@suse.de>
Each plane's and CRTC's atomic_check might trigger a full modeset. As
this affects the plane's damage handling, evaluate damage clips after
running the atomic_check helpers.
Examples can be found in a number of drivers, such as ast, gud, ingenic,
mgag200 or vmwgfx, which all set mode_changed in the CRTC state to true.
Ingenic even reevaluates damage information in its plane's atomic_check.
Doing this after the atomic_check helpers ran benefits all drivers.
There's already a damage evaluation before the calls to atomic_check.
With a few fixes to drivers, this can be removed.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_atomic_helper.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 51f39edc31ed..4c37299e8ccb 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1065,6 +1065,10 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
}
}
+ for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
+ drm_atomic_helper_check_plane_damage(state, new_plane_state);
+ }
+
return ret;
}
EXPORT_SYMBOL(drm_atomic_helper_check_planes);
--
2.54.0
^ permalink raw reply related
* [PATCH v3 01/10] drm/damage-helper: Do not alter damage clips on modeset, but ignore them
From: Thomas Zimmermann @ 2026-05-27 14:46 UTC (permalink / raw)
To: mripard, maarten.lankhorst, airlied, airlied, simona, admin,
gargaditya08, paul, jani.nikula, mhklinux, zack.rusin,
bcm-kernel-feedback-list
Cc: dri-devel, linux-hyperv, intel-gfx, intel-xe, linux-mips,
virtualization, Thomas Zimmermann, stable
In-Reply-To: <20260527145113.241595-1-tzimmermann@suse.de>
User space supplies rectangles for damage clipping in a plane property.
For full mode sets, we still require a full plane update. In this case,
leave the information as-is and set the ignore_damage_clips flag instead.
The damage iterator will later ignore any damage information.
Also fixes a bug where ignore_damage_clips was not cleared across plane
duplications.
Leaving the damage information as-is might be helpful to drivers that
benefit from it even on full modesets (e.g., for cache management). It
will also help with consolidating the damage-handling logic.
Also add a new unit test that evaluates the ignore_damage_clips flag. It
sets two damage clips plus the flag and tests if the reported damage
covers the entire framebuffer.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 35ed38d58257 ("drm: Allow drivers to indicate the damage helpers to ignore damage clips")
Cc: dri-devel@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v6.10+
---
drivers/gpu/drm/drm_atomic_state_helper.c | 1 +
drivers/gpu/drm/drm_damage_helper.c | 6 ++--
.../gpu/drm/tests/drm_damage_helper_test.c | 28 +++++++++++++++++++
3 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index cc70508d4fdb..84d5231ccac1 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -359,6 +359,7 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
state->fence = NULL;
state->commit = NULL;
state->fb_damage_clips = NULL;
+ state->ignore_damage_clips = false;
state->color_mgmt_changed = false;
}
EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c
index 74a7f4252ecf..945fac8dc27b 100644
--- a/drivers/gpu/drm/drm_damage_helper.c
+++ b/drivers/gpu/drm/drm_damage_helper.c
@@ -78,10 +78,8 @@ void drm_atomic_helper_check_plane_damage(struct drm_atomic_commit *state,
if (WARN_ON(!crtc_state))
return;
- if (drm_atomic_crtc_needs_modeset(crtc_state)) {
- drm_property_blob_put(plane_state->fb_damage_clips);
- plane_state->fb_damage_clips = NULL;
- }
+ if (drm_atomic_crtc_needs_modeset(crtc_state))
+ plane_state->ignore_damage_clips = true;
}
}
EXPORT_SYMBOL(drm_atomic_helper_check_plane_damage);
diff --git a/drivers/gpu/drm/tests/drm_damage_helper_test.c b/drivers/gpu/drm/tests/drm_damage_helper_test.c
index 0df2e1a54b0d..64f038a62ffe 100644
--- a/drivers/gpu/drm/tests/drm_damage_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_damage_helper_test.c
@@ -603,6 +603,33 @@ static void drm_test_damage_iter_damage_not_visible(struct kunit *test)
KUNIT_EXPECT_EQ_MSG(test, num_hits, 0, "Should not return any damage.");
}
+static void drm_test_damage_iter_damage_ignore(struct kunit *test)
+{
+ struct drm_damage_mock *mock = test->priv;
+ struct drm_atomic_helper_damage_iter iter;
+ struct drm_property_blob damage_blob;
+ struct drm_mode_rect damage[2];
+ struct drm_rect clip;
+ u32 num_hits = 0;
+
+ set_plane_src(&mock->old_state, 0, 0, 1024 << 16, 768 << 16);
+ set_plane_src(&mock->state, 0, 0, 1024 << 16, 768 << 16);
+ /* 2 damage clips, but ignore them. */
+ set_damage_clip(&damage[0], 20, 30, 200, 180);
+ set_damage_clip(&damage[1], 240, 200, 280, 250);
+ set_damage_blob(&damage_blob, &damage[0], sizeof(damage));
+ set_plane_damage(&mock->state, &damage_blob);
+ mock->state.ignore_damage_clips = true;
+ drm_atomic_helper_damage_iter_init(&iter, &mock->old_state, &mock->state);
+ drm_atomic_for_each_plane_damage(&iter, &clip) {
+ if (num_hits == 0)
+ check_damage_clip(test, &clip, 0, 0, 1024, 768);
+ num_hits++;
+ }
+
+ KUNIT_EXPECT_EQ_MSG(test, num_hits, 1, "Should return full-framebuffer damage.");
+}
+
static struct kunit_case drm_damage_helper_tests[] = {
KUNIT_CASE(drm_test_damage_iter_no_damage),
KUNIT_CASE(drm_test_damage_iter_no_damage_fractional_src),
@@ -625,6 +652,7 @@ static struct kunit_case drm_damage_helper_tests[] = {
KUNIT_CASE(drm_test_damage_iter_damage_one_outside),
KUNIT_CASE(drm_test_damage_iter_damage_src_moved),
KUNIT_CASE(drm_test_damage_iter_damage_not_visible),
+ KUNIT_CASE(drm_test_damage_iter_damage_ignore),
{ }
};
--
2.54.0
^ permalink raw reply related
* [PATCH v3 00/10]
From: Thomas Zimmermann @ 2026-05-27 14:46 UTC (permalink / raw)
To: mripard, maarten.lankhorst, airlied, airlied, simona, admin,
gargaditya08, paul, jani.nikula, mhklinux, zack.rusin,
bcm-kernel-feedback-list
Cc: dri-devel, linux-hyperv, intel-gfx, intel-xe, linux-mips,
virtualization, Thomas Zimmermann
DRM clients can supply information on framebuffer areas to update as
part of each page flip, called damage clipping rectangles. But DRM's
processing of this information is inconsistent and prone to errors.
- There are multiple fields and tests that decide if damage clips
should be taken or ignored.
- Sometimes damage clips are removed behind the back of the DRM client.
- Atomic helpers evaluate damage clipping in the middle of the atomic
check: after connectors and encoders, but before planes and CRTCs. Hence
pipeline stages have an inconsistent view.
- Which leads to drivers (ingenic) doing a re-evaluation if necessary.
- Tests of plane source coordinates only happen during commits. At this
point, the driver should already know if damage clips are to be taken or
not. Because of this, some drivers (appletbdrm) might operate on incorrect
damage information for their internal workings. This also leads to excessive
use of the old plane state.
Therefore go through DRM helpers and drivers and fix the logic.
- Run all of the atomic checks with the damage information supplied by
DRM clients. Afterwards evaluate plane and CRTC states on whether to
take or ignore damage clips. Do all related tests in a single atomic
helper.
- Do not discard damage clips. Set ignore_damage_clips in struct
drm_plane_state instead. This includes changes to plane source-coordinates.
The damage iterator now only has to look at this flag to detect if it
should use the damage clips.
- Go over drivers and fix the damage handling in the plane's
atomic_update helpers. Most drivers no longer need the old plane state
in their update.
- The appletbdrm driver requires a fix in how it uses damage information.
Ingenic and vmwgfx can be simplified. These changes improve the drivers'
code organization.
- Kunit tests require some changes. Drop some obsolete tests and add a new
one for ignore_damage_flags.
Tested with bochs, mgag200, Kunit tests.
v3:
- fix error path in appletbdrm
v2:
- rebase on latest upstream
Thomas Zimmermann (10):
drm/damage-helper: Do not alter damage clips on modeset, but ignore
them
drm/atomic-helpers: Evaluate plane damage after atomic_check
drm/ingenic: Remove calls to drm_atomic_helper_check_plane_damage()
drm/damage-helper: Test src coord in
drm_atomic_helper_check_plane_damage()
drm/appletbdrm: Allocate request/response buffers in begin_fb_access
drm/damage-helper: Remove old state from
drm_atomic_helper_damage_iter_init()
drm/damage-helper: Remove old state from
drm_atomic_helper_damage_merged()
drm/atomic_helper: Do not evaluate plane damage before atomic_check
drm/damage-helper: Rename state parameters in damage helpers
drm/vmwgfx: Remove unused field struct
vmwgfx_du_update_plane.old_state
drivers/gpu/drm/ast/ast_cursor.c | 3 +-
drivers/gpu/drm/ast/ast_mode.c | 2 +-
drivers/gpu/drm/drm_atomic_helper.c | 6 +-
drivers/gpu/drm/drm_atomic_state_helper.c | 1 +
drivers/gpu/drm/drm_damage_helper.c | 44 ++--
drivers/gpu/drm/drm_fb_dma_helper.c | 2 +-
drivers/gpu/drm/drm_mipi_dbi.c | 3 +-
drivers/gpu/drm/gud/gud_pipe.c | 3 +-
drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 3 +-
drivers/gpu/drm/i915/display/intel_plane.c | 11 +-
drivers/gpu/drm/i915/display/intel_psr.c | 3 +-
drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 3 -
drivers/gpu/drm/ingenic/ingenic-ipu.c | 8 +-
drivers/gpu/drm/mgag200/mgag200_mode.c | 3 +-
drivers/gpu/drm/sitronix/st7571.c | 3 +-
drivers/gpu/drm/sitronix/st7586.c | 3 +-
drivers/gpu/drm/sitronix/st7920.c | 3 +-
drivers/gpu/drm/solomon/ssd130x.c | 9 +-
drivers/gpu/drm/sysfb/drm_sysfb_modeset.c | 3 +-
.../gpu/drm/tests/drm_damage_helper_test.c | 200 +++---------------
drivers/gpu/drm/tiny/appletbdrm.c | 59 +++---
drivers/gpu/drm/tiny/bochs.c | 3 +-
drivers/gpu/drm/tiny/cirrus-qemu.c | 2 +-
drivers/gpu/drm/tiny/gm12u320.c | 2 +-
drivers/gpu/drm/tiny/ili9225.c | 3 +-
drivers/gpu/drm/tiny/repaper.c | 2 +-
drivers/gpu/drm/tiny/sharp-memory.c | 3 +-
drivers/gpu/drm/udl/udl_modeset.c | 3 +-
drivers/gpu/drm/virtio/virtgpu_plane.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 5 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 -
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 12 +-
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 15 +-
include/drm/drm_damage_helper.h | 9 +-
34 files changed, 123 insertions(+), 315 deletions(-)
base-commit: 5fb5a9a63cf5ece68e0eeb6fa397da27712bccf0
--
2.54.0
^ permalink raw reply
* Re: [PATCH 0/9] drm: Limit DRM_IOCTL_WAIT_VBLANK to vblank interrupts
From: Michel Dänzer @ 2026-05-27 14:04 UTC (permalink / raw)
To: Thomas Zimmermann, simona, airlied, pekka.paalanen, jadahl,
contact, maarten.lankhorst, mripard
Cc: amd-gfx, dri-devel, linux-hyperv, virtualization, spice-devel,
wayland-devel
In-Reply-To: <4a7c2f87-60fd-458c-a579-a36799b86557@suse.de>
On 5/27/26 15:07, Thomas Zimmermann wrote:
> Am 15.05.26 um 18:56 schrieb Michel Dänzer:
>> On 5/15/26 17:12, Michel Dänzer wrote:
>>> On 5/15/26 13:55, Thomas Zimmermann wrote:
>>>> DRM's WAIT_VBLANK ioctl synchronizes user-space clients to display
>>>> refresh. This is meaningless with vblank timers, which run unrelated
>>>> to the hardware's vblank.
>>>>
>>>> Disable the ioctl for simulated vblanks. Set DRM_VBLANK_FLAG_SIMULATED
>>>> for CRTCs with simulated vblank events in all such drivers. The vblank
>>>> timers of these devices still rate-limit the number of page-flip events
>>>> to match the display refresh.
>>>>
>>>> According to maintainers, user-space compositors do not require the ioctl
>>>> for rate-limitting display output. Weston and Kwin rely on page-flip
>>>> events. Mutter uses and internal timer to limit the number of display
>>>> updates per second.
>>> Actually mutter fundamentally relies on atomic commit completion events for that, same as Weston & KWin. Mutter uses the WAIT_VBLANK ioctl only for minimizing input → output latency (which can hide issues when completion of atomic commits isn't properly throttled).
>>>
>>>
>>> (Just a side not on the cover letter, no objections to the patches themselves)
>> After more discussion on IRC, I have some concerns.
>>
>>
>> The big one first: For drivers with no strict refresh cycle (i.e. an atomic commit can take effect more or less anytime after at least one "refresh cycle" has passed since the last one), does this change really make sense / what's the actual benefit?
>
> I don't have a strong opinion on that matter. I just think we should clarify the meaning of these ioctls.
>
> Timing page flip is currently not supported on any driver without hardware vblank IRQ or a vblank timer. The situation might vary among compositors, but there have been plenty of reports of animation and frame rates either being too high or too low.
As discussed on IRC, that's due to insufficient throttling of atomic commits in the kernel, not directly related to the functionality in this series.
> So I think we should rollout vblank timers for all drivers without hardware vblank IRQ.
I'm not arguing against that. It doesn't necessarily invalidate my concern though.
> Right now, vblank timers act like a vblank IRQ in these ioctls. That's a convenient position for user space.
>
> But we don't really sync anything with hardware here, so the alternate proposal is to not support them. This also appears to be the original intention of these ioctls.
Speaking as the creator of the DRM_IOCTL_WAIT_VBLANK ioctl, I'm afraid it's not that simple. While it's true that the ioctl was originally only available if backed by corresponding HW & driver support, that's mostly because vblank timers and the scenarios which motivated them weren't really a thing until much later though, not an explicit intention at the time.
I created it to give user space information about the display refresh cycle timing, and to allow it to synchronize to that. This seems like it could be useful even with vblank timers. At least if this ioctl and atomic commits are properly integrated, similar to how they are with a proper HW display refresh cycle, which I'm not sure is currently the case though.
> Vblank timers would just limit the internal page-flip rate, but nothing else. That's the more defensive approach.
If those two things aren't properly integrated though, then this might indeed be less bad than the status quo.
--
Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer
https://redhat.com \ Libre software enthusiast
^ permalink raw reply
* [PATCH v2 8/9] drm/virtgpu: Set DRM_VBLANK_FLAG_SIMULATED
From: Thomas Zimmermann @ 2026-05-27 13:32 UTC (permalink / raw)
To: simona, airlied, mdaenzer, pekka.paalanen, jadahl, contact,
maarten.lankhorst, mripard, mhklinux
Cc: amd-gfx, dri-devel, wayland-devel, linux-hyperv, virtualization,
spice-devel, Thomas Zimmermann
In-Reply-To: <20260527133917.207150-1-tzimmermann@suse.de>
Mark the vblank event on virtgpu as simulated, so that the WAIT_VBLANK
ioctl fails with an error. The ioctl should not be supported because
the output is not synchronized to a display refresh.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/virtio/virtgpu_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c
index 44ffffec550f..558d8001c54f 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -381,7 +381,7 @@ int virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev)
for (i = 0 ; i < vgdev->num_scanouts; ++i)
vgdev_output_init(vgdev, i);
- ret = drm_vblank_init(vgdev->ddev, vgdev->num_scanouts);
+ ret = drmm_vblank_init(vgdev->ddev, vgdev->num_scanouts, DRM_VBLANK_FLAG_SIMULATED);
if (ret)
return ret;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 7/9] drm/qxl: Set DRM_VBLANK_FLAG_SIMULATED
From: Thomas Zimmermann @ 2026-05-27 13:32 UTC (permalink / raw)
To: simona, airlied, mdaenzer, pekka.paalanen, jadahl, contact,
maarten.lankhorst, mripard, mhklinux
Cc: amd-gfx, dri-devel, wayland-devel, linux-hyperv, virtualization,
spice-devel, Thomas Zimmermann
In-Reply-To: <20260527133917.207150-1-tzimmermann@suse.de>
Mark the vblank event on qxl as simulated, so that the WAIT_VBLANK
ioctl fails with an error. The ioctl should not be supported because
the output is not synchronized to a display refresh.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/qxl/qxl_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index a026bd35ef48..b808fdebbd89 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1300,7 +1300,7 @@ int qxl_modeset_init(struct qxl_device *qdev)
qxl_display_read_client_monitors_config(qdev);
- ret = drm_vblank_init(&qdev->ddev, qxl_num_crtc);
+ ret = drmm_vblank_init(&qdev->ddev, qxl_num_crtc, DRM_VBLANK_FLAG_SIMULATED);
if (ret)
return ret;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 9/9] drm/vkms: Set DRM_VBLANK_FLAG_SIMULATED
From: Thomas Zimmermann @ 2026-05-27 13:32 UTC (permalink / raw)
To: simona, airlied, mdaenzer, pekka.paalanen, jadahl, contact,
maarten.lankhorst, mripard, mhklinux
Cc: amd-gfx, dri-devel, wayland-devel, linux-hyperv, virtualization,
spice-devel, Thomas Zimmermann
In-Reply-To: <20260527133917.207150-1-tzimmermann@suse.de>
Mark the vblank event on vkms as simulated, so that the WAIT_VBLANK
ioctl fails with an error. The ioctl should not be supported because
the output is not synchronized to a display refresh.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/vkms/vkms_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 5a640b531d88..c4cfa1e5ab01 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -192,8 +192,8 @@ int vkms_create(struct vkms_config *config)
goto out_devres;
}
- ret = drm_vblank_init(&vkms_device->drm,
- vkms_config_get_num_crtcs(config));
+ ret = drmm_vblank_init(&vkms_device->drm, vkms_config_get_num_crtcs(config),
+ DRM_VBLANK_FLAG_SIMULATED);
if (ret) {
DRM_ERROR("Failed to vblank\n");
goto out_devres;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 6/9] drm/hypervdrm: Set DRM_VBLANK_FLAG_SIMULATED
From: Thomas Zimmermann @ 2026-05-27 13:32 UTC (permalink / raw)
To: simona, airlied, mdaenzer, pekka.paalanen, jadahl, contact,
maarten.lankhorst, mripard, mhklinux
Cc: amd-gfx, dri-devel, wayland-devel, linux-hyperv, virtualization,
spice-devel, Thomas Zimmermann
In-Reply-To: <20260527133917.207150-1-tzimmermann@suse.de>
Mark the vblank event on hypervdrm as simulated, so that the WAIT_VBLANK
ioctl fails with an error. The ioctl should not be supported because the
output is not synchronized to a display refresh.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
index 1bbb7de5ab49..24bed31c35e7 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
@@ -329,7 +329,7 @@ int hyperv_mode_config_init(struct hyperv_drm_device *hv)
return ret;
}
- ret = drm_vblank_init(dev, 1);
+ ret = drmm_vblank_init(dev, 1, DRM_VBLANK_FLAG_SIMULATED);
if (ret)
return ret;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 3/9] drm/amdgpu: vkms: Set DRM_VBLANK_FLAG_SIMULATED
From: Thomas Zimmermann @ 2026-05-27 13:32 UTC (permalink / raw)
To: simona, airlied, mdaenzer, pekka.paalanen, jadahl, contact,
maarten.lankhorst, mripard, mhklinux
Cc: amd-gfx, dri-devel, wayland-devel, linux-hyperv, virtualization,
spice-devel, Thomas Zimmermann
In-Reply-To: <20260527133917.207150-1-tzimmermann@suse.de>
Mark the vblank event on amdgpu's vkms as simulated, so that the
WAIT_VBLANK ioctl fails with an error. The ioctl should not be
supported because the output is not synchronized to a display refresh.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
index 170adaf7e76a..bc88acc819a6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
@@ -413,7 +413,8 @@ static int amdgpu_vkms_sw_init(struct amdgpu_ip_block *ip_block)
return r;
}
- r = drm_vblank_init(adev_to_drm(adev), adev->mode_info.num_crtc);
+ r = drmm_vblank_init(adev_to_drm(adev), adev->mode_info.num_crtc,
+ DRM_VBLANK_FLAG_SIMULATED);
if (r)
return r;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 5/9] drm/cirrus: Set DRM_VBLANK_FLAG_SIMULATED
From: Thomas Zimmermann @ 2026-05-27 13:32 UTC (permalink / raw)
To: simona, airlied, mdaenzer, pekka.paalanen, jadahl, contact,
maarten.lankhorst, mripard, mhklinux
Cc: amd-gfx, dri-devel, wayland-devel, linux-hyperv, virtualization,
spice-devel, Thomas Zimmermann
In-Reply-To: <20260527133917.207150-1-tzimmermann@suse.de>
Mark the vblank event on cirrus as simulated, so that the WAIT_VBLANK
ioctl fails with an error. The ioctl should not be supported because
the output is not synchronized to a display refresh.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/tiny/cirrus-qemu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tiny/cirrus-qemu.c b/drivers/gpu/drm/tiny/cirrus-qemu.c
index 075221b431d3..01522d1158b2 100644
--- a/drivers/gpu/drm/tiny/cirrus-qemu.c
+++ b/drivers/gpu/drm/tiny/cirrus-qemu.c
@@ -501,7 +501,7 @@ static int cirrus_pipe_init(struct cirrus_device *cirrus)
if (ret)
return ret;
- ret = drm_vblank_init(dev, 1);
+ ret = drmm_vblank_init(dev, 1, DRM_VBLANK_FLAG_SIMULATED);
if (ret)
return ret;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 4/9] drm/bochs: Set DRM_VBLANK_FLAG_SIMULATED
From: Thomas Zimmermann @ 2026-05-27 13:32 UTC (permalink / raw)
To: simona, airlied, mdaenzer, pekka.paalanen, jadahl, contact,
maarten.lankhorst, mripard, mhklinux
Cc: amd-gfx, dri-devel, wayland-devel, linux-hyperv, virtualization,
spice-devel, Thomas Zimmermann
In-Reply-To: <20260527133917.207150-1-tzimmermann@suse.de>
Mark the vblank event on bochs as simulated, so that the WAIT_VBLANK
ioctl fails with an error. The ioctl should not be supported because
the output is not synchronized to a display refresh.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/tiny/bochs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index e2d957e51505..b5955ef39e31 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -677,7 +677,7 @@ static int bochs_kms_init(struct bochs_device *bochs)
drm_connector_attach_edid_property(connector);
drm_connector_attach_encoder(connector, encoder);
- ret = drm_vblank_init(dev, 1);
+ ret = drmm_vblank_init(dev, 1, DRM_VBLANK_FLAG_SIMULATED);
if (ret)
return ret;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 2/9] drm/vblank: Add DRM_VBLANK_FLAG_SIMULATED
From: Thomas Zimmermann @ 2026-05-27 13:32 UTC (permalink / raw)
To: simona, airlied, mdaenzer, pekka.paalanen, jadahl, contact,
maarten.lankhorst, mripard, mhklinux
Cc: amd-gfx, dri-devel, wayland-devel, linux-hyperv, virtualization,
spice-devel, Thomas Zimmermann
In-Reply-To: <20260527133917.207150-1-tzimmermann@suse.de>
Add DRM_VBLANK_FLAG_SIMULATED for CRTCs that do not have a hardware
vblank interrupt. Setting the flag tells DRM to not report vblank
capabilities from the WAIT_VBLANK ioctl.
DRM_IOCTL_WAIT_VBLANK queries timestamps from a vblank event or waits
for the next vblank event to occur. DRM clients use this functionality
to synchronize their output with the display's vblank phase. Hence this
is only supported for hardware implementations.
Software implementations are not synchronized to the display and merely
act as a rate limiter for page-flip events. The WAIT_VBLANK ioctl thus
should fail with an error.
v2:
- add filter in CRTC_GET_SEQUENCE and CRTC_QUEUE_SEQUENCE ioctls (Michel)
Suggested-by: Simona Vetter <simona@ffwll.ch>
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_vblank.c | 10 ++++++++++
include/drm/drm_vblank.h | 5 +++++
2 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 21ca91b4c014..3ef4ed08e7f6 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1794,6 +1794,9 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
vblank = drm_vblank_crtc(dev, pipe);
+ if (vblank->flags & DRM_VBLANK_FLAG_SIMULATED)
+ return -EOPNOTSUPP;
+
/* If the counter is currently enabled and accurate, short-circuit
* queries to return the cached timestamp of the last vblank.
*/
@@ -2035,6 +2038,10 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data,
pipe = drm_crtc_index(crtc);
vblank = drm_crtc_vblank_crtc(crtc);
+
+ if (vblank->flags & DRM_VBLANK_FLAG_SIMULATED)
+ return -EOPNOTSUPP;
+
vblank_enabled = READ_ONCE(vblank->config.disable_immediate) &&
READ_ONCE(vblank->enabled);
@@ -2102,6 +2109,9 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,
vblank = drm_crtc_vblank_crtc(crtc);
+ if (vblank->flags & DRM_VBLANK_FLAG_SIMULATED)
+ return -EOPNOTSUPP;
+
e = kzalloc_obj(*e);
if (e == NULL)
return -ENOMEM;
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index 39a201b83781..03fa7259b6ac 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -37,6 +37,11 @@ struct drm_device;
struct drm_crtc;
struct drm_vblank_work;
+/**
+ * DRM_VBLANK_FLAG_SIMULATED - vblank uses a software timer
+ */
+#define DRM_VBLANK_FLAG_SIMULATED BIT(1)
+
/**
* struct drm_pending_vblank_event - pending vblank event tracking
*/
--
2.54.0
^ permalink raw reply related
* [PATCH v2 1/9] drm/vblank: Add drmm_vblank_init() to indicate managed cleanup
From: Thomas Zimmermann @ 2026-05-27 13:32 UTC (permalink / raw)
To: simona, airlied, mdaenzer, pekka.paalanen, jadahl, contact,
maarten.lankhorst, mripard, mhklinux
Cc: amd-gfx, dri-devel, wayland-devel, linux-hyperv, virtualization,
spice-devel, Thomas Zimmermann
In-Reply-To: <20260527133917.207150-1-tzimmermann@suse.de>
Rename drm_vblank_init() to drmm_vblank_init(). As the initializer
function sets up managed cleanup, it should use the drmm prefix. Keep
the old name around until all callers have been converted.
Also add a flags argument to the function. The first use of the flags
will be to distinguish between hardware vblank interrupts and simulated
vblank timeouts.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_vblank.c | 16 +++++++++-------
drivers/gpu/drm/drm_vblank_helper.c | 2 +-
include/drm/drm_crtc.h | 2 +-
include/drm/drm_device.h | 2 +-
include/drm/drm_vblank.h | 10 +++++++++-
5 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index f90fb2d13e42..21ca91b4c014 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -117,7 +117,7 @@
* optionally provide a hardware vertical blanking counter.
*
* Drivers must initialize the vertical blanking handling core with a call to
- * drm_vblank_init(). Minimally, a driver needs to implement
+ * drmm_vblank_init(). Minimally, a driver needs to implement
* &drm_crtc_funcs.enable_vblank and &drm_crtc_funcs.disable_vblank plus call
* drm_crtc_handle_vblank() in its vblank interrupt handler for working vblank
* support.
@@ -146,7 +146,7 @@
* See also DRM vblank helpers for more information.
*
* Drivers without support for vertical-blanking interrupts nor timers must
- * not call drm_vblank_init(). For these drivers, atomic helpers will
+ * not call drmm_vblank_init(). For these drivers, atomic helpers will
* automatically generate fake vblank events as part of the display update.
* This functionality also can be controlled by the driver by enabling and
* disabling struct drm_crtc_state.no_vblank.
@@ -519,7 +519,7 @@ static void vblank_disable_fn(struct timer_list *t)
spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
}
-static void drm_vblank_init_release(struct drm_device *dev, void *ptr)
+static void drmm_vblank_init_release(struct drm_device *dev, void *ptr)
{
struct drm_vblank_crtc *vblank = ptr;
@@ -534,9 +534,10 @@ static void drm_vblank_init_release(struct drm_device *dev, void *ptr)
}
/**
- * drm_vblank_init - initialize vblank support
+ * drmm_vblank_init - initialize vblank support
* @dev: DRM device
* @num_crtcs: number of CRTCs supported by @dev
+ * @flags: flags for vblank handling
*
* This function initializes vblank support for @num_crtcs display pipelines.
* Cleanup is handled automatically through a cleanup function added with
@@ -545,7 +546,7 @@ static void drm_vblank_init_release(struct drm_device *dev, void *ptr)
* Returns:
* Zero on success or a negative error code on failure.
*/
-int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs)
+int drmm_vblank_init(struct drm_device *dev, unsigned int num_crtcs, unsigned int flags)
{
int ret;
unsigned int i;
@@ -564,11 +565,12 @@ int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs)
vblank->dev = dev;
vblank->pipe = i;
+ vblank->flags = flags;
init_waitqueue_head(&vblank->queue);
timer_setup(&vblank->disable_timer, vblank_disable_fn, 0);
seqlock_init(&vblank->seqlock);
- ret = drmm_add_action_or_reset(dev, drm_vblank_init_release,
+ ret = drmm_add_action_or_reset(dev, drmm_vblank_init_release,
vblank);
if (ret)
return ret;
@@ -580,7 +582,7 @@ int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs)
return 0;
}
-EXPORT_SYMBOL(drm_vblank_init);
+EXPORT_SYMBOL(drmm_vblank_init);
/**
* drm_dev_has_vblank - test if vblanking has been initialized for
diff --git a/drivers/gpu/drm/drm_vblank_helper.c b/drivers/gpu/drm/drm_vblank_helper.c
index d3f8147ecdc1..5b05ab72e133 100644
--- a/drivers/gpu/drm/drm_vblank_helper.c
+++ b/drivers/gpu/drm/drm_vblank_helper.c
@@ -25,7 +25,7 @@
* for drivers without further requirements. The initializer macro
* DRM_CRTC_HELPER_VBLANK_FUNCS sets them coveniently.
*
- * Once the driver enables vblank support with drm_vblank_init(), each
+ * Once the driver enables vblank support with drmm_vblank_init(), each
* CRTC's vblank timer fires according to the programmed display mode. By
* default, the vblank timer invokes drm_crtc_handle_vblank(). Drivers with
* more specific requirements can set their own handler function in
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index c6dbe8b7db9e..f981468d9a00 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -163,7 +163,7 @@ struct drm_crtc_state {
*
* One usage is for drivers and/or hardware without support for VBLANK
* interrupts. Such drivers typically do not initialize vblanking
- * (i.e., call drm_vblank_init() with the number of CRTCs). For CRTCs
+ * (i.e., call drmm_vblank_init() with the number of CRTCs). For CRTCs
* without initialized vblanking, this field is set to true in
* drm_atomic_helper_check_modeset(), and a fake VBLANK event will be
* send out on each update of the display pipeline by
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 768a8dae83c5..742996576313 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -283,7 +283,7 @@ struct drm_device {
* Array of vblank tracking structures, one per &struct drm_crtc. For
* historical reasons (vblank support predates kernel modesetting) this
* is free-standing and not part of &struct drm_crtc itself. It must be
- * initialized explicitly by calling drm_vblank_init().
+ * initialized explicitly by calling drmm_vblank_init().
*/
struct drm_vblank_crtc *vblank;
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index 2fcef9c0f5b1..39a201b83781 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -282,10 +282,12 @@ struct drm_vblank_crtc {
* @vblank_timer: Holds the state of the vblank timer
*/
struct drm_vblank_crtc_timer vblank_timer;
+
+ unsigned int flags;
};
struct drm_vblank_crtc *drm_crtc_vblank_crtc(struct drm_crtc *crtc);
-int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs);
+int drmm_vblank_init(struct drm_device *dev, unsigned int num_crtcs, unsigned int flags);
bool drm_dev_has_vblank(const struct drm_device *dev);
u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
@@ -321,6 +323,12 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc);
void drm_crtc_vblank_cancel_timer(struct drm_crtc *crtc);
void drm_crtc_vblank_get_vblank_timeout(struct drm_crtc *crtc, ktime_t *vblank_time);
+/* deprecated; use drmm_vblank_init() instead */
+static inline int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs)
+{
+ return drmm_vblank_init(dev, num_crtcs, 0);
+}
+
/*
* Helpers for struct drm_crtc_funcs
*/
--
2.54.0
^ permalink raw reply related
* [PATCH v2 0/9] drm: Limit DRM_IOCTL_WAIT_VBLANK to vblank interrupts
From: Thomas Zimmermann @ 2026-05-27 13:32 UTC (permalink / raw)
To: simona, airlied, mdaenzer, pekka.paalanen, jadahl, contact,
maarten.lankhorst, mripard, mhklinux
Cc: amd-gfx, dri-devel, wayland-devel, linux-hyperv, virtualization,
spice-devel, Thomas Zimmermann
DRM's WAIT_VBLANK ioctl synchronizes user-space clients to display
refresh. This is meaningless with vblank timers, which run unrelated
to the hardware's vblank.
Disable the ioctl for simulated vblanks. Set DRM_VBLANK_FLAG_SIMULATED
for CRTCs with simulated vblank events in all such drivers. The vblank
timers of these devices still rate-limit the number of page-flip events
to match the display refresh.
According to maintainers, user-space compositors do not require the ioctl
for rate-limitting display output. Weston, Kwin and Mutter rely on completion
events. Mutter optionally uses the WAIT_VBLANK ioctl only to optimize the
time from input to output.
When testing with mutter and weston, the page-flip rate appears correct
with the patch set applied.
This change has been discussed at length on IRC recently.
https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2026-05-08&show_html=true
https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2026-05-12&show_html=true
https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2026-05-13&show_html=true
https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2026-05-15&show_html=true
v2:
- add filter to CRTC_GET_SEQUENCE and CRTC_QUEUE_SEQUENCE ioctls (Michel)
- clarify Mutter's behavior in cover letter (Michel)
Thomas Zimmermann (9):
drm/vblank: Add drmm_vblank_init() to indicate managed cleanup
drm/vblank: Add DRM_VBLANK_FLAG_SIMULATED
drm/amdgpu: vkms: Set DRM_VBLANK_FLAG_SIMULATED
drm/bochs: Set DRM_VBLANK_FLAG_SIMULATED
drm/cirrus: Set DRM_VBLANK_FLAG_SIMULATED
drm/hypervdrm: Set DRM_VBLANK_FLAG_SIMULATED
drm/qxl: Set DRM_VBLANK_FLAG_SIMULATED
drm/virtgpu: Set DRM_VBLANK_FLAG_SIMULATED
drm/vkms: Set DRM_VBLANK_FLAG_SIMULATED
drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c | 3 ++-
drivers/gpu/drm/drm_vblank.c | 26 +++++++++++++++------
drivers/gpu/drm/drm_vblank_helper.c | 2 +-
drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 2 +-
drivers/gpu/drm/qxl/qxl_display.c | 2 +-
drivers/gpu/drm/tiny/bochs.c | 2 +-
drivers/gpu/drm/tiny/cirrus-qemu.c | 2 +-
drivers/gpu/drm/virtio/virtgpu_display.c | 2 +-
drivers/gpu/drm/vkms/vkms_drv.c | 4 ++--
include/drm/drm_crtc.h | 2 +-
include/drm/drm_device.h | 2 +-
include/drm/drm_vblank.h | 15 +++++++++++-
12 files changed, 45 insertions(+), 19 deletions(-)
base-commit: 5fb5a9a63cf5ece68e0eeb6fa397da27712bccf0
--
2.54.0
^ permalink raw reply
* Re: [PATCH v6 5/7] locking: Add contended_release tracepoint to qspinlock
From: Dmitry Ilvokhin @ 2026-05-27 13:30 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
Thomas Bogendoerfer, Juergen Gross, Ajay Kaher, Alexey Makhalov,
Broadcom internal kernel review list, Thomas Gleixner,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Arnd Bergmann,
Dennis Zhou, Tejun Heo, Christoph Lameter, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, linux-kernel, linux-mips,
virtualization, linux-arch, linux-mm, linux-trace-kernel,
kernel-team, Paul E. McKenney
In-Reply-To: <agXBb0ga_6HJrrnm@shell.ilvokhin.com>
Hi Peter,
Gentle ping on this. I wanted to check if the assembly analysis in my
previous reply changed the picture at all.
You were right that the commit message was misleading about the total
size increase: it's 9 bytes per call site, not just the NOP.
That said, when I looked at the executed path with the tracepoint
disabled, the only addition is the 2-byte NOP (xchg %ax,%ax).
Both the baseline and instrumented _raw_spin_unlock() fit within a
single 64-byte cache line, and I wasn't able to measure any difference
with locktorture: lock() cost completely dominates, unlock() accounts
for less than 1% of the total, so any overhead is indistinguishable from
noise.
If the cost is still a concern, I see two possible paths forward:
1. Guard the spinlock/qrwlock instrumentation behind a Kconfig option
(disabled by default), so only kernels that explicitly opt in pay
the cost.
2. Drop the spinlock/qrwlock instrumentation entirely and keep
contended_release for sleepable locks only.
Happy to go whichever direction you prefer.
^ permalink raw reply
* Re: [patch 24/24] ptp: Switch to ktime_get_snapshot_id() for pre/post timestamps
From: Vadim Fedorenko @ 2026-05-27 13:10 UTC (permalink / raw)
To: Thomas Gleixner, LKML
Cc: David Woodhouse, Miroslav Lichvar, John Stultz, Stephen Boyd,
Anna-Maria Behnsen, Frederic Weisbecker, thomas.weissschuh,
Arthur Kiyanovski, Rodolfo Giometti, Vincent Donnefort,
Marc Zyngier, Oliver Upton, kvmarm, Oliver Upton, Richard Cochran,
netdev, Takashi Iwai, Miri Korenblit, Johannes Berg, Jacob Keller,
Tony Nguyen, Saeed Mahameed, Peter Hilber, Michael S. Tsirkin,
virtualization, linux-wireless, linux-sound
In-Reply-To: <20260526171224.499777655@kernel.org>
On 26/05/2026 18:15, Thomas Gleixner wrote:
> To prepare for a new PTP IOCTL, which exposes the raw counter value along
> with the requested system time snapshot, switch the pre/post time stamp
> sampling over to use ktime_get_snapshot_id() and fix up all usage sites.
>
> No functional change intended.
>
> The ptp_vmclock conversion was simplified by David Woodhouse.
>
> Signed-off-by: Thomas Gleixner <tglx@kernel.org>
for ptp_ocp:
Acked-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
^ permalink raw reply
* Re: [PATCH v15] can: virtio: Add virtio CAN driver
From: Matias Ezequiel Vara Larsen @ 2026-05-27 13:08 UTC (permalink / raw)
To: Harald Mommer, Marc Kleine-Budde, Vincent Mailhol,
Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
linux-can, virtualization
Cc: Mikhail Golubev-Ciuchea, Stefano Garzarella, francesco
In-Reply-To: <ahXNb+KzuHYbS24+@fedora>
On Tue, May 26, 2026 at 06:42:23PM +0200, Matias Ezequiel Vara Larsen wrote:
> Add virtio CAN driver based on Virtio 1.4 specification (see
> https://github.com/oasis-tcs/virtio-spec/tree/virtio-1.4). The driver
> implements a complete CAN bus interface over Virtio transport,
> supporting both CAN Classic and CAN-FD Ids. In term of frames, it
> supports classic and CAN FD. RTR frames are only supported with classic
> CAN.
>
> Usage:
> - "ip link set up can0" - start controller
> - "ip link set down can0" - stop controller
> - "candump can0" - receive frames
> - "cansend can0 123#DEADBEEF" - send frames
>
> Signed-off-by: Harald Mommer <harald.mommer@oss.qualcomm.com>
> Co-developed-by: Harald Mommer <harald.mommer@oss.qualcomm.com>
> Signed-off-by: Mikhail Golubev-Ciuchea <mikhail.golubev-ciuchea@oss.qualcomm.com>
> Co-developed-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: Damir Shaikhutdinov <Damir.Shaikhutdinov@opensynergy.com>
> Reviewed-by: Francesco Valla <francesco@valla.it>
> Tested-by: Francesco Valla <francesco@valla.it>
> Signed-off-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> ---
> V15:
> - Drop unchecked can_put_echo_skb() return value and dead error check in
> virtio_can_freeze() since virtio_can_close() always returns zero.
> - Fix two use-after-free bugs: in virtio_can_start_xmit(), returning
> NETDEV_TX_BUSY after virtqueue_add_sgs() fails is wrong because
> can_put_echo_skb() already consumed the skb; and in the freeze/restore/remove
> path, stale vq pointers left by a failed restore() are dereferenced by a
> subsequent remove().
>
Hello @Marc Kleine-Budde, it seems that the nipa/ai-review bot did not
find any new issue. Do you have any comment?
Thanks, Matias
> V14:
> - set length before memcpy for field defined with __counted_by_le()
> - move napi_enable() to open() and napi_disable() to close() thus following
> pattern in other CAN devices
> - add flag to prevent remove() to hang if restore() fails
>
> V13:
> - kick device only when some buffers have been added
> - add virtio_can_del_vq() when virtio_can_start() fails
> - move napi_disable() at the end of virtio_can_freeze()
> - move napi_enable() at beginning of virtio_can_restore()
> - let virtio_can_start() sets CAN_STATE_ERROR_ACTIVE
> - in freeze(), if can_stop() fails, restore the state of the device
> - in restore(), if !netif_running, set CAN_STATE_STOPPED
>
> V12:
> - check return value in virtio_can_start()
> - use devm_krealloc() in virtio_can_restore() to re-allocate rpkt
> - remove access to cf->len and replace it with len
> - use __counted_by_le()
> - free tx buffers during virtio_can_del_vq()
> - add reinit_completion() for ctrl msgs
>
> V11:
> * Set CAN_VIRTIO_CAN config before CAN_XILINXCAN
> * Use GFP_ATOMIC in virtio_can_alloc_tx_idx()
> * Use open_candev() and set bittiming.bitrate to CAN_BITRATE_UNKNOWN and
> data_bittiming.bitrate to CAN_BITRATE_UNKNOWN
> * Fix leak of skb by adding kfree_skb(skb)
> * Fix out-of-bounds in virtio_can_populate_rx_vq()
> * Initialize `ret`
> * Use virtio_reset_device()
> * Add napi_disable() in virtio_can_remove()
> * Propagate error in virtio_can_start() and virtio_can_stop()
>
> V10:
> * Follow Reverse Christmas Tree convention
>
> V9:
> * Remove unnecessary comments
> * Update maintainer list
>
> V8:
> * Address nits
>
> V7:
> * Address nits
> * Remove unnecessary comments
> * Remove io_callbacks[]
> * Use guard() syntax
> * Remove kicking for each inbuf
> * replace sdu_len with rpkt_len
> * Use devm_kzalloc()
> * Use scoped_guard() to protect virtqueue_add_sgs() and virtqueue_kicks() for
> tx queue
> * Tested with vhost-device-can
> (see https://github.com/rust-vmm/vhost-device/tree/main/vhost-device-can) and
> Qemu (942b0d3) with [1]. A reviewer observed that the device stops to work
> after flooding from host. This issue is still present.
>
> [1]
> https://lore.kernel.org/qemu-devel/20251031155617.1223248-1-mvaralar@redhat.com/
>
> V6:
> * Address nits
> * Check for error during register_virtio_can()
> * Remove virtio_device_ready()
> * Allocate virtio_can_rx rpkt[] at probe
> * Define virtio_can_control struct
> * Return VIRTIO_CAN_RESULT_NOT_OK after unlocking
> * Define sdu[] as a flex array for both tx and rx. For rx, use
> VIRTIO_CAN_F_CAN_FD to figure out the max len for sdu
> * Fix statistics in virtio_can_read_tx_queue() and
> how we indicate error to the user when getting
> VIRTIO_CAN_RESULT_NOT_OK
> * Fix syntax of virtio_find_vqs()
> * Drop tx_list
> * Fix values of VIRTIO_CAN_F_LATE_TX_ACK and VIRTIO_CAN_F_RTR_FRAMES
> * Tested with vhost-device-can
> (see
> https://github.com/rust-vmm/vhost-device/tree/main/vhost-device-can)
> and qemu (see
> https://github.com/virtualopensystems/qemu/tree/vhu-can-rfc)
>
> V5:
> * Re-base on top of linux-next (next-20240103)
> * Tested with https://github.com/OpenSynergy/qemu/tree/virtio-can-spec-rfc-v3
>
> RFC V4:
> * Apply reverse Christmas tree style
> * Add member *classic_dlc to RX and TX CAN frames
> * Fix race causing a NETDEV_TX_BUSY return
> ---
> MAINTAINERS | 9 +
> drivers/net/can/Kconfig | 12 +
> drivers/net/can/Makefile | 1 +
> drivers/net/can/virtio_can.c | 1022 +++++++++++++++++++++++++++++++
> include/uapi/linux/virtio_can.h | 78 +++
> 5 files changed, 1122 insertions(+)
> create mode 100644 drivers/net/can/virtio_can.c
> create mode 100644 include/uapi/linux/virtio_can.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 80cd3498c293..f295a904c93e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27068,6 +27068,15 @@ F: drivers/scsi/virtio_scsi.c
> F: include/uapi/linux/virtio_blk.h
> F: include/uapi/linux/virtio_scsi.h
>
> +VIRTIO CAN DRIVER
> +M: "Harald Mommer" <harald.mommer@oss.qualcomm.com>
> +M: "Matias Ezequiel Vara Larsen" <mvaralar@redhat.com>
> +L: virtualization@lists.linux.dev
> +L: linux-can@vger.kernel.org
> +S: Maintained
> +F: drivers/net/can/virtio_can.c
> +F: include/uapi/linux/virtio_can.h
> +
> VIRTIO CONSOLE DRIVER
> M: Amit Shah <amit@kernel.org>
> L: virtualization@lists.linux.dev
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index d43d56694667..f33ae46d9766 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -209,6 +209,18 @@ config CAN_TI_HECC
> Driver for TI HECC (High End CAN Controller) module found on many
> TI devices. The device specifications are available from www.ti.com
>
> +config CAN_VIRTIO_CAN
> + depends on VIRTIO
> + tristate "Virtio CAN device support"
> + default n
> + help
> + Say Y here if you want to support for Virtio CAN.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called virtio-can.
> +
> + If unsure, say N.
> +
> config CAN_XILINXCAN
> tristate "Xilinx CAN"
> depends on ARCH_ZYNQ || ARM64 || MICROBLAZE || COMPILE_TEST
> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> index 56138d8ddfd2..2ddea733ed5d 100644
> --- a/drivers/net/can/Makefile
> +++ b/drivers/net/can/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_CAN_PEAK_PCIEFD) += peak_canfd/
> obj-$(CONFIG_CAN_SJA1000) += sja1000/
> obj-$(CONFIG_CAN_SUN4I) += sun4i_can.o
> obj-$(CONFIG_CAN_TI_HECC) += ti_hecc.o
> +obj-$(CONFIG_CAN_VIRTIO_CAN) += virtio_can.o
> obj-$(CONFIG_CAN_XILINXCAN) += xilinx_can.o
>
> subdir-ccflags-$(CONFIG_CAN_DEBUG_DEVICES) += -DDEBUG
> diff --git a/drivers/net/can/virtio_can.c b/drivers/net/can/virtio_can.c
> new file mode 100644
> index 000000000000..f67d0bf09681
> --- /dev/null
> +++ b/drivers/net/can/virtio_can.c
> @@ -0,0 +1,1022 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * CAN bus driver for the Virtio CAN controller
> + *
> + * Copyright (C) 2021-2023 OpenSynergy GmbH
> + * Copyright Red Hat, Inc. 2025
> + */
> +
> +#include <linux/atomic.h>
> +#include <linux/idr.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/netdevice.h>
> +#include <linux/stddef.h>
> +#include <linux/can/dev.h>
> +#include <linux/virtio.h>
> +#include <linux/virtio_ring.h>
> +#include <linux/virtio_can.h>
> +
> +/* CAN device queues */
> +#define VIRTIO_CAN_QUEUE_TX 0
> +#define VIRTIO_CAN_QUEUE_RX 1
> +#define VIRTIO_CAN_QUEUE_CONTROL 2
> +#define VIRTIO_CAN_QUEUE_COUNT 3
> +
> +#define CAN_KNOWN_FLAGS \
> + (VIRTIO_CAN_FLAGS_EXTENDED |\
> + VIRTIO_CAN_FLAGS_FD |\
> + VIRTIO_CAN_FLAGS_RTR)
> +
> +/* Max. number of in flight TX messages */
> +#define VIRTIO_CAN_ECHO_SKB_MAX 128
> +
> +struct virtio_can_tx {
> + unsigned int putidx;
> + struct virtio_can_tx_in tx_in;
> + /* Keep virtio_can_tx_out at the end of the structure due to flex array */
> + struct virtio_can_tx_out tx_out;
> +};
> +
> +struct virtio_can_control {
> + struct virtio_can_control_out cpkt_out;
> + struct virtio_can_control_in cpkt_in;
> +};
> +
> +/* virtio_can private data structure */
> +struct virtio_can_priv {
> + struct can_priv can; /* must be the first member */
> + /* NAPI for RX messages */
> + struct napi_struct napi;
> + /* NAPI for TX messages */
> + struct napi_struct napi_tx;
> + /* The network device we're associated with */
> + struct net_device *dev;
> + /* The virtio device we're associated with */
> + struct virtio_device *vdev;
> + /* The virtqueues */
> + struct virtqueue *vqs[VIRTIO_CAN_QUEUE_COUNT];
> + /* Lock for TX operations */
> + spinlock_t tx_lock;
> + /* Control queue lock */
> + struct mutex ctrl_lock;
> + /* Wait for control queue processing without polling */
> + struct completion ctrl_done;
> + /* Array of receive queue messages */
> + struct virtio_can_rx *rpkt;
> + struct virtio_can_control can_ctr_msg;
> + /* Data to get and maintain the putidx for local TX echo */
> + struct ida tx_putidx_ida;
> + /* In flight TX messages */
> + atomic_t tx_inflight;
> + /* Packet length */
> + int rpkt_len;
> + /* BusOff pending. Reset after successful indication to upper layer */
> + bool busoff_pending;
> + /* Tracks whether NAPI instances are currently enabled */
> + bool napi_active;
> +};
> +
> +static void virtqueue_napi_schedule(struct napi_struct *napi,
> + struct virtqueue *vq)
> +{
> + if (napi_schedule_prep(napi)) {
> + virtqueue_disable_cb(vq);
> + __napi_schedule(napi);
> + }
> +}
> +
> +static void virtqueue_napi_complete(struct napi_struct *napi,
> + struct virtqueue *vq, int processed)
> +{
> + int opaque;
> +
> + opaque = virtqueue_enable_cb_prepare(vq);
> + if (napi_complete_done(napi, processed)) {
> + if (unlikely(virtqueue_poll(vq, opaque)))
> + virtqueue_napi_schedule(napi, vq);
> + } else {
> + virtqueue_disable_cb(vq);
> + }
> +}
> +
> +static void virtio_can_free_candev(struct net_device *ndev)
> +{
> + struct virtio_can_priv *priv = netdev_priv(ndev);
> +
> + ida_destroy(&priv->tx_putidx_ida);
> + free_candev(ndev);
> +}
> +
> +static void virtio_can_napi_enable(struct virtio_can_priv *priv)
> +{
> + if (!priv->napi_active) {
> + napi_enable(&priv->napi);
> + napi_enable(&priv->napi_tx);
> + priv->napi_active = true;
> + }
> +}
> +
> +static void virtio_can_napi_disable(struct virtio_can_priv *priv)
> +{
> + if (priv->napi_active) {
> + napi_disable(&priv->napi_tx);
> + napi_disable(&priv->napi);
> + priv->napi_active = false;
> + }
> +}
> +
> +static int virtio_can_alloc_tx_idx(struct virtio_can_priv *priv)
> +{
> + int tx_idx;
> +
> + tx_idx = ida_alloc_max(&priv->tx_putidx_ida,
> + priv->can.echo_skb_max - 1, GFP_ATOMIC);
> + if (tx_idx >= 0)
> + atomic_inc(&priv->tx_inflight);
> +
> + return tx_idx;
> +}
> +
> +static void virtio_can_free_tx_idx(struct virtio_can_priv *priv,
> + unsigned int idx)
> +{
> + ida_free(&priv->tx_putidx_ida, idx);
> + atomic_dec(&priv->tx_inflight);
> +}
> +
> +/* Create a scatter-gather list representing our input buffer and put
> + * it in the queue.
> + *
> + * Callers should take appropriate locks.
> + */
> +static int virtio_can_add_inbuf(struct virtqueue *vq, void *buf,
> + unsigned int size)
> +{
> + struct scatterlist sg[1];
> + int ret;
> +
> + sg_init_one(sg, buf, size);
> +
> + ret = virtqueue_add_inbuf(vq, sg, 1, buf, GFP_ATOMIC);
> +
> + return ret;
> +}
> +
> +/* Send a control message with message type either
> + *
> + * - VIRTIO_CAN_SET_CTRL_MODE_START or
> + * - VIRTIO_CAN_SET_CTRL_MODE_STOP.
> + *
> + */
> +static u8 virtio_can_send_ctrl_msg(struct net_device *ndev, u16 msg_type)
> +{
> + struct scatterlist sg_out, sg_in, *sgs[2] = { &sg_out, &sg_in };
> + struct virtio_can_priv *priv = netdev_priv(ndev);
> + struct virtqueue *vq = priv->vqs[VIRTIO_CAN_QUEUE_CONTROL];
> + struct device *dev = &priv->vdev->dev;
> + unsigned int len;
> + int err;
> +
> + if (!vq)
> + return VIRTIO_CAN_RESULT_NOT_OK;
> +
> + guard(mutex)(&priv->ctrl_lock);
> +
> + priv->can_ctr_msg.cpkt_out.msg_type = cpu_to_le16(msg_type);
> + sg_init_one(&sg_out, &priv->can_ctr_msg.cpkt_out,
> + sizeof(priv->can_ctr_msg.cpkt_out));
> + sg_init_one(&sg_in, &priv->can_ctr_msg.cpkt_in, sizeof(priv->can_ctr_msg.cpkt_in));
> +
> + reinit_completion(&priv->ctrl_done);
> +
> + err = virtqueue_add_sgs(vq, sgs, 1u, 1u, priv, GFP_ATOMIC);
> + if (err != 0) {
> + dev_err(dev, "%s(): virtqueue_add_sgs() failed\n", __func__);
> + return VIRTIO_CAN_RESULT_NOT_OK;
> + }
> +
> + if (!virtqueue_kick(vq)) {
> + dev_err(dev, "%s(): Kick failed\n", __func__);
> + return VIRTIO_CAN_RESULT_NOT_OK;
> + }
> +
> + while (!virtqueue_get_buf(vq, &len) && !virtqueue_is_broken(vq))
> + wait_for_completion(&priv->ctrl_done);
> +
> + return priv->can_ctr_msg.cpkt_in.result;
> +}
> +
> +static int virtio_can_start(struct net_device *ndev)
> +{
> + struct virtio_can_priv *priv = netdev_priv(ndev);
> + u8 result;
> +
> + result = virtio_can_send_ctrl_msg(ndev, VIRTIO_CAN_SET_CTRL_MODE_START);
> + if (result != VIRTIO_CAN_RESULT_OK) {
> + netdev_err(ndev, "CAN controller start failed\n");
> + return -EIO;
> + }
> +
> + priv->busoff_pending = false;
> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> + return 0;
> +}
> +
> +static int virtio_can_set_mode(struct net_device *dev, enum can_mode mode)
> +{
> + int err;
> +
> + switch (mode) {
> + case CAN_MODE_START:
> + err = virtio_can_start(dev);
> + if (err)
> + return err;
> + netif_wake_queue(dev);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int virtio_can_open(struct net_device *ndev)
> +{
> + struct virtio_can_priv *priv = netdev_priv(ndev);
> + int err;
> +
> + err = open_candev(ndev);
> + if (err)
> + return err;
> +
> + err = virtio_can_start(ndev);
> + if (err) {
> + close_candev(ndev);
> + return err;
> + }
> +
> + virtio_can_napi_enable(priv);
> + netif_start_queue(ndev);
> +
> + return 0;
> +}
> +
> +static int virtio_can_stop(struct net_device *ndev)
> +{
> + struct virtio_can_priv *priv = netdev_priv(ndev);
> + struct device *dev = &priv->vdev->dev;
> + u8 result;
> +
> + result = virtio_can_send_ctrl_msg(ndev, VIRTIO_CAN_SET_CTRL_MODE_STOP);
> + if (result != VIRTIO_CAN_RESULT_OK) {
> + dev_err(dev, "CAN controller stop failed\n");
> + return -EIO;
> + }
> +
> + priv->busoff_pending = false;
> + priv->can.state = CAN_STATE_STOPPED;
> +
> + /* Switch carrier off if device was connected to the bus */
> + if (netif_carrier_ok(ndev))
> + netif_carrier_off(ndev);
> +
> + return 0;
> +}
> +
> +static int virtio_can_close(struct net_device *dev)
> +{
> + struct virtio_can_priv *priv = netdev_priv(dev);
> +
> + netif_stop_queue(dev);
> + /* Ignore stop error: ndo_stop must always complete cleanup regardless.
> + * virtio_can_stop() already logs the error if it fails.
> + */
> + virtio_can_stop(dev);
> + virtio_can_napi_disable(priv);
> + close_candev(dev);
> +
> + return 0;
> +}
> +
> +static netdev_tx_t virtio_can_start_xmit(struct sk_buff *skb,
> + struct net_device *dev)
> +{
> + struct scatterlist sg_out, sg_in, *sgs[2] = { &sg_out, &sg_in };
> + const unsigned int hdr_size = sizeof(struct virtio_can_tx_out);
> + struct canfd_frame *cf = (struct canfd_frame *)skb->data;
> + struct virtio_can_priv *priv = netdev_priv(dev);
> + struct virtqueue *vq = priv->vqs[VIRTIO_CAN_QUEUE_TX];
> + netdev_tx_t xmit_ret = NETDEV_TX_OK;
> + struct virtio_can_tx *can_tx_msg;
> + u32 can_flags;
> + int putidx;
> + int err;
> +
> + if (can_dev_dropped_skb(dev, skb))
> + goto kick; /* No way to return NET_XMIT_DROP here */
> +
> + /* No local check for CAN_RTR_FLAG or FD frame against negotiated
> + * features. The device will reject those anyway if not supported.
> + */
> +
> + can_tx_msg = kzalloc(sizeof(*can_tx_msg) + cf->len, GFP_ATOMIC);
> + if (!can_tx_msg) {
> + kfree_skb(skb);
> + dev->stats.tx_dropped++;
> + goto kick; /* No way to return NET_XMIT_DROP here */
> + }
> +
> + can_tx_msg->tx_out.msg_type = cpu_to_le16(VIRTIO_CAN_TX);
> + can_tx_msg->tx_out.length = cpu_to_le16(cf->len);
> + can_flags = 0;
> +
> + if (cf->can_id & CAN_EFF_FLAG) {
> + can_flags |= VIRTIO_CAN_FLAGS_EXTENDED;
> + can_tx_msg->tx_out.can_id = cpu_to_le32(cf->can_id & CAN_EFF_MASK);
> + } else {
> + can_tx_msg->tx_out.can_id = cpu_to_le32(cf->can_id & CAN_SFF_MASK);
> + }
> + if (cf->can_id & CAN_RTR_FLAG)
> + can_flags |= VIRTIO_CAN_FLAGS_RTR;
> + else
> + memcpy(can_tx_msg->tx_out.sdu, cf->data, cf->len);
> + if (can_is_canfd_skb(skb))
> + can_flags |= VIRTIO_CAN_FLAGS_FD;
> +
> + can_tx_msg->tx_out.flags = cpu_to_le32(can_flags);
> +
> + sg_init_one(&sg_out, &can_tx_msg->tx_out, hdr_size + cf->len);
> + sg_init_one(&sg_in, &can_tx_msg->tx_in, sizeof(can_tx_msg->tx_in));
> +
> + putidx = virtio_can_alloc_tx_idx(priv);
> +
> + if (unlikely(putidx < 0)) {
> + /* -ENOMEM or -ENOSPC here. -ENOSPC should not be possible as
> + * tx_inflight >= can.echo_skb_max is checked in flow control
> + */
> + WARN_ON_ONCE(putidx == -ENOSPC);
> + kfree(can_tx_msg);
> + kfree_skb(skb);
> + dev->stats.tx_dropped++;
> + goto kick; /* No way to return NET_XMIT_DROP here */
> + }
> +
> + can_tx_msg->putidx = (unsigned int)putidx;
> +
> + /* Push loopback echo. Will be looped back on TX interrupt/TX NAPI */
> + err = can_put_echo_skb(skb, dev, can_tx_msg->putidx, 0);
> + if (unlikely(err)) {
> + /* skb was already freed by can_put_echo_skb() on error */
> + virtio_can_free_tx_idx(priv, can_tx_msg->putidx);
> + kfree(can_tx_msg);
> + dev->stats.tx_dropped++;
> + goto kick;
> + }
> +
> + /* Protect queue and list operations */
> + scoped_guard(spinlock_irqsave, &priv->tx_lock)
> + err = virtqueue_add_sgs(vq, sgs, 1u, 1u, can_tx_msg, GFP_ATOMIC);
> +
> + if (unlikely(err)) {
> + /*
> + * can_put_echo_skb() already consumed skb via consume_skb(),
> + * so returning NETDEV_TX_BUSY would cause the stack to requeue
> + * a freed pointer. Drop the frame and return OK instead.
> + */
> + can_free_echo_skb(dev, can_tx_msg->putidx, NULL);
> + virtio_can_free_tx_idx(priv, can_tx_msg->putidx);
> + netif_stop_queue(dev);
> + kfree(can_tx_msg);
> + dev->stats.tx_dropped++;
> + /* Expected never to be seen */
> + netdev_warn(dev, "TX: Stop queue, err = %d\n", err);
> + goto kick;
> + }
> +
> + /* Normal flow control: stop queue when no transmission slots left */
> + if (atomic_read(&priv->tx_inflight) >= priv->can.echo_skb_max ||
> + vq->num_free == 0 || (vq->num_free < ARRAY_SIZE(sgs) &&
> + !virtio_has_feature(vq->vdev, VIRTIO_RING_F_INDIRECT_DESC))) {
> + netif_stop_queue(dev);
> + netdev_dbg(dev, "TX: Normal stop queue\n");
> + }
> +
> +kick:
> + if (netif_queue_stopped(dev) || !netdev_xmit_more()) {
> + scoped_guard(spinlock_irqsave, &priv->tx_lock) {
> + if (!virtqueue_kick(vq))
> + netdev_err(dev, "%s(): Kick failed\n", __func__);
> + }
> + }
> +
> + return xmit_ret;
> +}
> +
> +static const struct net_device_ops virtio_can_netdev_ops = {
> + .ndo_open = virtio_can_open,
> + .ndo_stop = virtio_can_close,
> + .ndo_start_xmit = virtio_can_start_xmit,
> +};
> +
> +static int register_virtio_can_dev(struct net_device *dev)
> +{
> + dev->flags |= IFF_ECHO; /* we support local echo */
> + dev->netdev_ops = &virtio_can_netdev_ops;
> +
> + return register_candev(dev);
> +}
> +
> +static int virtio_can_read_tx_queue(struct virtqueue *vq)
> +{
> + struct virtio_can_priv *can_priv = vq->vdev->priv;
> + struct net_device *dev = can_priv->dev;
> + struct virtio_can_tx *can_tx_msg;
> + struct net_device_stats *stats;
> + unsigned int len;
> + u8 result;
> +
> + stats = &dev->stats;
> +
> + scoped_guard(spinlock_irqsave, &can_priv->tx_lock)
> + can_tx_msg = virtqueue_get_buf(vq, &len);
> +
> + if (!can_tx_msg)
> + return 0;
> +
> + if (unlikely(len < sizeof(struct virtio_can_tx_in))) {
> + netdev_err(dev, "TX ACK: Device sent no result code\n");
> + result = VIRTIO_CAN_RESULT_NOT_OK; /* Keep things going */
> + } else {
> + result = can_tx_msg->tx_in.result;
> + }
> +
> + if (can_priv->can.state < CAN_STATE_BUS_OFF) {
> + if (result != VIRTIO_CAN_RESULT_OK) {
> + struct can_frame *skb_cf;
> + struct sk_buff *skb = alloc_can_err_skb(dev, &skb_cf);
> +
> + if (skb) {
> + skb_cf->can_id |= CAN_ERR_CRTL;
> + skb_cf->data[1] |= CAN_ERR_CRTL_UNSPEC;
> + netif_rx(skb);
> + }
> + netdev_warn(dev, "TX ACK: Result = %u\n", result);
> + can_free_echo_skb(dev, can_tx_msg->putidx, NULL);
> + stats->tx_dropped++;
> + } else {
> + stats->tx_bytes += can_get_echo_skb(dev, can_tx_msg->putidx,
> + NULL);
> + stats->tx_packets++;
> + }
> + } else {
> + netdev_dbg(dev, "TX ACK: Controller inactive, drop echo\n");
> + can_free_echo_skb(dev, can_tx_msg->putidx, NULL);
> + stats->tx_dropped++;
> + }
> +
> + virtio_can_free_tx_idx(can_priv, can_tx_msg->putidx);
> +
> + /* Flow control */
> + if (netif_queue_stopped(dev)) {
> + netdev_dbg(dev, "TX ACK: Wake up stopped queue\n");
> + netif_wake_queue(dev);
> + }
> +
> + kfree(can_tx_msg);
> +
> + return 1; /* Queue was not empty so there may be more data */
> +}
> +
> +static int virtio_can_tx_poll(struct napi_struct *napi, int quota)
> +{
> + struct net_device *dev = napi->dev;
> + struct virtio_can_priv *priv = netdev_priv(dev);
> + struct virtqueue *vq = priv->vqs[VIRTIO_CAN_QUEUE_TX];
> + int work_done = 0;
> +
> + while (work_done < quota && virtio_can_read_tx_queue(vq) != 0)
> + work_done++;
> +
> + if (work_done < quota)
> + virtqueue_napi_complete(napi, vq, work_done);
> +
> + return work_done;
> +}
> +
> +static void virtio_can_tx_intr(struct virtqueue *vq)
> +{
> + struct virtio_can_priv *can_priv = vq->vdev->priv;
> +
> + virtqueue_disable_cb(vq);
> + napi_schedule(&can_priv->napi_tx);
> +}
> +
> +/* This function is the NAPI RX poll function and NAPI guarantees that this
> + * function is not invoked simultaneously on multiple processors.
> + * Read a RX message from the used queue and sends it to the upper layer.
> + */
> +static int virtio_can_read_rx_queue(struct virtqueue *vq)
> +{
> + const unsigned int header_size = sizeof(struct virtio_can_rx);
> + struct virtio_can_priv *priv = vq->vdev->priv;
> + struct net_device *dev = priv->dev;
> + struct net_device_stats *stats;
> + struct virtio_can_rx *can_rx;
> + unsigned int transport_len;
> + struct canfd_frame *cf;
> + struct sk_buff *skb;
> + unsigned int len;
> + u32 can_flags;
> + u16 msg_type;
> + u32 can_id;
> + int ret;
> +
> + stats = &dev->stats;
> +
> + can_rx = virtqueue_get_buf(vq, &transport_len);
> + if (!can_rx)
> + return 0; /* No more data */
> +
> + if (transport_len < header_size) {
> + netdev_warn(dev, "RX: Message too small\n");
> + goto putback;
> + }
> +
> + if (priv->can.state >= CAN_STATE_ERROR_PASSIVE) {
> + netdev_dbg(dev, "%s(): Controller not active\n", __func__);
> + goto putback;
> + }
> +
> + msg_type = le16_to_cpu(can_rx->msg_type);
> + if (msg_type != VIRTIO_CAN_RX) {
> + netdev_warn(dev, "RX: Got unknown msg_type %04x\n", msg_type);
> + goto putback;
> + }
> +
> + len = le16_to_cpu(can_rx->length);
> + can_flags = le32_to_cpu(can_rx->flags);
> + can_id = le32_to_cpu(can_rx->can_id);
> +
> + if (can_flags & ~CAN_KNOWN_FLAGS) {
> + stats->rx_dropped++;
> + netdev_warn(dev, "RX: CAN Id 0x%08x: Invalid flags 0x%x\n",
> + can_id, can_flags);
> + goto putback;
> + }
> +
> + if (can_flags & VIRTIO_CAN_FLAGS_EXTENDED) {
> + can_id &= CAN_EFF_MASK;
> + can_id |= CAN_EFF_FLAG;
> + } else {
> + can_id &= CAN_SFF_MASK;
> + }
> +
> + if (can_flags & VIRTIO_CAN_FLAGS_RTR) {
> + if (!virtio_has_feature(vq->vdev, VIRTIO_CAN_F_RTR_FRAMES)) {
> + stats->rx_dropped++;
> + netdev_warn(dev, "RX: CAN Id 0x%08x: RTR not negotiated\n",
> + can_id);
> + goto putback;
> + }
> + if (can_flags & VIRTIO_CAN_FLAGS_FD) {
> + stats->rx_dropped++;
> + netdev_warn(dev, "RX: CAN Id 0x%08x: RTR with FD not possible\n",
> + can_id);
> + goto putback;
> + }
> +
> + if (len > 0xF) {
> + stats->rx_dropped++;
> + netdev_warn(dev, "RX: CAN Id 0x%08x: RTR with DLC > 0xF\n",
> + can_id);
> + goto putback;
> + }
> +
> + if (len > 0x8)
> + len = 0x8;
> +
> + can_id |= CAN_RTR_FLAG;
> + }
> +
> + if (transport_len < header_size + len) {
> + netdev_warn(dev, "RX: Message too small for payload\n");
> + goto putback;
> + }
> +
> + if (can_flags & VIRTIO_CAN_FLAGS_FD) {
> + if (!virtio_has_feature(vq->vdev, VIRTIO_CAN_F_CAN_FD)) {
> + stats->rx_dropped++;
> + netdev_warn(dev, "RX: CAN Id 0x%08x: FD not negotiated\n",
> + can_id);
> + goto putback;
> + }
> +
> + if (len > CANFD_MAX_DLEN)
> + len = CANFD_MAX_DLEN;
> +
> + skb = alloc_canfd_skb(priv->dev, &cf);
> + } else {
> + if (!virtio_has_feature(vq->vdev, VIRTIO_CAN_F_CAN_CLASSIC)) {
> + stats->rx_dropped++;
> + netdev_warn(dev, "RX: CAN Id 0x%08x: classic not negotiated\n",
> + can_id);
> + goto putback;
> + }
> +
> + if (len > CAN_MAX_DLEN)
> + len = CAN_MAX_DLEN;
> +
> + skb = alloc_can_skb(priv->dev, (struct can_frame **)&cf);
> + }
> + if (!skb) {
> + stats->rx_dropped++;
> + netdev_warn(dev, "RX: No skb available\n");
> + goto putback;
> + }
> +
> + cf->can_id = can_id;
> + cf->len = len;
> + if (!(can_flags & VIRTIO_CAN_FLAGS_RTR)) {
> + /* RTR frames have a DLC but no payload */
> + memcpy(cf->data, can_rx->sdu, len);
> + }
> +
> + if (netif_receive_skb(skb) == NET_RX_SUCCESS) {
> + stats->rx_packets++;
> + if (!(can_flags & VIRTIO_CAN_FLAGS_RTR))
> + stats->rx_bytes += len;
> + }
> +
> +putback:
> + /* Put processed RX buffer back into avail queue */
> + ret = virtio_can_add_inbuf(vq, can_rx,
> + priv->rpkt_len);
> + if (!ret)
> + virtqueue_kick(vq);
> + return 1; /* Queue was not empty so there may be more data */
> +}
> +
> +static int virtio_can_handle_busoff(struct net_device *dev)
> +{
> + struct virtio_can_priv *priv = netdev_priv(dev);
> + struct can_frame *cf;
> + struct sk_buff *skb;
> +
> + if (!priv->busoff_pending)
> + return 0;
> +
> + if (priv->can.state < CAN_STATE_BUS_OFF) {
> + netdev_dbg(dev, "entered error bus off state\n");
> +
> + /* bus-off state */
> + priv->can.state = CAN_STATE_BUS_OFF;
> + priv->can.can_stats.bus_off++;
> + can_bus_off(dev);
> + }
> +
> + /* propagate the error condition to the CAN stack */
> + skb = alloc_can_err_skb(dev, &cf);
> + if (unlikely(!skb))
> + return 0;
> +
> + /* bus-off state */
> + cf->can_id |= CAN_ERR_BUSOFF;
> +
> + /* Ensure that the BusOff indication does not get lost */
> + if (netif_receive_skb(skb) == NET_RX_SUCCESS)
> + priv->busoff_pending = false;
> +
> + return 1;
> +}
> +
> +static int virtio_can_rx_poll(struct napi_struct *napi, int quota)
> +{
> + struct net_device *dev = napi->dev;
> + struct virtio_can_priv *priv = netdev_priv(dev);
> + struct virtqueue *vq = priv->vqs[VIRTIO_CAN_QUEUE_RX];
> + int work_done = 0;
> +
> + work_done += virtio_can_handle_busoff(dev);
> +
> + while (work_done < quota && virtio_can_read_rx_queue(vq) != 0)
> + work_done++;
> +
> + if (work_done < quota)
> + virtqueue_napi_complete(napi, vq, work_done);
> +
> + return work_done;
> +}
> +
> +static void virtio_can_rx_intr(struct virtqueue *vq)
> +{
> + struct virtio_can_priv *can_priv = vq->vdev->priv;
> +
> + virtqueue_disable_cb(vq);
> + napi_schedule(&can_priv->napi);
> +}
> +
> +static void virtio_can_control_intr(struct virtqueue *vq)
> +{
> + struct virtio_can_priv *can_priv = vq->vdev->priv;
> +
> + complete(&can_priv->ctrl_done);
> +}
> +
> +static void virtio_can_config_changed(struct virtio_device *vdev)
> +{
> + struct virtio_can_priv *can_priv = vdev->priv;
> + u16 status;
> +
> + status = virtio_cread16(vdev, offsetof(struct virtio_can_config,
> + status));
> +
> + if (!(status & VIRTIO_CAN_S_CTRL_BUSOFF))
> + return;
> +
> + if (!can_priv->busoff_pending &&
> + can_priv->can.state < CAN_STATE_BUS_OFF) {
> + can_priv->busoff_pending = true;
> + napi_schedule(&can_priv->napi);
> + }
> +}
> +
> +static void virtio_can_populate_rx_vq(struct virtio_device *vdev)
> +{
> + struct virtio_can_priv *priv = vdev->priv;
> + struct virtqueue *vq = priv->vqs[VIRTIO_CAN_QUEUE_RX];
> + unsigned int buf_size = priv->rpkt_len;
> + int num_elements = vq->num_free;
> + u8 *buf = (u8 *)priv->rpkt;
> + unsigned int idx;
> + int ret = 0;
> +
> + for (idx = 0; idx < num_elements; idx++) {
> + ret = virtio_can_add_inbuf(vq, buf, buf_size);
> + if (ret < 0) {
> + dev_dbg(&vdev->dev, "rpkt fill: ret=%d, idx=%u, size=%u\n",
> + ret, idx, buf_size);
> + break;
> + }
> + buf += buf_size;
> + }
> +
> + if (idx > 0)
> + virtqueue_kick(vq);
> +
> + dev_dbg(&vdev->dev, "%u rpkt added\n", idx);
> +}
> +
> +static int virtio_can_find_vqs(struct virtio_can_priv *priv)
> +{
> + struct virtqueue_info vqs_info[] = {
> + { "can-tx", virtio_can_tx_intr },
> + { "can-rx", virtio_can_rx_intr },
> + { "can-state-ctrl", virtio_can_control_intr },
> + };
> +
> + /* Find the queues. */
> + return virtio_find_vqs(priv->vdev, VIRTIO_CAN_QUEUE_COUNT, priv->vqs,
> + vqs_info, NULL);
> +}
> +
> +/* Function must not be called before virtio_can_find_vqs() has been run */
> +static void virtio_can_del_vq(struct virtio_device *vdev)
> +{
> + struct virtio_can_priv *priv = vdev->priv;
> + struct virtqueue *vq = priv->vqs[VIRTIO_CAN_QUEUE_TX];
> + struct virtio_can_tx *can_tx_msg;
> + int q;
> +
> + if (!vq)
> + return;
> +
> + /* Reset the device */
> + virtio_reset_device(vdev);
> +
> + /* From here we have dead silence from the device side so no locks
> + * are needed to protect against device side events.
> + */
> +
> + /* Free pending TX buffers which were allocated in virtio_can_start_xmit() */
> + while ((can_tx_msg = virtqueue_detach_unused_buf(vq))) {
> + can_free_echo_skb(priv->dev, can_tx_msg->putidx, NULL);
> + virtio_can_free_tx_idx(priv, can_tx_msg->putidx);
> + kfree(can_tx_msg);
> + }
> +
> + /* RX and control queue buffers are managed elsewhere, just detach */
> + for (q = VIRTIO_CAN_QUEUE_RX; q < VIRTIO_CAN_QUEUE_COUNT; q++)
> + while (virtqueue_detach_unused_buf(priv->vqs[q]))
> + ;
> +
> + if (vdev->config->del_vqs)
> + vdev->config->del_vqs(vdev);
> +
> + memset(priv->vqs, 0, sizeof(priv->vqs));
> +}
> +
> +static void virtio_can_remove(struct virtio_device *vdev)
> +{
> + struct virtio_can_priv *priv = vdev->priv;
> + struct net_device *dev = priv->dev;
> +
> + unregister_candev(dev);
> +
> + virtio_can_del_vq(vdev);
> +
> + virtio_can_free_candev(dev);
> +}
> +
> +static int virtio_can_validate(struct virtio_device *vdev)
> +{
> + /* CAN needs always access to the config space.
> + * Check that the driver can access the config space
> + */
> + if (!vdev->config->get) {
> + dev_err(&vdev->dev, "%s failure: config access disabled\n",
> + __func__);
> + return -EINVAL;
> + }
> +
> + if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> + dev_err(&vdev->dev,
> + "device does not comply with spec version 1.x\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int virtio_can_probe(struct virtio_device *vdev)
> +{
> + struct virtio_can_priv *priv;
> + struct net_device *dev;
> + size_t size;
> + int err;
> +
> + dev = alloc_candev(sizeof(struct virtio_can_priv),
> + VIRTIO_CAN_ECHO_SKB_MAX);
> + if (!dev)
> + return -ENOMEM;
> +
> + priv = netdev_priv(dev);
> +
> + ida_init(&priv->tx_putidx_ida);
> +
> + netif_napi_add(dev, &priv->napi, virtio_can_rx_poll);
> + netif_napi_add(dev, &priv->napi_tx, virtio_can_tx_poll);
> +
> + SET_NETDEV_DEV(dev, &vdev->dev);
> +
> + priv->dev = dev;
> + priv->vdev = vdev;
> + vdev->priv = priv;
> +
> + priv->can.do_set_mode = virtio_can_set_mode;
> + priv->can.bittiming.bitrate = CAN_BITRATE_UNKNOWN;
> + /* Set Virtio CAN supported operations */
> + priv->can.ctrlmode_supported = CAN_CTRLMODE_BERR_REPORTING;
> + if (virtio_has_feature(vdev, VIRTIO_CAN_F_CAN_FD)) {
> + priv->can.fd.data_bittiming.bitrate = CAN_BITRATE_UNKNOWN;
> + err = can_set_static_ctrlmode(dev, CAN_CTRLMODE_FD);
> + if (err != 0)
> + goto on_failure;
> + }
> +
> + /* Initialize virtqueues */
> + err = virtio_can_find_vqs(priv);
> + if (err != 0)
> + goto on_failure;
> +
> + spin_lock_init(&priv->tx_lock);
> + mutex_init(&priv->ctrl_lock);
> +
> + init_completion(&priv->ctrl_done);
> +
> + priv->rpkt_len = sizeof(struct virtio_can_rx);
> +
> + if (virtio_has_feature(vdev, VIRTIO_CAN_F_CAN_FD))
> + priv->rpkt_len += CANFD_MAX_DLEN;
> + else
> + priv->rpkt_len += CAN_MAX_DLEN;
> +
> + size = priv->rpkt_len * priv->vqs[VIRTIO_CAN_QUEUE_RX]->num_free;
> + priv->rpkt = devm_kzalloc(&vdev->dev, size, GFP_KERNEL);
> + if (!priv->rpkt) {
> + virtio_can_del_vq(vdev);
> + err = -ENOMEM;
> + goto on_failure;
> + }
> + virtio_can_populate_rx_vq(vdev);
> +
> + err = register_virtio_can_dev(dev);
> + if (err) {
> + virtio_can_del_vq(vdev);
> + goto on_failure;
> + }
> +
> + return 0;
> +
> +on_failure:
> + virtio_can_free_candev(dev);
> + return err;
> +}
> +
> +static int __maybe_unused virtio_can_freeze(struct virtio_device *vdev)
> +{
> + struct virtio_can_priv *priv = vdev->priv;
> + struct net_device *ndev = priv->dev;
> +
> + if (netif_running(ndev)) {
> + /* virtio_can_close() calls netif_stop_queue(), virtio_can_stop(),
> + * napi_disable() and close_candev(). Call it directly (not via
> + * dev_close()) to preserve IFF_UP so that netif_running() returns
> + * true in virtio_can_restore() and the device is brought back up.
> + */
> + virtio_can_close(ndev);
> + netif_device_detach(ndev);
> + }
> +
> + priv->can.state = CAN_STATE_SLEEPING;
> +
> + virtio_can_del_vq(vdev);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused virtio_can_restore(struct virtio_device *vdev)
> +{
> + struct virtio_can_priv *priv = vdev->priv;
> + struct net_device *ndev = priv->dev;
> + size_t size;
> + int err;
> +
> + err = virtio_can_find_vqs(priv);
> + if (err != 0)
> + return err;
> +
> + size = priv->rpkt_len * priv->vqs[VIRTIO_CAN_QUEUE_RX]->num_free;
> + priv->rpkt = devm_krealloc(&vdev->dev, priv->rpkt, size, GFP_KERNEL | __GFP_ZERO);
> + if (!priv->rpkt) {
> + virtio_can_del_vq(vdev);
> + return -ENOMEM;
> + }
> + virtio_can_populate_rx_vq(vdev);
> +
> + if (netif_running(ndev)) {
> + /* virtio_can_open() calls open_candev(), virtio_can_start(),
> + * napi_enable() and netif_start_queue(). Call it directly (not
> + * via dev_open()) since IFF_UP is still set from before freeze.
> + */
> + err = virtio_can_open(ndev);
> + if (err) {
> + virtio_can_del_vq(vdev);
> + return err;
> + }
> + netif_device_attach(ndev);
> + } else {
> + priv->can.state = CAN_STATE_STOPPED;
> + }
> +
> + return 0;
> +}
> +
> +static struct virtio_device_id virtio_can_id_table[] = {
> + { VIRTIO_ID_CAN, VIRTIO_DEV_ANY_ID },
> + { 0 },
> +};
> +
> +static unsigned int features[] = {
> + VIRTIO_CAN_F_CAN_CLASSIC,
> + VIRTIO_CAN_F_CAN_FD,
> + VIRTIO_CAN_F_LATE_TX_ACK,
> + VIRTIO_CAN_F_RTR_FRAMES,
> +};
> +
> +static struct virtio_driver virtio_can_driver = {
> + .feature_table = features,
> + .feature_table_size = ARRAY_SIZE(features),
> + .driver.name = KBUILD_MODNAME,
> + .driver.owner = THIS_MODULE,
> + .id_table = virtio_can_id_table,
> + .validate = virtio_can_validate,
> + .probe = virtio_can_probe,
> + .remove = virtio_can_remove,
> + .config_changed = virtio_can_config_changed,
> +#ifdef CONFIG_PM_SLEEP
> + .freeze = virtio_can_freeze,
> + .restore = virtio_can_restore,
> +#endif
> +};
> +
> +module_virtio_driver(virtio_can_driver);
> +MODULE_DEVICE_TABLE(virtio, virtio_can_id_table);
> +
> +MODULE_AUTHOR("OpenSynergy GmbH");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("CAN bus driver for Virtio CAN controller");
> diff --git a/include/uapi/linux/virtio_can.h b/include/uapi/linux/virtio_can.h
> new file mode 100644
> index 000000000000..08d7e3e78776
> --- /dev/null
> +++ b/include/uapi/linux/virtio_can.h
> @@ -0,0 +1,78 @@
> +/* SPDX-License-Identifier: BSD-3-Clause */
> +/*
> + * Copyright (C) 2021-2023 OpenSynergy GmbH
> + * Copyright Red Hat, Inc. 2025
> + */
> +#ifndef _LINUX_VIRTIO_VIRTIO_CAN_H
> +#define _LINUX_VIRTIO_VIRTIO_CAN_H
> +
> +#include <linux/types.h>
> +#include <linux/virtio_types.h>
> +#include <linux/virtio_ids.h>
> +#include <linux/virtio_config.h>
> +
> +/* Feature bit numbers */
> +#define VIRTIO_CAN_F_CAN_CLASSIC 0
> +#define VIRTIO_CAN_F_CAN_FD 1
> +#define VIRTIO_CAN_F_RTR_FRAMES 2
> +#define VIRTIO_CAN_F_LATE_TX_ACK 3
> +
> +/* CAN Result Types */
> +#define VIRTIO_CAN_RESULT_OK 0
> +#define VIRTIO_CAN_RESULT_NOT_OK 1
> +
> +/* CAN flags to determine type of CAN Id */
> +#define VIRTIO_CAN_FLAGS_EXTENDED 0x8000
> +#define VIRTIO_CAN_FLAGS_FD 0x4000
> +#define VIRTIO_CAN_FLAGS_RTR 0x2000
> +
> +#define VIRTIO_CAN_MAX_DLEN 64 // this is like CANFD_MAX_DLEN
> +
> +struct virtio_can_config {
> +#define VIRTIO_CAN_S_CTRL_BUSOFF (1u << 0) /* Controller BusOff */
> + /* CAN controller status */
> + __le16 status;
> +};
> +
> +/* TX queue message types */
> +struct virtio_can_tx_out {
> +#define VIRTIO_CAN_TX 0x0001
> + __le16 msg_type;
> + __le16 length; /* 0..8 CC, 0..64 CAN-FD, 0..2048 CAN-XL, 12 bits */
> + __u8 reserved_classic_dlc; /* If CAN classic length = 8 then DLC can be 8..15 */
> + __u8 padding;
> + __le16 reserved_xl_priority; /* May be needed for CAN XL priority */
> + __le32 flags;
> + __le32 can_id;
> + __u8 sdu[] __counted_by_le(length);
> +};
> +
> +struct virtio_can_tx_in {
> + __u8 result;
> +};
> +
> +/* RX queue message types */
> +struct virtio_can_rx {
> +#define VIRTIO_CAN_RX 0x0101
> + __le16 msg_type;
> + __le16 length; /* 0..8 CC, 0..64 CAN-FD, 0..2048 CAN-XL, 12 bits */
> + __u8 reserved_classic_dlc; /* If CAN classic length = 8 then DLC can be 8..15 */
> + __u8 padding;
> + __le16 reserved_xl_priority; /* May be needed for CAN XL priority */
> + __le32 flags;
> + __le32 can_id;
> + __u8 sdu[] __counted_by_le(length);
> +};
> +
> +/* Control queue message types */
> +struct virtio_can_control_out {
> +#define VIRTIO_CAN_SET_CTRL_MODE_START 0x0201
> +#define VIRTIO_CAN_SET_CTRL_MODE_STOP 0x0202
> + __le16 msg_type;
> +};
> +
> +struct virtio_can_control_in {
> + __u8 result;
> +};
> +
> +#endif /* #ifndef _LINUX_VIRTIO_VIRTIO_CAN_H */
> --
> 2.42.0
>
^ permalink raw reply
* Re: [PATCH 0/9] drm: Limit DRM_IOCTL_WAIT_VBLANK to vblank interrupts
From: Thomas Zimmermann @ 2026-05-27 13:07 UTC (permalink / raw)
To: Michel Dänzer, simona, airlied, pekka.paalanen, jadahl,
contact, maarten.lankhorst, mripard
Cc: amd-gfx, dri-devel, linux-hyperv, virtualization, spice-devel,
wayland-devel
In-Reply-To: <cb461424-e4c1-4d2c-934b-ffd7374e2a56@mailbox.org>
Hi
Am 15.05.26 um 18:56 schrieb Michel Dänzer:
> [ Adding the wayland-devel list for awareness ]
>
> On 5/15/26 17:12, Michel Dänzer wrote:
>> On 5/15/26 13:55, Thomas Zimmermann wrote:
>>> DRM's WAIT_VBLANK ioctl synchronizes user-space clients to display
>>> refresh. This is meaningless with vblank timers, which run unrelated
>>> to the hardware's vblank.
>>>
>>> Disable the ioctl for simulated vblanks. Set DRM_VBLANK_FLAG_SIMULATED
>>> for CRTCs with simulated vblank events in all such drivers. The vblank
>>> timers of these devices still rate-limit the number of page-flip events
>>> to match the display refresh.
>>>
>>> According to maintainers, user-space compositors do not require the ioctl
>>> for rate-limitting display output. Weston and Kwin rely on page-flip
>>> events. Mutter uses and internal timer to limit the number of display
>>> updates per second.
>> Actually mutter fundamentally relies on atomic commit completion events for that, same as Weston & KWin. Mutter uses the WAIT_VBLANK ioctl only for minimizing input → output latency (which can hide issues when completion of atomic commits isn't properly throttled).
>>
>>
>> (Just a side not on the cover letter, no objections to the patches themselves)
> After more discussion on IRC, I have some concerns.
>
>
> The big one first: For drivers with no strict refresh cycle (i.e. an atomic commit can take effect more or less anytime after at least one "refresh cycle" has passed since the last one), does this change really make sense / what's the actual benefit?
I don't have a strong opinion on that matter. I just think we should
clarify the meaning of these ioctls.
Timing page flip is currently not supported on any driver without
hardware vblank IRQ or a vblank timer. The situation might vary among
compositors, but there have been plenty of reports of animation and
frame rates either being too high or too low. So I think we should
rollout vblank timers for all drivers without hardware vblank IRQ.
Right now, vblank timers act like a vblank IRQ in these ioctls. That's a
convenient position for user space.
But we don't really sync anything with hardware here, so the alternate
proposal is to not support them. This also appears to be the original
intention of these ioctls. Vblank timers would just limit the internal
page-flip rate, but nothing else. That's the more defensive approach.
I'd prefer this, as it does not give out guarantees to user space that
future drivers might not be able to fulfill.
>
> In the case of the asahi & nvidia drivers, the problem with exposing this functionality to user space is that if the timestamps aren't accurate, it can result in missing display refresh cycles, which are dictated by hardware. That's why it makes sense to reject it there.
>
> When there's no strict refresh cycle, that issue doesn't apply though.
>
>
> Any changes made to the WAIT_VBLANK ioctl should also be made to the CRTC_GET_SEQUENCE / CRTC_QUEUE_SEQUENCE ioctls, which are slightly different UAPI for the same functionality.
I'll send out an update with that fixed in a bit.
Best regards
Thomas
>
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* Re: [PATCH RFC] virtio-mem: support Confidential Computing (CoCo) environments
From: Xiaoyao Li @ 2026-05-27 10:22 UTC (permalink / raw)
To: Marc-André Lureau, David Hildenbrand, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez
Cc: virtualization, linux-kernel, Chenyi Qiang
In-Reply-To: <20260401-coco-v1-1-b9c3072e2d9c@redhat.com>
On 4/1/2026 7:12 PM, Marc-André Lureau wrote:
> In Confidential Computing (CoCo) environments such as Intel TDX or AMD
> SEV-SNP, hotplugged memory must be explicitly "accepted" (transitioned to
> a private/encrypted state) before it can be safely used by the guest.
> Conversely, before returning memory to the hypervisor during an unplug
> operation, it must be converted back to a shared/decrypted state.
It's not a must to convert it back to shared. The memory is going to be
unplugged, the guest doesn't need to care the state of it unless there
is restriction that private memory cannot be unplugged. But we don't
have such restriction.
As I explained in the QEMU thread[1], the VMM needs to discard the
memory (both shared and private) on unplug. If the VMM fails to do so,
the memory is actually not unplugged and the guest is still able to
access them.
If the VMM fails to discard/remove the private memory, either
unintentionally or intentionally, it's the bug of the VMM. For TDX, this
kind of VMM bug can lead to re-accept error. To make TDX guest more
robust, we can let the guest release the memory itself on unplug, as
suggested by Paolo[2] and Kiryl[3], so that it can survive even with
buggy vmm. Converting the memory to shared is another approach for guest
to proactively "release" the private memory. But the justification of it
is not "guest must do so".
[1]
https://lore.kernel.org/qemu-devel/7a9fe710-679e-4366-9eeb-3aba148773d7@intel.com/
[2]
https://lore.kernel.org/lkml/CABgObfZ7_w8Q-dW=Sd4YA3P==BuN1edPv7Ty4EpPyU8ctW6RLg@mail.gmail.com/
[3] https://lore.kernel.org/lkml/acprNlPP7J_ttMrz@thinkstation/
> Attempting to handle memory acceptance automatically using generic
> architecture-level memory hotplug notifiers (e.g., MEM_GOING_ONLINE)
> is not viable for devices like virtio-mem:
>
> 1. Granularity Mismatch: virtio-mem can dynamically hot(un)plug memory
> at a subblock granularity (e.g., 2MB chunks within a 128MB memory
> block). Generic memory notifiers operate on the entire memory block.
> 2. Lifecycle Control: Memory must be explicitly accepted *before* it is
> handed to the core memory management subsystem (the buddy allocator),
> and it must be decrypted *before* being handed back to the device.
> 3. State Tracking (Offline -> Re-online): If memory is offlined and
> re-onlined without proper state transitions, TDX will panic on
> attempting to accept an already-accepted page (TDX_EPT_ENTRY_STATE_INCORRECT).
>
> To address this, this patch implements explicit CoCo memory conversions
> directly within the virtio-mem driver using set_memory_encrypted() and
> set_memory_decrypted():
>
> - During hotplug, explicitly accepts only the physically plugged subblocks
> right before fake-onlining them into the buddy allocator.
> - During unplug, memory is explicitly transitioned to the shared state
> before being handed back to the host. If the unplug operation fails,
> the driver attempts to re-accept (encrypt) the memory. If this
> re-acceptance fails, the memory is intentionally leaked to prevent
> confidentiality breaches or fatal hypervisor faults.
>
> This was discovered while testing virtio-mem resize with TDX guests.
> The associated QEMU virtio-mem + TDX patch series is under review at:
> https://patchew.org/QEMU/20260226140001.3622334-1-marcandre.lureau@redhat.com/
>
> Note that QEMU punches the guest_memfd on KVM_HC_MAP_GPA_RANGE, when the
> guest memory is decrypted. There is thus no need to discard the guest_memfd
> in the virtio-mem device.
>
> This patch is a follow-up and supersedes "[PATCH 0/2] x86/tdx: Fix
> memory hotplug in TDX guests".
>
^ permalink raw reply
* Re: [patch 06/24] KVM: arm64: Use ktime_get_snapshot_id() to snapshot CLOCK_REALTIME
From: Marc Zyngier @ 2026-05-27 9:03 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LKML, David Woodhouse, Miroslav Lichvar, John Stultz,
Stephen Boyd, Anna-Maria Behnsen, Frederic Weisbecker,
thomas.weissschuh, Arthur Kiyanovski, Oliver Upton, kvmarm,
Rodolfo Giometti, Vincent Donnefort, Oliver Upton,
Richard Cochran, netdev, Takashi Iwai, Miri Korenblit,
Johannes Berg, Jacob Keller, Tony Nguyen, Saeed Mahameed,
Peter Hilber, Michael S. Tsirkin, virtualization, linux-wireless,
linux-sound
In-Reply-To: <20260526171223.144327185@kernel.org>
On Tue, 26 May 2026 18:13:58 +0100,
Thomas Gleixner <tglx@kernel.org> wrote:
>
> ktime_get_snapshot() is replaced by ktime_get_snapshot_id() which allows to
> request a particular CLOCK ID to be captured along with the clocksource
> counter.
>
> Convert the usage in kvm_get_ptp_time() over and use the new
> system_time_snapshot::sys field, which holds the system timestamp selected
> by the CLOCK ID argument.
>
> No functional change intended.
>
> Signed-off-by: Thomas Gleixner <tglx@kernel.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Oliver Upton <oupton@kernel.org>
> Cc: kvmarm@lists.linux.dev
> ---
> arch/arm64/kvm/hypercalls.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Acked-by: Marc Zyngier <maz@kernel.org>
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [patch 05/24] KVM: arm64: Use ktime_get_snapshot_id() to retrieve CLOCK_BOOTTIME
From: Marc Zyngier @ 2026-05-27 9:02 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LKML, David Woodhouse, Miroslav Lichvar, John Stultz,
Stephen Boyd, Anna-Maria Behnsen, Frederic Weisbecker,
thomas.weissschuh, Arthur Kiyanovski, Vincent Donnefort,
Oliver Upton, kvmarm, Rodolfo Giometti, Oliver Upton,
Richard Cochran, netdev, Takashi Iwai, Miri Korenblit,
Johannes Berg, Jacob Keller, Tony Nguyen, Saeed Mahameed,
Peter Hilber, Michael S. Tsirkin, virtualization, linux-wireless,
linux-sound
In-Reply-To: <20260526171223.070087856@kernel.org>
On Tue, 26 May 2026 18:13:53 +0100,
Thomas Gleixner <tglx@kernel.org> wrote:
>
> ktime_get_snapshot() is replaced by ktime_get_snapshot_id() which allows to
> request a particular CLOCK ID to be captured along with the clocksource
> counter.
>
> Convert the tracing mechanism over and use the new
> system_time_snapshot::sys field, which holds the system timestamp selected
> by the CLOCK ID argument.
>
> No functional change intended.
>
> Signed-off-by: Thomas Gleixner <tglx@kernel.org>
> Cc: Vincent Donnefort <vdonnefort@google.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Oliver Upton <oliver.upton@linux.dev>
> Cc: kvmarm@lists.linux.dev
> ---
> arch/arm64/kvm/hyp_trace.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
Acked-by: Marc Zyngier <maz@kernel.org>
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [patch 23/24] timekeeping: Add support for AUX clock cross timestamping
From: Thomas Weißschuh @ 2026-05-27 7:24 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LKML, David Woodhouse, Miroslav Lichvar, John Stultz,
Stephen Boyd, Anna-Maria Behnsen, Frederic Weisbecker,
Arthur Kiyanovski, Rodolfo Giometti, Vincent Donnefort,
Marc Zyngier, Oliver Upton, kvmarm, Oliver Upton, Richard Cochran,
netdev, Takashi Iwai, Miri Korenblit, Johannes Berg, Jacob Keller,
Tony Nguyen, Saeed Mahameed, Peter Hilber, Michael S. Tsirkin,
virtualization, linux-wireless, linux-sound
In-Reply-To: <20260526171224.423155175@kernel.org>
On Tue, May 26, 2026 at 07:15:22PM +0200, Thomas Gleixner wrote:
> Now that all prerequisites are in place add the final support for AUX
> clocks in get_device_system_crosststamp(), which enables the PTP layer to
> support hardware cross timestamps with a new IOTCL.
>
> Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Reviewed-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
^ permalink raw reply
* Re: [patch 22/24] timekeeping: Remove system_device_crosststamp::sys_realtime
From: Thomas Weißschuh @ 2026-05-27 7:24 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LKML, David Woodhouse, Miroslav Lichvar, John Stultz,
Stephen Boyd, Anna-Maria Behnsen, Frederic Weisbecker,
Arthur Kiyanovski, Rodolfo Giometti, Vincent Donnefort,
Marc Zyngier, Oliver Upton, kvmarm, Oliver Upton, Richard Cochran,
netdev, Takashi Iwai, Miri Korenblit, Johannes Berg, Jacob Keller,
Tony Nguyen, Saeed Mahameed, Peter Hilber, Michael S. Tsirkin,
virtualization, linux-wireless, linux-sound
In-Reply-To: <20260526171224.347997043@kernel.org>
On Tue, May 26, 2026 at 07:15:17PM +0200, Thomas Gleixner wrote:
> All users are converted to sys_systime.
>
> Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Reviewed-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
^ permalink raw reply
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