* Re: [PATCH v2 00/13] objtool: Detect and warn about indirect calls in __nocfi functions
From: Peter Zijlstra @ 2025-06-06 13:20 UTC (permalink / raw)
To: Sean Christopherson
Cc: Josh Poimboeuf, H. Peter Anvin, x86, kys, haiyangz, wei.liu,
decui, tglx, mingo, bp, dave.hansen, pbonzini, ardb, kees,
Arnd Bergmann, gregkh, linux-hyperv, linux-kernel, kvm, linux-efi,
samitolvanen, ojeda, xin
In-Reply-To: <aELptV62mbTC3YA9@google.com>
On Fri, Jun 06, 2025 at 06:15:19AM -0700, Sean Christopherson wrote:
> On Fri, Jun 06, 2025, Peter Zijlstra wrote:
> > On Thu, Jun 05, 2025 at 10:19:41AM -0700, Josh Poimboeuf wrote:
> > > diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
> > > index 29c5c32c16c3..5d1eef193b79 100644
> > > --- a/arch/x86/entry/entry_64_fred.S
> > > +++ b/arch/x86/entry/entry_64_fred.S
> > > @@ -112,11 +112,12 @@ SYM_FUNC_START(asm_fred_entry_from_kvm)
> > > push %rax /* Return RIP */
> > > push $0 /* Error code, 0 for IRQ/NMI */
> > >
> > > - PUSH_AND_CLEAR_REGS clear_bp=0 unwind_hint=0
> > > + PUSH_AND_CLEAR_REGS clear_callee=0 unwind_hint=0
> > > movq %rsp, %rdi /* %rdi -> pt_regs */
> > > call __fred_entry_from_kvm /* Call the C entry point */
> > > - POP_REGS
> > > - ERETS
> > > + addq $C_PTREGS_SIZE, %rsp
> > > +
> > > + ALTERNATIVE "mov %rbp, %rsp", __stringify(ERETS), X86_FEATURE_FRED
> >
> > So... I was wondering.. do we actually ever need the ERETS?
>
> Yes, to unblock NMIs, because NMIs are blocked on VM-Exit due to NMI.
Ah, bah, indeed! Shame.
^ permalink raw reply
* Re: [PATCH v2 00/13] objtool: Detect and warn about indirect calls in __nocfi functions
From: Sean Christopherson @ 2025-06-06 13:15 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Josh Poimboeuf, H. Peter Anvin, x86, kys, haiyangz, wei.liu,
decui, tglx, mingo, bp, dave.hansen, pbonzini, ardb, kees,
Arnd Bergmann, gregkh, linux-hyperv, linux-kernel, kvm, linux-efi,
samitolvanen, ojeda, xin
In-Reply-To: <20250606104945.GY39944@noisy.programming.kicks-ass.net>
On Fri, Jun 06, 2025, Peter Zijlstra wrote:
> On Thu, Jun 05, 2025 at 10:19:41AM -0700, Josh Poimboeuf wrote:
> > diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
> > index 29c5c32c16c3..5d1eef193b79 100644
> > --- a/arch/x86/entry/entry_64_fred.S
> > +++ b/arch/x86/entry/entry_64_fred.S
> > @@ -112,11 +112,12 @@ SYM_FUNC_START(asm_fred_entry_from_kvm)
> > push %rax /* Return RIP */
> > push $0 /* Error code, 0 for IRQ/NMI */
> >
> > - PUSH_AND_CLEAR_REGS clear_bp=0 unwind_hint=0
> > + PUSH_AND_CLEAR_REGS clear_callee=0 unwind_hint=0
> > movq %rsp, %rdi /* %rdi -> pt_regs */
> > call __fred_entry_from_kvm /* Call the C entry point */
> > - POP_REGS
> > - ERETS
> > + addq $C_PTREGS_SIZE, %rsp
> > +
> > + ALTERNATIVE "mov %rbp, %rsp", __stringify(ERETS), X86_FEATURE_FRED
>
> So... I was wondering.. do we actually ever need the ERETS?
Yes, to unblock NMIs, because NMIs are blocked on VM-Exit due to NMI.
The !X86_FEATURE_FRED path can use RET (instead of IRET) because NMIs are routed
through vmx_do_nmi_irqoff(), as proposed in this version[*], after you pointed out
that the FRED entry doesn't have the legacy NMI madness.
[*] https://lore.kernel.org/all/aBUiwLV4ZY2HdRbz@google.com
^ permalink raw reply
* Re: [PATCH v2 00/13] objtool: Detect and warn about indirect calls in __nocfi functions
From: Peter Zijlstra @ 2025-06-06 10:49 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Sean Christopherson, H. Peter Anvin, x86, kys, haiyangz, wei.liu,
decui, tglx, mingo, bp, dave.hansen, pbonzini, ardb, kees,
Arnd Bergmann, gregkh, linux-hyperv, linux-kernel, kvm, linux-efi,
samitolvanen, ojeda, xin
In-Reply-To: <4z4fhaqesjlevwiugiqpnxdths5qkkj7vd4q3wgdosu4p24ppl@nb6c2gybuwe5>
On Thu, Jun 05, 2025 at 10:19:41AM -0700, Josh Poimboeuf wrote:
> diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
> index d83236b96f22..e680afbf65b6 100644
> --- a/arch/x86/entry/calling.h
> +++ b/arch/x86/entry/calling.h
> @@ -99,7 +99,7 @@ For 32-bit we have the following conventions - kernel is built with
> .endif
> .endm
>
> -.macro CLEAR_REGS clear_bp=1
> +.macro CLEAR_REGS clear_callee=1
> /*
> * Sanitize registers of values that a speculation attack might
> * otherwise want to exploit. The lower registers are likely clobbered
> @@ -113,20 +113,19 @@ For 32-bit we have the following conventions - kernel is built with
> xorl %r9d, %r9d /* nospec r9 */
> xorl %r10d, %r10d /* nospec r10 */
> xorl %r11d, %r11d /* nospec r11 */
> + .if \clear_callee
> xorl %ebx, %ebx /* nospec rbx */
> - .if \clear_bp
> xorl %ebp, %ebp /* nospec rbp */
> - .endif
> xorl %r12d, %r12d /* nospec r12 */
> xorl %r13d, %r13d /* nospec r13 */
> xorl %r14d, %r14d /* nospec r14 */
> xorl %r15d, %r15d /* nospec r15 */
> -
> + .endif
> .endm
>
> -.macro PUSH_AND_CLEAR_REGS rdx=%rdx rcx=%rcx rax=%rax save_ret=0 clear_bp=1 unwind_hint=1
> +.macro PUSH_AND_CLEAR_REGS rdx=%rdx rcx=%rcx rax=%rax save_ret=0 clear_callee=1 unwind_hint=1
> PUSH_REGS rdx=\rdx, rcx=\rcx, rax=\rax, save_ret=\save_ret unwind_hint=\unwind_hint
> - CLEAR_REGS clear_bp=\clear_bp
> + CLEAR_REGS clear_callee=\clear_callee
> .endm
>
> .macro POP_REGS pop_rdi=1
Nice. So that leaves the callee-clobbered, extra caller-saved and return
registers cleared, and preserves the callee-saved regs.
> diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
> index 29c5c32c16c3..5d1eef193b79 100644
> --- a/arch/x86/entry/entry_64_fred.S
> +++ b/arch/x86/entry/entry_64_fred.S
> @@ -112,11 +112,12 @@ SYM_FUNC_START(asm_fred_entry_from_kvm)
> push %rax /* Return RIP */
> push $0 /* Error code, 0 for IRQ/NMI */
>
> - PUSH_AND_CLEAR_REGS clear_bp=0 unwind_hint=0
> + PUSH_AND_CLEAR_REGS clear_callee=0 unwind_hint=0
> movq %rsp, %rdi /* %rdi -> pt_regs */
> call __fred_entry_from_kvm /* Call the C entry point */
> - POP_REGS
> - ERETS
> + addq $C_PTREGS_SIZE, %rsp
> +
> + ALTERNATIVE "mov %rbp, %rsp", __stringify(ERETS), X86_FEATURE_FRED
So... I was wondering.. do we actually ever need the ERETS? AFAICT this
will only ever 'inject' external interrupts, and those are not supposed
to change the exception frame, like ever. Only exceptions get to change
the exception frame, but those are explicitly excluded in fred_extint().
As such, it should always be correct to just do:
leave;
RET;
at this point, and call it a day, no? Just completely forget about all
this sillyness with alternatives and funky stack state.
Only problem seems to be that if we do this, then
has_modified_stack_frame() has a fit, because of the register state.
The first to complain is bx, the push %rbx modifies the CFI state to
track where on the stack its saved, and that's not what initial_func_cfi
has.
We can stomp on that with UNWIND_HINT_FUNC right before RET. It's all a
bit magical, but should work, right?
So keeping your CLEAR_REGS changes, I've ended up with the below:
---
diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
index 29c5c32c16c3..8c03d04ea69d 100644
--- a/arch/x86/entry/entry_64_fred.S
+++ b/arch/x86/entry/entry_64_fred.S
@@ -62,8 +62,6 @@ SYM_FUNC_START(asm_fred_entry_from_kvm)
push %rbp
mov %rsp, %rbp
- UNWIND_HINT_SAVE
-
/*
* Both IRQ and NMI from VMX can be handled on current task stack
* because there is no need to protect from reentrancy and the call
@@ -112,19 +110,35 @@ SYM_FUNC_START(asm_fred_entry_from_kvm)
push %rax /* Return RIP */
push $0 /* Error code, 0 for IRQ/NMI */
- PUSH_AND_CLEAR_REGS clear_bp=0 unwind_hint=0
+ PUSH_AND_CLEAR_REGS clear_callee=0 unwind_hint=0
movq %rsp, %rdi /* %rdi -> pt_regs */
+
+ /*
+ * At this point: {rdi, rsi, rdx, rcx, r8, r9}, {r10, r11}, {rax, rdx}
+ * are clobbered, which corresponds to: arguments, extra caller-saved
+ * and return. All registers a C function is allowed to clobber.
+ *
+ * Notably, the callee-saved registers: {rbx, r12, r13, r14, r15}
+ * are untouched, with the exception of rbp, which carries the stack
+ * frame and will be restored before exit.
+ *
+ * Further calling another C function will not alter this state.
+ */
call __fred_entry_from_kvm /* Call the C entry point */
- POP_REGS
- ERETS
-1:
+
+1: /*
+ * Therefore, all that remains to be done at this point is restore the
+ * stack and frame pointer register.
+ */
+ leave
/*
- * Objtool doesn't understand what ERETS does, this hint tells it that
- * yes, we'll reach here and with what stack state. A save/restore pair
- * isn't strictly needed, but it's the simplest form.
+ * Objtool gets confused by the cfi register state; this doesn't match
+ * initial_func_cfi because of PUSH_REGS, where it tracks where those
+ * registers are on the stack.
+ *
+ * Forcefully make it forget this before returning.
*/
- UNWIND_HINT_RESTORE
- pop %rbp
+ UNWIND_HINT_FUNC
RET
SYM_FUNC_END(asm_fred_entry_from_kvm)
^ permalink raw reply related
* Re: [PATCH 1/6] drm/vblank: Add vblank timer
From: Louis Chauvet @ 2025-06-06 8:43 UTC (permalink / raw)
To: Thomas Zimmermann, mhklinux, maarten.lankhorst, mripard, airlied,
simona, drawat.floss, javierm, kraxel, hamohammed.sa, melissa.srw,
fvogt
Cc: dri-devel, linux-hyperv, virtualization
In-Reply-To: <20250605152637.98493-2-tzimmermann@suse.de>
Le 05/06/2025 à 17:24, Thomas Zimmermann a écrit :
> The vblank timer simulates a vblank interrupt for hardware without
> support. Rate-limits the display update frequency.
>
> DRM drivers for hardware without vblank support apply display updates
> ASAP. A vblank event informs DRM clients of the completed update.
>
> Userspace compositors immediately schedule the next update, which
> creates significant load on virtualization outputs. Display updates
> are usually fast on virtualization outputs, as their framebuffers are
> in regular system memory and there's no hardware vblank interrupt to
> throttle the update rate.
>
> The vblank timer is a HR timer that signals the vblank in software.
> It limits the update frequency of a DRM driver similar to a hardware
> vblank interrupt. The timer is not synchronized to the actual vblank
> interval of the display.
>
> The code has been adopted from vkms, which added the funtionality
> in commit 3a0709928b17 ("drm/vkms: Add vblank events simulated by
> hrtimers").
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Louis Chauvet <louis.chauvet@bootlin.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
> drivers/gpu/drm/Makefile | 3 +-
> drivers/gpu/drm/drm_vblank_timer.c | 100 +++++++++++++++++++++++++++++
> include/drm/drm_vblank_timer.h | 26 ++++++++
> 3 files changed, 128 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/drm_vblank_timer.c
> create mode 100644 include/drm/drm_vblank_timer.h
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index b5d5561bbe5f..6722e2d1aa7e 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -146,7 +146,8 @@ drm_kms_helper-y := \
> drm_plane_helper.o \
> drm_probe_helper.o \
> drm_self_refresh_helper.o \
> - drm_simple_kms_helper.o
> + drm_simple_kms_helper.o \
> + drm_vblank_timer.o
> drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
> drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
> obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
> diff --git a/drivers/gpu/drm/drm_vblank_timer.c b/drivers/gpu/drm/drm_vblank_timer.c
> new file mode 100644
> index 000000000000..be46d3135c8e
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_vblank_timer.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <linux/hrtimer.h>
> +
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_managed.h>
> +#include <drm/drm_print.h>
> +#include <drm/drm_vblank.h>
> +#include <drm/drm_vblank_timer.h>
> +
> +static enum hrtimer_restart drm_vblank_timer_function(struct hrtimer *timer)
> +{
> + struct drm_vblank_timer *vtimer = container_of(timer, struct drm_vblank_timer, timer);
> + struct drm_crtc *crtc = vtimer->crtc;
> + struct drm_device *dev = crtc->dev;
> + u64 ret_overrun;
> + bool succ;
> +
> + ret_overrun = hrtimer_forward_now(&vtimer->timer, vtimer->period_ns);
> + if (ret_overrun != 1)
> + drm_warn(dev, "vblank timer overrun\n");
> +
> + if (vtimer->crtc_handle_vblank)
> + succ = vtimer->crtc_handle_vblank(crtc);
> + else
> + succ = drm_crtc_handle_vblank(crtc);
> + if (!succ)
> + return HRTIMER_NORESTART;
> +
> + return HRTIMER_RESTART;
> +}
> +
> +static void drmm_vblank_timer_release(struct drm_device *dev, void *res)
> +{
> + struct drm_vblank_timer *vtimer = res;
> +
> + hrtimer_cancel(&vtimer->timer);
> +}
> +
> +int drmm_vblank_timer_init(struct drm_vblank_timer *vtimer, struct drm_crtc *crtc,
> + bool (*crtc_handle_vblank)(struct drm_crtc *crtc))
> +{
> + struct hrtimer *timer = &vtimer->timer;
> +
> + vtimer->crtc = crtc;
> + vtimer->crtc_handle_vblank = crtc_handle_vblank;
> +
> + hrtimer_setup(timer, drm_vblank_timer_function, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +
> + return drmm_add_action_or_reset(crtc->dev, drmm_vblank_timer_release, vtimer);
> +}
> +EXPORT_SYMBOL(drmm_vblank_timer_init);
> +
> +void drm_vblank_timer_start(struct drm_vblank_timer *vtimer)
> +{
> + struct drm_crtc *crtc = vtimer->crtc;
> + struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
> +
> + drm_calc_timestamping_constants(crtc, &crtc->mode);
> +
> + vtimer->period_ns = ktime_set(0, vblank->framedur_ns);
> + hrtimer_start(&vtimer->timer, vtimer->period_ns, HRTIMER_MODE_REL);
> +}
> +EXPORT_SYMBOL(drm_vblank_timer_start);
> +
> +void drm_vblank_timer_cancel(struct drm_vblank_timer *vtimer)
> +{
> + hrtimer_cancel(&vtimer->timer);
> +}
> +EXPORT_SYMBOL(drm_vblank_timer_cancel);
> +
> +bool drm_vblank_timer_get_vblank_timestamp(struct drm_vblank_timer *vtimer,
> + int *max_error, ktime_t *vblank_time,
> + bool in_vblank_irq)
> +{
> + struct drm_crtc *crtc = vtimer->crtc;
> + struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
> +
> + if (!READ_ONCE(vblank->enabled)) {
> + *vblank_time = ktime_get();
> + return true;
> + }
> +
> + *vblank_time = READ_ONCE(vtimer->timer.node.expires);
> +
> + if (WARN_ON(*vblank_time == vblank->time))
> + return true;
> +
> + /*
> + * To prevent races we roll the hrtimer forward before we do any
> + * interrupt processing - this is how real hw works (the interrupt is
> + * only generated after all the vblank registers are updated) and what
> + * the vblank core expects. Therefore we need to always correct the
> + * timestampe by one frame.
> + */
> + *vblank_time -= vtimer->period_ns;
> +
> + return true;
> +}
> +EXPORT_SYMBOL(drm_vblank_timer_get_vblank_timestamp);
> diff --git a/include/drm/drm_vblank_timer.h b/include/drm/drm_vblank_timer.h
> new file mode 100644
> index 000000000000..0b827ff1f59c
> --- /dev/null
> +++ b/include/drm/drm_vblank_timer.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#ifndef _DRM_VBLANK_TIMER_H_
> +#define _DRM_VBLANK_TIMER_H_
> +
> +#include <linux/hrtimer_types.h>
> +#include <linux/types.h>
> +
> +struct drm_crtc;
> +
> +struct drm_vblank_timer {
> + struct drm_crtc *crtc;
> + bool (*crtc_handle_vblank)(struct drm_crtc *crtc);
> + ktime_t period_ns;
> + struct hrtimer timer;
> +};
> +
> +int drmm_vblank_timer_init(struct drm_vblank_timer *vtimer, struct drm_crtc *crtc,
> + bool (*handle_vblank)(struct drm_crtc *crtc));
> +void drm_vblank_timer_start(struct drm_vblank_timer *vtimer);
> +void drm_vblank_timer_cancel(struct drm_vblank_timer *vtimer);
> +bool drm_vblank_timer_get_vblank_timestamp(struct drm_vblank_timer *vtimer,
> + int *max_error, ktime_t *vblank_time,
> + bool in_vblank_irq);
> +
> +#endif
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 2/6] drm/vkms: Use vblank timer
From: Louis Chauvet @ 2025-06-06 9:14 UTC (permalink / raw)
To: Thomas Zimmermann, mhklinux, maarten.lankhorst, mripard, airlied,
simona, drawat.floss, javierm, kraxel, hamohammed.sa, melissa.srw,
fvogt
Cc: dri-devel, linux-hyperv, virtualization
In-Reply-To: <20250605152637.98493-3-tzimmermann@suse.de>
Le 05/06/2025 à 17:24, Thomas Zimmermann a écrit :
> Replace vkms' vblank timer with the DRM implementation. The DRM
> code is mostly identical.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Tested-by: Louis Chauvet <louis.chauvet@bootlin>
> ---
> drivers/gpu/drm/vkms/vkms_crtc.c | 49 +++++++-------------------------
> drivers/gpu/drm/vkms/vkms_drv.h | 6 ++--
> 2 files changed, 15 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 8c9898b9055d..5b7829e8c900 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -10,22 +10,14 @@
>
> #include "vkms_drv.h"
>
> -static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
> +static bool vkms_crtc_handle_vblank(struct drm_crtc *crtc)
> {
> - struct vkms_output *output = container_of(timer, struct vkms_output,
> - vblank_hrtimer);
> - struct drm_crtc *crtc = &output->crtc;
> + struct vkms_output *output = drm_crtc_to_vkms_output(crtc);
> struct vkms_crtc_state *state;
> - u64 ret_overrun;
> bool ret, fence_cookie;
>
> fence_cookie = dma_fence_begin_signalling();
>
> - ret_overrun = hrtimer_forward_now(&output->vblank_hrtimer,
> - output->period_ns);
> - if (ret_overrun != 1)
> - pr_warn("%s: vblank timer overrun\n", __func__);
> -
> spin_lock(&output->lock);
> ret = drm_crtc_handle_vblank(crtc);
> if (!ret)
> @@ -57,18 +49,14 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
>
> dma_fence_end_signalling(fence_cookie);
>
> - return HRTIMER_RESTART;
> + return true;
> }
>
> static int vkms_enable_vblank(struct drm_crtc *crtc)
> {
> - struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
> struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
>
> - hrtimer_setup(&out->vblank_hrtimer, &vkms_vblank_simulate, CLOCK_MONOTONIC,
> - HRTIMER_MODE_REL);
> - out->period_ns = ktime_set(0, vblank->framedur_ns);
> - hrtimer_start(&out->vblank_hrtimer, out->period_ns, HRTIMER_MODE_REL);
> + drm_vblank_timer_start(&out->vtimer);
>
> return 0;
> }
> @@ -77,7 +65,7 @@ static void vkms_disable_vblank(struct drm_crtc *crtc)
> {
> struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
>
> - hrtimer_cancel(&out->vblank_hrtimer);
> + drm_vblank_timer_cancel(&out->vtimer);
> }
>
> static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
> @@ -85,28 +73,9 @@ static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
> bool in_vblank_irq)
> {
> struct vkms_output *output = drm_crtc_to_vkms_output(crtc);
> - struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
> -
> - if (!READ_ONCE(vblank->enabled)) {
> - *vblank_time = ktime_get();
> - return true;
> - }
> -
> - *vblank_time = READ_ONCE(output->vblank_hrtimer.node.expires);
>
> - if (WARN_ON(*vblank_time == vblank->time))
> - return true;
> -
> - /*
> - * To prevent races we roll the hrtimer forward before we do any
> - * interrupt processing - this is how real hw works (the interrupt is
> - * only generated after all the vblank registers are updated) and what
> - * the vblank core expects. Therefore we need to always correct the
> - * timestampe by one frame.
> - */
> - *vblank_time -= output->period_ns;
> -
> - return true;
> + return drm_vblank_timer_get_vblank_timestamp(&output->vtimer, max_error,
> + vblank_time, in_vblank_irq);
> }
>
> static struct drm_crtc_state *
> @@ -274,6 +243,7 @@ struct vkms_output *vkms_crtc_init(struct drm_device *dev, struct drm_plane *pri
> {
> struct vkms_output *vkms_out;
> struct drm_crtc *crtc;
> + struct drm_vblank_timer *vtimer;
> int ret;
>
> vkms_out = drmm_crtc_alloc_with_planes(dev, struct vkms_output, crtc,
> @@ -285,6 +255,7 @@ struct vkms_output *vkms_crtc_init(struct drm_device *dev, struct drm_plane *pri
> }
>
> crtc = &vkms_out->crtc;
> + vtimer = &vkms_out->vtimer;
>
> drm_crtc_helper_add(crtc, &vkms_crtc_helper_funcs);
>
> @@ -305,5 +276,7 @@ struct vkms_output *vkms_crtc_init(struct drm_device *dev, struct drm_plane *pri
> if (!vkms_out->composer_workq)
> return ERR_PTR(-ENOMEM);
>
> + drmm_vblank_timer_init(vtimer, crtc, vkms_crtc_handle_vblank);
> +
> return vkms_out;
> }
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index a74a7fc3a056..126016898285 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -10,6 +10,7 @@
> #include <drm/drm_gem.h>
> #include <drm/drm_gem_atomic_helper.h>
> #include <drm/drm_encoder.h>
> +#include <drm/drm_vblank_timer.h>
> #include <drm/drm_writeback.h>
>
> #define DEFAULT_DEVICE_NAME "vkms"
> @@ -180,8 +181,9 @@ struct vkms_output {
> struct drm_crtc crtc;
> struct drm_writeback_connector wb_connector;
> struct drm_encoder wb_encoder;
> - struct hrtimer vblank_hrtimer;
> - ktime_t period_ns;
> + struct drm_vblank_timer vtimer;
> + struct drm_pending_vblank_event *event;
> + /* ordered wq for composer_work */
> struct workqueue_struct *composer_workq;
> spinlock_t lock;
>
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Thomas Zimmermann @ 2025-06-06 7:05 UTC (permalink / raw)
To: Michael Kelley, Simona Vetter
Cc: David Hildenbrand, simona@ffwll.ch, deller@gmx.de,
haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, akpm@linux-foundation.org, weh@microsoft.com,
hch@lst.de, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, linux-mm@kvack.org
In-Reply-To: <SN6PR02MB4157F630284939E084486AFED46FA@SN6PR02MB4157.namprd02.prod.outlook.com>
Hi
Am 05.06.25 um 19:38 schrieb Michael Kelley:
[...]
>> I try to address the problem with the patches at
>>
>> https://lore.kernel.org/dri-devel/20250605152637.98493-1-tzimmermann@suse.de/
>>
>> Testing and feedback is much appreciated.
>>
> Nice!
>
> I ran the same test case with your patches, and everything works well. The
> hyperv_drm numbers are now pretty much the same as the hyperv_fb
> numbers for both elapsed time and system CPU time -- within a few percent.
> For hyperv_drm, there's no longer a gap in the elapsed time and system
> CPU time. No errors due to the guest-to-host ring buffer being full. Total
> messages to Hyper-V for hyperv_drm are now a few hundred instead of 3M.
Sounds great. Credit also goes to the vkms devs, which already have the
software vblank in their driver.
This might need better support for cases where display updates take
exceptionally long, but I can see this being merged as a DRM feature.
> The hyperv_drm message count is still a little higher than for hyperv_fb,
> presumably because the simulated vblank rate in hyperv_drm is higher than
> the 20 Hz rate used by hyperv_fb deferred I/O. But the overall numbers are
> small enough that the difference is in the noise. Question: what is the default
> value for the simulated vblank rate? Just curious ...
As with a hardware interrupt, the vblank rate comes from the programmed
display mode, so most likely 60 Hz. The difference in the update
frequency could explain the remaining differences to hyperv_fb.
Best regards
Thomas
>
> Michael
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply
* RE: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Michael Kelley @ 2025-06-05 17:38 UTC (permalink / raw)
To: Thomas Zimmermann, Simona Vetter
Cc: David Hildenbrand, simona@ffwll.ch, deller@gmx.de,
haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, akpm@linux-foundation.org, weh@microsoft.com,
hch@lst.de, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, linux-mm@kvack.org
In-Reply-To: <154aa365-0e27-458c-b801-62fd1cbfa169@suse.de>
From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Thursday, June 5, 2025 8:36 AM
>
> Hi
>
> Am 04.06.25 um 23:43 schrieb Michael Kelley:
> [...]
> > Nonetheless, there's an underlying issue. A main cause of the difference
> > is the number of messages to Hyper-V to update dirty regions. With
> > hyperv_fb using deferred I/O, the messages are limited 20/second, so
> > the total number of messages to Hyper-V is about 480. But hyperv_drm
> > appears to send 3 messages to Hyper-V for each line of output, or a total of
> > about 3,000,000 messages (~90K/second). That's a lot of additional load
> > on the Hyper-V host, and it adds the 10 seconds of additional elapsed
> > time seen in the guest. There also this ugly output in dmesg because the
> > ring buffer for sending messages to the Hyper-V host gets full -- Hyper-V
> > doesn't always keep up, at least not on my local laptop where I'm
> > testing:
> >
> > [12574.327615] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm]
> *ERROR* Unable to send packet via vmbus; error -11
> > [12574.327684] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm]
> *ERROR* Unable to send packet via vmbus; error -11
> > [12574.327760] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm]
> *ERROR* Unable to send packet via vmbus; error -11
> > [12574.327841] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm]
> *ERROR* Unable to send packet via vmbus; error -11
> > [12597.016128] hyperv_sendpacket: 6211 callbacks suppressed
> > [12597.016133] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm]
> *ERROR* Unable to send packet via vmbus; error -11
> > [12597.016172] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm]
> *ERROR* Unable to send packet via vmbus; error -11
> > [12597.016220] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm]
> *ERROR* Unable to send packet via vmbus; error -11
> > [12597.016267] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm]
> *ERROR* Unable to send packet via vmbus; error -11
> >
> > hyperv_drm could be fixed to not output the ugly messages, but there's
> > still the underlying issue of overrunning the ring buffer, and excessively
> > hammering on the host. If we could get hyperv_drm doing deferred I/O, I
> > would feel much better about going full-on with deprecating hyperv_fb.
>
> I try to address the problem with the patches at
>
> https://lore.kernel.org/dri-devel/20250605152637.98493-1-tzimmermann@suse.de/
>
> Testing and feedback is much appreciated.
>
Nice!
I ran the same test case with your patches, and everything works well. The
hyperv_drm numbers are now pretty much the same as the hyperv_fb
numbers for both elapsed time and system CPU time -- within a few percent.
For hyperv_drm, there's no longer a gap in the elapsed time and system
CPU time. No errors due to the guest-to-host ring buffer being full. Total
messages to Hyper-V for hyperv_drm are now a few hundred instead of 3M.
The hyperv_drm message count is still a little higher than for hyperv_fb,
presumably because the simulated vblank rate in hyperv_drm is higher than
the 20 Hz rate used by hyperv_fb deferred I/O. But the overall numbers are
small enough that the difference is in the noise. Question: what is the default
value for the simulated vblank rate? Just curious ...
Michael
^ permalink raw reply
* Re: [PATCH v2 00/13] objtool: Detect and warn about indirect calls in __nocfi functions
From: Josh Poimboeuf @ 2025-06-05 17:19 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Sean Christopherson, H. Peter Anvin, x86, kys, haiyangz, wei.liu,
decui, tglx, mingo, bp, dave.hansen, pbonzini, ardb, kees,
Arnd Bergmann, gregkh, linux-hyperv, linux-kernel, kvm, linux-efi,
samitolvanen, ojeda, xin
In-Reply-To: <eegs5wq4eoqpu5yqlzug7icptiwzusracrp3nlmjkxwfywzvez@jngbkb3xqj6o>
On Tue, Jun 03, 2025 at 09:29:45AM -0700, Josh Poimboeuf wrote:
> On Mon, Jun 02, 2025 at 10:43:42PM -0700, Josh Poimboeuf wrote:
> > On Thu, May 29, 2025 at 11:30:17AM +0200, Peter Zijlstra wrote:
> > > > > So the sequence of fail is:
> > > > >
> > > > > push %rbp
> > > > > mov %rsp, %rbp # cfa.base = BP
> > > > >
> > > > > SAVE
> > >
> > > sub $0x40,%rsp
> > > and $0xffffffffffffffc0,%rsp
> > >
> > > This hits the 'older GCC, drap with frame pointer' case in OP_SRC_AND.
> > > Which means we then hard rely on the frame pointer to get things right.
> > >
> > > However, per all the PUSH/POP_REGS nonsense, BP can get clobbered.
> > > Specifically the code between the CALL and POP %rbp below are up in the
> > > air. I don't think it can currently unwind properly there.
> >
> > RBP is callee saved, so there's no need to pop it or any of the other
> > callee-saved regs. If they were to change, that would break C ABI
> > pretty badly. Maybe add a skip_callee=1 arg to POP_REGS?
>
> This compiles for me:
That last patch had a pretty heinous bug: it didn't adjust the stack
accordingly when it skipped the callee-saved pops.
But actually there's no need to pop *any* regs there.
asm_fred_entry_from_kvm() uses C function ABI, so changes to
callee-saved regs aren't allowed, and changes to caller-saved regs would
have no effect.
How about something like this?
diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index d83236b96f22..e680afbf65b6 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -99,7 +99,7 @@ For 32-bit we have the following conventions - kernel is built with
.endif
.endm
-.macro CLEAR_REGS clear_bp=1
+.macro CLEAR_REGS clear_callee=1
/*
* Sanitize registers of values that a speculation attack might
* otherwise want to exploit. The lower registers are likely clobbered
@@ -113,20 +113,19 @@ For 32-bit we have the following conventions - kernel is built with
xorl %r9d, %r9d /* nospec r9 */
xorl %r10d, %r10d /* nospec r10 */
xorl %r11d, %r11d /* nospec r11 */
+ .if \clear_callee
xorl %ebx, %ebx /* nospec rbx */
- .if \clear_bp
xorl %ebp, %ebp /* nospec rbp */
- .endif
xorl %r12d, %r12d /* nospec r12 */
xorl %r13d, %r13d /* nospec r13 */
xorl %r14d, %r14d /* nospec r14 */
xorl %r15d, %r15d /* nospec r15 */
-
+ .endif
.endm
-.macro PUSH_AND_CLEAR_REGS rdx=%rdx rcx=%rcx rax=%rax save_ret=0 clear_bp=1 unwind_hint=1
+.macro PUSH_AND_CLEAR_REGS rdx=%rdx rcx=%rcx rax=%rax save_ret=0 clear_callee=1 unwind_hint=1
PUSH_REGS rdx=\rdx, rcx=\rcx, rax=\rax, save_ret=\save_ret unwind_hint=\unwind_hint
- CLEAR_REGS clear_bp=\clear_bp
+ CLEAR_REGS clear_callee=\clear_callee
.endm
.macro POP_REGS pop_rdi=1
diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
index 29c5c32c16c3..5d1eef193b79 100644
--- a/arch/x86/entry/entry_64_fred.S
+++ b/arch/x86/entry/entry_64_fred.S
@@ -112,11 +112,12 @@ SYM_FUNC_START(asm_fred_entry_from_kvm)
push %rax /* Return RIP */
push $0 /* Error code, 0 for IRQ/NMI */
- PUSH_AND_CLEAR_REGS clear_bp=0 unwind_hint=0
+ PUSH_AND_CLEAR_REGS clear_callee=0 unwind_hint=0
movq %rsp, %rdi /* %rdi -> pt_regs */
call __fred_entry_from_kvm /* Call the C entry point */
- POP_REGS
- ERETS
+ addq $C_PTREGS_SIZE, %rsp
+
+ ALTERNATIVE "mov %rbp, %rsp", __stringify(ERETS), X86_FEATURE_FRED
1:
/*
* Objtool doesn't understand what ERETS does, this hint tells it that
diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
index ad4ea6fb3b6c..d4f9bfdc24a7 100644
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -94,6 +94,7 @@ static void __used common(void)
BLANK();
DEFINE(PTREGS_SIZE, sizeof(struct pt_regs));
+ OFFSET(C_PTREGS_SIZE, pt_regs, orig_ax);
/* TLB state for the entry code */
OFFSET(TLB_STATE_user_pcid_flush_mask, tlb_state, user_pcid_flush_mask);
^ permalink raw reply related
* Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Thomas Zimmermann @ 2025-06-05 15:35 UTC (permalink / raw)
To: Michael Kelley, Simona Vetter
Cc: David Hildenbrand, simona@ffwll.ch, deller@gmx.de,
haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, akpm@linux-foundation.org, weh@microsoft.com,
hch@lst.de, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, linux-mm@kvack.org
In-Reply-To: <SN6PR02MB415702B00D6D52B0EE962C98D46CA@SN6PR02MB4157.namprd02.prod.outlook.com>
Hi
Am 04.06.25 um 23:43 schrieb Michael Kelley:
[...]
> Nonetheless, there's an underlying issue. A main cause of the difference
> is the number of messages to Hyper-V to update dirty regions. With
> hyperv_fb using deferred I/O, the messages are limited 20/second, so
> the total number of messages to Hyper-V is about 480. But hyperv_drm
> appears to send 3 messages to Hyper-V for each line of output, or a total of
> about 3,000,000 messages (~90K/second). That's a lot of additional load
> on the Hyper-V host, and it adds the 10 seconds of additional elapsed
> time seen in the guest. There also this ugly output in dmesg because the
> ring buffer for sending messages to the Hyper-V host gets full -- Hyper-V
> doesn't always keep up, at least not on my local laptop where I'm
> testing:
>
> [12574.327615] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
> [12574.327684] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
> [12574.327760] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
> [12574.327841] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
> [12597.016128] hyperv_sendpacket: 6211 callbacks suppressed
> [12597.016133] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
> [12597.016172] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
> [12597.016220] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
> [12597.016267] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
>
> hyperv_drm could be fixed to not output the ugly messages, but there's
> still the underlying issue of overrunning the ring buffer, and excessively
> hammering on the host. If we could get hyperv_drm doing deferred I/O, I
> would feel much better about going full-on with deprecating hyperv_fb.
I try to address the problem with the patches at
https://lore.kernel.org/dri-devel/20250605152637.98493-1-tzimmermann@suse.de/
Testing and feedback is much appreciated.
Best regards
Thomas
>
> Michael
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply
* Re: [PATCH] firmware: smccc: support both conduits for getting hyp UUID
From: Sudeep Holla @ 2025-06-05 15:33 UTC (permalink / raw)
To: Anirudh Rayabharam
Cc: Mark Rutland, Sudeep Holla, Lorenzo Pieralisi, linux-hyperv,
linux-arm-kernel, linux-kernel
In-Reply-To: <20250521094049.960056-1-anirudh@anirudhrb.com>
(sorry for the delay, found the patch in the spam 🙁)
On Wed, May 21, 2025 at 09:40:48AM +0000, Anirudh Rayabharam wrote:
> From: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
>
> When Linux is running as the root partition under Microsoft Hypervisor
> (MSHV) a.k.a Hyper-V, smc is used as the conduit for smc calls.
>
> Extend arm_smccc_hypervisor_has_uuid() to support this usecase. Use
> arm_smccc_1_1_invoke to retrieve and use the appropriate conduit instead
> of supporting only hvc.
>
> Boot tested on MSHV guest, MSHV root & KVM guest.
>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Are they any dependent patches or series using this ? Do you plan to
route it via KVM tree if there are any dependency. Or else I can push
it through (arm-)soc tree. Let me know.
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH 6/6] drm/fb-helper: Synchronize dirty worker with vblank
From: Thomas Zimmermann @ 2025-06-05 15:24 UTC (permalink / raw)
To: mhklinux, maarten.lankhorst, mripard, airlied, simona,
drawat.floss, javierm, kraxel, louis.chauvet, hamohammed.sa,
melissa.srw, fvogt
Cc: dri-devel, linux-hyperv, virtualization, Thomas Zimmermann
In-Reply-To: <20250605152637.98493-1-tzimmermann@suse.de>
Before updating the display from the console's shadow buffer, the dirty
worker now waits for a vblank. This allows several screen updates to pile
up and acts as a rate limiter. If a DRM master is present, it could
interfere with the vblank. Don't wait in this case.
v3:
* add back helper->lock
* acquire DRM master status while waiting for vblank
v2:
* don't hold helper->lock while waiting for vblank
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_fb_helper.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 937c3939e502..e0b780634a84 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -364,9 +364,29 @@ static void drm_fb_helper_fb_dirty(struct drm_fb_helper *helper)
struct drm_device *dev = helper->dev;
struct drm_clip_rect *clip = &helper->damage_clip;
struct drm_clip_rect clip_copy;
+ struct drm_crtc *crtc;
unsigned long flags;
int ret;
+ /*
+ * Rate-limit update frequency to vblank. If there's a DRM master
+ * present, it could interfere while we're waiting for the vblank
+ * event. Don't wait in this case.
+ */
+ mutex_lock(&helper->lock);
+ if (!drm_master_internal_acquire(helper->dev)) {
+ goto unlock;
+ }
+ crtc = helper->client.modesets[0].crtc;
+ ret = drm_crtc_vblank_get(crtc);
+ if (!ret) {
+ drm_crtc_wait_one_vblank(crtc);
+ drm_crtc_vblank_put(crtc);
+ }
+ drm_master_internal_release(helper->dev);
+unlock:
+ mutex_unlock(&helper->lock);
+
if (drm_WARN_ON_ONCE(dev, !helper->funcs->fb_dirty))
return;
--
2.49.0
^ permalink raw reply related
* [PATCH 5/6] drm/hypervdrm: Use vblank timer
From: Thomas Zimmermann @ 2025-06-05 15:24 UTC (permalink / raw)
To: mhklinux, maarten.lankhorst, mripard, airlied, simona,
drawat.floss, javierm, kraxel, louis.chauvet, hamohammed.sa,
melissa.srw, fvogt
Cc: dri-devel, linux-hyperv, virtualization, Thomas Zimmermann
In-Reply-To: <20250605152637.98493-1-tzimmermann@suse.de>
HyperV's virtual hardware does not provide vblank interrupts. Use a
vblank timer to simulate the interrupt in software. Rate-limits the
display's update frequency to the display-mode settings. Avoids
excessive CPU overhead with compositors that do not rate-limit their
output.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/hyperv/hyperv_drm.h | 4 ++
drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 70 +++++++++++++++++++++
2 files changed, 74 insertions(+)
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm.h b/drivers/gpu/drm/hyperv/hyperv_drm.h
index 9e776112c03e..730e4a37545d 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm.h
+++ b/drivers/gpu/drm/hyperv/hyperv_drm.h
@@ -6,12 +6,16 @@
#ifndef _HYPERV_DRM_H_
#define _HYPERV_DRM_H_
+#include <drm/drm_vblank.h>
+#include <drm/drm_vblank_timer.h>
+
#define VMBUS_MAX_PACKET_SIZE 0x4000
struct hyperv_drm_device {
/* drm */
struct drm_device dev;
struct drm_plane plane;
+ struct drm_vblank_timer vtimer;
struct drm_crtc crtc;
struct drm_encoder encoder;
struct drm_connector connector;
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
index f7d2e973f79e..ed19e471b96f 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
@@ -97,6 +97,28 @@ static const uint64_t hyperv_modifiers[] = {
DRM_FORMAT_MOD_INVALID
};
+static void hyperv_crtc_helper_atomic_flush(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ struct drm_pending_vblank_event *event;
+
+ spin_lock_irq(&dev->event_lock);
+
+ event = crtc_state->event;
+ crtc_state->event = NULL;
+
+ if (event) {
+ if (drm_crtc_vblank_get(crtc) == 0)
+ drm_crtc_arm_vblank_event(crtc, event);
+ else
+ drm_crtc_send_vblank_event(crtc, event);
+ }
+
+ spin_unlock_irq(&dev->event_lock);
+}
+
static void hyperv_crtc_helper_atomic_enable(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
@@ -110,13 +132,49 @@ static void hyperv_crtc_helper_atomic_enable(struct drm_crtc *crtc,
crtc_state->mode.hdisplay,
crtc_state->mode.vdisplay,
plane_state->fb->pitches[0]);
+
+ drm_crtc_vblank_on(crtc);
+}
+
+static void hyperv_crtc_helper_atomic_disable(struct drm_crtc *crtc,
+ struct drm_atomic_state *crtc_state)
+{
+ drm_crtc_vblank_off(crtc);
}
static const struct drm_crtc_helper_funcs hyperv_crtc_helper_funcs = {
.atomic_check = drm_crtc_helper_atomic_check,
+ .atomic_flush = hyperv_crtc_helper_atomic_flush,
.atomic_enable = hyperv_crtc_helper_atomic_enable,
+ .atomic_disable = hyperv_crtc_helper_atomic_disable,
};
+static int hyperv_crtc_enable_vblank(struct drm_crtc *crtc)
+{
+ struct hyperv_drm_device *hv = to_hv(crtc->dev);
+
+ drm_vblank_timer_start(&hv->vtimer);
+
+ return 0;
+}
+
+static void hyperv_crtc_disable_vblank(struct drm_crtc *crtc)
+{
+ struct hyperv_drm_device *hv = to_hv(crtc->dev);
+
+ drm_vblank_timer_cancel(&hv->vtimer);
+}
+
+static bool hyperv_crtc_get_vblank_timestamp(struct drm_crtc *crtc,
+ int *max_error, ktime_t *vblank_time,
+ bool in_vblank_irq)
+{
+ struct hyperv_drm_device *hv = to_hv(crtc->dev);
+
+ return drm_vblank_timer_get_vblank_timestamp(&hv->vtimer, max_error,
+ vblank_time, in_vblank_irq);
+}
+
static const struct drm_crtc_funcs hyperv_crtc_funcs = {
.reset = drm_atomic_helper_crtc_reset,
.destroy = drm_crtc_cleanup,
@@ -124,6 +182,9 @@ static const struct drm_crtc_funcs hyperv_crtc_funcs = {
.page_flip = drm_atomic_helper_page_flip,
.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+ .enable_vblank = hyperv_crtc_enable_vblank,
+ .disable_vblank = hyperv_crtc_disable_vblank,
+ .get_vblank_timestamp = hyperv_crtc_get_vblank_timestamp,
};
static int hyperv_plane_atomic_check(struct drm_plane *plane,
@@ -285,6 +346,15 @@ int hyperv_mode_config_init(struct hyperv_drm_device *hv)
return ret;
}
+ /* Vertical blanking */
+
+ ret = drm_vblank_init(dev, 1);
+ if (ret)
+ return ret;
+ ret = drmm_vblank_timer_init(&hv->vtimer, &hv->crtc, NULL);
+ if (ret)
+ return ret;
+
drm_mode_config_reset(dev);
return 0;
--
2.49.0
^ permalink raw reply related
* [PATCH 4/6] drm/bochs: Use vblank timer
From: Thomas Zimmermann @ 2025-06-05 15:24 UTC (permalink / raw)
To: mhklinux, maarten.lankhorst, mripard, airlied, simona,
drawat.floss, javierm, kraxel, louis.chauvet, hamohammed.sa,
melissa.srw, fvogt
Cc: dri-devel, linux-hyperv, virtualization, Thomas Zimmermann
In-Reply-To: <20250605152637.98493-1-tzimmermann@suse.de>
Bochs' virtual hardware does not provide vblank interrupts. Use a
vblank timer to simulate the interrupt in software. Rate-limits the
display's update frequency to the display-mode settings. Avoids
excessive CPU overhead with compositors that do not rate-limit their
output.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/tiny/bochs.c | 68 ++++++++++++++++++++++++++++++++++++
1 file changed, 68 insertions(+)
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 8706763af8fb..dbc6a15c1a58 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -21,6 +21,8 @@
#include <drm/drm_module.h>
#include <drm/drm_plane_helper.h>
#include <drm/drm_probe_helper.h>
+#include <drm/drm_vblank.h>
+#include <drm/drm_vblank_timer.h>
#include <video/vga.h>
@@ -95,6 +97,7 @@ struct bochs_device {
/* drm */
struct drm_plane primary_plane;
+ struct drm_vblank_timer vtimer;
struct drm_crtc crtc;
struct drm_encoder encoder;
struct drm_connector connector;
@@ -501,12 +504,35 @@ static int bochs_crtc_helper_atomic_check(struct drm_crtc *crtc,
return drm_atomic_helper_check_crtc_primary_plane(crtc_state);
}
+static void bochs_crtc_helper_atomic_flush(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ struct drm_pending_vblank_event *event;
+
+ spin_lock_irq(&dev->event_lock);
+
+ event = crtc_state->event;
+ crtc_state->event = NULL;
+
+ if (event) {
+ if (drm_crtc_vblank_get(crtc) == 0)
+ drm_crtc_arm_vblank_event(crtc, event);
+ else
+ drm_crtc_send_vblank_event(crtc, event);
+ }
+
+ spin_unlock_irq(&dev->event_lock);
+}
+
static void bochs_crtc_helper_atomic_enable(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
struct bochs_device *bochs = to_bochs_device(crtc->dev);
bochs_hw_blank(bochs, false);
+ drm_crtc_vblank_on(crtc);
}
static void bochs_crtc_helper_atomic_disable(struct drm_crtc *crtc,
@@ -514,16 +540,44 @@ static void bochs_crtc_helper_atomic_disable(struct drm_crtc *crtc,
{
struct bochs_device *bochs = to_bochs_device(crtc->dev);
+ drm_crtc_vblank_off(crtc);
bochs_hw_blank(bochs, true);
}
static const struct drm_crtc_helper_funcs bochs_crtc_helper_funcs = {
.mode_set_nofb = bochs_crtc_helper_mode_set_nofb,
.atomic_check = bochs_crtc_helper_atomic_check,
+ .atomic_flush = bochs_crtc_helper_atomic_flush,
.atomic_enable = bochs_crtc_helper_atomic_enable,
.atomic_disable = bochs_crtc_helper_atomic_disable,
};
+static int bochs_crtc_enable_vblank(struct drm_crtc *crtc)
+{
+ struct bochs_device *bochs = to_bochs_device(crtc->dev);
+
+ drm_vblank_timer_start(&bochs->vtimer);
+
+ return 0;
+}
+
+static void bochs_crtc_disable_vblank(struct drm_crtc *crtc)
+{
+ struct bochs_device *bochs = to_bochs_device(crtc->dev);
+
+ drm_vblank_timer_cancel(&bochs->vtimer);
+}
+
+static bool bochs_crtc_get_vblank_timestamp(struct drm_crtc *crtc,
+ int *max_error, ktime_t *vblank_time,
+ bool in_vblank_irq)
+{
+ struct bochs_device *bochs = to_bochs_device(crtc->dev);
+
+ return drm_vblank_timer_get_vblank_timestamp(&bochs->vtimer, max_error,
+ vblank_time, in_vblank_irq);
+}
+
static const struct drm_crtc_funcs bochs_crtc_funcs = {
.reset = drm_atomic_helper_crtc_reset,
.destroy = drm_crtc_cleanup,
@@ -531,6 +585,9 @@ static const struct drm_crtc_funcs bochs_crtc_funcs = {
.page_flip = drm_atomic_helper_page_flip,
.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+ .enable_vblank = bochs_crtc_enable_vblank,
+ .disable_vblank = bochs_crtc_disable_vblank,
+ .get_vblank_timestamp = bochs_crtc_get_vblank_timestamp,
};
static const struct drm_encoder_funcs bochs_encoder_funcs = {
@@ -602,6 +659,7 @@ static int bochs_kms_init(struct bochs_device *bochs)
struct drm_crtc *crtc;
struct drm_connector *connector;
struct drm_encoder *encoder;
+ struct drm_vblank_timer *vtimer;
int ret;
ret = drmm_mode_config_init(dev);
@@ -651,6 +709,16 @@ static int bochs_kms_init(struct bochs_device *bochs)
drm_connector_attach_edid_property(connector);
drm_connector_attach_encoder(connector, encoder);
+ /* Vertical blanking */
+
+ ret = drm_vblank_init(dev, 1);
+ if (ret)
+ return ret;
+ vtimer = &bochs->vtimer;
+ ret = drmm_vblank_timer_init(vtimer, crtc, NULL);
+ if (ret)
+ return ret;
+
drm_mode_config_reset(dev);
return 0;
--
2.49.0
^ permalink raw reply related
* [PATCH 3/6] drm/simpledrm: Use vblank timer
From: Thomas Zimmermann @ 2025-06-05 15:24 UTC (permalink / raw)
To: mhklinux, maarten.lankhorst, mripard, airlied, simona,
drawat.floss, javierm, kraxel, louis.chauvet, hamohammed.sa,
melissa.srw, fvogt
Cc: dri-devel, linux-hyperv, virtualization, Thomas Zimmermann
In-Reply-To: <20250605152637.98493-1-tzimmermann@suse.de>
Simpledrm hardware does not provide vblank interrupts. Use a vblank
timer to simulate the inerrup tin software. Rate-limits the display's
update frequency to the display-mode settings.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/sysfb/simpledrm.c | 81 +++++++++++++++++++++++++++++++
1 file changed, 81 insertions(+)
diff --git a/drivers/gpu/drm/sysfb/simpledrm.c b/drivers/gpu/drm/sysfb/simpledrm.c
index a1c3119330de..b9766129f564 100644
--- a/drivers/gpu/drm/sysfb/simpledrm.c
+++ b/drivers/gpu/drm/sysfb/simpledrm.c
@@ -26,6 +26,8 @@
#include <drm/drm_managed.h>
#include <drm/drm_modeset_helper_vtables.h>
#include <drm/drm_probe_helper.h>
+#include <drm/drm_vblank.h>
+#include <drm/drm_vblank_timer.h>
#include "drm_sysfb_helper.h"
@@ -229,11 +231,17 @@ struct simpledrm_device {
/* modesetting */
u32 formats[DRM_SYSFB_PLANE_NFORMATS(1)];
struct drm_plane primary_plane;
+ struct drm_vblank_timer vtimer;
struct drm_crtc crtc;
struct drm_encoder encoder;
struct drm_connector connector;
};
+static struct simpledrm_device *to_simpledrm_device(struct drm_device *dev)
+{
+ return container_of(to_drm_sysfb_device(dev), struct simpledrm_device, sysfb);
+}
+
/*
* Hardware
*/
@@ -564,13 +572,79 @@ static const struct drm_plane_funcs simpledrm_primary_plane_funcs = {
.destroy = drm_plane_cleanup,
};
+static void simpledrm_crtc_helper_atomic_flush(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ struct drm_pending_vblank_event *event;
+
+ spin_lock_irq(&dev->event_lock);
+
+ event = crtc_state->event;
+ crtc_state->event = NULL;
+
+ if (event) {
+ if (drm_crtc_vblank_get(crtc) == 0)
+ drm_crtc_arm_vblank_event(crtc, event);
+ else
+ drm_crtc_send_vblank_event(crtc, event);
+ }
+
+ spin_unlock_irq(&dev->event_lock);
+}
+
+static void simpledrm_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ drm_crtc_vblank_on(crtc);
+}
+
+static void simpledrm_crtc_helper_atomic_disable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ drm_crtc_vblank_off(crtc);
+}
+
static const struct drm_crtc_helper_funcs simpledrm_crtc_helper_funcs = {
DRM_SYSFB_CRTC_HELPER_FUNCS,
+ .atomic_flush = simpledrm_crtc_helper_atomic_flush,
+ .atomic_enable = simpledrm_crtc_helper_atomic_enable,
+ .atomic_disable = simpledrm_crtc_helper_atomic_disable,
};
+static int simpledrm_crtc_enable_vblank(struct drm_crtc *crtc)
+{
+ struct simpledrm_device *sdev = to_simpledrm_device(crtc->dev);
+
+ drm_vblank_timer_start(&sdev->vtimer);
+
+ return 0;
+}
+
+static void simpledrm_crtc_disable_vblank(struct drm_crtc *crtc)
+{
+ struct simpledrm_device *sdev = to_simpledrm_device(crtc->dev);
+
+ drm_vblank_timer_cancel(&sdev->vtimer);
+}
+
+static bool simpledrm_crtc_get_vblank_timestamp(struct drm_crtc *crtc,
+ int *max_error, ktime_t *vblank_time,
+ bool in_vblank_irq)
+{
+ struct simpledrm_device *sdev = to_simpledrm_device(crtc->dev);
+
+ return drm_vblank_timer_get_vblank_timestamp(&sdev->vtimer, max_error,
+ vblank_time, in_vblank_irq);
+}
+
static const struct drm_crtc_funcs simpledrm_crtc_funcs = {
DRM_SYSFB_CRTC_FUNCS,
.destroy = drm_crtc_cleanup,
+ .enable_vblank = simpledrm_crtc_enable_vblank,
+ .disable_vblank = simpledrm_crtc_disable_vblank,
+ .get_vblank_timestamp = simpledrm_crtc_get_vblank_timestamp,
};
static const struct drm_encoder_funcs simpledrm_encoder_funcs = {
@@ -611,6 +685,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
struct drm_crtc *crtc;
struct drm_encoder *encoder;
struct drm_connector *connector;
+ struct drm_vblank_timer *vtimer;
unsigned long max_width, max_height;
size_t nformats;
int ret;
@@ -812,6 +887,12 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
if (ret)
return ERR_PTR(ret);
+ /* Vertical blanking */
+
+ vtimer = &sdev->vtimer;
+ drmm_vblank_timer_init(vtimer, crtc, NULL);
+ drm_vblank_init(dev, 1);
+
drm_mode_config_reset(dev);
return sdev;
--
2.49.0
^ permalink raw reply related
* [RFC PATCH 0/6] drm: Add vblank timers for devices without interrupts
From: Thomas Zimmermann @ 2025-06-05 15:24 UTC (permalink / raw)
To: mhklinux, maarten.lankhorst, mripard, airlied, simona,
drawat.floss, javierm, kraxel, louis.chauvet, hamohammed.sa,
melissa.srw, fvogt
Cc: dri-devel, linux-hyperv, virtualization, Thomas Zimmermann
Compositors often depend on vblanks to limit their display-update
rate. Without, they see vblank events ASAP, which creates high CPU
overhead. This is especially a problem with virtual devices with fast
framebuffer access.
The series moves vkms' vblank timer to DRM and converts a number of
drivers. The final patch rate-limits the output of DRM's framebuffer
console to vblank intervals. It has been taken from [1]. It reduces
the time for doing
time find /usr/src/linux
from 24s to 20s with the patched bochs driver. Apparently the system
is spending less CPU overhead on display updates.
This is an RFC patchset to see if the approach is feasible. It's been
motivated by a recent discussion about hypervdrm [2] and other long-
standing bug reports. [3][4]
[1] https://patchwork.freedesktop.org/series/66442/
[2] https://lore.kernel.org/dri-devel/20250523161522.409504-1-mhklinux@outlook.com/T/#ma2ebb52b60bfb0325879349377738fadcd7cb7ef
[3] https://bugzilla.suse.com/show_bug.cgi?id=1189174
[4] https://invent.kde.org/plasma/kwin/-/merge_requests/1229#note_284606
Thomas Zimmermann (6):
drm/vblank: Add vblank timer
drm/vkms: Use vblank timer
drm/simpledrm: Use vblank timer
drm/bochs: Use vblank timer
drm/hypervdrm: Use vblank timer
drm/fb-helper: Synchronize dirty worker with vblank
drivers/gpu/drm/Makefile | 3 +-
drivers/gpu/drm/drm_fb_helper.c | 20 ++++
drivers/gpu/drm/drm_vblank_timer.c | 100 ++++++++++++++++++++
drivers/gpu/drm/hyperv/hyperv_drm.h | 4 +
drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 70 ++++++++++++++
drivers/gpu/drm/sysfb/simpledrm.c | 81 ++++++++++++++++
drivers/gpu/drm/tiny/bochs.c | 68 +++++++++++++
drivers/gpu/drm/vkms/vkms_crtc.c | 49 +++-------
drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
include/drm/drm_vblank_timer.h | 26 +++++
10 files changed, 386 insertions(+), 41 deletions(-)
create mode 100644 drivers/gpu/drm/drm_vblank_timer.c
create mode 100644 include/drm/drm_vblank_timer.h
--
2.49.0
^ permalink raw reply
* [PATCH 2/6] drm/vkms: Use vblank timer
From: Thomas Zimmermann @ 2025-06-05 15:24 UTC (permalink / raw)
To: mhklinux, maarten.lankhorst, mripard, airlied, simona,
drawat.floss, javierm, kraxel, louis.chauvet, hamohammed.sa,
melissa.srw, fvogt
Cc: dri-devel, linux-hyperv, virtualization, Thomas Zimmermann
In-Reply-To: <20250605152637.98493-1-tzimmermann@suse.de>
Replace vkms' vblank timer with the DRM implementation. The DRM
code is mostly identical.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/vkms/vkms_crtc.c | 49 +++++++-------------------------
drivers/gpu/drm/vkms/vkms_drv.h | 6 ++--
2 files changed, 15 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 8c9898b9055d..5b7829e8c900 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -10,22 +10,14 @@
#include "vkms_drv.h"
-static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
+static bool vkms_crtc_handle_vblank(struct drm_crtc *crtc)
{
- struct vkms_output *output = container_of(timer, struct vkms_output,
- vblank_hrtimer);
- struct drm_crtc *crtc = &output->crtc;
+ struct vkms_output *output = drm_crtc_to_vkms_output(crtc);
struct vkms_crtc_state *state;
- u64 ret_overrun;
bool ret, fence_cookie;
fence_cookie = dma_fence_begin_signalling();
- ret_overrun = hrtimer_forward_now(&output->vblank_hrtimer,
- output->period_ns);
- if (ret_overrun != 1)
- pr_warn("%s: vblank timer overrun\n", __func__);
-
spin_lock(&output->lock);
ret = drm_crtc_handle_vblank(crtc);
if (!ret)
@@ -57,18 +49,14 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
dma_fence_end_signalling(fence_cookie);
- return HRTIMER_RESTART;
+ return true;
}
static int vkms_enable_vblank(struct drm_crtc *crtc)
{
- struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
- hrtimer_setup(&out->vblank_hrtimer, &vkms_vblank_simulate, CLOCK_MONOTONIC,
- HRTIMER_MODE_REL);
- out->period_ns = ktime_set(0, vblank->framedur_ns);
- hrtimer_start(&out->vblank_hrtimer, out->period_ns, HRTIMER_MODE_REL);
+ drm_vblank_timer_start(&out->vtimer);
return 0;
}
@@ -77,7 +65,7 @@ static void vkms_disable_vblank(struct drm_crtc *crtc)
{
struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
- hrtimer_cancel(&out->vblank_hrtimer);
+ drm_vblank_timer_cancel(&out->vtimer);
}
static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
@@ -85,28 +73,9 @@ static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
bool in_vblank_irq)
{
struct vkms_output *output = drm_crtc_to_vkms_output(crtc);
- struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
-
- if (!READ_ONCE(vblank->enabled)) {
- *vblank_time = ktime_get();
- return true;
- }
-
- *vblank_time = READ_ONCE(output->vblank_hrtimer.node.expires);
- if (WARN_ON(*vblank_time == vblank->time))
- return true;
-
- /*
- * To prevent races we roll the hrtimer forward before we do any
- * interrupt processing - this is how real hw works (the interrupt is
- * only generated after all the vblank registers are updated) and what
- * the vblank core expects. Therefore we need to always correct the
- * timestampe by one frame.
- */
- *vblank_time -= output->period_ns;
-
- return true;
+ return drm_vblank_timer_get_vblank_timestamp(&output->vtimer, max_error,
+ vblank_time, in_vblank_irq);
}
static struct drm_crtc_state *
@@ -274,6 +243,7 @@ struct vkms_output *vkms_crtc_init(struct drm_device *dev, struct drm_plane *pri
{
struct vkms_output *vkms_out;
struct drm_crtc *crtc;
+ struct drm_vblank_timer *vtimer;
int ret;
vkms_out = drmm_crtc_alloc_with_planes(dev, struct vkms_output, crtc,
@@ -285,6 +255,7 @@ struct vkms_output *vkms_crtc_init(struct drm_device *dev, struct drm_plane *pri
}
crtc = &vkms_out->crtc;
+ vtimer = &vkms_out->vtimer;
drm_crtc_helper_add(crtc, &vkms_crtc_helper_funcs);
@@ -305,5 +276,7 @@ struct vkms_output *vkms_crtc_init(struct drm_device *dev, struct drm_plane *pri
if (!vkms_out->composer_workq)
return ERR_PTR(-ENOMEM);
+ drmm_vblank_timer_init(vtimer, crtc, vkms_crtc_handle_vblank);
+
return vkms_out;
}
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index a74a7fc3a056..126016898285 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -10,6 +10,7 @@
#include <drm/drm_gem.h>
#include <drm/drm_gem_atomic_helper.h>
#include <drm/drm_encoder.h>
+#include <drm/drm_vblank_timer.h>
#include <drm/drm_writeback.h>
#define DEFAULT_DEVICE_NAME "vkms"
@@ -180,8 +181,9 @@ struct vkms_output {
struct drm_crtc crtc;
struct drm_writeback_connector wb_connector;
struct drm_encoder wb_encoder;
- struct hrtimer vblank_hrtimer;
- ktime_t period_ns;
+ struct drm_vblank_timer vtimer;
+ struct drm_pending_vblank_event *event;
+ /* ordered wq for composer_work */
struct workqueue_struct *composer_workq;
spinlock_t lock;
--
2.49.0
^ permalink raw reply related
* [PATCH 1/6] drm/vblank: Add vblank timer
From: Thomas Zimmermann @ 2025-06-05 15:24 UTC (permalink / raw)
To: mhklinux, maarten.lankhorst, mripard, airlied, simona,
drawat.floss, javierm, kraxel, louis.chauvet, hamohammed.sa,
melissa.srw, fvogt
Cc: dri-devel, linux-hyperv, virtualization, Thomas Zimmermann
In-Reply-To: <20250605152637.98493-1-tzimmermann@suse.de>
The vblank timer simulates a vblank interrupt for hardware without
support. Rate-limits the display update frequency.
DRM drivers for hardware without vblank support apply display updates
ASAP. A vblank event informs DRM clients of the completed update.
Userspace compositors immediately schedule the next update, which
creates significant load on virtualization outputs. Display updates
are usually fast on virtualization outputs, as their framebuffers are
in regular system memory and there's no hardware vblank interrupt to
throttle the update rate.
The vblank timer is a HR timer that signals the vblank in software.
It limits the update frequency of a DRM driver similar to a hardware
vblank interrupt. The timer is not synchronized to the actual vblank
interval of the display.
The code has been adopted from vkms, which added the funtionality
in commit 3a0709928b17 ("drm/vkms: Add vblank events simulated by
hrtimers").
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/Makefile | 3 +-
drivers/gpu/drm/drm_vblank_timer.c | 100 +++++++++++++++++++++++++++++
include/drm/drm_vblank_timer.h | 26 ++++++++
3 files changed, 128 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/drm_vblank_timer.c
create mode 100644 include/drm/drm_vblank_timer.h
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index b5d5561bbe5f..6722e2d1aa7e 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -146,7 +146,8 @@ drm_kms_helper-y := \
drm_plane_helper.o \
drm_probe_helper.o \
drm_self_refresh_helper.o \
- drm_simple_kms_helper.o
+ drm_simple_kms_helper.o \
+ drm_vblank_timer.o
drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
diff --git a/drivers/gpu/drm/drm_vblank_timer.c b/drivers/gpu/drm/drm_vblank_timer.c
new file mode 100644
index 000000000000..be46d3135c8e
--- /dev/null
+++ b/drivers/gpu/drm/drm_vblank_timer.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <linux/hrtimer.h>
+
+#include <drm/drm_crtc.h>
+#include <drm/drm_managed.h>
+#include <drm/drm_print.h>
+#include <drm/drm_vblank.h>
+#include <drm/drm_vblank_timer.h>
+
+static enum hrtimer_restart drm_vblank_timer_function(struct hrtimer *timer)
+{
+ struct drm_vblank_timer *vtimer = container_of(timer, struct drm_vblank_timer, timer);
+ struct drm_crtc *crtc = vtimer->crtc;
+ struct drm_device *dev = crtc->dev;
+ u64 ret_overrun;
+ bool succ;
+
+ ret_overrun = hrtimer_forward_now(&vtimer->timer, vtimer->period_ns);
+ if (ret_overrun != 1)
+ drm_warn(dev, "vblank timer overrun\n");
+
+ if (vtimer->crtc_handle_vblank)
+ succ = vtimer->crtc_handle_vblank(crtc);
+ else
+ succ = drm_crtc_handle_vblank(crtc);
+ if (!succ)
+ return HRTIMER_NORESTART;
+
+ return HRTIMER_RESTART;
+}
+
+static void drmm_vblank_timer_release(struct drm_device *dev, void *res)
+{
+ struct drm_vblank_timer *vtimer = res;
+
+ hrtimer_cancel(&vtimer->timer);
+}
+
+int drmm_vblank_timer_init(struct drm_vblank_timer *vtimer, struct drm_crtc *crtc,
+ bool (*crtc_handle_vblank)(struct drm_crtc *crtc))
+{
+ struct hrtimer *timer = &vtimer->timer;
+
+ vtimer->crtc = crtc;
+ vtimer->crtc_handle_vblank = crtc_handle_vblank;
+
+ hrtimer_setup(timer, drm_vblank_timer_function, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+
+ return drmm_add_action_or_reset(crtc->dev, drmm_vblank_timer_release, vtimer);
+}
+EXPORT_SYMBOL(drmm_vblank_timer_init);
+
+void drm_vblank_timer_start(struct drm_vblank_timer *vtimer)
+{
+ struct drm_crtc *crtc = vtimer->crtc;
+ struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
+
+ drm_calc_timestamping_constants(crtc, &crtc->mode);
+
+ vtimer->period_ns = ktime_set(0, vblank->framedur_ns);
+ hrtimer_start(&vtimer->timer, vtimer->period_ns, HRTIMER_MODE_REL);
+}
+EXPORT_SYMBOL(drm_vblank_timer_start);
+
+void drm_vblank_timer_cancel(struct drm_vblank_timer *vtimer)
+{
+ hrtimer_cancel(&vtimer->timer);
+}
+EXPORT_SYMBOL(drm_vblank_timer_cancel);
+
+bool drm_vblank_timer_get_vblank_timestamp(struct drm_vblank_timer *vtimer,
+ int *max_error, ktime_t *vblank_time,
+ bool in_vblank_irq)
+{
+ struct drm_crtc *crtc = vtimer->crtc;
+ struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
+
+ if (!READ_ONCE(vblank->enabled)) {
+ *vblank_time = ktime_get();
+ return true;
+ }
+
+ *vblank_time = READ_ONCE(vtimer->timer.node.expires);
+
+ if (WARN_ON(*vblank_time == vblank->time))
+ return true;
+
+ /*
+ * To prevent races we roll the hrtimer forward before we do any
+ * interrupt processing - this is how real hw works (the interrupt is
+ * only generated after all the vblank registers are updated) and what
+ * the vblank core expects. Therefore we need to always correct the
+ * timestampe by one frame.
+ */
+ *vblank_time -= vtimer->period_ns;
+
+ return true;
+}
+EXPORT_SYMBOL(drm_vblank_timer_get_vblank_timestamp);
diff --git a/include/drm/drm_vblank_timer.h b/include/drm/drm_vblank_timer.h
new file mode 100644
index 000000000000..0b827ff1f59c
--- /dev/null
+++ b/include/drm/drm_vblank_timer.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef _DRM_VBLANK_TIMER_H_
+#define _DRM_VBLANK_TIMER_H_
+
+#include <linux/hrtimer_types.h>
+#include <linux/types.h>
+
+struct drm_crtc;
+
+struct drm_vblank_timer {
+ struct drm_crtc *crtc;
+ bool (*crtc_handle_vblank)(struct drm_crtc *crtc);
+ ktime_t period_ns;
+ struct hrtimer timer;
+};
+
+int drmm_vblank_timer_init(struct drm_vblank_timer *vtimer, struct drm_crtc *crtc,
+ bool (*handle_vblank)(struct drm_crtc *crtc));
+void drm_vblank_timer_start(struct drm_vblank_timer *vtimer);
+void drm_vblank_timer_cancel(struct drm_vblank_timer *vtimer);
+bool drm_vblank_timer_get_vblank_timestamp(struct drm_vblank_timer *vtimer,
+ int *max_error, ktime_t *vblank_time,
+ bool in_vblank_irq);
+
+#endif
--
2.49.0
^ permalink raw reply related
* Re: [PATCH v3] vmbus: retrieve connection-id from DeviceTree
From: Krzysztof Kozlowski @ 2025-06-05 14:34 UTC (permalink / raw)
To: Hardik Garg, kys, haiyangz, wei.liu, decui, ssengar
Cc: romank, linux-hyperv, linux-kernel, stable, apais
In-Reply-To: <6a92ca86-ad6b-4d49-af6e-1ed7651b8ab8@linux.microsoft.com>
On 05/06/2025 08:09, Hardik Garg wrote:
> The connection-id determines which hypervisor communication channel the
> guest should use to talk to the VMBus host. This patch adds support to
> read this value from the DeviceTree where it exists as a property under
> the vmbus node with the compatible ID "microsoft,message-connection-id".
> The property name follows the format <vendor>,<field> where
> "vendor": "microsoft" and "field": "message-connection-id"
>
> Reading from DeviceTree allows platforms to specify their preferred
> communication channel, making it more flexible. If the property is
> not found in the DeviceTree, use the default connection ID
> (VMBUS_MESSAGE_CONNECTION_ID or VMBUS_MESSAGE_CONNECTION_ID_4
> based on protocol version).
>
> Signed-off-by: Hardik Garg <hargar@linux.microsoft.com>
> ---
> v3:
> - Added documentation for the new property in DeviceTree bindings
Please run scripts/checkpatch.pl on the patches and fix reported
warnings. After that, run also 'scripts/checkpatch.pl --strict' on the
patches and (probably) fix more warnings. Some warnings can be ignored,
especially from --strict run, but the code here looks like it needs a
fix. Feel free to get in touch if the warning is not clear.
<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.
Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.
You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time.
Please kindly resend and include all necessary To/Cc entries.
</form letter>
>
> v2:
> https://lore.kernel.org/all/096edaf7-cc90-42b6-aff4-c5f088574e1e@linux.microsoft.com/
>
> v1:
> https://lore.kernel.org/all/6acee4bf-cb04-43b9-9476-e8d811d26dfd@linux.microsoft.com/
> ---
> .../devicetree/bindings/bus/microsoft,vmbus.yaml | 8 ++++++++
> drivers/hv/connection.c | 6 ++++--
> drivers/hv/vmbus_drv.c | 13 +++++++++++++
> 3 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
> b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
> index 0bea4f5287ce..729ca6defec6 100644
> --- a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
> +++ b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
> @@ -17,6 +17,14 @@ properties:
> compatible:
> const: microsoft,vmbus
>
> + microsoft,message-connection-id:
> + description:
> + VMBus message connection ID to be used for communication between
> host and
Malformed email patch. Don't use Microsoft tools to send emails... They
don't work outside of Microsoft. :/
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH RFC drm-misc-next v2 1/1] drm/hyperv: Add support for drm_panic
From: Jocelyn Falempe @ 2025-06-05 13:02 UTC (permalink / raw)
To: Ryosuke Yasuoka, drawat.floss, maarten.lankhorst, mripard,
tzimmermann, airlied, simona
Cc: linux-hyperv, linux-kernel, dri-devel
In-Reply-To: <20250526090117.80593-2-ryasuoka@redhat.com>
On 26/05/2025 11:01, Ryosuke Yasuoka wrote:
> Add drm_panic module for hyperv drm so that panic screen can be
> displayed on panic.
I've just pushed it to drm-misc-next.
Thanks for your contribution.
--
Jocelyn
>
> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> ---
> drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 36 +++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
> index f7d2e973f79e..945b9482bcb3 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
> @@ -17,6 +17,7 @@
> #include <drm/drm_gem_framebuffer_helper.h>
> #include <drm/drm_gem_shmem_helper.h>
> #include <drm/drm_probe_helper.h>
> +#include <drm/drm_panic.h>
> #include <drm/drm_plane.h>
>
> #include "hyperv_drm.h"
> @@ -181,10 +182,45 @@ static void hyperv_plane_atomic_update(struct drm_plane *plane,
> }
> }
>
> +static int hyperv_plane_get_scanout_buffer(struct drm_plane *plane,
> + struct drm_scanout_buffer *sb)
> +{
> + struct hyperv_drm_device *hv = to_hv(plane->dev);
> + struct iosys_map map = IOSYS_MAP_INIT_VADDR_IOMEM(hv->vram);
> +
> + if (plane->state && plane->state->fb) {
> + sb->format = plane->state->fb->format;
> + sb->width = plane->state->fb->width;
> + sb->height = plane->state->fb->height;
> + sb->pitch[0] = plane->state->fb->pitches[0];
> + sb->map[0] = map;
> + return 0;
> + }
> + return -ENODEV;
> +}
> +
> +static void hyperv_plane_panic_flush(struct drm_plane *plane)
> +{
> + struct hyperv_drm_device *hv = to_hv(plane->dev);
> + struct drm_rect rect;
> +
> + if (!plane->state || !plane->state->fb)
> + return;
> +
> + rect.x1 = 0;
> + rect.y1 = 0;
> + rect.x2 = plane->state->fb->width;
> + rect.y2 = plane->state->fb->height;
> +
> + hyperv_update_dirt(hv->hdev, &rect);
> +}
> +
> static const struct drm_plane_helper_funcs hyperv_plane_helper_funcs = {
> DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
> .atomic_check = hyperv_plane_atomic_check,
> .atomic_update = hyperv_plane_atomic_update,
> + .get_scanout_buffer = hyperv_plane_get_scanout_buffer,
> + .panic_flush = hyperv_plane_panic_flush,
> };
>
> static const struct drm_plane_funcs hyperv_plane_funcs = {
^ permalink raw reply
* Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: David Hildenbrand @ 2025-06-05 8:10 UTC (permalink / raw)
To: Michael Kelley, simona@ffwll.ch, deller@gmx.de,
haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, akpm@linux-foundation.org
Cc: weh@microsoft.com, tzimmermann@suse.de, hch@lst.de,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
linux-mm@kvack.org
In-Reply-To: <SN6PR02MB41574078A6785C3E2E1A6391D46CA@SN6PR02MB4157.namprd02.prod.outlook.com>
On 04.06.25 23:58, Michael Kelley wrote:
> From: Michael Kelley <mhklinux@outlook.com> Sent: Tuesday, June 3, 2025 10:25 AM
>>
>> From: David Hildenbrand <david@redhat.com> Sent: Tuesday, June 3, 2025 12:55 AM
>>>
>>> On 03.06.25 03:49, Michael Kelley wrote:
>>>> From: David Hildenbrand <david@redhat.com> Sent: Monday, June 2, 2025 2:48 AM
>>>>>
>
> [snip]
>
>>>>>> @@ -182,20 +221,34 @@ static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long
>>>>>> }
>>>>>>
>>>>>> /*
>>>>>> - * We want the page to remain locked from ->page_mkwrite until
>>>>>> - * the PTE is marked dirty to avoid mapping_wrprotect_range()
>>>>>> - * being called before the PTE is updated, which would leave
>>>>>> - * the page ignored by defio.
>>>>>> - * Do this by locking the page here and informing the caller
>>>>>> - * about it with VM_FAULT_LOCKED.
>>>>>> + * The PTE must be marked writable before the defio deferred work runs
>>>>>> + * again and potentially marks the PTE write-protected. If the order
>>>>>> + * should be switched, the PTE would become writable without defio
>>>>>> + * tracking the page, leaving the page forever ignored by defio.
>>>>>> + *
>>>>>> + * For vmalloc() framebuffers, the associated struct page is locked
>>>>>> + * before releasing the defio lock. mm will later mark the PTE writaable
>>>>>> + * and release the struct page lock. The struct page lock prevents
>>>>>> + * the page from being prematurely being marked write-protected.
>>>>>> + *
>>>>>> + * For FBINFO_KMEMFB framebuffers, mm assumes there is no struct page,
>>>>>> + * so the PTE must be marked writable while the defio lock is held.
>>>>>> */
>>>>>> - lock_page(pageref->page);
>>>>>> + if (info->flags & FBINFO_KMEMFB) {
>>>>>> + unsigned long pfn = page_to_pfn(pageref->page);
>>>>>> +
>>>>>> + ret = vmf_insert_mixed_mkwrite(vmf->vma, vmf->address,
>>>>>> + __pfn_to_pfn_t(pfn, PFN_SPECIAL));
>>>>>
>>>>> Will the VMA have VM_PFNMAP or VM_MIXEDMAP set? PFN_SPECIAL is a
>>>>> horrible hack.
>>>>>
>>>>> In another thread, you mention that you use PFN_SPECIAL to bypass the
>>>>> check in vm_mixed_ok(), so VM_MIXEDMAP is likely not set?
>>>>
>>>> The VMA has VM_PFNMAP set, not VM_MIXEDMAP. It seemed like
>>>> VM_MIXEDMAP is somewhat of a superset of VM_PFNMAP, but maybe that's
>>>> a wrong impression.
>>>
>>> VM_PFNMAP: nothing is refcounted except anon pages
>>>
>>> VM_MIXEDMAP: anything with a "struct page" (pfn_valid()) is refcounted
>>>
>>> pte_special() is a way for GUP-fast to distinguish these refcounted (can
>>> GUP) from non-refcounted (camnnot GUP) pages mapped by PTEs without any
>>> locks or the VMA being available.
>>>
>>> Setting pte_special() in VM_MIXEDMAP on ptes that have a "struct page"
>>> (pfn_valid()) is likely very bogus.
>>
>> OK, good to know.
>>
>>>
>>>> vm_mixed_ok() does a thorough job of validating the
>>>> use of __vm_insert_mixed(), and since what I did was allowed, I thought
>>>> perhaps it was OK. Your feedback has set me straight, and that's what I
>>>> needed. :-)
>>>
>>> What exactly are you trying to achieve? :)
>>>
>>> If it's mapping a page with a "struct page" and *not* refcounting it,
>>> then vmf_insert_pfn() is the current way to achieve that in a VM_PFNMAP
>>> mapping. It will set pte_special() automatically for you.
>>>
>>
>> Yes, that's what I'm using to initially create the special PTE in the
>> .fault callback.
>>
>>>>
>>>> But the whole approach is moot with Alistair Popple's patch set that
>>>> eliminates pfn_t. Is there an existing mm API that will do mkwrite on a
>>>> special PTE in a VM_PFNMAP VMA? I didn't see one, but maybe I missed
>>>> it. If there's not one, I'll take a crack at adding it in the next version of my
>>>> patch set.
>>>
>>> I assume you'd want vmf_insert_pfn_mkwrite(), correct? Probably
>>> vmf_insert_pfn_prot() can be used by adding PAGE_WRITE to pgprot. (maybe
>>> :) )
>>
>> Ok, I'll look at that more closely. The sequence is that the special
>> PTE gets created with vmf_insert_pfn(). Then when the page is first
>> written to, the .pfn_mkwrite callback is invoked by mm. The question
>> is the best way for that callback to mark the existing PTE as writable.
>>
>
> FWIW, vmf_insert_pfn_prot() won't work. It calls insert_pfn() with
> the "mkwrite" parameter set to 'false', in which case insert_pfn()
> does nothing if the PTE already exists.
Ah, you are worried about the "already exists but is R/O case".
>
> So I would need to create a new API that does appropriate validation
> for a VM_PFNMAP VMA, and then calls insert_pfn() with the "mkwrite"
> parameter set to 'true'.
IMHO, nothing would speak against vmf_insert_pfn_mkwrite().
Much better than using that "mixed" ... beauty of a function.
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Thomas Zimmermann @ 2025-06-05 7:55 UTC (permalink / raw)
To: Michael Kelley, Simona Vetter
Cc: David Hildenbrand, simona@ffwll.ch, deller@gmx.de,
haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, akpm@linux-foundation.org, weh@microsoft.com,
hch@lst.de, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, linux-mm@kvack.org
In-Reply-To: <SN6PR02MB415702B00D6D52B0EE962C98D46CA@SN6PR02MB4157.namprd02.prod.outlook.com>
Hi
Am 04.06.25 um 23:43 schrieb Michael Kelley:
> From: Simona Vetter <simona.vetter@ffwll.ch> Sent: Wednesday, June 4, 2025 7:46 AM
>> On Wed, Jun 04, 2025 at 10:12:45AM +0200, Thomas Zimmermann wrote:
>>> Hi
>>>
>>> Am 03.06.25 um 19:50 schrieb Michael Kelley:
>>>> From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Monday, June 2, 2025 11:25 PM
>>>>> Hi
>>>>>
>>>>> Am 03.06.25 um 03:49 schrieb Michael Kelley:
>>>>> [...]
>>>>> What is the motivation behind this work? The driver or fbdev as a whole
>>>>> does not have much of a future anyway.
>>>>>
>>>>> I'd like to suggest removing hyperv_fb entirely in favor of hypervdrm?
>>>>>
>>>> Yes, I think that's the longer term direction. A couple months ago I had an
>>>> email conversation with Saurabh Sengar from the Microsoft Linux team where
>>>> he raised this idea. I think the Microsoft folks will need to drive the deprecation
>>>> process, as they need to coordinate with the distro vendors who publish
>>>> images for running on local Hyper-V and in the Azure cloud. And my
>>>> understanding is that the Linux kernel process would want the driver to
>>>> be available but marked "deprecated" for a year or so before it actually
>>>> goes away.
>>> We (DRM upstream) recently considered moving some fbdev drivers to
>>> drivers/staging or marking them with !DRM if a DRM driver is available.
>>> Hyverv_fb would be a candidate.
>>>
>>> At least at SUSE, we ship hypervdrm instead of hyperv_fb. This works well on
>>> the various generations of the hyperv system. Much of our userspace would
>>> not be able to use hyperv_fb anyway.
> Good to know. Red Hat has made the switch as well. The Ubuntu images
> in Azure have both hyperv_fb and hyperv_drm. I don't know what other
> distros have done.
>
>> Yeah investing into fbdev drivers, especially when some mm surgery seems
>> needed, does not sound like a good idea to me overall.
>>
>>>> I do have some concerns about the maturity of the hyperv_drm driver
>>>> "around the edges". For example, somebody just recently submitted a
>>>> patch to flush output on panic. I have less familiarity hyperv_drm vs.
>>>> hyperv_fb, so some of my concern is probably due to that. We might
>>>> need to do review of hyperv_drm and see if there's anything else to
>>>> deal with before hyperv_fb goes away.
>>> The panic output is a feature that we recently added to the kernel. It
>>> allows a DRM driver to display a final error message in the case of a kernel
>>> panic (think of blue screens on Windows). Drivers require a minimum of
>>> support to make it work. That's what the hypervdrm patches were about.
>> I'm also happy to help with any other issues and shortfalls of drm vs
>> fbdev. There are some, but I thought it was mostly around some of the low
>> bit color formats that really old devices want, and not anything that
>> hyperv would need.
> You've set me up perfectly to raise an issue. :-) I'm still relatively new
> to the hyperv_drm driver and DRM in general, compared with hyperv_fb.
> One capability in fbdev is deferred I/O, which is what this entire patch
> series is about. The hyperv_drm driver doesn't currently use anything
> similar to deferred I/O like hyperv_fb. I don't know if that's because
> hyperv_drm doesn't make use of what DRM has to offer, or if DRM doesn't
> have a deferred I/O framework like fbdev. Do you know what the situation
> is? Or could you point me to an example of doing deferred I/O with DRM
> that hyperv_drm should be following?
Fbdev deferred I/O is a workaround for the fact that fbdev does not
require a flush operation on its I/O buffers. Writing to an mmaped
buffer is expected to go to hardware immediately. On devices where this
is not the case, deferred I/O tracks written pages and writes them back
to hardware at intervals.
For DRM, there's the MODE_DIRTYFB ioctl [1] that all userspace has to
call after writing to mmap'ed buffers. So regular DRM doesn't need
deferred I/O as userspace triggers writeback explicitly.
[1]
https://elixir.bootlin.com/linux/v6.15/source/drivers/gpu/drm/drm_ioctl.c#L686
>
> I ran a quick performance test comparing hyperv_drm with hyperv_fb.
> The test does "cat" of a big text file in the Hyper-V graphics console. The
> file has 1024 * 1024 lines, each with 64 characters, so total file size is
> 64 MiB.
>
> With hyperv_fb the test completes in 24 seconds elapsed time, with
> 24 seconds of system CPU time. With hyperv_drm, it takes 34 seconds
> elapsed time, but with about the same 24 seconds of system CPU time.
> Overall this difference isn't huge, and probably isn't that noticeable
> when doing human-scale work (i.e., 'dmesg' outputting several
> hundred lines in 0.19 seconds vs. my test doing 1M lines) on the Hyper-V
> graphics console. To me, the console doesn't feel slow with hyperv_drm
> compared to hyperv_fb, which is good.
DRM consoles are technically an fbdev device that operates on a DRM
device. Both, DRM and fbdev, have some differences that can make this
problematic. I'm not surprised that there are issues.
>
> Nonetheless, there's an underlying issue. A main cause of the difference
> is the number of messages to Hyper-V to update dirty regions. With
> hyperv_fb using deferred I/O, the messages are limited 20/second, so
> the total number of messages to Hyper-V is about 480. But hyperv_drm
> appears to send 3 messages to Hyper-V for each line of output, or a total of
> about 3,000,000 messages (~90K/second). That's a lot of additional load
> on the Hyper-V host, and it adds the 10 seconds of additional elapsed
> time seen in the guest. There also this ugly output in dmesg because the
> ring buffer for sending messages to the Hyper-V host gets full -- Hyper-V
> doesn't always keep up, at least not on my local laptop where I'm
> testing:
>
> [12574.327615] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
> [12574.327684] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
> [12574.327760] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
> [12574.327841] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
> [12597.016128] hyperv_sendpacket: 6211 callbacks suppressed
> [12597.016133] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
> [12597.016172] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
> [12597.016220] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
> [12597.016267] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
>
> hyperv_drm could be fixed to not output the ugly messages, but there's
> still the underlying issue of overrunning the ring buffer, and excessively
> hammering on the host. If we could get hyperv_drm doing deferred I/O, I
> would feel much better about going full-on with deprecating hyperv_fb.
Thanks for debugging this. A number of things are playing into this.
- DRM performs display output along vblank IRQs. For example, if the
display runs with 60 Hz there should be no more than 60 display updates
per second. From what I can tell, there's no IRQ support in hypervdrm
(or HyperV in general?). Without IRQ support, drivers output to hardware
ASAP, which can result in large numbers of buffer updates per second.
I've heard about this problem in other context [2] and you're likely
seeing a similar issue.
- DRM's console also needs better support for vblank interrupts. It
currently sends out updates ASAP as well.
Both points are not much of a problem on most desktop and server
systems, but can be an be an issue with virtualization.
[2] https://bugzilla.suse.com/show_bug.cgi?id=1189174
Best regards
Thomas
>
> Michael
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply
* [PATCH v3] vmbus: retrieve connection-id from DeviceTree
From: Hardik Garg @ 2025-06-05 6:09 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, ssengar
Cc: romank, linux-hyperv, linux-kernel, stable, apais
The connection-id determines which hypervisor communication channel the
guest should use to talk to the VMBus host. This patch adds support to
read this value from the DeviceTree where it exists as a property under
the vmbus node with the compatible ID "microsoft,message-connection-id".
The property name follows the format <vendor>,<field> where
"vendor": "microsoft" and "field": "message-connection-id"
Reading from DeviceTree allows platforms to specify their preferred
communication channel, making it more flexible. If the property is
not found in the DeviceTree, use the default connection ID
(VMBUS_MESSAGE_CONNECTION_ID or VMBUS_MESSAGE_CONNECTION_ID_4
based on protocol version).
Signed-off-by: Hardik Garg <hargar@linux.microsoft.com>
---
v3:
- Added documentation for the new property in DeviceTree bindings
v2:
https://lore.kernel.org/all/096edaf7-cc90-42b6-aff4-c5f088574e1e@linux.microsoft.com/
v1:
https://lore.kernel.org/all/6acee4bf-cb04-43b9-9476-e8d811d26dfd@linux.microsoft.com/
---
.../devicetree/bindings/bus/microsoft,vmbus.yaml | 8 ++++++++
drivers/hv/connection.c | 6 ++++--
drivers/hv/vmbus_drv.c | 13 +++++++++++++
3 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
index 0bea4f5287ce..729ca6defec6 100644
--- a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
+++ b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
@@ -17,6 +17,14 @@ properties:
compatible:
const: microsoft,vmbus
+ microsoft,message-connection-id:
+ description:
+ VMBus message connection ID to be used for communication between
host and
+ guest. If not specified, defaults to
VMBUS_MESSAGE_CONNECTION_ID_4 for
+ protocol version VERSION_WIN10_V5 and above, or
VMBUS_MESSAGE_CONNECTION_ID
+ for older versions.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
ranges: true
'#address-cells':
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index be490c598785..e0f38d799d06 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -99,12 +99,14 @@ int vmbus_negotiate_version(struct
vmbus_channel_msginfo *msginfo, u32 version)
if (version >= VERSION_WIN10_V5) {
msg->msg_sint = VMBUS_MESSAGE_SINT;
msg->msg_vtl = ms_hyperv.vtl;
- vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID_4;
} else {
msg->interrupt_page = virt_to_phys(vmbus_connection.int_page);
- vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID;
}
+ /* Set default connection ID if not provided via DeviceTree */
+ if (!vmbus_connection.msg_conn_id)
+ vmbus_connection.msg_conn_id = (version >= VERSION_WIN10_V5) ?
+ VMBUS_MESSAGE_CONNECTION_ID_4 : VMBUS_MESSAGE_CONNECTION_ID;
/*
* shared_gpa_boundary is zero in non-SNP VMs, so it's safe to always
* bitwise OR it
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index c236081d0a87..d14d286671cc 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2541,10 +2541,23 @@ static int vmbus_device_add(struct
platform_device *pdev)
struct of_range range;
struct of_range_parser parser;
struct device_node *np = pdev->dev.of_node;
+ unsigned int conn_id;
int ret;
vmbus_root_device = &pdev->dev;
+ /*
+ * Read connection ID from DeviceTree. The property name follows the
+ * format <vendor>,<field> where:
+ * - vendor: "microsoft"
+ * - field: "message-connection-id"
+ */
+ ret = of_property_read_u32(np, "microsoft,message-connection-id",
&conn_id);
+ if (!ret) {
+ pr_info("VMBus message connection ID: %u\n", conn_id);
+ vmbus_connection.msg_conn_id = conn_id;
+ }
+
ret = of_range_parser_init(&parser, np);
if (ret)
return ret;
--
2.40.4
^ permalink raw reply related
* RE: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Michael Kelley @ 2025-06-04 21:58 UTC (permalink / raw)
To: Michael Kelley, David Hildenbrand, simona@ffwll.ch, deller@gmx.de,
haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, akpm@linux-foundation.org
Cc: weh@microsoft.com, tzimmermann@suse.de, hch@lst.de,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
linux-mm@kvack.org
In-Reply-To: <SN6PR02MB4157A3F9E646C060553E5D90D46DA@SN6PR02MB4157.namprd02.prod.outlook.com>
From: Michael Kelley <mhklinux@outlook.com> Sent: Tuesday, June 3, 2025 10:25 AM
>
> From: David Hildenbrand <david@redhat.com> Sent: Tuesday, June 3, 2025 12:55 AM
> >
> > On 03.06.25 03:49, Michael Kelley wrote:
> > > From: David Hildenbrand <david@redhat.com> Sent: Monday, June 2, 2025 2:48 AM
> > >>
[snip]
> > >>> @@ -182,20 +221,34 @@ static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long
> > >>> }
> > >>>
> > >>> /*
> > >>> - * We want the page to remain locked from ->page_mkwrite until
> > >>> - * the PTE is marked dirty to avoid mapping_wrprotect_range()
> > >>> - * being called before the PTE is updated, which would leave
> > >>> - * the page ignored by defio.
> > >>> - * Do this by locking the page here and informing the caller
> > >>> - * about it with VM_FAULT_LOCKED.
> > >>> + * The PTE must be marked writable before the defio deferred work runs
> > >>> + * again and potentially marks the PTE write-protected. If the order
> > >>> + * should be switched, the PTE would become writable without defio
> > >>> + * tracking the page, leaving the page forever ignored by defio.
> > >>> + *
> > >>> + * For vmalloc() framebuffers, the associated struct page is locked
> > >>> + * before releasing the defio lock. mm will later mark the PTE writaable
> > >>> + * and release the struct page lock. The struct page lock prevents
> > >>> + * the page from being prematurely being marked write-protected.
> > >>> + *
> > >>> + * For FBINFO_KMEMFB framebuffers, mm assumes there is no struct page,
> > >>> + * so the PTE must be marked writable while the defio lock is held.
> > >>> */
> > >>> - lock_page(pageref->page);
> > >>> + if (info->flags & FBINFO_KMEMFB) {
> > >>> + unsigned long pfn = page_to_pfn(pageref->page);
> > >>> +
> > >>> + ret = vmf_insert_mixed_mkwrite(vmf->vma, vmf->address,
> > >>> + __pfn_to_pfn_t(pfn, PFN_SPECIAL));
> > >>
> > >> Will the VMA have VM_PFNMAP or VM_MIXEDMAP set? PFN_SPECIAL is a
> > >> horrible hack.
> > >>
> > >> In another thread, you mention that you use PFN_SPECIAL to bypass the
> > >> check in vm_mixed_ok(), so VM_MIXEDMAP is likely not set?
> > >
> > > The VMA has VM_PFNMAP set, not VM_MIXEDMAP. It seemed like
> > > VM_MIXEDMAP is somewhat of a superset of VM_PFNMAP, but maybe that's
> > > a wrong impression.
> >
> > VM_PFNMAP: nothing is refcounted except anon pages
> >
> > VM_MIXEDMAP: anything with a "struct page" (pfn_valid()) is refcounted
> >
> > pte_special() is a way for GUP-fast to distinguish these refcounted (can
> > GUP) from non-refcounted (camnnot GUP) pages mapped by PTEs without any
> > locks or the VMA being available.
> >
> > Setting pte_special() in VM_MIXEDMAP on ptes that have a "struct page"
> > (pfn_valid()) is likely very bogus.
>
> OK, good to know.
>
> >
> > > vm_mixed_ok() does a thorough job of validating the
> > > use of __vm_insert_mixed(), and since what I did was allowed, I thought
> > > perhaps it was OK. Your feedback has set me straight, and that's what I
> > > needed. :-)
> >
> > What exactly are you trying to achieve? :)
> >
> > If it's mapping a page with a "struct page" and *not* refcounting it,
> > then vmf_insert_pfn() is the current way to achieve that in a VM_PFNMAP
> > mapping. It will set pte_special() automatically for you.
> >
>
> Yes, that's what I'm using to initially create the special PTE in the
> .fault callback.
>
> > >
> > > But the whole approach is moot with Alistair Popple's patch set that
> > > eliminates pfn_t. Is there an existing mm API that will do mkwrite on a
> > > special PTE in a VM_PFNMAP VMA? I didn't see one, but maybe I missed
> > > it. If there's not one, I'll take a crack at adding it in the next version of my
> > > patch set.
> >
> > I assume you'd want vmf_insert_pfn_mkwrite(), correct? Probably
> > vmf_insert_pfn_prot() can be used by adding PAGE_WRITE to pgprot. (maybe
> > :) )
>
> Ok, I'll look at that more closely. The sequence is that the special
> PTE gets created with vmf_insert_pfn(). Then when the page is first
> written to, the .pfn_mkwrite callback is invoked by mm. The question
> is the best way for that callback to mark the existing PTE as writable.
>
FWIW, vmf_insert_pfn_prot() won't work. It calls insert_pfn() with
the "mkwrite" parameter set to 'false', in which case insert_pfn()
does nothing if the PTE already exists.
So I would need to create a new API that does appropriate validation
for a VM_PFNMAP VMA, and then calls insert_pfn() with the "mkwrite"
parameter set to 'true'.
Michael
^ permalink raw reply
* [PATCH] firmware: smccc: support both conduits for getting hyp UUID
From: Roman Kisel @ 2025-06-04 21:45 UTC (permalink / raw)
To: anirudh
Cc: linux-arm-kernel, linux-hyperv, linux-kernel, lpieralisi,
mark.rutland, sudeep.holla, apais, benhill, bperkins, sunilmut
In-Reply-To: <20250521094049.960056-1-anirudh@anirudhrb.com>
> From: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
>
> When Linux is running as the root partition under Microsoft Hypervisor
> (MSHV) a.k.a Hyper-V, smc is used as the conduit for smc calls.
>
> Extend arm_smccc_hypervisor_has_uuid() to support this usecase. Use
> arm_smccc_1_1_invoke to retrieve and use the appropriate conduit instead
> of supporting only hvc.
>
> Boot tested on MSHV guest, MSHV root & KVM guest.
>
> Signed-off-by: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
> ---
> drivers/firmware/smccc/smccc.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
> index cd65b434dc6e..bdee057db2fd 100644
> --- a/drivers/firmware/smccc/smccc.c
> +++ b/drivers/firmware/smccc/smccc.c
> @@ -72,10 +72,7 @@ bool arm_smccc_hypervisor_has_uuid(const uuid_t *hyp_uuid)
> struct arm_smccc_res res = {};
> uuid_t uuid;
>
> - if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_HVC)
> - return false;
> -
> - arm_smccc_1_1_hvc(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, &res);
> + arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID, &res);
> if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
> return false;
The patch is a strict superset of the existing functionality.
Validated on baremetal Linux and KVM, Apple M3 nested virt., various client and
server ARM64 systems running Windows ARM64 with VMs managed by Hyper-V and OpenVMM.
Looks good to me.
Tested-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Roman Kisel <romank@linux.microsoft.com>
^ permalink raw reply
* RE: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Michael Kelley @ 2025-06-04 21:43 UTC (permalink / raw)
To: Simona Vetter, Thomas Zimmermann
Cc: David Hildenbrand, simona@ffwll.ch, deller@gmx.de,
haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, akpm@linux-foundation.org, weh@microsoft.com,
hch@lst.de, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, linux-mm@kvack.org
In-Reply-To: <aEBcCjMWZJgbsRas@phenom.ffwll.local>
From: Simona Vetter <simona.vetter@ffwll.ch> Sent: Wednesday, June 4, 2025 7:46 AM
>
> On Wed, Jun 04, 2025 at 10:12:45AM +0200, Thomas Zimmermann wrote:
> > Hi
> >
> > Am 03.06.25 um 19:50 schrieb Michael Kelley:
> > > From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Monday, June 2, 2025 11:25 PM
> > > > Hi
> > > >
> > > > Am 03.06.25 um 03:49 schrieb Michael Kelley:
> > > > [...]
> > > > What is the motivation behind this work? The driver or fbdev as a whole
> > > > does not have much of a future anyway.
> > > >
> > > > I'd like to suggest removing hyperv_fb entirely in favor of hypervdrm?
> > > >
> > > Yes, I think that's the longer term direction. A couple months ago I had an
> > > email conversation with Saurabh Sengar from the Microsoft Linux team where
> > > he raised this idea. I think the Microsoft folks will need to drive the deprecation
> > > process, as they need to coordinate with the distro vendors who publish
> > > images for running on local Hyper-V and in the Azure cloud. And my
> > > understanding is that the Linux kernel process would want the driver to
> > > be available but marked "deprecated" for a year or so before it actually
> > > goes away.
> >
> > We (DRM upstream) recently considered moving some fbdev drivers to
> > drivers/staging or marking them with !DRM if a DRM driver is available.
> > Hyverv_fb would be a candidate.
> >
> > At least at SUSE, we ship hypervdrm instead of hyperv_fb. This works well on
> > the various generations of the hyperv system. Much of our userspace would
> > not be able to use hyperv_fb anyway.
Good to know. Red Hat has made the switch as well. The Ubuntu images
in Azure have both hyperv_fb and hyperv_drm. I don't know what other
distros have done.
>
> Yeah investing into fbdev drivers, especially when some mm surgery seems
> needed, does not sound like a good idea to me overall.
>
> > > I do have some concerns about the maturity of the hyperv_drm driver
> > > "around the edges". For example, somebody just recently submitted a
> > > patch to flush output on panic. I have less familiarity hyperv_drm vs.
> > > hyperv_fb, so some of my concern is probably due to that. We might
> > > need to do review of hyperv_drm and see if there's anything else to
> > > deal with before hyperv_fb goes away.
> >
> > The panic output is a feature that we recently added to the kernel. It
> > allows a DRM driver to display a final error message in the case of a kernel
> > panic (think of blue screens on Windows). Drivers require a minimum of
> > support to make it work. That's what the hypervdrm patches were about.
>
> I'm also happy to help with any other issues and shortfalls of drm vs
> fbdev. There are some, but I thought it was mostly around some of the low
> bit color formats that really old devices want, and not anything that
> hyperv would need.
You've set me up perfectly to raise an issue. :-) I'm still relatively new
to the hyperv_drm driver and DRM in general, compared with hyperv_fb.
One capability in fbdev is deferred I/O, which is what this entire patch
series is about. The hyperv_drm driver doesn't currently use anything
similar to deferred I/O like hyperv_fb. I don't know if that's because
hyperv_drm doesn't make use of what DRM has to offer, or if DRM doesn't
have a deferred I/O framework like fbdev. Do you know what the situation
is? Or could you point me to an example of doing deferred I/O with DRM
that hyperv_drm should be following?
I ran a quick performance test comparing hyperv_drm with hyperv_fb.
The test does "cat" of a big text file in the Hyper-V graphics console. The
file has 1024 * 1024 lines, each with 64 characters, so total file size is
64 MiB.
With hyperv_fb the test completes in 24 seconds elapsed time, with
24 seconds of system CPU time. With hyperv_drm, it takes 34 seconds
elapsed time, but with about the same 24 seconds of system CPU time.
Overall this difference isn't huge, and probably isn't that noticeable
when doing human-scale work (i.e., 'dmesg' outputting several
hundred lines in 0.19 seconds vs. my test doing 1M lines) on the Hyper-V
graphics console. To me, the console doesn't feel slow with hyperv_drm
compared to hyperv_fb, which is good.
Nonetheless, there's an underlying issue. A main cause of the difference
is the number of messages to Hyper-V to update dirty regions. With
hyperv_fb using deferred I/O, the messages are limited 20/second, so
the total number of messages to Hyper-V is about 480. But hyperv_drm
appears to send 3 messages to Hyper-V for each line of output, or a total of
about 3,000,000 messages (~90K/second). That's a lot of additional load
on the Hyper-V host, and it adds the 10 seconds of additional elapsed
time seen in the guest. There also this ugly output in dmesg because the
ring buffer for sending messages to the Hyper-V host gets full -- Hyper-V
doesn't always keep up, at least not on my local laptop where I'm
testing:
[12574.327615] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
[12574.327684] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
[12574.327760] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
[12574.327841] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
[12597.016128] hyperv_sendpacket: 6211 callbacks suppressed
[12597.016133] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
[12597.016172] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
[12597.016220] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
[12597.016267] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
hyperv_drm could be fixed to not output the ugly messages, but there's
still the underlying issue of overrunning the ring buffer, and excessively
hammering on the host. If we could get hyperv_drm doing deferred I/O, I
would feel much better about going full-on with deprecating hyperv_fb.
Michael
^ 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