* Re: [PATCH v0 1/6] x86/hyperv: Rename guest shutdown functions
From: Anirudh Rayabharam @ 2025-09-04 17:22 UTC (permalink / raw)
To: Mukesh Rathor
Cc: linux-hyperv, linux-kernel, linux-arch, kys, haiyangz, wei.liu,
decui, tglx, mingo, bp, dave.hansen, x86, hpa, arnd
In-Reply-To: <20250904021017.1628993-2-mrathor@linux.microsoft.com>
On Wed, Sep 03, 2025 at 07:10:12PM -0700, Mukesh Rathor wrote:
> This commit renames hv_machine_crash_shutdown to more appropriate
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
Please describe your changes in imperative mood. For e.g.:
“make xyzzy do frotz” instead of “[This patch] makes xyzzy do frotz”
Please fix this for all the patches in this series.
Thanks,
Anirudh.
> hv_guest_crash_shutdown and makes it applicable to guests only. This
> in preparation for the subsequent hypervisor root/dom0 crash support
> patches.
>
> Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
> ---
> arch/x86/kernel/cpu/mshyperv.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index b8caf78c4336..f5d76d854d39 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -237,7 +237,7 @@ static void hv_machine_shutdown(void)
> #endif /* CONFIG_KEXEC_CORE */
>
> #ifdef CONFIG_CRASH_DUMP
> -static void hv_machine_crash_shutdown(struct pt_regs *regs)
> +static void hv_guest_crash_shutdown(struct pt_regs *regs)
> {
> if (hv_crash_handler)
> hv_crash_handler(regs);
> @@ -824,7 +824,8 @@ static void __init ms_hyperv_init_platform(void)
> machine_ops.shutdown = hv_machine_shutdown;
> #endif
> #if defined(CONFIG_CRASH_DUMP)
> - machine_ops.crash_shutdown = hv_machine_crash_shutdown;
> + if (!hv_root_partition)
> + machine_ops.crash_shutdown = hv_guest_crash_shutdown;
> #endif
> #endif
> /*
> --
> 2.36.1.vfs.0.0
>
^ permalink raw reply
* Re: [PATCH 3/6] mshv: Add the HVCALL_GET_PARTITION_PROPERTY_EX hypercall
From: Anirudh Rayabharam @ 2025-09-04 17:12 UTC (permalink / raw)
To: Nuno Das Neves
Cc: linux-hyperv, linux-kernel, kys, haiyangz, wei.liu, mhklinux,
decui, paekkaladevi
In-Reply-To: <1756428230-3599-4-git-send-email-nunodasneves@linux.microsoft.com>
On Thu, Aug 28, 2025 at 05:43:47PM -0700, Nuno Das Neves wrote:
> From: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
>
> This hypercall can be used to fetch extended properties of a
> partition. Extended properties are properties with values larger than
> a u64. Some of these also need additional input arguments.
>
> Add helper function for using the hypercall in the mshv_root driver.
>
> Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
> drivers/hv/mshv_root.h | 2 ++
> drivers/hv/mshv_root_hv_call.c | 31 ++++++++++++++++++++++++++
> include/hyperv/hvgdk_mini.h | 1 +
> include/hyperv/hvhdk.h | 40 ++++++++++++++++++++++++++++++++++
> include/hyperv/hvhdk_mini.h | 26 ++++++++++++++++++++++
> 5 files changed, 100 insertions(+)
>
> diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
> index e3931b0f1269..4aeb03bea6b6 100644
> --- a/drivers/hv/mshv_root.h
> +++ b/drivers/hv/mshv_root.h
> @@ -303,6 +303,8 @@ int hv_call_unmap_stat_page(enum hv_stats_object_type type,
> int hv_call_modify_spa_host_access(u64 partition_id, struct page **pages,
> u64 page_struct_count, u32 host_access,
> u32 flags, u8 acquire);
> +int hv_call_get_partition_property_ex(u64 partition_id, u64 property_code, u64 arg,
> + void *property_value, size_t property_value_sz);
>
> extern struct mshv_root mshv_root;
> extern enum hv_scheduler_type hv_scheduler_type;
> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
> index 1c38576a673c..7589b1ff3515 100644
> --- a/drivers/hv/mshv_root_hv_call.c
> +++ b/drivers/hv/mshv_root_hv_call.c
> @@ -590,6 +590,37 @@ int hv_call_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> return hv_result_to_errno(status);
> }
>
> +int
> +hv_call_get_partition_property_ex(u64 partition_id, u64 property_code, u64 arg,
> + void *property_value, size_t property_value_sz)
> +{
> + u64 status;
> + unsigned long flags;
> + struct hv_input_get_partition_property_ex *input;
> + struct hv_output_get_partition_property_ex *output;
> +
> + local_irq_save(flags);
> + input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> + output = *this_cpu_ptr(hyperv_pcpu_output_arg);
> +
> + memset(input, 0, sizeof(*input));
> + input->partition_id = partition_id;
> + input->property_code = property_code;
> + input->arg = arg;
> + status = hv_do_hypercall(HVCALL_GET_PARTITION_PROPERTY_EX, input, output);
> +
> + if (!hv_result_success(status)) {
> + hv_status_debug(status, "\n");
> + local_irq_restore(flags);
> + return hv_result_to_errno(status);
> + }
> + memcpy(property_value, &output->property_value, property_value_sz);
> +
> + local_irq_restore(flags);
> +
> + return 0;
> +}
> +
> int
> hv_call_clear_virtual_interrupt(u64 partition_id)
> {
> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
> index 79b7324e4ef5..1bde0aa102ec 100644
> --- a/include/hyperv/hvgdk_mini.h
> +++ b/include/hyperv/hvgdk_mini.h
> @@ -490,6 +490,7 @@ union hv_vp_assist_msr_contents { /* HV_REGISTER_VP_ASSIST_PAGE */
> #define HVCALL_GET_VP_STATE 0x00e3
> #define HVCALL_SET_VP_STATE 0x00e4
> #define HVCALL_GET_VP_CPUID_VALUES 0x00f4
> +#define HVCALL_GET_PARTITION_PROPERTY_EX 0x0101
> #define HVCALL_MMIO_READ 0x0106
> #define HVCALL_MMIO_WRITE 0x0107
>
> diff --git a/include/hyperv/hvhdk.h b/include/hyperv/hvhdk.h
> index 57f3f9c2a685..fd3555def008 100644
> --- a/include/hyperv/hvhdk.h
> +++ b/include/hyperv/hvhdk.h
> @@ -411,6 +411,46 @@ struct hv_input_set_partition_property {
> u64 property_value;
> } __packed;
>
> +union hv_partition_property_arg {
> + u64 as_uint64;
> + struct {
> + union {
> + u32 arg;
> + u32 vp_index;
> + };
> + u16 reserved0;
> + u8 reserved1;
> + u8 object_type;
> + };
> +} __packed;
> +
> +struct hv_input_get_partition_property_ex {
> + u64 partition_id;
> + u32 property_code; /* enum hv_partition_property_code */
> + u32 padding;
> + union {
> + union hv_partition_property_arg arg_data;
> + u64 arg;
> + };
> +} __packed;
> +
> +/*
> + * NOTE: Should use hv_input_set_partition_property_ex_header to compute this
> + * size, but hv_input_get_partition_property_ex is identical so it suffices
> + */
> +#define HV_PARTITION_PROPERTY_EX_MAX_VAR_SIZE \
> + (HV_HYP_PAGE_SIZE - sizeof(struct hv_input_get_partition_property_ex))
> +
> +union hv_partition_property_ex {
> + u8 buffer[HV_PARTITION_PROPERTY_EX_MAX_VAR_SIZE];
> + struct hv_partition_property_vmm_capabilities vmm_capabilities;
> + /* More fields to be filled in when needed */
> +} __packed;
> +
> +struct hv_output_get_partition_property_ex {
> + union hv_partition_property_ex property_value;
> +} __packed;
> +
> enum hv_vp_state_page_type {
> HV_VP_STATE_PAGE_REGISTERS = 0,
> HV_VP_STATE_PAGE_INTERCEPT_MESSAGE = 1,
> diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
> index 858f6a3925b3..bf2ce27dfcc5 100644
> --- a/include/hyperv/hvhdk_mini.h
> +++ b/include/hyperv/hvhdk_mini.h
> @@ -96,8 +96,34 @@ enum hv_partition_property_code {
> HV_PARTITION_PROPERTY_XSAVE_STATES = 0x00060007,
> HV_PARTITION_PROPERTY_MAX_XSAVE_DATA_SIZE = 0x00060008,
> HV_PARTITION_PROPERTY_PROCESSOR_CLOCK_FREQUENCY = 0x00060009,
> +
> + /* Extended properties with larger property values */
> + HV_PARTITION_PROPERTY_VMM_CAPABILITIES = 0x00090007,
> };
>
> +#define HV_PARTITION_VMM_CAPABILITIES_BANK_COUNT 1
> +#define HV_PARTITION_VMM_CAPABILITIES_RESERVED_BITFIELD_COUNT 59
> +
> +struct hv_partition_property_vmm_capabilities {
> + u16 bank_count;
> + u16 reserved[3];
> + union {
> + u64 as_uint64[HV_PARTITION_VMM_CAPABILITIES_BANK_COUNT];
> + struct {
> + u64 map_gpa_preserve_adjustable: 1;
> + u64 vmm_can_provide_overlay_gpfn: 1;
> + u64 vp_affinity_property: 1;
> +#if IS_ENABLED(CONFIG_ARM64)
> + u64 vmm_can_provide_gic_overlay_locations: 1;
> +#else
> + u64 reservedbit3: 1;
> +#endif
> + u64 assignable_synthetic_proc_features: 1;
> + u64 reserved0: HV_PARTITION_VMM_CAPABILITIES_RESERVED_BITFIELD_COUNT;
> + } __packed;
> + };
> +} __packed;
> +
> enum hv_snp_status {
> HV_SNP_STATUS_NONE = 0,
> HV_SNP_STATUS_AVAILABLE = 1,
> --
> 2.34.1
>
Reviewed-by: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
^ permalink raw reply
* Re: [PATCH 1/6] mshv: Only map vp->vp_stats_pages if on root scheduler
From: Anirudh Rayabharam @ 2025-09-04 17:00 UTC (permalink / raw)
To: Nuno Das Neves
Cc: linux-hyperv, linux-kernel, kys, haiyangz, wei.liu, mhklinux,
decui, paekkaladevi
In-Reply-To: <1756428230-3599-2-git-send-email-nunodasneves@linux.microsoft.com>
On Thu, Aug 28, 2025 at 05:43:45PM -0700, Nuno Das Neves wrote:
> This mapping is only used for checking if the dispatch thread is
> blocked. This is only relevant for the root scheduler, so check the
> scheduler type to determine whether to map/unmap these pages, instead of
> the current check, which is incorrect.
>
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
> drivers/hv/mshv_root_main.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index e4ee9beddaf5..bbdefe8a2e9c 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
> @@ -987,7 +987,11 @@ mshv_partition_ioctl_create_vp(struct mshv_partition *partition,
> goto unmap_register_page;
> }
>
> - if (hv_parent_partition()) {
> + /*
> + * This mapping of the stats page is for detecting if dispatch thread
> + * is blocked - only relevant for root scheduler
> + */
> + if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT) {
> ret = mshv_vp_stats_map(partition->pt_id, args.vp_index,
> stats_pages);
> if (ret)
> @@ -1016,7 +1020,7 @@ mshv_partition_ioctl_create_vp(struct mshv_partition *partition,
> if (mshv_partition_encrypted(partition) && is_ghcb_mapping_available())
> vp->vp_ghcb_page = page_to_virt(ghcb_page);
>
> - if (hv_parent_partition())
> + if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
> memcpy(vp->vp_stats_pages, stats_pages, sizeof(stats_pages));
>
> /*
> @@ -1039,7 +1043,7 @@ mshv_partition_ioctl_create_vp(struct mshv_partition *partition,
> free_vp:
> kfree(vp);
> unmap_stats_pages:
> - if (hv_parent_partition())
> + if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
> mshv_vp_stats_unmap(partition->pt_id, args.vp_index);
> unmap_ghcb_page:
> if (mshv_partition_encrypted(partition) && is_ghcb_mapping_available()) {
> @@ -1793,7 +1797,7 @@ static void destroy_partition(struct mshv_partition *partition)
> if (!vp)
> continue;
>
> - if (hv_parent_partition())
> + if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
> mshv_vp_stats_unmap(partition->pt_id, vp->vp_index);
>
> if (vp->vp_register_page) {
> --
> 2.34.1
>
Reviewed-by: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
^ permalink raw reply
* Re: [PATCH net-next v5 4/9] vsock/loopback: add netns support
From: Bobby Eshleman @ 2025-09-04 16:57 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, berrange,
Bobby Eshleman
In-Reply-To: <yuumxtgizcrkn4uspczro76p6u2dqvra5otqkrb57bi2se2lpx@zg4f4rtix7hb>
On Wed, Sep 03, 2025 at 05:10:31PM +0200, Stefano Garzarella wrote:
> On Tue, Sep 02, 2025 at 11:09:36AM -0700, Bobby Eshleman wrote:
> > On Tue, Sep 02, 2025 at 05:39:33PM +0200, Stefano Garzarella wrote:
> > > On Wed, Aug 27, 2025 at 05:31:32PM -0700, Bobby Eshleman wrote:
> > > > From: Bobby Eshleman <bobbyeshleman@meta.com>
> > > >
> > >
[...]
> >
> > I could be wrong here, but I think net->vsock.loopback being set before
> > vsock_core_register() prevents racing with net->vsock.loopback reads. We
> > could add a lock to make sure and to make the protection explicit
> > though.
>
> I see, talkink about vsock_core_register(), I was thinking about,
> extending it, maybe passing a struct with all parameters (e.g. transport
> type, net callbacks, etc.). In this way we can easily check if the type
> of transport is allowed to register net callbacks or not.
>
> Also because currently we don't do any check in
> __vsock_register_net_callbacks() about transport type or even about
> overriding calls.
>
That makes a lot of sense. I'll make that change if pernet_operations
doesn't solve the probelem.
Best,
Bobby
^ permalink raw reply
* RE: [PATCH V0 0/2] Fix CONFIG_HYPERV and vmbus related anamoly
From: Michael Kelley @ 2025-09-04 16:26 UTC (permalink / raw)
To: Mukesh R, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
linux-hyperv@vger.kernel.org, netdev@vger.kernel.org,
linux-pci@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-arch@vger.kernel.org,
virtualization@lists.linux.dev
Cc: maarten.lankhorst@linux.intel.com, mripard@kernel.org,
tzimmermann@suse.de, airlied@gmail.com, simona@ffwll.ch,
jikos@kernel.org, bentiss@kernel.org, kys@microsoft.com,
haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
dmitry.torokhov@gmail.com, andrew+netdev@lunn.ch,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, bhelgaas@google.com,
James.Bottomley@HansenPartnership.com, martin.petersen@oracle.com,
gregkh@linuxfoundation.org, deller@gmx.de, arnd@arndb.de,
sgarzare@redhat.com, horms@kernel.org
In-Reply-To: <ff4c58f1-564d-ddfa-bdff-48ffee6e0d72@linux.microsoft.com>
From: Mukesh R <mrathor@linux.microsoft.com> Sent: Wednesday, September 3, 2025 7:17 PM
>
> On 9/2/25 07:42, Michael Kelley wrote:
> > From: Mukesh Rathor <mrathor@linux.microsoft.com> Sent: Wednesday, August 27, 2025 6:00 PM
> >>
> >> At present, drivers/Makefile will subst =m to =y for CONFIG_HYPERV for hv
> >> subdir. Also, drivers/hv/Makefile replaces =m to =y to build in
> >> hv_common.c that is needed for the drivers. Moreover, vmbus driver is
> >> built if CONFIG_HYPER is set, either loadable or builtin.
> >>
> >> This is not a good approach. CONFIG_HYPERV is really an umbrella config that
> >> encompasses builtin code and various other things and not a dedicated config
> >> option for VMBUS. Vmbus should really have a config option just like
> >> CONFIG_HYPERV_BALLOON etc. This small series introduces CONFIG_HYPERV_VMBUS
> >> to build VMBUS driver and make that distinction explicit. With that
> >> CONFIG_HYPERV could be changed to bool.
> >
> > Separating the core hypervisor support (CONFIG_HYPERV) from the VMBus
> > support (CONFIG_HYPERV_VMBUS) makes sense to me. Overall the code
> > is already mostly in separate source files code, though there's some
> > entanglement in the handling of VMBus interrupts, which could be
> > improved later.
> >
> > However, I have a compatibility concern. Consider this scenario:
> >
> > 1) Assume running in a Hyper-V VM with a current Linux kernel version
> > built with CONFIG_HYPERV=m.
> > 2) Grab a new version of kernel source code that contains this patch set.
> > 3) Run 'make olddefconfig' to create the .config file for the new kernel.
> > 4) Build the new kernel. This succeeds.
> > 5) Install and run the new kernel in the Hyper-V VM. This fails.
> >
> > The failure occurs because CONFIG_HYPERV=m is no longer legal,
> > so the .config file created in Step 3 has CONFIG_HYPERV=n. The
> > newly built kernel has no Hyper-V support and won't run in a
> > Hyper-V VM.
> >
> > As a second issue, if in Step 1 the current kernel was built with
> > CONFIG_HYPERV=y, then the .config file for the new kernel will have
> > CONFIG_HYPERV=y, which is better. But CONFIG_HYPERV_VMBUS
> > defaults to 'n', so the new kernel doesn't have any VMBus drivers
> > and won't run in a typical Hyper-V VM.
> >
> > The second issue could be fixed by assigning CONFIG_HYPERV_VMBUS
> > a default value, such as whatever CONFIG_HYPERV is set to. But
> > I'm not sure how to fix the first issue, except by continuing to
> > allow CONFIG_HYPERV=m.
>
> To certain extent, imo, users are expected to check config files
> for changes when moving to new versions/releases, so it would be a
> one time burden.
I'm not so sanguine about the impact. For those of us who work with
Hyper-V frequently, yes, it's probably not that big of an issue -- we can
figure it out. But a lot of Azure/Hyper-V users aren't that familiar with
the details of how the Kconfig files are put together. And the issue occurs
with no error messages that something has gone wrong in building
the kernel, except that it won't boot. Just running "make olddefconfig"
has worked in the past, so some users will be befuddled and end up
generating Azure support incidents. I also wonder about breaking
automated test suites for new kernels, as they are likely to be running
"make olddefconfig" or something similar as part of the automation.
> CONFIG_HYPERV=m is just broken imo as one sees that
> in .config but magically symbols in drivers/hv are in kerenel.
>
I agree that's not ideal. But note that some Hyper-V code and symbols
like ms_hyperv_init_platform() and related functions show up when
CONFIG_HYPERVISOR_GUEST=y, even if CONFIG_HYPERV=n. That's
the code in arch/x86/kernel/cpu/mshyperv.c and it's because Hyper-V
is one of the recognized and somewhat hardwired hypervisors (like
VMware, for example).
Finally, there are about a dozen other places in the kernel that use
the same Makefile construct to make some code built-in even though
the CONFIG option is set to "m". That may not be enough occurrences
to make it standard practice, but Hyper-V guests are certainly not the
only case.
In my mind, this is judgment call with no absolute right answer. What
do others think about the tradeoffs?
Michael
^ permalink raw reply
* [PATCH v3 3/4] drm/vkms: Convert to DRM's vblank timer
From: Thomas Zimmermann @ 2025-09-04 14:56 UTC (permalink / raw)
To: louis.chauvet, drawat.floss, hamohammed.sa, melissa.srw, mhklinux,
simona, airlied, maarten.lankhorst, ville.syrjala, lyude, javierm
Cc: dri-devel, linux-hyperv, Thomas Zimmermann
In-Reply-To: <20250904145806.430568-1-tzimmermann@suse.de>
Replace vkms' vblank timer with the DRM implementation. The DRM
code is identical in concept, but differs in implementation.
Vblank timers are covered in vblank helpers and initializer macros,
so remove the corresponding hrtimer in struct vkms_output. The
vblank timer calls vkms' custom timeout code via handle_vblank_timeout
in struct drm_crtc_helper_funcs.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Louis Chauvet <louis.chauvet@bootlin.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
drivers/gpu/drm/vkms/vkms_crtc.c | 83 +++-----------------------------
drivers/gpu/drm/vkms/vkms_drv.h | 2 -
2 files changed, 7 insertions(+), 78 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index e60573e0f3e9..bd79f24686dc 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -7,25 +7,18 @@
#include <drm/drm_managed.h>
#include <drm/drm_probe_helper.h>
#include <drm/drm_vblank.h>
+#include <drm/drm_vblank_helper.h>
#include "vkms_drv.h"
-static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
+static bool vkms_crtc_handle_vblank_timeout(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,55 +50,6 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
dma_fence_end_signalling(fence_cookie);
- return HRTIMER_RESTART;
-}
-
-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);
-
- return 0;
-}
-
-static void vkms_disable_vblank(struct drm_crtc *crtc)
-{
- struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
-
- hrtimer_cancel(&out->vblank_hrtimer);
-}
-
-static bool vkms_get_vblank_timestamp(struct drm_crtc *crtc,
- int *max_error, ktime_t *vblank_time,
- 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;
}
@@ -159,9 +103,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
.reset = vkms_atomic_crtc_reset,
.atomic_duplicate_state = vkms_atomic_crtc_duplicate_state,
.atomic_destroy_state = vkms_atomic_crtc_destroy_state,
- .enable_vblank = vkms_enable_vblank,
- .disable_vblank = vkms_disable_vblank,
- .get_vblank_timestamp = vkms_get_vblank_timestamp,
+ DRM_CRTC_VBLANK_TIMER_FUNCS,
.get_crc_sources = vkms_get_crc_sources,
.set_crc_source = vkms_set_crc_source,
.verify_crc_source = vkms_verify_crc_source,
@@ -213,18 +155,6 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
return 0;
}
-static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
- struct drm_atomic_state *state)
-{
- drm_crtc_vblank_on(crtc);
-}
-
-static void vkms_crtc_atomic_disable(struct drm_crtc *crtc,
- struct drm_atomic_state *state)
-{
- drm_crtc_vblank_off(crtc);
-}
-
static void vkms_crtc_atomic_begin(struct drm_crtc *crtc,
struct drm_atomic_state *state)
__acquires(&vkms_output->lock)
@@ -265,8 +195,9 @@ static const struct drm_crtc_helper_funcs vkms_crtc_helper_funcs = {
.atomic_check = vkms_crtc_atomic_check,
.atomic_begin = vkms_crtc_atomic_begin,
.atomic_flush = vkms_crtc_atomic_flush,
- .atomic_enable = vkms_crtc_atomic_enable,
- .atomic_disable = vkms_crtc_atomic_disable,
+ .atomic_enable = drm_crtc_vblank_atomic_enable,
+ .atomic_disable = drm_crtc_vblank_atomic_disable,
+ .handle_vblank_timeout = vkms_crtc_handle_vblank_timeout,
};
struct vkms_output *vkms_crtc_init(struct drm_device *dev, struct drm_plane *primary,
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 8013c31efe3b..fb9711e1c6fb 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -215,8 +215,6 @@ 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 workqueue_struct *composer_workq;
spinlock_t lock;
--
2.50.1
^ permalink raw reply related
* [PATCH v3 2/4] drm/vblank: Add CRTC helpers for simple use cases
From: Thomas Zimmermann @ 2025-09-04 14:56 UTC (permalink / raw)
To: louis.chauvet, drawat.floss, hamohammed.sa, melissa.srw, mhklinux,
simona, airlied, maarten.lankhorst, ville.syrjala, lyude, javierm
Cc: dri-devel, linux-hyperv, Thomas Zimmermann
In-Reply-To: <20250904145806.430568-1-tzimmermann@suse.de>
Implement atomic_flush, atomic_enable and atomic_disable of struct
drm_crtc_helper_funcs for vblank handling. Driver with no further
requirements can use these functions instead of adding their own.
Also simplifies the use of vblank timers.
The code has been adopted from vkms, which added the funtionality
in commit 3a0709928b17 ("drm/vkms: Add vblank events simulated by
hrtimers").
v3:
- mention vkms (Javier)
v2:
- fix docs
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
drivers/gpu/drm/drm_vblank_helper.c | 80 +++++++++++++++++++++++++++++
include/drm/drm_vblank_helper.h | 23 +++++++++
2 files changed, 103 insertions(+)
diff --git a/drivers/gpu/drm/drm_vblank_helper.c b/drivers/gpu/drm/drm_vblank_helper.c
index f94d1e706191..a04a6ba1b0ca 100644
--- a/drivers/gpu/drm/drm_vblank_helper.c
+++ b/drivers/gpu/drm/drm_vblank_helper.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: MIT
+#include <drm/drm_atomic.h>
#include <drm/drm_crtc.h>
#include <drm/drm_managed.h>
#include <drm/drm_modeset_helper_vtables.h>
@@ -17,6 +18,12 @@
* Drivers enable support for vblank timers by setting the vblank callbacks
* in struct &drm_crtc_funcs to the helpers provided by this library. The
* initializer macro DRM_CRTC_VBLANK_TIMER_FUNCS does this conveniently.
+ * The driver further has to send the VBLANK event from its atomic_flush
+ * callback and control vblank from the CRTC's atomic_enable and atomic_disable
+ * callbacks. The callbacks are located in struct &drm_crtc_helper_funcs.
+ * The vblank helper library provides implementations of these callbacks
+ * for drivers without further requirements. The initializer macro
+ * DRM_CRTC_HELPER_VBLANK_FUNCS sets them coveniently.
*
* Once the driver enables vblank support with drm_vblank_init(), each
* CRTC's vblank timer fires according to the programmed display mode. By
@@ -25,6 +32,79 @@
* struct &drm_crtc_helper_funcs.handle_vblank_timeout.
*/
+/*
+ * VBLANK helpers
+ */
+
+/**
+ * drm_crtc_vblank_atomic_flush -
+ * Implements struct &drm_crtc_helper_funcs.atomic_flush
+ * @crtc: The CRTC
+ * @state: The atomic state to apply
+ *
+ * The helper drm_crtc_vblank_atomic_flush() implements atomic_flush of
+ * struct drm_crtc_helper_funcs for CRTCs that only need to send out a
+ * VBLANK event.
+ *
+ * See also struct &drm_crtc_helper_funcs.atomic_flush.
+ */
+void drm_crtc_vblank_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);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_atomic_flush);
+
+/**
+ * drm_crtc_vblank_atomic_enable - Implements struct &drm_crtc_helper_funcs.atomic_enable
+ * @crtc: The CRTC
+ * @state: The atomic state
+ *
+ * The helper drm_crtc_vblank_atomic_enable() implements atomic_enable
+ * of struct drm_crtc_helper_funcs for CRTCs the only need to enable VBLANKs.
+ *
+ * See also struct &drm_crtc_helper_funcs.atomic_enable.
+ */
+void drm_crtc_vblank_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ drm_crtc_vblank_on(crtc);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_atomic_enable);
+
+/**
+ * drm_crtc_vblank_atomic_disable - Implements struct &drm_crtc_helper_funcs.atomic_disable
+ * @crtc: The CRTC
+ * @state: The atomic state
+ *
+ * The helper drm_crtc_vblank_atomic_disable() implements atomic_disable
+ * of struct drm_crtc_helper_funcs for CRTCs the only need to disable VBLANKs.
+ *
+ * See also struct &drm_crtc_funcs.atomic_disable.
+ */
+void drm_crtc_vblank_atomic_disable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ drm_crtc_vblank_off(crtc);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_atomic_disable);
+
/*
* VBLANK timer
*/
diff --git a/include/drm/drm_vblank_helper.h b/include/drm/drm_vblank_helper.h
index 74a971d0cfba..fcd8a9b35846 100644
--- a/include/drm/drm_vblank_helper.h
+++ b/include/drm/drm_vblank_helper.h
@@ -6,8 +6,31 @@
#include <linux/hrtimer_types.h>
#include <linux/types.h>
+struct drm_atomic_state;
struct drm_crtc;
+/*
+ * VBLANK helpers
+ */
+
+void drm_crtc_vblank_atomic_flush(struct drm_crtc *crtc,
+ struct drm_atomic_state *state);
+void drm_crtc_vblank_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state);
+void drm_crtc_vblank_atomic_disable(struct drm_crtc *crtc,
+ struct drm_atomic_state *crtc_state);
+
+/**
+ * DRM_CRTC_HELPER_VBLANK_FUNCS - Default implementation for VBLANK helpers
+ *
+ * This macro initializes struct &drm_crtc_helper_funcs to default helpers
+ * for VBLANK handling.
+ */
+#define DRM_CRTC_HELPER_VBLANK_FUNCS \
+ .atomic_flush = drm_crtc_vblank_atomic_flush, \
+ .atomic_enable = drm_crtc_vblank_atomic_enable, \
+ .atomic_disable = drm_crtc_vblank_atomic_disable
+
/*
* VBLANK timer
*/
--
2.50.1
^ permalink raw reply related
* [PATCH v3 4/4] drm/hypervdrm: Use vblank timer
From: Thomas Zimmermann @ 2025-09-04 14:56 UTC (permalink / raw)
To: louis.chauvet, drawat.floss, hamohammed.sa, melissa.srw, mhklinux,
simona, airlied, maarten.lankhorst, ville.syrjala, lyude, javierm
Cc: dri-devel, linux-hyperv, Thomas Zimmermann
In-Reply-To: <20250904145806.430568-1-tzimmermann@suse.de>
HyperV's virtual hardware does not provide vblank interrupts. Use a
vblank timer to simulate the interrupt. 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>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
index 945b9482bcb3..6e6eb1c12a68 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
@@ -19,6 +19,8 @@
#include <drm/drm_probe_helper.h>
#include <drm/drm_panic.h>
#include <drm/drm_plane.h>
+#include <drm/drm_vblank.h>
+#include <drm/drm_vblank_helper.h>
#include "hyperv_drm.h"
@@ -111,11 +113,15 @@ 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 const struct drm_crtc_helper_funcs hyperv_crtc_helper_funcs = {
.atomic_check = drm_crtc_helper_atomic_check,
+ .atomic_flush = drm_crtc_vblank_atomic_flush,
.atomic_enable = hyperv_crtc_helper_atomic_enable,
+ .atomic_disable = drm_crtc_vblank_atomic_disable,
};
static const struct drm_crtc_funcs hyperv_crtc_funcs = {
@@ -125,6 +131,7 @@ 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,
+ DRM_CRTC_VBLANK_TIMER_FUNCS,
};
static int hyperv_plane_atomic_check(struct drm_plane *plane,
@@ -321,6 +328,10 @@ int hyperv_mode_config_init(struct hyperv_drm_device *hv)
return ret;
}
+ ret = drm_vblank_init(dev, 1);
+ if (ret)
+ return ret;
+
drm_mode_config_reset(dev);
return 0;
--
2.50.1
^ permalink raw reply related
* [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts
From: Thomas Zimmermann @ 2025-09-04 14:56 UTC (permalink / raw)
To: louis.chauvet, drawat.floss, hamohammed.sa, melissa.srw, mhklinux,
simona, airlied, maarten.lankhorst, ville.syrjala, lyude, javierm
Cc: dri-devel, linux-hyperv, Thomas Zimmermann
Compositors often depend on vblanks to limit their display-update
rate. Without, they see vblank events ASAP, which breaks the rate-
limit feature. This creates high CPU overhead. It is especially a
problem with virtual devices with fast framebuffer access.
The series moves vkms' vblank timer to DRM and converts the hyperv
DRM driver. An earlier version of this series contains examples of
other updated drivers. In principle, any DRM driver without vblank
hardware can use the timer.
The series has been motivated by a recent discussion about hypervdrm [1]
and other long-standing bug reports. [2][3]
v3:
- fix deadlock (Ville, Lyude)
v2:
- rework interfaces
[1] https://lore.kernel.org/dri-devel/20250523161522.409504-1-mhklinux@outlook.com/T/#ma2ebb52b60bfb0325879349377738fadcd7cb7ef
[2] https://bugzilla.suse.com/show_bug.cgi?id=1189174
[3] https://invent.kde.org/plasma/kwin/-/merge_requests/1229#note_284606
Thomas Zimmermann (4):
drm/vblank: Add vblank timer
drm/vblank: Add CRTC helpers for simple use cases
drm/vkms: Convert to DRM's vblank timer
drm/hypervdrm: Use vblank timer
Documentation/gpu/drm-kms-helpers.rst | 12 ++
drivers/gpu/drm/Makefile | 3 +-
drivers/gpu/drm/drm_vblank.c | 161 +++++++++++++++++-
drivers/gpu/drm/drm_vblank_helper.c | 176 ++++++++++++++++++++
drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 11 ++
drivers/gpu/drm/vkms/vkms_crtc.c | 83 +--------
drivers/gpu/drm/vkms/vkms_drv.h | 2 -
include/drm/drm_modeset_helper_vtables.h | 12 ++
include/drm/drm_vblank.h | 32 ++++
include/drm/drm_vblank_helper.h | 56 +++++++
10 files changed, 467 insertions(+), 81 deletions(-)
create mode 100644 drivers/gpu/drm/drm_vblank_helper.c
create mode 100644 include/drm/drm_vblank_helper.h
--
2.50.1
^ permalink raw reply
* [PATCH v3 1/4] drm/vblank: Add vblank timer
From: Thomas Zimmermann @ 2025-09-04 14:56 UTC (permalink / raw)
To: louis.chauvet, drawat.floss, hamohammed.sa, melissa.srw, mhklinux,
simona, airlied, maarten.lankhorst, ville.syrjala, lyude, javierm
Cc: dri-devel, linux-hyperv, Thomas Zimmermann
In-Reply-To: <20250904145806.430568-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").
The new implementation is part of the existing vblank support,
which sets up the timer automatically. Drivers only have to start
and cancel the vblank timer as part of enabling and disabling the
CRTC. The new vblank helper library provides callbacks for struct
drm_crtc_funcs.
The standard way for handling vblank is to call drm_crtc_handle_vblank().
Drivers that require additional processing, such as vkms, can init
handle_vblank_timeout in struct drm_crtc_helper_funcs to refer to
their timeout handler.
There's a possible deadlock between drm_crtc_handle_vblank() and
hrtimer_cancel(). [1] The implementation avoids to call hrtimer_cancel()
directly and instead signals to the timer function to not restart
itself.
v3:
- avoid deadlock when cancelling timer (Ville, Lyude)
v2:
- implement vblank timer entirely in vblank helpers
- downgrade overrun warning to debug
- fix docs
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Louis Chauvet <louis.chauvet@bootlin.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Link: https://lore.kernel.org/all/20250510094757.4174662-1-zengheng4@huawei.com/ # [1]
---
Documentation/gpu/drm-kms-helpers.rst | 12 ++
drivers/gpu/drm/Makefile | 3 +-
drivers/gpu/drm/drm_vblank.c | 161 ++++++++++++++++++++++-
drivers/gpu/drm/drm_vblank_helper.c | 96 ++++++++++++++
include/drm/drm_modeset_helper_vtables.h | 12 ++
include/drm/drm_vblank.h | 32 +++++
include/drm/drm_vblank_helper.h | 33 +++++
7 files changed, 346 insertions(+), 3 deletions(-)
create mode 100644 drivers/gpu/drm/drm_vblank_helper.c
create mode 100644 include/drm/drm_vblank_helper.h
diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 5139705089f2..781129f78b06 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -92,6 +92,18 @@ GEM Atomic Helper Reference
.. kernel-doc:: drivers/gpu/drm/drm_gem_atomic_helper.c
:export:
+VBLANK Helper Reference
+-----------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_vblank_helper.c
+ :doc: overview
+
+.. kernel-doc:: include/drm/drm_vblank_helper.h
+ :internal:
+
+.. kernel-doc:: drivers/gpu/drm/drm_vblank_helper.c
+ :export:
+
Simple KMS Helper Reference
===========================
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 4dafbdc8f86a..5ba4ffdb8055 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -150,7 +150,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_helper.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.c b/drivers/gpu/drm/drm_vblank.c
index 46f59883183d..364f5ba969d0 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -136,8 +136,17 @@
* vblanks after a timer has expired, which can be configured through the
* ``vblankoffdelay`` module parameter.
*
- * Drivers for hardware without support for vertical-blanking interrupts
- * must not call drm_vblank_init(). For such drivers, atomic helpers will
+ * Drivers for hardware without support for vertical-blanking interrupts can
+ * use DRM vblank timers to send vblank events at the rate of the current
+ * display mode's refresh. While not synchronized to the hardware's
+ * vertical-blanking regions, the timer helps DRM clients and compositors to
+ * adapt their update cycle to the display output. Drivers should set up
+ * vblanking as usual, but call drm_crtc_vblank_start_timer() and
+ * drm_crtc_vblank_cancel_timer() as part of their atomic mode setting.
+ * See also DRM vblank helpers for more information.
+ *
+ * Drivers without support for vertical-blanking interrupts nor timers must
+ * not call drm_vblank_init(). For these drivers, atomic helpers will
* automatically generate fake vblank events as part of the display update.
* This functionality also can be controlled by the driver by enabling and
* disabling struct drm_crtc_state.no_vblank.
@@ -508,6 +517,9 @@ static void drm_vblank_init_release(struct drm_device *dev, void *ptr)
drm_WARN_ON(dev, READ_ONCE(vblank->enabled) &&
drm_core_check_feature(dev, DRIVER_MODESET));
+ if (vblank->vblank_timer.crtc)
+ hrtimer_cancel(&vblank->vblank_timer.timer);
+
drm_vblank_destroy_worker(vblank);
timer_delete_sync(&vblank->disable_timer);
}
@@ -2162,3 +2174,148 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,
return ret;
}
+/*
+ * VBLANK timer
+ */
+
+static enum hrtimer_restart drm_vblank_timer_function(struct hrtimer *timer)
+{
+ struct drm_vblank_crtc_timer *vtimer =
+ container_of(timer, struct drm_vblank_crtc_timer, timer);
+ struct drm_crtc *crtc = vtimer->crtc;
+ const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
+ struct drm_device *dev = crtc->dev;
+ unsigned long flags;
+ ktime_t interval;
+ u64 ret_overrun;
+ bool succ;
+
+ spin_lock_irqsave(&vtimer->interval_lock, flags);
+ interval = vtimer->interval;
+ spin_unlock_irqrestore(&vtimer->interval_lock, flags);
+
+ if (!interval)
+ return HRTIMER_NORESTART;
+
+ ret_overrun = hrtimer_forward_now(&vtimer->timer, interval);
+ if (ret_overrun != 1)
+ drm_dbg_vbl(dev, "vblank timer overrun\n");
+
+ if (crtc_funcs->handle_vblank_timeout)
+ succ = crtc_funcs->handle_vblank_timeout(crtc);
+ else
+ succ = drm_crtc_handle_vblank(crtc);
+ if (!succ)
+ return HRTIMER_NORESTART;
+
+ return HRTIMER_RESTART;
+}
+
+/**
+ * drm_crtc_vblank_start_timer - Starts the vblank timer on the given CRTC
+ * @crtc: the CRTC
+ *
+ * Drivers should call this function from their CRTC's enable_vblank
+ * function to start a vblank timer. The timer will fire after the duration
+ * of a full frame. drm_crtc_vblank_cancel_timer() disables a running timer.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ */
+int drm_crtc_vblank_start_timer(struct drm_crtc *crtc)
+{
+ struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
+ struct drm_vblank_crtc_timer *vtimer = &vblank->vblank_timer;
+ unsigned long flags;
+
+ if (!vtimer->crtc) {
+ /*
+ * Set up the data structures on the first invocation.
+ */
+ vtimer->crtc = crtc;
+ spin_lock_init(&vtimer->interval_lock);
+ hrtimer_setup(&vtimer->timer, drm_vblank_timer_function,
+ CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ } else {
+ /*
+ * Timer should not be active. If it is, wait for the
+ * previous cancel operations to finish.
+ */
+ while (hrtimer_active(&vtimer->timer))
+ hrtimer_try_to_cancel(&vtimer->timer);
+ }
+
+ drm_calc_timestamping_constants(crtc, &crtc->mode);
+
+ spin_lock_irqsave(&vtimer->interval_lock, flags);
+ vtimer->interval = ns_to_ktime(vblank->framedur_ns);
+ spin_unlock_irqrestore(&vtimer->interval_lock, flags);
+
+ hrtimer_start(&vtimer->timer, vtimer->interval, HRTIMER_MODE_REL);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_crtc_vblank_start_timer);
+
+/**
+ * drm_crtc_vblank_start_timer - Cancels the given CRTC's vblank timer
+ * @crtc: the CRTC
+ *
+ * Drivers should call this function from their CRTC's disable_vblank
+ * function to stop a vblank timer.
+ */
+void drm_crtc_vblank_cancel_timer(struct drm_crtc *crtc)
+{
+ struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
+ struct drm_vblank_crtc_timer *vtimer = &vblank->vblank_timer;
+ unsigned long flags;
+
+ /*
+ * Calling hrtimer_cancel() can result in a deadlock with DRM's
+ * vblank_time_lime_lock and hrtimers' softirq_expiry_lock. So
+ * clear interval and indicate cancellation. The timer function
+ * will cancel itself on the next invocation.
+ */
+
+ spin_lock_irqsave(&vtimer->interval_lock, flags);
+ vtimer->interval = 0;
+ spin_unlock_irqrestore(&vtimer->interval_lock, flags);
+
+ hrtimer_try_to_cancel(&vtimer->timer);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_cancel_timer);
+
+/**
+ * drm_crtc_vblank_get_vblank_timeout - Returns the vblank timeout
+ * @crtc: The CRTC
+ * @vblank_time: Returns the next vblank timestamp
+ *
+ * The helper drm_crtc_vblank_get_vblank_timeout() returns the next vblank
+ * timestamp of the CRTC's vblank timer according to the timer's expiry
+ * time.
+ */
+void drm_crtc_vblank_get_vblank_timeout(struct drm_crtc *crtc, ktime_t *vblank_time)
+{
+ struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
+ struct drm_vblank_crtc_timer *vtimer = &vblank->vblank_timer;
+
+ if (!READ_ONCE(vblank->enabled)) {
+ *vblank_time = ktime_get();
+ return;
+ }
+
+ *vblank_time = READ_ONCE(vtimer->timer.node.expires);
+
+ if (drm_WARN_ON(crtc->dev, !ktime_compare(*vblank_time, vblank->time)))
+ return; /* Already expired */
+
+ /*
+ * 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 timestamp by one frame.
+ */
+ *vblank_time = ktime_sub(*vblank_time, vtimer->interval);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_get_vblank_timeout);
diff --git a/drivers/gpu/drm/drm_vblank_helper.c b/drivers/gpu/drm/drm_vblank_helper.c
new file mode 100644
index 000000000000..f94d1e706191
--- /dev/null
+++ b/drivers/gpu/drm/drm_vblank_helper.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: MIT
+
+#include <drm/drm_crtc.h>
+#include <drm/drm_managed.h>
+#include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_print.h>
+#include <drm/drm_vblank.h>
+#include <drm/drm_vblank_helper.h>
+
+/**
+ * DOC: overview
+ *
+ * The vblank helper library provides functions for supporting vertical
+ * blanking in DRM drivers.
+ *
+ * For vblank timers, several callback implementations are available.
+ * Drivers enable support for vblank timers by setting the vblank callbacks
+ * in struct &drm_crtc_funcs to the helpers provided by this library. The
+ * initializer macro DRM_CRTC_VBLANK_TIMER_FUNCS does this conveniently.
+ *
+ * Once the driver enables vblank support with drm_vblank_init(), each
+ * CRTC's vblank timer fires according to the programmed display mode. By
+ * default, the vblank timer invokes drm_crtc_handle_vblank(). Drivers with
+ * more specific requirements can set their own handler function in
+ * struct &drm_crtc_helper_funcs.handle_vblank_timeout.
+ */
+
+/*
+ * VBLANK timer
+ */
+
+/**
+ * drm_crtc_vblank_helper_enable_vblank_timer - Implements struct &drm_crtc_funcs.enable_vblank
+ * @crtc: The CRTC
+ *
+ * The helper drm_crtc_vblank_helper_enable_vblank_timer() implements
+ * enable_vblank of struct drm_crtc_helper_funcs for CRTCs that require
+ * a VBLANK timer. It sets up the timer on the first invocation. The
+ * started timer expires after the current frame duration. See struct
+ * &drm_vblank_crtc.framedur_ns.
+ *
+ * See also struct &drm_crtc_helper_funcs.enable_vblank.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ */
+int drm_crtc_vblank_helper_enable_vblank_timer(struct drm_crtc *crtc)
+{
+ return drm_crtc_vblank_start_timer(crtc);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_helper_enable_vblank_timer);
+
+/**
+ * drm_crtc_vblank_helper_disable_vblank_timer - Implements struct &drm_crtc_funcs.disable_vblank
+ * @crtc: The CRTC
+ *
+ * The helper drm_crtc_vblank_helper_disable_vblank_timer() implements
+ * disable_vblank of struct drm_crtc_funcs for CRTCs that require a
+ * VBLANK timer.
+ *
+ * See also struct &drm_crtc_helper_funcs.disable_vblank.
+ */
+void drm_crtc_vblank_helper_disable_vblank_timer(struct drm_crtc *crtc)
+{
+ drm_crtc_vblank_cancel_timer(crtc);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_helper_disable_vblank_timer);
+
+/**
+ * drm_crtc_vblank_helper_get_vblank_timestamp_from_timer -
+ * Implements struct &drm_crtc_funcs.get_vblank_timestamp
+ * @crtc: The CRTC
+ * @max_error: Maximum acceptable error
+ * @vblank_time: Returns the next vblank timestamp
+ * @in_vblank_irq: True is called from drm_crtc_handle_vblank()
+ *
+ * The helper drm_crtc_helper_get_vblank_timestamp_from_timer() implements
+ * get_vblank_timestamp of struct drm_crtc_funcs for CRTCs that require a
+ * VBLANK timer. It returns the timestamp according to the timer's expiry
+ * time.
+ *
+ * See also struct &drm_crtc_funcs.get_vblank_timestamp.
+ *
+ * Returns:
+ * True on success, or false otherwise.
+ */
+bool drm_crtc_vblank_helper_get_vblank_timestamp_from_timer(struct drm_crtc *crtc,
+ int *max_error,
+ ktime_t *vblank_time,
+ bool in_vblank_irq)
+{
+ drm_crtc_vblank_get_vblank_timeout(crtc, vblank_time);
+
+ return true;
+}
+EXPORT_SYMBOL(drm_crtc_vblank_helper_get_vblank_timestamp_from_timer);
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index ce7c7aeac887..fe32854b7ffe 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -490,6 +490,18 @@ struct drm_crtc_helper_funcs {
bool in_vblank_irq, int *vpos, int *hpos,
ktime_t *stime, ktime_t *etime,
const struct drm_display_mode *mode);
+
+ /**
+ * @handle_vblank_timeout: Handles timeouts of the vblank timer.
+ *
+ * Called by CRTC's the vblank timer on each timeout. Semantics is
+ * equivalient to drm_crtc_handle_vblank(). Implementations should
+ * invoke drm_crtc_handle_vblank() as part of processing the timeout.
+ *
+ * This callback is optional. If unset, the vblank timer invokes
+ * drm_crtc_handle_vblank() directly.
+ */
+ bool (*handle_vblank_timeout)(struct drm_crtc *crtc);
};
/**
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index 151ab1e85b1b..ffa564d79638 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -25,6 +25,7 @@
#define _DRM_VBLANK_H_
#include <linux/seqlock.h>
+#include <linux/hrtimer.h>
#include <linux/idr.h>
#include <linux/poll.h>
#include <linux/kthread.h>
@@ -103,6 +104,28 @@ struct drm_vblank_crtc_config {
bool disable_immediate;
};
+/**
+ * struct drm_vblank_crtc_timer - vblank timer for a CRTC
+ */
+struct drm_vblank_crtc_timer {
+ /**
+ * @timer: The vblank's high-resolution timer
+ */
+ struct hrtimer timer;
+ /**
+ * @interval_lock: Protects @interval
+ */
+ spinlock_t interval_lock;
+ /**
+ * @interval: Duration between two vblanks
+ */
+ ktime_t interval;
+ /**
+ * @crtc: The timer's CRTC
+ */
+ struct drm_crtc *crtc;
+};
+
/**
* struct drm_vblank_crtc - vblank tracking for a CRTC
*
@@ -254,6 +277,11 @@ struct drm_vblank_crtc {
* cancelled.
*/
wait_queue_head_t work_wait_queue;
+
+ /**
+ * @vblank_timer: Holds the state of the vblank timer
+ */
+ struct drm_vblank_crtc_timer vblank_timer;
};
struct drm_vblank_crtc *drm_crtc_vblank_crtc(struct drm_crtc *crtc);
@@ -290,6 +318,10 @@ wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc);
void drm_crtc_set_max_vblank_count(struct drm_crtc *crtc,
u32 max_vblank_count);
+int drm_crtc_vblank_start_timer(struct drm_crtc *crtc);
+void drm_crtc_vblank_cancel_timer(struct drm_crtc *crtc);
+void drm_crtc_vblank_get_vblank_timeout(struct drm_crtc *crtc, ktime_t *vblank_time);
+
/*
* Helpers for struct drm_crtc_funcs
*/
diff --git a/include/drm/drm_vblank_helper.h b/include/drm/drm_vblank_helper.h
new file mode 100644
index 000000000000..74a971d0cfba
--- /dev/null
+++ b/include/drm/drm_vblank_helper.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef _DRM_VBLANK_HELPER_H_
+#define _DRM_VBLANK_HELPER_H_
+
+#include <linux/hrtimer_types.h>
+#include <linux/types.h>
+
+struct drm_crtc;
+
+/*
+ * VBLANK timer
+ */
+
+int drm_crtc_vblank_helper_enable_vblank_timer(struct drm_crtc *crtc);
+void drm_crtc_vblank_helper_disable_vblank_timer(struct drm_crtc *crtc);
+bool drm_crtc_vblank_helper_get_vblank_timestamp_from_timer(struct drm_crtc *crtc,
+ int *max_error,
+ ktime_t *vblank_time,
+ bool in_vblank_irq);
+
+/**
+ * DRM_CRTC_VBLANK_TIMER_FUNCS - Default implementation for VBLANK timers
+ *
+ * This macro initializes struct &drm_crtc_funcs to default helpers for
+ * VBLANK timers.
+ */
+#define DRM_CRTC_VBLANK_TIMER_FUNCS \
+ .enable_vblank = drm_crtc_vblank_helper_enable_vblank_timer, \
+ .disable_vblank = drm_crtc_vblank_helper_disable_vblank_timer, \
+ .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp_from_timer
+
+#endif
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v2 1/4] drm/vblank: Add vblank timer
From: Thomas Zimmermann @ 2025-09-04 14:21 UTC (permalink / raw)
To: Lyude Paul, Ville Syrjälä
Cc: louis.chauvet, drawat.floss, hamohammed.sa, melissa.srw, mhklinux,
simona, airlied, maarten.lankhorst, dri-devel, linux-hyperv
In-Reply-To: <363bea2eb447d85d07e793b4a3e0abbbaea7c788.camel@redhat.com>
Hi,
thanks for the reply.
Am 02.09.25 um 17:58 schrieb Lyude Paul:
> A solution down below + some other things you should be aware of :)
>
> On Tue, 2025-09-02 at 16:16 +0200, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 02.09.25 um 15:27 schrieb Ville Syrjälä:
>>> On Mon, Sep 01, 2025 at 01:06:58PM +0200, Thomas Zimmermann wrote:
>>>> 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").
>>> Does this suffer from the same deadlocks as well?
>>> https://lore.kernel.org/all/20250510094757.4174662-1-zengheng4@huawei.com/
>> Thanks for this pointer. It has not been fixed yet, right? If vkms is
>> affected, this series probably is as well.
>>
>> Best regards
>> Thomas
>>
> Hi! I am glad I saw this mail fly by the list because I actually have a fix
> for this deadlock in my very incomplete vkms port to rust:
>
> https://gitlab.freedesktop.org/lyudess/linux/-/blob/rvkms-slim/drivers/gpu/drm/rvkms/crtc.rs#L336
>
> Basically what we do is keep track of when we're reporting a vblank event from
> the hrtimer thread we use to emulate vblanks along with if we're trying to
> stop the vblank timer:
>
> https://gitlab.freedesktop.org/lyudess/linux/-/blob/rvkms-slim/drivers/gpu/drm/rvkms/crtc.rs#L336
>
> Stopping the timer happens like this:
>
> https://gitlab.freedesktop.org/lyudess/linux/-/blob/rvkms-slim/drivers/gpu/drm/rvkms/crtc.rs#L232
>
> We grab the lock protecting cancelled and reporting, set cancelled, and then
> only attempt to cancel the timer if it's not busy reporting. If it is, we
> simply leave the timer be and rely on it noticing that cancelled has been set.
That code still contains race conditions AFAICT.
There are a number of ways to resolve this. I think the best would be to
remove the softirq_expiry_lock from the hrtimer's cancel logic. The
comment at [1] sounds like it is not really necessary to use that
specific lock.
[1]
https://elixir.bootlin.com/linux/v6.16.4/source/kernel/time/hrtimer.c#L1453
But IDK about the implications. Another idea is to push
drm_handle_vblank() out of the vblank timer callback. But that might
confuse DRM's vblank logic.
I ended up with a solution similar to yours, but much simpler. The
cancel function signals to the timer to not restart itself. The start
function waits for any dangling timer callback to stop itself. The code
further uses hrtimer_try_to_cancel(), which is not affected by the deadlock.
>
> The place where we actually unconditionally stop the timer is on
> atomic_disable:
>
> https://gitlab.freedesktop.org/lyudess/linux/-/blob/rvkms-slim/drivers/gpu/drm/rvkms/crtc.rs#L136
>
> Which works fine since unlike vblank_disable(), we're outside of the various
> vblank_time locks - and thus can wait on hrtimer_cancel() to complete without
> worrying about a deadlock.
I'd be careful with that. You're modifying the vblank state behind the
back of the vblank helpers. Probably works here, but can also backfire.
>
> JFYI, there is one other fix here you might want too. When vkms disables the
> vblank timer and then re-enables it later, I'm fairly certain it doesn't
> actually schedule the timer for the correct time:
>
> https://gitlab.freedesktop.org/lyudess/linux/-/blob/rvkms-slim/drivers/gpu/drm/vkms/vkms_crtc.c#L68
>
> Notice that it's a relative timer, and in enable_vblank() we schedule the
> timer to happen at now + frame_duration. The problem with this is that we
> should be rescheduling the hrtimer for when the next vblank would happen in
> relation to when the display pipe originally had vblank events enabled - not
> in relation to the current time.
>
> In other words: you could have vblanks enabled, disable them, and then
> (frame_duration / 2) nanoseconds later re-enable the timer - meaning that
> every vblank interrupt is now (frame_duration / 2) nanoseconds offset from
> when the vblank interval should actually be occurring. I'm not sure if this
> actually breaks anything though, but it certainly doesn't seem correct. To fix
> this in rvkms, we keep a timestamp of when we originally enabled the pipe and
> do some modulus fun to figure out the exact timestamp for the next vblank
> interval:
>
> https://gitlab.freedesktop.org/lyudess/linux/-/blob/rvkms-slim/drivers/gpu/drm/rvkms/crtc.rs#L168
>
> We also use absolute timers instead of relative to really make sure things get
> scheduled at just the right tie.
I've been wondering about this problem as well, but never found any
issues with it. See [2] for how it's currently done in vkms. If the
timer is still off, it takes the current time for the last vblank. And
if the timer is on, it computes the previous vblank time. So the values
should be fine.
[2]
https://elixir.bootlin.com/linux/v6.17-rc4/source/drivers/gpu/drm/vkms/vkms_crtc.c#L83
On the problem in general: we have the get_scanout_position [3]
callback, which would be the right thing to call here. The timeout could
be computed from the returned values. For virtual displays it wouldn't
matter much. But for hardware, the vblank timer could be synced to the
real vblank if the hardware provides the necessary information.
[3]
https://elixir.bootlin.com/linux/v6.17-rc4/source/include/drm/drm_modeset_helper_vtables.h#L489
Best regards
Thomas
>
> (I will try to port these fixes over to vkms at some point unless someone else
> gets to them first…)
>
--
--
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 1/1] x86/hyperv: Switch to msi_create_parent_irq_domain()
From: Thomas Gleixner @ 2025-09-04 9:57 UTC (permalink / raw)
To: Nuno Das Neves, Nam Cao, K . Y . Srinivasan, Marc Zyngier,
Haiyang Zhang, Wei Liu, Dexuan Cui, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H . Peter Anvin, linux-hyperv, linux-kernel
In-Reply-To: <0105fb29-1d42-49cb-8146-d2dfcb600843@linux.microsoft.com>
On Wed, Sep 03 2025 at 12:40, Nuno Das Neves wrote:
> On 7/18/2025 12:57 PM, Nam Cao wrote:
>> Move away from the legacy MSI domain setup, switch to use
>> msi_create_parent_irq_domain().
>>
>> While doing the conversion, I noticed that hv_irq_compose_msi_msg() is
>> doing more than it is supposed to (composing message content). The
>> interrupt allocation bits should be moved into hv_msi_domain_alloc().
>> However, I have no hardware to test this change, therefore I leave a TODO
>> note.
>>
>> Signed-off-by: Nam Cao <namcao@linutronix.de>
>> ---
>> arch/x86/hyperv/irqdomain.c | 111 ++++++++++++++++++++++++------------
>> drivers/hv/Kconfig | 1 +
>> 2 files changed, 77 insertions(+), 35 deletions(-)
>
> Tested on nested root partition.
>
> Looks good, thanks.
>
> Tested-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
I assume this goes through the hyper-V tree.
Acked-by: Thomas Gleixner <tglx@linutronix.de>
^ permalink raw reply
* RE: [PATCH v2 4/4] drm/hypervdrm: Use vblank timer
From: Javier Martinez Canillas @ 2025-09-04 5:50 UTC (permalink / raw)
To: Michael Kelley, Thomas Zimmermann, louis.chauvet@bootlin.com,
drawat.floss@gmail.com, hamohammed.sa@gmail.com,
melissa.srw@gmail.com, simona@ffwll.ch, airlied@gmail.com,
maarten.lankhorst@linux.intel.com
Cc: dri-devel@lists.freedesktop.org, linux-hyperv@vger.kernel.org
In-Reply-To: <SN6PR02MB4157EFAA19227FAFD21E1466D400A@SN6PR02MB4157.namprd02.prod.outlook.com>
Michael Kelley <mhklinux@outlook.com> writes:
Hello Michael,
> From: Javier Martinez Canillas <javierm@redhat.com> Sent: Tuesday, September 2, 2025 8:41 AM
>>
>> Thomas Zimmermann <tzimmermann@suse.de> writes:
>>
>> [...]
>>
>> >>
>> >> I'm not familiar with hyperv to know whether is a problem or not for the
>> >> host to not be notified that the guest display is disabled. But I thought
>> >> that should raise this question for the folks familiar with it.
>> >
>> > The feedback I got at
>> > https://lore.kernel.org/dri-devel/SN6PR02MB4157F630284939E084486AFED46FA@SN6PR02MB4157.namprd02.prod.outlook.com/
>> > is that the vblank timer solves the problem of excessive CPU consumption
>> > on hypervdrm. Ans that's also the observation I had with other drivers.
>> > I guess, telling the host about the disabled display would still make sense.
>> >
>>
>> Yes, I read the other thread you referenced and that's why I said that
>> your patch is correct to solve the issue.
>>
>> I just wanted to point out, since it could be that as a follow-up the
>> driver could need its own .atomic_disable instead of using the default
>> drm_crtc_vblank_atomic_disable(). Something like the following maybe:
>>
>> +static void hyperv_crtc_helper_atomic_disable(struct drm_crtc *crtc,
>> + struct drm_atomic_state *state)
>> +{
>> + struct hyperv_drm_device *hv = to_hv(crtc->dev);
>> + struct drm_plane *plane = &hv->plane;
>> + struct drm_plane_state *plane_state = plane->state;
>> + struct drm_crtc_state *crtc_state = crtc->state;
>> +
>> + hyperv_hide_hw_ptr(hv->hdev);
>> + /* Notify the host that the guest display is disabled */
>> + hyperv_update_situation(hv->hdev, 0, hv->screen_depth,
>> + crtc_state->mode.hdisplay,
>> + crtc_state->mode.vdisplay,
>> + plane_state->fb->pitches[0]);
>> +
>> + 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 = drm_crtc_vblank_atomic_flush,
>> .atomic_enable = hyperv_crtc_helper_atomic_enable,
>> - .atomic_disable = drm_crtc_vblank_atomic_disable,
>> + .atomic_disable = hyperv_crtc_helper_atomic_disable,
>> };
>
> I have some historical expertise in the Hyper-V fbdev driver from
> back when I was a Microsoft employee (I'm now retired). The fbdev
> driver is similar to the DRM driver in that it tells the Hyper-V host
> that the device is "active" during initial setup, but there's never a
> time when the driver tells Hyper-V that the device is "not active".
>
> I agree that symmetry suggests having disable function that sets
> "active" to 0, but I don't know what the effect would be. I don't know
> if Hyper-V anticipates any circumstances when the driver should tell
> Hyper-V the device is not active. My chances are not good in finding
> someone on the Hyper-V team who could give a definitive answer,
> as it's probably an area that is not under active development. The
> Hyper-V VMBus frame buffer device functionality is what it is, and
> isn't likely to be getting enhancements.
>
> I suggest that we assume it's not necessary to add a "disable"
> function, and proceed with Thomas' proposed changes to the Hyper-V
> DRM driver. Adding "disable" now risks breaking something due
> to effects we're unaware of. If in the future the need arises to mark
> the device not active, the "disable" function can be added after
> a clarifying conversation with the Hyper-V team.
>
> If anyone at Microsoft wants to chime in, please do so. :-)
>
Thanks a lot for the insight. I agree that probably is not worth the risk
to notify of a display disable, since is unclear what the effect would be.
> Michael
>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* RE: [PATCH v2 4/4] drm/hypervdrm: Use vblank timer
From: Michael Kelley @ 2025-09-04 3:38 UTC (permalink / raw)
To: Javier Martinez Canillas, Thomas Zimmermann,
louis.chauvet@bootlin.com, drawat.floss@gmail.com,
hamohammed.sa@gmail.com, melissa.srw@gmail.com, simona@ffwll.ch,
airlied@gmail.com, maarten.lankhorst@linux.intel.com
Cc: dri-devel@lists.freedesktop.org, linux-hyperv@vger.kernel.org
In-Reply-To: <877bygg8vb.fsf@minerva.mail-host-address-is-not-set>
From: Javier Martinez Canillas <javierm@redhat.com> Sent: Tuesday, September 2, 2025 8:41 AM
>
> Thomas Zimmermann <tzimmermann@suse.de> writes:
>
> [...]
>
> >>
> >> I'm not familiar with hyperv to know whether is a problem or not for the
> >> host to not be notified that the guest display is disabled. But I thought
> >> that should raise this question for the folks familiar with it.
> >
> > The feedback I got at
> > https://lore.kernel.org/dri-devel/SN6PR02MB4157F630284939E084486AFED46FA@SN6PR02MB4157.namprd02.prod.outlook.com/
> > is that the vblank timer solves the problem of excessive CPU consumption
> > on hypervdrm. Ans that's also the observation I had with other drivers.
> > I guess, telling the host about the disabled display would still make sense.
> >
>
> Yes, I read the other thread you referenced and that's why I said that
> your patch is correct to solve the issue.
>
> I just wanted to point out, since it could be that as a follow-up the
> driver could need its own .atomic_disable instead of using the default
> drm_crtc_vblank_atomic_disable(). Something like the following maybe:
>
> +static void hyperv_crtc_helper_atomic_disable(struct drm_crtc *crtc,
> + struct drm_atomic_state *state)
> +{
> + struct hyperv_drm_device *hv = to_hv(crtc->dev);
> + struct drm_plane *plane = &hv->plane;
> + struct drm_plane_state *plane_state = plane->state;
> + struct drm_crtc_state *crtc_state = crtc->state;
> +
> + hyperv_hide_hw_ptr(hv->hdev);
> + /* Notify the host that the guest display is disabled */
> + hyperv_update_situation(hv->hdev, 0, hv->screen_depth,
> + crtc_state->mode.hdisplay,
> + crtc_state->mode.vdisplay,
> + plane_state->fb->pitches[0]);
> +
> + 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 = drm_crtc_vblank_atomic_flush,
> .atomic_enable = hyperv_crtc_helper_atomic_enable,
> - .atomic_disable = drm_crtc_vblank_atomic_disable,
> + .atomic_disable = hyperv_crtc_helper_atomic_disable,
> };
I have some historical expertise in the Hyper-V fbdev driver from
back when I was a Microsoft employee (I'm now retired). The fbdev
driver is similar to the DRM driver in that it tells the Hyper-V host
that the device is "active" during initial setup, but there's never a
time when the driver tells Hyper-V that the device is "not active".
I agree that symmetry suggests having disable function that sets
"active" to 0, but I don't know what the effect would be. I don't know
if Hyper-V anticipates any circumstances when the driver should tell
Hyper-V the device is not active. My chances are not good in finding
someone on the Hyper-V team who could give a definitive answer,
as it's probably an area that is not under active development. The
Hyper-V VMBus frame buffer device functionality is what it is, and
isn't likely to be getting enhancements.
I suggest that we assume it's not necessary to add a "disable"
function, and proceed with Thomas' proposed changes to the Hyper-V
DRM driver. Adding "disable" now risks breaking something due
to effects we're unaware of. If in the future the need arises to mark
the device not active, the "disable" function can be added after
a clarifying conversation with the Hyper-V team.
If anyone at Microsoft wants to chime in, please do so. :-)
Michael
^ permalink raw reply
* Re: [PATCH V0 0/2] Fix CONFIG_HYPERV and vmbus related anamoly
From: Mukesh R @ 2025-09-04 2:16 UTC (permalink / raw)
To: Michael Kelley, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
linux-hyperv@vger.kernel.org, netdev@vger.kernel.org,
linux-pci@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-arch@vger.kernel.org,
virtualization@lists.linux.dev
Cc: maarten.lankhorst@linux.intel.com, mripard@kernel.org,
tzimmermann@suse.de, airlied@gmail.com, simona@ffwll.ch,
jikos@kernel.org, bentiss@kernel.org, kys@microsoft.com,
haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
dmitry.torokhov@gmail.com, andrew+netdev@lunn.ch,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, bhelgaas@google.com,
James.Bottomley@HansenPartnership.com, martin.petersen@oracle.com,
gregkh@linuxfoundation.org, deller@gmx.de, arnd@arndb.de,
sgarzare@redhat.com, horms@kernel.org
In-Reply-To: <SN6PR02MB4157917D84D00DBDAF54BD69D406A@SN6PR02MB4157.namprd02.prod.outlook.com>
On 9/2/25 07:42, Michael Kelley wrote:
> From: Mukesh Rathor <mrathor@linux.microsoft.com> Sent: Wednesday, August 27, 2025 6:00 PM
>>
>> At present, drivers/Makefile will subst =m to =y for CONFIG_HYPERV for hv
>> subdir. Also, drivers/hv/Makefile replaces =m to =y to build in
>> hv_common.c that is needed for the drivers. Moreover, vmbus driver is
>> built if CONFIG_HYPER is set, either loadable or builtin.
>>
>> This is not a good approach. CONFIG_HYPERV is really an umbrella config that
>> encompasses builtin code and various other things and not a dedicated config
>> option for VMBUS. Vmbus should really have a config option just like
>> CONFIG_HYPERV_BALLOON etc. This small series introduces CONFIG_HYPERV_VMBUS
>> to build VMBUS driver and make that distinction explicit. With that
>> CONFIG_HYPERV could be changed to bool.
>
> Separating the core hypervisor support (CONFIG_HYPERV) from the VMBus
> support (CONFIG_HYPERV_VMBUS) makes sense to me. Overall the code
> is already mostly in separate source files code, though there's some
> entanglement in the handling of VMBus interrupts, which could be
> improved later.
>
> However, I have a compatibility concern. Consider this scenario:
>
> 1) Assume running in a Hyper-V VM with a current Linux kernel version
> built with CONFIG_HYPERV=m.
> 2) Grab a new version of kernel source code that contains this patch set.
> 3) Run 'make olddefconfig' to create the .config file for the new kernel.
> 4) Build the new kernel. This succeeds.
> 5) Install and run the new kernel in the Hyper-V VM. This fails.
>
> The failure occurs because CONFIG_HYPERV=m is no longer legal,
> so the .config file created in Step 3 has CONFIG_HYPERV=n. The
> newly built kernel has no Hyper-V support and won't run in a
> Hyper-V VM.
>
> As a second issue, if in Step 1 the current kernel was built with
> CONFIG_HYPERV=y, then the .config file for the new kernel will have
> CONFIG_HYPERV=y, which is better. But CONFIG_HYPERV_VMBUS
> defaults to 'n', so the new kernel doesn't have any VMBus drivers
> and won't run in a typical Hyper-V VM.
>
> The second issue could be fixed by assigning CONFIG_HYPERV_VMBUS
> a default value, such as whatever CONFIG_HYPERV is set to. But
> I'm not sure how to fix the first issue, except by continuing to
> allow CONFIG_HYPERV=m.
To certain extent, imo, users are expected to check config files
for changes when moving to new versions/releases, so it would be a
one time burden. CONFIG_HYPERV=m is just broken imo as one sees that
in .config but magically symbols in drivers/hv are in kerenel.
Thanks,
-Mukesh
> See additional minor comments in Patches 1 and 2.
>
> Michael
>
>>
>> For now, hv_common.c is left as is to reduce conflicts for upcoming patches,
>> but once merges are mostly done, that and some others should be moved to
>> virt/hyperv directory.
>>
>> Mukesh Rathor (2):
>> hyper-v: Add CONFIG_HYPERV_VMBUS option
>> hyper-v: Make CONFIG_HYPERV bool
>>
>> drivers/Makefile | 2 +-
>> drivers/gpu/drm/Kconfig | 2 +-
>> drivers/hid/Kconfig | 2 +-
>> drivers/hv/Kconfig | 14 ++++++++++----
>> drivers/hv/Makefile | 4 ++--
>> drivers/input/serio/Kconfig | 4 ++--
>> drivers/net/hyperv/Kconfig | 2 +-
>> drivers/pci/Kconfig | 2 +-
>> drivers/scsi/Kconfig | 2 +-
>> drivers/uio/Kconfig | 2 +-
>> drivers/video/fbdev/Kconfig | 2 +-
>> include/asm-generic/mshyperv.h | 8 +++++---
>> net/vmw_vsock/Kconfig | 2 +-
>> 13 files changed, 28 insertions(+), 20 deletions(-)
>>
>> --
>> 2.36.1.vfs.0.0
>>
^ permalink raw reply
* [PATCH v0 4/6] x86/hyperv: Add trampoline asm code to transition from hypervisor
From: Mukesh Rathor @ 2025-09-04 2:10 UTC (permalink / raw)
To: linux-hyperv, linux-kernel, linux-arch
Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, arnd
In-Reply-To: <20250904021017.1628993-1-mrathor@linux.microsoft.com>
This commit introduces a small asm stub to transition from the hypervisor
to linux upon devirtualization. At a high level, during panic of either
the hypervisor or the dom0 (aka root), the nmi handler asks hypervisor
to devirtualize. As part of that, the arguments include the entry point
to return to linux. This asm stub implements the entry point.
The stub is entered in protected mode, uses temporary gdt and page table
to enable long mode and get to kernel entry point to restore full kernel
context to resume execution to kexec.
Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
---
arch/x86/hyperv/hv_trampoline.S | 99 +++++++++++++++++++++++++++++++++
1 file changed, 99 insertions(+)
create mode 100644 arch/x86/hyperv/hv_trampoline.S
diff --git a/arch/x86/hyperv/hv_trampoline.S b/arch/x86/hyperv/hv_trampoline.S
new file mode 100644
index 000000000000..307663c8891e
--- /dev/null
+++ b/arch/x86/hyperv/hv_trampoline.S
@@ -0,0 +1,99 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * X86 specific Hyper-V kdump/crash related code.
+ *
+ * Copyright (C) 2025, Microsoft, Inc.
+ *
+ */
+#include <linux/linkage.h>
+#include <asm/alternative.h>
+#include <asm/msr.h>
+#include <asm/processor-flags.h>
+#include <asm/nospec-branch.h>
+
+/*
+ * void noreturn hv_crash_asm32(arg1)
+ * arg1 == edi == 32bit PA of struct hv_crash_trdata
+ *
+ * The hypervisor jumps here upon devirtualization in protected mode. This
+ * code gets copied to a page in the low 4G ie, 32bit space so it can run
+ * in the protected mode. Hence we cannot use any compile/link time offsets or
+ * addresses. It restores long mode via temporary gdt and page tables and
+ * eventually jumps to kernel code entry at HV_CRASHDATA_OFFS_C_entry.
+ *
+ * PreCondition (ie, Hypervisor call back ABI):
+ * o CR0 is set to 0x0021: PE(prot mode) and NE are set, paging is disabled
+ * o CR4 is set to 0x0
+ * o IA32_EFER is set to 0x901 (SCE and NXE are set)
+ * o EDI is set to the Arg passed to HVCALL_DISABLE_HYP_EX.
+ * o CS, DS, ES, FS, GS are all initialized with a base of 0 and limit 0xFFFF
+ * o IDTR, TR and GDTR are initialized with a base of 0 and limit of 0xFFFF
+ * o LDTR is initialized as invalid (limit of 0)
+ * o MSR PAT is power on default.
+ * o Other state/registers are cleared. All TLBs flushed.
+ *
+ * See Intel SDM 10.8.5
+ */
+
+#define HV_CRASHDATA_OFFS_TRAMPCR3 0x0 /* 0 */
+#define HV_CRASHDATA_OFFS_KERNCR3 0x8 /* 8 */
+#define HV_CRASHDATA_OFFS_GDTRLIMIT 0x12 /* 18 */
+#define HV_CRASHDATA_OFFS_CS_JMPTGT 0x28 /* 40 */
+#define HV_CRASHDATA_OFFS_C_entry 0x30 /* 48 */
+#define HV_CRASHDATA_TRAMPOLINE_CS 0x8
+
+ .text
+ .code32
+
+SYM_CODE_START(hv_crash_asm32)
+ movl $X86_CR4_PAE, %ecx
+ movl %ecx, %cr4
+
+ movl %edi, %ebx
+ add $HV_CRASHDATA_OFFS_TRAMPCR3, %ebx
+ movl %cs:(%ebx), %eax
+ movl %eax, %cr3
+
+ # Setup EFER for long mode now.
+ movl $MSR_EFER, %ecx
+ rdmsr
+ btsl $_EFER_LME, %eax
+ wrmsr
+
+ # Turn paging on using the temp 32bit trampoline page table.
+ movl %cr0, %eax
+ orl $(X86_CR0_PG), %eax
+ movl %eax, %cr0
+
+ /* since kernel cr3 could be above 4G, we need to be in the long mode
+ * before we can load 64bits of the kernel cr3. We use a temp gdt for
+ * that with CS.L=1 and CS.D=0 */
+ mov %edi, %eax
+ add $HV_CRASHDATA_OFFS_GDTRLIMIT, %eax
+ lgdtl %cs:(%eax)
+
+ /* not done yet, restore CS now to switch to CS.L=1 */
+ mov %edi, %eax
+ add $HV_CRASHDATA_OFFS_CS_JMPTGT, %eax
+ ljmp %cs:*(%eax)
+
+ /* we now run in full 64bit IA32-e long mode, CS.L=1 and CS.D=0 */
+ .code64
+ .balign 8
+SYM_INNER_LABEL(hv_crash_asm64_lbl, SYM_L_GLOBAL)
+ /* restore kernel page tables so we can jump to kernel code */
+ mov %edi, %eax
+ add $HV_CRASHDATA_OFFS_KERNCR3, %eax
+ movq %cs:(%eax), %rbx
+ movq %rbx, %cr3
+
+ mov %edi, %eax
+ add $HV_CRASHDATA_OFFS_C_entry, %eax
+ movq %cs:(%eax), %rbx
+ ANNOTATE_RETPOLINE_SAFE
+ jmp *%rbx
+
+ int $3
+
+SYM_INNER_LABEL(hv_crash_asm32_end, SYM_L_GLOBAL)
+SYM_CODE_END(hv_crash_asm32)
--
2.36.1.vfs.0.0
^ permalink raw reply related
* [PATCH v0 3/6] Hyper-V: Add definitions for hypervisor crash dump support
From: Mukesh Rathor @ 2025-09-04 2:10 UTC (permalink / raw)
To: linux-hyperv, linux-kernel, linux-arch
Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, arnd
In-Reply-To: <20250904021017.1628993-1-mrathor@linux.microsoft.com>
This commit adds data structures for hypervisor crash dump support
to the hypervisor host ABI public header file. Details of their usages
can be found in subsequent commits.
Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
---
include/hyperv/hvhdk_mini.h | 55 +++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
index 858f6a3925b3..ad9a8048fb4e 100644
--- a/include/hyperv/hvhdk_mini.h
+++ b/include/hyperv/hvhdk_mini.h
@@ -116,6 +116,17 @@ enum hv_system_property {
/* Add more values when needed */
HV_SYSTEM_PROPERTY_SCHEDULER_TYPE = 15,
HV_DYNAMIC_PROCESSOR_FEATURE_PROPERTY = 21,
+ HV_SYSTEM_PROPERTY_CRASHDUMPAREA = 47,
+};
+
+#define HV_PFN_RANGE_PGBITS 24 /* HV_SPA_PAGE_RANGE_ADDITIONAL_PAGES_BITS */
+union hv_pfn_range { /* HV_SPA_PAGE_RANGE */
+ u64 as_uint64;
+ struct {
+ /* 39:0: base pfn. 63:40: additional pages */
+ u64 base_pfn : 64 - HV_PFN_RANGE_PGBITS;
+ u64 add_pfns : HV_PFN_RANGE_PGBITS;
+ } __packed;
};
enum hv_dynamic_processor_feature_property {
@@ -142,6 +153,8 @@ struct hv_output_get_system_property {
#if IS_ENABLED(CONFIG_X86)
u64 hv_processor_feature_value;
#endif
+ union hv_pfn_range hv_cda_info; /* CrashdumpAreaAddress */
+ u64 hv_tramp_pa; /* CrashdumpTrampolineAddress */
};
} __packed;
@@ -234,6 +247,48 @@ union hv_gpa_page_access_state {
u8 as_uint8;
} __packed;
+enum hv_crashdump_action {
+ HV_CRASHDUMP_NONE = 0,
+ HV_CRASHDUMP_SUSPEND_ALL_VPS,
+ HV_CRASHDUMP_PREPARE_FOR_STATE_SAVE,
+ HV_CRASHDUMP_STATE_SAVED,
+ HV_CRASHDUMP_ENTRY,
+};
+
+struct hv_partition_event_root_crashdump_input {
+ u32 crashdump_action; /* enum hv_crashdump_action */
+} __packed;
+
+struct hv_input_disable_hyp_ex { /* HV_X64_INPUT_DISABLE_HYPERVISOR_EX */
+ u64 rip;
+ u64 arg;
+} __packed;
+
+struct hv_crashdump_area { /* HV_CRASHDUMP_AREA */
+ u32 version;
+ union {
+ u32 flags_as_uint32;
+ struct {
+ u32 cda_valid : 1;
+ u32 cda_unused : 31;
+ } __packed;
+ };
+ /* more unused fields */
+} __packed;
+
+union hv_partition_event_input {
+ struct hv_partition_event_root_crashdump_input crashdump_input;
+};
+
+enum hv_partition_event {
+ HV_PARTITION_EVENT_ROOT_CRASHDUMP = 2,
+};
+
+struct hv_input_notify_partition_event {
+ u32 event; /* enum hv_partition_event */
+ union hv_partition_event_input input;
+} __packed;
+
struct hv_lp_startup_status {
u64 hv_status;
u64 substatus1;
--
2.36.1.vfs.0.0
^ permalink raw reply related
* [PATCH v0 6/6] Hyper-V: Enable build of hypervisor crash collection files
From: Mukesh Rathor @ 2025-09-04 2:10 UTC (permalink / raw)
To: linux-hyperv, linux-kernel, linux-arch
Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, arnd
In-Reply-To: <20250904021017.1628993-1-mrathor@linux.microsoft.com>
This final commit enables build of the new files introduced in the
earlier commits and adds call to do the setup during boot.
Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
---
arch/x86/hyperv/Makefile | 6 ++++++
arch/x86/hyperv/hv_init.c | 2 ++
include/asm-generic/mshyperv.h | 9 +++++++++
3 files changed, 17 insertions(+)
diff --git a/arch/x86/hyperv/Makefile b/arch/x86/hyperv/Makefile
index d55f494f471d..6f5d97cddd80 100644
--- a/arch/x86/hyperv/Makefile
+++ b/arch/x86/hyperv/Makefile
@@ -5,4 +5,10 @@ obj-$(CONFIG_HYPERV_VTL_MODE) += hv_vtl.o
ifdef CONFIG_X86_64
obj-$(CONFIG_PARAVIRT_SPINLOCKS) += hv_spinlock.o
+
+ ifdef CONFIG_MSHV_ROOT
+ CFLAGS_REMOVE_hv_trampoline.o += -pg
+ CFLAGS_hv_trampoline.o += -fno-stack-protector
+ obj-$(CONFIG_CRASH_DUMP) += hv_crash.o hv_trampoline.o
+ endif
endif
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 6919f1af2e51..e3a4650b9ec7 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -513,6 +513,8 @@ void __init hyperv_init(void)
/* mark ram reserved for hypervisor as owned by hypervisor */
hv_mark_resources();
+
+ hv_root_crash_init();
} else {
hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg);
wrmsrq(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index 1d2ad1304ad4..57cc3e0cac90 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -369,6 +369,15 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages);
int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id);
int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags);
+#if CONFIG_CRASH_DUMP
+void hv_root_crash_init(void);
+void hv_crash_asm32(void);
+void hv_crash_asm64_lbl(void);
+void hv_crash_asm32_end(void);
+#else /* CONFIG_CRASH_DUMP */
+static inline void hv_root_crash_init(void) {}
+#endif /* CONFIG_CRASH_DUMP */
+
#else /* CONFIG_MSHV_ROOT */
static inline bool hv_root_partition(void) { return false; }
static inline bool hv_l1vh_partition(void) { return false; }
--
2.36.1.vfs.0.0
^ permalink raw reply related
* [PATCH v0 2/6] Hyper-V: Add two new hypercall numbers to guest ABI public header
From: Mukesh Rathor @ 2025-09-04 2:10 UTC (permalink / raw)
To: linux-hyperv, linux-kernel, linux-arch
Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, arnd
In-Reply-To: <20250904021017.1628993-1-mrathor@linux.microsoft.com>
In preparation for upcoming crashdump patches, this commit copies two new
hypercall numbers to the guest ABI header published by Hyper-V. One to
notify hypervisor of an event that occurs in the root partition, other
to ask hypervisor to disable the hypervisor.
Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
---
include/hyperv/hvgdk_mini.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
index 79b7324e4ef5..b288906c79d0 100644
--- a/include/hyperv/hvgdk_mini.h
+++ b/include/hyperv/hvgdk_mini.h
@@ -469,6 +469,7 @@ union hv_vp_assist_msr_contents { /* HV_REGISTER_VP_ASSIST_PAGE */
#define HVCALL_MAP_DEVICE_INTERRUPT 0x007c
#define HVCALL_UNMAP_DEVICE_INTERRUPT 0x007d
#define HVCALL_RETARGET_INTERRUPT 0x007e
+#define HVCALL_NOTIFY_PARTITION_EVENT 0x0087
#define HVCALL_NOTIFY_PORT_RING_EMPTY 0x008b
#define HVCALL_REGISTER_INTERCEPT_RESULT 0x0091
#define HVCALL_ASSERT_VIRTUAL_INTERRUPT 0x0094
@@ -492,6 +493,7 @@ union hv_vp_assist_msr_contents { /* HV_REGISTER_VP_ASSIST_PAGE */
#define HVCALL_GET_VP_CPUID_VALUES 0x00f4
#define HVCALL_MMIO_READ 0x0106
#define HVCALL_MMIO_WRITE 0x0107
+#define HVCALL_DISABLE_HYP_EX 0x010f
/* HV_HYPERCALL_INPUT */
#define HV_HYPERCALL_RESULT_MASK GENMASK_ULL(15, 0)
--
2.36.1.vfs.0.0
^ permalink raw reply related
* [PATCH v0 5/6] x86/hyperv: Implement hypervisor ram collection into vmcore
From: Mukesh Rathor @ 2025-09-04 2:10 UTC (permalink / raw)
To: linux-hyperv, linux-kernel, linux-arch
Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, arnd
In-Reply-To: <20250904021017.1628993-1-mrathor@linux.microsoft.com>
This commit introduces a new file to enable collection of hypervisor ram
into the vmcore collected by linux. By default, the hypervisor ram is locked,
ie, protected via hw page table. Hyper-V implements a disable hypercall which
essentially devirtualizes the system on the fly. This mechanism makes the
hypervisor ram accessible to linux without any extra work because it is
already mapped into linux address space. Details of the implementation
are available in the file prologue.
Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
---
arch/x86/hyperv/hv_crash.c | 618 +++++++++++++++++++++++++++++++++++++
1 file changed, 618 insertions(+)
create mode 100644 arch/x86/hyperv/hv_crash.c
diff --git a/arch/x86/hyperv/hv_crash.c b/arch/x86/hyperv/hv_crash.c
new file mode 100644
index 000000000000..50c54d39f0e2
--- /dev/null
+++ b/arch/x86/hyperv/hv_crash.c
@@ -0,0 +1,618 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * X86 specific Hyper-V kdump/crash support module
+ *
+ * Copyright (C) 2025, Microsoft, Inc.
+ *
+ * This module implements hypervisor ram collection into vmcore for both
+ * cases of the hypervisor crash and linux dom0/root crash. Hyper-V implements
+ * a devirtualization hypercall with a 32bit protected mode ABI callback. This
+ * mechanism must be used to unlock hypervisor ram. Since the hypervisor ram
+ * is already mapped in linux, it is automatically collected into linux vmcore,
+ * and can be examined by the crash command (raw ram dump) or windbg.
+ *
+ * At a high level:
+ *
+ * Hypervisor Crash:
+ * Upon crash, hypervisor goes into minimal dispatch loop, a restrictive
+ * mode with very limited hypercall and msr support. Each cpu then injects
+ * NMIs into dom0/root vcpus. A shared page is used to check by linux if
+ * hypervisor has crashed in the nmi handler. This shared page is setup
+ * in hv_root_crash_init during boot.
+ *
+ * Linux Crash:
+ * In case of linux crash, the callback hv_crash_stop_other_cpus will send
+ * NMIs to all cpus, then proceed to the crash_nmi_callback where it waits
+ * for all cpus to be in NMI.
+ *
+ * NMI Handler (upon quorum):
+ * Hyper-V requires the disable hypervisor must be done from the bsp. So
+ * the bsp nmi handler saves current context, does some fixups and makes
+ * the hypercall to disable the hypervisor, ie, devirtualize. Hypervisor
+ * at that point will suspend all vcpus (except the bsp), unlock all its
+ * ram, and return to linux at the 32bit mode entry RIP.
+ *
+ * Linux 32bit entry trampoline will then restore long mode and call C
+ * function here to restore context and continue execution to crash kexec.
+ */
+
+#include <linux/delay.h>
+#include <linux/kexec.h>
+#include <linux/crash_dump.h>
+#include <linux/panic.h>
+#include <asm/apic.h>
+#include <asm/desc.h>
+#include <asm/page.h>
+#include <asm/pgalloc.h>
+#include <asm/mshyperv.h>
+#include <asm/nmi.h>
+#include <asm/idtentry.h>
+#include <asm/reboot.h>
+#include <asm/intel_pt.h>
+
+int hv_crash_enabled;
+EXPORT_SYMBOL_GPL(hv_crash_enabled);
+
+struct hv_crash_ctxt {
+ ulong rsp;
+ ulong cr0;
+ ulong cr2;
+ ulong cr4;
+ ulong cr8;
+
+ u16 cs;
+ u16 ss;
+ u16 ds;
+ u16 es;
+ u16 fs;
+ u16 gs;
+
+ u16 gdt_fill;
+ struct desc_ptr gdtr;
+ char idt_fill[6];
+ struct desc_ptr idtr;
+
+ u64 gsbase;
+ u64 efer;
+ u64 pat;
+};
+static struct hv_crash_ctxt hv_crash_ctxt;
+
+/* Shared hypervisor page that contains crash dump area we peek into.
+ * NB: windbg looks for "hv_cda" symbol so don't change it.
+ */
+static struct hv_crashdump_area *hv_cda;
+
+static u32 trampoline_pa, devirt_cr3arg;
+static atomic_t crash_cpus_wait;
+static void *hv_crash_ptpgs[4];
+static int hv_has_crashed, lx_has_crashed;
+
+/* This cannot be inlined as it needs stack */
+static noinline __noclone void hv_crash_restore_tss(void)
+{
+ load_TR_desc();
+}
+
+/* This cannot be inlined as it needs stack */
+static noinline void hv_crash_clear_kernpt(void)
+{
+ pgd_t *pgd;
+ p4d_t *p4d;
+
+ /* Clear entry so it's not confusing to someone looking at the core */
+ pgd = pgd_offset_k(trampoline_pa);
+ p4d = p4d_offset(pgd, trampoline_pa);
+ native_p4d_clear(p4d);
+}
+
+/*
+ * This is the C entry point from the asm glue code after the devirt hypercall.
+ * We enter here in IA32-e long mode, ie, full 64bit mode running on kernel
+ * page tables with our below 4G page identity mapped, but using a temporary
+ * GDT. ds/fs/gs/es are null. ss is not usable. bp is null. stack is not
+ * available. We restore kernel GDT, and rest of the context, and continue
+ * to kexec.
+ */
+static asmlinkage void __noreturn hv_crash_c_entry(void)
+{
+ struct hv_crash_ctxt *ctxt = &hv_crash_ctxt;
+
+ /* first thing, restore kernel gdt */
+ native_load_gdt(&ctxt->gdtr);
+
+ asm volatile("movw %%ax, %%ss" : : "a"(ctxt->ss));
+ asm volatile("movq %0, %%rsp" : : "m"(ctxt->rsp));
+
+ asm volatile("movw %%ax, %%ds" : : "a"(ctxt->ds));
+ asm volatile("movw %%ax, %%es" : : "a"(ctxt->es));
+ asm volatile("movw %%ax, %%fs" : : "a"(ctxt->fs));
+ asm volatile("movw %%ax, %%gs" : : "a"(ctxt->gs));
+
+ native_wrmsrq(MSR_IA32_CR_PAT, ctxt->pat);
+ asm volatile("movq %0, %%cr0" : : "r"(ctxt->cr0));
+
+ asm volatile("movq %0, %%cr8" : : "r"(ctxt->cr8));
+ asm volatile("movq %0, %%cr4" : : "r"(ctxt->cr4));
+ asm volatile("movq %0, %%cr2" : : "r"(ctxt->cr4));
+
+ native_load_idt(&ctxt->idtr);
+ native_wrmsrq(MSR_GS_BASE, ctxt->gsbase);
+ native_wrmsrq(MSR_EFER, ctxt->efer);
+
+ /* restore the original kernel CS now via far return */
+ asm volatile("movzwq %0, %%rax\n\t"
+ "pushq %%rax\n\t"
+ "pushq $1f\n\t"
+ "lretq\n\t"
+ "1:nop\n\t" : : "m"(ctxt->cs) : "rax");
+
+ /* We are in asmlinkage without stack frame, hence make a C function
+ * call which will buy stack frame to restore the tss or clear PT entry.
+ */
+ hv_crash_restore_tss();
+ hv_crash_clear_kernpt();
+
+ /* we are now fully in devirtualized normal kernel mode */
+ __crash_kexec(NULL);
+
+ BUG();
+}
+/* Tell gcc we are using lretq long jump in the above function intentionally */
+STACK_FRAME_NON_STANDARD(hv_crash_c_entry);
+
+static void hv_mark_tss_not_busy(void)
+{
+ struct desc_struct *desc = get_current_gdt_rw();
+ tss_desc tss;
+
+ memcpy(&tss, &desc[GDT_ENTRY_TSS], sizeof(tss_desc));
+ tss.type = 0x9; /* available 64-bit TSS. 0xB is busy TSS */
+ write_gdt_entry(desc, GDT_ENTRY_TSS, &tss, DESC_TSS);
+}
+
+/* Save essential context */
+static void hv_hvcrash_ctxt_save(void)
+{
+ struct hv_crash_ctxt *ctxt = &hv_crash_ctxt;
+
+ asm volatile("movq %%rsp,%0" : "=m"(ctxt->rsp));
+
+ ctxt->cr0 = native_read_cr0();
+ ctxt->cr4 = native_read_cr4();
+
+ asm volatile("movq %%cr2, %0" : "=a"(ctxt->cr2));
+ asm volatile("movq %%cr8, %0" : "=a"(ctxt->cr8));
+
+ asm volatile("movl %%cs, %%eax" : "=a"(ctxt->cs));
+ asm volatile("movl %%ss, %%eax" : "=a"(ctxt->ss));
+ asm volatile("movl %%ds, %%eax" : "=a"(ctxt->ds));
+ asm volatile("movl %%es, %%eax" : "=a"(ctxt->es));
+ asm volatile("movl %%fs, %%eax" : "=a"(ctxt->fs));
+ asm volatile("movl %%gs, %%eax" : "=a"(ctxt->gs));
+
+ native_store_gdt(&ctxt->gdtr);
+ store_idt(&ctxt->idtr);
+
+ ctxt->gsbase = __rdmsr(MSR_GS_BASE);
+ ctxt->efer = __rdmsr(MSR_EFER);
+ ctxt->pat = __rdmsr(MSR_IA32_CR_PAT);
+}
+
+/* Add trampoline page to the kernel pagetable for transition to kernel PT */
+static void hv_crash_fixup_kernpt(void)
+{
+ pgd_t *pgd;
+ p4d_t *p4d;
+
+ pgd = pgd_offset_k(trampoline_pa);
+ p4d = p4d_offset(pgd, trampoline_pa);
+
+ /* trampoline_pa is below 4G, so no pre-existing entry to clobber */
+ p4d_populate(&init_mm, p4d, (pud_t *)hv_crash_ptpgs[1]);
+ p4d->p4d = p4d->p4d & ~(_PAGE_NX); /* enable execute */
+}
+
+/*
+ * Now that all cpus are in nmi and spinning, we notify the hyp that dom0 has
+ * crashed and will collect core. This will cause the hyp to quiesce and
+ * suspend all VPs except the bsp.
+ */
+static void hv_notify_prepare_hyp(void)
+{
+ u64 status;
+ struct hv_input_notify_partition_event *input;
+ struct hv_partition_event_root_crashdump_input *cda;
+
+ input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+ cda = &input->input.crashdump_input;
+ memset(input, 0, sizeof(*input));
+ input->event = HV_PARTITION_EVENT_ROOT_CRASHDUMP;
+
+ cda->crashdump_action = HV_CRASHDUMP_ENTRY;
+ status = hv_do_hypercall(HVCALL_NOTIFY_PARTITION_EVENT, input, NULL);
+ if (!hv_result_success(status))
+ return;
+
+ cda->crashdump_action = HV_CRASHDUMP_SUSPEND_ALL_VPS;
+ hv_do_hypercall(HVCALL_NOTIFY_PARTITION_EVENT, input, NULL);
+}
+
+/*
+ * Common function for all cpus before devirtualization.
+ *
+ * Hypervisor crash: all cpus get here in nmi context.
+ * Linux crash: the panicing cpu gets here at base level, all others in nmi
+ * context. Note, panicing cpu may not be the bsp.
+ *
+ * The function is not inlined so it will show on the stack. It is named so
+ * because the crash cmd looks for certain well known function names on the
+ * stack before looking into the cpu saved note in the elf section, and
+ * that work is currently incomplete.
+ *
+ * Notes:
+ * Hypervisor crash:
+ * - the hypervisor is in a very restrictive mode at this point and any
+ * vmexit it cannot handle would result in reboot. For example, console
+ * output from here would result in synic ipi hcall, which would result
+ * in reboot. So, no mumbo jumbo, just get to kexec as quickly as possible.
+ *
+ * Devirtualization is supported from the bsp only.
+ */
+static noinline __noclone void crash_nmi_callback(struct pt_regs *regs)
+{
+ struct hv_input_disable_hyp_ex *input;
+ u64 status;
+ int msecs = 1000, ccpu = smp_processor_id();
+
+ if (ccpu == 0) {
+ /* crash_save_cpu() will be done in the kexec path */
+ cpu_emergency_stop_pt(); /* disable performance trace */
+ atomic_inc(&crash_cpus_wait);
+ } else {
+ crash_save_cpu(regs, ccpu);
+ cpu_emergency_stop_pt(); /* disable performance trace */
+ atomic_inc(&crash_cpus_wait);
+ for (;;); /* cause no vmexits */
+ }
+
+ while (atomic_read(&crash_cpus_wait) < num_online_cpus() && msecs--)
+ mdelay(1);
+
+ stop_nmi();
+ if (!hv_has_crashed)
+ hv_notify_prepare_hyp();
+
+ if (crashing_cpu == -1)
+ crashing_cpu = ccpu; /* crash cmd uses this */
+
+ hv_hvcrash_ctxt_save();
+ hv_mark_tss_not_busy();
+ hv_crash_fixup_kernpt();
+
+ input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+ memset(input, 0, sizeof(*input));
+ input->rip = trampoline_pa; /* PA of hv_crash_asm32 */
+ input->arg = devirt_cr3arg; /* PA of trampoline page table L4 */
+
+ status = hv_do_hypercall(HVCALL_DISABLE_HYP_EX, input, NULL);
+ if (!hv_result_success(status)) {
+ pr_emerg("%s: %s\n", __func__, hv_result_to_string(status));
+ pr_emerg("Hyper-V: disable hyp failed. kexec not possible\n");
+ }
+
+ native_wrmsrq(HV_X64_MSR_RESET, 1); /* get hv to reboot */
+}
+
+/*
+ * generic nmi callback handler: could be called without any crash also.
+ * hv crash: hypervisor injects nmi's into all cpus
+ * lx crash: panicing cpu sends nmi to all but self via crash_stop_other_cpus
+ */
+static int hv_crash_nmi_local(unsigned int cmd, struct pt_regs *regs)
+{
+ int ccpu = smp_processor_id();
+
+ if (!hv_has_crashed && hv_cda && hv_cda->cda_valid)
+ hv_has_crashed = 1;
+
+ if (!hv_has_crashed && !lx_has_crashed)
+ return NMI_DONE; /* ignore the nmi */
+
+ if (hv_has_crashed && !hv_crash_enabled) {
+ if (ccpu == 0) {
+ pr_emerg("Hyper-V: core collect not setup. Reboot\n");
+ native_wrmsrq(HV_X64_MSR_RESET, 1); /* reboot */
+ } else
+ for (;;)
+ cpu_relax();
+ }
+
+ crash_nmi_callback(regs);
+ return NMI_DONE;
+}
+
+/*
+ * hv_crash_stop_other_cpus() == smp_ops.crash_stop_other_cpus
+ *
+ * On normal linux panic, this is called twice: first from panic and then again
+ * from native_machine_crash_shutdown.
+ *
+ * In case of mshv, 3 ways to get here:
+ * 1. hv crash (only bsp will get here):
+ * BSP : nmi callback -> DisableHv -> hv_crash_asm32 -> hv_crash_c_entry
+ * -> __crash_kexec -> native_machine_crash_shutdown
+ * -> crash_smp_send_stop -> smp_ops.crash_stop_other_cpus
+ * linux panic:
+ * 2. panic cpu x: panic() -> crash_smp_send_stop
+ * -> smp_ops.crash_stop_other_cpus
+ * 3. bsp: native_machine_crash_shutdown -> crash_smp_send_stop
+ *
+ * NB: noclone and non standard stack because of call to crash_setup_regs().
+ */
+static void __noclone hv_crash_stop_other_cpus(void)
+{
+ static int crash_stop_done;
+ struct pt_regs lregs;
+ int ccpu = smp_processor_id();
+
+ if (hv_has_crashed)
+ return; /* all cpus already in nmi handler path */
+
+ if (crash_stop_done)
+ return;
+ crash_stop_done = 1;
+
+ /* linux has crashed: hv is healthy, we can ipi safely */
+ lx_has_crashed = 1;
+ wmb(); /* nmi handlers look at lx_has_crashed */
+
+ apic->send_IPI_allbutself(NMI_VECTOR);
+
+ if (crashing_cpu == -1)
+ crashing_cpu = ccpu; /* crash cmd uses this */
+
+ /* crash_setup_regs() happens in kexec also, but for the kexec cpu which
+ * is the bsp. We could be here on non-bsp cpu, collect regs if so.
+ */
+ if (ccpu)
+ crash_setup_regs(&lregs, NULL);
+
+ crash_nmi_callback(&lregs);
+}
+STACK_FRAME_NON_STANDARD(hv_crash_stop_other_cpus);
+
+/* This GDT is accessed in IA32-e compat mode which uses 32bits addresses */
+struct hv_gdtreg_32 {
+ u16 fill;
+ u16 limit;
+ u32 address;
+} __packed;
+
+/* We need a CS with L bit to goto IA32-e long mode from 32bit compat mode */
+struct hv_crash_tramp_gdt {
+ u64 null; /* index 0, selector 0, null selector */
+ u64 cs64; /* index 1, selector 8, cs64 selector */
+} __packed;
+
+/* No stack, so jump via far ptr in memory to load the 64bit CS */
+struct hv_cs_jmptgt {
+ u32 address;
+ u16 csval;
+ u16 fill;
+} __packed;
+
+/* This trampoline data is copied onto the trampoline page after the asm code */
+struct hv_crash_tramp_data {
+ u64 tramp32_cr3;
+ u64 kernel_cr3;
+ struct hv_gdtreg_32 gdtr32;
+ struct hv_crash_tramp_gdt tramp_gdt;
+ struct hv_cs_jmptgt cs_jmptgt;
+ u64 c_entry_addr;
+} __packed;
+
+/*
+ * Setup a temporary gdt to allow the asm code to switch to the long mode.
+ * Since the asm code is relocated/copied to a below 4G page, it cannot use rip
+ * relative addressing, hence, we must use trampoline_pa here. Also save
+ * other jmp and C entry targets for same reasons.
+ *
+ * Returns: 0 on success, -1 on error
+ */
+static int hv_crash_setup_trampdata(u64 trampoline_va)
+{
+ int size, offs;
+ void *dest;
+ struct hv_crash_tramp_data *tramp;
+
+ /* These must match exactly the ones in the corresponding asm file */
+ BUILD_BUG_ON(offsetof(struct hv_crash_tramp_data, tramp32_cr3) != 0);
+ BUILD_BUG_ON(offsetof(struct hv_crash_tramp_data, kernel_cr3) != 8);
+ BUILD_BUG_ON(offsetof(struct hv_crash_tramp_data, gdtr32.limit) != 18);
+ BUILD_BUG_ON(offsetof(struct hv_crash_tramp_data,
+ cs_jmptgt.address) != 40);
+
+ /* end is beyond last byte by 1 */
+ size = &hv_crash_asm32_end - &hv_crash_asm32;
+ if (size + sizeof(struct hv_crash_tramp_data) > PAGE_SIZE) {
+ pr_err("%s: trampoline page overflow\n", __func__);
+ return -1;
+ }
+
+ dest = (void *)trampoline_va;
+ memcpy(dest, &hv_crash_asm32, size);
+
+ dest += size;
+ dest = (void *)round_up((ulong)dest, 16);
+ tramp = (struct hv_crash_tramp_data *)dest;
+
+ /* see MAX_ASID_AVAILABLE in tlb.c: "PCID 0 is reserved for use by
+ * non-PCID-aware users". Build cr3 with pcid 0
+ */
+ tramp->tramp32_cr3 = __sme_pa(hv_crash_ptpgs[0]);
+
+ /* Note, when restoring X86_CR4_PCIDE, cr3[11:0] must be zero */
+ tramp->kernel_cr3 = __sme_pa(init_mm.pgd);
+
+ tramp->gdtr32.limit = sizeof(struct hv_crash_tramp_gdt);
+ tramp->gdtr32.address = trampoline_pa +
+ (ulong)&tramp->tramp_gdt - trampoline_va;
+
+ /* base:0 limit:0xfffff type:b dpl:0 P:1 L:1 D:0 avl:0 G:1 */
+ tramp->tramp_gdt.cs64 = 0x00af9a000000ffff;
+
+ tramp->cs_jmptgt.csval = 0x8;
+ offs = (ulong)&hv_crash_asm64_lbl - (ulong)&hv_crash_asm32;
+ tramp->cs_jmptgt.address = trampoline_pa + offs;
+
+ tramp->c_entry_addr = (u64)&hv_crash_c_entry;
+
+ memcpy(dest, tramp, sizeof(struct hv_crash_tramp_data));
+
+ devirt_cr3arg = trampoline_pa + (ulong)dest - trampoline_va;
+
+ return 0;
+}
+
+/*
+ * Build 32bit trampoline page table for transition from protected mode
+ * non-paging to long-mode paging. This transition needs pagetables below 4G.
+ */
+static void hv_crash_build_tramp_pt(void)
+{
+ pgd_t *pgd;
+ p4d_t *p4d;
+ pud_t *pud;
+ pmd_t *pmd;
+ pte_t *pte;
+ u64 pa, addr = trampoline_pa;
+
+ p4d = hv_crash_ptpgs[0] + pgd_index(addr) * sizeof(p4d);
+ pa = virt_to_phys(hv_crash_ptpgs[1]);
+ set_p4d(p4d, __p4d(_PAGE_TABLE | pa));
+ p4d->p4d &= ~(_PAGE_NX); /* disable no execute */
+
+ pud = hv_crash_ptpgs[1] + pud_index(addr) * sizeof(pud);
+ pa = virt_to_phys(hv_crash_ptpgs[2]);
+ set_pud(pud, __pud(_PAGE_TABLE | pa));
+
+ pmd = hv_crash_ptpgs[2] + pmd_index(addr) * sizeof(pmd);
+ pa = virt_to_phys(hv_crash_ptpgs[3]);
+ set_pmd(pmd, __pmd(_PAGE_TABLE | pa));
+
+ pte = hv_crash_ptpgs[3] + pte_index(addr) * sizeof(pte);
+ set_pte(pte, pfn_pte(addr >> PAGE_SHIFT, PAGE_KERNEL_EXEC));
+}
+
+/*
+ * Setup trampoline for devirtualization:
+ * - a page below 4G, ie 32bit addr containing asm glue code that mshv jmps to
+ * in protected mode.
+ * - 4 pages for a temporary page table that asm code uses to turn paging on
+ * - a temporary gdt to use in the compat mode.
+ *
+ * Returns: 0 on success
+ */
+static int hv_crash_trampoline_setup(void)
+{
+ int i, rc, order;
+ struct page *page;
+ u64 trampoline_va;
+ gfp_t flags32 = GFP_KERNEL | GFP_DMA32 | __GFP_ZERO;
+
+ /* page for 32bit trampoline assembly code + hv_crash_tramp_data */
+ page = alloc_page(flags32);
+ if (page == NULL) {
+ pr_err("%s: failed to alloc asm stub page\n", __func__);
+ return -1;
+ }
+
+ trampoline_va = (u64)page_to_virt(page);
+ trampoline_pa = (u32)page_to_phys(page);
+
+ order = 2; /* alloc 2^2 pages */
+ page = alloc_pages(flags32, order);
+ if (page == NULL) {
+ pr_err("%s: failed to alloc pt pages\n", __func__);
+ free_page(trampoline_va);
+ return -1;
+ }
+
+ for (i = 0; i < 4; i++, page++)
+ hv_crash_ptpgs[i] = page_to_virt(page);
+
+ hv_crash_build_tramp_pt();
+
+ rc = hv_crash_setup_trampdata(trampoline_va);
+ if (rc)
+ goto errout;
+
+ return 0;
+
+errout:
+ free_page(trampoline_va);
+ free_pages((ulong)hv_crash_ptpgs[0], order);
+
+ return rc;
+}
+
+/* Setup for kdump kexec to collect hypervisor ram when running as mshv root */
+void hv_root_crash_init(void)
+{
+ int rc;
+ struct hv_input_get_system_property *input;
+ struct hv_output_get_system_property *output;
+ unsigned long flags;
+ u64 status;
+ union hv_pfn_range cda_info;
+
+ if (pgtable_l5_enabled()) {
+ pr_err("Hyper-V: crash dump not yet supported on 5level PTs\n");
+ goto err_out;
+ }
+
+ local_irq_save(flags);
+ input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+ output = *this_cpu_ptr(hyperv_pcpu_output_arg);
+
+ memset(input, 0, sizeof(*input));
+ memset(output, 0, sizeof(*output));
+ input->property_id = HV_SYSTEM_PROPERTY_CRASHDUMPAREA;
+
+ status = hv_do_hypercall(HVCALL_GET_SYSTEM_PROPERTY, input, output);
+ local_irq_restore(flags);
+ if (!hv_result_success(status))
+ goto prop_err_out;
+
+ cda_info.as_uint64 = output->hv_cda_info.as_uint64;
+
+ if (cda_info.base_pfn == 0) {
+ pr_err("Hyper-V: hypervisor crash dump area pfn is 0\n");
+ goto err_out;
+ }
+
+ hv_cda = phys_to_virt(cda_info.base_pfn << PAGE_SHIFT);
+
+ rc = hv_crash_trampoline_setup();
+ if (rc)
+ goto err_out;
+
+ register_nmi_handler(NMI_LOCAL, hv_crash_nmi_local, NMI_FLAG_FIRST,
+ "hv_crash_nmi");
+
+ smp_ops.crash_stop_other_cpus = hv_crash_stop_other_cpus;
+
+ crash_kexec_post_notifiers = true;
+ hv_crash_enabled = 1;
+ pr_info("Hyper-V: linux and hv kdump support enabled\n");
+
+ return;
+
+prop_err_out:
+ pr_err("Hyper-V: %s: property:%d %s\n", __func__, input->property_id,
+ hv_result_to_string(status));
+err_out:
+ pr_err("Hyper-V: only linux (but not hv) kdump support enabled\n");
+}
+
--
2.36.1.vfs.0.0
^ permalink raw reply related
* [PATCH v0 1/6] x86/hyperv: Rename guest shutdown functions
From: Mukesh Rathor @ 2025-09-04 2:10 UTC (permalink / raw)
To: linux-hyperv, linux-kernel, linux-arch
Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, arnd
In-Reply-To: <20250904021017.1628993-1-mrathor@linux.microsoft.com>
This commit renames hv_machine_crash_shutdown to more appropriate
hv_guest_crash_shutdown and makes it applicable to guests only. This
in preparation for the subsequent hypervisor root/dom0 crash support
patches.
Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
---
arch/x86/kernel/cpu/mshyperv.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index b8caf78c4336..f5d76d854d39 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -237,7 +237,7 @@ static void hv_machine_shutdown(void)
#endif /* CONFIG_KEXEC_CORE */
#ifdef CONFIG_CRASH_DUMP
-static void hv_machine_crash_shutdown(struct pt_regs *regs)
+static void hv_guest_crash_shutdown(struct pt_regs *regs)
{
if (hv_crash_handler)
hv_crash_handler(regs);
@@ -824,7 +824,8 @@ static void __init ms_hyperv_init_platform(void)
machine_ops.shutdown = hv_machine_shutdown;
#endif
#if defined(CONFIG_CRASH_DUMP)
- machine_ops.crash_shutdown = hv_machine_crash_shutdown;
+ if (!hv_root_partition)
+ machine_ops.crash_shutdown = hv_guest_crash_shutdown;
#endif
#endif
/*
--
2.36.1.vfs.0.0
^ permalink raw reply related
* [PATCH v0 0/6] Hyper-V: Implement hypervisor core collection
From: Mukesh Rathor @ 2025-09-04 2:10 UTC (permalink / raw)
To: linux-hyperv, linux-kernel, linux-arch
Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, arnd
This patch series implements hypervisor core collection when running
under linux as root (aka dom0). By default initial hypervisor ram is
already mapped into linux as reserved. Further any ram deposited comes
from linux direct map. The hypervisor locks all that ram to protect
it from dom0 or any other domains. At a high level, the methodology
involes devirtualizing the system on the fly upon either linux crash
or the hypervisor crash, then collecting ram as usual. This means
hypervisor ram is automatically collected into the vmcore.
Hypervisor pages are then accessible via crash command (using raw mem
dump) or windbg which has the ability to read hypervisor symbol pdb
file.
Mukesh Rathor (6):
x86/hyperv: Rename guest shutdown functions
Hyper-V: Add two new hypercall numbers to guest ABI public header
Hyper-V: Add definitions for hypervisor crash dump support
x86/hyperv: Add trampoline asm code to transition from hypervisor
x86/hyperv: Implement hypervisor ram collection into vmcore
Hyper-V: Enable build of hypervisor crash collection files
arch/x86/hyperv/Makefile | 6 +
arch/x86/hyperv/hv_crash.c | 618 ++++++++++++++++++++++++++++++++
arch/x86/hyperv/hv_init.c | 2 +
arch/x86/hyperv/hv_trampoline.S | 99 +++++
arch/x86/kernel/cpu/mshyperv.c | 5 +-
include/asm-generic/mshyperv.h | 9 +
include/hyperv/hvgdk_mini.h | 2 +
include/hyperv/hvhdk_mini.h | 55 +++
8 files changed, 794 insertions(+), 2 deletions(-)
create mode 100644 arch/x86/hyperv/hv_crash.c
create mode 100644 arch/x86/hyperv/hv_trampoline.S
--
2.36.1.vfs.0.0
^ permalink raw reply
* Re: [PATCH v3 1/1] x86/hyperv: Switch to msi_create_parent_irq_domain()
From: Nuno Das Neves @ 2025-09-03 19:40 UTC (permalink / raw)
To: Nam Cao, K . Y . Srinivasan, Marc Zyngier, Haiyang Zhang, Wei Liu,
Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H . Peter Anvin, linux-hyperv, linux-kernel
In-Reply-To: <45df1cc0088057cbf60cb84d8e9f9ff09f12f670.1752868165.git.namcao@linutronix.de>
On 7/18/2025 12:57 PM, Nam Cao wrote:
> Move away from the legacy MSI domain setup, switch to use
> msi_create_parent_irq_domain().
>
> While doing the conversion, I noticed that hv_irq_compose_msi_msg() is
> doing more than it is supposed to (composing message content). The
> interrupt allocation bits should be moved into hv_msi_domain_alloc().
> However, I have no hardware to test this change, therefore I leave a TODO
> note.
>
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
> arch/x86/hyperv/irqdomain.c | 111 ++++++++++++++++++++++++------------
> drivers/hv/Kconfig | 1 +
> 2 files changed, 77 insertions(+), 35 deletions(-)
Tested on nested root partition.
Looks good, thanks.
Tested-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
^ permalink raw reply
* Re: [PATCH net-next v5 4/9] vsock/loopback: add netns support
From: Stefano Garzarella @ 2025-09-03 15:10 UTC (permalink / raw)
To: Bobby Eshleman
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, berrange,
Bobby Eshleman
In-Reply-To: <aLcy4Kk0joVPbxkd@devvm6216.cco0.facebook.com>
On Tue, Sep 02, 2025 at 11:09:36AM -0700, Bobby Eshleman wrote:
>On Tue, Sep 02, 2025 at 05:39:33PM +0200, Stefano Garzarella wrote:
>> On Wed, Aug 27, 2025 at 05:31:32PM -0700, Bobby Eshleman wrote:
>> > From: Bobby Eshleman <bobbyeshleman@meta.com>
>> >
>> > Add NS support to vsock loopback. Sockets in a global mode netns
>> > communicate with each other, regardless of namespace. Sockets in a local
>> > mode netns may only communicate with other sockets within the same
>> > namespace.
>> >
>> > Add callbacks for transport to hook into the initialization and exit of
>> > net namespaces.
>> >
>> > The transport's init hook will be called once per netns init. Likewise
>> > for exit.
>> >
>> > When a set of init/exit callbacks is registered, the init callback is
>> > called on each already existing namespace.
>> >
>> > Only one callback registration is supported for now. Currently
>> > vsock_loopback is the only user.
>>
>> Why?
>>
>> In general, commit descriptions (and code comments) should focus on the
>> reason (why?) to simplify also the review.
>>
>
>Sounds good, will improve the message/comments. I'm realizing as I type
>this there may be a way to avoid the callbacks altogether with
>pernet_operations, so I'll clarify that before next rev.
Yeah, that would be great.
>
>> >
>> > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>> >
>> > ---
>> > Changes in v5:
>> > - add callbacks code to avoid reverse dependency
>> > - add logic for handling vsock_loopback setup for already existing
>> > namespaces
>> > ---
>> > include/net/af_vsock.h | 34 +++++++++++++
>> > include/net/netns/vsock.h | 5 ++
>> > net/vmw_vsock/af_vsock.c | 110 +++++++++++++++++++++++++++++++++++++++++
>> > net/vmw_vsock/vsock_loopback.c | 72 ++++++++++++++++++++++++---
>> > 4 files changed, 213 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>> > index 83f873174ba3..9333a98b9a1e 100644
>> > --- a/include/net/af_vsock.h
>> > +++ b/include/net/af_vsock.h
>> > @@ -305,4 +305,38 @@ static inline bool vsock_net_check_mode(struct net *n1, struct net *n2)
>> > (vsock_net_mode(n1) == VSOCK_NET_MODE_GLOBAL &&
>> > vsock_net_mode(n2) == VSOCK_NET_MODE_GLOBAL);
>> > }
>> > +
>> > +struct vsock_net_callbacks {
>> > + int (*init)(struct net *net);
>> > + void (*exit)(struct net *net);
>> > + struct module *owner;
>> > +};
>> > +
>> > +#if IS_ENABLED(CONFIG_VSOCKETS_LOOPBACK)
>> > +
>> > +#define vsock_register_net_callbacks(__init, __exit) \
>> > + __vsock_register_net_callbacks((__init), (__exit), THIS_MODULE)
>> > +
>> > +int __vsock_register_net_callbacks(int (*init)(struct net *net),
>> > + void (*exit)(struct net *net),
>> > + struct module *owner);
>> > +void vsock_unregister_net_callbacks(void);
>> > +
>> > +#else
>> > +
>> > +#define vsock_register_net_callbacks(__init, __exit) do { } while (0)
>> > +
>> > +static inline int __vsock_register_net_callbacks(int (*init)(struct net *net),
>> > + void (*exit)(struct net *net),
>> > + struct module *owner)
>> > +{
>> > + return 0;
>> > +}
>> > +
>> > +static inline void vsock_unregister_net_callbacks(void) {}
>> > +static inline int vsock_net_call_init(struct net *net) { return 0; }
>> > +static inline void vsock_net_call_exit(struct net *net) {}
>> > +
>> > +#endif /* CONFIG_VSOCKETS_LOOPBACK */
>> > +
>> > #endif /* __AF_VSOCK_H__ */
>> > diff --git a/include/net/netns/vsock.h b/include/net/netns/vsock.h
>> > index d4593c0b8dc4..08d9a933c540 100644
>> > --- a/include/net/netns/vsock.h
>> > +++ b/include/net/netns/vsock.h
>> > @@ -9,6 +9,8 @@ enum vsock_net_mode {
>> > VSOCK_NET_MODE_LOCAL,
>> > };
>> >
>> > +struct vsock_loopback;
>> > +
>> > struct netns_vsock {
>> > struct ctl_table_header *vsock_hdr;
>> > spinlock_t lock;
>> > @@ -16,5 +18,8 @@ struct netns_vsock {
>> > /* protected by lock */
>> > enum vsock_net_mode mode;
>> > bool written;
>> > +#if IS_ENABLED(CONFIG_VSOCKETS_LOOPBACK)
>> > + struct vsock_loopback *loopback;
>>
>> If this is not protected by `lock`, please leave an empty line, but maybe we
>> should consider using locking (see comment later).
>>
>
>Will do.
>
>> > +#endif
>> > };
>> > #endif /* __NET_NET_NAMESPACE_VSOCK_H */
>> > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> > index 68a8875c8106..5a73d9e1a96f 100644
>> > --- a/net/vmw_vsock/af_vsock.c
>> > +++ b/net/vmw_vsock/af_vsock.c
>> > @@ -134,6 +134,9 @@
>> > #include <uapi/linux/vm_sockets.h>
>> > #include <uapi/asm-generic/ioctls.h>
>> >
>> > +static struct vsock_net_callbacks vsock_net_callbacks;
>> > +static DEFINE_MUTEX(vsock_net_callbacks_lock);
>> > +
>> > static int __vsock_bind(struct sock *sk, struct sockaddr_vm *addr);
>> > static void vsock_sk_destruct(struct sock *sk);
>> > static int vsock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
>> > @@ -2781,6 +2784,49 @@ static void vsock_net_init(struct net *net)
>> > net->vsock.mode = VSOCK_NET_MODE_GLOBAL;
>> > }
>> >
>> > +#if IS_ENABLED(CONFIG_VSOCKETS_LOOPBACK)
>> > +static int vsock_net_call_init(struct net *net)
>> > +{
>> > + struct vsock_net_callbacks *cbs;
>> > + int ret;
>> > +
>> > + mutex_lock(&vsock_net_callbacks_lock);
>> > + cbs = &vsock_net_callbacks;
>> > +
>> > + ret = 0;
>> > + if (!cbs->owner)
>> > + goto out;
>> > +
>> > + if (try_module_get(cbs->owner)) {
>> > + ret = cbs->init(net);
>> > + module_put(cbs->owner);
>> > + }
>> > +
>> > +out:
>> > + mutex_unlock(&vsock_net_callbacks_lock);
>> > + return ret;
>> > +}
>> > +
>> > +static void vsock_net_call_exit(struct net *net)
>> > +{
>> > + struct vsock_net_callbacks *cbs;
>> > +
>> > + mutex_lock(&vsock_net_callbacks_lock);
>> > + cbs = &vsock_net_callbacks;
>> > +
>> > + if (!cbs->owner)
>> > + goto out;
>> > +
>> > + if (try_module_get(cbs->owner)) {
>> > + cbs->exit(net);
>> > + module_put(cbs->owner);
>> > + }
>> > +
>> > +out:
>> > + mutex_unlock(&vsock_net_callbacks_lock);
>> > +}
>> > +#endif /* CONFIG_VSOCKETS_LOOPBACK */
>> > +
>> > static __net_init int vsock_sysctl_init_net(struct net *net)
>> > {
>> > vsock_net_init(net);
>> > @@ -2788,12 +2834,20 @@ static __net_init int vsock_sysctl_init_net(struct net *net)
>> > if (vsock_sysctl_register(net))
>> > return -ENOMEM;
>> >
>> > + if (vsock_net_call_init(net) < 0)
>> > + goto err_sysctl;
>> > +
>> > return 0;
>> > +
>> > +err_sysctl:
>> > + vsock_sysctl_unregister(net);
>> > + return -ENOMEM;
>> > }
>> >
>> > static __net_exit void vsock_sysctl_exit_net(struct net *net)
>> > {
>> > vsock_sysctl_unregister(net);
>> > + vsock_net_call_exit(net);
>> > }
>> >
>> > static struct pernet_operations vsock_sysctl_ops __net_initdata = {
>> > @@ -2938,6 +2992,62 @@ void vsock_core_unregister(const struct
>> > vsock_transport *t)
>> > }
>> > EXPORT_SYMBOL_GPL(vsock_core_unregister);
>> >
>> > +#if IS_ENABLED(CONFIG_VSOCKETS_LOOPBACK)
>> > +int __vsock_register_net_callbacks(int (*init)(struct net *net),
>> > + void (*exit)(struct net *net),
>> > + struct module *owner)
>> > +{
>> > + struct vsock_net_callbacks *cbs;
>> > + struct net *net;
>> > + int ret = 0;
>> > +
>> > + mutex_lock(&vsock_net_callbacks_lock);
>> > +
>> > + cbs = &vsock_net_callbacks;
>> > + cbs->init = init;
>> > + cbs->exit = exit;
>> > + cbs->owner = owner;
>> > +
>> > + /* call callbacks on any net previously created */
>> > + down_read(&net_rwsem);
>> > +
>> > + if (try_module_get(cbs->owner)) {
>> > + for_each_net(net) {
>> > + ret = cbs->init(net);
>> > + if (ret < 0)
>> > + break;
>> > + }
>> > +
>> > + if (ret < 0)
>> > + for_each_net(net)
>> > + cbs->exit(net);
>> > +
>> > + module_put(cbs->owner);
>> > + }
>> > +
>> > + up_read(&net_rwsem);
>> > + mutex_unlock(&vsock_net_callbacks_lock);
>> > +
>> > + return ret;
>> > +}
>> > +EXPORT_SYMBOL_GPL(__vsock_register_net_callbacks);
>> > +
>> > +void vsock_unregister_net_callbacks(void)
>> > +{
>> > + struct vsock_net_callbacks *cbs;
>> > +
>> > + mutex_lock(&vsock_net_callbacks_lock);
>> > +
>> > + cbs = &vsock_net_callbacks;
>> > + cbs->init = NULL;
>> > + cbs->exit = NULL;
>> > + cbs->owner = NULL;
>> > +
>> > + mutex_unlock(&vsock_net_callbacks_lock);
>> > +}
>> > +EXPORT_SYMBOL_GPL(vsock_unregister_net_callbacks);
>>
>> IIUC this function is called only in the error path of
>> `vsock_loopback_init()`, but shuold we call it also in the
>> vsock_loopback_exit() ?
>>
>
>Ah right, that needs to be done there too.
>
>> > +#endif /* CONFIG_VSOCKETS_LOOPBACK */
>> > +
>> > module_init(vsock_init);
>> > module_exit(vsock_exit);
>> >
>> > diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
>> > index 1b2fab73e0d0..f16d21711cb0 100644
>> > --- a/net/vmw_vsock/vsock_loopback.c
>> > +++ b/net/vmw_vsock/vsock_loopback.c
>> > @@ -28,8 +28,19 @@ static u32 vsock_loopback_get_local_cid(void)
>> >
>> > static int vsock_loopback_send_pkt(struct sk_buff *skb)
>> > {
>> > - struct vsock_loopback *vsock = &the_vsock_loopback;
>> > + struct vsock_loopback *vsock;
>> > int len = skb->len;
>> > + struct net *net;
>> > +
>> > + if (skb->sk)
>> > + net = sock_net(skb->sk);
>> > + else
>> > + net = NULL;
>>
>> Why we can't use `virtio_vsock_skb_net` here?
>>
>
>No reason why not. Using it should make it more uniform.
>
>> > +
>> > + if (net && net->vsock.mode == VSOCK_NET_MODE_LOCAL)
>> > + vsock = net->vsock.loopback;
>> > + else
>> > + vsock = &the_vsock_loopback;
>> >
>> > virtio_vsock_skb_queue_tail(&vsock->pkt_queue, skb);
>> > queue_work(vsock->workqueue, &vsock->pkt_work);
>> > @@ -134,27 +145,72 @@ static void vsock_loopback_work(struct work_struct *work)
>> > }
>> > }
>> >
>> > -static int __init vsock_loopback_init(void)
>> > +static int vsock_loopback_init_vsock(struct vsock_loopback *vsock)
>> > {
>> > - struct vsock_loopback *vsock = &the_vsock_loopback;
>> > - int ret;
>> > -
>> > vsock->workqueue = alloc_workqueue("vsock-loopback", 0, 0);
>> > if (!vsock->workqueue)
>> > return -ENOMEM;
>> >
>> > skb_queue_head_init(&vsock->pkt_queue);
>> > INIT_WORK(&vsock->pkt_work, vsock_loopback_work);
>> > + return 0;
>> > +}
>> > +
>> > +static void vsock_loopback_deinit_vsock(struct vsock_loopback *vsock)
>> > +{
>> > + if (vsock->workqueue)
>> > + destroy_workqueue(vsock->workqueue);
>> > +}
>> > +
>> > +/* called with vsock_net_callbacks lock held */
>> > +static int vsock_loopback_init_net(struct net *net)
>> > +{
>> > + if (WARN_ON_ONCE(net->vsock.loopback))
>> > + return 0;
>> > +
>>
>> Do we need some kind of locking here? I mean when reading/setting
>> `net->vsock.loopback`?
>>
>
>I could be wrong here, but I think net->vsock.loopback being set before
>vsock_core_register() prevents racing with net->vsock.loopback reads. We
>could add a lock to make sure and to make the protection explicit
>though.
I see, talkink about vsock_core_register(), I was thinking about,
extending it, maybe passing a struct with all parameters (e.g. transport
type, net callbacks, etc.). In this way we can easily check if the type
of transport is allowed to register net callbacks or not.
Also because currently we don't do any check in
__vsock_register_net_callbacks() about transport type or even about
overriding calls.
>
>> > + net->vsock.loopback = kmalloc(sizeof(*net->vsock.loopback),
>> > GFP_KERNEL);
>> > + if (!net->vsock.loopback)
>> > + return -ENOMEM;
>> > +
>> > + return vsock_loopback_init_vsock(net->vsock.loopback);
>> > +}
>> > +
>> > +/* called with vsock_net_callbacks lock held */
>> > +static void vsock_loopback_exit_net(struct net *net)
>> > +{
>> > + if (net->vsock.loopback) {
>> > + vsock_loopback_deinit_vsock(net->vsock.loopback);
>> > + kfree(net->vsock.loopback);
>>
>> Should we set `net->vsock.loopback` to NULL here?
>>
>
>Yes.
>
>> > + }
>> > +}
>> > +
>> > +static int __init vsock_loopback_init(void)
>> > +{
>> > + struct vsock_loopback *vsock = &the_vsock_loopback;
>> > + int ret;
>> > +
>> > + ret = vsock_loopback_init_vsock(vsock);
>> > + if (ret < 0)
>> > + return ret;
>> > +
>> > + ret = vsock_register_net_callbacks(vsock_loopback_init_net,
>> > + vsock_loopback_exit_net);
>>
>> IIUC we need this only here because for now the only other transport
>> supported is vhost-vsock, and IIUC `struct vhost_vsock *` there is handled
>> with a map instead of a static variable, and `net` assigned when
>> /dev/vhost-vsock is opened, right?
>>
>
>Correct. The vhost map lookup is modified to account for namespaces, but
>vsock loopback doesn't have a map to do that. The callbacks are used to
>hook into the netns initialization.
>
>I wonder if it is possible to do this with just pernet_operations
>though... when I wrote this I was pretty laser-focused on the
>sysctl/procfs + netns init code, and may not have realized there may be
>similar hooks that aren't bound to the sysctl/proc init. I'll clarify
>this before the next rev.
I like the idea of removing vsock_register_net_callbacks() if possible,
but if it's not possible I'd like to reuse vsock_core_register() as much
as possible, avoiding to add a new register function that is not clear
when it needs to be called or not by the transport.
So, to be clear, I'd like to have a single registration function that
transports need to call (if possible).
>
>
>> If in the future we will need to support G2H transports, like
>> virtio-transport, we need to do something similar, right?
>>
>
>My guess is that we'll be able to avoid using these callbacks unless
>there is some per-net data we need to initialize. I'm guessing if we
>follow a similar path as using ioctl to assign the dev netns, then we
>won't need it. It might be moot if pernet_operations work to avoid the
>module circular dependency.
Cool!
>
>> BTW I think we really need to exaplin this better in the commit description.
>> It tooks me a while to get all of this (if it's correct)
>>
>
>Roger that, I'll improve this going forward.
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH v3 0/1] x86/hyperv: MSI parent domain conversion
From: Thomas Gleixner @ 2025-09-03 14:00 UTC (permalink / raw)
To: Wei Liu, Nam Cao
Cc: K . Y . Srinivasan, Nuno Das Neves, Marc Zyngier, Haiyang Zhang,
Wei Liu, Dexuan Cui, Ingo Molnar, Borislav Petkov, Dave Hansen,
x86, H . Peter Anvin, linux-hyperv, linux-kernel
In-Reply-To: <aIJjrYne1VvGjBux@liuwe-devbox-ubuntu-v2.tail21d00.ts.net>
On Thu, Jul 24 2025 at 16:47, Wei Liu wrote:
> Nuno, can you please take a look at this patch?
Can we get this moving so that the legacy PCI stuff can be retired?
Thanks,
tglx
^ 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