* [PATCH v2 09/15] drm/mediatek: remove dependency on DRM simple helpers
From: Diogo Silva @ 2026-07-20 15:40 UTC (permalink / raw)
To: Jingoo Han, Inki Dae, Seung-Woo Kim, Kyungmin Park, David Airlie,
Simona Vetter, Krzysztof Kozlowski, Peter Griffin, Alim Akhtar,
Laurent Pinchart, Tomi Valkeinen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Michal Simek, Thierry Reding,
Mikko Perttunen, Jonathan Hunter, Stefan Agner, Alison Wang,
Anitha Chrisanthus, David Airlie, Gerd Hoffmann, Dmitry Osipenko,
Gurchetan Singh, Chia-I Wu, Jyri Sarha, Liu Ying, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Geert Uytterhoeven, Xinliang Liu,
Sumit Semwal, Yongqin Liu, John Stultz, Liviu Dudau,
Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Jonathan Corbet, Shuah Khan
Cc: dri-devel, linux-arm-kernel, linux-samsung-soc, linux-kernel,
linux-tegra, virtualization, imx, linux-mediatek,
linux-renesas-soc, linux-amlogic, linux-doc, Diogo Silva
In-Reply-To: <20260720-drm_simple_encoder_init-v2-0-5020b630668a@gmail.com>
The simple KMS helpers are deprecated because they only add an
intermediate layer between drivers and atomic modesetting.
Open-code drm_simple_encoder_init() by calling drm_encoder_init()
directly and providing driver-local drm_encoder_funcs.
Signed-off-by: Diogo Silva <diogompaissilva@gmail.com>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 3f3f56eed3f9..7cd136bd9605 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -21,12 +21,12 @@
#include <drm/drm_atomic_helper.h>
#include <drm/drm_bridge.h>
#include <drm/drm_bridge_connector.h>
+#include <drm/drm_encoder.h>
#include <drm/drm_mipi_dsi.h>
#include <drm/drm_of.h>
#include <drm/drm_panel.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
#include "mtk_ddp_comp.h"
#include "mtk_disp_drv.h"
@@ -913,12 +913,16 @@ void mtk_dsi_ddp_stop(struct device *dev)
mtk_dsi_poweroff(dsi);
}
+static const struct drm_encoder_funcs mtk_dsi_encoder_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
static int mtk_dsi_encoder_init(struct drm_device *drm, struct mtk_dsi *dsi)
{
int ret;
- ret = drm_simple_encoder_init(drm, &dsi->encoder,
- DRM_MODE_ENCODER_DSI);
+ ret = drm_encoder_init(drm, &dsi->encoder, &mtk_dsi_encoder_funcs,
+ DRM_MODE_ENCODER_DSI, NULL);
if (ret) {
drm_err(drm, "Failed to encoder init to drm\n");
return ret;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 10/15] drm/renesas/shmobile: remove dependency on DRM simple helpers
From: Diogo Silva @ 2026-07-20 15:40 UTC (permalink / raw)
To: Jingoo Han, Inki Dae, Seung-Woo Kim, Kyungmin Park, David Airlie,
Simona Vetter, Krzysztof Kozlowski, Peter Griffin, Alim Akhtar,
Laurent Pinchart, Tomi Valkeinen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Michal Simek, Thierry Reding,
Mikko Perttunen, Jonathan Hunter, Stefan Agner, Alison Wang,
Anitha Chrisanthus, David Airlie, Gerd Hoffmann, Dmitry Osipenko,
Gurchetan Singh, Chia-I Wu, Jyri Sarha, Liu Ying, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Geert Uytterhoeven, Xinliang Liu,
Sumit Semwal, Yongqin Liu, John Stultz, Liviu Dudau,
Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Jonathan Corbet, Shuah Khan
Cc: dri-devel, linux-arm-kernel, linux-samsung-soc, linux-kernel,
linux-tegra, virtualization, imx, linux-mediatek,
linux-renesas-soc, linux-amlogic, linux-doc, Diogo Silva
In-Reply-To: <20260720-drm_simple_encoder_init-v2-0-5020b630668a@gmail.com>
The simple KMS helpers are deprecated because they only add an
intermediate layer between drivers and atomic modesetting.
Open-code drm_simple_encoder_init() by calling drm_encoder_init()
directly and providing driver-local drm_encoder_funcs.
Signed-off-by: Diogo Silva <diogompaissilva@gmail.com>
---
drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c
index 1a2b9b68af6f..2dc477c7eda6 100644
--- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c
+++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_crtc.c
@@ -21,6 +21,7 @@
#include <drm/drm_bridge_connector.h>
#include <drm/drm_crtc.h>
#include <drm/drm_crtc_helper.h>
+#include <drm/drm_encoder.h>
#include <drm/drm_fb_dma_helper.h>
#include <drm/drm_fourcc.h>
#include <drm/drm_framebuffer.h>
@@ -29,7 +30,6 @@
#include <drm/drm_modeset_helper_vtables.h>
#include <drm/drm_panel.h>
#include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
#include <drm/drm_vblank.h>
#include <video/videomode.h>
@@ -436,6 +436,10 @@ static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
.mode_fixup = shmob_drm_encoder_mode_fixup,
};
+static const struct drm_encoder_funcs shmob_encoder_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
/* -----------------------------------------------------------------------------
* Encoder
*/
@@ -448,8 +452,8 @@ int shmob_drm_encoder_create(struct shmob_drm_device *sdev)
encoder->possible_crtcs = 1;
- ret = drm_simple_encoder_init(&sdev->ddev, encoder,
- DRM_MODE_ENCODER_DPI);
+ ret = drm_encoder_init(&sdev->ddev, encoder, &shmob_encoder_funcs,
+ DRM_MODE_ENCODER_DPI, NULL);
if (ret < 0)
return ret;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 11/15] drm/hisilicon/kirin: remove dependency on DRM simple helpers
From: Diogo Silva @ 2026-07-20 15:40 UTC (permalink / raw)
To: Jingoo Han, Inki Dae, Seung-Woo Kim, Kyungmin Park, David Airlie,
Simona Vetter, Krzysztof Kozlowski, Peter Griffin, Alim Akhtar,
Laurent Pinchart, Tomi Valkeinen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Michal Simek, Thierry Reding,
Mikko Perttunen, Jonathan Hunter, Stefan Agner, Alison Wang,
Anitha Chrisanthus, David Airlie, Gerd Hoffmann, Dmitry Osipenko,
Gurchetan Singh, Chia-I Wu, Jyri Sarha, Liu Ying, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Geert Uytterhoeven, Xinliang Liu,
Sumit Semwal, Yongqin Liu, John Stultz, Liviu Dudau,
Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Jonathan Corbet, Shuah Khan
Cc: dri-devel, linux-arm-kernel, linux-samsung-soc, linux-kernel,
linux-tegra, virtualization, imx, linux-mediatek,
linux-renesas-soc, linux-amlogic, linux-doc, Diogo Silva
In-Reply-To: <20260720-drm_simple_encoder_init-v2-0-5020b630668a@gmail.com>
The simple KMS helpers are deprecated because they only add an
intermediate layer between drivers and atomic modesetting.
Open-code drm_simple_encoder_init() by calling drm_encoder_init()
directly and providing driver-local drm_encoder_funcs.
Signed-off-by: Diogo Silva <diogompaissilva@gmail.com>
---
drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
index 15042365dec0..62c5bd3277da 100644
--- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
+++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
@@ -20,11 +20,11 @@
#include <drm/drm_atomic_helper.h>
#include <drm/drm_bridge.h>
#include <drm/drm_device.h>
+#include <drm/drm_encoder.h>
#include <drm/drm_mipi_dsi.h>
#include <drm/drm_of.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
#include "dw_dsi_reg.h"
@@ -687,6 +687,10 @@ static int dsi_encoder_atomic_check(struct drm_encoder *encoder,
return 0;
}
+static const struct drm_encoder_funcs dw_encoder_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
static const struct drm_encoder_helper_funcs dw_encoder_helper_funcs = {
.atomic_check = dsi_encoder_atomic_check,
.mode_valid = dsi_encoder_mode_valid,
@@ -708,7 +712,8 @@ static int dw_drm_encoder_init(struct device *dev,
}
encoder->possible_crtcs = crtc_mask;
- ret = drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_DSI);
+ ret = drm_encoder_init(drm_dev, encoder, &dw_encoder_funcs,
+ DRM_MODE_ENCODER_DSI, NULL);
if (ret) {
DRM_ERROR("failed to init dsi encoder\n");
return ret;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 12/15] drm/arm/komeda: remove dependency on DRM simple helpers
From: Diogo Silva @ 2026-07-20 15:40 UTC (permalink / raw)
To: Jingoo Han, Inki Dae, Seung-Woo Kim, Kyungmin Park, David Airlie,
Simona Vetter, Krzysztof Kozlowski, Peter Griffin, Alim Akhtar,
Laurent Pinchart, Tomi Valkeinen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Michal Simek, Thierry Reding,
Mikko Perttunen, Jonathan Hunter, Stefan Agner, Alison Wang,
Anitha Chrisanthus, David Airlie, Gerd Hoffmann, Dmitry Osipenko,
Gurchetan Singh, Chia-I Wu, Jyri Sarha, Liu Ying, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Geert Uytterhoeven, Xinliang Liu,
Sumit Semwal, Yongqin Liu, John Stultz, Liviu Dudau,
Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Jonathan Corbet, Shuah Khan
Cc: dri-devel, linux-arm-kernel, linux-samsung-soc, linux-kernel,
linux-tegra, virtualization, imx, linux-mediatek,
linux-renesas-soc, linux-amlogic, linux-doc, Diogo Silva
In-Reply-To: <20260720-drm_simple_encoder_init-v2-0-5020b630668a@gmail.com>
The simple KMS helpers are deprecated because they only add an
intermediate layer between drivers and atomic modesetting.
Open-code drm_simple_encoder_init() by calling drm_encoder_init()
directly and providing driver-local drm_encoder_funcs.
Signed-off-by: Diogo Silva <diogompaissilva@gmail.com>
---
drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index e8cb782a6f8e..719568d9f7c2 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -11,9 +11,9 @@
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
+#include <drm/drm_encoder.h>
#include <drm/drm_print.h>
#include <drm/drm_vblank.h>
-#include <drm/drm_simple_kms_helper.h>
#include <drm/drm_bridge.h>
#include "komeda_dev.h"
@@ -635,6 +635,10 @@ static int komeda_attach_bridge(struct device *dev,
return err;
}
+static const struct drm_encoder_funcs komeda_encoder_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
static int komeda_crtc_add(struct komeda_kms_dev *kms,
struct komeda_crtc *kcrtc)
{
@@ -658,7 +662,8 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms,
* bridge
*/
kcrtc->encoder.possible_crtcs = drm_crtc_mask(crtc);
- err = drm_simple_encoder_init(base, encoder, DRM_MODE_ENCODER_TMDS);
+ err = drm_encoder_init(base, encoder, &komeda_encoder_funcs,
+ DRM_MODE_ENCODER_TMDS, NULL);
if (err)
return err;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 13/15] drm/meson: remove dependency on DRM simple helpers
From: Diogo Silva @ 2026-07-20 15:40 UTC (permalink / raw)
To: Jingoo Han, Inki Dae, Seung-Woo Kim, Kyungmin Park, David Airlie,
Simona Vetter, Krzysztof Kozlowski, Peter Griffin, Alim Akhtar,
Laurent Pinchart, Tomi Valkeinen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Michal Simek, Thierry Reding,
Mikko Perttunen, Jonathan Hunter, Stefan Agner, Alison Wang,
Anitha Chrisanthus, David Airlie, Gerd Hoffmann, Dmitry Osipenko,
Gurchetan Singh, Chia-I Wu, Jyri Sarha, Liu Ying, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Geert Uytterhoeven, Xinliang Liu,
Sumit Semwal, Yongqin Liu, John Stultz, Liviu Dudau,
Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Jonathan Corbet, Shuah Khan
Cc: dri-devel, linux-arm-kernel, linux-samsung-soc, linux-kernel,
linux-tegra, virtualization, imx, linux-mediatek,
linux-renesas-soc, linux-amlogic, linux-doc, Diogo Silva
In-Reply-To: <20260720-drm_simple_encoder_init-v2-0-5020b630668a@gmail.com>
The simple KMS helpers are deprecated because they only add an
intermediate layer between drivers and atomic modesetting.
Open-code drm_simple_encoder_init() by calling drm_encoder_init()
directly and providing driver-local drm_encoder_funcs.
Signed-off-by: Diogo Silva <diogompaissilva@gmail.com>
---
drivers/gpu/drm/meson/meson_encoder_cvbs.c | 11 ++++++++---
drivers/gpu/drm/meson/meson_encoder_dsi.c | 11 ++++++++---
drivers/gpu/drm/meson/meson_encoder_hdmi.c | 11 ++++++++---
3 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
index 22cacb1660c4..cdb84d2283f8 100644
--- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
+++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
@@ -17,8 +17,8 @@
#include <drm/drm_bridge_connector.h>
#include <drm/drm_device.h>
#include <drm/drm_edid.h>
+#include <drm/drm_encoder.h>
#include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
#include "meson_registers.h"
#include "meson_vclk.h"
@@ -218,6 +218,10 @@ static const struct drm_bridge_funcs meson_encoder_cvbs_bridge_funcs = {
.atomic_create_state = drm_atomic_helper_bridge_create_state,
};
+static const struct drm_encoder_funcs meson_encoder_cvbs_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
int meson_encoder_cvbs_probe(struct meson_drm *priv)
{
struct drm_device *drm = priv->drm;
@@ -257,8 +261,9 @@ int meson_encoder_cvbs_probe(struct meson_drm *priv)
meson_encoder_cvbs->priv = priv;
/* Encoder */
- ret = drm_simple_encoder_init(priv->drm, &meson_encoder_cvbs->encoder,
- DRM_MODE_ENCODER_TVDAC);
+ ret = drm_encoder_init(priv->drm, &meson_encoder_cvbs->encoder,
+ &meson_encoder_cvbs_funcs,
+ DRM_MODE_ENCODER_TVDAC, NULL);
if (ret)
return dev_err_probe(priv->dev, ret,
"Failed to init CVBS encoder\n");
diff --git a/drivers/gpu/drm/meson/meson_encoder_dsi.c b/drivers/gpu/drm/meson/meson_encoder_dsi.c
index 3e422b612f74..faa309cb97a6 100644
--- a/drivers/gpu/drm/meson/meson_encoder_dsi.c
+++ b/drivers/gpu/drm/meson/meson_encoder_dsi.c
@@ -10,10 +10,10 @@
#include <linux/of_graph.h>
#include <drm/drm_atomic_helper.h>
-#include <drm/drm_simple_kms_helper.h>
#include <drm/drm_bridge.h>
#include <drm/drm_bridge_connector.h>
#include <drm/drm_device.h>
+#include <drm/drm_encoder.h>
#include <drm/drm_probe_helper.h>
#include "meson_drv.h"
@@ -99,6 +99,10 @@ static const struct drm_bridge_funcs meson_encoder_dsi_bridge_funcs = {
.atomic_create_state = drm_atomic_helper_bridge_create_state,
};
+static const struct drm_encoder_funcs meson_encoder_dsi_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
int meson_encoder_dsi_probe(struct meson_drm *priv)
{
struct meson_encoder_dsi *meson_encoder_dsi;
@@ -133,8 +137,9 @@ int meson_encoder_dsi_probe(struct meson_drm *priv)
meson_encoder_dsi->priv = priv;
/* Encoder */
- ret = drm_simple_encoder_init(priv->drm, &meson_encoder_dsi->encoder,
- DRM_MODE_ENCODER_DSI);
+ ret = drm_encoder_init(priv->drm, &meson_encoder_dsi->encoder,
+ &meson_encoder_dsi_funcs, DRM_MODE_ENCODER_DSI,
+ NULL);
if (ret)
return dev_err_probe(priv->dev, ret,
"Failed to init DSI encoder\n");
diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
index 0c7a72cb514a..c4355c5cc340 100644
--- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
@@ -23,8 +23,8 @@
#include <drm/drm_bridge_connector.h>
#include <drm/drm_device.h>
#include <drm/drm_edid.h>
+#include <drm/drm_encoder.h>
#include <drm/drm_probe_helper.h>
-#include <drm/drm_simple_kms_helper.h>
#include <linux/media-bus-format.h>
#include <linux/videodev2.h>
@@ -369,6 +369,10 @@ static const struct drm_bridge_funcs meson_encoder_hdmi_bridge_funcs = {
.atomic_create_state = drm_atomic_helper_bridge_create_state,
};
+static const struct drm_encoder_funcs meson_encoder_hdmi_funcs = {
+ .destroy = drm_encoder_cleanup,
+};
+
int meson_encoder_hdmi_probe(struct meson_drm *priv)
{
struct meson_encoder_hdmi *meson_encoder_hdmi;
@@ -407,8 +411,9 @@ int meson_encoder_hdmi_probe(struct meson_drm *priv)
meson_encoder_hdmi->priv = priv;
/* Encoder */
- ret = drm_simple_encoder_init(priv->drm, &meson_encoder_hdmi->encoder,
- DRM_MODE_ENCODER_TMDS);
+ ret = drm_encoder_init(priv->drm, &meson_encoder_hdmi->encoder,
+ &meson_encoder_hdmi_funcs,
+ DRM_MODE_ENCODER_TMDS, NULL);
if (ret) {
dev_err_probe(priv->dev, ret, "Failed to init HDMI encoder\n");
goto err_put_node;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 14/15] drm/drm_simple: remove deprecated drm_simple_encoder_init function
From: Diogo Silva @ 2026-07-20 15:40 UTC (permalink / raw)
To: Jingoo Han, Inki Dae, Seung-Woo Kim, Kyungmin Park, David Airlie,
Simona Vetter, Krzysztof Kozlowski, Peter Griffin, Alim Akhtar,
Laurent Pinchart, Tomi Valkeinen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Michal Simek, Thierry Reding,
Mikko Perttunen, Jonathan Hunter, Stefan Agner, Alison Wang,
Anitha Chrisanthus, David Airlie, Gerd Hoffmann, Dmitry Osipenko,
Gurchetan Singh, Chia-I Wu, Jyri Sarha, Liu Ying, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Geert Uytterhoeven, Xinliang Liu,
Sumit Semwal, Yongqin Liu, John Stultz, Liviu Dudau,
Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Jonathan Corbet, Shuah Khan
Cc: dri-devel, linux-arm-kernel, linux-samsung-soc, linux-kernel,
linux-tegra, virtualization, imx, linux-mediatek,
linux-renesas-soc, linux-amlogic, linux-doc, Diogo Silva
In-Reply-To: <20260720-drm_simple_encoder_init-v2-0-5020b630668a@gmail.com>
The simple KMS helpers are deprecated because they only add an
intermediate layer between drivers and atomic modesetting.
All driver users of drm_simple_encoder_init() have been converted to
drm_encoder_init(). Drop the helper and open-code its remaining internal
use in drm_simple_display_pipe_init() to prevent new users.
Signed-off-by: Diogo Silva <diogompaissilva@gmail.com>
---
drivers/gpu/drm/drm_simple_kms_helper.c | 13 ++-----------
include/drm/drm_simple_kms_helper.h | 4 ----
2 files changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index 8e1d07b9f1e3..7878b9d7d524 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -20,16 +20,6 @@ static const struct drm_encoder_funcs drm_simple_encoder_funcs_cleanup = {
.destroy = drm_encoder_cleanup,
};
-int drm_simple_encoder_init(struct drm_device *dev,
- struct drm_encoder *encoder,
- int encoder_type)
-{
- return drm_encoder_init(dev, encoder,
- &drm_simple_encoder_funcs_cleanup,
- encoder_type, NULL);
-}
-EXPORT_SYMBOL(drm_simple_encoder_init);
-
void *__drmm_simple_encoder_alloc(struct drm_device *dev, size_t size,
size_t offset, int encoder_type)
{
@@ -363,7 +353,8 @@ int drm_simple_display_pipe_init(struct drm_device *dev,
return ret;
encoder->possible_crtcs = drm_crtc_mask(crtc);
- ret = drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_NONE);
+ ret = drm_encoder_init(dev, encoder, &drm_simple_encoder_funcs_cleanup,
+ DRM_MODE_ENCODER_NONE, NULL);
if (ret || !connector)
return ret;
diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
index cb672ce0e856..c95f86ff355f 100644
--- a/include/drm/drm_simple_kms_helper.h
+++ b/include/drm/drm_simple_kms_helper.h
@@ -68,10 +68,6 @@ int drm_simple_display_pipe_init(struct drm_device *dev,
const uint64_t *format_modifiers,
struct drm_connector *connector);
-int drm_simple_encoder_init(struct drm_device *dev,
- struct drm_encoder *encoder,
- int encoder_type);
-
void *__drmm_simple_encoder_alloc(struct drm_device *dev, size_t size,
size_t offset, int encoder_type);
--
2.54.0
^ permalink raw reply related
* [PATCH v2 15/15] Documentation/gpu: remove completed drm_simple_encoder_init() todo
From: Diogo Silva @ 2026-07-20 15:40 UTC (permalink / raw)
To: Jingoo Han, Inki Dae, Seung-Woo Kim, Kyungmin Park, David Airlie,
Simona Vetter, Krzysztof Kozlowski, Peter Griffin, Alim Akhtar,
Laurent Pinchart, Tomi Valkeinen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Michal Simek, Thierry Reding,
Mikko Perttunen, Jonathan Hunter, Stefan Agner, Alison Wang,
Anitha Chrisanthus, David Airlie, Gerd Hoffmann, Dmitry Osipenko,
Gurchetan Singh, Chia-I Wu, Jyri Sarha, Liu Ying, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Geert Uytterhoeven, Xinliang Liu,
Sumit Semwal, Yongqin Liu, John Stultz, Liviu Dudau,
Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Jonathan Corbet, Shuah Khan
Cc: dri-devel, linux-arm-kernel, linux-samsung-soc, linux-kernel,
linux-tegra, virtualization, imx, linux-mediatek,
linux-renesas-soc, linux-amlogic, linux-doc, Diogo Silva
In-Reply-To: <20260720-drm_simple_encoder_init-v2-0-5020b630668a@gmail.com>
All drm_simple_encoder_init() users have been removed, so drop the
completed todo item.
Signed-off-by: Diogo Silva <diogompaissilva@gmail.com>
---
Documentation/gpu/todo.rst | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 14cf37590fc7..b7351467dc74 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -29,21 +29,6 @@ refactorings already and are an expert in the specific area
Subsystem-wide refactorings
===========================
-Open-code drm_simple_encoder_init()
------------------------------------
-
-The helper drm_simple_encoder_init() was supposed to simplify encoder
-initialization. Instead it only added an intermediate layer between atomic
-modesetting and the DRM driver.
-
-The task here is to remove drm_simple_encoder_init(). Search for a driver
-that calls drm_simple_encoder_init() and inline the helper. The driver will
-also need its own instance of drm_encoder_funcs.
-
-Contact: Thomas Zimmermann, respective driver maintainer
-
-Level: Easy
-
Replace struct drm_simple_display_pipe with regular atomic helpers
------------------------------------------------------------------
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v4 6/8] media: virtio: Add virtio_media_driver
From: Brian Daniels @ 2026-07-20 15:48 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Michael S. Tsirkin, Mauro Carvalho Chehab, acourbot, adelva,
aesteve, changyeon, daniel.almeida, eperezma, gnurou,
gurchetansingh, hverkuil, jasowang, linux-kernel, linux-media,
nicolas.dufresne, virtualization, xuanzhuo
In-Reply-To: <20260712085726.19198fda@foz.lan>
On Sun, Jul 12, 2026 at 2:57 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> On Thu, 25 Jun 2026 16:18:48 -0400
> Brian Daniels <briandaniels@google.com> wrote:
>
> > > > From: Alexandre Courbot <gnurou@gmail.com>
> > > >
> > > > virtio_media_driver.c provides the expected driver hooks, and support
> > > > for mmapping and polling.
> > > >
> > > > Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
> > > > Co-developed-by: Brian Daniels <briandaniels@google.com>
> > > > Signed-off-by: Brian Daniels <briandaniels@google.com>
> > > > ---
> > > > drivers/media/virtio/virtio_media_driver.c | 959 +++++++++++++++++++++
> > > > 1 file changed, 959 insertions(+)
> > > > create mode 100644 drivers/media/virtio/virtio_media_driver.c
> > > >
> > > > diff --git a/drivers/media/virtio/virtio_media_driver.c b/drivers/media/virtio/virtio_media_driver.c
> > > > new file mode 100644
> > > > index 000000000..d6363c673
> > > > --- /dev/null
> > > > +++ b/drivers/media/virtio/virtio_media_driver.c
> > > > @@ -0,0 +1,959 @@
> > > > +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0+
> > > > +
> > > > +/*
> > > > + * Virtio-media driver.
> > > > + *
> > > > + * Copyright (c) 2024-2025 Google LLC.
> > > > + */
> > > > +
> > > > +#include <linux/delay.h>
> > > > +#include <linux/device.h>
> > > > +#include <linux/dev_printk.h>
> > > > +#include <linux/mm.h>
> > > > +#include <linux/mutex.h>
> > > > +#include <linux/scatterlist.h>
> > > > +#include <linux/types.h>
> > > > +#include <linux/videodev2.h>
> > > > +#include <linux/vmalloc.h>
> > > > +#include <linux/wait.h>
> > > > +#include <linux/workqueue.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/moduleparam.h>
> > > > +#include <linux/virtio.h>
> > > > +#include <linux/virtio_config.h>
> > > > +#include <linux/virtio_ids.h>
> > > > +
> > > > +#include <media/frame_vector.h>
> > > > +#include <media/v4l2-dev.h>
> > > > +#include <media/v4l2-event.h>
> > > > +#include <media/videobuf2-memops.h>
> > > > +#include <media/v4l2-device.h>
> > > > +#include <media/v4l2-ioctl.h>
> > > > +
> > > > +#include "protocol.h"
> > > > +#include "session.h"
> > > > +#include "virtio_media.h"
> > > > +
> > > > +#define VIRTIO_MEDIA_NUM_EVENT_BUFS 16
> > > > +
> > > > +/* ID of the SHM region into which MMAP buffer will be mapped. */
> > > > +#define VIRTIO_MEDIA_SHM_MMAP 0
> > > > +
> > > > +/*
> > > > + * Name of the driver to expose to user-space.
> > > > + *
> > > > + * This is configurable because v4l2-compliance has workarounds specific to
> > > > + * some drivers. When proxying these directly from the host, this allows it to
> > > > + * apply them as needed.
> > > > + */
> > > > +char *virtio_media_driver_name;
> > > > +module_param_named(driver_name, virtio_media_driver_name, charp, 0660);
> > >
> > >
> > > Um. What? Not how it should be handled.
> >
> > I can remove this module param. I didn't end up using this when compliance testing.
> > Instead, I patched v4l-utils:
> > https://lore.kernel.org/all/20260528163448.4031965-1-briandaniels@google.com/
> >
> > Let me know if you think the v4l-utils patch is a good approach, otherwise let
> > me know how you'd prefer to address the v4l2-compliance driver-specific workounds
> > when they're being proxied with virtio-media.
>
> This kind of discussion should happen on a separate PR for v4l2-compliance,
> c/c to the proper developers and maintainers of it.
I've removed the driver name module parameter in v5. Let's continue
the compliance testing discussion on the v4l-utils patch:
https://lore.kernel.org/all/20260528163448.4031965-1-briandaniels@google.com/
> >
> > > > +
> > > > +/*
> > > > + * Whether USERPTR buffers are allowed.
> > > > + *
> > > > + * This is disabled by default as USERPTR buffers are dangerous, but the option
> > > > + * is left to enable them if desired.
> > > > + */
> > > > +bool virtio_media_allow_userptr;
> > > > +module_param_named(allow_userptr, virtio_media_allow_userptr, bool, 0660);
> > >
> > >
> > > is this kind of thing common?
>
> There is one old media device that has it (saa7134).
>
> >
> > To be honest, I don't really know. I'm also not that familiar with the USERPTR
> > issues. I see a few references online about their use being discouraged due to
> > possible race conditions, perhaps that was the original motivation for this
> > parameter (I'm not the original author of this driver).
> >
> > I'm open to alternatives, feel free to let me know if you have a preference.
>
> We tend to not implement USERPTR on newer drivers. I suggest you
> to place the logic with regards to V4L2_MEMORY_USERPTR on a separate
> patch for further discussions.
Will do in v5.
> >
> > > > +
> > > > +/**
> > > > + * virtio_media_session_alloc - Allocate a new session.
> > > > + * @vv: virtio-media device the session belongs to.
> > > > + * @id: ID of the session.
> > > > + * @nonblocking_dequeue: whether dequeuing of buffers should be blocking or
> > > > + * not.
> > > > + *
> > > > + * The ``id`` and ``list`` fields must still be set by the caller.
> > >
> > > still in what sense?
> >
> > Based on the code below, I'm not so sure that the caller is responsible for
> > setting these values. They seem to be initialized in the function.
> >
> > Perhaps Alexandre Courbot (the original author) would know more. Unless he
> > says otherwise though I'm inclined to remove this comment.
Removed in v5
> > > > + */
> > > > +static struct virtio_media_session *
> > > > +virtio_media_session_alloc(struct virtio_media *vv, u32 id,
> > > > + struct file *file)
> > > > +{
> > > > + struct virtio_media_session *session;
> > > > + int i;
> > > > + int ret;
> > > > +
> > > > + session = kzalloc_obj(*session, GFP_KERNEL);
> > > > + if (!session)
> > > > + goto err_session;
> > > > +
> > > > + session->shadow_buf = kzalloc(VIRTIO_SHADOW_BUF_SIZE, GFP_KERNEL);
> > > > + if (!session->shadow_buf)
> > > > + goto err_shadow_buf;
> > > > +
> > > > + ret = sg_alloc_table(&session->command_sgs, DESC_CHAIN_MAX_LEN,
> > > > + GFP_KERNEL);
> > > > + if (ret)
> > > > + goto err_payload_sgs;
> > > > +
> > > > + session->id = id;
> > > > + session->nonblocking_dequeue = file->f_flags & O_NONBLOCK;
> > > > +
> > > > + INIT_LIST_HEAD(&session->list);
> > > > + v4l2_fh_init(&session->fh, &vv->video_dev);
> > > > + virtio_media_session_fh_add(session, file);
> > > > +
> > > > + for (i = 0; i <= VIRTIO_MEDIA_LAST_QUEUE; i++)
> > > > + INIT_LIST_HEAD(&session->queues[i].pending_dqbufs);
> > > > + mutex_init(&session->queues_lock);
> > > > +
> > > > + init_waitqueue_head(&session->dqbuf_wait);
> > > > +
> > > > + mutex_lock(&vv->sessions_lock);
> > > > + list_add_tail(&session->list, &vv->sessions);
> > > > + mutex_unlock(&vv->sessions_lock);
> > > > +
> > > > + return session;
> > > > +
> > > > +err_payload_sgs:
> > > > + kfree(session->shadow_buf);
> > > > +err_shadow_buf:
> > > > + kfree(session);
> > > > +err_session:
> > > > + return ERR_PTR(-ENOMEM);
> > > > +}
> > > > +
> > > > +/**
> > > > + * virtio_media_session_free - Free all resources of a session.
> > > > + * @vv: virtio-media device the session belongs to.
> > > > + * @session: session to destroy.
> > > > + *
> > > > + * All the resources of @sesssion, as well as the backing memory of @session
> > > > + * itself, are freed.
> > >
> > > why @ here and `` above? And typo in the name.
> >
> > The `@` here was an attempt to follow the guide here for referencing function
> > parameters:
> > https://docs.kernel.org/doc-guide/kernel-doc.html#highlights-and-cross-references
>
> Yes. This is part of Linux Kernel kernel-doc markup: when referring to
> struct fields, you should use @field (or ``field`` if one wants to place an
> asterisk on it, like ``*field``).
>
> >
> > That being said, I don't believe this file is 100% consistent with that. I will
> > spend some time cleaning up the comments throughout this patch set to get them
> > consistent for v5. Thanks!
>
> Please use it on a consistent way along the driver.
I went through and cleaned up all of the files in v5 so they should
use kernel-doc consistently now.
> Thanks,
> Mauro
^ permalink raw reply
* [PATCH v8 01/11] sched/docs: Document cpu_preferred_mask and Preferred CPU concept
From: Shrikanth Hegde @ 2026-07-20 17:22 UTC (permalink / raw)
To: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet
Cc: sshegde, tglx, gregkh, pbonzini, seanjc, vschneid, huschle,
rostedt, dietmar.eggemann, maddy, srikar, hdanton, chleroy,
vineeth, frederic, arighi, pauld, christian.loehle, tj,
tommaso.cucinotta, maz, rafael, rdunlap, kernellwp, linux-doc,
jgross, virtualization, kernel test robot
In-Reply-To: <20260720172250.2257582-1-sshegde@linux.ibm.com>
Add documentation for new cpumask called cpu_preferred_mask. This could
help users in understanding what this mask is and the concept behind it.
Document how to enable it and implementation aspects of it.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202606180717.yNM0yb41-lkp@intel.com/
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
Documentation/scheduler/sched-arch.rst | 58 ++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/Documentation/scheduler/sched-arch.rst b/Documentation/scheduler/sched-arch.rst
index ed07efea7d02..1ce0f92bc3f2 100644
--- a/Documentation/scheduler/sched-arch.rst
+++ b/Documentation/scheduler/sched-arch.rst
@@ -62,6 +62,64 @@ Your cpu_idle routines need to obey the following rules:
arch/x86/kernel/process.c has examples of both polling and
sleeping idle functions.
+Preferred CPUs
+==============
+
+In virtualised environments it is possible to overcommit CPU resources. i.e.
+the sum of virtual CPUs (vCPUs) of all VMs is greater than number of physical
+CPUs (pCPUs). Under such conditions when all or many VMs have high utilization,
+hypervisor won't be able to satisfy the CPU requirement and has to context
+switch within or across VMs. The hypervisor needs to preempt one vCPU to run
+another. This is called vCPU preemption. This is more expensive compared to
+task context switch within a vCPU.
+
+In such cases it is better that combined vCPU ask from all VMs is reduced
+by not using some of the vCPUs in each VM. vCPUs where workload can be safely
+scheduled which won't increase any contention for pCPU are called as
+"Preferred CPUs".
+
+Main design construct is preferred CPUs are always a subset of active CPUs.
+In most cases preferred CPUs will be same as active CPUs, when there is pCPU
+contention, Preferred CPUs will reduce based on the amount of steal time.
+When the pCPU contention goes away as indicated by steal time, Preferred CPUs
+will become same as active CPUs again. This is done by loading the
+steal_governor driver available at drivers/virt/steal_governor.
+
+For scheduling decisions such as wakeup, pushing the task etc, needs this
+CPU state info. This is maintained in cpu_preferred_mask.
+vCPUs which are not in cpu_preferred_mask should be treated as vCPUs which
+should not be used at this moment provided it doesn't break user affinity.
+
+This is achieved by:
+
+1. Selecting a preferred CPU at wakeup using fallback mechanism.
+2. Push the task away from non-preferred CPU at tick.
+3. Only select preferred CPUs for load balance.
+
+/sys/devices/system/cpu/preferred prints the current cpu_preferred_mask in
+cpulist format.
+
+Notes:
+
+1. This feature is available under CONFIG_PREFERRED_CPU. It is selected
+ by steal_governor driver (CONFIG_STEAL_GOVERNOR). On enabling the
+ driver, CPU preferred state can change based on steal time. Without that
+ driver, preferred CPUs is same as active CPUs.
+
+2. This feature works for FAIR class only.
+
+3. A task pinned, which can't be moved to preferred CPUs will continue
+ to run based on its affinity. But no load balancing happens.
+
+4. Decision to use/not use is driven by kernel. Hence it shouldn't
+ break user affinities. One of the main reasons why CPU hotplug
+ or Isolated cpuset partitions was not a solution.
+
+5. This feature works best only when all the VMs enable the feature as
+ it is a co-operative scheme. If a specific VM doesn't enable this feature
+ it may end up with more CPUs than others, still should lead to better
+ performance when seen from system view.
+ Users who enable this driver must ensure it is enabled in all VMs.
Possible arch/ problems
=======================
--
2.47.3
^ permalink raw reply related
* [PATCH v8 00/11] sched, steal_governor: Introduce cpu_preferred_mask and steal-driven vCPU backoff
From: Shrikanth Hegde @ 2026-07-20 17:22 UTC (permalink / raw)
To: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet
Cc: sshegde, tglx, gregkh, pbonzini, seanjc, vschneid, huschle,
rostedt, dietmar.eggemann, maddy, srikar, hdanton, chleroy,
vineeth, frederic, arighi, pauld, christian.loehle, tj,
tommaso.cucinotta, maz, rafael, rdunlap, kernellwp, linux-doc,
jgross, virtualization
This patch series represents the result of multiple iterations,
redesigns and community feedback. What started as an arch-specific RFC
has evolved into a scheduler mechanism paired with a virtualization
driver.
Special thanks to Yury Norov for the rigorous reviews that greatly
improved the series and to everyone who have provided their review
comments so far. Really appreciated! _/\_
I have put a detailed context around problem statement, design, best
practises and performance numbers below. This cover-letter is a good
starting point for anyone looking into this solution without the pain of
browsing through all the previous patches/videos.
Apologies in advance if any review comments are missed or any
missing implementation for the new driver. If so would be purely
accidental, not in any way intentional.
Background and Problem Statement
================================
As hardware scales, the density of physical CPUs (pCPUs) per server is
increasing across many architectures. On these massive systems, deploying
a single bare-metal OS for general workloads becomes increasingly difficult
to manage if not impossible. The natural shift is to deploy
Virtual Machines(VMs). For example, on IBM PowerPC architecture customers
frequently deploy Shared Processor LPARs (SPLPARs) to maximize hardware ROI.
Typical enterprise workloads are combination of bursty and long running;
their average CPU utilization is low, but they require high core counts
during peak transactions. To accommodate this, customers often
use CPU overcommit strategies i.e. configuring VMs with a large number
of virtual CPUs (vCPUs) while backing them with a smaller, shared pool
of physical CPUs (pCPUs). This achieves a high server consolidation
and excellent cost efficiency.
However, when multiple such VMs have high utilization simultaneously,
the shared pCPU pool becomes contended. The hypervisor is forced to preempt
one vCPU to run another to maintain fairness. It maybe schedule vCPU of same
VM or different VM. If a vCPU is preempted while holding a lock or
irq disabled section, overall forward progress collapses. There are some
mitigation strategies such as yielding the vCPU to lock-holder, but they
don't cover all the cases. In addition there are hidden costs such as cache,
tlb misses, cost of vCPU preemption, host scheduling overheads etc.
Under heavy contention, the most effective mitigation strategy is for
the guests/VMs to voluntarily fold its workload onto a smaller subset
of its vCPUs. By demanding fewer pCPUs, the VMs reduces overall host
contention, which decreases vCPU preemption and improves total throughput
for the system.
Limitations of Existing Approaches
==================================
CPU Hotplug, Isolated cpusets, cpuset:
- This is a heavy and administrative operation that requires topology rebuild.
Crucially, it breaks userspace CPU affinities.
Explicit task affinity:
- Very difficult to manage for the users, if not impossible.
We need a fast, co-operative backoff mechanism inside the kernel that can
dynamically react to contention without violating user/task affinity
contracts. Since reacting to the contention is agnostic to the user
it cannot violate user affinity contracts.
When there is high contention, fold the workload and use limited vCPUs
and when there is no contention, use all the vCPUs again. This natural
expansion/contraction gives the best possible performance to the users
based on the underlying contention.
Proposed Architecture
=====================
Current design is built on basis that contention is effectively
quantified by steal time as seen in guest kernel.
Steal time is already a well established construct today in
para-virtualization world. All major archs support this feature.
It is indication of the contention of physical CPU. It scales according
to the amount of contention. Today it is used by administrative users
for changing the VM configurations. During high contention the steal
time shows up in each guest based on its configuration. The proposed
solution works well when all VMs honor the hint and work in co-operative
manner. Note there is still no inter-guest communication to achieve this
co-operation. Read the section on best practises on how to get the
best out of this solution.
This series introduces a dynamic vCPU backoff mechanism.
It is separated into a core scheduler mechanism and a loadable
virtualization policy module.
Layer A: The Scheduler Mechanism (preferred CPUs)
=================================================
Series introduces a new CPU state called preferred. It indicates that
vCPU can be safely used and using that vCPU won't increase contention
for underlying physical CPUs. This state info is made available via
cpu_preferred_mask, which is strictly maintained as a subset of
cpu_active_mask.
The scheduler uses this mask as a hint to fold workloads onto preferred
CPUs using a few mechanisms.
1, Wakeup: is_cpu_allowed() checks if CPU is preferred. If not calls
select_fallback_rq, which selects a preferred CPU if tasks's affinity
permits.
2. The Tick (Push): During sched_tick(), if the current CPU is non-preferred,
the scheduler actively pushes the running task onto a preferred CPU
using a stopper thread.
3. Load Balance: sched_balance_rq restricts its domain span to
cpu_preferred_mask, preventing tasks from being pulled toward
non-preferred CPUs.
Design Constraint: The scheduler strictly respects user affinities.
If a task is pinned exclusively to non-preferred CPUs, it will remain there.
The kernel will not break user/task affinity contracts.
Layer B: The Policy Engine (virt/steal_governor)
================================================
The core scheduler should not dictate virtualization policy.
Therefore, the policy is isolated into a new driver: steal_governor.
(Can be selected by CONFIG_STEAL_GOVERNOR)
This module latches onto that concept that contention is quantified by
steal time. It periodically samples the steal time values across the
system and depending on high/low steal values, takes appropriate action.
When it sees high steal times, i.e. steal time exceeds high_threshold
(default 5%), driver reduces the preferred CPUs by 1 core.
When it sees Low Steal Times, i.e. steal time drops below low_threshold
(default 2%), driver increases the preferred CPUs by 1 core.
This creates a dynamic, self-maintained stepwise loop. The guest automatically
shrinks its pCPU footprint when the host is saturated, and expands it when
the noise clears while requiring zero cross-VM communication.
Policy Design Constraints:
- Ensure at least one core is kept as preferred.
- Ensure preferred is always subset of active.
Best Practises
==============
1. Ensure all the VM run kernel which has the patches.
2. Keep CONFIG_STEAL_GOVERNOR=m. Build it as module, but don't load it by
default. When the administrative user enables it in one VM, he/she
will likely enable it in all VMs. Also module parameters can
only be changed at module load. Having it as module also allows one
to disable it to remove additional overhead it brings.
3. Keep the interval_ms=500 to 5000. I.e. between 500ms to 5second.
Though parameters allows slightly higher range.
4. Fine tune low and high threshold depending on your platform for best
results. Even where is no contention, very small steal values
might show up. So it might be better to keep low threshold higher
than 0.
Baseline and Revision History
==============================
tip/sched/core at
commit: '04998aa54848 ("sched/eevdf: Delayed dequeue task can't preempt")'
For a detailed talk on the problem and discussion on this issue, one can also
refer to the OSPM26 talk[1].
[1]: https://youtu.be/adxUKFPlOp0
[2]: https://www.ibm.com/support/pages/ibm-power-virtualization-best-practices-guide
[3]: https://www.ibm.com/docs/en/linux-on-systems?topic=bad-daytrader
v7->v8:
- Rename to STEAL_GOVERNOR from STEAL_MONITOR.
- Remove additional defaults.c and move it to core.c (Yury Norov)
- Remove SM_DIR gating for direction control. (Yury Norov)
- Enforce design constraint and restore the state if not met (Yury
Norov)
- Drop nohz_full tick enable patch.
- Move Kconfig patch as the last patch for enablement. (Yury Norov)
- Use disable_delayed_work_sync to avoid race condition during
module unload. (Yury Norov)
- Add same kconfig dependency and fail to compile the driver (Yury Norov)
- Make low < high comparison during module init instead as they
are dependent parameters (Sashiko)
- Update sysfs file helper section (Yury Norov)
- Make preferred sysfs file available only with CONFIG_PREFERRED_CPU=y
(Yury Norov)
- A few documentation and comments fixes. (Randy Dunlap)
- Fix possible race in sched_push_current_non_preferred_cpu (Yury Norov)
- Move is_migration_disabled check just before actual migration.
- Make 100ms as minimal interval_ms from 10ms.
- Make helper functions static and remove from header file as there
are no other callers.
- Collapse helper functions and periodic work into one patch.
Short summary on previous versions:
v6->v7:
- Consolidate new driver code to 4-5 patches.
- deffer the arch specific interface.
- Use possible CPUs instead of active for steal value calculations.
- Simplify is_cpu_allowed.
- Make module parameters fixed at module load
- Define CONFIG_STEAL_MONITOR and Make it select CONFIG_PREFERRED_CPU
v5->v6:
- Drop the optimization of caching the preferred state
in select_fallback_rq
- Drop wakeup patch
v4->v5:
- Move the computation of steal time and decide on preferred CPU state
to a driver. i.e new driver called STEAL_MONITOR
v3->v4:
- Make preferred subset of active instead of online.
- Dropped RT patch and Defer sched_ext. Support only FAIR class.
v2->v3:
- Introduce a new config CONFIG_PREFERRED_CPU
v1->v2:
- A new name - Preferred CPUs and cpu_preferred_mask
- Arch independent code. Everything happens in scheduler.
- Steal time computation is gated with sched feature STEAL_MONITOR
RFC v3-> RFC v4:
- Introduced computation of steal time in arch/powerpc.
RFC PATCH v1:
- push task mechanism.
- No steal time computation. Manual sysfs hint for preferred CPUs
v1: https://lore.kernel.org/all/236f4925-dd3c-41ef-be04-47708c9ce129@linux.ibm.com/
v2: https://lore.kernel.org/all/20260407191950.643549-1-sshegde@linux.ibm.com/#t
v3: https://lore.kernel.org/all/20260514152204.481115-1-sshegde@linux.ibm.com/#r
v4: https://lore.kernel.org/all/20260617174139.155540-1-sshegde@linux.ibm.com/#t
v5: https://lore.kernel.org/all/20260625124648.802832-1-sshegde@linux.ibm.com/
v6: https://lore.kernel.org/all/20260701141654.500125-1-sshegde@linux.ibm.com/#t
v7: https://lore.kernel.org/all/20260709215648.1246821-1-sshegde@linux.ibm.com/
Even earlier version:
https://lore.kernel.org/all/236f4925-dd3c-41ef-be04-47708c9ce129@linux.ibm.com/
========================================
Performance Numbers (powerpc, x86, s390)
========================================
PowerPC:
===================
VM1: 60VP/30EC and VM2: 30VP/20EC
Shared physical CPU pool size: 50 Cores. Each core is SMT8.
(VP - Virtual Core, EC - Entitles Core) - PowerVM terminologies of SPLPAR[2]
Default parameter values: 1000ms, 200 low threshold, 500 high threshold
Both the VMs are running the same workload. Total throughput/time of VM1+VM2
is being mentioned in all cases.
Hackbench
baseline steal_governor steal_governor
disabled enabled
======================================================================
10 groups 5.20 | 5.40 (-3.85%) | 4.65 (+10.58%)
20 groups 11.39 | 12.01 (-5.44%) | 7.09 (+37.75%)
40 groups 20.32 | 19.80 (+2.56%) | 11.31 (+44.34%)
10 groups(-p) 2.37 | 2.26 (+4.64%) | 2.06 (+13.08%)
20 groups(-p) 3.34 | 3.28 (+1.80%) | 3.20 (+4.19%)
40 groups(-p) 4.46 | 4.83 (-8.30%) | 4.26 (+4.48%)
Remarks: Net improvement with steal_governor specially high load points.
schbench ( -L -n 0 -r 30 -s 0)
baseline steal_governor steal_governor
disabled enabled
======================================================================
-m 1 -t 128 2475162 | 2621246 (+5.90%) | 2527299 (+2.11%)
-m 1 -t 256 1467350 | 1470032 (+0.18%) | 1492372 (+1.71%)
-m 1 -t 512 1408813 | 1454687 (+3.26%) | 1437605 (+2.04%)
Remarks: Effectively means no-improvements or regressions
kernbench baseline steal_governor steal_governor
(elapsed time) disabled enabled
======================================================================
-j nr_cpus 231 | 235 (-1.7%) | 199 (+14%)
Remarks: Net improvement in elapsed time.
Daytrader - A real life work which is a proxy for trading based
on db2[3]
baseline steal_governor steal_governor
disabled enabled
======================================================================
Load@30% 1x | 0.96x | 1.53x
Load@60% 1x | 0.94x | 1.41x
Remarks: Good improvement seen at different load points.
When there is no steal time (such as dedicated LPAR, or only VM2
is running) throughput was same with steal_governor enabled/disabled
which indicates minimal overhead of steal_governor.
Data from x86,s390 KVM which Ilya Leoshkevich carried out during OSPM26
time. *This was based on v2*. Idea is still the name, numbers are
expected to be better in v8 as some of the overhead has been removed.
Note: Other variations of the benchmark shows no observable
difference.
x86:
====
cascade-lake: 32 threads = 16 cores
Benchmark #VMs #CPUs/VM ΔRPS (%std)
===============================================
hackbench 8 16 90.73% ± 9.97%
hackbench 4 24 52.67% ± 7.43%
hackbench 4 16 37.96% ± 11.19%
hackbench 4 32 37.82% ± 4.38%
hackbench 12 8 36.90% ± 4.74%
hackbench 8 8 35.30% ± 3.61%
pgbench 16 4 31.77% ± 2.44%
hackbench 2 24 25.85% ± 8.63%
hackbench 16 8 24.87% ± 3.46%
pgbench 16 8 21.83% ± 2.20%
pgbench 12 8 21.35% ± 2.15%
pgbench 8 8 18.46% ± 1.01%
hackbench 2 32 15.56% ± 4.53%
pgbench 12 4 14.28% ± 2.04%
hackbench 16 4 14.07% ± 2.90%
hackbench 12 4 9.60% ± 3.49%
[...]
pgbench 4 8 -1.16% ± 3.60%
hackbench 4 4 -1.80% ± 9.55%
sysbench 12 4 -2.19% ± 0.78%
pgbench 4 24 -2.43% ± 4.38%
pgbench 4 32 -3.21% ± 0.79%
sysbench 16 4 -3.22% ± 1.09%
S390:
=====
z16: 16 threads = 8 cores (SMT-2)
Benchmark #VMs #CPUs/VM ΔRPS (std%)
===============================================
pgbench 2 8 73.50% ± 35.91%
pgbench 16 4 61.30% ± 4.09%
hackbench 16 4 54.11% ± 4.38%
hackbench 12 4 36.34% ± 4.63%
pgbench 12 4 34.83% ± 2.57%
hackbench 8 4 29.75% ± 5.86%
hackbench 8 8 25.98% ± 5.09%
pgbench 2 4 23.31% ± 33.44%
pgbench 2 16 19.95% ± 17.12%
hackbench 4 8 19.43% ± 9.33%
pgbench 8 4 19.32% ± 4.50%
[...]
schbench 8 8 -0.79% ± 0.33%
sysbench 8 8 -0.81% ± 0.39%
hackbench 4 16 -1.11% ± 5.82%
sysbench 8 4 -1.62% ± 0.49%
sysbench 16 4 -2.70% ± 0.58%
schbench 16 4 -2.73% ± 0.91%
sysbench 12 4 -2.91% ± 0.61%
hackbench 2 24 -4.99% ± 3.31%
Summary:
- Many improvement across archs specially with real life workloads.
- No major regressions observed.
- Overhead of steal_governor looks minimal when there is no steal time.
- Overhead when STEAL_GOVERNOR=n is negligible.
Testing and Validation
======================
Apart from performance, To ensure the robustness of the preferred
CPU masking and push mechanisms, the following scenarios were tested:
- CPU Hotplug: bringing CPUs up/down change the preferred mask
accordingly under no-contention and contention.
- Housekeeping cores: Verified with different combinations of
nohz_full=<beginning, middle, end set of CPUs> to ensure that
policy engine restricts to first housekeeping core in extreme cases.
- User Affinity: Confirmed that tasks explicitly pinned to non-preferred
CPUs via taskset remain on their assigned CPUs.
- Affine Move: Confirmed the affinity move using "taskset -cp" happens
on all combinations of non-preferred, non-preferred under contention.
- Affinity and hotplug: It works as expected. I.e affinity gets
reset if all the CPUs of p->cpus_ptr go offline even if they are
non-preferred CPUs.
- Extreme load and running threads: for example 4800 stress-ng threads
on 480 CPU system and it still packs to preferred CPUs.
Known Limitations & Future Work
===============================
To keep this initial implementation clean and minimal, a few optimizations
have been deferred:
- Push all tasks on rq: Currently, the stopper thread only pushes the current
running task off a non-preferred CPU. Future optimizations may look into
migrating all queued tasks on that runqueue.
- Sched Classes: This feature currently only works for the FAIR
class. Real-time (RT) and sched_ext classes are deferred for now,
as there is no need for it.
- Arch specific hints from HW and framework for it as been deferred to
the future.
- NUMA Splicing: The steal_governor currently removes last active core
based on CPU number. It does not yet do complex NUMA-aware splicing,
expecting that CPUs are spread out uniformly across nodes in
most cases.
Shrikanth Hegde (11):
sched/docs: Document cpu_preferred_mask and Preferred CPU concept
cpumask: Introduce cpu_preferred_mask
sysfs: Add preferred CPU file
sched/core: Try to use a preferred CPU in is_cpu_allowed
sched/fair: Load balance only among preferred CPUs
sched/core: Push current task from non preferred CPU
sched/debug: Add migration stats due to non preferred CPUs
virt: Introduce steal governor driver
virt/steal_governor: Add control knobs for handling steal values
virt/steal_governor: Implement steal_governor policy loop
virt/steal_governor: Enable the driver
.../ABI/testing/sysfs-devices-system-cpu | 14 +
Documentation/driver-api/index.rst | 1 +
Documentation/driver-api/steal-governor.rst | 117 ++++++++
Documentation/scheduler/sched-arch.rst | 58 ++++
MAINTAINERS | 9 +
drivers/base/cpu.c | 12 +
drivers/virt/Kconfig | 2 +
drivers/virt/Makefile | 1 +
drivers/virt/steal_governor/Kconfig | 18 ++
drivers/virt/steal_governor/Makefile | 6 +
drivers/virt/steal_governor/core.c | 277 ++++++++++++++++++
drivers/virt/steal_governor/core.h | 30 ++
include/linux/cpumask.h | 24 ++
include/linux/sched.h | 1 +
kernel/Kconfig.preempt | 4 +
kernel/cpu.c | 6 +
kernel/sched/core.c | 100 ++++++-
kernel/sched/debug.c | 1 +
kernel/sched/fair.c | 11 +-
kernel/sched/sched.h | 20 ++
20 files changed, 704 insertions(+), 8 deletions(-)
create mode 100644 Documentation/driver-api/steal-governor.rst
create mode 100644 drivers/virt/steal_governor/Kconfig
create mode 100644 drivers/virt/steal_governor/Makefile
create mode 100644 drivers/virt/steal_governor/core.c
create mode 100644 drivers/virt/steal_governor/core.h
--
2.47.3
^ permalink raw reply
* [PATCH v8 02/11] cpumask: Introduce cpu_preferred_mask
From: Shrikanth Hegde @ 2026-07-20 17:22 UTC (permalink / raw)
To: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet
Cc: sshegde, tglx, gregkh, pbonzini, seanjc, vschneid, huschle,
rostedt, dietmar.eggemann, maddy, srikar, hdanton, chleroy,
vineeth, frederic, arighi, pauld, christian.loehle, tj,
tommaso.cucinotta, maz, rafael, rdunlap, kernellwp, linux-doc,
jgross, virtualization
In-Reply-To: <20260720172250.2257582-1-sshegde@linux.ibm.com>
Provide cpu_preferred_mask infrastructure. Define get/set macros
which could be used to get/set CPU state as preferred.
PREFERRED_CPU config will be selected by the driver which handles
steal time values. It is going to set/clear preferred CPU state.
This driver will be called steal_governor and it is introduced in
subsequent patches. It periodically samples the steal time and
decides on preferred CPU state.
A CPU is set to preferred when it becomes active. Later it may be
marked as non-preferred depending on steal time values with
steal_governor being enabled.
Always maintain design construct of preferred is subset of active.
i.e. preferred ⊆ active ⊆ online ⊆ present ⊆ possible
With PREFERRED_CPU=n, ensure set_cpu_preferred is a nop and get
method returns the active state in that case.
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
include/linux/cpumask.h | 24 ++++++++++++++++++++++++
kernel/Kconfig.preempt | 4 ++++
kernel/cpu.c | 6 ++++++
kernel/sched/core.c | 5 +++++
4 files changed, 39 insertions(+)
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index d3cda0544954..34d08a3d80e1 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -122,12 +122,20 @@ extern struct cpumask __cpu_enabled_mask;
extern struct cpumask __cpu_present_mask;
extern struct cpumask __cpu_active_mask;
extern struct cpumask __cpu_dying_mask;
+
+#ifdef CONFIG_PREFERRED_CPU
+extern struct cpumask __cpu_preferred_mask;
+#else
+#define __cpu_preferred_mask __cpu_active_mask
+#endif
+
#define cpu_possible_mask ((const struct cpumask *)&__cpu_possible_mask)
#define cpu_online_mask ((const struct cpumask *)&__cpu_online_mask)
#define cpu_enabled_mask ((const struct cpumask *)&__cpu_enabled_mask)
#define cpu_present_mask ((const struct cpumask *)&__cpu_present_mask)
#define cpu_active_mask ((const struct cpumask *)&__cpu_active_mask)
#define cpu_dying_mask ((const struct cpumask *)&__cpu_dying_mask)
+#define cpu_preferred_mask ((const struct cpumask *)&__cpu_preferred_mask)
extern atomic_t __num_online_cpus;
extern unsigned int __num_possible_cpus;
@@ -1164,6 +1172,12 @@ void init_cpu_possible(const struct cpumask *src);
#define set_cpu_active(cpu, active) assign_cpu((cpu), &__cpu_active_mask, (active))
#define set_cpu_dying(cpu, dying) assign_cpu((cpu), &__cpu_dying_mask, (dying))
+#ifdef CONFIG_PREFERRED_CPU
+#define set_cpu_preferred(cpu, preferred) assign_cpu((cpu), &__cpu_preferred_mask, (preferred))
+#else
+#define set_cpu_preferred(cpu, preferred) do { } while (0)
+#endif
+
void set_cpu_online(unsigned int cpu, bool online);
void set_cpu_possible(unsigned int cpu, bool possible);
@@ -1258,6 +1272,11 @@ static __always_inline bool cpu_dying(unsigned int cpu)
return cpumask_test_cpu(cpu, cpu_dying_mask);
}
+static __always_inline bool cpu_preferred(unsigned int cpu)
+{
+ return cpumask_test_cpu(cpu, cpu_preferred_mask);
+}
+
#else
#define num_online_cpus() 1U
@@ -1296,6 +1315,11 @@ static __always_inline bool cpu_dying(unsigned int cpu)
return false;
}
+static __always_inline bool cpu_preferred(unsigned int cpu)
+{
+ return cpu == 0;
+}
+
#endif /* NR_CPUS > 1 */
#define cpu_is_offline(cpu) unlikely(!cpu_online(cpu))
diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt
index 88c594c6d7fc..de789b274ba3 100644
--- a/kernel/Kconfig.preempt
+++ b/kernel/Kconfig.preempt
@@ -192,3 +192,7 @@ config SCHED_CLASS_EXT
For more information:
Documentation/scheduler/sched-ext.rst
https://github.com/sched-ext/scx
+
+config PREFERRED_CPU
+ bool
+ depends on SMP && PARAVIRT
diff --git a/kernel/cpu.c b/kernel/cpu.c
index b3c8553d7bd6..376d297a6292 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -3103,6 +3103,11 @@ EXPORT_SYMBOL(__cpu_dying_mask);
atomic_t __num_online_cpus __read_mostly;
EXPORT_SYMBOL(__num_online_cpus);
+#ifdef CONFIG_PREFERRED_CPU
+struct cpumask __cpu_preferred_mask __read_mostly;
+EXPORT_SYMBOL_GPL(__cpu_preferred_mask);
+#endif
+
void init_cpu_present(const struct cpumask *src)
{
cpumask_copy(&__cpu_present_mask, src);
@@ -3160,6 +3165,7 @@ void __init boot_cpu_init(void)
/* Mark the boot cpu "present", "online" etc for SMP and UP case */
set_cpu_online(cpu, true);
set_cpu_active(cpu, true);
+ set_cpu_preferred(cpu, true);
set_cpu_present(cpu, true);
set_cpu_possible(cpu, true);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 2e7cde033a31..a45f7c308329 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8690,6 +8690,9 @@ int sched_cpu_activate(unsigned int cpu)
*/
sched_set_rq_online(rq, cpu);
+ /* preferred is subset of active and follows its state */
+ set_cpu_preferred(cpu, true);
+
return 0;
}
@@ -8703,6 +8706,8 @@ int sched_cpu_deactivate(unsigned int cpu)
if (ret)
return ret;
+ set_cpu_preferred(cpu, false);
+
/*
* Remove CPU from nohz.idle_cpus_mask to prevent participating in
* load balancing when not active
--
2.47.3
^ permalink raw reply related
* [PATCH v8 03/11] sysfs: Add preferred CPU file
From: Shrikanth Hegde @ 2026-07-20 17:22 UTC (permalink / raw)
To: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet
Cc: sshegde, tglx, gregkh, pbonzini, seanjc, vschneid, huschle,
rostedt, dietmar.eggemann, maddy, srikar, hdanton, chleroy,
vineeth, frederic, arighi, pauld, christian.loehle, tj,
tommaso.cucinotta, maz, rafael, rdunlap, kernellwp, linux-doc,
jgross, virtualization
In-Reply-To: <20260720172250.2257582-1-sshegde@linux.ibm.com>
Add "preferred" file in /sys/devices/system/cpu
This would help
- Users to quickly check which CPUs are marked as preferred.
- Userspace daemons such as irqbalance to use this mask to
send irq into preferred CPUs.
For example:
cat /sys/devices/system/cpu/online
0-719
cat /sys/devices/system/cpu/preferred
0-599 <<< Implies 0-599 are preferred for workloads and 600-719
should be avoided at this moment.
cat /sys/devices/system/cpu/preferred
0-719 <<< All CPUs are usable. There is no preference.
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
Documentation/ABI/testing/sysfs-devices-system-cpu | 14 ++++++++++++++
drivers/base/cpu.c | 12 ++++++++++++
2 files changed, 26 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
index 82d10d556cc8..676412c46b2a 100644
--- a/Documentation/ABI/testing/sysfs-devices-system-cpu
+++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
@@ -806,3 +806,17 @@ Date: Nov 2022
Contact: Linux kernel mailing list <linux-kernel@vger.kernel.org>
Description:
(RO) the list of CPUs that can be brought online.
+
+What: /sys/devices/system/cpu/preferred
+Date: July 2026
+Contact: Linux kernel mailing list <linux-kernel@vger.kernel.org>
+Description:
+ (RO) the list of preferred CPUs applicable in
+ paravirtualized environments.
+
+ The steal governor driver dynamically adjusts this mask
+ based on observed steal time. Scheduling tasks on
+ CPUs outside of this list may lead to performance
+ degradations due to underlying physical CPU contention.
+
+ See Documentation/scheduler/sched-arch.rst for more details.
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 19d288a3c80c..5da2a96fb37e 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -391,6 +391,15 @@ static int cpu_uevent(const struct device *dev, struct kobj_uevent_env *env)
}
#endif
+#ifdef CONFIG_PREFERRED_CPU
+static ssize_t preferred_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%*pbl\n", cpumask_pr_args(cpu_preferred_mask));
+}
+static DEVICE_ATTR_RO(preferred);
+#endif
+
const struct bus_type cpu_subsys = {
.name = "cpu",
.dev_name = "cpu",
@@ -531,6 +540,9 @@ static struct attribute *cpu_root_attrs[] = {
#endif
#ifdef CONFIG_GENERIC_CPU_AUTOPROBE
&dev_attr_modalias.attr,
+#endif
+#ifdef CONFIG_PREFERRED_CPU
+ &dev_attr_preferred.attr,
#endif
NULL
};
--
2.47.3
^ permalink raw reply related
* [PATCH v8 04/11] sched/core: Try to use a preferred CPU in is_cpu_allowed
From: Shrikanth Hegde @ 2026-07-20 17:22 UTC (permalink / raw)
To: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet
Cc: sshegde, tglx, gregkh, pbonzini, seanjc, vschneid, huschle,
rostedt, dietmar.eggemann, maddy, srikar, hdanton, chleroy,
vineeth, frederic, arighi, pauld, christian.loehle, tj,
tommaso.cucinotta, maz, rafael, rdunlap, kernellwp, linux-doc,
jgross, virtualization
In-Reply-To: <20260720172250.2257582-1-sshegde@linux.ibm.com>
When possible, choose a preferred CPUs to pick.
This is essential to maintain user affinities when preferred
CPUs change. A task pinned on non-preferred CPU should continue
to run there, since this is non-user triggered.
If CPU is non-preferred and task can run on other CPUs which are
currently preferred, then choose those other CPUs instead.
This is decided by checking cpus_ptr and cpu_preferred_mask
intersect or not. If yes, task has other preferred CPUs, it can
run there instead.
Overhead is minimal when CPU is preferred.
Push task mechanism uses stopper thread which going to call
select_fallback_rq and use this mechanism to pick only a preferred CPU.
This takes care of wakeup path for FAIR tasks too.
is_cpu_allowed is called to ensure wakeup happens on preferred CPUs.
With that, additional checks in available_idle_cpu is not necessary.
For majority of the cases this would still keep select_fallback_rq
as O(N). task_has_preferred_cpus which is O(N) is called only if
!cpu_preferred. Then task running there is expected to move out.
So subsequent it should run on preferred CPU. This becomes O(N**2)
only for tasks pinned only non preferred CPUs. That is rare case.
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
kernel/sched/core.c | 12 ++++++++++--
kernel/sched/sched.h | 12 ++++++++++++
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index a45f7c308329..9e8eec4451b6 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2509,8 +2509,12 @@ static inline bool is_cpu_allowed(struct task_struct *p, int cpu)
return cpu_online(cpu);
/* Non kernel threads are not allowed during either online or offline. */
- if (!(p->flags & PF_KTHREAD))
+ if (!(p->flags & PF_KTHREAD)) {
+ /* Try to use preferred CPU if task's affinity allows */
+ if (task_can_sched_on_preferred(cpu, p))
+ return false;
return cpu_active(cpu);
+ }
/* KTHREAD_IS_PER_CPU is always allowed. */
if (kthread_is_per_cpu(p))
@@ -2520,7 +2524,11 @@ static inline bool is_cpu_allowed(struct task_struct *p, int cpu)
if (cpu_dying(cpu))
return false;
- /* But are allowed during online. */
+ /* Try to keep unbound kthreads on a preferred CPU if possible. */
+ if (task_can_sched_on_preferred(cpu, p))
+ return false;
+
+ /* Otherwise, they are allowed to run on online CPU. */
return cpu_online(cpu);
}
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 26ae13c86b69..6de6366f2faa 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -4230,4 +4230,16 @@ DEFINE_CLASS_IS_UNCONDITIONAL(sched_change)
#include "ext/ext.h"
+static inline bool task_can_sched_on_preferred(int cpu, struct task_struct *p)
+{
+ if (cpu_preferred(cpu))
+ return false;
+
+ /* Only FAIR tasks honor preferred CPU state */
+ if (unlikely(p->sched_class != &fair_sched_class))
+ return false;
+
+ return cpumask_intersects(p->cpus_ptr, cpu_preferred_mask);
+}
+
#endif /* _KERNEL_SCHED_SCHED_H */
--
2.47.3
^ permalink raw reply related
* [PATCH v8 05/11] sched/fair: Load balance only among preferred CPUs
From: Shrikanth Hegde @ 2026-07-20 17:22 UTC (permalink / raw)
To: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet
Cc: sshegde, tglx, gregkh, pbonzini, seanjc, vschneid, huschle,
rostedt, dietmar.eggemann, maddy, srikar, hdanton, chleroy,
vineeth, frederic, arighi, pauld, christian.loehle, tj,
tommaso.cucinotta, maz, rafael, rdunlap, kernellwp, linux-doc,
jgross, virtualization
In-Reply-To: <20260720172250.2257582-1-sshegde@linux.ibm.com>
When cpu is marked as non preferred, any load pulled towards it is
pointless since in the next tick task will be pushed out again.
So, Consider only preferred CPUs for load balance.
This makes it not fight against the push task mechanism which happens
at tick. Also, this stops active balance to happen on non-preferred CPU
pulling the load.
This means there is no load balancing if the task is pinned only to
non-preferred CPUs. They will continue to run where they were previously
running before the CPUs was marked as non-preferred.
Bailout early for NEWIDLE and IDLE balance as load balancing is done
only on preferred CPUs.
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
kernel/sched/fair.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index df8c9c2c7918..12f5b7de28d2 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -13399,7 +13399,7 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
};
bool need_unlock = false;
- cpumask_and(cpus, sched_domain_span(sd), cpu_active_mask);
+ cpumask_and(cpus, sched_domain_span(sd), cpu_preferred_mask);
schedstat_inc(sd->lb_count[idle]);
@@ -14337,7 +14337,8 @@ static void _nohz_idle_balance(struct rq *this_rq, unsigned int flags)
update_rq_clock(rq);
rq_unlock_irqrestore(rq, &rf);
- if (flags & NOHZ_BALANCE_KICK)
+ if (flags & NOHZ_BALANCE_KICK &&
+ cpu_preferred(balance_cpu))
sched_balance_domains(rq, CPU_IDLE);
}
@@ -14481,10 +14482,8 @@ static int sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf)
*/
this_rq->idle_stamp = rq_clock(this_rq);
- /*
- * Do not pull tasks towards !active CPUs...
- */
- if (!cpu_active(this_cpu))
+ /* Do not pull tasks towards !preferred CPUs */
+ if (!cpu_preferred(this_cpu))
return 0;
/*
--
2.47.3
^ permalink raw reply related
* [PATCH v8 06/11] sched/core: Push current task from non preferred CPU
From: Shrikanth Hegde @ 2026-07-20 17:22 UTC (permalink / raw)
To: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet
Cc: sshegde, tglx, gregkh, pbonzini, seanjc, vschneid, huschle,
rostedt, dietmar.eggemann, maddy, srikar, hdanton, chleroy,
vineeth, frederic, arighi, pauld, christian.loehle, tj,
tommaso.cucinotta, maz, rafael, rdunlap, kernellwp, linux-doc,
jgross, virtualization
In-Reply-To: <20260720172250.2257582-1-sshegde@linux.ibm.com>
Actively push out task running on a non-preferred CPU. Since the task is
running on the CPU, need to stop the cpu and push the task out.
However, if the task is pinned only to non-preferred CPUs, it will continue
running there. This will help in maintaining the userspace affinities
unlike CPU hotplug or isolated cpusets.
Though code is similar to __balance_push_cpu_stop and quite close to
push_cpu_stop, it is being kept separate as it provides a cleaner
implementation with CONFIG_PREFERRED_CPU.
Add push_task_work_done flag to protect work buffer.
Works only with FAIR class.
For now, only current running task is pushed out. This keeps the code
simpler. In future optimization maybe done to move all the queued
task on the rq.
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
kernel/sched/core.c | 78 ++++++++++++++++++++++++++++++++++++++++++++
kernel/sched/sched.h | 8 +++++
2 files changed, 86 insertions(+)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9e8eec4451b6..704043531b24 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5774,6 +5774,9 @@ void sched_tick(void)
unsigned long hw_pressure;
u64 resched_latency;
+ if (!cpu_preferred(cpu))
+ sched_push_current_non_preferred_cpu(rq);
+
if (housekeeping_cpu(cpu, HK_TYPE_KERNEL_NOISE))
arch_scale_freq_tick();
@@ -11292,3 +11295,78 @@ void sched_change_end(struct sched_change_ctx *ctx)
p->sched_class->prio_changed(rq, p, ctx->prio);
}
}
+
+#ifdef CONFIG_PREFERRED_CPU
+static DEFINE_PER_CPU(struct cpu_stop_work, npc_push_task_work);
+
+static int sched_non_preferred_cpu_push_stop(void *arg)
+{
+ struct task_struct *p = arg;
+ struct rq *rq = this_rq();
+ struct rq_flags rf;
+ int cpu;
+
+ if (cpu_preferred(rq->cpu)) {
+ scoped_guard(rq_lock, rq)
+ rq->push_task_work_done = false;
+ put_task_struct(p);
+ return 0;
+ }
+
+ raw_spin_lock_irq(&p->pi_lock);
+
+ /* This could take rq lock. So call it before rq lock is taken */
+ cpu = select_fallback_rq(rq->cpu, p);
+ rq_lock(rq, &rf);
+ rq->push_task_work_done = false;
+ update_rq_clock(rq);
+
+ context_unsafe_alias(rq);
+
+ if (task_rq(p) == rq && task_on_rq_queued(p) &&
+ !is_migration_disabled(p))
+ rq = __migrate_task(rq, &rf, p, cpu);
+
+ rq_unlock(rq, &rf);
+ raw_spin_unlock_irq(&p->pi_lock);
+ put_task_struct(p);
+
+ return 0;
+}
+
+/*
+ * Push the current task running on non-preferred CPU(npc).
+ * Using this non preferred CPU will lead to more contention
+ * in the host. So it is better not to use this CPU.
+ *
+ * Since task is running, call a stopper to push the task out. This is
+ * similar to how task moves during hotplug. In select_fallback_rq a
+ * preferred CPU will be chosen and henceforth task shouldn't come back to
+ * this CPU again.
+ *
+ * Works for FAIR class only.
+ *
+ * If task is affined only non-preferred CPUs, no point in moving it out.
+ */
+void sched_push_current_non_preferred_cpu(struct rq *rq)
+{
+ struct task_struct *push_task = rq->curr;
+
+ scoped_guard(rq_lock, rq) {
+ /* Push the task if its explicit affinity allows */
+ if (!task_can_sched_on_preferred(rq->cpu, push_task))
+ return;
+
+ /* There is already a stopper thread. Don't race with it. */
+ if (rq->push_task_work_done)
+ return;
+
+ rq->push_task_work_done = true;
+ }
+
+ /* sched_tick runs with interrupts disabled. */
+ get_task_struct(push_task);
+ stop_one_cpu_nowait(rq->cpu, sched_non_preferred_cpu_push_stop,
+ push_task, this_cpu_ptr(&npc_push_task_work));
+}
+#endif
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 6de6366f2faa..80c02e2c09eb 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1277,6 +1277,8 @@ struct rq {
struct list_head cfs_tasks;
+ bool push_task_work_done;
+
struct sched_avg avg_rt;
struct sched_avg avg_dl;
#ifdef CONFIG_HAVE_SCHED_AVG_IRQ
@@ -4242,4 +4244,10 @@ static inline bool task_can_sched_on_preferred(int cpu, struct task_struct *p)
return cpumask_intersects(p->cpus_ptr, cpu_preferred_mask);
}
+#ifdef CONFIG_PREFERRED_CPU
+void sched_push_current_non_preferred_cpu(struct rq *rq);
+#else /* !CONFIG_PREFERRED_CPU */
+static inline void sched_push_current_non_preferred_cpu(struct rq *rq) { }
+#endif
+
#endif /* _KERNEL_SCHED_SCHED_H */
--
2.47.3
^ permalink raw reply related
* [PATCH v8 07/11] sched/debug: Add migration stats due to non preferred CPUs
From: Shrikanth Hegde @ 2026-07-20 17:22 UTC (permalink / raw)
To: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet
Cc: sshegde, tglx, gregkh, pbonzini, seanjc, vschneid, huschle,
rostedt, dietmar.eggemann, maddy, srikar, hdanton, chleroy,
vineeth, frederic, arighi, pauld, christian.loehle, tj,
tommaso.cucinotta, maz, rafael, rdunlap, kernellwp, linux-doc,
jgross, virtualization
In-Reply-To: <20260720172250.2257582-1-sshegde@linux.ibm.com>
Add a new stat,
- nr_migrations_cpu_non_preferred: number of migrations happened since
a CPU was marked as non preferred due to high steal time.
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
include/linux/sched.h | 1 +
kernel/sched/core.c | 9 +++++++--
kernel/sched/debug.c | 1 +
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 968b18a7f470..37849d2f1dbd 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -554,6 +554,7 @@ struct sched_statistics {
u64 nr_failed_migrations_running;
u64 nr_failed_migrations_hot;
u64 nr_forced_migrations;
+ u64 nr_migrations_cpu_non_preferred;
u64 nr_wakeups;
u64 nr_wakeups_sync;
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 704043531b24..f0d9bcfd35e3 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -11324,8 +11324,13 @@ static int sched_non_preferred_cpu_push_stop(void *arg)
context_unsafe_alias(rq);
if (task_rq(p) == rq && task_on_rq_queued(p) &&
- !is_migration_disabled(p))
- rq = __migrate_task(rq, &rf, p, cpu);
+ !is_migration_disabled(p)) {
+ struct rq *dest_rq = __migrate_task(rq, &rf, p, cpu);
+
+ if (rq != dest_rq)
+ schedstat_inc(p->stats.nr_migrations_cpu_non_preferred);
+ rq = dest_rq;
+ }
rq_unlock(rq, &rf);
raw_spin_unlock_irq(&p->pi_lock);
diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 72236db67983..5ebb2055e6d5 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -1446,6 +1446,7 @@ void proc_sched_show_task(struct task_struct *p, struct pid_namespace *ns,
P_SCHEDSTAT(nr_failed_migrations_running);
P_SCHEDSTAT(nr_failed_migrations_hot);
P_SCHEDSTAT(nr_forced_migrations);
+ P_SCHEDSTAT(nr_migrations_cpu_non_preferred);
P_SCHEDSTAT(nr_wakeups);
P_SCHEDSTAT(nr_wakeups_sync);
P_SCHEDSTAT(nr_wakeups_migrate);
--
2.47.3
^ permalink raw reply related
* [PATCH v8 08/11] virt: Introduce steal governor driver
From: Shrikanth Hegde @ 2026-07-20 17:22 UTC (permalink / raw)
To: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet
Cc: sshegde, tglx, gregkh, pbonzini, seanjc, vschneid, huschle,
rostedt, dietmar.eggemann, maddy, srikar, hdanton, chleroy,
vineeth, frederic, arighi, pauld, christian.loehle, tj,
tommaso.cucinotta, maz, rafael, rdunlap, kernellwp, linux-doc,
jgross, virtualization
In-Reply-To: <20260720172250.2257582-1-sshegde@linux.ibm.com>
Introduce a new driver in virt named steal_governor. This driver
will compute the steal time and drive the policy decisions of preferred
CPU state.
More on it can be found in the Documentation/driver-api/steal-governor.rst
There is a new kconfig called STEAL_GOVERNOR which is introduced in
subsequent patches. That driver is going to select PREFERRED_CPU.
This makes configs driven by user preference.
When the driver is disabled, preferred CPUs is same as active CPUs.
File layout of the driver is being kept simple.
- core.c - contains main driver code. This includes the periodic
work function and take action on steal time which is introduced
in subsequent patches.
- core.h - header file which includes data structure.
Main structure of steal governor has,
- work: deferred periodic work function
- steal, time: To calculate the delta in periodic work.
- interval_ms, high_threshold, low_threshold: debug knobs of
steal_governor.
While there, Add MAINTAINERS entry for this new driver.
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
Documentation/driver-api/index.rst | 1 +
Documentation/driver-api/steal-governor.rst | 117 ++++++++++++++++++++
MAINTAINERS | 9 ++
drivers/virt/steal_governor/core.c | 48 ++++++++
drivers/virt/steal_governor/core.h | 25 +++++
5 files changed, 200 insertions(+)
create mode 100644 Documentation/driver-api/steal-governor.rst
create mode 100644 drivers/virt/steal_governor/core.c
create mode 100644 drivers/virt/steal_governor/core.h
diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
index eaf7161ff957..0a973b59cba3 100644
--- a/Documentation/driver-api/index.rst
+++ b/Documentation/driver-api/index.rst
@@ -138,6 +138,7 @@ Subsystem-specific APIs
sm501
soundwire/index
spi
+ steal-governor
surface_aggregator/index
switchtec
sync_file
diff --git a/Documentation/driver-api/steal-governor.rst b/Documentation/driver-api/steal-governor.rst
new file mode 100644
index 000000000000..1039d598fc2c
--- /dev/null
+++ b/Documentation/driver-api/steal-governor.rst
@@ -0,0 +1,117 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==============
+Steal Governor
+==============
+
+:Author: Shrikanth Hegde <sshegde@linux.ibm.com>
+
+Introduction
+============
+
+Steal governor is a driver aimed at solving the Noisy Neighbour problem
+in paravirtualized environments. The performance of workload
+running in one VM gets affected significantly due to other VMs and
+combined they make slower forward progress.
+
+When there is overcommit of CPU resources, i.e. sum of virtual CPUs (vCPUs)
+of all VMs is greater than number of physical CPUs (pCPUs) and
+when all or many VMs have high utilization, hypervisor won't be able
+to satisfy the CPU requirement and has to context switch within or
+across VMs. I.e. the hypervisor needs to preempt one vCPU to run
+another. This is called vCPU preemption.
+This is more expensive compared to task context switch within a vCPU.
+
+In such cases it is better that combined vCPU ask from all VMs is reduced
+by not using some of the vCPUs. vCPUs where workload can be safely
+scheduled which won't increase any contention for pCPU are called as
+"Preferred CPUs".
+
+See more on "Preferred CPUs" in Documentation/scheduler/sched-arch.rst.
+
+This driver makes CONFIG_PREFERRED_CPU=y which enables the scheduler core
+infrastructure to move tasks to Preferred CPUs where possible.
+
+Core idea
+=========
+
+steal time is an indication available today in Guest which shows contention
+for underlying physical CPU. Use it as a hint in the guest to fold the
+workload to a reduced set of vCPUs. When there is contention, steal time
+will show up in all the guests. When each guest honors the hint and folds
+the workload to a smaller set of vCPUs (Preferred CPUs), it reduces the
+contention and thereby reduces vCPU preemption.
+This is achieved without any cross-guest communication.
+
+Steal governor driver effectively does:
+
+1. Periodically computes steal time across the system.
+
+2. If steal time is greater than high threshold, reduce the number of
+ preferred CPUs by 1 core. Ensure at least one core is left always.
+
+3. If steal time is lower or equal to low threshold, increase the
+ number of preferred CPUs by 1 core. If preferred is same as active,
+ nothing to be done.
+
+4. Ensure preferred CPUs is always subset of active CPUs.
+ On feature disable it is same as active CPUs.
+
+This feature works best only when all the VMs enable the feature as
+it is a co-operative scheme. If a specific VM doesn't enable this feature
+it may end up with more CPUs than others, still should lead to better
+performance when seen from system view.
+Those who enable this driver must ensure it is enabled in all VMs.
+
+Module Parameters
+=================
+
+interval_ms
+-----------
+
+How often steal governor checks for steal time.
+Default: 1000 i.e 1 second. Value should be in between 100ms to 100sec.
+
+This controls how fast steal governor driver reacts to changes to
+the contention of physical CPUs. Since it does a fair amount of
+work, setting too low may have overhead. Setting it too
+high might render it ineffective.
+
+low_threshold
+-------------
+
+lower threshold value in percentage * 100.
+Default: 200, i.e 2% steal is considered as low threshold.
+Can't be higher than high_threshold.
+
+This determines what values should be considered as nil/no steal values.
+When steal governor see steal time is below or equal to this value, it
+will increase the preferred CPUs by 1 core. Having value as zero
+might cause oscillations.
+
+high_threshold
+--------------
+
+higher threshold value in percentage * 100
+Default: 500, i.e 5% steal is considered as high threshold.
+Can't be lower than low_threshold. Must be less than 10000.
+
+This determines what values should be considered as high steal values.
+When steal governor sees steal time is higher than this value, it will
+reduce the preferred CPUs by 1 core.
+
+Notes
+=====
+
+Selecting this driver makes CONFIG_PREFERRED_CPU=y. That makes configs
+driven by user preference.
+
+It is recommended to build CONFIG_STEAL_GOVERNOR=m due to below reasons:
+
+1. Doing periodic work has additional overheads. Enabling this driver
+ in systems where steal time cannot happen is of no use. There is no
+ benefit with additional overheads in such systems.
+
+2. This works well when all VMs work in co-operative manner. When an
+ administrative user enables it in one VM, he/she will likely enable
+ it all VMs.
diff --git a/MAINTAINERS b/MAINTAINERS
index 15011f5752a9..0906684b2243 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25914,6 +25914,15 @@ F: rust/helpers/jump_label.c
F: rust/kernel/generated_arch_static_branch_asm.rs.S
F: rust/kernel/jump_label.rs
+STEAL GOVERNOR DRIVER
+M: Shrikanth Hegde <sshegde@linux.ibm.com>
+R: Yury Norov <yury.norov@gmail.com>
+L: linux-kernel@vger.kernel.org
+S: Maintained
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git sched/core
+F: Documentation/driver-api/steal-governor.rst
+F: drivers/virt/steal_governor/
+
STI AUDIO (ASoC) DRIVERS
M: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
L: linux-sound@vger.kernel.org
diff --git a/drivers/virt/steal_governor/core.c b/drivers/virt/steal_governor/core.c
new file mode 100644
index 000000000000..055f155d2045
--- /dev/null
+++ b/drivers/virt/steal_governor/core.c
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Steal time governor driver periodically computes steal time.
+ * Based on the thresholds it either reduce/increase the preferred
+ * CPUs which can be used by the workload to avoid vCPU preemption
+ * to an extent possible in paravirtualized environment.
+ *
+ * Available as module with CONFIG_STEAL_GOVERNOR
+ *
+ * Copyright (C) 2026 IBM
+ * Author: Shrikanth Hegde <sshegde@linux.ibm.com>
+ */
+
+#include "core.h"
+
+#if !IS_ENABLED(CONFIG_PREFERRED_CPU)
+#error "Steal Governor requires CONFIG_PREFERRED_CPU"
+#endif
+
+static struct steal_governor sg_core_ctx;
+
+static void restore_preferred_to_active(void)
+{
+ int cpu;
+
+ guard(cpus_read_lock)();
+ for_each_cpu(cpu, cpu_active_mask)
+ set_cpu_preferred(cpu, true);
+}
+
+static int __init steal_governor_init(void)
+{
+ pr_info("steal_governor is enabled\n");
+ return 0;
+}
+
+static void __exit steal_governor_exit(void)
+{
+ restore_preferred_to_active();
+ pr_info("steal_governor is disabled\n");
+}
+
+module_init(steal_governor_init);
+module_exit(steal_governor_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("IBM Corporation");
+MODULE_DESCRIPTION("Virtualization Steal Time Governor");
diff --git a/drivers/virt/steal_governor/core.h b/drivers/virt/steal_governor/core.h
new file mode 100644
index 000000000000..e27305284ac0
--- /dev/null
+++ b/drivers/virt/steal_governor/core.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __VIRT_STEAL_CORE_H
+#define __VIRT_STEAL_CORE_H
+
+#include <linux/types.h>
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/cpuhplock.h>
+#include <linux/cpumask.h>
+#include <linux/workqueue.h>
+#include <linux/ktime.h>
+#include <linux/kconfig.h>
+
+struct steal_governor {
+ struct delayed_work work;
+ ktime_t time;
+ u64 steal;
+ unsigned int interval_ms;
+ unsigned int high_threshold;
+ unsigned int low_threshold;
+};
+
+#endif /* __VIRT_STEAL_CORE_H */
--
2.47.3
^ permalink raw reply related
* [PATCH v8 09/11] virt/steal_governor: Add control knobs for handling steal values
From: Shrikanth Hegde @ 2026-07-20 17:22 UTC (permalink / raw)
To: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet
Cc: sshegde, tglx, gregkh, pbonzini, seanjc, vschneid, huschle,
rostedt, dietmar.eggemann, maddy, srikar, hdanton, chleroy,
vineeth, frederic, arighi, pauld, christian.loehle, tj,
tommaso.cucinotta, maz, rafael, rdunlap, kernellwp, linux-doc,
jgross, virtualization
In-Reply-To: <20260720172250.2257582-1-sshegde@linux.ibm.com>
These are the knobs to control the steal_governor.
interval_ms:
How often steal governor checks for steal time.
(Default: 1000 i.e 1 second)
This controls how fast steal governor driver reacts to changes to
the contention of physical CPUs.
Can be set between 100 to 100000. i.e. 100ms to 100seconds.
100ms is kept as minimum to ensure few meaningful steal values
accumulate even with HZ=100.
low_threshold:
lower threshold value in percentage * 100.
(Default: 200, i.e 2% steal is considered as low threshold)
This determines what values should be considered as nil/no steal values.
When steal governor see steal time is below or equal to this value, it
will increase the preferred CPUs by 1 core. Having value as zero
might cause oscillations
high_threshold:
higher threshold value in percentage * 100
(Default: 500, i.e 5% steal is considered as high threshold)
This determines what values should be considered as high steal values.
When steal governor sees steal time is higher than this value, it will
reduce the preferred CPUs by 1 core.
module_param_cb methods are used to do the validation checks.
This helps to ensure one configures sane values.
Since low and high are dependent, that check is done at module init.
Parameters values can't be changed at runtime. One has to unload
the module and change it. Hence recommended to build it as module.
Also available at: Documentation/driver-api/steal-governor.rst
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
drivers/virt/steal_governor/core.c | 73 +++++++++++++++++++++++++++++-
1 file changed, 71 insertions(+), 2 deletions(-)
diff --git a/drivers/virt/steal_governor/core.c b/drivers/virt/steal_governor/core.c
index 055f155d2045..1cb766a8ce28 100644
--- a/drivers/virt/steal_governor/core.c
+++ b/drivers/virt/steal_governor/core.c
@@ -17,7 +17,11 @@
#error "Steal Governor requires CONFIG_PREFERRED_CPU"
#endif
-static struct steal_governor sg_core_ctx;
+static struct steal_governor sg_core_ctx = {
+ .interval_ms = 1000, /* 1 second */
+ .high_threshold = 500, /* 5% */
+ .low_threshold = 200, /* 2% */
+};
static void restore_preferred_to_active(void)
{
@@ -28,9 +32,74 @@ static void restore_preferred_to_active(void)
set_cpu_preferred(cpu, true);
}
+static int param_set_interval_ms(const char *val, const struct kernel_param *kp)
+{
+ unsigned int interval;
+ int ret;
+
+ ret = kstrtouint(val, 0, &interval);
+ if (ret)
+ return ret;
+
+ if (interval < 100 || interval > 100000) {
+ pr_err("steal_governor: interval_ms must be between 100 and 100000\n");
+ return -EINVAL;
+ }
+
+ return param_set_uint(val, kp);
+}
+
+static const struct kernel_param_ops interval_ms_ops = {
+ .set = param_set_interval_ms,
+ .get = param_get_uint,
+};
+
+module_param_cb(interval_ms, &interval_ms_ops, &sg_core_ctx.interval_ms, 0444);
+MODULE_PARM_DESC(interval_ms,
+ "Sampling frequency in milliseconds. default: 1000");
+
+static int param_set_high_threshold(const char *val, const struct kernel_param *kp)
+{
+ unsigned int threshold;
+ int ret;
+
+ ret = kstrtouint(val, 0, &threshold);
+ if (ret)
+ return ret;
+
+ if (threshold >= 100 * 100) {
+ pr_err("steal_governor: high_threshold (%u) can't be more than 99.99%%\n",
+ threshold);
+ return -EINVAL;
+ }
+
+ return param_set_uint(val, kp);
+}
+
+static const struct kernel_param_ops high_threshold_ops = {
+ .set = param_set_high_threshold,
+ .get = param_get_uint,
+};
+
+module_param_cb(high_threshold, &high_threshold_ops, &sg_core_ctx.high_threshold, 0444);
+MODULE_PARM_DESC(high_threshold,
+ "High steal threshold. default: 500 i.e 5%. Must be > low_threshold");
+
+module_param_named(low_threshold, sg_core_ctx.low_threshold, uint, 0444);
+MODULE_PARM_DESC(low_threshold,
+ "Low steal threshold. default: 200 i.e 2%. Must be < high_threshold");
+
static int __init steal_governor_init(void)
{
- pr_info("steal_governor is enabled\n");
+ if (sg_core_ctx.low_threshold >= sg_core_ctx.high_threshold) {
+ pr_err("steal_governor: low_threshold (%u) must be less than high_threshold (%u)\n",
+ sg_core_ctx.low_threshold, sg_core_ctx.high_threshold);
+ return -EINVAL;
+ }
+
+ pr_info("steal_governor is enabled. interval: %ums, high_threshold: %u, low_threshold: %u\n",
+ sg_core_ctx.interval_ms, sg_core_ctx.high_threshold, sg_core_ctx.low_threshold);
+
return 0;
}
--
2.47.3
^ permalink raw reply related
* [PATCH v8 10/11] virt/steal_governor: Implement steal_governor policy loop
From: Shrikanth Hegde @ 2026-07-20 17:22 UTC (permalink / raw)
To: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet
Cc: sshegde, tglx, gregkh, pbonzini, seanjc, vschneid, huschle,
rostedt, dietmar.eggemann, maddy, srikar, hdanton, chleroy,
vineeth, frederic, arighi, pauld, christian.loehle, tj,
tommaso.cucinotta, maz, rafael, rdunlap, kernellwp, linux-doc,
jgross, virtualization
In-Reply-To: <20260720172250.2257582-1-sshegde@linux.ibm.com>
schedule work at regular intervals to implement the steal_governor
policy of monitoring the steal time and take action on the state of
preferred CPUs. The interval is determined by interval_ms parameter.
schedule_delayed_work is used since interval_ms
is in the order of milliseconds. Work need not happen instantly.
Periodic policy loop essentially does:
- Gets the total/delta steal values and cpus to use steal_ratio.
(get_system_steal_time, get_num_cpus_steal_ratio)
- Calculate the steal_ratio as below.
steal_ratio = (delta_steal * 100*100)/(delta_ns * num_cpus())
It is calculated to consider the fractional values of steal time.
I.e 10 means 0.1% steal time. A few tricks such as divide by 10,000
are used to avoid possible overflow.
- If steal value is higher than high threshold, call the method to reduce
the preferred CPUs. (decrease_preferred_cpus)
- If steal value is lower or equal to low threshold, call the method to
increase the preferred CPUs. (increase_preferred_cpus)
- If the steal value is in between, no action is taken.
- Save the values for next delta calculations.
- Ensure design checks are met.
1. At least one core/CPU must be there in preferred mask.
2. preferred CPUs is subset of active CPUs.
If not met, then restore preferred CPUs to active and stop
requeue of the work. Driver is effectively non-functional after that.
In Order to help the above loop, a few helper functions have been added.
1. get_system_steal_time()
- steal governor takes global view of steal time instead of individual
vCPU. Collect the steal values across the vCPUs of interest.
- Sum up steal time values across possible CPUs. This helps to keep it
a monotonically increasing number and avoids spikes due to CPU
hotplug.
2. decrease_preferred_cpus()
- Called when there is high steal time. It needs to decide which CPUs to
mark as non-preferred and set that state.
- Get first housekeeping CPU and its core mask. Mark it as
protected core. This helps to keep at least one core as preferred.
kernel ensures at least one housekeeping CPU stays active.
- Find the last CPU outside of this protected core mask. (target CPU)
- Based on that target CPU, get its sibling and mark them as
non-preferred.
3. increase_preferred_cpus()
- Called when there is low steal time. It needs to decide which CPUs to
mark as preferred and set that state.
- Get the first active non-preferred CPUs. This likely is the last
set of CPUs being marked as non-preferred.
- get the siblings of that CPU and mark them as preferred.
4. get_num_cpus_steal_ratio()
- informs how many CPUs needs to be considered for steal_ratio
calculations.
- Return number of possible CPUs as get_system_steal_time computes
steal values across possible CPUs.
Notes:
1. Using core instead of individual CPUs performs better as SMT is
quite common and some hypervisor such as powerVM does core scheduling.
2. This doesn't do any NUMA splicing to keep the code simpler and
minimal overhead. Current code expects CPUs spread uniformly
across NUMA nodes.
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
drivers/virt/steal_governor/core.c | 160 +++++++++++++++++++++++++++++
drivers/virt/steal_governor/core.h | 5 +
2 files changed, 165 insertions(+)
diff --git a/drivers/virt/steal_governor/core.c b/drivers/virt/steal_governor/core.c
index 1cb766a8ce28..97f82d6df60f 100644
--- a/drivers/virt/steal_governor/core.c
+++ b/drivers/virt/steal_governor/core.c
@@ -89,6 +89,158 @@ module_param_named(low_threshold, sg_core_ctx.low_threshold, uint, 0444);
MODULE_PARM_DESC(low_threshold,
"Low steal threshold. default: 200 i.e 2%. Must be < high_threshold");
+/*
+ * Returns steal time of the full system.
+ * Compute collective steal time across all possible CPUs.
+ */
+static u64 get_system_steal_time(void)
+{
+ int cpu;
+ u64 total_steal = 0;
+
+ for_each_possible_cpu(cpu)
+ total_steal += kcpustat_cpu(cpu).cpustat[CPUTIME_STEAL];
+
+ return total_steal;
+}
+
+/*
+ * Returns number of CPUs to consider for steal ratio.
+ * Return possible CPUs.
+ */
+static unsigned int get_num_cpus_steal_ratio(void)
+{
+ return num_possible_cpus();
+}
+
+/*
+ * Take action to decrease preferred CPUs.
+ *
+ * Decrease the preferred CPUs by 1 core.
+ * Take out the last core in the active & preferred.
+ *
+ * Must ensure
+ * - least one housekeeping core is always kept as preferred
+ * - preferred is always subset of active.
+ */
+static void decrease_preferred_cpus(void)
+{
+ int tmp_cpu, first_hk_cpu, last_cpu;
+ const struct cpumask *first_hk_core;
+ int target_cpu = nr_cpu_ids;
+
+ guard(cpus_read_lock)();
+ first_hk_cpu = cpumask_first_and(housekeeping_cpumask(HK_TYPE_KERNEL_NOISE),
+ cpu_preferred_mask);
+ if (first_hk_cpu >= nr_cpu_ids)
+ return;
+
+ last_cpu = cpumask_last(cpu_preferred_mask);
+
+ if (last_cpu >= nr_cpu_ids)
+ return;
+
+ /* Always leave first housekeeping core as preferred. */
+ first_hk_core = topology_sibling_cpumask(first_hk_cpu);
+
+ /* Find the last CPU which doesn't belong to that first hk_core. */
+ if (!cpumask_test_cpu(last_cpu, first_hk_core)) {
+ target_cpu = last_cpu;
+ } else {
+ for_each_cpu_andnot(tmp_cpu, cpu_preferred_mask, first_hk_core)
+ target_cpu = tmp_cpu;
+ }
+
+ /* Only the first housekeeping core remains */
+ if (target_cpu >= nr_cpu_ids)
+ return;
+
+ for_each_cpu_and(tmp_cpu, topology_sibling_cpumask(target_cpu),
+ cpu_preferred_mask)
+ set_cpu_preferred(tmp_cpu, false);
+}
+
+/*
+ * Take action to increase preferred CPUs.
+ *
+ * Increase the preferred CPUs by 1 core.
+ * Add the first core in active & !preferred
+ *
+ * Must ensure preferred is subset of active.
+ */
+static void increase_preferred_cpus(void)
+{
+ int first_cpu, tmp_cpu;
+
+ guard(cpus_read_lock)();
+ first_cpu = cpumask_first_andnot(cpu_active_mask, cpu_preferred_mask);
+
+ /* All CPUs are preferred. Nothing to increase further */
+ if (first_cpu >= nr_cpu_ids)
+ return;
+
+ for_each_cpu_and(tmp_cpu, topology_sibling_cpumask(first_cpu),
+ cpu_active_mask)
+ set_cpu_preferred(tmp_cpu, true);
+}
+
+static void compute_preferred_cpus_work(struct work_struct *work)
+{
+ u64 curr_steal, delta_steal, delta_ns, steal_ratio;
+ ktime_t now;
+
+ now = ktime_get();
+ delta_ns = ktime_to_ns(ktime_sub(now, sg_core_ctx.time));
+
+ if (unlikely(delta_ns < NSEC_PER_MSEC)) {
+ pr_err_ratelimited("steal_governor: work scheduled too soon delta_ns: %llu\n",
+ delta_ns);
+ goto requeue_work;
+ }
+
+ curr_steal = get_system_steal_time();
+ delta_steal = curr_steal > sg_core_ctx.steal ?
+ curr_steal - sg_core_ctx.steal : 0;
+
+ /* Update for next calculation */
+ sg_core_ctx.steal = curr_steal;
+ sg_core_ctx.time = now;
+
+ /*
+ * steal_ratio = (delta_steal * 100*100)/(delta_ns * num_cpus())
+ * To avoid possible overflow, divide the denominator early.
+ * Note minimum interval is 100ms.
+ */
+ delta_ns = max_t(u64, div_u64(delta_ns * get_num_cpus_steal_ratio(),
+ 100 * 100), 1);
+ steal_ratio = div64_u64(delta_steal, delta_ns);
+
+ if (steal_ratio > sg_core_ctx.high_threshold)
+ decrease_preferred_cpus();
+ if (steal_ratio <= sg_core_ctx.low_threshold)
+ increase_preferred_cpus();
+
+ /* maintain design constructs always */
+ if (cpumask_empty(cpu_preferred_mask)) {
+ pr_err("empty preferred mask. stop steal governor\n");
+ restore_preferred_to_active();
+ return;
+ }
+
+ if (!cpumask_subset(cpu_preferred_mask, cpu_active_mask)) {
+ pr_err("preferred: %*pbl is not subset of active: %*pbl, stop steal governor\n",
+ cpumask_pr_args(cpu_preferred_mask),
+ cpumask_pr_args(cpu_active_mask));
+ restore_preferred_to_active();
+ return;
+ }
+
+requeue_work:
+ /* Trigger for next sampling */
+ schedule_delayed_work(&sg_core_ctx.work,
+ msecs_to_jiffies(sg_core_ctx.interval_ms));
+}
+
static int __init steal_governor_init(void)
{
if (sg_core_ctx.low_threshold >= sg_core_ctx.high_threshold) {
@@ -100,11 +252,19 @@ static int __init steal_governor_init(void)
pr_info("steal_governor is enabled. interval: %ums, high_threshold: %u, low_threshold: %u\n",
sg_core_ctx.interval_ms, sg_core_ctx.high_threshold, sg_core_ctx.low_threshold);
+ INIT_DELAYED_WORK(&sg_core_ctx.work, compute_preferred_cpus_work);
+ sg_core_ctx.steal = get_system_steal_time();
+ sg_core_ctx.time = ktime_get();
+
+ schedule_delayed_work(&sg_core_ctx.work,
+ msecs_to_jiffies(sg_core_ctx.interval_ms));
+
return 0;
}
static void __exit steal_governor_exit(void)
{
+ disable_delayed_work_sync(&sg_core_ctx.work);
restore_preferred_to_active();
pr_info("steal_governor is disabled\n");
}
diff --git a/drivers/virt/steal_governor/core.h b/drivers/virt/steal_governor/core.h
index e27305284ac0..59329c1d7109 100644
--- a/drivers/virt/steal_governor/core.h
+++ b/drivers/virt/steal_governor/core.h
@@ -12,6 +12,11 @@
#include <linux/workqueue.h>
#include <linux/ktime.h>
#include <linux/kconfig.h>
+#include <linux/kernel_stat.h>
+#include <linux/topology.h>
+#include <linux/sched/isolation.h>
+#include <linux/cleanup.h>
+#include <linux/math64.h>
struct steal_governor {
struct delayed_work work;
--
2.47.3
^ permalink raw reply related
* [PATCH v8 11/11] virt/steal_governor: Enable the driver
From: Shrikanth Hegde @ 2026-07-20 17:22 UTC (permalink / raw)
To: linux-kernel, mingo, peterz, juri.lelli, vincent.guittot,
yury.norov, kprateek.nayak, iii, corbet
Cc: sshegde, tglx, gregkh, pbonzini, seanjc, vschneid, huschle,
rostedt, dietmar.eggemann, maddy, srikar, hdanton, chleroy,
vineeth, frederic, arighi, pauld, christian.loehle, tj,
tommaso.cucinotta, maz, rafael, rdunlap, kernellwp, linux-doc,
jgross, virtualization
In-Reply-To: <20260720172250.2257582-1-sshegde@linux.ibm.com>
Provide a config option for users to enable the driver.
Since the feature works for paravirtualized case and make sense only
with SMP, enforce those dependencies. Driver is going to select
CONFIG_PREFERRED_CPU=y for scheduler mechanisms to work.
It is recommended to build the driver as module instead of yes
due to below reasons.
- Module parameters are read only after init. If one wants to change
them having as module allows that.
- Driver can be disabled if it is built as module.
- If it is module, admin user has to enable it. Since this feature works
best when all VMs work in co-operative manner, admin will likely
enable it in all VMs.
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
drivers/virt/Kconfig | 2 ++
drivers/virt/Makefile | 1 +
drivers/virt/steal_governor/Kconfig | 18 ++++++++++++++++++
drivers/virt/steal_governor/Makefile | 6 ++++++
4 files changed, 27 insertions(+)
create mode 100644 drivers/virt/steal_governor/Kconfig
create mode 100644 drivers/virt/steal_governor/Makefile
diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig
index 52eb7e4ba71f..4fbc928022a4 100644
--- a/drivers/virt/Kconfig
+++ b/drivers/virt/Kconfig
@@ -47,6 +47,8 @@ source "drivers/virt/nitro_enclaves/Kconfig"
source "drivers/virt/acrn/Kconfig"
+source "drivers/virt/steal_governor/Kconfig"
+
endif
source "drivers/virt/coco/Kconfig"
diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile
index f29901bd7820..512dcfd6c73e 100644
--- a/drivers/virt/Makefile
+++ b/drivers/virt/Makefile
@@ -10,3 +10,4 @@ obj-y += vboxguest/
obj-$(CONFIG_NITRO_ENCLAVES) += nitro_enclaves/
obj-$(CONFIG_ACRN_HSM) += acrn/
obj-y += coco/
+obj-$(CONFIG_STEAL_GOVERNOR) += steal_governor/
diff --git a/drivers/virt/steal_governor/Kconfig b/drivers/virt/steal_governor/Kconfig
new file mode 100644
index 000000000000..926bb351db4f
--- /dev/null
+++ b/drivers/virt/steal_governor/Kconfig
@@ -0,0 +1,18 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config STEAL_GOVERNOR
+ tristate "Dynamic vCPU management based on steal time"
+ depends on PARAVIRT && SMP
+ select PREFERRED_CPU
+ default m
+ help
+ This driver helps to reduce the steal time in paravirtualised
+ environment, thereby reducing vCPU preemption. Reducing vCPU
+ preemption provides improved lock holder preemption and reduces
+ cost of vCPU preemption in the host.
+
+ By default preferred CPUs will be same as active CPUs. Depending
+ on the steal time when steal_governor driver is enabled,
+ preferred CPUs could become subset of active CPUs.
+
+ It is recommended to build it as module and load the module
+ to enable it.
diff --git a/drivers/virt/steal_governor/Makefile b/drivers/virt/steal_governor/Makefile
new file mode 100644
index 000000000000..eb90ed6b94d7
--- /dev/null
+++ b/drivers/virt/steal_governor/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Steal time governor to alter preferred CPU state.
+obj-$(CONFIG_STEAL_GOVERNOR) += steal_governor.o
+
+steal_governor-y := core.o
--
2.47.3
^ permalink raw reply related
* Re: [PATCH] vsock: use sock_error() to consume sk_err after connect timeout
From: Phi Nguyen @ 2026-07-20 17:34 UTC (permalink / raw)
To: Stefano Garzarella
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, syzbot+1b2c9c4a0f8708082678, virtualization, netdev,
linux-kernel
In-Reply-To: <al3UuvBhfxp82krV@sgarzare-redhat>
On 7/20/2026 4:17 PM, Stefano Garzarella wrote:
> On Mon, Jul 20, 2026 at 05:57:47AM +0800, Nguyen Dinh Phi wrote:
>> After vsock_connect() exits the wait loop due to sk->sk_err being
>> set, the error was read but not cleared. This left sk->sk_err set
>> for subsequent operations.
>
> So, is this a fix? If yes, we should put a Fixes tag.
>
> Also, can you describe how to trigger the issue?
>
> Because I see this in vsock_connect(), so I thought it was in some way
> already handled:
>
> /* sk_err might have been set as a result of an earlier
> * (failed) connect attempt.
> */
> sk->sk_err = 0;
>
This only handles the case where the function following the failed
connect is another connect() call.
>> Switch to sock_error() which atomically reads and clears sk->sk_err,
>> so the error is consumed when returned.
>>
>> Signed-off-by: Nguyen Dinh Phi <phind.uet@gmail.com>
>> Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
>
> Can you explain how this patch fixes that issue?
> (this should be the first information to be put in the commit message IMHO)
>
> I'd like to understand better if this is a fix of real bug or just an
> improvement to the code (which is fine by me).
>
> Thanks,
> Stefano
>
Here are the steps of the syzkaller reproducer:
r0 = socket(AF_VSOCK, SOCK_STREAM, 0)
bind(r0, {VMADDR_CID_ANY, PORT})
connect(r0, {VMADDR_CID_LOCAL, PORT})
listen(r0, backlog)
r1 = socket(AF_VSOCK, SOCK_STREAM, 0)
connect(r1, {VMADDR_CID_LOCAL, PORT})
connect(r0 -> self) -> -1, EPROTO
listen(r0) -> 0
connect(r1 -> r0) -> 0
accept(r0) -> -1, EPROTO
Basically, it creates a socket (r0) and triggers a self-connect after
binding it. This self-connect fails with EPROTO because it loops back to
r0 while the socket is still in the TCP_SYN_SENT state, causing it to be
incorrectly dispatched to the connecting-client path. The unexpected
packet type encountered there sets sk_err to EPROTO.
After that, it invokes a listen() call on the same socket. This listen()
call succeeds because the kernel's listening path never inspects or
clears sk_err. Then, a new socket (r1) is created as a normal client and
connects to r0. However, vsock_accept() rejects this incoming connection
because the listener's sk_err still holds the EPROTO error from the
earlier failed self-connect.
This rejection causes the child socket created for r1's connection to
never be freed on virtio or hyperv transports; only the VMCI transport
implements pending_work to revisit and clean up a rejected socket
This patch will prevent the rejection branch to occur in this scenario.
I think we might schedule the cleanup worker to run in the rejection
path for these transports as well.
Thanks,
Phi
^ permalink raw reply
* Re: [PATCH v2 02/15] drm/xlnx/zynqmp_dpsub: remove dependency on DRM simple helpers
From: Pandey, Radhey Shyam @ 2026-07-20 17:49 UTC (permalink / raw)
To: Diogo Silva, Jingoo Han, Inki Dae, Seung-Woo Kim, Kyungmin Park,
David Airlie, Simona Vetter, Krzysztof Kozlowski, Peter Griffin,
Alim Akhtar, Laurent Pinchart, Tomi Valkeinen, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Michal Simek, Thierry Reding,
Mikko Perttunen, Jonathan Hunter, Stefan Agner, Alison Wang,
Anitha Chrisanthus, David Airlie, Gerd Hoffmann, Dmitry Osipenko,
Gurchetan Singh, Chia-I Wu, Jyri Sarha, Liu Ying, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Geert Uytterhoeven, Xinliang Liu,
Sumit Semwal, Yongqin Liu, John Stultz, Liviu Dudau,
Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Jonathan Corbet, Shuah Khan
Cc: dri-devel, linux-arm-kernel, linux-samsung-soc, linux-kernel,
linux-tegra, virtualization, imx, linux-mediatek,
linux-renesas-soc, linux-amlogic, linux-doc
In-Reply-To: <20260720-drm_simple_encoder_init-v2-2-5020b630668a@gmail.com>
On 7/20/2026 9:10 PM, Diogo Silva wrote:
> The simple KMS helpers are deprecated because they only add an
> intermediate layer between drivers and atomic modesetting.
>
> Open-code drm_simple_encoder_init() by calling drm_encoder_init()
> directly and providing driver-local drm_encoder_funcs.
> Also check the return value from drm_encoder_init() to avoid silent
> failures.
>
> Signed-off-by: Diogo Silva <diogompaissilva@gmail.com>
Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Thanks!
> ---
> drivers/gpu/drm/xlnx/zynqmp_kms.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> index d5f922450565..ac9197e026af 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c
> @@ -29,8 +29,8 @@
> #include <drm/drm_managed.h>
> #include <drm/drm_mode_config.h>
> #include <drm/drm_plane.h>
> +#include <drm/drm_print.h>
> #include <drm/drm_probe_helper.h>
> -#include <drm/drm_simple_kms_helper.h>
> #include <drm/drm_vblank.h>
>
> #include <linux/clk.h>
> @@ -417,6 +417,10 @@ static const struct drm_driver zynqmp_dpsub_drm_driver = {
> .minor = 0,
> };
>
> +static const struct drm_encoder_funcs zynqmp_dpsub_encoder_funcs = {
> + .destroy = drm_encoder_cleanup,
> +};
> +
> static int zynqmp_dpsub_kms_init(struct zynqmp_dpsub *dpsub)
> {
> struct drm_encoder *encoder = &dpsub->drm->encoder;
> @@ -436,7 +440,13 @@ static int zynqmp_dpsub_kms_init(struct zynqmp_dpsub *dpsub)
>
> /* Create the encoder and attach the bridge. */
> encoder->possible_crtcs |= drm_crtc_mask(&dpsub->drm->crtc);
> - drm_simple_encoder_init(&dpsub->drm->dev, encoder, DRM_MODE_ENCODER_NONE);
> + ret = drm_encoder_init(&dpsub->drm->dev, encoder,
> + &zynqmp_dpsub_encoder_funcs,
> + DRM_MODE_ENCODER_NONE, NULL);
> + if (ret) {
> + drm_err(&dpsub->drm->dev, "failed to initialize encoder\n");
> + return ret;
> + }
>
> ret = drm_bridge_attach(encoder, dpsub->bridge, NULL,
> DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>
^ permalink raw reply
* Re: [PATCH v4 0/8] media: add virtio-media driver
From: Brian Daniels @ 2026-07-20 18:11 UTC (permalink / raw)
To: Albert Esteve
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, acourbot, adelva,
changyeon, daniel.almeida, eperezma, gnurou, gurchetansingh,
hverkuil, jasowang, linux-kernel, linux-media, mst,
nicolas.dufresne, virtualization, xuanzhuo
In-Reply-To: <CADSE00JkzGmN8Hvo1fuAsO1o9TKMrPwpmag-dtcFH1+NvkP6HA@mail.gmail.com>
On Mon, Jul 13, 2026 at 3:00 AM Albert Esteve <aesteve@redhat.com> wrote:
>
> On Sun, Jul 12, 2026 at 9:10 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > On Mon, 22 Jun 2026 16:43:35 -0400
> > Brian Daniels <briandaniels@google.com> wrote:
> >
> > > From: Alexandre Courbot <gnurou@gmail.com>
> > >
> > > Add the first version of the virtio-media driver.
> > >
> > > This driver acts roughly as a V4L2 relay between user-space and the
> > > virtio virtual device on the host, so it is relatively simple, yet
> > > unconventional. It doesn't use VB2 or other frameworks typically used in
> > > a V4L2 driver, and most of its complexity resides in correctly and
> > > efficiently building the virtio descriptor chain to pass to the host,
> > > avoiding copies whenever possible. This is done by
> > > scatterlist_builder.[ch].
> > >
> > > This version supports MMAP buffers, while USERPTR buffers can also be
> > > enabled through a driver option. DMABUF support is still pending.
> >
> > In practice, USERPTR was used on several drivers that wanted to
> > share buffers between V4L2 and GPU (so, a previous approach before
> > DMABUF implementation).
> >
> > On my tests with this driver, I was unable use a 1080p camera with
> > V4L2 and GPU on crossvm. Lower resolutions worked. No idea if this
> > was a limitation of crossvm (I only used it to test this driver)
> > or if it is due to a poor MMAP implementation.
> >
I followed these instructions to test with crosvm on my wayland machine:
https://crosvm.dev/book/devices/wayland.html
Note that I built with `cargo build --features "gpu,media"`, not just
`cargo build --features "gpu"` to ensure virtio-media was enabled.
My crosvm command was as follows:
./crosvm/target/release/crosvm run \
linux_upstream/arch/x86/boot/bzImage \
--disable-sandbox \
--block old-debian-12.img \
-p "root=/dev/vda1" \
--net tap-name=crosvm_tap \
-m 8192 \
-c num-cores=16 \
-s /tmp/crosvm.sock \
--v4l2-proxy=/dev/video0 \
--gpu=context-types=cross-domain \
--wayland-sock $XDG_RUNTIME_DIR/wayland-0 \
--serial type=stdout,hardware=virtio-console,console=true,stdin=true
The path to your kernel, block device image, and /dev/video* device
may be different
depending on your development setup.
In the guest, I launched qv4l2 with the following command:
sommelier --virtgpu-channel qv4l2 -platform wayland
I had to disable OpenGL rendering under the "Capture" menu (otherwise
the rendered
window just showed up as white) but other than that it rendered 1080p
smoothly using
the MJPEG format.
Here's a video showing 1080p working smoothly:
https://www.youtube.com/watch?v=aw03pM_-7oA
> >
> > > Compliance Testing
> > >
> > > This was tested using v4l2-compliance. Since virtio-media serves as
> > > a proxy to host devices for the guest VMs, we expect the guest
> > > compliance test to essentially match the host compliance test for the
> > > same device.
> > >
> > > NOTE: v4l2-compliance changes its test behavior depending on the driver
> > > name. In the guest, the driver name for virtio-media proxied-devices is
> > > always "virtio-media", even if the actual host device has a driver name
> > > of e.g. "uvcvideo". To ensure the test is consistent between the host
> > > and the guest, I created a patch for the v4l2-compliance tool that
> > > allows you to override the driver name. All test results that follow use
> > > this patch:
> > > https://lore.kernel.org/r/20260528163448.4031965-1-briandaniels@google.com/
> >
> > As mentioned before, please submit this with their rationale in
> > separate as a [PATCH v4l-utils] to linux-media ML.
Sounds good, let's continue the discussion on my existing [PATCH
v4l-utils] thread:
https://lore.kernel.org/all/20260528163448.4031965-1-briandaniels@google.com/
> > >
> > > All tests used a Logitech USB Webcam C925e.
> >
> > Please test it displaying inside crossvm - or even better to QEMU if
> > you manage to add virtio-media support to it.
> >
> > Being at QEMU makes a lot easier for everyone to test it.
>
> Hi,
>
> Regarding the QEMU support mention, I created this series in QEMU to
> add the virtio-media PCI device:
> https://lore.kernel.org/all/20260630112310.552606-1-aesteve@redhat.com/
>
> Testing was done using an older driver version at
> https://github.com/chromeos/virtio-media/tree/main/driver as described
> in the cover letter. But I can try testing it with this series. Either
> way, the procedure for using QEMU is in the cover letter, so anyone
> can try it.
If you'd be able to test again with this series and post your results
that would be great, thanks Albert!
> BR,
> Albert.
>
> >
> >
> > Thanks,
> > Mauro
> >
>
^ permalink raw reply
* Re: [PATCH v4 1/8] media: virtio: Add protocol
From: Brian Daniels @ 2026-07-20 20:07 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Mauro Carvalho Chehab, acourbot, adelva, aesteve, changyeon,
daniel.almeida, eperezma, gnurou, gurchetansingh, hverkuil,
jasowang, linux-kernel, linux-media, mst, nicolas.dufresne,
virtualization, xuanzhuo
In-Reply-To: <20260712092836.488acb8d@foz.lan>
On Sun, Jul 12, 2026 at 3:28 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> On Mon, 22 Jun 2026 16:43:36 -0400
> Brian Daniels <briandaniels@google.com> wrote:
>
> > From: Alexandre Courbot <gnurou@gmail.com>
> >
> > Add the identifiers and structs used to implement the virtio interface
> > as described in section 5.22 of version 1.4 of the virtio spec:
> >
> > https://docs.oasis-open.org/virtio/virtio/v1.4/csprd01/virtio-v1.4-csprd01-diff-from-v1.2-cs01.html#x1-82200022
> >
> > Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
> > Co-developed-by: Brian Daniels <briandaniels@google.com>
> > Signed-off-by: Brian Daniels <briandaniels@google.com>
> > ---
> > drivers/media/virtio/protocol.h | 287 ++++++++++++++++++++++++++++++++
> > 1 file changed, 287 insertions(+)
> > create mode 100644 drivers/media/virtio/protocol.h
> >
> > diff --git a/drivers/media/virtio/protocol.h b/drivers/media/virtio/protocol.h
> > new file mode 100644
> > index 000000000..5878d107c
> > --- /dev/null
> > +++ b/drivers/media/virtio/protocol.h
> > @@ -0,0 +1,287 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0+ */
> > +
> > +/*
> > + * Definitions of virtio-media protocol structures.
> > + *
> > + * Copyright (c) 2024-2025 Google LLC.
> > + */
> > +
> > +#ifndef __VIRTIO_MEDIA_PROTOCOL_H
> > +#define __VIRTIO_MEDIA_PROTOCOL_H
> > +
> > +#include <linux/videodev2.h>
> > +#include <linux/bits.h>
> > +
> > +/*
> > + * Virtio protocol definition.
> > + */
> > +
> > +/**
> > + * struct virtio_media_cmd_header - Header for all virtio-media commands.
> > + * @cmd: one of VIRTIO_MEDIA_CMD_*.
> > + * @__reserved: must be set to zero by the driver.
> > + *
> > + * This header starts all commands from the driver to the device on the
> > + * commandq.
> > + */
> > +struct virtio_media_cmd_header {
> > + u32 cmd;
> > + u32 __reserved;
> > +};
> > +
> > +/**
> > + * struct virtio_media_resp_header - Header for all virtio-media responses.
> > + * @status: 0 if the command was successful, or one of the standard Linux error
> > + * codes.
> > + * @__reserved: must be set to zero by the device.
> > + *
> > + * This header starts all responses from the device to the driver on the
> > + * commandq.
> > + */
> > +struct virtio_media_resp_header {
> > + u32 status;
> > + u32 __reserved;
> > +};
> > +
> > +/**
> > + * VIRTIO_MEDIA_CMD_OPEN - Command for creating a new session.
> > + *
> > + * This is the equivalent of calling `open` on a V4L2 device node. Upon
> > + * success, a session id is returned which can be used to perform other
> > + * commands on the session, notably ioctls.
> > + */
> > +#define VIRTIO_MEDIA_CMD_OPEN 1
> > +
> > +/**
> > + * struct virtio_media_cmd_open - Driver command for VIRTIO_MEDIA_CMD_OPEN.
> > + * @hdr: header with cmd member set to VIRTIO_MEDIA_CMD_OPEN.
> > + */
> > +struct virtio_media_cmd_open {
> > + struct virtio_media_cmd_header hdr;
> > +};
> > +
> > +/**
> > + * struct virtio_media_resp_open - Device response for VIRTIO_MEDIA_CMD_OPEN.
> > + * @hdr: header containing the status of the command.
> > + * @session_id: if hdr.status == 0, contains the id of the newly created
> > + * session.
> > + * @__reserved: must be set to zero by the device.
> > + */
> > +struct virtio_media_resp_open {
> > + struct virtio_media_resp_header hdr;
> > + u32 session_id;
> > + u32 __reserved;
> > +};
> > +
> > +/**
> > + * VIRTIO_MEDIA_CMD_CLOSE - Command for closing an active session.
> > + *
> > + * This is the equivalent of calling `close` on a previously opened V4L2
> > + * session. All resources associated with this session will be freed and the
> > + * session ID shall not be used again after queueing this command.
> > + *
> > + * This command does not require a response from the device.
> > + */
> > +#define VIRTIO_MEDIA_CMD_CLOSE 2
> > +
> > +/**
> > + * struct virtio_media_cmd_close - Driver command for VIRTIO_MEDIA_CMD_CLOSE.
> > + * @hdr: header with cmd member set to VIRTIO_MEDIA_CMD_CLOSE.
> > + * @session_id: id of the session to close.
> > + * @__reserved: must be set to zero by the driver.
> > + */
> > +struct virtio_media_cmd_close {
> > + struct virtio_media_cmd_header hdr;
> > + u32 session_id;
> > + u32 __reserved;
> > +};
> > +
> > +/**
> > + * VIRTIO_MEDIA_CMD_IOCTL - Driver command for executing an ioctl.
> > + *
> > + * This command asks the device to run one of the `VIDIOC_*` ioctls on the
> > + * active session.
> > + *
> > + * The code of the ioctl is extracted from the VIDIOC_* definitions in
> > + * `videodev2.h`, and consists of the second argument of the `_IO*` macro.
> > + *
> > + * Each ioctl has a payload, which is defined by the third argument of the
> > + * `_IO*` macro defining it. It can be writable by the driver (`_IOW`), the
> > + * device (`_IOR`), or both (`_IOWR`).
> > + *
> > + * If an ioctl is writable by the driver, it must be followed by a
> > + * driver-writable descriptor containing the payload.
> > + *
> > + * If an ioctl is writable by the device, it must be followed by a
> > + * device-writable descriptor of the size of the payload that the device will
> > + * write into.
> > + *
> > + */
> > +#define VIRTIO_MEDIA_CMD_IOCTL 3
> > +
> > +/**
> > + * struct virtio_media_cmd_ioctl - Driver command for VIRTIO_MEDIA_CMD_IOCTL.
> > + * @hdr: header with cmd member set to VIRTIO_MEDIA_CMD_IOCTL.
> > + * @session_id: id of the session to run the ioctl on.
> > + * @code: code of the ioctl to run.
> > + */
> > +struct virtio_media_cmd_ioctl {
> > + struct virtio_media_cmd_header hdr;
> > + u32 session_id;
> > + u32 code;
> > +};
> > +
> > +/**
> > + * struct virtio_media_resp_ioctl - Device response for VIRTIO_MEDIA_CMD_IOCTL.
> > + * @hdr: header containing the status of the ioctl.
> > + */
> > +struct virtio_media_resp_ioctl {
> > + struct virtio_media_resp_header hdr;
> > +};
> > +
> > +/**
> > + * struct virtio_media_sg_entry - Description of part of a scattered guest
> > + * memory.
> > + * @start: start guest address of the memory segment.
> > + * @len: length of this memory segment.
> > + * @__reserved: must be set to zero by the driver.
> > + */
> > +struct virtio_media_sg_entry {
> > + u64 start;
> > + u32 len;
> > + u32 __reserved;
> > +};
> > +
> > +#define VIRTIO_MEDIA_MMAP_FLAG_RW BIT(0)
> > +
> > +/**
> > + * VIRTIO_MEDIA_CMD_MMAP - Command for mapping a MMAP buffer into the driver's
> > + * address space.
> > + *
> > + */
> > +#define VIRTIO_MEDIA_CMD_MMAP 4
> > +
> > +/**
> > + * struct virtio_media_cmd_mmap - Driver command for VIRTIO_MEDIA_CMD_MMAP.
> > + * @hdr: header with cmd member set to VIRTIO_MEDIA_CMD_MMAP.
> > + * @session_id: ID of the session we are mapping for.
> > + * @flags: combination of VIRTIO_MEDIA_MMAP_FLAG_*.
> > + * @offset: mem_offset field of the plane to map, as returned by
> > + * VIDIOC_QUERYBUF.
> > + */
> > +struct virtio_media_cmd_mmap {
> > + struct virtio_media_cmd_header hdr;
> > + u32 session_id;
> > + u32 flags;
> > + u32 offset;
> > +};
> > +
> > +/**
> > + * struct virtio_media_resp_mmap - Device response for VIRTIO_MEDIA_CMD_MMAP.
> > + * @hdr: header containing the status of the command.
> > + * @driver_addr: offset into SHM region 0 of the start of the mapping.
> > + * @len: length of the mapping.
> > + */
> > +struct virtio_media_resp_mmap {
> > + struct virtio_media_resp_header hdr;
> > + u64 driver_addr;
> > + u64 len;
> > +};
> > +
> > +/**
> > + * VIRTIO_MEDIA_CMD_MUNMAP - Unmap a MMAP buffer previously mapped using
> > + * VIRTIO_MEDIA_CMD_MMAP.
> > + */
> > +#define VIRTIO_MEDIA_CMD_MUNMAP 5
> > +
> > +/**
> > + * struct virtio_media_cmd_munmap - Driver command for VIRTIO_MEDIA_CMD_MUNMAP.
> > + * @hdr: header with cmd member set to VIRTIO_MEDIA_CMD_MUNMAP.
> > + * @driver_addr: offset into SHM region 0 at which the buffer has been
> > + * previously
> > + * mapped.
> > + */
> > +struct virtio_media_cmd_munmap {
> > + struct virtio_media_cmd_header hdr;
> > + u64 driver_addr;
> > +};
> > +
> > +/**
> > + * struct virtio_media_resp_munmap - Device response for
> > + * VIRTIO_MEDIA_CMD_MUNMAP.
> > + * @hdr: header containing the status of the command.
> > + */
> > +struct virtio_media_resp_munmap {
> > + struct virtio_media_resp_header hdr;
> > +};
> > +
> > +/* The values for these events are set by the virtio-media specification. */
> > +#define VIRTIO_MEDIA_EVT_ERROR 0
> > +#define VIRTIO_MEDIA_EVT_DQBUF 1
> > +#define VIRTIO_MEDIA_EVT_EVENT 2
>
> As I mentioned on v3, media also has events. Some userspace apps may
> need them to do some optimizations related to buffer sync, source
> changes, motion detection and such. See the specs:
>
> https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/vidioc-dqevent.html#event-type
> https://linuxtv.org/downloads/v4l-dvb-apis-new/driver-api/v4l2-event.html
>
> As the virtio driver may end needing such events in the future,
> it needs to start numbering its private events after:
>
> #define V4L2_EVENT_PRIVATE_START 0x08000000
>
> which is where driver-specific events should be placed.
The VIRTIO_MEDIA_EVT_* event IDs are supposed to be seperate from the
V4L2_EVENT_* event IDs. The virtio events are for the private use of
the virtio driver and are never directly exposed to the V4L2 framework
or the user. They're events for the VIRTIO transport.
The VIRTIO_MEDIA_EVT_EVENT is used to pass through the V4L2 events
(shown in section 5.22.6.2.4 of the virtio spec
[https://docs.oasis-open.org/virtio/virtio/v1.4/csprd01/virtio-v1.4-csprd01-diff-from-v1.2-cs01.html#x1-8620004]).
As an example, here's the virtio message for passing through a
V4L2_EVENT_VSYNC event:
struct virtio_media_event_event {
struct virtio_media_event_header hdr; // hdr.event =
VIRTIO_MEDIA_EVT_EVENT
struct v4l2_event event; // event.type = V4L2_EVENT_VSYNC
};
The virtio-media driver currently doesn't send events to the V4L2
framework unless they were sent to it by the host device (through
VIRTIO_MEDIA_EVT_EVENT). However, if this changes in the future, I
anticipate we would have a function that mapped the virtio events to
the v4l2 events if that were necessary.
Does that make sense to you? Or perhaps I'm misunderstanding?
> Thanks,
> Mauro
^ permalink raw reply
* Re: [PATCH v3] media: add virtio-media driver
From: Brian Daniels @ 2026-07-20 20:50 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: acourbot, adelva, aesteve, changyeon, daniel.almeida, eperezma,
gnurou, gurchetansingh, hverkuil, jasowang, linux-kernel,
linux-media, mchehab, mst, nicolas.dufresne, virtualization,
xuanzhuo
In-Reply-To: <20260712082408.49de652a@foz.lan>
On Sun, Jul 12, 2026 at 2:30 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> On Fri, 29 May 2026 12:03:04 -0400
> Brian Daniels <briandaniels@google.com> wrote:
>
> > Hi there! My name is Brian Daniels and I'll be taking over upstreaming this
> > driver from Alexandre Courbot.
> >
> > I've consulted with Alexandre and my plan is to upload a v4 set of patches
> > shortly based on the feedback from this revision.
>
> As you promised a v5 to Michael, I'll wait for it to review the new series.
>
> For now, let me address the replies from you on the top of v3.
Thanks very much! I'll try to note any comments that we can move to
the v4 thread so we can consolidate our discussion.
> > Before doing so, I'd like to address some of your comments:
> >
> > > Hi Alex,
> > >
> > > I didn't see on a first glance anything that would cause locking
> > > issues here, but, as I pointed on my last e-mail, testing with
> > > qv4l2 at the max res of my C920 camera, it ended keeping 24 CPUs
> > > busy without showing any results with qv4l2 (via ssh at the same
> > > machine). So, I suspect that there are issues somewhere, but I didn't
> > > debug any further.
> > >
> > > Please do some tests with a high-res camera, using either ssh or
> > > GPU emulation to see how this behaves with real apps.
> >
> > I did some testing with a 1080p USB webcam over ssh using ffmpeg and I was able
> > to stream video to disk without any issue. Let me know if you'd prefer I test
> > with a specific setup.
>
> You need to test it displaying on a X11 or Wayland window inside the VM, not
> just via an app like ffmeg.
I was able to test this successfully. Please see my comment in the v4 thread.
> >
> > > > +config MEDIA_VIRTIO
> > > > + tristate "Virtio-media Driver"
> > > > + depends on VIRTIO && VIDEO_DEV && 64BIT && (X86 || (ARM && CPU_LITTLE_ENDIAN))
> > >
> > > Why are you limiting it to x86_64 and arm64 little endian?
> >
> > The little endian requirement comes for the section 5.22.6.1.5 of the virtio
> > v1.4 specification [1]. The limitation to x86_64 and arm64 is for two reasons:
> >
> > 1. The specification requires all v4l2 structures to use the 64-bit layout
> > 2. This driver has only been tested on x86_64 and arm64 so far
>
> I don't like non-portable interfaces. If the spec is not good, it needs to
> be fixed; if the problem is at the API interface, you may need compat32 or
> similar to ensure that the interface will work independently on host/guest
> CPU differences.
>
For guests that are not natively 64-bit little endian, the driver
would be required to perform the translation. This is done in other
virtio drivers (e.g. virtio input: in drivers/virtio/virtio_input.c,
you see `cpu_to_le*` and `le*_to_cpu` doing the conversions).
This driver currently doesn't perform this translation, which is why
the Kconfig includes the restrictions. Does this need to be addressed
in the initial version? Or can the translation be added in the future
for expanded platform support?
> >
> >
> > > > +/**
> > > > + * enum virtio_media_memory - Memory types supported by virtio-media.
> > > > + * @VIRTIO_MEDIA_MMAP: memory allocated and managed by device. Can be mapped
> > > > + * into the guest using VIRTIO_MEDIA_CMD_MMAP.
> > > > + * @VIRTIO_MEDIA_SHARED_PAGES: memory allocated by the driver. Passed to the
> > > > + * device using virtio_media_sg_entry.
> > > > + * @VIRTIO_MEDIA_OBJECT: memory backed by a virtio object.
> > > > + */
> > > > +enum virtio_media_memory {
> > > > + VIRTIO_MEDIA_MMAP = V4L2_MEMORY_MMAP,
> > > > + VIRTIO_MEDIA_SHARED_PAGES = V4L2_MEMORY_USERPTR,
> > > > + VIRTIO_MEDIA_OBJECT = V4L2_MEMORY_DMABUF,
> > > > +};
> > >
> > > I'm not a big fan of renaming USERPTR to SHARED_PAGES and
> > > DMABUF to OBJECT, as it makes harder for reviewers and contributors
> > > to remember about this mapping. Also, everybody knows exactly what
> > > DMABUF means, so, it sounds to me that this obfuscates a little bit
> > > the driver.
> > >
> > > Also, why are you encapsulating V4L2 names into VIRTIO_* namespace?
> > > This just adds extra complexity for reviewers without any real
> > > benefit.
> > >
> > > Besides that, doing a grep on the patch, it sounds that this ma
> > > is not used anywhere.
> > >
> > > So, please drop this mapping.
> >
> > Agreed I don't see it being used anywhere, I think this was included originally
> > since it's part of the virtio spec. I will remove it.
> >
> >
> > > > +#define VIRTIO_MEDIA_EVT_ERROR 0
> > > > +#define VIRTIO_MEDIA_EVT_DQBUF 1
> > > > +#define VIRTIO_MEDIA_EVT_EVENT 2
> > >
> > > OK, here, media events are different than virtio events, so having
> > > a virtio-specific events make sense. Yet, better to add a comment
> > > about that.
> > >
> > > Also, V4L events are defined as:
> > >
> > > #define V4L2_EVENT_ALL 0
> > > #define V4L2_EVENT_VSYNC 1
> > > #define V4L2_EVENT_EOS 2
> > > #define V4L2_EVENT_CTRL 3
> > > #define V4L2_EVENT_FRAME_SYNC 4
> > > #define V4L2_EVENT_SOURCE_CHANGE 5
> > > #define V4L2_EVENT_MOTION_DET 6
> > > #define V4L2_EVENT_PRIVATE_START 0x08000000
> > >
> > > As one may end wanting to map them on some future, I would change
> > > the definitions above to:
> > >
> > > #define VIRTIO_MEDIA_EVT_ERROR V4L2_EVENT_PRIVATE_START
> > > #define VIRTIO_MEDIA_EVT_DQBUF (V4L2_EVENT_PRIVATE_START + 1)
> > > #define VIRTIO_MEDIA_EVT_EVENT (V4L2_EVENT_PRIVATE_START + 2)
> >
> > These event values (VIRTIO_MEDIA_EVT_*) are set by section 5.22.6.2.1 of the
> > virtio v1.4 specification [2]. I don't believe we have the ability to change
> > these values as suggested without changing the specification. That would most
> > likely be a lengthy process, so I'd prefer to keep it as written if that's not
> > an issue.
>
> The specs you pointed seem to be specific to V4L2. I can't see any reason
> why not adjusting it to provide an interface that doesn't conflict with
> V4L2 existing events.
I added a response in the v4 thread where you brought it up as well,
let's continue the discussion there. Thanks!
> >
> > > > +#define VIRTIO_MEDIA_MAX_PLANES VIDEO_MAX_PLANES
> > >
> > > Here: why renaming it to VIRTIO_* namespace?
> >
> > This matches the name in section 5.22.6.2.3 of the virtio v1.4 specification
> > [3]. And Alexandre stated the reason why it was renamed from VIDEO_* to
> > VIRTIO_MEDIA_* was from an early piece of feedback to avoid V4L2-specific names.
> > Even though virtio-media reuses the v4l2 structures and API, its possible to use
> > virtio-media with a different media implementation other than v4l2.
>
> In thesis, it would be possible, but it sounds unlikely that this would
> ever happen, as other OSes may have completely different media APIs.
>
> So, in practice, if one wants, let's say, a Windows host to run a media
> virio, it would either need to implement V4L2 internally or will need
> different virtio drivers using a different API.
As far as using a non-Linux OS as your host, then yes you would need
to implement v4l2 internally,
or at least a subset that can interact with v4l2 drivers.
For your original question about why VIDEO_MAX_PLANES was renamed to
VIRTIO_MEDIA_MAX_PLANES, is there any action to take here? Or can it
be left as is?
> > > > +/**
> > > > + * struct virtio_media_session - A session on a virtio_media device.
> > > > + * @fh: file handler for the session.
> > > > + * @id: session ID used to communicate with the device.
> > > > + * @nonblocking_dequeue: whether dequeue should block or not (nonblocking if
> > > > + * file opened with O_NONBLOCK).
> > > > + * @uses_mplane: whether the queues for this session use the MPLANE API or not.
> > > > + * @cmd: union of session-related commands. A session can have one command currently running.
> > > > + * @resp: union of session-related responses. A session can wait on one command only.
> > > > + * @shadow_buf: shadow buffer where data to be added to the descriptor chain can
> > > > + * be staged before being sent to the device.
> > > > + * @command_sgs: SG table gathering descriptors for a given command and its response.
> > > > + * @queues: state of all the queues for this session.
> > > > + * @queues_lock: protects all members fo the queues for this session.
> > > > + * virtio_media_queue_state`.
> > > > + * @dqbuf_wait: waitqueue for dequeued buffers, if ``VIDIOC_DQBUF`` needs to
> > > > + * block or when polling.
> > > > + * @list: link into the list of sessions for the device.
> > > > + */
> > > > +struct virtio_media_session {
> > > > + struct v4l2_fh fh;
> > > > + u32 id;
> > > > + bool nonblocking_dequeue;
> > > > + bool uses_mplane;
> > > > +
> > > > + union {
> > > > + struct virtio_media_cmd_close close;
> > > > + struct virtio_media_cmd_ioctl ioctl;
> > > > + struct virtio_media_cmd_mmap mmap;
> > > > + } cmd;
> > > > +
> > > > + union {
> > > > + struct virtio_media_resp_ioctl ioctl;
> > > > + struct virtio_media_resp_mmap mmap;
> > > > + } resp;
> > > > +
> > >
> > > Heh, the above is tricky, as to parse the struct, one needs first
> > > to check cmd.cmd to identify what values to pick from enums.
> > > Also, the command is stored as: cmd.[close|ioctl|imap].cmd.
> > >
> > > IMO, better to place the headers explicitly there, e.g.
> > >
> > > union {
> > > struct virtio_media_cmd_header hdr;
> > > struct virtio_media_cmd_close close;
> > > struct virtio_media_cmd_ioctl ioctl;
> > > struct virtio_media_cmd_mmap mmap;
> > > } send;
> > > union {
> > > struct virtio_media_resp_header hdr;
> > > struct virtio_media_resp_ioctl ioctl;
> > > struct virtio_media_resp_mmap mmap;
> > > } resp;
> > >
> > > Also, currently, there are 5 defined commands:
> > >
> > > #define VIRTIO_MEDIA_CMD_OPEN 1
> > > #define VIRTIO_MEDIA_CMD_CLOSE 2
> > > #define VIRTIO_MEDIA_CMD_IOCTL 3
> > > #define VIRTIO_MEDIA_CMD_MMAP 4
> > > #define VIRTIO_MEDIA_CMD_MUNMAP 5
> > >
> > > If the data struct is limited only for close/ioctl/mmap, please
> > > document it and point what structure(s) other commands use.
> >
> > Tricky indeed!
> >
> > However, I don't believe its necessary to add an explict header member to the
> > union.
>
> My main concern is that the field used to check what is the content
> of the union is inside the union itself, e.g. virtio_media_cmd_header.cmd.
>
> So, if one does:
>
> struct virtio_media_session session;
>
> it can't check what's the command with session.cmd or session.hdr.cmd.
> Instead, it needs to guess one type, like session.close.cmd to check
> if the command encoded there is close, mmap or ioctl.
>
> > Whenever the driver parses the union, it already has the necessary
> > context to determine which union member to use:
> >
> > - The struct virtio_media_cmd_* instances are always created by the driver and
> > sent to the device, so there are no unknowns there
> > - The struct virtio_media_resp_* instances are always parsed in the same
> > function that sends the corresponding command, so again there's no
> > uncertainty about which union member to access.
>
> If it is always the same function that creates and uses it, you don't
> need an union. You can just use for instance
>
> struct virtio_media_cmd_ioctl
>
> inside the instances that deal with ioctl resp (or ioctl creation).
I will look into this further in v5. It sounds like there are some DMA
alignment and padding requirements that need addressing for these
unions as well.
I can add the "struct virtio_media_*_header" to the unions if it helps
improve readability, but the headers won't be used by the driver
as it's currently written.
> >
> > For these reasons, I would hesistate to add the `struct virtio_media_cmd_header
> > hdr` as previously suggested. Please let me know if you disagree or if I've
> > misunderstood your concern.
>
> >
> > That all being said, I have added comments about which structures use which
> > commands in the upcoming v4 of the patches.
> >
> >
> > > > +/**
> > > > + * struct virtio_media - Virtio-media device.
> > > > + * @v4l2_dev: v4l2_device for the media device.
> > > > + * @video_dev: video_device for the media device.
> > > > + * @virtio_dev: virtio device for the media device.
> > > > + * @commandq: virtio command queue.
> > > > + * @eventq: virtio event queue.
> > > > + * @eventq_work: work to run when events are received on @eventq.
> > > > + * @mmap_region: region into which MMAP buffers are mapped by the host.
> > > > + * @event_buffer: buffer for event descriptors.
> > > > + * @sessions: list of active sessions on the device.
> > > > + * @sessions_lock: protects @sessions and ``virtio_media_session::list``.
> > > > + * @events_lock: prevents concurrent processing of events.
> > > > + * @cmd: union of device-related commands.
> > > > + * @resp: union of device-related responses.
> > > > + * @vlock: serializes access to the command queue.
> > > > + * @wq: waitqueue for host responses on the command queue.
> > > > + */
> > > > +struct virtio_media {
> > > > + struct v4l2_device v4l2_dev;
> > > > + struct video_device video_dev;
> > > > +
> > > > + struct virtio_device *virtio_dev;
> > > > + struct virtqueue *commandq;
> > > > + struct virtqueue *eventq;
> > > > + struct work_struct eventq_work;
> > > > +
> > > > + struct virtio_shm_region mmap_region;
> > > > +
> > > > + void *event_buffer;
> > > > +
> > > > + struct list_head sessions;
> > > > + struct mutex sessions_lock;
> > > > +
> > > > + struct mutex events_lock;
> > > > +
>
> > > > + union {
> > > > + struct virtio_media_cmd_open open;
> > > > + struct virtio_media_cmd_munmap munmap;
> > > > + } cmd;
> > > > +
> > > > + union {
> > > > + struct virtio_media_resp_open open;
> > > > + struct virtio_media_resp_munmap munmap;
> > > > + } resp;
>
> btw the same comment about the past union also applies here:
> if the code needs somehow to identify if the command is open or
> munmap, the field used to determine what command is there should
> be independent of open/munmap.
Yup makes sense. Let's see where our discussion above goes and assume
it applies here as well going forward.
> > >
> > > Based on struct virtio_media_session, I'm assuming here that
> > > this struct is used only for two commands, right? Please document
> > > it at kernel-doc markup and add a point to the other structure used
> > > for the other commands.
> > >
> > > The same comment about headers apply to the union here: place
> > > the header explicitly at the union.
> >
> > The same thing I said above applies here as well. I will add the comments as
> > requested in v4.
> >
> > [1] https://docs.oasis-open.org/virtio/virtio/v1.4/csprd01/virtio-v1.4-csprd01-diff-from-v1.2-cs01.html#x1-8360005
> > [2] https://docs.oasis-open.org/virtio/virtio/v1.4/csprd01/virtio-v1.4-csprd01-diff-from-v1.2-cs01.html#x1-8560001
> > [3] https://docs.oasis-open.org/virtio/virtio/v1.4/csprd01/virtio-v1.4-csprd01-diff-from-v1.2-cs01.html#x1-8600003
>
>
>
> Thanks,
> Mauro
^ 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