* [PATCH 4/5] entry: Rename "kvm" entry code assets to "virt" to genericize APIs
From: Sean Christopherson @ 2025-08-25 20:06 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon,
Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Peter Zijlstra, Andy Lutomirski, Paul E. McKenney,
Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes,
Josh Triplett, Boqun Feng, Uladzislau Rezki
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, loongarch, kvm-riscv,
linux-riscv, linux-hyperv, rcu
In-Reply-To: <20250825200622.3759571-1-seanjc@google.com>
Rename the "kvm" entry code files and Kconfigs to use generic "virt"
nomenclature so that the code can be reused by other hypervisors (or
rather, their root/dom0 partition drivers), without incorrectly suggesting
the code somehow relies on and/or involves KVM.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
MAINTAINERS | 2 +-
arch/arm64/kvm/Kconfig | 2 +-
arch/loongarch/kvm/Kconfig | 2 +-
arch/riscv/kvm/Kconfig | 2 +-
arch/x86/kvm/Kconfig | 2 +-
include/linux/{entry-kvm.h => entry-virt.h} | 8 ++++----
include/linux/kvm_host.h | 6 +++---
include/linux/rcupdate.h | 2 +-
kernel/entry/Makefile | 2 +-
kernel/entry/{kvm.c => virt.c} | 2 +-
kernel/rcu/tree.c | 6 +++---
virt/kvm/Kconfig | 2 +-
12 files changed, 19 insertions(+), 19 deletions(-)
rename include/linux/{entry-kvm.h => entry-virt.h} (94%)
rename kernel/entry/{kvm.c => virt.c} (97%)
diff --git a/MAINTAINERS b/MAINTAINERS
index fed6cd812d79..17b9c9d7958e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10211,7 +10211,7 @@ L: linux-kernel@vger.kernel.org
S: Maintained
T: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core/entry
F: include/linux/entry-common.h
-F: include/linux/entry-kvm.h
+F: include/linux/entry-virt.h
F: include/linux/irq-entry-common.h
F: kernel/entry/
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index 713248f240e0..6f4fc3caa31a 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -25,7 +25,7 @@ menuconfig KVM
select HAVE_KVM_CPU_RELAX_INTERCEPT
select KVM_MMIO
select KVM_GENERIC_DIRTYLOG_READ_PROTECT
- select KVM_XFER_TO_GUEST_WORK
+ select VIRT_XFER_TO_GUEST_WORK
select KVM_VFIO
select HAVE_KVM_DIRTY_RING_ACQ_REL
select NEED_KVM_DIRTY_RING_WITH_BITMAP
diff --git a/arch/loongarch/kvm/Kconfig b/arch/loongarch/kvm/Kconfig
index 40eea6da7c25..ae64bbdf83a7 100644
--- a/arch/loongarch/kvm/Kconfig
+++ b/arch/loongarch/kvm/Kconfig
@@ -31,7 +31,7 @@ config KVM
select KVM_GENERIC_HARDWARE_ENABLING
select KVM_GENERIC_MMU_NOTIFIER
select KVM_MMIO
- select KVM_XFER_TO_GUEST_WORK
+ select VIRT_XFER_TO_GUEST_WORK
select SCHED_INFO
select GUEST_PERF_EVENTS if PERF_EVENTS
help
diff --git a/arch/riscv/kvm/Kconfig b/arch/riscv/kvm/Kconfig
index 5a62091b0809..c50328212917 100644
--- a/arch/riscv/kvm/Kconfig
+++ b/arch/riscv/kvm/Kconfig
@@ -30,7 +30,7 @@ config KVM
select KVM_GENERIC_DIRTYLOG_READ_PROTECT
select KVM_GENERIC_HARDWARE_ENABLING
select KVM_MMIO
- select KVM_XFER_TO_GUEST_WORK
+ select VIRT_XFER_TO_GUEST_WORK
select KVM_GENERIC_MMU_NOTIFIER
select SCHED_INFO
select GUEST_PERF_EVENTS if PERF_EVENTS
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 2c86673155c9..f81074b0c0a8 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -40,7 +40,7 @@ config KVM_X86
select HAVE_KVM_MSI
select HAVE_KVM_CPU_RELAX_INTERCEPT
select HAVE_KVM_NO_POLL
- select KVM_XFER_TO_GUEST_WORK
+ select VIRT_XFER_TO_GUEST_WORK
select KVM_GENERIC_DIRTYLOG_READ_PROTECT
select KVM_VFIO
select HAVE_KVM_PM_NOTIFIER if PM
diff --git a/include/linux/entry-kvm.h b/include/linux/entry-virt.h
similarity index 94%
rename from include/linux/entry-kvm.h
rename to include/linux/entry-virt.h
index 3644de7e6019..42c89e3e5ca7 100644
--- a/include/linux/entry-kvm.h
+++ b/include/linux/entry-virt.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __LINUX_ENTRYKVM_H
-#define __LINUX_ENTRYKVM_H
+#ifndef __LINUX_ENTRYVIRT_H
+#define __LINUX_ENTRYVIRT_H
#include <linux/static_call_types.h>
#include <linux/resume_user_mode.h>
@@ -10,7 +10,7 @@
#include <linux/tick.h>
/* Transfer to guest mode work */
-#ifdef CONFIG_KVM_XFER_TO_GUEST_WORK
+#ifdef CONFIG_VIRT_XFER_TO_GUEST_WORK
#ifndef ARCH_XFER_TO_GUEST_MODE_WORK
# define ARCH_XFER_TO_GUEST_MODE_WORK (0)
@@ -90,6 +90,6 @@ static inline bool xfer_to_guest_mode_work_pending(void)
lockdep_assert_irqs_disabled();
return __xfer_to_guest_mode_work_pending();
}
-#endif /* CONFIG_KVM_XFER_TO_GUEST_WORK */
+#endif /* CONFIG_VIRT_XFER_TO_GUEST_WORK */
#endif
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 598b9473e46d..70ac2267d5d0 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2,7 +2,7 @@
#ifndef __KVM_HOST_H
#define __KVM_HOST_H
-#include <linux/entry-kvm.h>
+#include <linux/entry-virt.h>
#include <linux/types.h>
#include <linux/hardirq.h>
#include <linux/list.h>
@@ -2444,7 +2444,7 @@ static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
}
#endif /* CONFIG_HAVE_KVM_VCPU_RUN_PID_CHANGE */
-#ifdef CONFIG_KVM_XFER_TO_GUEST_WORK
+#ifdef CONFIG_VIRT_XFER_TO_GUEST_WORK
static inline void kvm_handle_signal_exit(struct kvm_vcpu *vcpu)
{
vcpu->run->exit_reason = KVM_EXIT_INTR;
@@ -2461,7 +2461,7 @@ static inline int kvm_xfer_to_guest_mode_handle_work(struct kvm_vcpu *vcpu)
}
return r;
}
-#endif /* CONFIG_KVM_XFER_TO_GUEST_WORK */
+#endif /* CONFIG_VIRT_XFER_TO_GUEST_WORK */
/*
* If more than one page is being (un)accounted, @virt must be the address of
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 120536f4c6eb..1e1f3aa375d9 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -129,7 +129,7 @@ static inline void rcu_sysrq_start(void) { }
static inline void rcu_sysrq_end(void) { }
#endif /* #else #ifdef CONFIG_RCU_STALL_COMMON */
-#if defined(CONFIG_NO_HZ_FULL) && (!defined(CONFIG_GENERIC_ENTRY) || !defined(CONFIG_KVM_XFER_TO_GUEST_WORK))
+#if defined(CONFIG_NO_HZ_FULL) && (!defined(CONFIG_GENERIC_ENTRY) || !defined(CONFIG_VIRT_XFER_TO_GUEST_WORK))
void rcu_irq_work_resched(void);
#else
static __always_inline void rcu_irq_work_resched(void) { }
diff --git a/kernel/entry/Makefile b/kernel/entry/Makefile
index 77fcd83dd663..2333d70802e4 100644
--- a/kernel/entry/Makefile
+++ b/kernel/entry/Makefile
@@ -14,4 +14,4 @@ CFLAGS_common.o += -fno-stack-protector
obj-$(CONFIG_GENERIC_IRQ_ENTRY) += common.o
obj-$(CONFIG_GENERIC_SYSCALL) += syscall-common.o syscall_user_dispatch.o
-obj-$(CONFIG_KVM_XFER_TO_GUEST_WORK) += kvm.o
+obj-$(CONFIG_VIRT_XFER_TO_GUEST_WORK) += virt.o
diff --git a/kernel/entry/kvm.c b/kernel/entry/virt.c
similarity index 97%
rename from kernel/entry/kvm.c
rename to kernel/entry/virt.c
index 6fc762eaacca..c52f99249763 100644
--- a/kernel/entry/kvm.c
+++ b/kernel/entry/virt.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
-#include <linux/entry-kvm.h>
+#include <linux/entry-virt.h>
static int xfer_to_guest_mode_work(unsigned long ti_work)
{
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 8eff357b0436..371447651929 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -573,7 +573,7 @@ void rcutorture_format_gp_seqs(unsigned long long seqs, char *cp, size_t len)
}
EXPORT_SYMBOL_GPL(rcutorture_format_gp_seqs);
-#if defined(CONFIG_NO_HZ_FULL) && (!defined(CONFIG_GENERIC_ENTRY) || !defined(CONFIG_KVM_XFER_TO_GUEST_WORK))
+#if defined(CONFIG_NO_HZ_FULL) && (!defined(CONFIG_GENERIC_ENTRY) || !defined(CONFIG_VIRT_XFER_TO_GUEST_WORK))
/*
* An empty function that will trigger a reschedule on
* IRQ tail once IRQs get re-enabled on userspace/guest resume.
@@ -602,7 +602,7 @@ noinstr void rcu_irq_work_resched(void)
if (IS_ENABLED(CONFIG_GENERIC_ENTRY) && !(current->flags & PF_VCPU))
return;
- if (IS_ENABLED(CONFIG_KVM_XFER_TO_GUEST_WORK) && (current->flags & PF_VCPU))
+ if (IS_ENABLED(CONFIG_VIRT_XFER_TO_GUEST_WORK) && (current->flags & PF_VCPU))
return;
instrumentation_begin();
@@ -611,7 +611,7 @@ noinstr void rcu_irq_work_resched(void)
}
instrumentation_end();
}
-#endif /* #if defined(CONFIG_NO_HZ_FULL) && (!defined(CONFIG_GENERIC_ENTRY) || !defined(CONFIG_KVM_XFER_TO_GUEST_WORK)) */
+#endif /* #if defined(CONFIG_NO_HZ_FULL) && (!defined(CONFIG_GENERIC_ENTRY) || !defined(CONFIG_VIRT_XFER_TO_GUEST_WORK)) */
#ifdef CONFIG_PROVE_RCU
/**
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index 727b542074e7..ce843db53831 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -87,7 +87,7 @@ config HAVE_KVM_VCPU_RUN_PID_CHANGE
config HAVE_KVM_NO_POLL
bool
-config KVM_XFER_TO_GUEST_WORK
+config VIRT_XFER_TO_GUEST_WORK
bool
config HAVE_KVM_PM_NOTIFIER
--
2.51.0.261.g7ce5a0a67e-goog
^ permalink raw reply related
* [PATCH 5/5] Drivers: hv: Use common "entry virt" APIs to do work before running guest
From: Sean Christopherson @ 2025-08-25 20:06 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon,
Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Peter Zijlstra, Andy Lutomirski, Paul E. McKenney,
Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes,
Josh Triplett, Boqun Feng, Uladzislau Rezki
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, loongarch, kvm-riscv,
linux-riscv, linux-hyperv, rcu
In-Reply-To: <20250825200622.3759571-1-seanjc@google.com>
Use the kernel's common "entry virt" APIs to handle pending work prior to
(re)entering guest mode, now that the virt APIs don't have a superfluous
dependency on KVM.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
drivers/hv/Kconfig | 1 +
drivers/hv/mshv.h | 2 --
drivers/hv/mshv_common.c | 45 -------------------------------------
drivers/hv/mshv_root_main.c | 9 +++++---
4 files changed, 7 insertions(+), 50 deletions(-)
diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
index 57623ca7f350..cdb210cd3710 100644
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -66,6 +66,7 @@ config MSHV_ROOT
# no particular order, making it impossible to reassemble larger pages
depends on PAGE_SIZE_4KB
select EVENTFD
+ select VIRT_XFER_TO_GUEST_WORK
default n
help
Select this option to enable support for booting and running as root
diff --git a/drivers/hv/mshv.h b/drivers/hv/mshv.h
index db3aa3831c43..d4813df92b9c 100644
--- a/drivers/hv/mshv.h
+++ b/drivers/hv/mshv.h
@@ -25,6 +25,4 @@ int hv_call_set_vp_registers(u32 vp_index, u64 partition_id, u16 count,
int hv_call_get_partition_property(u64 partition_id, u64 property_code,
u64 *property_value);
-int mshv_do_pre_guest_mode_work(void);
-
#endif /* _MSHV_H */
diff --git a/drivers/hv/mshv_common.c b/drivers/hv/mshv_common.c
index b953b5e21110..aa2be51979fd 100644
--- a/drivers/hv/mshv_common.c
+++ b/drivers/hv/mshv_common.c
@@ -138,48 +138,3 @@ int hv_call_get_partition_property(u64 partition_id,
return 0;
}
EXPORT_SYMBOL_GPL(hv_call_get_partition_property);
-
-/*
- * Handle any pre-processing before going into the guest mode on this cpu, most
- * notably call schedule(). Must be invoked with both preemption and
- * interrupts enabled.
- *
- * Returns: 0 on success, -errno on error.
- */
-static int __mshv_do_pre_guest_mode_work(ulong th_flags)
-{
- if (th_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
- return -EINTR;
-
- if (th_flags & (_TIF_NEED_RESCHED | _TIF_NEED_RESCHED_LAZY))
- schedule();
-
- if (th_flags & _TIF_NOTIFY_RESUME)
- resume_user_mode_work(NULL);
-
- return 0;
-}
-
-int mshv_do_pre_guest_mode_work(void)
-{
- const ulong work_flags = _TIF_NOTIFY_SIGNAL | _TIF_SIGPENDING |
- _TIF_NEED_RESCHED | _TIF_NEED_RESCHED_LAZY |
- _TIF_NOTIFY_RESUME;
- ulong th_flags;
-
- th_flags = read_thread_flags();
- while (th_flags & work_flags) {
- int ret;
-
- /* nb: following will call schedule */
- ret = __mshv_do_pre_guest_mode_work(th_flags);
- if (ret)
- return ret;
-
- th_flags = read_thread_flags();
- }
-
- return 0;
-
-}
-EXPORT_SYMBOL_GPL(mshv_do_pre_guest_mode_work);
diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index 6f677fb93af0..387491ca16d6 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -8,6 +8,7 @@
* Authors: Microsoft Linux virtualization team
*/
+#include <linux/entry-virt.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/fs.h>
@@ -507,9 +508,11 @@ static long mshv_run_vp_with_root_scheduler(struct mshv_vp *vp)
u32 flags = 0;
struct hv_output_dispatch_vp output;
- ret = mshv_do_pre_guest_mode_work();
- if (ret)
- break;
+ if (__xfer_to_guest_mode_work_pending()) {
+ ret = xfer_to_guest_mode_handle_work();
+ if (ret)
+ break;
+ }
if (vp->run.flags.intercept_suspend)
flags |= HV_DISPATCH_VP_FLAG_CLEAR_INTERCEPT_SUSPEND;
--
2.51.0.261.g7ce5a0a67e-goog
^ permalink raw reply related
* RE: [PATCH v3 1/7] Drivers: hv: Introduce hv_hvcall_*() functions for hypercall arguments
From: Michael Kelley @ 2025-08-25 21:01 UTC (permalink / raw)
To: Mukesh R, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com, tglx@linutronix.de,
mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
hpa@zytor.com, lpieralisi@kernel.org, kw@linux.com,
manivannan.sadhasivam@linaro.org, robh@kernel.org,
bhelgaas@google.com, arnd@arndb.de
Cc: x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org
In-Reply-To: <d6e63ef3-bdd2-f185-f065-76b333dd1fc3@linux.microsoft.com>
From: Mukesh R <mrathor@linux.microsoft.com> Sent: Friday, August 22, 2025 7:25 PM
>
> On 8/21/25 19:10, Michael Kelley wrote:
> > From: Mukesh R <mrathor@linux.microsoft.com> Sent: Thursday, August 21, 2025 1:50 PM
> >>
> >> On 8/21/25 12:24, Michael Kelley wrote:
> >>> From: Mukesh R <mrathor@linux.microsoft.com> Sent: Wednesday, August 20, 2025 7:58 PM
> >>>>
> >>>> On 8/20/25 17:31, Mukesh R wrote:
> >>>>> On 4/15/25 11:07, mhkelley58@gmail.com wrote:
> >>>>>> From: Michael Kelley <mhklinux@outlook.com>
> >>>>>>
> >>>>>>
> >> <snip>
> >>>>>
> >>>>>
> >>>>> IMHO, this is unnecessary change that just obfuscates code. With status quo
> >>>>> one has the advantage of seeing what exactly is going on, one can use the
> >>>>> args any which way, change batch size any which way, and is thus flexible.
> >>>
> >>> I started this patch set in response to some errors in open coding the
> >>> use of hyperv_pcpu_input/output_arg, to see if helper functions could
> >>> regularize the usage and reduce the likelihood of future errors. Balancing
> >>> the pluses and minuses of the result, in my view the helper functions are
> >>> an improvement, though not overwhelmingly so. Others may see the
> >>> tradeoffs differently, and as such I would not go to the mat in arguing the
> >>> patches must be taken. But if we don't take them, we need to go back and
> >>> clean up minor errors and inconsistencies in the open coding at some
> >>> existing hypercall call sites.
> >>
> >> Yes, definitely. Assuming Nuno knows what issues you are referring to,
> >> I'll work with him to get them addressed asap. Thanks for noticing them.
> >> If Nuno is not aware, I'll ping you for more info.
> >>
> >>
> >>>>> With time these functions only get more complicated and error prone. The
> >>>>> saving of ram is very minimal, this makes analyzing crash dumps harder,
> >>>>> and in some cases like in your patch 3/7 disables unnecessarily in error case:
> >>>>>
> >>>>> - if (count > HV_MAX_MODIFY_GPA_REP_COUNT) {
> >>>>> - pr_err("Hyper-V: GPA count:%d exceeds supported:%lu\n", count,
> >>>>> - HV_MAX_MODIFY_GPA_REP_COUNT);
> >>>>> + local_irq_save(flags); <<<<<<<
> >>>>> ...
> >>>
> >>> FWIW, this error case is not disabled. It is checked a few lines further down as:
> >>
> >> I meant disabled interrupts. The check moves after disabling interrupts, so
> >> it runs "disabled" in traditional OS terminology :).
> >
> > Got it. But why is it problem to make this check with interrupts disabled?
>
> You are creating disabling overhead where that overhead previously
> did not exist.
I'm not clear on what you mean by "disabling overhead". The existing code
does the following:
1) Validate that "count" is not too big, and return an error if it is.
2) Disable interrupts
3) Populate the per-cpu hypercall input arg
4) Make the hypercall
5) Re-enable interrupts
With the patch, steps 1 and 2 are done in a different order:
2) Disable interrupts
1) Validate that "count" is not too big. Re-enable interrupts and return an error if it is.
3) Populate the per-cpu hypercall input arg
4) Make the hypercall
5) Re-enable interrupts
Validating "count" with interrupts disabled is probably an additional
2 or 3 instructions executed with interrupts disabled, which is negligible
compared to the thousands (or more) of instructions the hypercall will
execute with interrupts disabled.
Or are you referring to something else as "disabling overhead"?
>
>
> > The check is just for robustness and should never be true since
> > hv_mark_gpa_visiblity() is called from only one place that already ensures
> > the PFN count won't overflow a single page.
> >
> >>
> >>>
> >>> + if (count > batch_size) {
> >>> + pr_err("Hyper-V: GPA count:%d exceeds supported:%u\n", count,
> >>> + batch_size);
> >>>
> >>>>>
> >>>>> So, this is a nak from me. sorry.
> >>>>>
> >>>>
> >>>> Furthermore, this makes us lose the ability to permanently map
> >>>> input/output pages in the hypervisor. So, Wei kindly undo.
> >>>>
> >>>
> >>> Could you elaborate on "lose the ability to permanently map
> >>> input/output pages in the hypervisor"? What specifically can't be
> >>> done and why?
> >>
> >> Input and output are mapped at fixed GPA/SPA always to avoid hyp
> >> having to map/unmap every time.
> >
> > OK. But how does this patch set impede doing a fixed mapping?
>
> The output address can be varied depending on the hypercall, instead
> of it being fixed always at fixed address:
>
> *(void **)output = space + offset; <<<<<<
Agreed. But since mappings from GPA to SPA are page granular, having
such a fixed mapping means that there's a mapping for every byte in
the page containing the GPA to the corresponding byte in the SPA,
right? So even though the offset above may vary across hypercalls,
the output GPA still refers to the same page (since the offset is always
less than 4096), and that page has a fixed mapping. I would expect the
hypercall code in the hypervisor to look for an existing mapping based
on the output page, not the output address that includes the offset.
But I'm haven't looked at the hypervisor code. If the Hyper-V folks say
that a non-zero offset thwarts finding the existing mapping, what does
the hypervisor end up doing? Creating a 2nd mapping wouldn't seem
to make sense. So I'm really curious about what's going on ....
Michael
>
> > Wouldn't that fixed mapping be done at the time the page or pages
> > are allocated, and then be transparent to hypercall call sites?
> >
> >>
> >>> <snip>
> >>>
> >>>>>
> >>>>>> +/*
> >>>>>> + * Allocate one page that is shared between input and output args, which is
> >>>>>> + * sufficient for all current hypercalls. If a future hypercall requires
> >>>>>
> >>>>> That is incorrect. We've iommu map hypercalls that will use up entire page
> >>>>> for input. More coming as we implement ram withdrawl from the hypervisor.
> >>>
> >>> At least some form of ram withdrawal is already implemented upstream as
> >>> hv_call_withdraw_memory(). The hypercall has a very small input using the
> >>> hv_setup_in() helper, but the output list of PFNs must go to a separately
> >>> allocated page so it can be retained with interrupts enabled while
> >>> __free_page() is called. The use of this separate output page predates the
> >>> introduction of the hv_setup_in() helper.
> >>
> >> Yeah, I am talking about hyp memory that loader gives it, and during the
> >> lifetime it accumulates for VMs. We are opening the flood gates, so you
> >> will see lots patches very soon.
> >>
> >>
> >>> For iommu map hypercalls, what do the input and output look like? Is the
> >>> paradigm different from the typical small fixed portion plus a variable size
> >>> array of values that are fed into a rep hypercall? Is there also a large amount
> >>> of output from the hypercall? Just curious if there's a case that's fundamentally
> >>> different from the current set of hypercalls.
> >>
> >> Patches coming soon, but at high level, hypercall includes list of SPAs
> >> that hypevisor will map into the iommu. These can get large. We will be
> >> exploring what we can do better to pass them, perhaps multiple pages, not
> >> sure yet, but for now it's single page.
> >
> > To be clear, if the iommu hypercall does not produce any output, this patch
> > set uses the entire per-cpu hypercall arg page for input. For example,
>
> Good
>
> > hv_mark_gpa_visibility() uses the entire page for input, which is mostly an
> > array of PFNs.
> >
> > Using multiple input pages is definitely a new paradigm, on both the
> > hypervisor and guest sides, and that will need additional infrastructure,
> > with or without this patch set.
>
> Right. With this patch set, every hcall is affected rather than just one
> when code is modified to support that. That means one must test every
> hypercall.
>
> > I'm just trying to understand where there are real technical blockers vs.
> > concern about the style and the encapsulation the helpers impose.
>
> Well no technical blockers that can't be resolved, but style and obfuscation
> that helpers impose are big concern.
>
> Thanks,
> -Mukesh
^ permalink raw reply
* Re: [PATCH v3 0/7] hyperv: Introduce new way to manage hypercall args
From: Wei Liu @ 2025-08-25 21:39 UTC (permalink / raw)
To: mhklinux
Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, hpa,
lpieralisi, kw, manivannan.sadhasivam, robh, bhelgaas, arnd, x86,
linux-hyperv, linux-kernel, linux-pci, linux-arch
In-Reply-To: <20250415180728.1789-1-mhklinux@outlook.com>
On Tue, Apr 15, 2025 at 11:07:21AM -0700, mhkelley58@gmail.com wrote:
[...]
>
> This patch set is built against linux-next20250411.
>
> [1] https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/tlfs
>
> Michael Kelley (7):
> Drivers: hv: Introduce hv_hvcall_*() functions for hypercall arguments
> x86/hyperv: Use hv_hvcall_*() to set up hypercall arguments -- part 1
> x86/hyperv: Use hv_hvcall_*() to set up hypercall arguments -- part 2
> Drivers: hv: Use hv_hvcall_*() to set up hypercall arguments
> PCI: hv: Use hv_hvcall_*() to set up hypercall arguments
> Drivers: hv: Use hv_hvcall_*() to set up hypercall arguments for mshv
> code
> Drivers: hv: Replace hyperv_pcpu_input/output_arg with hyperv_pcpu_arg
I applied this series before but then there is a new discussion ongoing,
so I've dropped it from my tree for now until the discussion settles.
Wei
^ permalink raw reply
* Re: [PATCH 0/5] Drivers: hv: Fix NEED_RESCHED_LAZY and use common APIs
From: Wei Liu @ 2025-08-25 21:45 UTC (permalink / raw)
To: Sean Christopherson
Cc: Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon,
Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Peter Zijlstra,
Andy Lutomirski, Paul E. McKenney, Frederic Weisbecker,
Neeraj Upadhyay, Joel Fernandes, Josh Triplett, Boqun Feng,
Uladzislau Rezki, linux-kernel, linux-arm-kernel, kvmarm, kvm,
loongarch, kvm-riscv, linux-riscv, linux-hyperv, rcu, mrathor,
nunodasneves
In-Reply-To: <20250825200622.3759571-1-seanjc@google.com>
On Mon, Aug 25, 2025 at 01:06:17PM -0700, Sean Christopherson wrote:
> Fix a bug where MSHV root partitions don't honor NEED_RESCHED_LAZY, and then
> deduplicate the TIF related MSHV code by turning the "kvm" entry APIs into
> more generic "virt" APIs (which ideally would have been done when MSHV root
> support was added).
>
It is nice to have a common infrastructure.
Cc Mukesh and Nuno for review and test.
Thanks,
Wei
> Assuming all is well, maybe this could go through the tip tree?
>
> The Hyper-V stuff and non-x86 architectures are compile-tested only.
>
> Sean Christopherson (5):
> Drivers: hv: Move TIF pre-guest work handling fully into mshv_common.c
> Drivers: hv: Handle NEED_RESCHED_LAZY before transferring to guest
> entry/kvm: KVM: Move KVM details related to signal/-EINTR into KVM
> proper
> entry: Rename "kvm" entry code assets to "virt" to genericize APIs
> Drivers: hv: Use common "entry virt" APIs to do work before running
> guest
>
> MAINTAINERS | 2 +-
> arch/arm64/kvm/Kconfig | 2 +-
> arch/arm64/kvm/arm.c | 3 +-
> arch/loongarch/kvm/Kconfig | 2 +-
> arch/loongarch/kvm/vcpu.c | 3 +-
> arch/riscv/kvm/Kconfig | 2 +-
> arch/riscv/kvm/vcpu.c | 3 +-
> arch/x86/kvm/Kconfig | 2 +-
> arch/x86/kvm/vmx/vmx.c | 1 -
> arch/x86/kvm/x86.c | 3 +-
> drivers/hv/Kconfig | 1 +
> drivers/hv/mshv.h | 2 --
> drivers/hv/mshv_common.c | 22 ---------------
> drivers/hv/mshv_root_main.c | 31 ++++-----------------
> include/linux/{entry-kvm.h => entry-virt.h} | 19 +++++--------
> include/linux/kvm_host.h | 17 +++++++++--
> include/linux/rcupdate.h | 2 +-
> kernel/entry/Makefile | 2 +-
> kernel/entry/{kvm.c => virt.c} | 15 ++++------
> kernel/rcu/tree.c | 6 ++--
> virt/kvm/Kconfig | 2 +-
> 21 files changed, 49 insertions(+), 93 deletions(-)
> rename include/linux/{entry-kvm.h => entry-virt.h} (83%)
> rename kernel/entry/{kvm.c => virt.c} (66%)
>
>
> base-commit: 1b237f190eb3d36f52dffe07a40b5eb210280e00
> --
> 2.51.0.261.g7ce5a0a67e-goog
>
^ permalink raw reply
* Re: [PATCH 0/5] Drivers: hv: Fix NEED_RESCHED_LAZY and use common APIs
From: Peter Zijlstra @ 2025-08-25 22:23 UTC (permalink / raw)
To: Sean Christopherson
Cc: Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon,
Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Andy Lutomirski,
Paul E. McKenney, Frederic Weisbecker, Neeraj Upadhyay,
Joel Fernandes, Josh Triplett, Boqun Feng, Uladzislau Rezki,
linux-kernel, linux-arm-kernel, kvmarm, kvm, loongarch, kvm-riscv,
linux-riscv, linux-hyperv, rcu
In-Reply-To: <20250825200622.3759571-1-seanjc@google.com>
On Mon, Aug 25, 2025 at 01:06:17PM -0700, Sean Christopherson wrote:
> Fix a bug where MSHV root partitions don't honor NEED_RESCHED_LAZY, and then
> deduplicate the TIF related MSHV code by turning the "kvm" entry APIs into
> more generic "virt" APIs (which ideally would have been done when MSHV root
> support was added).
>
> Assuming all is well, maybe this could go through the tip tree?
>
> The Hyper-V stuff and non-x86 architectures are compile-tested only.
I suspect there's more of this wreckage in the new VTL driver that
they just put in:
https://lore.kernel.org/all/20250729051436.190703-3-namjain@linux.microsoft.com/
although ideally they rip that thing out. I don't know how that code can
ever be correct.
^ permalink raw reply
* Re: [PATCH 0/5] Drivers: hv: Fix NEED_RESCHED_LAZY and use common APIs
From: Wei Liu @ 2025-08-25 22:26 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Sean Christopherson, Marc Zyngier, Oliver Upton, Catalin Marinas,
Will Deacon, Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Andy Lutomirski, Paul E. McKenney, Frederic Weisbecker,
Neeraj Upadhyay, Joel Fernandes, Josh Triplett, Boqun Feng,
Uladzislau Rezki, linux-kernel, linux-arm-kernel, kvmarm, kvm,
loongarch, kvm-riscv, linux-riscv, linux-hyperv, rcu
In-Reply-To: <20250825222319.GO3419281@noisy.programming.kicks-ass.net>
On Tue, Aug 26, 2025 at 12:23:19AM +0200, Peter Zijlstra wrote:
> On Mon, Aug 25, 2025 at 01:06:17PM -0700, Sean Christopherson wrote:
> > Fix a bug where MSHV root partitions don't honor NEED_RESCHED_LAZY, and then
> > deduplicate the TIF related MSHV code by turning the "kvm" entry APIs into
> > more generic "virt" APIs (which ideally would have been done when MSHV root
> > support was added).
> >
> > Assuming all is well, maybe this could go through the tip tree?
> >
> > The Hyper-V stuff and non-x86 architectures are compile-tested only.
>
> I suspect there's more of this wreckage in the new VTL driver that
> they just put in:
>
> https://lore.kernel.org/all/20250729051436.190703-3-namjain@linux.microsoft.com/
>
> although ideally they rip that thing out. I don't know how that code can
> ever be correct.
Please give us some time, Peter. I have asked the corresponding team to
respond to your comments.
Wei
^ permalink raw reply
* Re: [PATCH hyperv-next v4 15/16] Drivers: hv: Support establishing the confidential VMBus connection
From: Roman Kisel @ 2025-08-25 22:41 UTC (permalink / raw)
To: dan.j.williams
Cc: Tianyu.Lan, alok.a.tiwari, apais, arnd, benhill, bp, bperkins,
corbet, dave.hansen, decui, haiyangz, hpa, kys, linux-arch,
linux-coco, linux-doc, linux-hyperv, linux-kernel, mhklinux,
mingo, rdunlap, romank, sunilmut, tglx, wei.liu, x86
In-Reply-To: <68a4ddff258de_2709100ba@dwillia2-xfh.jf.intel.com.notmuch>
[...]
>> + *
>> + * All scenarios here are:
>> + * 1. No paravisor,
>> + * 2. Paravisor without VMBus relay, no hardware isolation,
>> + * 3. Paravisor without VMBus relay, with hardware isolation,
>> + * 4. Paravisor with VMBus relay, no hardware isolation,
>> + * 5. Paravisor with VMBus relay, with hardware isolation.
>> + *
>>
> I read this blurb looking for answers to my question below, no luck, and
> left further wondering what is the comment trying to convey to future
> maintenance?
The intention was to enumerate scenarios in which the driver executes
this code to document what to expect of the conditional statement
| if (ms_hyperv.paravisor_present && (hv_isolation_type_tdx() || hv_isolation_type_snp()))
[...]
> In comparison to PCIe TDISP where there is an explicit validation step
> of cryptographic evidence that the platform is what it claims to be, I
> am missing the same for this.
>
This doesn't replace TDISP, I'll do a better job of supplementing the code changes
with documentation and comments! Any suggestions are greatly appreciated.
A fully-enlightened Linux guest could just use TDISP once support for that is available
in the Linux kernel. Before it is, the non-fully enlightened Linux guests (they can only deal
with accepting memory and sharing memory with the host) could rely on the paravisor to talk
to such devices. The TDISP device will be connected to the paravisor, and the paravisor will
provide the paravirtualized storage and network over the VMBus channels to the Linux guest.
The patch set is a building block for building a confidential I/O path for the non-fully
enlightened Linux guests. It would be great to have the Linux storage and network stack not
to share pages with the host (and not bounce-buffer) if the storage and network are
paravirtualized && use the Confidential VMBus. In the first version of the patchset I had
patches for that, yet that was considered too naive to be merged in the main line kernel so
I dropped them. But even without that, this patch series protects the control plane and the
data plane from the host with the exception of the pages the guest might use for bounce-buffering
although it could've avoided that in this case.
I mentioned that the paravisor will be handling the TDISP device for such guests.
As folks might know, we use the OpenHCL paravisor which is a Linux kernel with the VTL
mode patches we've been upstreaming (links to the repos are in the cover letter), and
the OpenVMM running in the user land. The question would be if TDISP isn't available
in the Linux kernel, how one would get it working in the OpenHCL paravisor that itself
runs Linux? The SEV guest device in the paravisor kernel is being extended to handle
TIO. Once TDISP support is available in the mainline kernel, the paravisor will switch
to using the mainline implementation.
> I would expect something like a paravisor signed golden measurement with
> a certificate that can be built-in to the kernel to validate that "yes,
> in addition to the platform claims that can be emulated, this bus
> enumeration is signed by an authority this kernel image trusts."
>
> My motivation for commenting here is for alignment purposes with the
> PCIe TDISP enabling and wider concerns about accepting other devices for
> private operation. Specifically, I want to align on a shared
> representation in the device-core (struct device) to communicate that a
> device is either on a bus that has been accepted for private operation
> (confidential-vmbus today, potentially signed-ACPI-devices tomorrow), or
> is a device that has been individually accepted for private operation
> (PCIe TDISP). In both cases there needs to be either a golden
> measurement mechanism built-in, or a userspace acceptance dependency in
> the flow.
>
> Otherwise what mitigates a guest conveying secrets to a device that is
> merely emulating a trusted bus/device?
^ permalink raw reply
* Re: [PATCH 0/5] Drivers: hv: Fix NEED_RESCHED_LAZY and use common APIs
From: Nuno Das Neves @ 2025-08-25 23:08 UTC (permalink / raw)
To: Sean Christopherson, Marc Zyngier, Oliver Upton, Catalin Marinas,
Will Deacon, Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Peter Zijlstra, Andy Lutomirski, Paul E. McKenney,
Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes,
Josh Triplett, Boqun Feng, Uladzislau Rezki
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, loongarch, kvm-riscv,
linux-riscv, linux-hyperv, rcu
In-Reply-To: <20250825200622.3759571-1-seanjc@google.com>
On 8/25/2025 1:06 PM, Sean Christopherson wrote:
> Fix a bug where MSHV root partitions don't honor NEED_RESCHED_LAZY, and then
> deduplicate the TIF related MSHV code by turning the "kvm" entry APIs into
> more generic "virt" APIs (which ideally would have been done when MSHV root
> support was added).
>
> Assuming all is well, maybe this could go through the tip tree?
>
> The Hyper-V stuff and non-x86 architectures are compile-tested only.
>
Thanks Sean, I can test the root partition changes.
A similar change will be needed in mshv_vtl_main.c since it also calls
mshv_do_pre_guest_mode_work() (hence the "common" in mshv_common.c).
Also, is it possible to make all the mshv driver changes in a single patch?
It seems like it would be cleaner than refactoring it in patches 1 & 2 and
then deleting all the refactored code in patch 5.
Thanks
Nuno
> Sean Christopherson (5):
> Drivers: hv: Move TIF pre-guest work handling fully into mshv_common.c
> Drivers: hv: Handle NEED_RESCHED_LAZY before transferring to guest
> entry/kvm: KVM: Move KVM details related to signal/-EINTR into KVM
> proper
> entry: Rename "kvm" entry code assets to "virt" to genericize APIs
> Drivers: hv: Use common "entry virt" APIs to do work before running
> guest
>
> MAINTAINERS | 2 +-
> arch/arm64/kvm/Kconfig | 2 +-
> arch/arm64/kvm/arm.c | 3 +-
> arch/loongarch/kvm/Kconfig | 2 +-
> arch/loongarch/kvm/vcpu.c | 3 +-
> arch/riscv/kvm/Kconfig | 2 +-
> arch/riscv/kvm/vcpu.c | 3 +-
> arch/x86/kvm/Kconfig | 2 +-
> arch/x86/kvm/vmx/vmx.c | 1 -
> arch/x86/kvm/x86.c | 3 +-
> drivers/hv/Kconfig | 1 +
> drivers/hv/mshv.h | 2 --
> drivers/hv/mshv_common.c | 22 ---------------
> drivers/hv/mshv_root_main.c | 31 ++++-----------------
> include/linux/{entry-kvm.h => entry-virt.h} | 19 +++++--------
> include/linux/kvm_host.h | 17 +++++++++--
> include/linux/rcupdate.h | 2 +-
> kernel/entry/Makefile | 2 +-
> kernel/entry/{kvm.c => virt.c} | 15 ++++------
> kernel/rcu/tree.c | 6 ++--
> virt/kvm/Kconfig | 2 +-
> 21 files changed, 49 insertions(+), 93 deletions(-)
> rename include/linux/{entry-kvm.h => entry-virt.h} (83%)
> rename kernel/entry/{kvm.c => virt.c} (66%)
>
>
> base-commit: 1b237f190eb3d36f52dffe07a40b5eb210280e00
^ permalink raw reply
* Re: [PATCH v3 1/7] Drivers: hv: Introduce hv_hvcall_*() functions for hypercall arguments
From: Nuno Das Neves @ 2025-08-26 0:13 UTC (permalink / raw)
To: Michael Kelley, Mukesh R, kys@microsoft.com,
haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
dave.hansen@linux.intel.com, hpa@zytor.com, lpieralisi@kernel.org,
kw@linux.com, manivannan.sadhasivam@linaro.org, robh@kernel.org,
bhelgaas@google.com, arnd@arndb.de
Cc: x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org
In-Reply-To: <SN6PR02MB41576739C778676C009D5A86D43DA@SN6PR02MB4157.namprd02.prod.outlook.com>
On 8/21/2025 7:16 PM, Michael Kelley wrote:
> From: Mukesh R <mrathor@linux.microsoft.com> Sent: Thursday, August 21, 2025 2:16 PM
>>
>> On 8/21/25 13:49, Mukesh R wrote:
>>> On 8/21/25 12:24, Michael Kelley wrote:
>>>> From: Mukesh R <mrathor@linux.microsoft.com> Sent: Wednesday, August 20, 2025 7:58 PM
>>>>>
>>>>> On 8/20/25 17:31, Mukesh R wrote:
>>>>>> On 4/15/25 11:07, mhkelley58@gmail.com wrote:
>>>>>>> From: Michael Kelley <mhklinux@outlook.com>
>>>>>>>
>>>>>>>
>>> <snip>
>>>>>>
>>>>>>
>>>>>> IMHO, this is unnecessary change that just obfuscates code. With status quo
>>>>>> one has the advantage of seeing what exactly is going on, one can use the
>>>>>> args any which way, change batch size any which way, and is thus flexible.
>>>>
>>>> I started this patch set in response to some errors in open coding the
>>>> use of hyperv_pcpu_input/output_arg, to see if helper functions could
>>>> regularize the usage and reduce the likelihood of future errors. Balancing
>>>> the pluses and minuses of the result, in my view the helper functions are
>>>> an improvement, though not overwhelmingly so. Others may see the
>>>> tradeoffs differently, and as such I would not go to the mat in arguing the
>>>> patches must be taken. But if we don't take them, we need to go back and
>>>> clean up minor errors and inconsistencies in the open coding at some
>>>> existing hypercall call sites.
>>>
>>> Yes, definitely. Assuming Nuno knows what issues you are referring to,
>>> I'll work with him to get them addressed asap. Thanks for noticing them.
>>> If Nuno is not aware, I'll ping you for more info.
>>
>> Talked to Nuno, he's not aware of anything pending or details. So if you
>> can kindly list them out here, I will make sure it gets addressed right
>> away.
>>
>
> I didn't catalog the issues as I came across them when doing this patch
> set. :-( I don't think any are bugs that could break things now. They were
> things like not ensuring that all hypercall input fields are initialized to zero,
> duplicate initialization to zero, and unnecessary initialization of hypercall
> output memory. In general, how the hypercall args are set up is inconsistent
> across different hypercall call sites, and that inconsistency can lead to errors,
> which is what I was trying to address.
>
> But I can go back and come up with a list if that's where we're headed.
Hi Michael and Mukesh,
Just a suggestion, how about a simpler set of macros that doesn't really change
the existing paradigm, but can be used to improve the consistency across the
various hypercall sites.
e.g. for getting and zeroing the input page:
#define hv_get_input_ptr(in_ptr) \
({ \
static_assert(sizeof(*in_ptr) <= HV_HYP_PAGE_SIZE); \
void *__arg = *this_cpu_ptr(hyperv_pcpu_input_arg); \
memset(__arg, 0, sizeof(*in_ptr)); \
__arg; \
})
(And something similar for the output arg which doesn't need memset())
And for batch size, it can be very simple, although there's both the case
of argument + array elements, and just array elements:
#define hv_arg_get_batch_size(arg_ptr, element_ptr) \
((HV_HYP_PAGE_SIZE - sizeof(*arg_ptr)) / sizeof(*element_ptr))
#define hv_get_batch_size(element_ptr) (HV_HYP_PAGE_SIZE / sizeof(*element_ptr))
Usage:
struct hv_input_map_gpa_pages *input_page = hv_get_input_ptr(input_page);
int batch_size = hv_arg_get_batch_size(input_page, &input_page->source_gpa_page_list[0]);
Nuno
>
> Michael
^ permalink raw reply
* Re: [PATCH 0/5] Drivers: hv: Fix NEED_RESCHED_LAZY and use common APIs
From: Sean Christopherson @ 2025-08-26 0:27 UTC (permalink / raw)
To: Nuno Das Neves
Cc: Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon,
Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Peter Zijlstra,
Andy Lutomirski, Paul E. McKenney, Frederic Weisbecker,
Neeraj Upadhyay, Joel Fernandes, Josh Triplett, Boqun Feng,
Uladzislau Rezki, linux-kernel, linux-arm-kernel, kvmarm, kvm,
loongarch, kvm-riscv, linux-riscv, linux-hyperv, rcu
In-Reply-To: <3188ca61-2591-4576-9777-1671689b7235@linux.microsoft.com>
On Mon, Aug 25, 2025, Nuno Das Neves wrote:
> On 8/25/2025 1:06 PM, Sean Christopherson wrote:
> > Fix a bug where MSHV root partitions don't honor NEED_RESCHED_LAZY, and then
> > deduplicate the TIF related MSHV code by turning the "kvm" entry APIs into
> > more generic "virt" APIs (which ideally would have been done when MSHV root
> > support was added).
> >
> > Assuming all is well, maybe this could go through the tip tree?
> >
> > The Hyper-V stuff and non-x86 architectures are compile-tested only.
> >
>
> Thanks Sean, I can test the root partition changes.
>
> A similar change will be needed in mshv_vtl_main.c since it also calls
> mshv_do_pre_guest_mode_work() (hence the "common" in mshv_common.c).
Oof, more dependencies. I suppose the easiest thing would be to send a series
against
git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git queue
and then route everything through there?
Alternatively, frontload the MSHV fixes (which I'll do regardless) and take those
through hyperv and the rest through the tip tree? That seems like an absurd
amount of juggling though, especially if we want to get the cleanups into 6.18.
And if none of these lands, it's MSHV that'll suffer the most, so betting it all
on the hyperv tree doesn't seem terrible.
> Also, is it possible to make all the mshv driver changes in a single patch?
It's certainly possible, but I'd prefer not do to that.
> It seems like it would be cleaner than refactoring it in patches 1 & 2 and
> then deleting all the refactored code in patch 5.
Only if you don't care about backporting fixes, bisection, or maintaining code.
E.g. if checking NEED_RESCHED_LAZY somehow causes issues, it would be really nice
for that to bisect to exactly that patch, not a patch that also switches to a
completely different set of APIs.
And if someone is wants the fixes in a pre-6.18 kernel, they don't need to
backport all of the KVM and entry code changes just to get the fix.
As for the maintenance headache, see above.
^ permalink raw reply
* Re: [PATCH net] net: mana: Remove redundant netdev_lock_ops_to_full() calls
From: Jakub Kicinski @ 2025-08-26 0:41 UTC (permalink / raw)
To: Saurabh Sengar
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
pabeni, shradhagupta, ernis, dipayanroy, shirazsaleem,
linux-hyperv, netdev, linux-kernel, ssengar, stable
In-Reply-To: <1756119794-20110-1-git-send-email-ssengar@linux.microsoft.com>
On Mon, 25 Aug 2025 04:03:14 -0700 Saurabh Sengar wrote:
> NET_SHAPER is always selected for MANA driver. When NET_SHAPER is enabled,
> netdev_lock_ops_to_full() reduces effectively to only an assert for lock,
> which is always held in the path when NET_SHAPER is enabled.
>
> Remove the redundant netdev_lock_ops_to_full() call.
>
> Fixes: d5c8f0e4e0cb ("net: mana: Fix potential deadlocks in mana napi ops")
> Cc: stable@vger.kernel.org
If the call is a nop why is this a stable-worthy fix?
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH v3 1/7] Drivers: hv: Introduce hv_hvcall_*() functions for hypercall arguments
From: Mukesh R @ 2025-08-26 1:46 UTC (permalink / raw)
To: Nuno Das Neves, Michael Kelley, kys@microsoft.com,
haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
dave.hansen@linux.intel.com, hpa@zytor.com, lpieralisi@kernel.org,
kw@linux.com, manivannan.sadhasivam@linaro.org, robh@kernel.org,
bhelgaas@google.com, arnd@arndb.de
Cc: x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org
In-Reply-To: <209e7fe9-cb5c-4e7c-8b5c-544387cf0927@linux.microsoft.com>
On 8/25/25 17:13, Nuno Das Neves wrote:
> On 8/21/2025 7:16 PM, Michael Kelley wrote:
>> From: Mukesh R <mrathor@linux.microsoft.com> Sent: Thursday, August 21, 2025 2:16 PM
>>>
>>> On 8/21/25 13:49, Mukesh R wrote:
>>>> On 8/21/25 12:24, Michael Kelley wrote:
>>>>> From: Mukesh R <mrathor@linux.microsoft.com> Sent: Wednesday, August 20, 2025 7:58 PM
>>>>>>
>>>>>> On 8/20/25 17:31, Mukesh R wrote:
>>>>>>> On 4/15/25 11:07, mhkelley58@gmail.com wrote:
>>>>>>>> From: Michael Kelley <mhklinux@outlook.com>
>>>>>>>>
>>>>>>>>
>>>> <snip>
>>>>>>>
>>>>>>>
>>>>>>> IMHO, this is unnecessary change that just obfuscates code. With status quo
>>>>>>> one has the advantage of seeing what exactly is going on, one can use the
>>>>>>> args any which way, change batch size any which way, and is thus flexible.
>>>>>
>>>>> I started this patch set in response to some errors in open coding the
>>>>> use of hyperv_pcpu_input/output_arg, to see if helper functions could
>>>>> regularize the usage and reduce the likelihood of future errors. Balancing
>>>>> the pluses and minuses of the result, in my view the helper functions are
>>>>> an improvement, though not overwhelmingly so. Others may see the
>>>>> tradeoffs differently, and as such I would not go to the mat in arguing the
>>>>> patches must be taken. But if we don't take them, we need to go back and
>>>>> clean up minor errors and inconsistencies in the open coding at some
>>>>> existing hypercall call sites.
>>>>
>>>> Yes, definitely. Assuming Nuno knows what issues you are referring to,
>>>> I'll work with him to get them addressed asap. Thanks for noticing them.
>>>> If Nuno is not aware, I'll ping you for more info.
>>>
>>> Talked to Nuno, he's not aware of anything pending or details. So if you
>>> can kindly list them out here, I will make sure it gets addressed right
>>> away.
>>>
>>
>> I didn't catalog the issues as I came across them when doing this patch
>> set. :-( I don't think any are bugs that could break things now. They were
>> things like not ensuring that all hypercall input fields are initialized to zero,
>> duplicate initialization to zero, and unnecessary initialization of hypercall
>> output memory. In general, how the hypercall args are set up is inconsistent
>> across different hypercall call sites, and that inconsistency can lead to errors,
>> which is what I was trying to address.
>>
>> But I can go back and come up with a list if that's where we're headed.
>
> Hi Michael and Mukesh,
>
> Just a suggestion, how about a simpler set of macros that doesn't really change
> the existing paradigm, but can be used to improve the consistency across the
> various hypercall sites.
>
> e.g. for getting and zeroing the input page:
>
> #define hv_get_input_ptr(in_ptr) \
> ({ \
> static_assert(sizeof(*in_ptr) <= HV_HYP_PAGE_SIZE); \
> void *__arg = *this_cpu_ptr(hyperv_pcpu_input_arg); \
> memset(__arg, 0, sizeof(*in_ptr)); \
> __arg; \
> })
Ugh! What is the problem that we are trying to solve? The code is
simple and clear today, tells the reader exactly what is being used and
for how many bytes etc. What if the input to hyp is a list of pfns, maybe
a void *? And if we want to do any complex stuff, we'll just keep adding
parameters to the macro. IMO, complex macros just obfuscate code! I think
this is just not worth it right now. We'll look ways to enhance hcall params
in future, perhaps we can address it then if there are any real issues.
Thanks,
-Mukesh
> (And something similar for the output arg which doesn't need memset())
>
> And for batch size, it can be very simple, although there's both the case
> of argument + array elements, and just array elements:
>
> #define hv_arg_get_batch_size(arg_ptr, element_ptr) \
> ((HV_HYP_PAGE_SIZE - sizeof(*arg_ptr)) / sizeof(*element_ptr))
>
> #define hv_get_batch_size(element_ptr) (HV_HYP_PAGE_SIZE / sizeof(*element_ptr))
>
> Usage:
>
> struct hv_input_map_gpa_pages *input_page = hv_get_input_ptr(input_page);
> int batch_size = hv_arg_get_batch_size(input_page, &input_page->source_gpa_page_list[0]);
>
>
>
> Nuno
>
>>
>> Michael
^ permalink raw reply
* Re: [PATCH net] net: mana: Remove redundant netdev_lock_ops_to_full() calls
From: Saurabh Singh Sengar @ 2025-08-26 4:57 UTC (permalink / raw)
To: Jakub Kicinski
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
pabeni, shradhagupta, ernis, dipayanroy, shirazsaleem,
linux-hyperv, netdev, linux-kernel, ssengar, stable
In-Reply-To: <20250825174133.30e58c60@kernel.org>
On Mon, Aug 25, 2025 at 05:41:33PM -0700, Jakub Kicinski wrote:
> On Mon, 25 Aug 2025 04:03:14 -0700 Saurabh Sengar wrote:
> > NET_SHAPER is always selected for MANA driver. When NET_SHAPER is enabled,
> > netdev_lock_ops_to_full() reduces effectively to only an assert for lock,
> > which is always held in the path when NET_SHAPER is enabled.
> >
> > Remove the redundant netdev_lock_ops_to_full() call.
> >
> > Fixes: d5c8f0e4e0cb ("net: mana: Fix potential deadlocks in mana napi ops")
> > Cc: stable@vger.kernel.org
>
> If the call is a nop why is this a stable-worthy fix?
I am fine removing CC and fixes tag.
I can send a V2 for it.
- Saurabh
^ permalink raw reply
* Re: [PATCH] x86/hyperv: Export hv_hypercall_pg unconditionally
From: Naman Jain @ 2025-08-26 11:30 UTC (permalink / raw)
To: Peter Zijlstra
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H . Peter Anvin, linux-hyperv, linux-kernel, mhklinux
In-Reply-To: <20250825094247.GU3245006@noisy.programming.kicks-ass.net>
On 8/25/2025 3:12 PM, Peter Zijlstra wrote:
> On Mon, Aug 25, 2025 at 11:22:08AM +0530, Naman Jain wrote:
>> With commit 0e20f1f4c2cb ("x86/hyperv: Clean up hv_do_hypercall()"),
>> config checks were added to conditionally restrict export
>> of hv_hypercall_pg symbol at the same time when a usage of that symbol
>> was added in mshv_vtl_main.c driver. This results in missing symbol
>> warning when mshv_vtl_main is compiled. Change the logic to
>> export it unconditionally.
>>
>> Fixes: 96a1d2495c2f ("Drivers: hv: Introduce mshv_vtl driver")
>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
>
> Oh gawd, that commit is terrible and adds yet another hypercall
> interface.
>
> I would argue the proper fix is moving the whole of mshv_vtl_return()
> into the kernel proper and doing it like hv_std_hypercall() on x86_64.
Thanks for the review comments.
This is doable, I can move the hypercall part of it to
arch/x86/hyperv/hv_init.c if I understand correctly.
>
> Additionally, how is that function not utterly broken? What happens if
> an interrupt or NMI comes in after native_write_cr2() and before the
> actual hypercall does VMEXIT and trips a #PF?
mshv_vtl driver is used for OpenHCL paravisor. The interrupts are
disabled, and NMIs aren't sent to the paravisor by the virt stack.
>
> And an rax:rcx return, I though the canonical pair was AX,DX !?!?
Here, the code uses rax and rcx not as a means to return one 128 bit
value. The code uses them in that way as an ABI.
>
> Also, that STACK_FRAME_NON_STANDARD() annotation is broken, this must
> not be used for anything that can end up in vmlinux.o -- that is, the
> moment you built-in this driver (=y) this comes unstuck.
>
> The reason you're getting warnings is because you're violating the
> normal calling convention and scribbling BP, we yelled at the TDX guys
> for doing this, now you're getting yelled at. WTF !?!
>
> Please explain how just shutting up objtool makes the unwind work when
> the NMI hits your BP scribble?
>
Returning to a lower VTL treats the base pointer register as a general
purpose one and this VTL transition function makes sure to preserve the
bp register due to which the objtool trips over on the assembly touching
the bp register. We considered this warning harmless and thus we are
using this macro. Moreover NMIs are not an issue here as they won't be
coming as mentioned other. If there are alternate approaches that I
should be using, please suggest.
I now understand that as part of your effort to enable IBT config on
x64, you changed the indirect calls to direct calls in Hyper-V. As of
today, there is no requirement to enable IBT in OpenHCL kernel as this
runs as a paravisor in VTL2 and it does not effect the guest VMs running
in VTL0.
I can disable CONFIG_X86_KERNEL_IBT when CONFIG_MSHV_VTL is enabled in
Kconfig in next version.
> All in all, I would suggest fixing this by reverting that patch and
> trying again after fixing the calling convention of that hypercall.
>
>
> Yours grumpy..
Regards,
Naman
^ permalink raw reply
* Re: [PATCH] x86/hyperv: Export hv_hypercall_pg unconditionally
From: Peter Zijlstra @ 2025-08-26 12:07 UTC (permalink / raw)
To: Naman Jain
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H . Peter Anvin, linux-hyperv, linux-kernel, mhklinux
In-Reply-To: <f154d997-f7a6-4379-b7e8-ac4ba990425c@linux.microsoft.com>
On Tue, Aug 26, 2025 at 05:00:31PM +0530, Naman Jain wrote:
>
>
> On 8/25/2025 3:12 PM, Peter Zijlstra wrote:
> > On Mon, Aug 25, 2025 at 11:22:08AM +0530, Naman Jain wrote:
> > > With commit 0e20f1f4c2cb ("x86/hyperv: Clean up hv_do_hypercall()"),
> > > config checks were added to conditionally restrict export
> > > of hv_hypercall_pg symbol at the same time when a usage of that symbol
> > > was added in mshv_vtl_main.c driver. This results in missing symbol
> > > warning when mshv_vtl_main is compiled. Change the logic to
> > > export it unconditionally.
> > >
> > > Fixes: 96a1d2495c2f ("Drivers: hv: Introduce mshv_vtl driver")
> > > Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> >
> > Oh gawd, that commit is terrible and adds yet another hypercall
> > interface.
> >
> > I would argue the proper fix is moving the whole of mshv_vtl_return()
> > into the kernel proper and doing it like hv_std_hypercall() on x86_64.
>
> Thanks for the review comments.
>
> This is doable, I can move the hypercall part of it to
> arch/x86/hyperv/hv_init.c if I understand correctly.
>
> >
> > Additionally, how is that function not utterly broken? What happens if
> > an interrupt or NMI comes in after native_write_cr2() and before the
> > actual hypercall does VMEXIT and trips a #PF?
>
> mshv_vtl driver is used for OpenHCL paravisor. The interrupts are
> disabled, and NMIs aren't sent to the paravisor by the virt stack.
I do not know what OpenHCL is. Nor is it clear from the code what NMIs
can't happen. Anyway, same can be achieved with breakpoints / kprobes.
You can get a trap after setting CR2 and scribble it.
You simply cannot use CR2 this way.
> > And an rax:rcx return, I though the canonical pair was AX,DX !?!?
>
> Here, the code uses rax and rcx not as a means to return one 128 bit
> value. The code uses them in that way as an ABI.
Still daft. Creating an ABI that goes against pre-existing conventions
is weird.
> > Also, that STACK_FRAME_NON_STANDARD() annotation is broken, this must
> > not be used for anything that can end up in vmlinux.o -- that is, the
> > moment you built-in this driver (=y) this comes unstuck.
> >
> > The reason you're getting warnings is because you're violating the
> > normal calling convention and scribbling BP, we yelled at the TDX guys
> > for doing this, now you're getting yelled at. WTF !?!
> >
> > Please explain how just shutting up objtool makes the unwind work when
> > the NMI hits your BP scribble?
>
> Returning to a lower VTL treats the base pointer register as a general
> purpose one and this VTL transition function makes sure to preserve the
> bp register due to which the objtool trips over on the assembly touching
> the bp register. We considered this warning harmless and thus we are
> using this macro. Moreover NMIs are not an issue here as they won't be
> coming as mentioned other. If there are alternate approaches that I should
> be using, please suggest.
Using BP in an ABI like that is ridiculous and broken. We told the same
to the TDX folks when they tried, IIRC TDX was fixed.
It is simply not acceptable to break the regular calling convention with
a new ABI.
Again, I can put a breakpoint or kprobe in the region where BP is
scribbled.
Basically the argument is really simple: you run in Linux, you play by
the Linux rules. Using BP as argument is simply not possible. If your
ABI requires that, your ABI is broken and will not be supported. Rev the
ABI and try again. Same for CR2, that is not an available register in
any way.
> I now understand that as part of your effort to enable IBT config on
> x64, you changed the indirect calls to direct calls in Hyper-V.
Yeah, I was cleaning up indirect calls, and this really didn't need to
be one.
> As of today, there is no requirement to enable IBT in OpenHCL kernel
> as this runs as a paravisor in VTL2 and it does not effect the guest
> VMs running
> in VTL0.
I do not know what OpenHCL or VTLn means and as such pretty much the
whole of your statement makes no sense.
Anyway, AFAICT the whole idea of a hypercall page is to 'abtract' out
the VMCALL vs VMMCALL nonsense Intel/AMD inflicted on us. Surely you
don't actually need that. HyperV already knows about all the gazillion
of ways to do hypercalls.
> I can disable CONFIG_X86_KERNEL_IBT when CONFIG_MSHV_VTL is enabled in
> Kconfig in next version.
Or you can just straight up say: "We at microsoft don't care about
security." :-/
Doing that will ensure no distro will build your module, most build bots
will not build your module, nobody cares about your module.
And no, the problems with BP and CR2 are not related to IBT, that is
separate and no less broken. They violate the basic rules of x86_64.
^ permalink raw reply
* Re: [PATCH 0/5] Drivers: hv: Fix NEED_RESCHED_LAZY and use common APIs
From: Wei Liu @ 2025-08-26 16:58 UTC (permalink / raw)
To: Sean Christopherson
Cc: Nuno Das Neves, Marc Zyngier, Oliver Upton, Catalin Marinas,
Will Deacon, Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Peter Zijlstra, Andy Lutomirski, Paul E. McKenney,
Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes,
Josh Triplett, Boqun Feng, Uladzislau Rezki, linux-kernel,
linux-arm-kernel, kvmarm, kvm, loongarch, kvm-riscv, linux-riscv,
linux-hyperv, rcu
In-Reply-To: <aKz_ZMvvF0e9nwSn@google.com>
On Mon, Aug 25, 2025 at 05:27:16PM -0700, Sean Christopherson wrote:
> On Mon, Aug 25, 2025, Nuno Das Neves wrote:
> > On 8/25/2025 1:06 PM, Sean Christopherson wrote:
> > > Fix a bug where MSHV root partitions don't honor NEED_RESCHED_LAZY, and then
> > > deduplicate the TIF related MSHV code by turning the "kvm" entry APIs into
> > > more generic "virt" APIs (which ideally would have been done when MSHV root
> > > support was added).
> > >
> > > Assuming all is well, maybe this could go through the tip tree?
> > >
> > > The Hyper-V stuff and non-x86 architectures are compile-tested only.
> > >
> >
> > Thanks Sean, I can test the root partition changes.
> >
> > A similar change will be needed in mshv_vtl_main.c since it also calls
> > mshv_do_pre_guest_mode_work() (hence the "common" in mshv_common.c).
>
> Oof, more dependencies. I suppose the easiest thing would be to send a series
> against
>
> git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git queue
>
> and then route everything through there?
Our fixes branch is on 6.17-rc1. You can use it as a base if you want
to.
>
> Alternatively, frontload the MSHV fixes (which I'll do regardless) and take those
> through hyperv and the rest through the tip tree? That seems like an absurd
> amount of juggling though, especially if we want to get the cleanups into 6.18.
> And if none of these lands, it's MSHV that'll suffer the most, so betting it all
> on the hyperv tree doesn't seem terrible.
>
I'm happy to do it however the community sees fit.
> > Also, is it possible to make all the mshv driver changes in a single patch?
>
> It's certainly possible, but I'd prefer not do to that.
>
> > It seems like it would be cleaner than refactoring it in patches 1 & 2 and
> > then deleting all the refactored code in patch 5.
>
> Only if you don't care about backporting fixes, bisection, or maintaining code.
>
> E.g. if checking NEED_RESCHED_LAZY somehow causes issues, it would be really nice
> for that to bisect to exactly that patch, not a patch that also switches to a
> completely different set of APIs.
>
> And if someone is wants the fixes in a pre-6.18 kernel, they don't need to
> backport all of the KVM and entry code changes just to get the fix.
+1 on this.
Thanks,
Wei
>
> As for the maintenance headache, see above.
^ permalink raw reply
* Re: [PATCH] x86/hyperv: Export hv_hypercall_pg unconditionally
From: Roman Kisel @ 2025-08-26 23:04 UTC (permalink / raw)
To: Peter Zijlstra, Naman Jain
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H . Peter Anvin, linux-hyperv, linux-kernel, mhklinux
In-Reply-To: <20250826120752.GW4067720@noisy.programming.kicks-ass.net>
On 8/26/2025 5:07 AM, Peter Zijlstra wrote:
> On Tue, Aug 26, 2025 at 05:00:31PM +0530, Naman Jain wrote:
Peter,
Naman is OOF; genuinely hoping it's not presumptuous of me to reply
to the thread - seems time sensitive.
[...]
>
> I do not know what OpenHCL is. Nor is it clear from the code what NMIs
> can't happen. Anyway, same can be achieved with breakpoints / kprobes.
> You can get a trap after setting CR2 and scribble it.
>
> You simply cannot use CR2 this way.
>
The code in question runs with interrupts disabled, and the kernel runs
without the memory swapping when using that module - the kernel is
a firmware to host a vTPM for virtual machines. Somewhat similar to SMM.
That should've been reflected somewhere in the comments and in Kconfig,
we could do better. All in all, the page fault cannot happen in that
path thus CR2 won't be trashed.
Nor this kind of code can be stepped through in a self-hosted
kernel debugger like kgdb. There are other examples of such code iiuc:
the asm glue code in the interrupt exception entries where the trap
frames are handled, likely return from the kernel to the user land.
The thread context-swap code hardly can be stepped through with
convenience if at all in the self-hosted debugger, too.
>>> And an rax:rcx return, I though the canonical pair was AX,DX !?!?
>>
>> Here, the code uses rax and rcx not as a means to return one 128 bit
>> value. The code uses them in that way as an ABI.
>
> Still daft. Creating an ABI that goes against pre-existing conventions
> is weird.
>
It is weird. It really sucks to have to conform to ABIs introduced a
decade+ ago when Hyper-V just appeared in the kernel and just for x86.
As weird as the pair ax:cx looks for anyone who takes joy and pride in
writing x86 asm code, it still works for the customers, we have to care
about the backward compat.
From the discussion, it doesn't appear we can ask for much as you're
right: the asm chunk looks abominable due to being essentially a
context-swap with an entity foreign to the Linux/System-V ABI. Same
must be true for the calls into the UEFI runtime services to a certain
extent due to different calling conventions.
We have a much cleaner story on ARM64 due to no legacy and using
their calling conventions aka AAPCS64 and other standards everywhere
(not only in Linux Hyper-V code) to the best of my knowledge.
If nothing of that saves the patches from the death row, maybe it'd be
possible to give the patches the experimental status or get some time
extension to learn what can be improved? I am asking to save the
time spent by folks reviewing the parts that you don't see as being
prohibitively bad.
>>> Also, that STACK_FRAME_NON_STANDARD() annotation is broken, this must
>>> not be used for anything that can end up in vmlinux.o -- that is, the
>>> moment you built-in this driver (=y) this comes unstuck.
>>>
>>> The reason you're getting warnings is because you're violating the
>>> normal calling convention and scribbling BP, we yelled at the TDX guys
>>> for doing this, now you're getting yelled at. WTF !?!
>>>
>>> Please explain how just shutting up objtool makes the unwind work when
>>> the NMI hits your BP scribble?
>>
>> Returning to a lower VTL treats the base pointer register as a general
>> purpose one and this VTL transition function makes sure to preserve the
>> bp register due to which the objtool trips over on the assembly touching
>> the bp register. We considered this warning harmless and thus we are
>> using this macro. Moreover NMIs are not an issue here as they won't be
>> coming as mentioned other. If there are alternate approaches that I should
>> be using, please suggest.
>
> Using BP in an ABI like that is ridiculous and broken. We told the same
> to the TDX folks when they tried, IIRC TDX was fixed.
>
> It is simply not acceptable to break the regular calling convention with
> a new ABI.
>
> Again, I can put a breakpoint or kprobe in the region where BP is
> scribbled.
>
> Basically the argument is really simple: you run in Linux, you play by
> the Linux rules. Using BP as argument is simply not possible. If your
> ABI requires that, your ABI is broken and will not be supported. Rev the
> ABI and try again. Same for CR2, that is not an available register in
> any way.
>
>> I now understand that as part of your effort to enable IBT config on
>> x64, you changed the indirect calls to direct calls in Hyper-V.
>
> Yeah, I was cleaning up indirect calls, and this really didn't need to
> be one.
>
>> As of today, there is no requirement to enable IBT in OpenHCL kernel
>> as this runs as a paravisor in VTL2 and it does not effect the guest
>> VMs running
>> in VTL0.
>
> I do not know what OpenHCL or VTLn means and as such pretty much the
> whole of your statement makes no sense.
>
> Anyway, AFAICT the whole idea of a hypercall page is to 'abtract' out
> the VMCALL vs VMMCALL nonsense Intel/AMD inflicted on us. Surely you
> don't actually need that. HyperV already knows about all the gazillion
> of ways to do hypercalls.
>
>> I can disable CONFIG_X86_KERNEL_IBT when CONFIG_MSHV_VTL is enabled in
>> Kconfig in next version.
>
> Or you can just straight up say: "We at microsoft don't care about
> security." :-/
>
> Doing that will ensure no distro will build your module, most build bots
> will not build your module, nobody cares about your module.
>
> And no, the problems with BP and CR2 are not related to IBT, that is
> separate and no less broken. They violate the basic rules of x86_64.
--
Thank you,
Roman
^ permalink raw reply
* RE: [PATCH v3] x86/hyperv: Fix kdump on Azure CVMs
From: Michael Kelley @ 2025-08-27 0:11 UTC (permalink / raw)
To: Vitaly Kuznetsov, linux-hyperv@vger.kernel.org
Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
x86@kernel.org, linux-kernel@vger.kernel.org, Nuno Das Neves,
Tianyu Lan, Li Tian, Philipp Rudo
In-Reply-To: <20250821151655.3051386-1-vkuznets@redhat.com>
From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Thursday, August 21, 2025 8:17 AM
>
> Azure CVM instance types featuring a paravisor hang upon kdump. The
> investigation shows that makedumpfile causes a hang when it steps on a page
> which was previously share with the host
> (HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY). The new kernel has no
> knowledge of these 'special' regions (which are Vmbus connection pages,
> GPADL buffers, ...). There are several ways to approach the issue:
> - Convey the knowledge about these regions to the new kernel somehow.
> - Unshare these regions before accessing in the new kernel (it is unclear
> if there's a way to query the status for a given GPA range).
> - Unshare these regions before jumping to the new kernel (which this patch
> implements).
>
> To make the procedure as robust as possible, store PFN ranges of shared
> regions in a linked list instead of storing GVAs and re-using
> hv_vtom_set_host_visibility(). This also allows to avoid memory allocation
> on the kdump/kexec path.
>
> The patch skips implementing weird corner case in hv_list_enc_remove()
> when a PFN in the middle of a region is unshared. First, it is unlikely
> that such requests happen. Second, it is not a big problem if
> hv_list_enc_remove() doesn't actually remove some regions as this will
> only result in an extra hypercall doing nothing upon kexec/kdump; there's
> no need to be perfect.
This last paragraph is left over from the previous version. It's no
longer correct and should be removed.
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> Changes since v2 [Michael Kelley]:
> - Rebase to hyperv-next.
> - Move hv_ivm_clear_host_access() call to hyperv_cleanup(). This also
> makes ARM stub unneeded.
> - Implement the missing corner case in hv_list_enc_remove(). With this,
> the math should (hopefully!) always be correct so we don't rely on
> the idempotency of HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY
> hypercall. As the case is not something we see, I tested the code
> with a few synthetic tests.
> - Fix the math in hv_list_enc_remove() (count -> ent->count).
> - Typos.
> ---
> arch/x86/hyperv/hv_init.c | 3 +
> arch/x86/hyperv/ivm.c | 213 ++++++++++++++++++++++++++++++--
> arch/x86/include/asm/mshyperv.h | 2 +
> 3 files changed, 210 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 2979d15223cf..4bb1578237eb 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -596,6 +596,9 @@ void hyperv_cleanup(void)
> union hv_x64_msr_hypercall_contents hypercall_msr;
> union hv_reference_tsc_msr tsc_msr;
>
> + /* Retract host access to shared memory in case of isolation */
> + hv_ivm_clear_host_access();
> +
> /* Reset our OS id */
> wrmsrq(HV_X64_MSR_GUEST_OS_ID, 0);
> hv_ivm_msr_write(HV_X64_MSR_GUEST_OS_ID, 0);
> diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
> index 3084ae8a3eed..0d74156ad6a7 100644
> --- a/arch/x86/hyperv/ivm.c
> +++ b/arch/x86/hyperv/ivm.c
> @@ -462,6 +462,188 @@ void hv_ivm_msr_read(u64 msr, u64 *value)
> hv_ghcb_msr_read(msr, value);
> }
>
> +/*
> + * Keep track of the PFN regions which were shared with the host. The access
> + * must be revoked upon kexec/kdump (see hv_ivm_clear_host_access()).
> + */
> +struct hv_enc_pfn_region {
> + struct list_head list;
> + u64 pfn;
> + int count;
> +};
> +
> +static LIST_HEAD(hv_list_enc);
> +static DEFINE_RAW_SPINLOCK(hv_list_enc_lock);
> +
> +static int hv_list_enc_add(const u64 *pfn_list, int count)
> +{
> + struct hv_enc_pfn_region *ent;
> + unsigned long flags;
> + u64 pfn;
> + int i;
> +
> + for (i = 0; i < count; i++) {
> + pfn = pfn_list[i];
> +
> + raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
> + /* Check if the PFN already exists in some region first */
> + list_for_each_entry(ent, &hv_list_enc, list) {
> + if ((ent->pfn <= pfn) && (ent->pfn + ent->count - 1 >= pfn))
> + /* Nothing to do - pfn is already in the list */
> + goto unlock_done;
> + }
> +
> + /*
> + * Check if the PFN is adjacent to an existing region. Growing
> + * a region can make it adjacent to another one but merging is
> + * not (yet) implemented for simplicity. A PFN cannot be added
> + * to two regions to keep the logic in hv_list_enc_remove()
> + * correct.
> + */
> + list_for_each_entry(ent, &hv_list_enc, list) {
> + if (ent->pfn + ent->count == pfn) {
> + /* Grow existing region up */
> + ent->count++;
> + goto unlock_done;
> + } else if (pfn + 1 == ent->pfn) {
> + /* Grow existing region down */
> + ent->pfn--;
> + ent->count++;
> + goto unlock_done;
> + }
> + }
> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
> +
> + /* No adjacent region found -- create a new one */
> + ent = kzalloc(sizeof(struct hv_enc_pfn_region), GFP_KERNEL);
> + if (!ent)
> + return -ENOMEM;
> +
> + ent->pfn = pfn;
> + ent->count = 1;
> +
> + raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
> + list_add(&ent->list, &hv_list_enc);
> +
> +unlock_done:
> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
> + }
> +
> + return 0;
> +}
> +
> +static void hv_list_enc_remove(const u64 *pfn_list, int count)
> +{
> + struct hv_enc_pfn_region *ent, *t;
> + struct hv_enc_pfn_region new_region;
> + unsigned long flags;
> + u64 pfn;
> + int i;
> +
> + for (i = 0; i < count; i++) {
> + pfn = pfn_list[i];
> +
> + raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
> + list_for_each_entry_safe(ent, t, &hv_list_enc, list) {
> + if (pfn == ent->pfn + ent->count - 1) {
> + /* Removing tail pfn */
> + ent->count--;
> + if (!ent->count) {
> + list_del(&ent->list);
> + kfree(ent);
> + }
> + goto unlock_done;
> + } else if (pfn == ent->pfn) {
> + /* Removing head pfn */
> + ent->count--;
> + ent->pfn++;
> + if (!ent->count) {
> + list_del(&ent->list);
> + kfree(ent);
> + }
> + goto unlock_done;
> + } else if (pfn > ent->pfn && pfn < ent->pfn + ent->count - 1) {
> + /*
> + * Removing a pfn in the middle. Cut off the tail
> + * of the existing region and create a template for
> + * the new one.
> + */
> + new_region.pfn = pfn + 1;
> + new_region.count = ent->count - (pfn - ent->pfn + 1);
> + ent->count = pfn - ent->pfn;
> + goto unlock_split;
> + }
> +
> + }
> +unlock_done:
> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
> + continue;
> +
> +unlock_split:
> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
> +
> + ent = kzalloc(sizeof(struct hv_enc_pfn_region), GFP_KERNEL);
> + /*
> + * There is no apparent good way to recover from -ENOMEM
> + * situation, the accouting is going to be wrong either way.
> + * Proceed with the rest of the list to make it 'less wrong'.
> + */
> + if (WARN_ON_ONCE(!ent))
> + continue;
> +
> + ent->pfn = new_region.pfn;
> + ent->count = new_region.count;
> +
> + raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
> + list_add(&ent->list, &hv_list_enc);
> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
> + }
> +}
> +
> +void hv_ivm_clear_host_access(void)
> +{
> + struct hv_gpa_range_for_visibility *input;
> + struct hv_enc_pfn_region *ent;
> + unsigned long flags;
> + u64 hv_status;
> + int batch_size, cur, i;
> +
> + if (!hv_is_isolation_supported())
> + return;
I seem to recall that some separate work has been done
to support kexec/kdump for the more generic SEV-SNP and
TDX cases where there's no paravisor mediating. I haven't
gone looking for that code to see when it runs.
hv_ivm_clear_host_access() is needed to update the
paravisor records about the page state, but if other code
has already updated the hypervisor/processor state, that
might be problematic. If this runs first, then the more
generic code will presumably find nothing to do, which
should be OK.
I'll try to go look further at this situation, unless you already
have. If necessary, this function could be gated to run
only when a paravisor is present.
> +
> + raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
Since this function is now called after other CPUs have
been stopped, the spin lock is no longer necessary, unless
you were counting on it to provide the interrupt disable
needed for accessing the per-cpu hypercall argument page.
But even then, I'd suggest just doing the interrupt disable
instead of the spin lock so there's no chance of the
panic or kexec path getting hung waiting on the spin lock.
There's also a potentially rare problem if other CPUs are
stopped while hv_list_enc_add() or hv_list_nec_remove()
is being executed. The list might be inconsistent, or not
fully reflect what the paravisor and hypervisor think about
the private/shared state of the page. But I don't think there's
anything we can do about that. Again, I'd suggest a code
comment acknowledging this case, and that there's nothing
that can be done.
> +
> + batch_size = MIN(hv_setup_in_array(&input, sizeof(*input),
> + sizeof(input->gpa_page_list[0])),
> + HV_MAX_MODIFY_GPA_REP_COUNT);
The patches that added hv_setup_in_array() were pulled from
hyperv-next due to some renewed discussion. You'll need to revert
back to the previous handling of hyperv_pcpu_input_arg. :-(
> + if (unlikely(!input))
> + goto unlock;
> +
> + list_for_each_entry(ent, &hv_list_enc, list) {
> + for (i = 0, cur = 0; i < ent->count; i++) {
> + input->gpa_page_list[cur] = ent->pfn + i;
> + cur++;
> +
> + if (cur == batch_size || i == ent->count - 1) {
> + input->partition_id = HV_PARTITION_ID_SELF;
> + input->host_visibility = VMBUS_PAGE_NOT_VISIBLE;
> + input->reserved0 = 0;
> + input->reserved1 = 0;
> + hv_status = hv_do_rep_hypercall(
> + HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY,
> + cur, 0, input, NULL);
> + WARN_ON_ONCE(!hv_result_success(hv_status));
> + cur = 0;
> + }
> + }
> +
> + };
> +
> +unlock:
> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(hv_ivm_clear_host_access);
> +
> /*
> * hv_mark_gpa_visibility - Set pages visible to host via hvcall.
> *
> @@ -476,24 +658,33 @@ static int hv_mark_gpa_visibility(u16 count, const u64 pfn[],
> u64 hv_status;
> int batch_size;
> unsigned long flags;
> + int ret;
>
> /* no-op if partition isolation is not enabled */
> if (!hv_is_isolation_supported())
> return 0;
>
> + if (visibility == VMBUS_PAGE_NOT_VISIBLE) {
> + hv_list_enc_remove(pfn, count);
> + } else {
> + ret = hv_list_enc_add(pfn, count);
> + if (ret)
> + return ret;
> + }
> +
> local_irq_save(flags);
> batch_size = hv_setup_in_array(&input, sizeof(*input),
> sizeof(input->gpa_page_list[0]));
> if (unlikely(!input)) {
> - local_irq_restore(flags);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto unlock;
> }
>
> if (count > batch_size) {
> pr_err("Hyper-V: GPA count:%d exceeds supported:%u\n", count,
> batch_size);
> - local_irq_restore(flags);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto unlock;
> }
>
> input->partition_id = HV_PARTITION_ID_SELF;
> @@ -502,12 +693,18 @@ static int hv_mark_gpa_visibility(u16 count, const u64 pfn[],
> hv_status = hv_do_rep_hypercall(
> HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY, count,
> 0, input, NULL);
> - local_irq_restore(flags);
> -
> if (hv_result_success(hv_status))
> - return 0;
> + ret = 0;
> else
> - return -EFAULT;
> + ret = -EFAULT;
> +
> +unlock:
> + local_irq_restore(flags);
> +
> + if (ret)
> + hv_list_enc_remove(pfn, count);
If making the pages shared fails, this is an "undo". But what
about an "undo" if making the pages private fails? Maybe
your thinking is that leaving the pages on the shared list just
sets things up for a failure when hv_ivm_clear_host_access()
tries to make them private. But I wonder if it would be better
to go ahead and "undo" here in case the failure is transient
and hv_ivm_clear_host_access() might later succeed. I don't
understand the hypercall failure modes well enough to
know whether that's plausible. But if you think the "undo"
here really isn't warranted, please add a code comment to
that effect since a future reader might expect to the two
cases to be symmetrical here.
> +
> + return ret;
> }
>
> /*
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index abc4659f5809..6a988001e46f 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -263,10 +263,12 @@ static inline int hv_snp_boot_ap(u32 apic_id, unsigned long
> start_ip,
> void hv_vtom_init(void);
> void hv_ivm_msr_write(u64 msr, u64 value);
> void hv_ivm_msr_read(u64 msr, u64 *value);
> +void hv_ivm_clear_host_access(void);
> #else
> static inline void hv_vtom_init(void) {}
> static inline void hv_ivm_msr_write(u64 msr, u64 value) {}
> static inline void hv_ivm_msr_read(u64 msr, u64 *value) {}
> +static inline void hv_ivm_clear_host_access(void) {}
> #endif
>
> static inline bool hv_is_synic_msr(unsigned int reg)
> --
> 2.50.1
^ permalink raw reply
* Re: [PATCH net] net: hv_netvsc: fix loss of early receive events from host during channel open.
From: patchwork-bot+netdevbpf @ 2025-08-27 1:30 UTC (permalink / raw)
To: Dipayaan Roy
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, linux-hyperv, netdev, linux-kernel, dipayanroy,
ssengar
In-Reply-To: <20250825115627.GA32189@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Mon, 25 Aug 2025 04:56:27 -0700 you wrote:
> The hv_netvsc driver currently enables NAPI after opening the primary and
> subchannels. This ordering creates a race: if the Hyper-V host places data
> in the host -> guest ring buffer and signals the channel before
> napi_enable() has been called, the channel callback will run but
> napi_schedule_prep() will return false. As a result, the NAPI poller never
> gets scheduled, the data in the ring buffer is not consumed, and the
> receive queue may remain permanently stuck until another interrupt happens
> to arrive.
>
> [...]
Here is the summary with links:
- [net] net: hv_netvsc: fix loss of early receive events from host during channel open.
https://git.kernel.org/netdev/net/c/9448ccd85336
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* RE: [PATCH v3] x86/hyperv: Fix kdump on Azure CVMs
From: Vitaly Kuznetsov @ 2025-08-27 12:50 UTC (permalink / raw)
To: Michael Kelley, linux-hyperv@vger.kernel.org
Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
x86@kernel.org, linux-kernel@vger.kernel.org, Nuno Das Neves,
Tianyu Lan, Li Tian, Philipp Rudo
In-Reply-To: <SN6PR02MB4157581777244FE17DA3C7C2D438A@SN6PR02MB4157.namprd02.prod.outlook.com>
Michael Kelley <mhklinux@outlook.com> writes:
> From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Thursday, August 21, 2025 8:17 AM
>>
>> Azure CVM instance types featuring a paravisor hang upon kdump. The
>> investigation shows that makedumpfile causes a hang when it steps on a page
>> which was previously share with the host
>> (HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY). The new kernel has no
>> knowledge of these 'special' regions (which are Vmbus connection pages,
>> GPADL buffers, ...). There are several ways to approach the issue:
>> - Convey the knowledge about these regions to the new kernel somehow.
>> - Unshare these regions before accessing in the new kernel (it is unclear
>> if there's a way to query the status for a given GPA range).
>> - Unshare these regions before jumping to the new kernel (which this patch
>> implements).
>>
>> To make the procedure as robust as possible, store PFN ranges of shared
>> regions in a linked list instead of storing GVAs and re-using
>> hv_vtom_set_host_visibility(). This also allows to avoid memory allocation
>> on the kdump/kexec path.
>>
>> The patch skips implementing weird corner case in hv_list_enc_remove()
>> when a PFN in the middle of a region is unshared. First, it is unlikely
>> that such requests happen. Second, it is not a big problem if
>> hv_list_enc_remove() doesn't actually remove some regions as this will
>> only result in an extra hypercall doing nothing upon kexec/kdump; there's
>> no need to be perfect.
>
> This last paragraph is left over from the previous version. It's no
> longer correct and should be removed.
>
Indeed, will drop!
>>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>> Changes since v2 [Michael Kelley]:
>> - Rebase to hyperv-next.
>> - Move hv_ivm_clear_host_access() call to hyperv_cleanup(). This also
>> makes ARM stub unneeded.
>> - Implement the missing corner case in hv_list_enc_remove(). With this,
>> the math should (hopefully!) always be correct so we don't rely on
>> the idempotency of HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY
>> hypercall. As the case is not something we see, I tested the code
>> with a few synthetic tests.
>> - Fix the math in hv_list_enc_remove() (count -> ent->count).
>> - Typos.
>> ---
>> arch/x86/hyperv/hv_init.c | 3 +
>> arch/x86/hyperv/ivm.c | 213 ++++++++++++++++++++++++++++++--
>> arch/x86/include/asm/mshyperv.h | 2 +
>> 3 files changed, 210 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
>> index 2979d15223cf..4bb1578237eb 100644
>> --- a/arch/x86/hyperv/hv_init.c
>> +++ b/arch/x86/hyperv/hv_init.c
>> @@ -596,6 +596,9 @@ void hyperv_cleanup(void)
>> union hv_x64_msr_hypercall_contents hypercall_msr;
>> union hv_reference_tsc_msr tsc_msr;
>>
>> + /* Retract host access to shared memory in case of isolation */
>> + hv_ivm_clear_host_access();
>> +
>> /* Reset our OS id */
>> wrmsrq(HV_X64_MSR_GUEST_OS_ID, 0);
>> hv_ivm_msr_write(HV_X64_MSR_GUEST_OS_ID, 0);
>> diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
>> index 3084ae8a3eed..0d74156ad6a7 100644
>> --- a/arch/x86/hyperv/ivm.c
>> +++ b/arch/x86/hyperv/ivm.c
>> @@ -462,6 +462,188 @@ void hv_ivm_msr_read(u64 msr, u64 *value)
>> hv_ghcb_msr_read(msr, value);
>> }
>>
>> +/*
>> + * Keep track of the PFN regions which were shared with the host. The access
>> + * must be revoked upon kexec/kdump (see hv_ivm_clear_host_access()).
>> + */
>> +struct hv_enc_pfn_region {
>> + struct list_head list;
>> + u64 pfn;
>> + int count;
>> +};
>> +
>> +static LIST_HEAD(hv_list_enc);
>> +static DEFINE_RAW_SPINLOCK(hv_list_enc_lock);
>> +
>> +static int hv_list_enc_add(const u64 *pfn_list, int count)
>> +{
>> + struct hv_enc_pfn_region *ent;
>> + unsigned long flags;
>> + u64 pfn;
>> + int i;
>> +
>> + for (i = 0; i < count; i++) {
>> + pfn = pfn_list[i];
>> +
>> + raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
>> + /* Check if the PFN already exists in some region first */
>> + list_for_each_entry(ent, &hv_list_enc, list) {
>> + if ((ent->pfn <= pfn) && (ent->pfn + ent->count - 1 >= pfn))
>> + /* Nothing to do - pfn is already in the list */
>> + goto unlock_done;
>> + }
>> +
>> + /*
>> + * Check if the PFN is adjacent to an existing region. Growing
>> + * a region can make it adjacent to another one but merging is
>> + * not (yet) implemented for simplicity. A PFN cannot be added
>> + * to two regions to keep the logic in hv_list_enc_remove()
>> + * correct.
>> + */
>> + list_for_each_entry(ent, &hv_list_enc, list) {
>> + if (ent->pfn + ent->count == pfn) {
>> + /* Grow existing region up */
>> + ent->count++;
>> + goto unlock_done;
>> + } else if (pfn + 1 == ent->pfn) {
>> + /* Grow existing region down */
>> + ent->pfn--;
>> + ent->count++;
>> + goto unlock_done;
>> + }
>> + }
>> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
>> +
>> + /* No adjacent region found -- create a new one */
>> + ent = kzalloc(sizeof(struct hv_enc_pfn_region), GFP_KERNEL);
>> + if (!ent)
>> + return -ENOMEM;
>> +
>> + ent->pfn = pfn;
>> + ent->count = 1;
>> +
>> + raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
>> + list_add(&ent->list, &hv_list_enc);
>> +
>> +unlock_done:
>> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void hv_list_enc_remove(const u64 *pfn_list, int count)
>> +{
>> + struct hv_enc_pfn_region *ent, *t;
>> + struct hv_enc_pfn_region new_region;
>> + unsigned long flags;
>> + u64 pfn;
>> + int i;
>> +
>> + for (i = 0; i < count; i++) {
>> + pfn = pfn_list[i];
>> +
>> + raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
>> + list_for_each_entry_safe(ent, t, &hv_list_enc, list) {
>> + if (pfn == ent->pfn + ent->count - 1) {
>> + /* Removing tail pfn */
>> + ent->count--;
>> + if (!ent->count) {
>> + list_del(&ent->list);
>> + kfree(ent);
>> + }
>> + goto unlock_done;
>> + } else if (pfn == ent->pfn) {
>> + /* Removing head pfn */
>> + ent->count--;
>> + ent->pfn++;
>> + if (!ent->count) {
>> + list_del(&ent->list);
>> + kfree(ent);
>> + }
>> + goto unlock_done;
>> + } else if (pfn > ent->pfn && pfn < ent->pfn + ent->count - 1) {
>> + /*
>> + * Removing a pfn in the middle. Cut off the tail
>> + * of the existing region and create a template for
>> + * the new one.
>> + */
>> + new_region.pfn = pfn + 1;
>> + new_region.count = ent->count - (pfn - ent->pfn + 1);
>> + ent->count = pfn - ent->pfn;
>> + goto unlock_split;
>> + }
>> +
>> + }
>> +unlock_done:
>> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
>> + continue;
>> +
>> +unlock_split:
>> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
>> +
>> + ent = kzalloc(sizeof(struct hv_enc_pfn_region), GFP_KERNEL);
>> + /*
>> + * There is no apparent good way to recover from -ENOMEM
>> + * situation, the accouting is going to be wrong either way.
>> + * Proceed with the rest of the list to make it 'less wrong'.
>> + */
>> + if (WARN_ON_ONCE(!ent))
>> + continue;
>> +
>> + ent->pfn = new_region.pfn;
>> + ent->count = new_region.count;
>> +
>> + raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
>> + list_add(&ent->list, &hv_list_enc);
>> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
>> + }
>> +}
>> +
>> +void hv_ivm_clear_host_access(void)
>> +{
>> + struct hv_gpa_range_for_visibility *input;
>> + struct hv_enc_pfn_region *ent;
>> + unsigned long flags;
>> + u64 hv_status;
>> + int batch_size, cur, i;
>> +
>> + if (!hv_is_isolation_supported())
>> + return;
>
> I seem to recall that some separate work has been done
> to support kexec/kdump for the more generic SEV-SNP and
> TDX cases where there's no paravisor mediating. I haven't
> gone looking for that code to see when it runs.
> hv_ivm_clear_host_access() is needed to update the
> paravisor records about the page state, but if other code
> has already updated the hypervisor/processor state, that
> might be problematic. If this runs first, then the more
> generic code will presumably find nothing to do, which
> should be OK.
>
> I'll try to go look further at this situation, unless you already
> have. If necessary, this function could be gated to run
> only when a paravisor is present.
Yes, there are SEV-SNP and TDX specific
snp_kexec_finish()/tdx_kexec_finish() which do memory unsharing but I
convinced myself that these are not called on Azure CVM which uses
paravisor. In particular, for SEV-SNP 'sme_me_mask == 0' in
sme_early_init().
I have to admit I've never seen an Azure/Hyper-V CVM without a
paravisor, but I agree it may make sense to hide all this tracking and
cleanup logic under 'if (hyperv_paravisor_present)' (or even 'if
(hv_is_isolation_supported() && hyperv_paravisor_present)').
>
>> +
>> + raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
>
> Since this function is now called after other CPUs have
> been stopped, the spin lock is no longer necessary, unless
> you were counting on it to provide the interrupt disable
> needed for accessing the per-cpu hypercall argument page.
> But even then, I'd suggest just doing the interrupt disable
> instead of the spin lock so there's no chance of the
> panic or kexec path getting hung waiting on the spin lock.
Makes sense, will do.
>
> There's also a potentially rare problem if other CPUs are
> stopped while hv_list_enc_add() or hv_list_nec_remove()
> is being executed. The list might be inconsistent, or not
> fully reflect what the paravisor and hypervisor think about
> the private/shared state of the page. But I don't think there's
> anything we can do about that. Again, I'd suggest a code
> comment acknowledging this case, and that there's nothing
> that can be done.
True, will add a comment. Just like with a lot of other corner cases in
panic, it's hard to guarantee correctness in ALL cases as the system can
be in any state (e.g. if the panic is caused by memory corruption -- who
knows what's corrupted?). I'm hoping that with the newly added logic
we're covering the most common kdump case and it'll 'generally work' on
Azure CVMs.
>
>> +
>> + batch_size = MIN(hv_setup_in_array(&input, sizeof(*input),
>> + sizeof(input->gpa_page_list[0])),
>> + HV_MAX_MODIFY_GPA_REP_COUNT);
>
> The patches that added hv_setup_in_array() were pulled from
> hyperv-next due to some renewed discussion. You'll need to revert
> back to the previous handling of hyperv_pcpu_input_arg. :-(
>
No worries, it's not a big change for this patch)
>> + if (unlikely(!input))
>> + goto unlock;
>> +
>> + list_for_each_entry(ent, &hv_list_enc, list) {
>> + for (i = 0, cur = 0; i < ent->count; i++) {
>> + input->gpa_page_list[cur] = ent->pfn + i;
>> + cur++;
>> +
>> + if (cur == batch_size || i == ent->count - 1) {
>> + input->partition_id = HV_PARTITION_ID_SELF;
>> + input->host_visibility = VMBUS_PAGE_NOT_VISIBLE;
>> + input->reserved0 = 0;
>> + input->reserved1 = 0;
>> + hv_status = hv_do_rep_hypercall(
>> + HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY,
>> + cur, 0, input, NULL);
>> + WARN_ON_ONCE(!hv_result_success(hv_status));
>> + cur = 0;
>> + }
>> + }
>> +
>> + };
>> +
>> +unlock:
>> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
>> +}
>> +EXPORT_SYMBOL_GPL(hv_ivm_clear_host_access);
>> +
>> /*
>> * hv_mark_gpa_visibility - Set pages visible to host via hvcall.
>> *
>> @@ -476,24 +658,33 @@ static int hv_mark_gpa_visibility(u16 count, const u64 pfn[],
>> u64 hv_status;
>> int batch_size;
>> unsigned long flags;
>> + int ret;
>>
>> /* no-op if partition isolation is not enabled */
>> if (!hv_is_isolation_supported())
>> return 0;
>>
>> + if (visibility == VMBUS_PAGE_NOT_VISIBLE) {
>> + hv_list_enc_remove(pfn, count);
>> + } else {
>> + ret = hv_list_enc_add(pfn, count);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> local_irq_save(flags);
>> batch_size = hv_setup_in_array(&input, sizeof(*input),
>> sizeof(input->gpa_page_list[0]));
>> if (unlikely(!input)) {
>> - local_irq_restore(flags);
>> - return -EINVAL;
>> + ret = -EINVAL;
>> + goto unlock;
>> }
>>
>> if (count > batch_size) {
>> pr_err("Hyper-V: GPA count:%d exceeds supported:%u\n", count,
>> batch_size);
>> - local_irq_restore(flags);
>> - return -EINVAL;
>> + ret = -EINVAL;
>> + goto unlock;
>> }
>>
>> input->partition_id = HV_PARTITION_ID_SELF;
>> @@ -502,12 +693,18 @@ static int hv_mark_gpa_visibility(u16 count, const u64 pfn[],
>> hv_status = hv_do_rep_hypercall(
>> HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY, count,
>> 0, input, NULL);
>> - local_irq_restore(flags);
>> -
>> if (hv_result_success(hv_status))
>> - return 0;
>> + ret = 0;
>> else
>> - return -EFAULT;
>> + ret = -EFAULT;
>> +
>> +unlock:
>> + local_irq_restore(flags);
>> +
>> + if (ret)
>> + hv_list_enc_remove(pfn, count);
>
> If making the pages shared fails, this is an "undo". But what
> about an "undo" if making the pages private fails? Maybe
> your thinking is that leaving the pages on the shared list just
> sets things up for a failure when hv_ivm_clear_host_access()
> tries to make them private. But I wonder if it would be better
> to go ahead and "undo" here in case the failure is transient
> and hv_ivm_clear_host_access() might later succeed. I don't
> understand the hypercall failure modes well enough to
> know whether that's plausible. But if you think the "undo"
> here really isn't warranted, please add a code comment to
> that effect since a future reader might expect to the two
> cases to be symmetrical here.
Nice catch, I missed the fact that making pages private can fail too. As
the assumption we have is that the hypercall does 'all or nothing', it's
better to handle the case symmetrically. Will add the logic.
>
>> +
>> + return ret;
>> }
>>
>> /*
>> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
>> index abc4659f5809..6a988001e46f 100644
>> --- a/arch/x86/include/asm/mshyperv.h
>> +++ b/arch/x86/include/asm/mshyperv.h
>> @@ -263,10 +263,12 @@ static inline int hv_snp_boot_ap(u32 apic_id, unsigned long
>> start_ip,
>> void hv_vtom_init(void);
>> void hv_ivm_msr_write(u64 msr, u64 value);
>> void hv_ivm_msr_read(u64 msr, u64 *value);
>> +void hv_ivm_clear_host_access(void);
>> #else
>> static inline void hv_vtom_init(void) {}
>> static inline void hv_ivm_msr_write(u64 msr, u64 value) {}
>> static inline void hv_ivm_msr_read(u64 msr, u64 *value) {}
>> +static inline void hv_ivm_clear_host_access(void) {}
>> #endif
>>
>> static inline bool hv_is_synic_msr(unsigned int reg)
>> --
>> 2.50.1
>
--
Vitaly
^ permalink raw reply
* RE: [PATCH v3] x86/hyperv: Fix kdump on Azure CVMs
From: Michael Kelley @ 2025-08-27 16:01 UTC (permalink / raw)
To: Vitaly Kuznetsov, linux-hyperv@vger.kernel.org
Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
x86@kernel.org, linux-kernel@vger.kernel.org, Nuno Das Neves,
Tianyu Lan, Li Tian, Philipp Rudo
In-Reply-To: <878qj5dj2z.fsf@redhat.com>
From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Wednesday, August 27, 2025 5:51 AM
>
> Michael Kelley <mhklinux@outlook.com> writes:
>
> > From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Thursday, August 21, 2025 8:17 AM
> >>
[snip]
> >
> > I seem to recall that some separate work has been done
> > to support kexec/kdump for the more generic SEV-SNP and
> > TDX cases where there's no paravisor mediating. I haven't
> > gone looking for that code to see when it runs.
> > hv_ivm_clear_host_access() is needed to update the
> > paravisor records about the page state, but if other code
> > has already updated the hypervisor/processor state, that
> > might be problematic. If this runs first, then the more
> > generic code will presumably find nothing to do, which
> > should be OK.
> >
> > I'll try to go look further at this situation, unless you already
> > have. If necessary, this function could be gated to run
> > only when a paravisor is present.
>
> Yes, there are SEV-SNP and TDX specific
> snp_kexec_finish()/tdx_kexec_finish() which do memory unsharing but I
> convinced myself that these are not called on Azure CVM which uses
> paravisor. In particular, for SEV-SNP 'sme_me_mask == 0' in
> sme_early_init().
Thanks for the pointer to snp_kexec_finish() and tdx_kexec_finish().
Yes, I agree they won't get called in the Hyper-V paravisor case.
>
> I have to admit I've never seen an Azure/Hyper-V CVM without a
> paravisor, but I agree it may make sense to hide all this tracking and
> cleanup logic under 'if (hyperv_paravisor_present)' (or even 'if
> (hv_is_isolation_supported() && hyperv_paravisor_present)').
I think this patch should be plugged into the .enc_kexec_begin and
.enc_kexec_finish mechanism. By using .enc_kexec_begin similarly
to snp/tdx_kexec_begin(), synchronizing with in-progress private<->shared
conversions is done in the kexec() case, though not in the panic/kdump
case. That machinery is there, and the Hyper-V paravisor case can use it.
hv_ivm_clear_host_access() should be the .enc_kexec_finish function,
and it will get invoked at the right time without having to be
explicitly called in hyperv_cleanup().
Hyper-V *does* support running SNP and TDX in what Microsoft
internally calls the "fully enlightened" case, where the guest OS does
everything necessary to operate in SNP or TDX mode, without
depending on a paravisor. In such a case, sme_me_mask will be
non-zero in SNP mode, for example. But I'm unsure if Microsoft has kept
the Linux guest support on Hyper-V up-to-date enough for this to actually
work in practice. I thought at one point there was an internal use case
for SNP without a paravisor, but it's been nearly two years now
since I retired, and so I don't really know anymore. For running on
Hyper-V in TDX mode without a paravisor, I know there's one Linux
patch missing that is needed by the netvsc driver. That patch would
provide the TDX support for set_memory_encrypted/decrypted() to work
on a vmalloc area where the underlying physical memory is not
contiguous. SNP has the support, but it's a known gap for TDX.
If you wire up the .enc_kexec_begin and .enc_kexec_finish functions
in hv_vtom_init() along with the other x86_platform.guest.enc_*
functions, then you don't need to test for a paravisor being present
because hv_vtom_init() is called only when a paravisor is present.
The non-paravisor cases on Hyper-V will fall back to the existing
snp/tdx_kexec_being/finish() functions, and everything *should*
just work. If it doesn't just work, that's a different problem for
the Microsoft folks to look at if they care.
I will ping my contacts on the Microsoft side to see if the "no
paravisor" case is still of interest, and if so, whether someone
can test it. My only test environment is as a normal Azure user,
so like you, I don't have a way to do such a test.
>
> >
> >> +
> >> + raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
> >
> > Since this function is now called after other CPUs have
> > been stopped, the spin lock is no longer necessary, unless
> > you were counting on it to provide the interrupt disable
> > needed for accessing the per-cpu hypercall argument page.
> > But even then, I'd suggest just doing the interrupt disable
> > instead of the spin lock so there's no chance of the
> > panic or kexec path getting hung waiting on the spin lock.
>
> Makes sense, will do.
>
> >
> > There's also a potentially rare problem if other CPUs are
> > stopped while hv_list_enc_add() or hv_list_nec_remove()
> > is being executed. The list might be inconsistent, or not
> > fully reflect what the paravisor and hypervisor think about
> > the private/shared state of the page. But I don't think there's
> > anything we can do about that. Again, I'd suggest a code
> > comment acknowledging this case, and that there's nothing
> > that can be done.
>
> True, will add a comment. Just like with a lot of other corner cases in
> panic, it's hard to guarantee correctness in ALL cases as the system can
> be in any state (e.g. if the panic is caused by memory corruption -- who
> knows what's corrupted?). I'm hoping that with the newly added logic
> we're covering the most common kdump case and it'll 'generally work' on
> Azure CVMs.
As noted above, in a non-panic kexec(), the synchronization with
in-progress private<->shared conversions can be solved.
Michael
^ permalink raw reply
* [PATCH v2 0/7] Drivers: hv: Fix NEED_RESCHED_LAZY and use common APIs
From: Sean Christopherson @ 2025-08-28 0:01 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon,
Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Peter Zijlstra, Andy Lutomirski, Paul E. McKenney,
Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes,
Josh Triplett, Boqun Feng, Uladzislau Rezki
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, loongarch, kvm-riscv,
linux-riscv, linux-hyperv, rcu, Nuno Das Neves, Mukesh R
Fix a bug where MSHV root partitions (and upper-level VTL code) don't honor
NEED_RESCHED_LAZY, and then deduplicate the TIF related MSHV code by turning
the "kvm" entry APIs into more generic "virt" APIs.
This version is based on
git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git hyperv-next
in order to pickup the VTL changes that are queued for 6.18. I also
squashed the NEED_RESCHED_LAZY fixes for root and VTL modes into a single
patch, as it should be easy/straightforward to drop the VTL change as needed
if we want this in 6.17 or earlier.
That effectively means the full series is dependent on the VTL changes being
fully merged for 6.18. But I think that's ok as it's really only the MSHV
changes that have any urgency whatsoever, and I assume that Microsoft is
the only user that truly cares about the MSHV root fix. I.e. if the whole
thing gets delayed, I think it's only the Hyper-V folks that are impacted.
I have no preference what tree this goes through, or when, and can respin
and/or split as needed.
As with v1, the Hyper-V stuff and non-x86 architectures are compile-tested
only.
v2:
- Rebase on hyperv-next.
- Fix and converge the VTL code as well. [Peter, Nuno]
v1: https://lore.kernel.org/all/20250825200622.3759571-1-seanjc@google.com
Sean Christopherson (7):
Drivers: hv: Handle NEED_RESCHED_LAZY before transferring to guest
Drivers: hv: Disentangle VTL return cancellation from SIGPENDING
Drivers: hv: Disable IRQs only after handling pending work before VTL
return
entry/kvm: KVM: Move KVM details related to signal/-EINTR into KVM
proper
entry: Rename "kvm" entry code assets to "virt" to genericize APIs
Drivers: hv: Use common "entry virt" APIs to do work in root before
running guest
Drivers: hv: Use "entry virt" APIs to do work before returning to
lower VTL
MAINTAINERS | 2 +-
arch/arm64/kvm/Kconfig | 2 +-
arch/arm64/kvm/arm.c | 3 +-
arch/loongarch/kvm/Kconfig | 2 +-
arch/loongarch/kvm/vcpu.c | 3 +-
arch/riscv/kvm/Kconfig | 2 +-
arch/riscv/kvm/vcpu.c | 3 +-
arch/x86/kvm/Kconfig | 2 +-
arch/x86/kvm/vmx/vmx.c | 1 -
arch/x86/kvm/x86.c | 3 +-
drivers/hv/Kconfig | 2 ++
drivers/hv/mshv.h | 2 --
drivers/hv/mshv_common.c | 22 ---------------
drivers/hv/mshv_root_main.c | 31 ++++-----------------
drivers/hv/mshv_vtl_main.c | 23 +++++++--------
include/linux/{entry-kvm.h => entry-virt.h} | 19 +++++--------
include/linux/kvm_host.h | 17 +++++++++--
include/linux/rcupdate.h | 2 +-
kernel/entry/Makefile | 2 +-
kernel/entry/{kvm.c => virt.c} | 15 ++++------
kernel/rcu/tree.c | 6 ++--
virt/kvm/Kconfig | 2 +-
22 files changed, 60 insertions(+), 106 deletions(-)
rename include/linux/{entry-kvm.h => entry-virt.h} (83%)
rename kernel/entry/{kvm.c => virt.c} (66%)
base-commit: 03ac62a578566730ab3c320f289f7320798ee2e1
--
2.51.0.268.g9569e192d0-goog
^ permalink raw reply
* [PATCH v2 1/7] Drivers: hv: Handle NEED_RESCHED_LAZY before transferring to guest
From: Sean Christopherson @ 2025-08-28 0:01 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon,
Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Peter Zijlstra, Andy Lutomirski, Paul E. McKenney,
Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes,
Josh Triplett, Boqun Feng, Uladzislau Rezki
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, loongarch, kvm-riscv,
linux-riscv, linux-hyperv, rcu, Nuno Das Neves, Mukesh R
In-Reply-To: <20250828000156.23389-1-seanjc@google.com>
Check for NEED_RESCHED_LAZY, not just NEED_RESCHED, prior to transferring
control to a guest. Failure to check for lazy resched can unnecessarily
delay rescheduling until the next tick when using a lazy preemption model.
Note, ideally both the checking and processing of TIF bits would be handled
in common code, to avoid having to keep three separate paths synchronized,
but defer such cleanups to the future to keep the fix as standalone as
possible.
Cc: Nuno Das Neves <nunodasneves@linux.microsoft.com>
Cc: Mukesh R <mrathor@linux.microsoft.com>
Fixes: 621191d709b1 ("Drivers: hv: Introduce mshv_root module to expose /dev/mshv to VMMs")
Fixes: 64503b4f4468 ("Drivers: hv: Introduce mshv_vtl driver")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
drivers/hv/mshv_common.c | 2 +-
drivers/hv/mshv_root_main.c | 3 ++-
drivers/hv/mshv_vtl_main.c | 3 ++-
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/hv/mshv_common.c b/drivers/hv/mshv_common.c
index 6f227a8a5af7..eb3df3e296bb 100644
--- a/drivers/hv/mshv_common.c
+++ b/drivers/hv/mshv_common.c
@@ -151,7 +151,7 @@ int mshv_do_pre_guest_mode_work(ulong th_flags)
if (th_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
return -EINTR;
- if (th_flags & _TIF_NEED_RESCHED)
+ if (th_flags & (_TIF_NEED_RESCHED | _TIF_NEED_RESCHED_LAZY))
schedule();
if (th_flags & _TIF_NOTIFY_RESUME)
diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index 932932cb91ea..0d849f09160a 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -484,7 +484,8 @@ mshv_vp_wait_for_hv_kick(struct mshv_vp *vp)
static int mshv_pre_guest_mode_work(struct mshv_vp *vp)
{
const ulong work_flags = _TIF_NOTIFY_SIGNAL | _TIF_SIGPENDING |
- _TIF_NEED_RESCHED | _TIF_NOTIFY_RESUME;
+ _TIF_NEED_RESCHED | _TIF_NEED_RESCHED_LAZY |
+ _TIF_NOTIFY_RESUME;
ulong th_flags;
th_flags = read_thread_flags();
diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
index dc6594ae03ad..12f5e77b7095 100644
--- a/drivers/hv/mshv_vtl_main.c
+++ b/drivers/hv/mshv_vtl_main.c
@@ -728,7 +728,8 @@ static int mshv_vtl_ioctl_return_to_lower_vtl(void)
preempt_disable();
for (;;) {
const unsigned long VTL0_WORK = _TIF_SIGPENDING | _TIF_NEED_RESCHED |
- _TIF_NOTIFY_RESUME | _TIF_NOTIFY_SIGNAL;
+ _TIF_NOTIFY_RESUME | _TIF_NOTIFY_SIGNAL |
+ _TIF_NEED_RESCHED_LAZY;
unsigned long ti_work;
u32 cancel;
unsigned long irq_flags;
--
2.51.0.268.g9569e192d0-goog
^ permalink raw reply related
* [PATCH v2 2/7] Drivers: hv: Disentangle VTL return cancellation from SIGPENDING
From: Sean Christopherson @ 2025-08-28 0:01 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon,
Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Peter Zijlstra, Andy Lutomirski, Paul E. McKenney,
Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes,
Josh Triplett, Boqun Feng, Uladzislau Rezki
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, loongarch, kvm-riscv,
linux-riscv, linux-hyperv, rcu, Nuno Das Neves, Mukesh R
In-Reply-To: <20250828000156.23389-1-seanjc@google.com>
Check for return to a lower VTL being cancelled separately from handling
pending TIF-based work, as there is no need to immediately process pending
work; the kernel will immediately exit to userspace (ignoring preemption)
and handle the pending work at that time.
Disentangling cancellation from the TIF-based work will allow switching to
common virtualization APIs for detecting and processing pending work.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
drivers/hv/mshv_vtl_main.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
index 12f5e77b7095..aa09a76f0eff 100644
--- a/drivers/hv/mshv_vtl_main.c
+++ b/drivers/hv/mshv_vtl_main.c
@@ -731,19 +731,21 @@ static int mshv_vtl_ioctl_return_to_lower_vtl(void)
_TIF_NOTIFY_RESUME | _TIF_NOTIFY_SIGNAL |
_TIF_NEED_RESCHED_LAZY;
unsigned long ti_work;
- u32 cancel;
unsigned long irq_flags;
struct hv_vp_assist_page *hvp;
int ret;
local_irq_save(irq_flags);
+ if (READ_ONCE(mshv_vtl_this_run()->cancel)) {
+ local_irq_restore(irq_flags);
+ preempt_enable();
+ return -EINTR;
+ }
+
ti_work = READ_ONCE(current_thread_info()->flags);
- cancel = READ_ONCE(mshv_vtl_this_run()->cancel);
- if (unlikely((ti_work & VTL0_WORK) || cancel)) {
+ if (unlikely(ti_work & VTL0_WORK)) {
local_irq_restore(irq_flags);
preempt_enable();
- if (cancel)
- ti_work |= _TIF_SIGPENDING;
ret = mshv_do_pre_guest_mode_work(ti_work);
if (ret)
return ret;
--
2.51.0.268.g9569e192d0-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox