* [PATCH hyperv-next v6 09/17] Drivers: hv: remove stale comment
From: Roman Kisel @ 2025-10-03 22:27 UTC (permalink / raw)
To: arnd, bp, corbet, dave.hansen, decui, haiyangz, hpa, kys,
mikelley, mingo, tglx, Tianyu.Lan, wei.liu, x86, linux-hyperv,
linux-doc, linux-kernel, linux-arch
Cc: benhill, bperkins, sunilmut, romank
In-Reply-To: <20251003222710.6257-1-romank@linux.microsoft.com>
The comment about the x2v shim is ancient and long since incorrect.
Remove the incorrect comment.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Tianyu Lan <tiala@microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/hv/hv.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index b1085473778c..95631c08a71f 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -268,11 +268,7 @@ void hv_synic_free(void)
}
/*
- * hv_synic_init - Initialize the Synthetic Interrupt Controller.
- *
- * If it is already initialized by another entity (ie x2v shim), we need to
- * retrieve the initialized message and event pages. Otherwise, we create and
- * initialize the message and event pages.
+ * hv_synic_enable_regs - Initialize the Synthetic Interrupt Controller.
*/
void hv_synic_enable_regs(unsigned int cpu)
{
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v6 10/17] Drivers: hv: Check message and event pages for non-NULL before iounmap()
From: Roman Kisel @ 2025-10-03 22:27 UTC (permalink / raw)
To: arnd, bp, corbet, dave.hansen, decui, haiyangz, hpa, kys,
mikelley, mingo, tglx, Tianyu.Lan, wei.liu, x86, linux-hyperv,
linux-doc, linux-kernel, linux-arch
Cc: benhill, bperkins, sunilmut, romank
In-Reply-To: <20251003222710.6257-1-romank@linux.microsoft.com>
It might happen that some hyp SynIC pages aren't allocated.
Check for that and only then call iounmap().
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Tianyu Lan <tiala@microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/hv/hv.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 95631c08a71f..8e102bcc0be8 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -369,8 +369,10 @@ void hv_synic_disable_regs(unsigned int cpu)
*/
simp.simp_enabled = 0;
if (ms_hyperv.paravisor_present || hv_root_partition()) {
- iounmap(hv_cpu->hyp_synic_message_page);
- hv_cpu->hyp_synic_message_page = NULL;
+ if (hv_cpu->hyp_synic_message_page) {
+ iounmap(hv_cpu->hyp_synic_message_page);
+ hv_cpu->hyp_synic_message_page = NULL;
+ }
} else {
simp.base_simp_gpa = 0;
}
@@ -381,8 +383,10 @@ void hv_synic_disable_regs(unsigned int cpu)
siefp.siefp_enabled = 0;
if (ms_hyperv.paravisor_present || hv_root_partition()) {
- iounmap(hv_cpu->hyp_synic_event_page);
- hv_cpu->hyp_synic_event_page = NULL;
+ if (hv_cpu->hyp_synic_event_page) {
+ iounmap(hv_cpu->hyp_synic_event_page);
+ hv_cpu->hyp_synic_event_page = NULL;
+ }
} else {
siefp.base_siefp_gpa = 0;
}
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v6 11/17] Drivers: hv: Rename the SynIC enable and disable routines
From: Roman Kisel @ 2025-10-03 22:27 UTC (permalink / raw)
To: arnd, bp, corbet, dave.hansen, decui, haiyangz, hpa, kys,
mikelley, mingo, tglx, Tianyu.Lan, wei.liu, x86, linux-hyperv,
linux-doc, linux-kernel, linux-arch
Cc: benhill, bperkins, sunilmut, romank
In-Reply-To: <20251003222710.6257-1-romank@linux.microsoft.com>
The confidential VMBus requires support for the both hypervisor
facing SynIC and the paravisor one.
Rename the functions that enable and disable SynIC with the
hypervisor. No functional changes.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Tianyu Lan <tiala@microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/hv/channel_mgmt.c | 2 +-
drivers/hv/hv.c | 11 ++++++-----
drivers/hv/hyperv_vmbus.h | 4 ++--
drivers/hv/vmbus_drv.c | 6 +++---
4 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 1a33c6944b3c..6d66cbc9030b 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -846,7 +846,7 @@ static void vmbus_wait_for_unload(void)
/*
* In a CoCo VM the hyp_synic_message_page is not allocated
* in hv_synic_alloc(). Instead it is set/cleared in
- * hv_synic_enable_regs() and hv_synic_disable_regs()
+ * hv_hyp_synic_enable_regs() and hv_hyp_synic_disable_regs()
* such that it is set only when the CPU is online. If
* not all present CPUs are online, the message page
* might be NULL, so skip such CPUs.
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 8e102bcc0be8..76138ebe7c0c 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -268,9 +268,10 @@ void hv_synic_free(void)
}
/*
- * hv_synic_enable_regs - Initialize the Synthetic Interrupt Controller.
+ * hv_hyp_synic_enable_regs - Initialize the Synthetic Interrupt Controller
+ * with the hypervisor.
*/
-void hv_synic_enable_regs(unsigned int cpu)
+void hv_hyp_synic_enable_regs(unsigned int cpu)
{
struct hv_per_cpu_context *hv_cpu =
per_cpu_ptr(hv_context.cpu_context, cpu);
@@ -336,14 +337,14 @@ void hv_synic_enable_regs(unsigned int cpu)
int hv_synic_init(unsigned int cpu)
{
- hv_synic_enable_regs(cpu);
+ hv_hyp_synic_enable_regs(cpu);
hv_stimer_legacy_init(cpu, VMBUS_MESSAGE_SINT);
return 0;
}
-void hv_synic_disable_regs(unsigned int cpu)
+void hv_hyp_synic_disable_regs(unsigned int cpu)
{
struct hv_per_cpu_context *hv_cpu =
per_cpu_ptr(hv_context.cpu_context, cpu);
@@ -530,7 +531,7 @@ int hv_synic_cleanup(unsigned int cpu)
always_cleanup:
hv_stimer_legacy_cleanup(cpu);
- hv_synic_disable_regs(cpu);
+ hv_hyp_synic_disable_regs(cpu);
return ret;
}
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 3c70051c0431..552ed782bcfc 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -190,10 +190,10 @@ extern int hv_synic_alloc(void);
extern void hv_synic_free(void);
-extern void hv_synic_enable_regs(unsigned int cpu);
+extern void hv_hyp_synic_enable_regs(unsigned int cpu);
extern int hv_synic_init(unsigned int cpu);
-extern void hv_synic_disable_regs(unsigned int cpu);
+extern void hv_hyp_synic_disable_regs(unsigned int cpu);
extern int hv_synic_cleanup(unsigned int cpu);
/* Interface */
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index e12f0ba0701f..2b5bf672c467 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2810,7 +2810,7 @@ static void hv_crash_handler(struct pt_regs *regs)
*/
cpu = smp_processor_id();
hv_stimer_cleanup(cpu);
- hv_synic_disable_regs(cpu);
+ hv_hyp_synic_disable_regs(cpu);
};
static int hv_synic_suspend(void)
@@ -2835,14 +2835,14 @@ static int hv_synic_suspend(void)
* interrupts-disabled context.
*/
- hv_synic_disable_regs(0);
+ hv_hyp_synic_disable_regs(0);
return 0;
}
static void hv_synic_resume(void)
{
- hv_synic_enable_regs(0);
+ hv_hyp_synic_enable_regs(0);
/*
* Note: we don't need to call hv_stimer_init(0), because the timer
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v6 12/17] Drivers: hv: Functions for setting up and tearing down the paravisor SynIC
From: Roman Kisel @ 2025-10-03 22:27 UTC (permalink / raw)
To: arnd, bp, corbet, dave.hansen, decui, haiyangz, hpa, kys,
mikelley, mingo, tglx, Tianyu.Lan, wei.liu, x86, linux-hyperv,
linux-doc, linux-kernel, linux-arch
Cc: benhill, bperkins, sunilmut, romank
In-Reply-To: <20251003222710.6257-1-romank@linux.microsoft.com>
The confidential VMBus runs with the paravisor SynIC and requires
configuring it with the paravisor.
Add the functions for configuring the paravisor SynIC. Update
overall SynIC initialization logic to initialize the SynIC if it
is present. Finally, break out SynIC interrupt enable/disable
code into separate functions so that SynIC interrupts can be
enabled or disabled via the paravisor instead of the hypervisor
if the paravisor SynIC is present.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/hv/hv.c | 138 +++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 126 insertions(+), 12 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 76138ebe7c0c..5789b41be76c 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -278,9 +278,8 @@ void hv_hyp_synic_enable_regs(unsigned int cpu)
union hv_synic_simp simp;
union hv_synic_siefp siefp;
union hv_synic_sint shared_sint;
- union hv_synic_scontrol sctrl;
- /* Setup the Synic's message page */
+ /* Setup the Synic's message page with the hypervisor. */
simp.as_uint64 = hv_get_msr(HV_MSR_SIMP);
simp.simp_enabled = 1;
@@ -299,7 +298,7 @@ void hv_hyp_synic_enable_regs(unsigned int cpu)
hv_set_msr(HV_MSR_SIMP, simp.as_uint64);
- /* Setup the Synic's event page */
+ /* Setup the Synic's event page with the hypervisor. */
siefp.as_uint64 = hv_get_msr(HV_MSR_SIEFP);
siefp.siefp_enabled = 1;
@@ -327,6 +326,11 @@ void hv_hyp_synic_enable_regs(unsigned int cpu)
shared_sint.masked = false;
shared_sint.auto_eoi = hv_recommend_using_aeoi();
hv_set_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
+}
+
+static void hv_hyp_synic_enable_interrupts(void)
+{
+ union hv_synic_scontrol sctrl;
/* Enable the global synic bit */
sctrl.as_uint64 = hv_get_msr(HV_MSR_SCONTROL);
@@ -335,9 +339,59 @@ void hv_hyp_synic_enable_regs(unsigned int cpu)
hv_set_msr(HV_MSR_SCONTROL, sctrl.as_uint64);
}
+static void hv_para_synic_enable_regs(unsigned int cpu)
+{
+ union hv_synic_simp simp;
+ union hv_synic_siefp siefp;
+ struct hv_per_cpu_context *hv_cpu
+ = per_cpu_ptr(hv_context.cpu_context, cpu);
+
+ /* Setup the Synic's message page with the paravisor. */
+ simp.as_uint64 = hv_para_get_synic_register(HV_MSR_SIMP);
+ simp.simp_enabled = 1;
+ simp.base_simp_gpa = virt_to_phys(hv_cpu->para_synic_message_page)
+ >> HV_HYP_PAGE_SHIFT;
+ hv_para_set_synic_register(HV_MSR_SIMP, simp.as_uint64);
+
+ /* Setup the Synic's event page with the paravisor. */
+ siefp.as_uint64 = hv_para_get_synic_register(HV_MSR_SIEFP);
+ siefp.siefp_enabled = 1;
+ siefp.base_siefp_gpa = virt_to_phys(hv_cpu->para_synic_event_page)
+ >> HV_HYP_PAGE_SHIFT;
+ hv_para_set_synic_register(HV_MSR_SIEFP, siefp.as_uint64);
+}
+
+static void hv_para_synic_enable_interrupts(void)
+{
+ union hv_synic_scontrol sctrl;
+
+ /* Enable the global synic bit */
+ sctrl.as_uint64 = hv_para_get_synic_register(HV_MSR_SCONTROL);
+ sctrl.enable = 1;
+ hv_para_set_synic_register(HV_MSR_SCONTROL, sctrl.as_uint64);
+}
+
int hv_synic_init(unsigned int cpu)
{
+ if (vmbus_is_confidential())
+ hv_para_synic_enable_regs(cpu);
+
+ /*
+ * The SINT is set in hv_hyp_synic_enable_regs() by calling
+ * hv_set_msr(). hv_set_msr() in turn has special case code for the
+ * SINT MSRs that write to the hypervisor version of the MSR *and*
+ * the paravisor version of the MSR (but *without* the proxy bit when
+ * VMBus is confidential).
+ *
+ * Then enable interrupts via the paravisor if VMBus is confidential,
+ * and otherwise via the hypervisor.
+ */
+
hv_hyp_synic_enable_regs(cpu);
+ if (vmbus_is_confidential())
+ hv_para_synic_enable_interrupts();
+ else
+ hv_hyp_synic_enable_interrupts();
hv_stimer_legacy_init(cpu, VMBUS_MESSAGE_SINT);
@@ -351,7 +405,6 @@ void hv_hyp_synic_disable_regs(unsigned int cpu)
union hv_synic_sint shared_sint;
union hv_synic_simp simp;
union hv_synic_siefp siefp;
- union hv_synic_scontrol sctrl;
shared_sint.as_uint64 = hv_get_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT);
@@ -363,7 +416,7 @@ void hv_hyp_synic_disable_regs(unsigned int cpu)
simp.as_uint64 = hv_get_msr(HV_MSR_SIMP);
/*
- * In Isolation VM, sim and sief pages are allocated by
+ * In Isolation VM, simp and sief pages are allocated by
* paravisor. These pages also will be used by kdump
* kernel. So just reset enable bit here and keep page
* addresses.
@@ -393,14 +446,42 @@ void hv_hyp_synic_disable_regs(unsigned int cpu)
}
hv_set_msr(HV_MSR_SIEFP, siefp.as_uint64);
+}
+
+static void hv_hyp_synic_disable_interrupts(void)
+{
+ union hv_synic_scontrol sctrl;
/* Disable the global synic bit */
sctrl.as_uint64 = hv_get_msr(HV_MSR_SCONTROL);
sctrl.enable = 0;
hv_set_msr(HV_MSR_SCONTROL, sctrl.as_uint64);
+}
- if (vmbus_irq != -1)
- disable_percpu_irq(vmbus_irq);
+static void hv_para_synic_disable_regs(unsigned int cpu)
+{
+ union hv_synic_simp simp;
+ union hv_synic_siefp siefp;
+
+ /* Disable SynIC's message page in the paravisor. */
+ simp.as_uint64 = hv_para_get_synic_register(HV_MSR_SIMP);
+ simp.simp_enabled = 0;
+ hv_para_set_synic_register(HV_MSR_SIMP, simp.as_uint64);
+
+ /* Disable SynIC's event page in the paravisor. */
+ siefp.as_uint64 = hv_para_get_synic_register(HV_MSR_SIEFP);
+ siefp.siefp_enabled = 0;
+ hv_para_set_synic_register(HV_MSR_SIEFP, siefp.as_uint64);
+}
+
+static void hv_para_synic_disable_interrupts(void)
+{
+ union hv_synic_scontrol sctrl;
+
+ /* Disable the global synic bit */
+ sctrl.as_uint64 = hv_para_get_synic_register(HV_MSR_SCONTROL);
+ sctrl.enable = 0;
+ hv_para_set_synic_register(HV_MSR_SCONTROL, sctrl.as_uint64);
}
#define HV_MAX_TRIES 3
@@ -413,16 +494,18 @@ void hv_hyp_synic_disable_regs(unsigned int cpu)
* that the normal interrupt handling mechanism will find and process the channel interrupt
* "very soon", and in the process clear the bit.
*/
-static bool hv_synic_event_pending(void)
+static bool __hv_synic_event_pending(union hv_synic_event_flags *event, int sint)
{
- struct hv_per_cpu_context *hv_cpu = this_cpu_ptr(hv_context.cpu_context);
- union hv_synic_event_flags *event =
- (union hv_synic_event_flags *)hv_cpu->hyp_synic_event_page + VMBUS_MESSAGE_SINT;
- unsigned long *recv_int_page = event->flags; /* assumes VMBus version >= VERSION_WIN8 */
+ unsigned long *recv_int_page;
bool pending;
u32 relid;
int tries = 0;
+ if (!event)
+ return false;
+
+ event += sint;
+ recv_int_page = event->flags; /* assumes VMBus version >= VERSION_WIN8 */
retry:
pending = false;
for_each_set_bit(relid, recv_int_page, HV_EVENT_FLAGS_COUNT) {
@@ -439,6 +522,17 @@ static bool hv_synic_event_pending(void)
return pending;
}
+static bool hv_synic_event_pending(void)
+{
+ struct hv_per_cpu_context *hv_cpu = this_cpu_ptr(hv_context.cpu_context);
+ union hv_synic_event_flags *hyp_synic_event_page = hv_cpu->hyp_synic_event_page;
+ union hv_synic_event_flags *para_synic_event_page = hv_cpu->para_synic_event_page;
+
+ return
+ __hv_synic_event_pending(hyp_synic_event_page, VMBUS_MESSAGE_SINT) ||
+ __hv_synic_event_pending(para_synic_event_page, VMBUS_MESSAGE_SINT);
+}
+
static int hv_pick_new_cpu(struct vmbus_channel *channel)
{
int ret = -EBUSY;
@@ -531,7 +625,27 @@ int hv_synic_cleanup(unsigned int cpu)
always_cleanup:
hv_stimer_legacy_cleanup(cpu);
+ /*
+ * First, disable the event and message pages
+ * used for communicating with the host, and then
+ * disable the host interrupts if VMBus is not
+ * confidential.
+ */
hv_hyp_synic_disable_regs(cpu);
+ if (!vmbus_is_confidential())
+ hv_hyp_synic_disable_interrupts();
+
+ /*
+ * Perform the same steps for the Confidential VMBus.
+ * The sequencing provides the guarantee that no data
+ * may be posted for processing before disabling interrupts.
+ */
+ if (vmbus_is_confidential()) {
+ hv_para_synic_disable_regs(cpu);
+ hv_para_synic_disable_interrupts();
+ }
+ if (vmbus_irq != -1)
+ disable_percpu_irq(vmbus_irq);
return ret;
}
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v6 14/17] Drivers: hv: Free msginfo when the buffer fails to decrypt
From: Roman Kisel @ 2025-10-03 22:27 UTC (permalink / raw)
To: arnd, bp, corbet, dave.hansen, decui, haiyangz, hpa, kys,
mikelley, mingo, tglx, Tianyu.Lan, wei.liu, x86, linux-hyperv,
linux-doc, linux-kernel, linux-arch
Cc: benhill, bperkins, sunilmut, romank
In-Reply-To: <20251003222710.6257-1-romank@linux.microsoft.com>
The early failure path in __vmbus_establish_gpadl() doesn't deallocate
msginfo if the buffer fails to decrypt.
Fix the leak by breaking out the cleanup code into a separate function
and calling it where required.
Fixes: d4dccf353db80 ("Drivers: hv: vmbus: Mark vmbus ring buffer visible to host in Isolation VM")
Reported-by: Michael Kelley <mkhlinux@outlook.com>
Closes: https://lore.kernel.org/linux-hyperv/SN6PR02MB41573796F9787F67E0E97049D472A@SN6PR02MB4157.namprd02.prod.outlook.com
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/hv/channel.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index d69713201bef..88485d255a42 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -410,6 +410,21 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
return 0;
}
+static void vmbus_free_channel_msginfo(struct vmbus_channel_msginfo *msginfo)
+{
+ struct vmbus_channel_msginfo *submsginfo, *tmp;
+
+ if (!msginfo)
+ return;
+
+ list_for_each_entry_safe(submsginfo, tmp, &msginfo->submsglist,
+ msglistentry) {
+ kfree(submsginfo);
+ }
+
+ kfree(msginfo);
+}
+
/*
* __vmbus_establish_gpadl - Establish a GPADL for a buffer or ringbuffer
*
@@ -429,7 +444,7 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
struct vmbus_channel_gpadl_header *gpadlmsg;
struct vmbus_channel_gpadl_body *gpadl_body;
struct vmbus_channel_msginfo *msginfo = NULL;
- struct vmbus_channel_msginfo *submsginfo, *tmp;
+ struct vmbus_channel_msginfo *submsginfo;
struct list_head *curr;
u32 next_gpadl_handle;
unsigned long flags;
@@ -459,6 +474,7 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
dev_warn(&channel->device_obj->device,
"Failed to set host visibility for new GPADL %d.\n",
ret);
+ vmbus_free_channel_msginfo(msginfo);
return ret;
}
}
@@ -535,12 +551,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
list_del(&msginfo->msglistentry);
spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
- list_for_each_entry_safe(submsginfo, tmp, &msginfo->submsglist,
- msglistentry) {
- kfree(submsginfo);
- }
- kfree(msginfo);
+ vmbus_free_channel_msginfo(msginfo);
if (ret) {
/*
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v6 13/17] Drivers: hv: Allocate encrypted buffers when requested
From: Roman Kisel @ 2025-10-03 22:27 UTC (permalink / raw)
To: arnd, bp, corbet, dave.hansen, decui, haiyangz, hpa, kys,
mikelley, mingo, tglx, Tianyu.Lan, wei.liu, x86, linux-hyperv,
linux-doc, linux-kernel, linux-arch
Cc: benhill, bperkins, sunilmut, romank
In-Reply-To: <20251003222710.6257-1-romank@linux.microsoft.com>
Confidential VMBus is built around using buffers not shared with
the host.
Support allocating encrypted buffers when requested.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Tianyu Lan <tiala@microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/hv/channel.c | 49 +++++++++++++++++++++++----------------
drivers/hv/hyperv_vmbus.h | 3 ++-
drivers/hv/ring_buffer.c | 5 ++--
3 files changed, 34 insertions(+), 23 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 162d6aeece7b..d69713201bef 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -444,20 +444,23 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
return ret;
}
- /*
- * Set the "decrypted" flag to true for the set_memory_decrypted()
- * success case. In the failure case, the encryption state of the
- * memory is unknown. Leave "decrypted" as true to ensure the
- * memory will be leaked instead of going back on the free list.
- */
- gpadl->decrypted = true;
- ret = set_memory_decrypted((unsigned long)kbuffer,
- PFN_UP(size));
- if (ret) {
- dev_warn(&channel->device_obj->device,
- "Failed to set host visibility for new GPADL %d.\n",
- ret);
- return ret;
+ gpadl->decrypted = !((channel->co_external_memory && type == HV_GPADL_BUFFER) ||
+ (channel->co_ring_buffer && type == HV_GPADL_RING));
+ if (gpadl->decrypted) {
+ /*
+ * The "decrypted" flag being true assumes that set_memory_decrypted() succeeds.
+ * But if it fails, the encryption state of the memory is unknown. In that case,
+ * leave "decrypted" as true to ensure the memory is leaked instead of going back
+ * on the free list.
+ */
+ ret = set_memory_decrypted((unsigned long)kbuffer,
+ PFN_UP(size));
+ if (ret) {
+ dev_warn(&channel->device_obj->device,
+ "Failed to set host visibility for new GPADL %d.\n",
+ ret);
+ return ret;
+ }
}
init_completion(&msginfo->waitevent);
@@ -545,8 +548,10 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
* left as true so the memory is leaked instead of being
* put back on the free list.
*/
- if (!set_memory_encrypted((unsigned long)kbuffer, PFN_UP(size)))
- gpadl->decrypted = false;
+ if (gpadl->decrypted) {
+ if (!set_memory_encrypted((unsigned long)kbuffer, PFN_UP(size)))
+ gpadl->decrypted = false;
+ }
}
return ret;
@@ -677,12 +682,13 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
goto error_clean_ring;
err = hv_ringbuffer_init(&newchannel->outbound,
- page, send_pages, 0);
+ page, send_pages, 0, newchannel->co_ring_buffer);
if (err)
goto error_free_gpadl;
err = hv_ringbuffer_init(&newchannel->inbound, &page[send_pages],
- recv_pages, newchannel->max_pkt_size);
+ recv_pages, newchannel->max_pkt_size,
+ newchannel->co_ring_buffer);
if (err)
goto error_free_gpadl;
@@ -863,8 +869,11 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, struct vmbus_gpadl *gpad
kfree(info);
- ret = set_memory_encrypted((unsigned long)gpadl->buffer,
- PFN_UP(gpadl->size));
+ if (gpadl->decrypted)
+ ret = set_memory_encrypted((unsigned long)gpadl->buffer,
+ PFN_UP(gpadl->size));
+ else
+ ret = 0;
if (ret)
pr_warn("Fail to set mem host visibility in GPADL teardown %d.\n", ret);
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 552ed782bcfc..f7fc2630c054 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -201,7 +201,8 @@ extern int hv_synic_cleanup(unsigned int cpu);
void hv_ringbuffer_pre_init(struct vmbus_channel *channel);
int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
- struct page *pages, u32 pagecnt, u32 max_pkt_size);
+ struct page *pages, u32 pagecnt, u32 max_pkt_size,
+ bool confidential);
void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info);
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 23ce1fb70de1..3c421a7f78c0 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -184,7 +184,8 @@ void hv_ringbuffer_pre_init(struct vmbus_channel *channel)
/* Initialize the ring buffer. */
int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
- struct page *pages, u32 page_cnt, u32 max_pkt_size)
+ struct page *pages, u32 page_cnt, u32 max_pkt_size,
+ bool confidential)
{
struct page **pages_wraparound;
int i;
@@ -208,7 +209,7 @@ int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
ring_info->ring_buffer = (struct hv_ring_buffer *)
vmap(pages_wraparound, page_cnt * 2 - 1, VM_MAP,
- pgprot_decrypted(PAGE_KERNEL));
+ confidential ? PAGE_KERNEL : pgprot_decrypted(PAGE_KERNEL));
kfree(pages_wraparound);
if (!ring_info->ring_buffer)
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v6 15/17] Drivers: hv: Support confidential VMBus channels
From: Roman Kisel @ 2025-10-03 22:27 UTC (permalink / raw)
To: arnd, bp, corbet, dave.hansen, decui, haiyangz, hpa, kys,
mikelley, mingo, tglx, Tianyu.Lan, wei.liu, x86, linux-hyperv,
linux-doc, linux-kernel, linux-arch
Cc: benhill, bperkins, sunilmut, romank
In-Reply-To: <20251003222710.6257-1-romank@linux.microsoft.com>
To make use of Confidential VMBus channels, initialize the
co_ring_buffers and co_external_memory fields of the channel
structure.
Advertise support upon negotiating the version and compute
values for those fields and initialize them.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/hv/channel_mgmt.c | 19 +++++++++++++++++++
drivers/hv/connection.c | 3 +++
2 files changed, 22 insertions(+)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 6d66cbc9030b..74fed2c073d4 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1022,6 +1022,7 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
struct vmbus_channel_offer_channel *offer;
struct vmbus_channel *oldchannel, *newchannel;
size_t offer_sz;
+ bool co_ring_buffer, co_external_memory;
offer = (struct vmbus_channel_offer_channel *)hdr;
@@ -1034,6 +1035,22 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
return;
}
+ co_ring_buffer = is_co_ring_buffer(offer);
+ co_external_memory = is_co_external_memory(offer);
+ if (!co_ring_buffer && co_external_memory) {
+ pr_err("Invalid offer relid=%d: the ring buffer isn't encrypted\n",
+ offer->child_relid);
+ return;
+ }
+ if (co_ring_buffer || co_external_memory) {
+ if (vmbus_proto_version < VERSION_WIN10_V6_0 || !vmbus_is_confidential()) {
+ pr_err("Invalid offer relid=%d: no support for confidential VMBus\n",
+ offer->child_relid);
+ atomic_dec(&vmbus_connection.offer_in_progress);
+ return;
+ }
+ }
+
oldchannel = find_primary_channel_by_offer(offer);
if (oldchannel != NULL) {
@@ -1112,6 +1129,8 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
pr_err("Unable to allocate channel object\n");
return;
}
+ newchannel->co_ring_buffer = co_ring_buffer;
+ newchannel->co_external_memory = co_external_memory;
vmbus_setup_channel_state(newchannel, offer);
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 1fe3573ae52a..5ac9232396f7 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -105,6 +105,9 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID;
}
+ if (vmbus_is_confidential() && version >= VERSION_WIN10_V6_0)
+ msg->feature_flags = VMBUS_FEATURE_FLAG_CONFIDENTIAL_CHANNELS;
+
/*
* shared_gpa_boundary is zero in non-SNP VMs, so it's safe to always
* bitwise OR it
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v6 16/17] Drivers: hv: Set the default VMBus version to 6.0
From: Roman Kisel @ 2025-10-03 22:27 UTC (permalink / raw)
To: arnd, bp, corbet, dave.hansen, decui, haiyangz, hpa, kys,
mikelley, mingo, tglx, Tianyu.Lan, wei.liu, x86, linux-hyperv,
linux-doc, linux-kernel, linux-arch
Cc: benhill, bperkins, sunilmut, romank
In-Reply-To: <20251003222710.6257-1-romank@linux.microsoft.com>
The confidential VMBus is supported by the protocol version
6.0 onwards.
Attempt to establish the VMBus 6.0 connection thus enabling
the confidential VMBus features when available.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/hv/connection.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 5ac9232396f7..5d9cb5bf2d62 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -51,6 +51,7 @@ EXPORT_SYMBOL_GPL(vmbus_proto_version);
* Linux guests and are not listed.
*/
static __u32 vmbus_versions[] = {
+ VERSION_WIN10_V6_0,
VERSION_WIN10_V5_3,
VERSION_WIN10_V5_2,
VERSION_WIN10_V5_1,
@@ -65,7 +66,7 @@ static __u32 vmbus_versions[] = {
* Maximal VMBus protocol version guests can negotiate. Useful to cap the
* VMBus version for testing and debugging purpose.
*/
-static uint max_version = VERSION_WIN10_V5_3;
+static uint max_version = VERSION_WIN10_V6_0;
module_param(max_version, uint, S_IRUGO);
MODULE_PARM_DESC(max_version,
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v6 17/17] Drivers: hv: Support establishing the confidential VMBus connection
From: Roman Kisel @ 2025-10-03 22:27 UTC (permalink / raw)
To: arnd, bp, corbet, dave.hansen, decui, haiyangz, hpa, kys,
mikelley, mingo, tglx, Tianyu.Lan, wei.liu, x86, linux-hyperv,
linux-doc, linux-kernel, linux-arch
Cc: benhill, bperkins, sunilmut, romank
In-Reply-To: <20251003222710.6257-1-romank@linux.microsoft.com>
To establish the confidential VMBus connection the CoCo VM, the guest
first checks on the confidential VMBus availability, and then proceeds
to initializing the communication stack.
Implement that in the VMBus driver initialization.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/hv/vmbus_drv.c | 168 ++++++++++++++++++++++++++---------------
1 file changed, 106 insertions(+), 62 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 2b5bf672c467..0dc4692b411a 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1057,12 +1057,9 @@ static void vmbus_onmessage_work(struct work_struct *work)
kfree(ctx);
}
-void vmbus_on_msg_dpc(unsigned long data)
+static void __vmbus_on_msg_dpc(void *message_page_addr)
{
- struct hv_per_cpu_context *hv_cpu = (void *)data;
- void *page_addr = hv_cpu->hyp_synic_message_page;
- struct hv_message msg_copy, *msg = (struct hv_message *)page_addr +
- VMBUS_MESSAGE_SINT;
+ struct hv_message msg_copy, *msg;
struct vmbus_channel_message_header *hdr;
enum vmbus_channel_message_type msgtype;
const struct vmbus_channel_message_table_entry *entry;
@@ -1070,6 +1067,10 @@ void vmbus_on_msg_dpc(unsigned long data)
__u8 payload_size;
u32 message_type;
+ if (!message_page_addr)
+ return;
+ msg = (struct hv_message *)message_page_addr + VMBUS_MESSAGE_SINT;
+
/*
* 'enum vmbus_channel_message_type' is supposed to always be 'u32' as
* it is being used in 'struct vmbus_channel_message_header' definition
@@ -1195,6 +1196,14 @@ void vmbus_on_msg_dpc(unsigned long data)
vmbus_signal_eom(msg, message_type);
}
+void vmbus_on_msg_dpc(unsigned long data)
+{
+ struct hv_per_cpu_context *hv_cpu = (void *)data;
+
+ __vmbus_on_msg_dpc(hv_cpu->hyp_synic_message_page);
+ __vmbus_on_msg_dpc(hv_cpu->para_synic_message_page);
+}
+
#ifdef CONFIG_PM_SLEEP
/*
* Fake RESCIND_CHANNEL messages to clean up hv_sock channels by force for
@@ -1233,21 +1242,19 @@ static void vmbus_force_channel_rescinded(struct vmbus_channel *channel)
#endif /* CONFIG_PM_SLEEP */
/*
- * Schedule all channels with events pending
+ * Schedule all channels with events pending.
+ * The event page can be directly checked to get the id of
+ * the channel that has the interrupt pending.
*/
-static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
+static void vmbus_chan_sched(void *event_page_addr)
{
unsigned long *recv_int_page;
u32 maxbits, relid;
+ union hv_synic_event_flags *event;
- /*
- * The event page can be directly checked to get the id of
- * the channel that has the interrupt pending.
- */
- void *page_addr = hv_cpu->hyp_synic_event_page;
- union hv_synic_event_flags *event
- = (union hv_synic_event_flags *)page_addr +
- VMBUS_MESSAGE_SINT;
+ if (!event_page_addr)
+ return;
+ event = (union hv_synic_event_flags *)event_page_addr + VMBUS_MESSAGE_SINT;
maxbits = HV_EVENT_FLAGS_COUNT;
recv_int_page = event->flags;
@@ -1255,6 +1262,11 @@ static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
if (unlikely(!recv_int_page))
return;
+ /*
+ * Suggested-by: Michael Kelley <mhklinux@outlook.com>
+ * One possible optimization would be to keep track of the largest relID that's in use,
+ * and only scan up to that relID.
+ */
for_each_set_bit(relid, recv_int_page, maxbits) {
void (*callback_fn)(void *context);
struct vmbus_channel *channel;
@@ -1318,26 +1330,35 @@ static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
}
}
-static void vmbus_isr(void)
+static void vmbus_message_sched(struct hv_per_cpu_context *hv_cpu, void *message_page_addr)
{
- struct hv_per_cpu_context *hv_cpu
- = this_cpu_ptr(hv_context.cpu_context);
- void *page_addr;
struct hv_message *msg;
- vmbus_chan_sched(hv_cpu);
-
- page_addr = hv_cpu->hyp_synic_message_page;
- msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT;
+ if (!message_page_addr)
+ return;
+ msg = (struct hv_message *)message_page_addr + VMBUS_MESSAGE_SINT;
/* Check if there are actual msgs to be processed */
if (msg->header.message_type != HVMSG_NONE) {
if (msg->header.message_type == HVMSG_TIMER_EXPIRED) {
hv_stimer0_isr();
vmbus_signal_eom(msg, HVMSG_TIMER_EXPIRED);
- } else
+ } else {
tasklet_schedule(&hv_cpu->msg_dpc);
+ }
}
+}
+
+static void vmbus_isr(void)
+{
+ struct hv_per_cpu_context *hv_cpu
+ = this_cpu_ptr(hv_context.cpu_context);
+
+ vmbus_chan_sched(hv_cpu->hyp_synic_event_page);
+ vmbus_chan_sched(hv_cpu->para_synic_event_page);
+
+ vmbus_message_sched(hv_cpu, hv_cpu->hyp_synic_message_page);
+ vmbus_message_sched(hv_cpu, hv_cpu->para_synic_message_page);
add_interrupt_randomness(vmbus_interrupt);
}
@@ -1355,6 +1376,59 @@ static void vmbus_percpu_work(struct work_struct *work)
hv_synic_init(cpu);
}
+static int vmbus_alloc_synic_and_connect(void)
+{
+ int ret, cpu;
+ struct work_struct __percpu *works;
+ int hyperv_cpuhp_online;
+
+ ret = hv_synic_alloc();
+ if (ret < 0)
+ goto err_alloc;
+
+ works = alloc_percpu(struct work_struct);
+ if (!works) {
+ ret = -ENOMEM;
+ goto err_alloc;
+ }
+
+ /*
+ * Initialize the per-cpu interrupt state and stimer state.
+ * Then connect to the host.
+ */
+ cpus_read_lock();
+ for_each_online_cpu(cpu) {
+ struct work_struct *work = per_cpu_ptr(works, cpu);
+
+ INIT_WORK(work, vmbus_percpu_work);
+ schedule_work_on(cpu, work);
+ }
+
+ for_each_online_cpu(cpu)
+ flush_work(per_cpu_ptr(works, cpu));
+
+ /* Register the callbacks for possible CPU online/offline'ing */
+ ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN, "hyperv/vmbus:online",
+ hv_synic_init, hv_synic_cleanup);
+ cpus_read_unlock();
+ free_percpu(works);
+ if (ret < 0)
+ goto err_alloc;
+ hyperv_cpuhp_online = ret;
+
+ ret = vmbus_connect();
+ if (ret)
+ goto err_connect;
+ return 0;
+
+err_connect:
+ cpuhp_remove_state(hyperv_cpuhp_online);
+ return -ENODEV;
+err_alloc:
+ hv_synic_free();
+ return -ENOMEM;
+}
+
/*
* vmbus_bus_init -Main vmbus driver initialization routine.
*
@@ -1365,8 +1439,7 @@ static void vmbus_percpu_work(struct work_struct *work)
*/
static int vmbus_bus_init(void)
{
- int ret, cpu;
- struct work_struct __percpu *works;
+ int ret;
ret = hv_init();
if (ret != 0) {
@@ -1401,41 +1474,15 @@ static int vmbus_bus_init(void)
}
}
- ret = hv_synic_alloc();
- if (ret)
- goto err_alloc;
-
- works = alloc_percpu(struct work_struct);
- if (!works) {
- ret = -ENOMEM;
- goto err_alloc;
- }
-
/*
- * Initialize the per-cpu interrupt state and stimer state.
- * Then connect to the host.
+ * Cache the value as getting it involves a VM exit on x86(_64), and
+ * doing that on each VP while initializing SynIC's wastes time.
*/
- cpus_read_lock();
- for_each_online_cpu(cpu) {
- struct work_struct *work = per_cpu_ptr(works, cpu);
-
- INIT_WORK(work, vmbus_percpu_work);
- schedule_work_on(cpu, work);
- }
-
- for_each_online_cpu(cpu)
- flush_work(per_cpu_ptr(works, cpu));
-
- /* Register the callbacks for possible CPU online/offline'ing */
- ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN, "hyperv/vmbus:online",
- hv_synic_init, hv_synic_cleanup);
- cpus_read_unlock();
- free_percpu(works);
- if (ret < 0)
- goto err_alloc;
- hyperv_cpuhp_online = ret;
-
- ret = vmbus_connect();
+ is_confidential = ms_hyperv.confidential_vmbus_available;
+ if (is_confidential)
+ pr_info("Establishing connection to the confidential VMBus\n");
+ hv_para_set_sint_proxy(!is_confidential);
+ ret = vmbus_alloc_synic_and_connect();
if (ret)
goto err_connect;
@@ -1451,9 +1498,6 @@ static int vmbus_bus_init(void)
return 0;
err_connect:
- cpuhp_remove_state(hyperv_cpuhp_online);
-err_alloc:
- hv_synic_free();
if (vmbus_irq == -1) {
hv_remove_vmbus_handler();
} else {
--
2.43.0
^ permalink raw reply related
* Re: [PATCH hyperv-next v6 05/17] arch/x86: mshyperv: Trap on access for some synthetic MSRs
From: kernel test robot @ 2025-10-04 8:09 UTC (permalink / raw)
To: Roman Kisel, arnd, bp, corbet, dave.hansen, decui, haiyangz, hpa,
kys, mikelley, mingo, tglx, Tianyu.Lan, wei.liu, x86,
linux-hyperv, linux-doc, linux-kernel, linux-arch
Cc: llvm, oe-kbuild-all, benhill, bperkins, sunilmut, romank
In-Reply-To: <20251003222710.6257-6-romank@linux.microsoft.com>
Hi Roman,
kernel test robot noticed the following build warnings:
[auto build test WARNING on b595edcb24727e7f93e7962c3f6f971cc16dd29e]
url: https://github.com/intel-lab-lkp/linux/commits/Roman-Kisel/Documentation-hyperv-Confidential-VMBus/20251004-063158
base: b595edcb24727e7f93e7962c3f6f971cc16dd29e
patch link: https://lore.kernel.org/r/20251003222710.6257-6-romank%40linux.microsoft.com
patch subject: [PATCH hyperv-next v6 05/17] arch/x86: mshyperv: Trap on access for some synthetic MSRs
config: i386-buildonly-randconfig-003-20251004 (https://download.01.org/0day-ci/archive/20251004/202510041544.zPic8ogA-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251004/202510041544.zPic8ogA-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510041544.zPic8ogA-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> arch/x86/kernel/cpu/mshyperv.c:46:13: warning: unused variable 'hv_para_sint_proxy' [-Wunused-variable]
46 | static bool hv_para_sint_proxy;
| ^~~~~~~~~~~~~~~~~~
1 warning generated.
vim +/hv_para_sint_proxy +46 arch/x86/kernel/cpu/mshyperv.c
41
42 /*
43 * When running with the paravisor, controls proxying the synthetic interrupts
44 * from the host
45 */
> 46 static bool hv_para_sint_proxy;
47
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH net-next] net: mana: Linearize SKB if TX SGEs exceeds hardware limit
From: Simon Horman @ 2025-10-04 9:38 UTC (permalink / raw)
To: Aditya Garg
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, longli, kotaranov, shradhagupta, ernis, dipayanroy,
shirazsaleem, linux-hyperv, netdev, linux-kernel, linux-rdma,
gargaditya, ssengar
In-Reply-To: <20251003154724.GA15670@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
On Fri, Oct 03, 2025 at 08:47:24AM -0700, Aditya Garg wrote:
> The MANA hardware supports a maximum of 30 scatter-gather entries (SGEs)
> per TX WQE. In rare configurations where MAX_SKB_FRAGS + 2 exceeds this
> limit, the driver drops the skb. Add a check in mana_start_xmit() to
> detect such cases and linearize the SKB before transmission.
>
> Return NETDEV_TX_BUSY only for -ENOSPC from mana_gd_post_work_request(),
> send other errors to free_sgl_ptr to free resources and record the tx
> drop.
>
> Signed-off-by: Aditya Garg <gargaditya@linux.microsoft.com>
> Reviewed-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
> ---
> drivers/net/ethernet/microsoft/mana/mana_en.c | 26 +++++++++++++++----
> include/net/mana/gdma.h | 8 +++++-
> include/net/mana/mana.h | 1 +
> 3 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index f4fc86f20213..22605753ca84 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -20,6 +20,7 @@
>
> #include <net/mana/mana.h>
> #include <net/mana/mana_auxiliary.h>
> +#include <linux/skbuff.h>
>
> static DEFINE_IDA(mana_adev_ida);
>
> @@ -289,6 +290,19 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> cq = &apc->tx_qp[txq_idx].tx_cq;
> tx_stats = &txq->stats;
>
> + BUILD_BUG_ON(MAX_TX_WQE_SGL_ENTRIES != MANA_MAX_TX_WQE_SGL_ENTRIES);
> + #if (MAX_SKB_FRAGS + 2 > MANA_MAX_TX_WQE_SGL_ENTRIES)
Hi Aditya,
I see that Eric has made a more substantial review of this patch,
so please follow his advice.
But I wanted to add something to keep in mind for the future: I if the #if
/ #else used here can be replaced by a simple if() statement, then that
would be preferable. The advantage being that it improves compile
coverage. And, as these are all constants, I would expect the compiler to
optimise away any unused code.
N.B: I did not check, so please consider this more of a general statement
> + if (skb_shinfo(skb)->nr_frags + 2 > MANA_MAX_TX_WQE_SGL_ENTRIES) {
> + netdev_info_once(ndev,
> + "nr_frags %d exceeds max supported sge limit. Attempting skb_linearize\n",
> + skb_shinfo(skb)->nr_frags);
> + if (skb_linearize(skb)) {
> + netdev_warn_once(ndev, "Failed to linearize skb\n");
> + goto tx_drop_count;
> + }
> + }
> + #endif
> +
> pkg.tx_oob.s_oob.vcq_num = cq->gdma_id;
> pkg.tx_oob.s_oob.vsq_frame = txq->vsq_frame;
>
...
^ permalink raw reply
* RE: [PATCH v4 4/5] mshv: Allocate vp state page for HVCALL_MAP_VP_STATE_PAGE on L1VH
From: Michael Kelley @ 2025-10-04 15:25 UTC (permalink / raw)
To: Michael Kelley, Nuno Das Neves, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, prapal@linux.microsoft.com,
easwar.hariharan@linux.microsoft.com, tiala@microsoft.com,
anirudh@anirudhrb.com, paekkaladevi@linux.microsoft.com,
skinsburskii@linux.microsoft.com
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, Jinank Jain
In-Reply-To: <SN6PR02MB4157BB0D3B4B8A616C8B2E29D4E7A@SN6PR02MB4157.namprd02.prod.outlook.com>
From: Michael Kelley <mhklinux@outlook.com> Sent: Wednesday, October 1, 2025 5:03 PM
>
> From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Wednesday, October 1, 2025 3:56 PM
> >
> > On 9/29/2025 10:56 AM, Michael Kelley wrote:
> > > From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Friday, September 26, 2025 9:23 AM
> > >>
> > >> From: Jinank Jain <jinankjain@linux.microsoft.com>
> > >>
> > >> Introduce mshv_use_overlay_gpfn() to check if a page needs to be
> > >> allocated and passed to the hypervisor to map VP state pages. This is
> > >> only needed on L1VH, and only on some (newer) versions of the
> > >> hypervisor, hence the need to check vmm_capabilities.
> > >>
> > >> Introduce functions hv_map/unmap_vp_state_page() to handle the
> > >> allocation and freeing.
> > >>
> > >> Signed-off-by: Jinank Jain <jinankjain@linux.microsoft.com>
> > >> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> > >> Reviewed-by: Praveen K Paladugu <prapal@linux.microsoft.com>
> > >> Reviewed-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>
> > >> Reviewed-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
> > >> Reviewed-by: Anirudh Rayabharam <anirudh@anirudhrb.com>
> > >> ---
> > >> drivers/hv/mshv_root.h | 11 ++---
> > >> drivers/hv/mshv_root_hv_call.c | 61 ++++++++++++++++++++++++---
> > >> drivers/hv/mshv_root_main.c | 76 +++++++++++++++++-----------------
> > >> 3 files changed, 98 insertions(+), 50 deletions(-)
> > >>
> > >> diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
> > >> index 0cb1e2589fe1..dbe2d1d0b22f 100644
> > >> --- a/drivers/hv/mshv_root.h
> > >> +++ b/drivers/hv/mshv_root.h
> > >> @@ -279,11 +279,12 @@ int hv_call_set_vp_state(u32 vp_index, u64 partition_id,
> > >> /* Choose between pages and bytes */
> > >> struct hv_vp_state_data state_data, u64 page_count,
> > >> struct page **pages, u32 num_bytes, u8 *bytes);
> > >> -int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> > >> - union hv_input_vtl input_vtl,
> > >> - struct page **state_page);
> > >> -int hv_call_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> > >> - union hv_input_vtl input_vtl);
> > >> +int hv_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> > >> + union hv_input_vtl input_vtl,
> > >> + struct page **state_page);
> > >> +int hv_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> > >> + struct page *state_page,
> > >> + union hv_input_vtl input_vtl);
> > >> int hv_call_create_port(u64 port_partition_id, union hv_port_id port_id,
> > >> u64 connection_partition_id, struct hv_port_info *port_info,
> > >> u8 port_vtl, u8 min_connection_vtl, int node);
> > >> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
> > >> index 3fd3cce23f69..98c6278ff151 100644
> > >> --- a/drivers/hv/mshv_root_hv_call.c
> > >> +++ b/drivers/hv/mshv_root_hv_call.c
> > >> @@ -526,9 +526,9 @@ int hv_call_set_vp_state(u32 vp_index, u64 partition_id,
> > >> return ret;
> > >> }
> > >>
> > >> -int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> > >> - union hv_input_vtl input_vtl,
> > >> - struct page **state_page)
> > >> +static int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> > >> + union hv_input_vtl input_vtl,
> > >> + struct page **state_page)
> > >> {
> > >> struct hv_input_map_vp_state_page *input;
> > >> struct hv_output_map_vp_state_page *output;
> > >> @@ -547,7 +547,14 @@ int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> > >> input->type = type;
> > >> input->input_vtl = input_vtl;
> > >>
> > >> - status = hv_do_hypercall(HVCALL_MAP_VP_STATE_PAGE, input, output);
> > >
> > > This function must zero the input area before using it. Otherwise,
> > > flags.map_location_provided is uninitialized when *state_page is NULL. It will
> > > have whatever value was left by the previous user of hyperv_pcpu_input_arg,
> > > potentially producing bizarre results. And there's a reserved field that won't be
> > > set to zero.
> > >
> > Good catch, will add a memset().
> >
> > >> + if (*state_page) {
> > >> + input->flags.map_location_provided = 1;
> > >> + input->requested_map_location =
> > >> + page_to_pfn(*state_page);
> > >
> > > Technically, this should be page_to_hvpfn() since the PFN value is being sent to
> > > Hyper-V. I know root (and L1VH?) partitions must run with the same page size
> > > as the Hyper-V host, but it's better to not leave code buried here that will blow
> > > up if the "same page size requirement" should ever change.
> > >
> > Good point...I could change these calls, but the other way doesn't work, see below.
> >
> > > And after making the hypercall, there's an invocation of pfn_to_page(), which
> > > should account for the same. Unfortunately, there's not an existing hvpfn_to_page()
> > > function.
> > >
> > This seems like a tricky scenario to get right. In the root partition case, the
> > hypervisor allocates the page. That pfn could be some page within a larger Linux page.
> > Converting that to a Linux pfn (naively) means losing the original hvpfn since it gets
> > truncated, which is no good if we want to unmap it later. Also page_address() would
> > give the wrong virtual address.
> >
> > In other words, we'd have to completely change how we track these pages in order to
> > support this scenario, and the same goes for various other hypervisor APIs where the
> > hypervisor does the allocating. I think it's out of scope to try and address that
> > here, even in part, especially since we will be making assumptions about something
> > that may never happen.
>
> OK, yes the hypervisor allocating the page is a problem when Linux tracks it
> as a struct page. I'll agree it's out of current scope to change this.
>
> It makes me think about hv_synic_enable_regs() where the paravisor or hypervisor
> allocates the synic_message_page and synic_event_page. But that case should work
> OK with a regular guest with page size greater than 4K because the pages are tracked
> based on the guest kernel virtual address, not the PFN. So hv_synic_enable_regs()
> should work on ARM64 Linux guests with 64K page size and a paravisor, as well as
> for my postulated root partition with page size greater than 4K.
>
> When it matters, cases where the hypervisor or paravisor allocate pages to give
> to the guest will require careful handling to ensure they work for guest page sizes
> greater than 4K. That's useful information for future consideration. Thanks for the
> discussion.
>
Upon further reflection, I think there's a subtle bug in the case where the
hypervisor supplies the state page. This bug is present in the existing code, and
persists after these patches.
I'm assuming that the hypervisor supplied page is *not* part of the memory
assigned to the root partition when it was created. I.e., the supplied page is part
of the hypervisor's private memory. Or does the root partition Linux "give"
the hypervisor some memory for it to later return as one of these state pages?
Assuming the page did *not* originate in Linux, then the page provided by the
hypervisor doesn't actually have a "struct page" entry in the root partition Linux
instance. Doing pfn_to_page() works because it just does some address
arithmetic, but the resulting "struct page *" value points somewhere off the
end of the root partition's "struct page" array.
Then page_to_virt() is done on this somewhat bogus "struct page *" value.
page_to_virt() also just does some address arithmetic, so it "works". But it
assumes that the "struct page *" input value is good, and that Linux has a valid
virtual-to-physical direct mapping for the physical memory represented by
that input value. Unfortunately, that assumption might not always be true. I
think it works most of the time because Linux uses huge page mappings for
the direct map, and they may extend far enough beyond the root partition's
actual memory to cover this hypervisor page. Or maybe there's something
else going on that I'm not aware of and that allows the current code to work.
So please check my thinking.
The robust fix is to do like hv_synic_enable_regs(), where a page returned
by the hypervisor/paravisor is explicitly mapped in order to have a valid
virtual address in the root partition Linux.
Note that on ARM64, when CONFIG_DEBUG_VIRTUAL is set to catch errors
like this, page_to_virt() does additional checks on its input value, and would
generate a WARN_ON() in this case. x86/x64 does not do the additional checks.
Michael
>
> >
> > >> + }
> > >> +
> > >> + status = hv_do_hypercall(HVCALL_MAP_VP_STATE_PAGE, input,
> > >> + output);
> > >>
> > >> if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY) {
> > >> if (hv_result_success(status))
^ permalink raw reply
* RE: [PATCH v3 1/5] Drivers: hv: Refactor and rename memory region handling functions
From: Michael Kelley @ 2025-10-06 3:08 UTC (permalink / raw)
To: Stanislav Kinsburskii, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com
Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <175942295032.128138.5037691911773684559.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net>
From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Thursday, October 2, 2025 9:36 AM
>
> Simplify and unify memory region management to improve code clarity and
> reliability. Consolidate pinning and invalidation logic, adopt consistent
> naming, and remove redundant checks to reduce complexity.
>
> Enhance documentation and update call sites for maintainability.
>
> Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
> ---
> drivers/hv/mshv_root_main.c | 78 +++++++++++++++++++------------------------
> 1 file changed, 35 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index fa42c40e1e02f..29d0c2c9ae4c8 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
[snip]
> @@ -1264,17 +1248,25 @@ static int mshv_partition_create_region(struct mshv_partition *partition,
> return 0;
> }
>
> -/*
> - * Map guest ram. if snp, make sure to release that from the host first
> - * Side Effects: In case of failure, pages are unpinned when feasible.
> +/**
> + * mshv_prepare_pinned_region - Pin and map memory regions
> + * @region: Pointer to the memory region structure
> + *
> + * This function processes memory regions that are explicitly marked as pinned.
> + * Pinned regions are preallocated, mapped upfront, and do not rely on fault-based
> + * population. The function ensures the region is properly populated, handles
> + * encryption requirements for SNP partitions if applicable, maps the region,
> + * and performs necessary sharing or eviction operations based on the mapping
> + * result.
> + *
> + * Return: 0 on success, negative error code on failure.
> */
> -static int
> -mshv_partition_mem_region_map(struct mshv_mem_region *region)
> +static int mshv_prepare_pinned_region(struct mshv_mem_region *region)
> {
> struct mshv_partition *partition = region->partition;
> int ret;
>
> - ret = mshv_region_populate(region);
> + ret = mshv_region_pin(region);
> if (ret) {
> pt_err(partition, "Failed to populate memory region: %d\n",
Nit: This error message should probably use the new "pin" terminology
instead of "populate".
Michael
^ permalink raw reply
* [PATCH v3 00/21] paravirt: cleanup and reorg
From: Juergen Gross @ 2025-10-06 7:45 UTC (permalink / raw)
To: linux-kernel, x86, linux-hyperv, virtualization, loongarch,
linuxppc-dev, linux-riscv, kvm
Cc: Juergen Gross, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Peter Zijlstra, Will Deacon,
Boqun Feng, Waiman Long, Jiri Kosina, Josh Poimboeuf, Pawan Gupta,
Boris Ostrovsky, xen-devel, Ajay Kaher, Alexey Makhalov,
Broadcom internal kernel review list, Russell King,
Catalin Marinas, Huacai Chen, WANG Xuerui, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt,
Ben Segall, Mel Gorman, Valentin Schneider, linux-arm-kernel,
Paolo Bonzini, Vitaly Kuznetsov, Stefano Stabellini,
Oleksandr Tyshchenko, Daniel Lezcano, Oleg Nesterov
Some cleanups and reorg of paravirt code and headers:
- The first 2 patches should be not controversial at all, as they
remove just some no longer needed #include and struct forward
declarations.
- The 3rd patch is removing CONFIG_PARAVIRT_DEBUG, which IMO has
no real value, as it just changes a crash to a BUG() (the stack
trace will basically be the same). As the maintainer of the main
paravirt user (Xen) I have never seen this crash/BUG() to happen.
- The 4th patch is just a movement of code.
- I don't know for what reason asm/paravirt_api_clock.h was added,
as all archs supporting it do it exactly in the same way. Patch
5 is removing it.
- Patches 6-14 are streamlining the paravirt clock interfaces by
using a common implementation across architectures where possible
and by moving the related code into common sched code, as this is
where it should live.
- Patches 15-20 are more like RFC material preparing the paravirt
infrastructure to support multiple pv_ops function arrays.
As a prerequisite for that it makes life in objtool much easier
with dropping the Xen static initializers of the pv_ops sub-
structures, which is done in patches 15-17.
Patches 18-20 are doing the real preparations for multiple pv_ops
arrays and using those arrays in multiple headers.
- Patch 21 is an example how the new scheme can look like using the
PV-spinlocks.
Changes in V2:
- new patches 13-18 and 20
- complete rework of patch 21
Changes in V3:
- fixed 2 issues detected by kernel test robot
Juergen Gross (21):
x86/paravirt: Remove not needed includes of paravirt.h
x86/paravirt: Remove some unneeded struct declarations
x86/paravirt: Remove PARAVIRT_DEBUG config option
x86/paravirt: Move thunk macros to paravirt_types.h
paravirt: Remove asm/paravirt_api_clock.h
sched: Move clock related paravirt code to kernel/sched
arm/paravirt: Use common code for paravirt_steal_clock()
arm64/paravirt: Use common code for paravirt_steal_clock()
loongarch/paravirt: Use common code for paravirt_steal_clock()
riscv/paravirt: Use common code for paravirt_steal_clock()
x86/paravirt: Use common code for paravirt_steal_clock()
x86/paravirt: Move paravirt_sched_clock() related code into tsc.c
x86/paravirt: Introduce new paravirt-base.h header
x86/paravirt: Move pv_native_*() prototypes to paravirt.c
x86/xen: Drop xen_irq_ops
x86/xen: Drop xen_cpu_ops
x86/xen: Drop xen_mmu_ops
objtool: Allow multiple pv_ops arrays
x86/paravirt: Allow pv-calls outside paravirt.h
x86/paravirt: Specify pv_ops array in paravirt macros
x86/pvlocks: Move paravirt spinlock functions into own header
arch/Kconfig | 3 +
arch/arm/Kconfig | 1 +
arch/arm/include/asm/paravirt.h | 22 --
arch/arm/include/asm/paravirt_api_clock.h | 1 -
arch/arm/kernel/Makefile | 1 -
arch/arm/kernel/paravirt.c | 23 --
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/paravirt.h | 14 -
arch/arm64/include/asm/paravirt_api_clock.h | 1 -
arch/arm64/kernel/paravirt.c | 11 +-
arch/loongarch/Kconfig | 1 +
arch/loongarch/include/asm/paravirt.h | 13 -
.../include/asm/paravirt_api_clock.h | 1 -
arch/loongarch/kernel/paravirt.c | 10 +-
arch/powerpc/include/asm/paravirt.h | 3 -
arch/powerpc/include/asm/paravirt_api_clock.h | 2 -
arch/powerpc/platforms/pseries/setup.c | 4 +-
arch/riscv/Kconfig | 1 +
arch/riscv/include/asm/paravirt.h | 14 -
arch/riscv/include/asm/paravirt_api_clock.h | 1 -
arch/riscv/kernel/paravirt.c | 11 +-
arch/x86/Kconfig | 8 +-
arch/x86/entry/entry_64.S | 1 -
arch/x86/entry/vsyscall/vsyscall_64.c | 1 -
arch/x86/hyperv/hv_spinlock.c | 11 +-
arch/x86/include/asm/apic.h | 4 -
arch/x86/include/asm/highmem.h | 1 -
arch/x86/include/asm/mshyperv.h | 1 -
arch/x86/include/asm/paravirt-base.h | 29 ++
arch/x86/include/asm/paravirt-spinlock.h | 146 ++++++++
arch/x86/include/asm/paravirt.h | 331 +++++-------------
arch/x86/include/asm/paravirt_api_clock.h | 1 -
arch/x86/include/asm/paravirt_types.h | 269 +++++++-------
arch/x86/include/asm/pgtable_32.h | 1 -
arch/x86/include/asm/ptrace.h | 2 +-
arch/x86/include/asm/qspinlock.h | 89 +----
arch/x86/include/asm/spinlock.h | 1 -
arch/x86/include/asm/timer.h | 1 +
arch/x86/include/asm/tlbflush.h | 4 -
arch/x86/kernel/Makefile | 2 +-
arch/x86/kernel/apm_32.c | 1 -
arch/x86/kernel/callthunks.c | 1 -
arch/x86/kernel/cpu/bugs.c | 1 -
arch/x86/kernel/cpu/vmware.c | 1 +
arch/x86/kernel/kvm.c | 11 +-
arch/x86/kernel/kvmclock.c | 1 +
arch/x86/kernel/paravirt-spinlocks.c | 26 +-
arch/x86/kernel/paravirt.c | 42 +--
arch/x86/kernel/tsc.c | 10 +-
arch/x86/kernel/vsmp_64.c | 1 -
arch/x86/kernel/x86_init.c | 1 -
arch/x86/lib/cache-smp.c | 1 -
arch/x86/mm/init.c | 1 -
arch/x86/xen/enlighten_pv.c | 82 ++---
arch/x86/xen/irq.c | 20 +-
arch/x86/xen/mmu_pv.c | 100 ++----
arch/x86/xen/spinlock.c | 11 +-
arch/x86/xen/time.c | 2 +
drivers/clocksource/hyperv_timer.c | 2 +
drivers/xen/time.c | 2 +-
include/linux/sched/cputime.h | 18 +
kernel/sched/core.c | 5 +
kernel/sched/cputime.c | 13 +
kernel/sched/sched.h | 3 +-
tools/objtool/arch/x86/decode.c | 8 +-
tools/objtool/check.c | 78 ++++-
tools/objtool/include/objtool/check.h | 2 +
67 files changed, 659 insertions(+), 827 deletions(-)
delete mode 100644 arch/arm/include/asm/paravirt.h
delete mode 100644 arch/arm/include/asm/paravirt_api_clock.h
delete mode 100644 arch/arm/kernel/paravirt.c
delete mode 100644 arch/arm64/include/asm/paravirt_api_clock.h
delete mode 100644 arch/loongarch/include/asm/paravirt_api_clock.h
delete mode 100644 arch/powerpc/include/asm/paravirt_api_clock.h
delete mode 100644 arch/riscv/include/asm/paravirt_api_clock.h
create mode 100644 arch/x86/include/asm/paravirt-base.h
create mode 100644 arch/x86/include/asm/paravirt-spinlock.h
delete mode 100644 arch/x86/include/asm/paravirt_api_clock.h
--
2.51.0
^ permalink raw reply
* [PATCH v3 01/21] x86/paravirt: Remove not needed includes of paravirt.h
From: Juergen Gross @ 2025-10-06 7:45 UTC (permalink / raw)
To: linux-kernel, x86, linux-hyperv
Cc: Juergen Gross, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Peter Zijlstra, Will Deacon,
Boqun Feng, Waiman Long, Jiri Kosina, Josh Poimboeuf, Pawan Gupta,
Boris Ostrovsky, xen-devel
In-Reply-To: <20251006074606.1266-1-jgross@suse.com>
In some places asm/paravirt.h is included without really being needed.
Remove the related #include statements.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
V3:
- reinstate the include in mmu_context.h (kernel test robot)
---
arch/x86/entry/entry_64.S | 1 -
arch/x86/entry/vsyscall/vsyscall_64.c | 1 -
arch/x86/hyperv/hv_spinlock.c | 1 -
arch/x86/include/asm/apic.h | 4 ----
arch/x86/include/asm/highmem.h | 1 -
arch/x86/include/asm/mshyperv.h | 1 -
arch/x86/include/asm/pgtable_32.h | 1 -
arch/x86/include/asm/spinlock.h | 1 -
arch/x86/include/asm/tlbflush.h | 4 ----
arch/x86/kernel/apm_32.c | 1 -
arch/x86/kernel/callthunks.c | 1 -
arch/x86/kernel/cpu/bugs.c | 1 -
arch/x86/kernel/vsmp_64.c | 1 -
arch/x86/kernel/x86_init.c | 1 -
arch/x86/lib/cache-smp.c | 1 -
arch/x86/mm/init.c | 1 -
arch/x86/xen/spinlock.c | 1 -
17 files changed, 23 deletions(-)
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index ed04a968cc7d..7a82305405af 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -30,7 +30,6 @@
#include <asm/hw_irq.h>
#include <asm/page_types.h>
#include <asm/irqflags.h>
-#include <asm/paravirt.h>
#include <asm/percpu.h>
#include <asm/asm.h>
#include <asm/smap.h>
diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
index 6e6c0a740837..4bd1e271bb22 100644
--- a/arch/x86/entry/vsyscall/vsyscall_64.c
+++ b/arch/x86/entry/vsyscall/vsyscall_64.c
@@ -37,7 +37,6 @@
#include <asm/unistd.h>
#include <asm/fixmap.h>
#include <asm/traps.h>
-#include <asm/paravirt.h>
#define CREATE_TRACE_POINTS
#include "vsyscall_trace.h"
diff --git a/arch/x86/hyperv/hv_spinlock.c b/arch/x86/hyperv/hv_spinlock.c
index 81b006601370..2a3c2afb0154 100644
--- a/arch/x86/hyperv/hv_spinlock.c
+++ b/arch/x86/hyperv/hv_spinlock.c
@@ -13,7 +13,6 @@
#include <linux/spinlock.h>
#include <asm/mshyperv.h>
-#include <asm/paravirt.h>
#include <asm/apic.h>
#include <asm/msr.h>
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index a26e66d66444..9cd493d467d4 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -90,10 +90,6 @@ static inline bool apic_from_smp_config(void)
/*
* Basic functions accessing APICs.
*/
-#ifdef CONFIG_PARAVIRT
-#include <asm/paravirt.h>
-#endif
-
static inline void native_apic_mem_write(u32 reg, u32 v)
{
volatile u32 *addr = (volatile u32 *)(APIC_BASE + reg);
diff --git a/arch/x86/include/asm/highmem.h b/arch/x86/include/asm/highmem.h
index 585bdadba47d..decfaaf52326 100644
--- a/arch/x86/include/asm/highmem.h
+++ b/arch/x86/include/asm/highmem.h
@@ -24,7 +24,6 @@
#include <linux/interrupt.h>
#include <linux/threads.h>
#include <asm/tlbflush.h>
-#include <asm/paravirt.h>
#include <asm/fixmap.h>
#include <asm/pgtable_areas.h>
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index abc4659f5809..a9ab46fcb6a1 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -7,7 +7,6 @@
#include <linux/msi.h>
#include <linux/io.h>
#include <asm/nospec-branch.h>
-#include <asm/paravirt.h>
#include <asm/msr.h>
#include <hyperv/hvhdk.h>
diff --git a/arch/x86/include/asm/pgtable_32.h b/arch/x86/include/asm/pgtable_32.h
index b612cc57a4d3..acea0cfa2460 100644
--- a/arch/x86/include/asm/pgtable_32.h
+++ b/arch/x86/include/asm/pgtable_32.h
@@ -16,7 +16,6 @@
#ifndef __ASSEMBLER__
#include <asm/processor.h>
#include <linux/threads.h>
-#include <asm/paravirt.h>
#include <linux/bitops.h>
#include <linux/list.h>
diff --git a/arch/x86/include/asm/spinlock.h b/arch/x86/include/asm/spinlock.h
index 5b6bc7016c22..934632b78d09 100644
--- a/arch/x86/include/asm/spinlock.h
+++ b/arch/x86/include/asm/spinlock.h
@@ -7,7 +7,6 @@
#include <asm/page.h>
#include <asm/processor.h>
#include <linux/compiler.h>
-#include <asm/paravirt.h>
#include <asm/bitops.h>
/*
diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 00daedfefc1b..238a6b807da5 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -300,10 +300,6 @@ static inline void mm_clear_asid_transition(struct mm_struct *mm) { }
static inline bool mm_in_asid_transition(struct mm_struct *mm) { return false; }
#endif /* CONFIG_BROADCAST_TLB_FLUSH */
-#ifdef CONFIG_PARAVIRT
-#include <asm/paravirt.h>
-#endif
-
#define flush_tlb_mm(mm) \
flush_tlb_mm_range(mm, 0UL, TLB_FLUSH_ALL, 0UL, true)
diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
index b37ab1095707..3175d7c134e9 100644
--- a/arch/x86/kernel/apm_32.c
+++ b/arch/x86/kernel/apm_32.c
@@ -229,7 +229,6 @@
#include <linux/uaccess.h>
#include <asm/desc.h>
#include <asm/olpc.h>
-#include <asm/paravirt.h>
#include <asm/reboot.h>
#include <asm/nospec-branch.h>
#include <asm/ibt.h>
diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c
index a951333c5995..e37728f70322 100644
--- a/arch/x86/kernel/callthunks.c
+++ b/arch/x86/kernel/callthunks.c
@@ -15,7 +15,6 @@
#include <asm/insn.h>
#include <asm/kexec.h>
#include <asm/nospec-branch.h>
-#include <asm/paravirt.h>
#include <asm/sections.h>
#include <asm/switch_to.h>
#include <asm/sync_core.h>
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 6a526ae1fe99..1f0d67f97788 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -25,7 +25,6 @@
#include <asm/fpu/api.h>
#include <asm/msr.h>
#include <asm/vmx.h>
-#include <asm/paravirt.h>
#include <asm/cpu_device_id.h>
#include <asm/e820/api.h>
#include <asm/hypervisor.h>
diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 73511332bb67..25625e3fc183 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -18,7 +18,6 @@
#include <asm/apic.h>
#include <asm/pci-direct.h>
#include <asm/io.h>
-#include <asm/paravirt.h>
#include <asm/setup.h>
#define TOPOLOGY_REGISTER_OFFSET 0x10
diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
index 0a2bbd674a6d..02ca90378bf9 100644
--- a/arch/x86/kernel/x86_init.c
+++ b/arch/x86/kernel/x86_init.c
@@ -12,7 +12,6 @@
#include <asm/acpi.h>
#include <asm/bios_ebda.h>
-#include <asm/paravirt.h>
#include <asm/pci_x86.h>
#include <asm/mpspec.h>
#include <asm/setup.h>
diff --git a/arch/x86/lib/cache-smp.c b/arch/x86/lib/cache-smp.c
index c5c60d07308c..ae5a5dfd33c7 100644
--- a/arch/x86/lib/cache-smp.c
+++ b/arch/x86/lib/cache-smp.c
@@ -1,5 +1,4 @@
// SPDX-License-Identifier: GPL-2.0
-#include <asm/paravirt.h>
#include <linux/smp.h>
#include <linux/export.h>
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 8bf6ad4b9400..76537d40493c 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -27,7 +27,6 @@
#include <asm/pti.h>
#include <asm/text-patching.h>
#include <asm/memtype.h>
-#include <asm/paravirt.h>
#include <asm/mmu_context.h>
/*
diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 8e4efe0fb6f9..fe56646d6919 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -8,7 +8,6 @@
#include <linux/slab.h>
#include <linux/atomic.h>
-#include <asm/paravirt.h>
#include <asm/qspinlock.h>
#include <xen/events.h>
--
2.51.0
^ permalink raw reply related
* [PATCH v3 12/21] x86/paravirt: Move paravirt_sched_clock() related code into tsc.c
From: Juergen Gross @ 2025-10-06 7:45 UTC (permalink / raw)
To: linux-kernel, x86, virtualization, kvm, linux-hyperv
Cc: Juergen Gross, Ajay Kaher, Alexey Makhalov,
Broadcom internal kernel review list, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin,
Paolo Bonzini, Vitaly Kuznetsov, Boris Ostrovsky,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Daniel Lezcano, xen-devel, Peter Zijlstra (Intel)
In-Reply-To: <20251006074606.1266-1-jgross@suse.com>
The only user of paravirt_sched_clock() is in tsc.c, so move the code
from paravirt.c and paravirt.h to tsc.c.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/x86/include/asm/paravirt.h | 12 ------------
arch/x86/include/asm/timer.h | 1 +
arch/x86/kernel/kvmclock.c | 1 +
arch/x86/kernel/paravirt.c | 7 -------
arch/x86/kernel/tsc.c | 10 +++++++++-
arch/x86/xen/time.c | 1 +
drivers/clocksource/hyperv_timer.c | 2 ++
7 files changed, 14 insertions(+), 20 deletions(-)
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 766a7cee3d64..b69e75a5c872 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -14,20 +14,8 @@
#ifndef __ASSEMBLER__
#include <linux/types.h>
#include <linux/cpumask.h>
-#include <linux/static_call_types.h>
#include <asm/frame.h>
-u64 dummy_sched_clock(void);
-
-DECLARE_STATIC_CALL(pv_sched_clock, dummy_sched_clock);
-
-void paravirt_set_sched_clock(u64 (*func)(void));
-
-static __always_inline u64 paravirt_sched_clock(void)
-{
- return static_call(pv_sched_clock)();
-}
-
__visible void __native_queued_spin_unlock(struct qspinlock *lock);
bool pv_is_native_spin_unlock(void);
__visible bool __native_vcpu_is_preempted(long cpu);
diff --git a/arch/x86/include/asm/timer.h b/arch/x86/include/asm/timer.h
index 23baf8c9b34c..fda18bcb19b4 100644
--- a/arch/x86/include/asm/timer.h
+++ b/arch/x86/include/asm/timer.h
@@ -12,6 +12,7 @@ extern void recalibrate_cpu_khz(void);
extern int no_timer_check;
extern bool using_native_sched_clock(void);
+void paravirt_set_sched_clock(u64 (*func)(void));
/*
* We use the full linear equation: f(x) = a + b*x, in order to allow
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index ca0a49eeac4a..b5991d53fc0e 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -19,6 +19,7 @@
#include <linux/cc_platform.h>
#include <asm/hypervisor.h>
+#include <asm/timer.h>
#include <asm/x86_init.h>
#include <asm/kvmclock.h>
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 42991d471bf3..4e37db8073f9 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -60,13 +60,6 @@ void __init native_pv_lock_init(void)
static_branch_enable(&virt_spin_lock_key);
}
-DEFINE_STATIC_CALL(pv_sched_clock, native_sched_clock);
-
-void paravirt_set_sched_clock(u64 (*func)(void))
-{
- static_call_update(pv_sched_clock, func);
-}
-
static noinstr void pv_native_safe_halt(void)
{
native_safe_halt();
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 87e749106dda..554b54783a04 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -266,19 +266,27 @@ u64 native_sched_clock_from_tsc(u64 tsc)
/* We need to define a real function for sched_clock, to override the
weak default version */
#ifdef CONFIG_PARAVIRT
+DEFINE_STATIC_CALL(pv_sched_clock, native_sched_clock);
+
noinstr u64 sched_clock_noinstr(void)
{
- return paravirt_sched_clock();
+ return static_call(pv_sched_clock)();
}
bool using_native_sched_clock(void)
{
return static_call_query(pv_sched_clock) == native_sched_clock;
}
+
+void paravirt_set_sched_clock(u64 (*func)(void))
+{
+ static_call_update(pv_sched_clock, func);
+}
#else
u64 sched_clock_noinstr(void) __attribute__((alias("native_sched_clock")));
bool using_native_sched_clock(void) { return true; }
+void paravirt_set_sched_clock(u64 (*func)(void)) { }
#endif
notrace u64 sched_clock(void)
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index e4754b2fa900..6f9f665bb7ae 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -19,6 +19,7 @@
#include <linux/sched/cputime.h>
#include <asm/pvclock.h>
+#include <asm/timer.h>
#include <asm/xen/hypervisor.h>
#include <asm/xen/hypercall.h>
#include <asm/xen/cpuid.h>
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index 2edc13ca184e..6397a7ba4a98 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -535,6 +535,8 @@ static __always_inline void hv_setup_sched_clock(void *sched_clock)
sched_clock_register(sched_clock, 64, NSEC_PER_SEC);
}
#elif defined CONFIG_PARAVIRT
+#include <asm/timer.h>
+
static __always_inline void hv_setup_sched_clock(void *sched_clock)
{
/* We're on x86/x64 *and* using PV ops */
--
2.51.0
^ permalink raw reply related
* [PATCH v3 21/21] x86/pvlocks: Move paravirt spinlock functions into own header
From: Juergen Gross @ 2025-10-06 7:46 UTC (permalink / raw)
To: linux-kernel, x86, linux-hyperv, virtualization, kvm
Cc: Juergen Gross, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Ajay Kaher, Alexey Makhalov,
Broadcom internal kernel review list, Paolo Bonzini,
Vitaly Kuznetsov, Boris Ostrovsky, Josh Poimboeuf, Peter Zijlstra,
xen-devel
In-Reply-To: <20251006074606.1266-1-jgross@suse.com>
Instead of having the pv spinlock function definitions in paravirt.h,
move them into the new header paravirt-spinlock.h.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- use new header instead of qspinlock.h
- use dedicated pv_ops_lock array
- move more paravirt related lock code
V3:
- hide native_pv_lock_init() with CONFIG_SMP (kernel test robot)
---
arch/x86/hyperv/hv_spinlock.c | 10 +-
arch/x86/include/asm/paravirt-spinlock.h | 146 +++++++++++++++++++++++
arch/x86/include/asm/paravirt.h | 61 ----------
arch/x86/include/asm/paravirt_types.h | 17 ---
arch/x86/include/asm/qspinlock.h | 89 ++------------
arch/x86/kernel/Makefile | 2 +-
arch/x86/kernel/kvm.c | 10 +-
arch/x86/kernel/paravirt-spinlocks.c | 26 +++-
arch/x86/kernel/paravirt.c | 21 ----
arch/x86/xen/spinlock.c | 10 +-
tools/objtool/check.c | 1 +
11 files changed, 194 insertions(+), 199 deletions(-)
create mode 100644 arch/x86/include/asm/paravirt-spinlock.h
diff --git a/arch/x86/hyperv/hv_spinlock.c b/arch/x86/hyperv/hv_spinlock.c
index 2a3c2afb0154..210b494e4de0 100644
--- a/arch/x86/hyperv/hv_spinlock.c
+++ b/arch/x86/hyperv/hv_spinlock.c
@@ -78,11 +78,11 @@ void __init hv_init_spinlocks(void)
pr_info("PV spinlocks enabled\n");
__pv_init_lock_hash();
- pv_ops.lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
- pv_ops.lock.queued_spin_unlock = PV_CALLEE_SAVE(__pv_queued_spin_unlock);
- pv_ops.lock.wait = hv_qlock_wait;
- pv_ops.lock.kick = hv_qlock_kick;
- pv_ops.lock.vcpu_is_preempted = PV_CALLEE_SAVE(hv_vcpu_is_preempted);
+ pv_ops_lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
+ pv_ops_lock.queued_spin_unlock = PV_CALLEE_SAVE(__pv_queued_spin_unlock);
+ pv_ops_lock.wait = hv_qlock_wait;
+ pv_ops_lock.kick = hv_qlock_kick;
+ pv_ops_lock.vcpu_is_preempted = PV_CALLEE_SAVE(hv_vcpu_is_preempted);
}
static __init int hv_parse_nopvspin(char *arg)
diff --git a/arch/x86/include/asm/paravirt-spinlock.h b/arch/x86/include/asm/paravirt-spinlock.h
new file mode 100644
index 000000000000..ed3ed343903d
--- /dev/null
+++ b/arch/x86/include/asm/paravirt-spinlock.h
@@ -0,0 +1,146 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _ASM_X86_PARAVIRT_SPINLOCK_H
+#define _ASM_X86_PARAVIRT_SPINLOCK_H
+
+#include <asm/paravirt_types.h>
+
+#ifdef CONFIG_SMP
+#include <asm/spinlock_types.h>
+#endif
+
+struct qspinlock;
+
+struct pv_lock_ops {
+ void (*queued_spin_lock_slowpath)(struct qspinlock *lock, u32 val);
+ struct paravirt_callee_save queued_spin_unlock;
+
+ void (*wait)(u8 *ptr, u8 val);
+ void (*kick)(int cpu);
+
+ struct paravirt_callee_save vcpu_is_preempted;
+} __no_randomize_layout;
+
+extern struct pv_lock_ops pv_ops_lock;
+
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
+void __init paravirt_set_cap(void);
+extern void native_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
+extern void __pv_init_lock_hash(void);
+extern void __pv_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
+extern void __raw_callee_save___pv_queued_spin_unlock(struct qspinlock *lock);
+extern bool nopvspin;
+
+static __always_inline void pv_queued_spin_lock_slowpath(struct qspinlock *lock,
+ u32 val)
+{
+ PVOP_VCALL2(pv_ops_lock, queued_spin_lock_slowpath, lock, val);
+}
+
+static __always_inline void pv_queued_spin_unlock(struct qspinlock *lock)
+{
+ PVOP_ALT_VCALLEE1(pv_ops_lock, queued_spin_unlock, lock,
+ "movb $0, (%%" _ASM_ARG1 ");",
+ ALT_NOT(X86_FEATURE_PVUNLOCK));
+}
+
+static __always_inline bool pv_vcpu_is_preempted(long cpu)
+{
+ return PVOP_ALT_CALLEE1(bool, pv_ops_lock, vcpu_is_preempted, cpu,
+ "xor %%" _ASM_AX ", %%" _ASM_AX ";",
+ ALT_NOT(X86_FEATURE_VCPUPREEMPT));
+}
+
+#define queued_spin_unlock queued_spin_unlock
+/**
+ * queued_spin_unlock - release a queued spinlock
+ * @lock : Pointer to queued spinlock structure
+ *
+ * A smp_store_release() on the least-significant byte.
+ */
+static inline void native_queued_spin_unlock(struct qspinlock *lock)
+{
+ smp_store_release(&lock->locked, 0);
+}
+
+static inline void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
+{
+ pv_queued_spin_lock_slowpath(lock, val);
+}
+
+static inline void queued_spin_unlock(struct qspinlock *lock)
+{
+ kcsan_release();
+ pv_queued_spin_unlock(lock);
+}
+
+#define vcpu_is_preempted vcpu_is_preempted
+static inline bool vcpu_is_preempted(long cpu)
+{
+ return pv_vcpu_is_preempted(cpu);
+}
+
+static __always_inline void pv_wait(u8 *ptr, u8 val)
+{
+ PVOP_VCALL2(pv_ops_lock, wait, ptr, val);
+}
+
+static __always_inline void pv_kick(int cpu)
+{
+ PVOP_VCALL1(pv_ops_lock, kick, cpu);
+}
+
+void __raw_callee_save___native_queued_spin_unlock(struct qspinlock *lock);
+bool __raw_callee_save___native_vcpu_is_preempted(long cpu);
+#endif /* CONFIG_PARAVIRT_SPINLOCKS */
+
+void __init native_pv_lock_init(void);
+__visible void __native_queued_spin_unlock(struct qspinlock *lock);
+bool pv_is_native_spin_unlock(void);
+__visible bool __native_vcpu_is_preempted(long cpu);
+bool pv_is_native_vcpu_is_preempted(void);
+
+/*
+ * virt_spin_lock_key - disables by default the virt_spin_lock() hijack.
+ *
+ * Native (and PV wanting native due to vCPU pinning) should keep this key
+ * disabled. Native does not touch the key.
+ *
+ * When in a guest then native_pv_lock_init() enables the key first and
+ * KVM/XEN might conditionally disable it later in the boot process again.
+ */
+DECLARE_STATIC_KEY_FALSE(virt_spin_lock_key);
+
+/*
+ * Shortcut for the queued_spin_lock_slowpath() function that allows
+ * virt to hijack it.
+ *
+ * Returns:
+ * true - lock has been negotiated, all done;
+ * false - queued_spin_lock_slowpath() will do its thing.
+ */
+#define virt_spin_lock virt_spin_lock
+static inline bool virt_spin_lock(struct qspinlock *lock)
+{
+ int val;
+
+ if (!static_branch_likely(&virt_spin_lock_key))
+ return false;
+
+ /*
+ * On hypervisors without PARAVIRT_SPINLOCKS support we fall
+ * back to a Test-and-Set spinlock, because fair locks have
+ * horrible lock 'holder' preemption issues.
+ */
+
+ __retry:
+ val = atomic_read(&lock->val);
+
+ if (val || !atomic_try_cmpxchg(&lock->val, &val, _Q_LOCKED_VAL)) {
+ cpu_relax();
+ goto __retry;
+ }
+
+ return true;
+}
+
+#endif /* _ASM_X86_PARAVIRT_SPINLOCK_H */
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index ec274d13bae0..b21072af731d 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -19,15 +19,6 @@
#include <linux/cpumask.h>
#include <asm/frame.h>
-__visible void __native_queued_spin_unlock(struct qspinlock *lock);
-bool pv_is_native_spin_unlock(void);
-__visible bool __native_vcpu_is_preempted(long cpu);
-bool pv_is_native_vcpu_is_preempted(void);
-
-#ifdef CONFIG_PARAVIRT_SPINLOCKS
-void __init paravirt_set_cap(void);
-#endif
-
/* The paravirtualized I/O functions */
static inline void slow_down_io(void)
{
@@ -522,46 +513,7 @@ static inline void __set_fixmap(unsigned /* enum fixed_addresses */ idx,
{
pv_ops.mmu.set_fixmap(idx, phys, flags);
}
-#endif
-
-#if defined(CONFIG_SMP) && defined(CONFIG_PARAVIRT_SPINLOCKS)
-
-static __always_inline void pv_queued_spin_lock_slowpath(struct qspinlock *lock,
- u32 val)
-{
- PVOP_VCALL2(pv_ops, lock.queued_spin_lock_slowpath, lock, val);
-}
-
-static __always_inline void pv_queued_spin_unlock(struct qspinlock *lock)
-{
- PVOP_ALT_VCALLEE1(pv_ops, lock.queued_spin_unlock, lock,
- "movb $0, (%%" _ASM_ARG1 ");",
- ALT_NOT(X86_FEATURE_PVUNLOCK));
-}
-
-static __always_inline void pv_wait(u8 *ptr, u8 val)
-{
- PVOP_VCALL2(pv_ops, lock.wait, ptr, val);
-}
-
-static __always_inline void pv_kick(int cpu)
-{
- PVOP_VCALL1(pv_ops, lock.kick, cpu);
-}
-
-static __always_inline bool pv_vcpu_is_preempted(long cpu)
-{
- return PVOP_ALT_CALLEE1(bool, pv_ops, lock.vcpu_is_preempted, cpu,
- "xor %%" _ASM_AX ", %%" _ASM_AX ";",
- ALT_NOT(X86_FEATURE_VCPUPREEMPT));
-}
-void __raw_callee_save___native_queued_spin_unlock(struct qspinlock *lock);
-bool __raw_callee_save___native_vcpu_is_preempted(long cpu);
-
-#endif /* SMP && PARAVIRT_SPINLOCKS */
-
-#ifdef CONFIG_PARAVIRT_XXL
static __always_inline unsigned long arch_local_save_flags(void)
{
return PVOP_ALT_CALLEE0(unsigned long, pv_ops, irq.save_fl, "pushf; pop %%rax;",
@@ -588,8 +540,6 @@ static __always_inline unsigned long arch_local_irq_save(void)
}
#endif
-void native_pv_lock_init(void) __init;
-
#else /* __ASSEMBLER__ */
#ifdef CONFIG_X86_64
@@ -613,12 +563,6 @@ void native_pv_lock_init(void) __init;
#endif /* __ASSEMBLER__ */
#else /* CONFIG_PARAVIRT */
# define default_banner x86_init_noop
-
-#ifndef __ASSEMBLER__
-static inline void native_pv_lock_init(void)
-{
-}
-#endif
#endif /* !CONFIG_PARAVIRT */
#ifndef __ASSEMBLER__
@@ -634,10 +578,5 @@ static inline void paravirt_arch_exit_mmap(struct mm_struct *mm)
}
#endif
-#ifndef CONFIG_PARAVIRT_SPINLOCKS
-static inline void paravirt_set_cap(void)
-{
-}
-#endif
#endif /* __ASSEMBLER__ */
#endif /* _ASM_X86_PARAVIRT_H */
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 01a485f1a7f1..e2b487d35d14 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -184,22 +184,6 @@ struct pv_mmu_ops {
#endif
} __no_randomize_layout;
-#ifdef CONFIG_SMP
-#include <asm/spinlock_types.h>
-#endif
-
-struct qspinlock;
-
-struct pv_lock_ops {
- void (*queued_spin_lock_slowpath)(struct qspinlock *lock, u32 val);
- struct paravirt_callee_save queued_spin_unlock;
-
- void (*wait)(u8 *ptr, u8 val);
- void (*kick)(int cpu);
-
- struct paravirt_callee_save vcpu_is_preempted;
-} __no_randomize_layout;
-
/* This contains all the paravirt structures: we get a convenient
* number for each function using the offset which we use to indicate
* what to patch. */
@@ -207,7 +191,6 @@ struct paravirt_patch_template {
struct pv_cpu_ops cpu;
struct pv_irq_ops irq;
struct pv_mmu_ops mmu;
- struct pv_lock_ops lock;
} __no_randomize_layout;
extern struct paravirt_patch_template pv_ops;
diff --git a/arch/x86/include/asm/qspinlock.h b/arch/x86/include/asm/qspinlock.h
index 68da67df304d..a2668bdf4c84 100644
--- a/arch/x86/include/asm/qspinlock.h
+++ b/arch/x86/include/asm/qspinlock.h
@@ -7,6 +7,9 @@
#include <asm-generic/qspinlock_types.h>
#include <asm/paravirt.h>
#include <asm/rmwcc.h>
+#ifdef CONFIG_PARAVIRT
+#include <asm/paravirt-spinlock.h>
+#endif
#define _Q_PENDING_LOOPS (1 << 9)
@@ -27,89 +30,13 @@ static __always_inline u32 queued_fetch_set_pending_acquire(struct qspinlock *lo
return val;
}
-#ifdef CONFIG_PARAVIRT_SPINLOCKS
-extern void native_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
-extern void __pv_init_lock_hash(void);
-extern void __pv_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
-extern void __raw_callee_save___pv_queued_spin_unlock(struct qspinlock *lock);
-extern bool nopvspin;
-
-#define queued_spin_unlock queued_spin_unlock
-/**
- * queued_spin_unlock - release a queued spinlock
- * @lock : Pointer to queued spinlock structure
- *
- * A smp_store_release() on the least-significant byte.
- */
-static inline void native_queued_spin_unlock(struct qspinlock *lock)
-{
- smp_store_release(&lock->locked, 0);
-}
-
-static inline void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
-{
- pv_queued_spin_lock_slowpath(lock, val);
-}
-
-static inline void queued_spin_unlock(struct qspinlock *lock)
-{
- kcsan_release();
- pv_queued_spin_unlock(lock);
-}
-
-#define vcpu_is_preempted vcpu_is_preempted
-static inline bool vcpu_is_preempted(long cpu)
-{
- return pv_vcpu_is_preempted(cpu);
-}
+#ifndef CONFIG_PARAVIRT_SPINLOCKS
+static inline void paravirt_set_cap(void) { }
#endif
-#ifdef CONFIG_PARAVIRT
-/*
- * virt_spin_lock_key - disables by default the virt_spin_lock() hijack.
- *
- * Native (and PV wanting native due to vCPU pinning) should keep this key
- * disabled. Native does not touch the key.
- *
- * When in a guest then native_pv_lock_init() enables the key first and
- * KVM/XEN might conditionally disable it later in the boot process again.
- */
-DECLARE_STATIC_KEY_FALSE(virt_spin_lock_key);
-
-/*
- * Shortcut for the queued_spin_lock_slowpath() function that allows
- * virt to hijack it.
- *
- * Returns:
- * true - lock has been negotiated, all done;
- * false - queued_spin_lock_slowpath() will do its thing.
- */
-#define virt_spin_lock virt_spin_lock
-static inline bool virt_spin_lock(struct qspinlock *lock)
-{
- int val;
-
- if (!static_branch_likely(&virt_spin_lock_key))
- return false;
-
- /*
- * On hypervisors without PARAVIRT_SPINLOCKS support we fall
- * back to a Test-and-Set spinlock, because fair locks have
- * horrible lock 'holder' preemption issues.
- */
-
- __retry:
- val = atomic_read(&lock->val);
-
- if (val || !atomic_try_cmpxchg(&lock->val, &val, _Q_LOCKED_VAL)) {
- cpu_relax();
- goto __retry;
- }
-
- return true;
-}
-
-#endif /* CONFIG_PARAVIRT */
+#ifndef CONFIG_PARAVIRT
+static inline void native_pv_lock_init(void) { }
+#endif
#include <asm-generic/qspinlock.h>
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index bc184dd38d99..e9aeeeafad17 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -126,7 +126,7 @@ obj-$(CONFIG_DEBUG_NMI_SELFTEST) += nmi_selftest.o
obj-$(CONFIG_KVM_GUEST) += kvm.o kvmclock.o
obj-$(CONFIG_PARAVIRT) += paravirt.o
-obj-$(CONFIG_PARAVIRT_SPINLOCKS)+= paravirt-spinlocks.o
+obj-$(CONFIG_PARAVIRT) += paravirt-spinlocks.o
obj-$(CONFIG_PARAVIRT_CLOCK) += pvclock.o
obj-$(CONFIG_X86_PMEM_LEGACY_DEVICE) += pmem.o
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index d54fd2bc0402..47426538b579 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -824,7 +824,7 @@ static void __init kvm_guest_init(void)
has_steal_clock = 1;
static_call_update(pv_steal_clock, kvm_steal_clock);
- pv_ops.lock.vcpu_is_preempted =
+ pv_ops_lock.vcpu_is_preempted =
PV_CALLEE_SAVE(__kvm_vcpu_is_preempted);
}
@@ -1121,11 +1121,11 @@ void __init kvm_spinlock_init(void)
pr_info("PV spinlocks enabled\n");
__pv_init_lock_hash();
- pv_ops.lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
- pv_ops.lock.queued_spin_unlock =
+ pv_ops_lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
+ pv_ops_lock.queued_spin_unlock =
PV_CALLEE_SAVE(__pv_queued_spin_unlock);
- pv_ops.lock.wait = kvm_wait;
- pv_ops.lock.kick = kvm_kick_cpu;
+ pv_ops_lock.wait = kvm_wait;
+ pv_ops_lock.kick = kvm_kick_cpu;
/*
* When PV spinlock is enabled which is preferred over
diff --git a/arch/x86/kernel/paravirt-spinlocks.c b/arch/x86/kernel/paravirt-spinlocks.c
index 9e1ea99ad9df..95452444868f 100644
--- a/arch/x86/kernel/paravirt-spinlocks.c
+++ b/arch/x86/kernel/paravirt-spinlocks.c
@@ -3,12 +3,22 @@
* Split spinlock implementation out into its own file, so it can be
* compiled in a FTRACE-compatible way.
*/
+#include <linux/static_call.h>
#include <linux/spinlock.h>
#include <linux/export.h>
#include <linux/jump_label.h>
-#include <asm/paravirt.h>
+DEFINE_STATIC_KEY_FALSE(virt_spin_lock_key);
+#ifdef CONFIG_SMP
+void __init native_pv_lock_init(void)
+{
+ if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
+ static_branch_enable(&virt_spin_lock_key);
+}
+#endif
+
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
__visible void __native_queued_spin_unlock(struct qspinlock *lock)
{
native_queued_spin_unlock(lock);
@@ -17,7 +27,7 @@ PV_CALLEE_SAVE_REGS_THUNK(__native_queued_spin_unlock);
bool pv_is_native_spin_unlock(void)
{
- return pv_ops.lock.queued_spin_unlock.func ==
+ return pv_ops_lock.queued_spin_unlock.func ==
__raw_callee_save___native_queued_spin_unlock;
}
@@ -29,7 +39,7 @@ PV_CALLEE_SAVE_REGS_THUNK(__native_vcpu_is_preempted);
bool pv_is_native_vcpu_is_preempted(void)
{
- return pv_ops.lock.vcpu_is_preempted.func ==
+ return pv_ops_lock.vcpu_is_preempted.func ==
__raw_callee_save___native_vcpu_is_preempted;
}
@@ -41,3 +51,13 @@ void __init paravirt_set_cap(void)
if (!pv_is_native_vcpu_is_preempted())
setup_force_cpu_cap(X86_FEATURE_VCPUPREEMPT);
}
+
+struct pv_lock_ops pv_ops_lock = {
+ .queued_spin_lock_slowpath = native_queued_spin_lock_slowpath,
+ .queued_spin_unlock = PV_CALLEE_SAVE(__native_queued_spin_unlock),
+ .wait = paravirt_nop,
+ .kick = paravirt_nop,
+ .vcpu_is_preempted = PV_CALLEE_SAVE(__native_vcpu_is_preempted),
+};
+EXPORT_SYMBOL(pv_ops_lock);
+#endif
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 5dfbd3f55792..a6ed52cae003 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -57,14 +57,6 @@ DEFINE_ASM_FUNC(pv_native_irq_enable, "sti", .noinstr.text);
DEFINE_ASM_FUNC(pv_native_read_cr2, "mov %cr2, %rax", .noinstr.text);
#endif
-DEFINE_STATIC_KEY_FALSE(virt_spin_lock_key);
-
-void __init native_pv_lock_init(void)
-{
- if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
- static_branch_enable(&virt_spin_lock_key);
-}
-
static noinstr void pv_native_safe_halt(void)
{
native_safe_halt();
@@ -221,19 +213,6 @@ struct paravirt_patch_template pv_ops = {
.mmu.set_fixmap = native_set_fixmap,
#endif /* CONFIG_PARAVIRT_XXL */
-
-#if defined(CONFIG_PARAVIRT_SPINLOCKS)
- /* Lock ops. */
-#ifdef CONFIG_SMP
- .lock.queued_spin_lock_slowpath = native_queued_spin_lock_slowpath,
- .lock.queued_spin_unlock =
- PV_CALLEE_SAVE(__native_queued_spin_unlock),
- .lock.wait = paravirt_nop,
- .lock.kick = paravirt_nop,
- .lock.vcpu_is_preempted =
- PV_CALLEE_SAVE(__native_vcpu_is_preempted),
-#endif /* SMP */
-#endif
};
#ifdef CONFIG_PARAVIRT_XXL
diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index fe56646d6919..83ac24ead289 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -134,10 +134,10 @@ void __init xen_init_spinlocks(void)
printk(KERN_DEBUG "xen: PV spinlocks enabled\n");
__pv_init_lock_hash();
- pv_ops.lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
- pv_ops.lock.queued_spin_unlock =
+ pv_ops_lock.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
+ pv_ops_lock.queued_spin_unlock =
PV_CALLEE_SAVE(__pv_queued_spin_unlock);
- pv_ops.lock.wait = xen_qlock_wait;
- pv_ops.lock.kick = xen_qlock_kick;
- pv_ops.lock.vcpu_is_preempted = PV_CALLEE_SAVE(xen_vcpu_stolen);
+ pv_ops_lock.wait = xen_qlock_wait;
+ pv_ops_lock.kick = xen_qlock_kick;
+ pv_ops_lock.vcpu_is_preempted = PV_CALLEE_SAVE(xen_vcpu_stolen);
}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 1675c16c3793..663fa5f281bd 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -549,6 +549,7 @@ static struct {
int idx_off;
} pv_ops_tables[] = {
{ .name = "pv_ops", },
+ { .name = "pv_ops_lock", },
{ .name = NULL, .idx_off = -1 }
};
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] x86/hyperv: Export hv_hypercall_pg unconditionally
From: Naman Jain @ 2025-10-06 10:50 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Sean Christopherson, Paolo Bonzini, Roman Kisel,
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: <20250918064713.GW3419281@noisy.programming.kicks-ass.net>
On 9/18/2025 12:17 PM, Peter Zijlstra wrote:
> On Thu, Sep 18, 2025 at 11:33:18AM +0530, Naman Jain wrote:
>
>> Thank you so much Sean and Paolo for your valuable inputs. I will try
>> out these things. Summarizing the suggestions here:
>> * Use noinstr (no instrumentation)
>> * Have separate .S file
>> * Don't use "register asm".
>> * Use static calls for solving IBT problems
>> * RAX:RCX is probably ok to be used, considering ABI. Whether we would still
>> need to use STACK_FRAME_NON_STANDARD, I am not sure, but I will see based on
>> how it goes.
>>
>> I hope this addresses the concerns Peter raised. If there's anything I might
>> have missed, I'm happy to make further adjustments if needed.
>
> It would be a definite improvement. I'm just *really* sad people still
> create interfaces like this, even though we've known for years how bad
> they are.
>
> At some point we've really have to push back and say enough is enough.
Hi Peter, Paolo, Sean,
I am facing issues in this approach, after moving the assembly code to a
separate file, using static calls, and making it noinstr.
We need to make a call to STATIC_CALL_TRAMP_STR(hv_hypercall_pg +
offset) in the assembly code. This offset is populated at run time in
the driver, so I have to pass this offset to the assembly function via
function parameters or a shared variable. This leaves noinstr section
and results in below warning:
[1]: vmlinux.o: warning: objtool: __mshv_vtl_return_call+0x4f: call to
mshv_vtl_call_addr() leaves .noinstr.text section
To fix this, one of the ways was to avoid making indirect calls. So I
used EXPORT_STATIC_CALL to export the static call *trampoline and key*
for the static call we created in C driver. Then I figured, we could
simply call __SCT__<static_callname> in assembly code and it should work
fine. But then it leads to this error in objtool.
[2]: arch/x86/hyperv/mshv_vtl_asm.o: error: objtool: static_call: can't
find static_call_key symbol: __SCK__mshv_vtl_return_hypercall
This gets hidden/fixed with X86_KERNEL_IBT config enablement.
If I comment out the objtool check that leads to this warning, I can see
the same symbols for __SCT__, __SCK__ in final vmlinux with and without
IBT, and it even works fine on the VM. So it seems to be a matter of
timing when objtool is checking for this symbol.
My theory is IBT enablement is helping here as it adds ENDBR
instructions to various static/indirect/direct calls, which may be
adding the missing symbol to the section before objtool checks for it.
Other ways I tried to fix the initial problem of noinstr was to make the
exported shared pointer variable as noinstr, but it does not work since
noinstr can only be applied to functions and not data types. KVM code
had similar noisntr calls from assembly, but they have actual functions
which are marked noinstr, not just some address stored in a variable.
I also tried of introducing wrapper functions marked as noinstr, to
initialize static calls but it does not help.
Adding compilation dependencies in Makefile also does not help.
$(obj)/mshv_vtl_asm.o: $(obj)/hv_vtl.o
This left me with no option, but to enable IBT and add the dependency.
But yes, this is controversial and if there are alternate ways to handle
this or make it work, I would be more than happy to hear your
suggestions on it and implement them.
Regards,
Naman
^ permalink raw reply
* Re: [PATCH] x86/hyperv: Export hv_hypercall_pg unconditionally
From: Peter Zijlstra @ 2025-10-06 11:10 UTC (permalink / raw)
To: Naman Jain
Cc: Sean Christopherson, Paolo Bonzini, Roman Kisel,
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: <9f8007a3-f810-4b60-8942-e721cd6a32c4@linux.microsoft.com>
On Mon, Oct 06, 2025 at 04:20:03PM +0530, Naman Jain wrote:
> I am facing issues in this approach, after moving the assembly code to a
> separate file, using static calls, and making it noinstr.
>
> We need to make a call to STATIC_CALL_TRAMP_STR(hv_hypercall_pg + offset) in
> the assembly code. This offset is populated at run time in the driver, so I
> have to pass this offset to the assembly function via function parameters or
> a shared variable. This leaves noinstr section and results in below warning:
>
> [1]: vmlinux.o: warning: objtool: __mshv_vtl_return_call+0x4f: call to
> mshv_vtl_call_addr() leaves .noinstr.text section
>
>
> To fix this, one of the ways was to avoid making indirect calls. So I used
> EXPORT_STATIC_CALL to export the static call *trampoline and key* for the
> static call we created in C driver. Then I figured, we could simply call
> __SCT__<static_callname> in assembly code and it should work fine. But then
> it leads to this error in objtool.
Easiest solution is to create a second static_call and have
hv_set_hypercall_pg() set that to +offset.
DEFINE_STATIC_CALL(__hv_vtl_hypercall);
hv_set_hypercall()
...
static_call_update(__hv_vtl_hypercall, ptr+offset); /* +- cast to right function type */
> [2]: arch/x86/hyperv/mshv_vtl_asm.o: error: objtool: static_call: can't find
> static_call_key symbol: __SCK__mshv_vtl_return_hypercall
Look at arch/x86/include/asm/preempt.h
you might need that __STATIC_CALL_MOD_ADDRESSABLE() thing somewhere.
Also, what's actually in that hypercall page that is so magical and
can't just be an ALTERNATIVE() ?
^ permalink raw reply
* Re: [PATCH] x86/hyperv: Export hv_hypercall_pg unconditionally
From: Paolo Bonzini @ 2025-10-06 11:19 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Naman Jain, Sean Christopherson, Roman Kisel, 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: <20251006111030.GU3245006@noisy.programming.kicks-ass.net>
On Mon, Oct 6, 2025 at 1:10 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Mon, Oct 06, 2025 at 04:20:03PM +0530, Naman Jain wrote:
>
> > I am facing issues in this approach, after moving the assembly code to a
> > separate file, using static calls, and making it noinstr.
> >
> > We need to make a call to STATIC_CALL_TRAMP_STR(hv_hypercall_pg + offset) in
> > the assembly code. This offset is populated at run time in the driver, so I
> > have to pass this offset to the assembly function via function parameters or
> > a shared variable. This leaves noinstr section and results in below warning:
> >
> > [1]: vmlinux.o: warning: objtool: __mshv_vtl_return_call+0x4f: call to
> > mshv_vtl_call_addr() leaves .noinstr.text section
> >
> >
> > To fix this, one of the ways was to avoid making indirect calls. So I used
> > EXPORT_STATIC_CALL to export the static call *trampoline and key* for the
> > static call we created in C driver. Then I figured, we could simply call
> > __SCT__<static_callname> in assembly code and it should work fine. But then
> > it leads to this error in objtool.
>
> Easiest solution is to create a second static_call and have
> hv_set_hypercall_pg() set that to +offset.
Yes, my idea was to add +offset directly in the static_call_update, as
you sketched below. Sorry if that wasn't too clear. I didn't think of
using a static call also for the base of the page, since that's not
what the assembly code needs.
And I think we agree that you absolutely don't want indirect calls, as
that makes register usage much simpler. This way static calls end up
killing multiple birds with a stone.
> Also, what's actually in that hypercall page that is so magical and
> can't just be an ALTERNATIVE() ?
Do you mean an alternative for VMCALL vs. VMMCALL? If so, that's just
not guaranteed to work: "An attempt to invoke a hypercall by any other
means (for example, copying the code from the hypercall code page to
an alternate location and executing it from there) might result in an
undefined operation (#UD) exception" (or it might not).
Paolo
^ permalink raw reply
* Re: [PATCH] x86/hyperv: Export hv_hypercall_pg unconditionally
From: Naman Jain @ 2025-10-06 14:27 UTC (permalink / raw)
To: Paolo Bonzini, Peter Zijlstra
Cc: Sean Christopherson, Roman Kisel, 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: <CABgObfZFgHY_ybfSnyzEF1dFr5c1s=_r+tAnHa6Q7rzFUUdt3g@mail.gmail.com>
On 10/6/2025 4:49 PM, Paolo Bonzini wrote:
> On Mon, Oct 6, 2025 at 1:10 PM Peter Zijlstra <peterz@infradead.org> wrote:
>>
>> On Mon, Oct 06, 2025 at 04:20:03PM +0530, Naman Jain wrote:
>>
>>> I am facing issues in this approach, after moving the assembly code to a
>>> separate file, using static calls, and making it noinstr.
>>>
>>> We need to make a call to STATIC_CALL_TRAMP_STR(hv_hypercall_pg + offset) in
>>> the assembly code. This offset is populated at run time in the driver, so I
>>> have to pass this offset to the assembly function via function parameters or
>>> a shared variable. This leaves noinstr section and results in below warning:
>>>
>>> [1]: vmlinux.o: warning: objtool: __mshv_vtl_return_call+0x4f: call to
>>> mshv_vtl_call_addr() leaves .noinstr.text section
>>>
>>>
>>> To fix this, one of the ways was to avoid making indirect calls. So I used
>>> EXPORT_STATIC_CALL to export the static call *trampoline and key* for the
>>> static call we created in C driver. Then I figured, we could simply call
>>> __SCT__<static_callname> in assembly code and it should work fine. But then
>>> it leads to this error in objtool.
>>
>> Easiest solution is to create a second static_call and have
>> hv_set_hypercall_pg() set that to +offset.
>
> Yes, my idea was to add +offset directly in the static_call_update, as
> you sketched below. Sorry if that wasn't too clear. I didn't think of
> using a static call also for the base of the page, since that's not
> what the assembly code needs.
>
> And I think we agree that you absolutely don't want indirect calls, as
> that makes register usage much simpler. This way static calls end up
> killing multiple birds with a stone.
>
Actually I was implementing it in the same way by introducing a new
static call using hypercall_pg+offset. The problem was using it in asm
file. When I tried introducing a wrapper static call earlier, I did not
use ASM_CALL_CONSTRAINT, so objtool was asking me to save/restore
registers again. Since this was not possible, I thought it may not work.
With something similar to how its done in
arch/x86/include/asm/preempt.h, I was able to make it work, and remove
dependency on IBT config.
Thanks a lot Peter and Paolo for helping on this.
Regards,
Naman
>> Also, what's actually in that hypercall page that is so magical and
>> can't just be an ALTERNATIVE() ?
>
> Do you mean an alternative for VMCALL vs. VMMCALL? If so, that's just
> not guaranteed to work: "An attempt to invoke a hypercall by any other
> means (for example, copying the code from the hypercall code page to
> an alternate location and executing it from there) might result in an
> undefined operation (#UD) exception" (or it might not).
>
> Paolo
>
^ permalink raw reply
* Re: [PATCH v3 1/5] Drivers: hv: Refactor and rename memory region handling functions
From: Stanislav Kinsburskii @ 2025-10-06 14:57 UTC (permalink / raw)
To: Michael Kelley
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <SN6PR02MB4157D88089F3221A853B1C67D4E3A@SN6PR02MB4157.namprd02.prod.outlook.com>
On Mon, Oct 06, 2025 at 03:08:02AM +0000, Michael Kelley wrote:
> From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Thursday, October 2, 2025 9:36 AM
> >
> > Simplify and unify memory region management to improve code clarity and
> > reliability. Consolidate pinning and invalidation logic, adopt consistent
> > naming, and remove redundant checks to reduce complexity.
> >
> > Enhance documentation and update call sites for maintainability.
> >
> > Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
> > ---
> > drivers/hv/mshv_root_main.c | 78 +++++++++++++++++++------------------------
> > 1 file changed, 35 insertions(+), 43 deletions(-)
> >
> > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> > index fa42c40e1e02f..29d0c2c9ae4c8 100644
> > --- a/drivers/hv/mshv_root_main.c
> > +++ b/drivers/hv/mshv_root_main.c
>
> [snip]
>
> > @@ -1264,17 +1248,25 @@ static int mshv_partition_create_region(struct mshv_partition *partition,
> > return 0;
> > }
> >
> > -/*
> > - * Map guest ram. if snp, make sure to release that from the host first
> > - * Side Effects: In case of failure, pages are unpinned when feasible.
> > +/**
> > + * mshv_prepare_pinned_region - Pin and map memory regions
> > + * @region: Pointer to the memory region structure
> > + *
> > + * This function processes memory regions that are explicitly marked as pinned.
> > + * Pinned regions are preallocated, mapped upfront, and do not rely on fault-based
> > + * population. The function ensures the region is properly populated, handles
> > + * encryption requirements for SNP partitions if applicable, maps the region,
> > + * and performs necessary sharing or eviction operations based on the mapping
> > + * result.
> > + *
> > + * Return: 0 on success, negative error code on failure.
> > */
> > -static int
> > -mshv_partition_mem_region_map(struct mshv_mem_region *region)
> > +static int mshv_prepare_pinned_region(struct mshv_mem_region *region)
> > {
> > struct mshv_partition *partition = region->partition;
> > int ret;
> >
> > - ret = mshv_region_populate(region);
> > + ret = mshv_region_pin(region);
> > if (ret) {
> > pt_err(partition, "Failed to populate memory region: %d\n",
>
> Nit: This error message should probably use the new "pin" terminology
> instead of "populate".
>
Yes, it should.
Thanks,
Stanislav
> Michael
^ permalink raw reply
* Re: [PATCH v3 3/5] Drivers: hv: Batch GPA unmap operations to improve large region performance
From: Stanislav Kinsburskii @ 2025-10-06 15:00 UTC (permalink / raw)
To: Michael Kelley
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <SN6PR02MB415777A957EF622587C54A35D4E4A@SN6PR02MB4157.namprd02.prod.outlook.com>
On Fri, Oct 03, 2025 at 09:41:32PM +0000, Michael Kelley wrote:
> From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Friday, October 3, 2025 9:52 AM
>
> >
> > On Fri, Oct 03, 2025 at 12:27:13AM +0000, Michael Kelley wrote:
> > > From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Thursday, October 2, 2025 9:36 AM
> > > >
> > > > Reduce overhead when unmapping large memory regions by batching GPA unmap
> > > > operations in 2MB-aligned chunks.
> > > >
> > > > Use a dedicated constant for batch size to improve code clarity and
> > > > maintainability.
> > > >
> > > > Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
<snip>
> > > > + MSHV_MAX_UNMAP_GPA_PAGES, unmap_flags);
> > > > +
> > > > + page_offset += MSHV_MAX_UNMAP_GPA_PAGES - 1;
> > >
> > > This update to the page_offset doesn't take into account the effect of the
> > > ALIGN (or ALIGN_DOWN) call. With a change to ALIGN_DOWN(), it may
> > > increment too far and perhaps cause the "for" loop to be exited prematurely,
> > > which would fail to unmap some of the pages.
> > >
> >
> > I’m not sure I see the problem here. If we align the offset by
> > MSHV_MAX_UNMAP_GPA_PAGES and unmap the same number of pages, then we
> > should increment the offset by that very same number, shouldn’t we?
>
> Here's an example showing the problem I see (assuming ALIGN_DOWN
> instead of ALIGN):
>
> 1) For simplicity in the example, assume region->start_gfn is zero.
> 2) Entries 0 thru 3 (i.e., 4 entries) in region->pages[] are zero.
> 3) Entries 4 thru 515 (the next 512 entries) are non-zero.
> 4) Entries 516 thru 1023 (the next 508 entries) are zero.
> 5) Entries 1024 thru 1535 (the last 512 entries) are non-zero.
>
> Upon entering the "for" loop for the first time, page_offset gets
> incremented to 4 because of skipping entries 0 thru 3 that are zero.
> On the next iteration where page_offset is 4, the hypercall is made,
> passing 0 for the gfn (because of ALIGN_DOWN), with a count of
> 512, so entries 0 thru 511 are unmapped. Entries 0 thru 3 are valid
> entries, and the fact that they aren't mapped is presumably ignored
> by the hypercall, so everything works.
>
> Then page_offset is incremented by 511, so it will be 515. Continuing
> the "for" loop increments page_offset to 516. The zero entries 516
> thru 1023 increment page_offset to 1024. Finally the hypercall is made
> again covering entries 1024 thru 1535, none of which are zero.
>
> But notice that entries 512 thru 515 (which are non-zero) got skipped.
> That's because the first invocation of the hypercall covered only through
> entry 511, while page_offset was incremented to 515. page_offset
> should have been set to 511, since that's the last entry processed by
> the first invocation of the hypercall.
>
> Michael
I see the problem now. Thank you.
I'll fix it in the next revision.
Thanks,
Stanislav
^ permalink raw reply
* [PATCH v4 0/5] Introduce movable pages for Hyper-V guests
From: Stanislav Kinsburskii @ 2025-10-06 15:06 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui; +Cc: linux-hyperv, linux-kernel
From the start, the root-partition driver allocates, pins, and maps all
guest memory into the hypervisor at guest creation. This is simple: Linux
cannot move the pages, so the guest’s view in Linux and in Microsoft
Hypervisor never diverges.
However, this approach has major drawbacks:
- NUMA: affinity can’t be changed at runtime, so you can’t migrate guest memory closer to the CPUs running it → performance hit.
- Memory management: unused guest memory can’t be swapped out, compacted, or merged.
- Provisioning time: upfront allocation/pinning slows guest create/destroy.
- Overcommit: no memory overcommit on hosts with pinned-guest memory.
This series adds movable memory pages for Hyper-V child partitions. Guest
pages are no longer allocated upfront; they’re allocated and mapped into
the hypervisor on demand (i.e., when the guest touches a GFN that isn’t yet
backed by a host PFN).
When a page is moved, Linux no longer holds it and it is unmapped from the hypervisor.
As a result, Hyper-V guests behave like regular Linux processes, enabling standard Linux memory features to apply to guests.
Exceptions (still pinned):
1. Encrypted guests (explicit).
2. Guests with passthrough devices (implicitly pinned by the VFIO framework).
v4:
- Fix a bug in batch unmapping can skip mapped pages when selecting a new
batch due to wrong offset calculation.
- Fix an error message in case of failed memory region pinning.
v3:
- Region is invalidated even if the mm has no users.
- Page remapping logic is updated to support 2M-unaligned remappings for
regions that are PMD-aligned, which can occur during both faults and
invalidations.
v2:
- Split unmap batching into a separate patch.
- Fixed commit messages from v1 review.
- Renamed a few functions for clarity.
---
Stanislav Kinsburskii (5):
Drivers: hv: Refactor and rename memory region handling functions
Drivers: hv: Centralize guest memory region destruction
Drivers: hv: Batch GPA unmap operations to improve large region performance
Drivers: hv: Ensure large page GPA mapping is PMD-aligned
Drivers: hv: Add support for movable memory regions
drivers/hv/Kconfig | 1
drivers/hv/mshv_root.h | 10 +
drivers/hv/mshv_root_hv_call.c | 2
drivers/hv/mshv_root_main.c | 495 +++++++++++++++++++++++++++++++++-------
4 files changed, 424 insertions(+), 84 deletions(-)
^ permalink raw reply
* [PATCH v4 1/5] Drivers: hv: Refactor and rename memory region handling functions
From: Stanislav Kinsburskii @ 2025-10-06 15:06 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui; +Cc: linux-hyperv, linux-kernel
In-Reply-To: <175976284493.16834.4572937416426518745.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net>
Simplify and unify memory region management to improve code clarity and
reliability. Consolidate pinning and invalidation logic, adopt consistent
naming, and remove redundant checks to reduce complexity.
Enhance documentation and update call sites for maintainability.
Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
---
drivers/hv/mshv_root_main.c | 80 +++++++++++++++++++------------------------
1 file changed, 36 insertions(+), 44 deletions(-)
diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index fa42c40e1e02f..e923947d3c54d 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -1120,8 +1120,8 @@ mshv_region_map(struct mshv_mem_region *region)
}
static void
-mshv_region_evict_pages(struct mshv_mem_region *region,
- u64 page_offset, u64 page_count)
+mshv_region_invalidate_pages(struct mshv_mem_region *region,
+ u64 page_offset, u64 page_count)
{
if (region->flags.range_pinned)
unpin_user_pages(region->pages + page_offset, page_count);
@@ -1131,29 +1131,24 @@ mshv_region_evict_pages(struct mshv_mem_region *region,
}
static void
-mshv_region_evict(struct mshv_mem_region *region)
+mshv_region_invalidate(struct mshv_mem_region *region)
{
- mshv_region_evict_pages(region, 0, region->nr_pages);
+ mshv_region_invalidate_pages(region, 0, region->nr_pages);
}
static int
-mshv_region_populate_pages(struct mshv_mem_region *region,
- u64 page_offset, u64 page_count)
+mshv_region_pin(struct mshv_mem_region *region)
{
u64 done_count, nr_pages;
struct page **pages;
__u64 userspace_addr;
int ret;
- if (page_offset + page_count > region->nr_pages)
- return -EINVAL;
-
- for (done_count = 0; done_count < page_count; done_count += ret) {
- pages = region->pages + page_offset + done_count;
+ for (done_count = 0; done_count < region->nr_pages; done_count += ret) {
+ pages = region->pages + done_count;
userspace_addr = region->start_uaddr +
- (page_offset + done_count) *
- HV_HYP_PAGE_SIZE;
- nr_pages = min(page_count - done_count,
+ done_count * HV_HYP_PAGE_SIZE;
+ nr_pages = min(region->nr_pages - done_count,
MSHV_PIN_PAGES_BATCH_SIZE);
/*
@@ -1164,34 +1159,23 @@ mshv_region_populate_pages(struct mshv_mem_region *region,
* with the FOLL_LONGTERM flag does a large temporary
* allocation of contiguous memory.
*/
- if (region->flags.range_pinned)
- ret = pin_user_pages_fast(userspace_addr,
- nr_pages,
- FOLL_WRITE | FOLL_LONGTERM,
- pages);
- else
- ret = -EOPNOTSUPP;
-
+ ret = pin_user_pages_fast(userspace_addr, nr_pages,
+ FOLL_WRITE | FOLL_LONGTERM,
+ pages);
if (ret < 0)
goto release_pages;
}
- if (PageHuge(region->pages[page_offset]))
+ if (PageHuge(region->pages[0]))
region->flags.large_pages = true;
return 0;
release_pages:
- mshv_region_evict_pages(region, page_offset, done_count);
+ mshv_region_invalidate_pages(region, 0, done_count);
return ret;
}
-static int
-mshv_region_populate(struct mshv_mem_region *region)
-{
- return mshv_region_populate_pages(region, 0, region->nr_pages);
-}
-
static struct mshv_mem_region *
mshv_partition_region_by_gfn(struct mshv_partition *partition, u64 gfn)
{
@@ -1264,19 +1248,27 @@ static int mshv_partition_create_region(struct mshv_partition *partition,
return 0;
}
-/*
- * Map guest ram. if snp, make sure to release that from the host first
- * Side Effects: In case of failure, pages are unpinned when feasible.
+/**
+ * mshv_prepare_pinned_region - Pin and map memory regions
+ * @region: Pointer to the memory region structure
+ *
+ * This function processes memory regions that are explicitly marked as pinned.
+ * Pinned regions are preallocated, mapped upfront, and do not rely on fault-based
+ * population. The function ensures the region is properly populated, handles
+ * encryption requirements for SNP partitions if applicable, maps the region,
+ * and performs necessary sharing or eviction operations based on the mapping
+ * result.
+ *
+ * Return: 0 on success, negative error code on failure.
*/
-static int
-mshv_partition_mem_region_map(struct mshv_mem_region *region)
+static int mshv_prepare_pinned_region(struct mshv_mem_region *region)
{
struct mshv_partition *partition = region->partition;
int ret;
- ret = mshv_region_populate(region);
+ ret = mshv_region_pin(region);
if (ret) {
- pt_err(partition, "Failed to populate memory region: %d\n",
+ pt_err(partition, "Failed to pin memory region: %d\n",
ret);
goto err_out;
}
@@ -1294,7 +1286,7 @@ mshv_partition_mem_region_map(struct mshv_mem_region *region)
pt_err(partition,
"Failed to unshare memory region (guest_pfn: %llu): %d\n",
region->start_gfn, ret);
- goto evict_region;
+ goto invalidate_region;
}
}
@@ -1304,7 +1296,7 @@ mshv_partition_mem_region_map(struct mshv_mem_region *region)
shrc = mshv_partition_region_share(region);
if (!shrc)
- goto evict_region;
+ goto invalidate_region;
pt_err(partition,
"Failed to share memory region (guest_pfn: %llu): %d\n",
@@ -1318,8 +1310,8 @@ mshv_partition_mem_region_map(struct mshv_mem_region *region)
return 0;
-evict_region:
- mshv_region_evict(region);
+invalidate_region:
+ mshv_region_invalidate(region);
err_out:
return ret;
}
@@ -1368,7 +1360,7 @@ mshv_map_user_memory(struct mshv_partition *partition,
ret = hv_call_map_mmio_pages(partition->pt_id, mem.guest_pfn,
mmio_pfn, HVPFN_DOWN(mem.size));
else
- ret = mshv_partition_mem_region_map(region);
+ ret = mshv_prepare_pinned_region(region);
if (ret)
goto errout;
@@ -1413,7 +1405,7 @@ mshv_unmap_user_memory(struct mshv_partition *partition,
hv_call_unmap_gpa_pages(partition->pt_id, region->start_gfn,
region->nr_pages, unmap_flags);
- mshv_region_evict(region);
+ mshv_region_invalidate(region);
vfree(region);
return 0;
@@ -1827,7 +1819,7 @@ static void destroy_partition(struct mshv_partition *partition)
}
}
- mshv_region_evict(region);
+ mshv_region_invalidate(region);
vfree(region);
}
^ 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