* [PATCH hyperv-next v7 12/17] Drivers: hv: Functions for setting up and tearing down the paravisor SynIC
From: Roman Kisel @ 2025-10-08 23:34 UTC (permalink / raw)
To: arnd, bp, bagasdotme, 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
In-Reply-To: <20251008233419.20372-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 v7 11/17] Drivers: hv: Rename the SynIC enable and disable routines
From: Roman Kisel @ 2025-10-08 23:34 UTC (permalink / raw)
To: arnd, bp, bagasdotme, 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
In-Reply-To: <20251008233419.20372-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 v7 10/17] Drivers: hv: Check message and event pages for non-NULL before iounmap()
From: Roman Kisel @ 2025-10-08 23:34 UTC (permalink / raw)
To: arnd, bp, bagasdotme, 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
In-Reply-To: <20251008233419.20372-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 v7 09/17] Drivers: hv: remove stale comment
From: Roman Kisel @ 2025-10-08 23:34 UTC (permalink / raw)
To: arnd, bp, bagasdotme, 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
In-Reply-To: <20251008233419.20372-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 v7 08/17] Drivers: hv: Post messages through the confidential VMBus if available
From: Roman Kisel @ 2025-10-08 23:34 UTC (permalink / raw)
To: arnd, bp, bagasdotme, 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
In-Reply-To: <20251008233419.20372-1-romank@linux.microsoft.com>
When the confidential VMBus is available, the guest should post
messages to the paravisor.
Update hv_post_message() to post messages to the paravisor rather than
through GHCB or TD calls.
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 | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 90db1e17582d..b1085473778c 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -74,7 +74,11 @@ int hv_post_message(union hv_connection_id connection_id,
aligned_msg->payload_size = payload_size;
memcpy((void *)aligned_msg->payload, payload, payload_size);
- if (ms_hyperv.paravisor_present) {
+ if (ms_hyperv.paravisor_present && !vmbus_is_confidential()) {
+ /*
+ * If the VMBus isn't confidential, use the CoCo-specific
+ * mechanism to communicate with the hypervisor.
+ */
if (hv_isolation_type_tdx())
status = hv_tdx_hypercall(HVCALL_POST_MESSAGE,
virt_to_phys(aligned_msg), 0);
@@ -88,6 +92,11 @@ int hv_post_message(union hv_connection_id connection_id,
u64 control = HVCALL_POST_MESSAGE;
control |= hv_nested ? HV_HYPERCALL_NESTED : 0;
+ /*
+ * If there is no paravisor, this will go to the hypervisor.
+ * In the Confidential VMBus case, there is the paravisor
+ * to which this will trap.
+ */
status = hv_do_hypercall(control, aligned_msg, NULL);
}
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v7 07/17] Drivers: hv: Allocate the paravisor SynIC pages when required
From: Roman Kisel @ 2025-10-08 23:34 UTC (permalink / raw)
To: arnd, bp, bagasdotme, 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
In-Reply-To: <20251008233419.20372-1-romank@linux.microsoft.com>
Confidential VMBus requires interacting with two SynICs -- one
provided by the host hypervisor, and one provided by the paravisor.
Each SynIC requires its own message and event pages.
Refactor and extend the existing code to add allocating and freeing
the message and event pages for the paravisor SynIC when it is
present.
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 | 184 +++++++++++++++++++-------------------
drivers/hv/hyperv_vmbus.h | 18 ++++
2 files changed, 112 insertions(+), 90 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index b7419d0fad1d..90db1e17582d 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -96,10 +96,70 @@ int hv_post_message(union hv_connection_id connection_id,
return hv_result(status);
}
+static int hv_alloc_page(void **page, bool decrypt, const char *note)
+{
+ int ret = 0;
+
+ /*
+ * After the page changes its encryption status, its contents might
+ * appear scrambled on some hardware. Thus `get_zeroed_page` would
+ * zero the page out in vain, so do that explicitly exactly once.
+ *
+ * By default, the page is allocated encrypted in a CoCo VM.
+ */
+ *page = (void *)__get_free_page(GFP_KERNEL);
+ if (!*page)
+ return -ENOMEM;
+
+ if (decrypt)
+ ret = set_memory_decrypted((unsigned long)*page, 1);
+ if (ret)
+ goto failed;
+
+ memset(*page, 0, PAGE_SIZE);
+ return 0;
+
+failed:
+ /*
+ * Report the failure but don't put the page back on the free list as
+ * its encryption status is unknown.
+ */
+ pr_err("allocation failed for %s page, error %d, decrypted %d\n",
+ note, ret, decrypt);
+ *page = NULL;
+ return ret;
+}
+
+static int hv_free_page(void **page, bool encrypt, const char *note)
+{
+ int ret = 0;
+
+ if (!*page)
+ return 0;
+
+ if (encrypt)
+ ret = set_memory_encrypted((unsigned long)*page, 1);
+
+ /*
+ * In the case of the failure, the page is leaked. Something is wrong,
+ * prefer to lose the page with the unknown encryption status and stay afloat.
+ */
+ if (ret)
+ pr_err("deallocation failed for %s page, error %d, encrypt %d\n",
+ note, ret, encrypt);
+ else
+ free_page((unsigned long)*page);
+
+ *page = NULL;
+
+ return ret;
+}
+
int hv_synic_alloc(void)
{
int cpu, ret = -ENOMEM;
struct hv_per_cpu_context *hv_cpu;
+ const bool decrypt = !vmbus_is_confidential();
/*
* First, zero all per-cpu memory areas so hv_synic_free() can
@@ -125,73 +185,37 @@ int hv_synic_alloc(void)
vmbus_on_msg_dpc, (unsigned long)hv_cpu);
if (ms_hyperv.paravisor_present && hv_isolation_type_tdx()) {
- hv_cpu->post_msg_page = (void *)get_zeroed_page(GFP_ATOMIC);
- if (!hv_cpu->post_msg_page) {
- pr_err("Unable to allocate post msg page\n");
+ ret = hv_alloc_page(&hv_cpu->post_msg_page,
+ decrypt, "post msg");
+ if (ret)
goto err;
- }
-
- ret = set_memory_decrypted((unsigned long)hv_cpu->post_msg_page, 1);
- if (ret) {
- pr_err("Failed to decrypt post msg page: %d\n", ret);
- /* Just leak the page, as it's unsafe to free the page. */
- hv_cpu->post_msg_page = NULL;
- goto err;
- }
-
- memset(hv_cpu->post_msg_page, 0, PAGE_SIZE);
}
/*
- * Synic message and event pages are allocated by paravisor.
- * Skip these pages allocation here.
+ * If these SynIC pages are not allocated, SIEF and SIM pages
+ * are configured using what the root partition or the paravisor
+ * provides upon reading the SIEFP and SIMP registers.
*/
if (!ms_hyperv.paravisor_present && !hv_root_partition()) {
- hv_cpu->hyp_synic_message_page =
- (void *)get_zeroed_page(GFP_ATOMIC);
- if (!hv_cpu->hyp_synic_message_page) {
- pr_err("Unable to allocate SYNIC message page\n");
+ ret = hv_alloc_page(&hv_cpu->hyp_synic_message_page,
+ decrypt, "hypervisor SynIC msg");
+ if (ret)
goto err;
- }
-
- hv_cpu->hyp_synic_event_page =
- (void *)get_zeroed_page(GFP_ATOMIC);
- if (!hv_cpu->hyp_synic_event_page) {
- pr_err("Unable to allocate SYNIC event page\n");
-
- free_page((unsigned long)hv_cpu->hyp_synic_message_page);
- hv_cpu->hyp_synic_message_page = NULL;
+ ret = hv_alloc_page(&hv_cpu->hyp_synic_event_page,
+ decrypt, "hypervisor SynIC event");
+ if (ret)
goto err;
- }
}
- if (!ms_hyperv.paravisor_present &&
- (hv_isolation_type_snp() || hv_isolation_type_tdx())) {
- ret = set_memory_decrypted((unsigned long)
- hv_cpu->hyp_synic_message_page, 1);
- if (ret) {
- pr_err("Failed to decrypt SYNIC msg page: %d\n", ret);
- hv_cpu->hyp_synic_message_page = NULL;
-
- /*
- * Free the event page here so that hv_synic_free()
- * won't later try to re-encrypt it.
- */
- free_page((unsigned long)hv_cpu->hyp_synic_event_page);
- hv_cpu->hyp_synic_event_page = NULL;
+ if (vmbus_is_confidential()) {
+ ret = hv_alloc_page(&hv_cpu->para_synic_message_page,
+ false, "paravisor SynIC msg");
+ if (ret)
goto err;
- }
-
- ret = set_memory_decrypted((unsigned long)
- hv_cpu->hyp_synic_event_page, 1);
- if (ret) {
- pr_err("Failed to decrypt SYNIC event page: %d\n", ret);
- hv_cpu->hyp_synic_event_page = NULL;
+ ret = hv_alloc_page(&hv_cpu->para_synic_event_page,
+ false, "paravisor SynIC event");
+ if (ret)
goto err;
- }
-
- memset(hv_cpu->hyp_synic_message_page, 0, PAGE_SIZE);
- memset(hv_cpu->hyp_synic_event_page, 0, PAGE_SIZE);
}
}
@@ -207,48 +231,28 @@ int hv_synic_alloc(void)
void hv_synic_free(void)
{
- int cpu, ret;
+ int cpu;
+ const bool encrypt = !vmbus_is_confidential();
for_each_present_cpu(cpu) {
struct hv_per_cpu_context *hv_cpu =
per_cpu_ptr(hv_context.cpu_context, cpu);
- /* It's better to leak the page if the encryption fails. */
- if (ms_hyperv.paravisor_present && hv_isolation_type_tdx()) {
- if (hv_cpu->post_msg_page) {
- ret = set_memory_encrypted((unsigned long)
- hv_cpu->post_msg_page, 1);
- if (ret) {
- pr_err("Failed to encrypt post msg page: %d\n", ret);
- hv_cpu->post_msg_page = NULL;
- }
- }
+ if (ms_hyperv.paravisor_present && hv_isolation_type_tdx())
+ hv_free_page(&hv_cpu->post_msg_page,
+ encrypt, "post msg");
+ if (!ms_hyperv.paravisor_present && !hv_root_partition()) {
+ hv_free_page(&hv_cpu->hyp_synic_event_page,
+ encrypt, "hypervisor SynIC event");
+ hv_free_page(&hv_cpu->hyp_synic_message_page,
+ encrypt, "hypervisor SynIC msg");
}
-
- if (!ms_hyperv.paravisor_present &&
- (hv_isolation_type_snp() || hv_isolation_type_tdx())) {
- if (hv_cpu->hyp_synic_message_page) {
- ret = set_memory_encrypted((unsigned long)
- hv_cpu->hyp_synic_message_page, 1);
- if (ret) {
- pr_err("Failed to encrypt SYNIC msg page: %d\n", ret);
- hv_cpu->hyp_synic_message_page = NULL;
- }
- }
-
- if (hv_cpu->hyp_synic_event_page) {
- ret = set_memory_encrypted((unsigned long)
- hv_cpu->hyp_synic_event_page, 1);
- if (ret) {
- pr_err("Failed to encrypt SYNIC event page: %d\n", ret);
- hv_cpu->hyp_synic_event_page = NULL;
- }
- }
+ if (vmbus_is_confidential()) {
+ hv_free_page(&hv_cpu->para_synic_event_page,
+ false, "paravisor SynIC event");
+ hv_free_page(&hv_cpu->para_synic_message_page,
+ false, "paravisor SynIC msg");
}
-
- free_page((unsigned long)hv_cpu->post_msg_page);
- free_page((unsigned long)hv_cpu->hyp_synic_event_page);
- free_page((unsigned long)hv_cpu->hyp_synic_message_page);
}
kfree(hv_context.hv_numa_map);
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index d593af45a5b2..3c70051c0431 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -121,8 +121,26 @@ enum {
* Per cpu state for channel handling
*/
struct hv_per_cpu_context {
+ /*
+ * SynIC pages for communicating with the host.
+ *
+ * These pages are accessible to the host partition and the hypervisor.
+ * They may be used for exchanging data with the host partition and the
+ * hypervisor even when they aren't trusted yet the guest partition
+ * must be prepared to handle the malicious behavior.
+ */
void *hyp_synic_message_page;
void *hyp_synic_event_page;
+ /*
+ * SynIC pages for communicating with the paravisor.
+ *
+ * These pages may be accessed from within the guest partition only in
+ * CoCo VMs. Neither the host partition nor the hypervisor can access
+ * these pages in that case; they are used for exchanging data with the
+ * paravisor.
+ */
+ void *para_synic_message_page;
+ void *para_synic_event_page;
/*
* The page is only used in hv_post_message() for a TDX VM (with the
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v7 06/17] Drivers: hv: Rename fields for SynIC message and event pages
From: Roman Kisel @ 2025-10-08 23:34 UTC (permalink / raw)
To: arnd, bp, bagasdotme, 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
In-Reply-To: <20251008233419.20372-1-romank@linux.microsoft.com>
Confidential VMBus requires interacting with two SynICs -- one
provided by the host hypervisor, and one provided by the paravisor.
Each SynIC requires its own message and event pages.
Rename the existing host-accessible SynIC message and event pages
with the "hyp_" prefix to clearly distinguish them from the paravisor
ones. The field name is also changed in mshv_root.* for consistency.
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 | 6 ++--
drivers/hv/hv.c | 66 +++++++++++++++++++--------------------
drivers/hv/hyperv_vmbus.h | 4 +--
drivers/hv/mshv_root.h | 2 +-
drivers/hv/mshv_synic.c | 6 ++--
drivers/hv/vmbus_drv.c | 6 ++--
6 files changed, 45 insertions(+), 45 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 65dd299e2944..1a33c6944b3c 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -844,14 +844,14 @@ static void vmbus_wait_for_unload(void)
= per_cpu_ptr(hv_context.cpu_context, cpu);
/*
- * In a CoCo VM the synic_message_page is not allocated
+ * 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()
* 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.
*/
- page_addr = hv_cpu->synic_message_page;
+ page_addr = hv_cpu->hyp_synic_message_page;
if (!page_addr)
continue;
@@ -892,7 +892,7 @@ static void vmbus_wait_for_unload(void)
struct hv_per_cpu_context *hv_cpu
= per_cpu_ptr(hv_context.cpu_context, cpu);
- page_addr = hv_cpu->synic_message_page;
+ page_addr = hv_cpu->hyp_synic_message_page;
if (!page_addr)
continue;
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index b14c5f9e0ef2..b7419d0fad1d 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -147,20 +147,20 @@ int hv_synic_alloc(void)
* Skip these pages allocation here.
*/
if (!ms_hyperv.paravisor_present && !hv_root_partition()) {
- hv_cpu->synic_message_page =
+ hv_cpu->hyp_synic_message_page =
(void *)get_zeroed_page(GFP_ATOMIC);
- if (!hv_cpu->synic_message_page) {
+ if (!hv_cpu->hyp_synic_message_page) {
pr_err("Unable to allocate SYNIC message page\n");
goto err;
}
- hv_cpu->synic_event_page =
+ hv_cpu->hyp_synic_event_page =
(void *)get_zeroed_page(GFP_ATOMIC);
- if (!hv_cpu->synic_event_page) {
+ if (!hv_cpu->hyp_synic_event_page) {
pr_err("Unable to allocate SYNIC event page\n");
- free_page((unsigned long)hv_cpu->synic_message_page);
- hv_cpu->synic_message_page = NULL;
+ free_page((unsigned long)hv_cpu->hyp_synic_message_page);
+ hv_cpu->hyp_synic_message_page = NULL;
goto err;
}
}
@@ -168,30 +168,30 @@ int hv_synic_alloc(void)
if (!ms_hyperv.paravisor_present &&
(hv_isolation_type_snp() || hv_isolation_type_tdx())) {
ret = set_memory_decrypted((unsigned long)
- hv_cpu->synic_message_page, 1);
+ hv_cpu->hyp_synic_message_page, 1);
if (ret) {
pr_err("Failed to decrypt SYNIC msg page: %d\n", ret);
- hv_cpu->synic_message_page = NULL;
+ hv_cpu->hyp_synic_message_page = NULL;
/*
* Free the event page here so that hv_synic_free()
* won't later try to re-encrypt it.
*/
- free_page((unsigned long)hv_cpu->synic_event_page);
- hv_cpu->synic_event_page = NULL;
+ free_page((unsigned long)hv_cpu->hyp_synic_event_page);
+ hv_cpu->hyp_synic_event_page = NULL;
goto err;
}
ret = set_memory_decrypted((unsigned long)
- hv_cpu->synic_event_page, 1);
+ hv_cpu->hyp_synic_event_page, 1);
if (ret) {
pr_err("Failed to decrypt SYNIC event page: %d\n", ret);
- hv_cpu->synic_event_page = NULL;
+ hv_cpu->hyp_synic_event_page = NULL;
goto err;
}
- memset(hv_cpu->synic_message_page, 0, PAGE_SIZE);
- memset(hv_cpu->synic_event_page, 0, PAGE_SIZE);
+ memset(hv_cpu->hyp_synic_message_page, 0, PAGE_SIZE);
+ memset(hv_cpu->hyp_synic_event_page, 0, PAGE_SIZE);
}
}
@@ -227,28 +227,28 @@ void hv_synic_free(void)
if (!ms_hyperv.paravisor_present &&
(hv_isolation_type_snp() || hv_isolation_type_tdx())) {
- if (hv_cpu->synic_message_page) {
+ if (hv_cpu->hyp_synic_message_page) {
ret = set_memory_encrypted((unsigned long)
- hv_cpu->synic_message_page, 1);
+ hv_cpu->hyp_synic_message_page, 1);
if (ret) {
pr_err("Failed to encrypt SYNIC msg page: %d\n", ret);
- hv_cpu->synic_message_page = NULL;
+ hv_cpu->hyp_synic_message_page = NULL;
}
}
- if (hv_cpu->synic_event_page) {
+ if (hv_cpu->hyp_synic_event_page) {
ret = set_memory_encrypted((unsigned long)
- hv_cpu->synic_event_page, 1);
+ hv_cpu->hyp_synic_event_page, 1);
if (ret) {
pr_err("Failed to encrypt SYNIC event page: %d\n", ret);
- hv_cpu->synic_event_page = NULL;
+ hv_cpu->hyp_synic_event_page = NULL;
}
}
}
free_page((unsigned long)hv_cpu->post_msg_page);
- free_page((unsigned long)hv_cpu->synic_event_page);
- free_page((unsigned long)hv_cpu->synic_message_page);
+ free_page((unsigned long)hv_cpu->hyp_synic_event_page);
+ free_page((unsigned long)hv_cpu->hyp_synic_message_page);
}
kfree(hv_context.hv_numa_map);
@@ -278,12 +278,12 @@ void hv_synic_enable_regs(unsigned int cpu)
/* Mask out vTOM bit. ioremap_cache() maps decrypted */
u64 base = (simp.base_simp_gpa << HV_HYP_PAGE_SHIFT) &
~ms_hyperv.shared_gpa_boundary;
- hv_cpu->synic_message_page =
+ hv_cpu->hyp_synic_message_page =
(void *)ioremap_cache(base, HV_HYP_PAGE_SIZE);
- if (!hv_cpu->synic_message_page)
+ if (!hv_cpu->hyp_synic_message_page)
pr_err("Fail to map synic message page.\n");
} else {
- simp.base_simp_gpa = virt_to_phys(hv_cpu->synic_message_page)
+ simp.base_simp_gpa = virt_to_phys(hv_cpu->hyp_synic_message_page)
>> HV_HYP_PAGE_SHIFT;
}
@@ -297,12 +297,12 @@ void hv_synic_enable_regs(unsigned int cpu)
/* Mask out vTOM bit. ioremap_cache() maps decrypted */
u64 base = (siefp.base_siefp_gpa << HV_HYP_PAGE_SHIFT) &
~ms_hyperv.shared_gpa_boundary;
- hv_cpu->synic_event_page =
+ hv_cpu->hyp_synic_event_page =
(void *)ioremap_cache(base, HV_HYP_PAGE_SIZE);
- if (!hv_cpu->synic_event_page)
+ if (!hv_cpu->hyp_synic_event_page)
pr_err("Fail to map synic event page.\n");
} else {
- siefp.base_siefp_gpa = virt_to_phys(hv_cpu->synic_event_page)
+ siefp.base_siefp_gpa = virt_to_phys(hv_cpu->hyp_synic_event_page)
>> HV_HYP_PAGE_SHIFT;
}
@@ -360,8 +360,8 @@ void hv_synic_disable_regs(unsigned int cpu)
*/
simp.simp_enabled = 0;
if (ms_hyperv.paravisor_present || hv_root_partition()) {
- iounmap(hv_cpu->synic_message_page);
- hv_cpu->synic_message_page = NULL;
+ iounmap(hv_cpu->hyp_synic_message_page);
+ hv_cpu->hyp_synic_message_page = NULL;
} else {
simp.base_simp_gpa = 0;
}
@@ -372,8 +372,8 @@ void hv_synic_disable_regs(unsigned int cpu)
siefp.siefp_enabled = 0;
if (ms_hyperv.paravisor_present || hv_root_partition()) {
- iounmap(hv_cpu->synic_event_page);
- hv_cpu->synic_event_page = NULL;
+ iounmap(hv_cpu->hyp_synic_event_page);
+ hv_cpu->hyp_synic_event_page = NULL;
} else {
siefp.base_siefp_gpa = 0;
}
@@ -403,7 +403,7 @@ 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 *event =
- (union hv_synic_event_flags *)hv_cpu->synic_event_page + VMBUS_MESSAGE_SINT;
+ (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 */
bool pending;
u32 relid;
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 9ac6f5520287..d593af45a5b2 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -121,8 +121,8 @@ enum {
* Per cpu state for channel handling
*/
struct hv_per_cpu_context {
- void *synic_message_page;
- void *synic_event_page;
+ void *hyp_synic_message_page;
+ void *hyp_synic_event_page;
/*
* The page is only used in hv_post_message() for a TDX VM (with the
diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
index e3931b0f1269..db6b42db2fdc 100644
--- a/drivers/hv/mshv_root.h
+++ b/drivers/hv/mshv_root.h
@@ -169,7 +169,7 @@ struct mshv_girq_routing_table {
};
struct hv_synic_pages {
- struct hv_message_page *synic_message_page;
+ struct hv_message_page *hyp_synic_message_page;
struct hv_synic_event_flags_page *synic_event_flags_page;
struct hv_synic_event_ring_page *synic_event_ring_page;
};
diff --git a/drivers/hv/mshv_synic.c b/drivers/hv/mshv_synic.c
index e6b6381b7c36..f8b0337cdc82 100644
--- a/drivers/hv/mshv_synic.c
+++ b/drivers/hv/mshv_synic.c
@@ -394,7 +394,7 @@ mshv_intercept_isr(struct hv_message *msg)
void mshv_isr(void)
{
struct hv_synic_pages *spages = this_cpu_ptr(mshv_root.synic_pages);
- struct hv_message_page **msg_page = &spages->synic_message_page;
+ struct hv_message_page **msg_page = &spages->hyp_synic_message_page;
struct hv_message *msg;
bool handled;
@@ -456,7 +456,7 @@ int mshv_synic_init(unsigned int cpu)
#endif
union hv_synic_scontrol sctrl;
struct hv_synic_pages *spages = this_cpu_ptr(mshv_root.synic_pages);
- struct hv_message_page **msg_page = &spages->synic_message_page;
+ struct hv_message_page **msg_page = &spages->hyp_synic_message_page;
struct hv_synic_event_flags_page **event_flags_page =
&spages->synic_event_flags_page;
struct hv_synic_event_ring_page **event_ring_page =
@@ -550,7 +550,7 @@ int mshv_synic_cleanup(unsigned int cpu)
union hv_synic_sirbp sirbp;
union hv_synic_scontrol sctrl;
struct hv_synic_pages *spages = this_cpu_ptr(mshv_root.synic_pages);
- struct hv_message_page **msg_page = &spages->synic_message_page;
+ struct hv_message_page **msg_page = &spages->hyp_synic_message_page;
struct hv_synic_event_flags_page **event_flags_page =
&spages->synic_event_flags_page;
struct hv_synic_event_ring_page **event_ring_page =
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 3c414560fa5f..e12f0ba0701f 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1060,7 +1060,7 @@ static void vmbus_onmessage_work(struct work_struct *work)
void vmbus_on_msg_dpc(unsigned long data)
{
struct hv_per_cpu_context *hv_cpu = (void *)data;
- void *page_addr = hv_cpu->synic_message_page;
+ void *page_addr = hv_cpu->hyp_synic_message_page;
struct hv_message msg_copy, *msg = (struct hv_message *)page_addr +
VMBUS_MESSAGE_SINT;
struct vmbus_channel_message_header *hdr;
@@ -1244,7 +1244,7 @@ static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
* The event page can be directly checked to get the id of
* the channel that has the interrupt pending.
*/
- void *page_addr = hv_cpu->synic_event_page;
+ 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;
@@ -1327,7 +1327,7 @@ static void vmbus_isr(void)
vmbus_chan_sched(hv_cpu);
- page_addr = hv_cpu->synic_message_page;
+ page_addr = hv_cpu->hyp_synic_message_page;
msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT;
/* Check if there are actual msgs to be processed */
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v7 05/17] arch/x86: mshyperv: Trap on access for some synthetic MSRs
From: Roman Kisel @ 2025-10-08 23:34 UTC (permalink / raw)
To: arnd, bp, bagasdotme, 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
In-Reply-To: <20251008233419.20372-1-romank@linux.microsoft.com>
hv_set_non_nested_msr() has special handling for SINT MSRs
when a paravisor is present. In addition to updating the MSR on the
host, the mirror MSR in the paravisor is updated, including with the
proxy bit. But with Confidential VMBus, the proxy bit must not be
used, so add a special case to skip it.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
Reviewed-by: Tianyu Lan <tiala@microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
arch/x86/kernel/cpu/mshyperv.c | 29 +++++++++++++++++++++++++----
drivers/hv/hv_common.c | 5 +++++
include/asm-generic/mshyperv.h | 1 +
3 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index af5a3bbbca9f..a42e72c415b5 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -28,6 +28,7 @@
#include <asm/apic.h>
#include <asm/timer.h>
#include <asm/reboot.h>
+#include <asm/msr.h>
#include <asm/nmi.h>
#include <clocksource/hyperv_timer.h>
#include <asm/msr.h>
@@ -43,6 +44,12 @@ bool hyperv_paravisor_present __ro_after_init;
EXPORT_SYMBOL_GPL(hyperv_paravisor_present);
#if IS_ENABLED(CONFIG_HYPERV)
+/*
+ * When running with the paravisor, controls proxying the synthetic interrupts
+ * from the host
+ */
+static bool hv_para_sint_proxy;
+
static inline unsigned int hv_get_nested_msr(unsigned int reg)
{
if (hv_is_sint_msr(reg))
@@ -79,17 +86,31 @@ EXPORT_SYMBOL_GPL(hv_get_non_nested_msr);
void hv_set_non_nested_msr(unsigned int reg, u64 value)
{
if (hv_is_synic_msr(reg) && ms_hyperv.paravisor_present) {
+ /* The hypervisor will get the intercept. */
hv_ivm_msr_write(reg, value);
- /* Write proxy bit via wrmsl instruction */
- if (hv_is_sint_msr(reg))
- wrmsrq(reg, value | 1 << 20);
+ /* Using wrmsrq so the following goes to the paravisor. */
+ if (hv_is_sint_msr(reg)) {
+ union hv_synic_sint sint = { .as_uint64 = value };
+
+ sint.proxy = hv_para_sint_proxy;
+ native_wrmsrq(reg, sint.as_uint64);
+ }
} else {
- wrmsrq(reg, value);
+ native_wrmsrq(reg, value);
}
}
EXPORT_SYMBOL_GPL(hv_set_non_nested_msr);
+/*
+ * Enable or disable proxying synthetic interrupts
+ * to the paravisor.
+ */
+void hv_para_set_sint_proxy(bool enable)
+{
+ hv_para_sint_proxy = enable;
+}
+
/*
* Get the SynIC register value from the paravisor.
*/
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index 8756ca834546..1a5c7a358971 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -716,6 +716,11 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 param2)
}
EXPORT_SYMBOL_GPL(hv_tdx_hypercall);
+void __weak hv_para_set_sint_proxy(bool enable)
+{
+}
+EXPORT_SYMBOL_GPL(hv_para_set_sint_proxy);
+
u64 __weak hv_para_get_synic_register(unsigned int reg)
{
return ~0ULL;
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index c010059f1518..3955ba6d60b8 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -298,6 +298,7 @@ bool hv_is_isolation_supported(void);
bool hv_isolation_type_snp(void);
u64 hv_ghcb_hypercall(u64 control, void *input, void *output, u32 input_size);
u64 hv_tdx_hypercall(u64 control, u64 param1, u64 param2);
+void hv_para_set_sint_proxy(bool enable);
u64 hv_para_get_synic_register(unsigned int reg);
void hv_para_set_synic_register(unsigned int reg, u64 val);
void hyperv_cleanup(void);
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v7 04/17] arch: hyperv: Get/set SynIC synth.registers via paravisor
From: Roman Kisel @ 2025-10-08 23:34 UTC (permalink / raw)
To: arnd, bp, bagasdotme, 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
In-Reply-To: <20251008233419.20372-1-romank@linux.microsoft.com>
The existing Hyper-V wrappers for getting and setting MSRs are
hv_get/set_msr(). Via hv_get/set_non_nested_msr(), they detect
when running in a CoCo VM with a paravisor, and use the TDX or
SNP guest-host communication protocol to bypass the paravisor
and go directly to the host hypervisor for SynIC MSRs. The "set"
function also implements the required special handling for the
SINT MSRs.
Provide functions that allow manipulating the SynIC registers
through the paravisor. Move vmbus_signal_eom() to a more
appropriate location (which also avoids breaking KVM).
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
arch/x86/kernel/cpu/mshyperv.c | 20 ++++++++++++++++
drivers/hv/hv_common.c | 11 +++++++++
drivers/hv/hyperv_vmbus.h | 44 ++++++++++++++++++++++++++++++++++
include/asm-generic/mshyperv.h | 42 ++------------------------------
4 files changed, 77 insertions(+), 40 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 57163c7a000f..af5a3bbbca9f 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -90,6 +90,26 @@ void hv_set_non_nested_msr(unsigned int reg, u64 value)
}
EXPORT_SYMBOL_GPL(hv_set_non_nested_msr);
+/*
+ * Get the SynIC register value from the paravisor.
+ */
+u64 hv_para_get_synic_register(unsigned int reg)
+{
+ if (WARN_ON(!ms_hyperv.paravisor_present || !hv_is_synic_msr(reg)))
+ return ~0ULL;
+ return native_read_msr(reg);
+}
+
+/*
+ * Set the SynIC register value with the paravisor.
+ */
+void hv_para_set_synic_register(unsigned int reg, u64 val)
+{
+ if (WARN_ON(!ms_hyperv.paravisor_present || !hv_is_synic_msr(reg)))
+ return;
+ native_write_msr(reg, val);
+}
+
u64 hv_get_msr(unsigned int reg)
{
if (hv_nested)
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index e109a620c83f..8756ca834546 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -716,6 +716,17 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 param2)
}
EXPORT_SYMBOL_GPL(hv_tdx_hypercall);
+u64 __weak hv_para_get_synic_register(unsigned int reg)
+{
+ return ~0ULL;
+}
+EXPORT_SYMBOL_GPL(hv_para_get_synic_register);
+
+void __weak hv_para_set_synic_register(unsigned int reg, u64 val)
+{
+}
+EXPORT_SYMBOL_GPL(hv_para_set_synic_register);
+
void hv_identify_partition_type(void)
{
/* Assume guest role */
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 4a01797d4851..9ac6f5520287 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -15,6 +15,7 @@
#include <linux/list.h>
#include <linux/bitops.h>
#include <asm/sync_bitops.h>
+#include <asm/mshyperv.h>
#include <linux/atomic.h>
#include <linux/hyperv.h>
#include <linux/interrupt.h>
@@ -335,6 +336,49 @@ extern const struct vmbus_channel_message_table_entry
bool vmbus_is_confidential(void);
+#if IS_ENABLED(CONFIG_HYPERV_VMBUS)
+/* Free the message slot and signal end-of-message if required */
+static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
+{
+ /*
+ * On crash we're reading some other CPU's message page and we need
+ * to be careful: this other CPU may already had cleared the header
+ * and the host may already had delivered some other message there.
+ * In case we blindly write msg->header.message_type we're going
+ * to lose it. We can still lose a message of the same type but
+ * we count on the fact that there can only be one
+ * CHANNELMSG_UNLOAD_RESPONSE and we don't care about other messages
+ * on crash.
+ */
+ if (cmpxchg(&msg->header.message_type, old_msg_type,
+ HVMSG_NONE) != old_msg_type)
+ return;
+
+ /*
+ * The cmxchg() above does an implicit memory barrier to
+ * ensure the write to MessageType (ie set to
+ * HVMSG_NONE) happens before we read the
+ * MessagePending and EOMing. Otherwise, the EOMing
+ * will not deliver any more messages since there is
+ * no empty slot
+ */
+ if (msg->header.message_flags.msg_pending) {
+ /*
+ * This will cause message queue rescan to
+ * possibly deliver another msg from the
+ * hypervisor
+ */
+ if (vmbus_is_confidential())
+ hv_para_set_synic_register(HV_MSR_EOM, 0);
+ else
+ hv_set_msr(HV_MSR_EOM, 0);
+ }
+}
+
+extern int vmbus_interrupt;
+extern int vmbus_irq;
+#endif /* CONFIG_HYPERV_VMBUS */
+
struct hv_device *vmbus_device_create(const guid_t *type,
const guid_t *instance,
struct vmbus_channel *channel);
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index 9049a9617324..c010059f1518 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -165,46 +165,6 @@ static inline u64 hv_generate_guest_id(u64 kernel_version)
return guest_id;
}
-#if IS_ENABLED(CONFIG_HYPERV_VMBUS)
-/* Free the message slot and signal end-of-message if required */
-static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
-{
- /*
- * On crash we're reading some other CPU's message page and we need
- * to be careful: this other CPU may already had cleared the header
- * and the host may already had delivered some other message there.
- * In case we blindly write msg->header.message_type we're going
- * to lose it. We can still lose a message of the same type but
- * we count on the fact that there can only be one
- * CHANNELMSG_UNLOAD_RESPONSE and we don't care about other messages
- * on crash.
- */
- if (cmpxchg(&msg->header.message_type, old_msg_type,
- HVMSG_NONE) != old_msg_type)
- return;
-
- /*
- * The cmxchg() above does an implicit memory barrier to
- * ensure the write to MessageType (ie set to
- * HVMSG_NONE) happens before we read the
- * MessagePending and EOMing. Otherwise, the EOMing
- * will not deliver any more messages since there is
- * no empty slot
- */
- if (msg->header.message_flags.msg_pending) {
- /*
- * This will cause message queue rescan to
- * possibly deliver another msg from the
- * hypervisor
- */
- hv_set_msr(HV_MSR_EOM, 0);
- }
-}
-
-extern int vmbus_interrupt;
-extern int vmbus_irq;
-#endif /* CONFIG_HYPERV_VMBUS */
-
int hv_get_hypervisor_version(union hv_hypervisor_version_info *info);
void hv_setup_vmbus_handler(void (*handler)(void));
@@ -338,6 +298,8 @@ bool hv_is_isolation_supported(void);
bool hv_isolation_type_snp(void);
u64 hv_ghcb_hypercall(u64 control, void *input, void *output, u32 input_size);
u64 hv_tdx_hypercall(u64 control, u64 param1, u64 param2);
+u64 hv_para_get_synic_register(unsigned int reg);
+void hv_para_set_synic_register(unsigned int reg, u64 val);
void hyperv_cleanup(void);
bool hv_query_ext_cap(u64 cap_query);
void hv_setup_dma_ops(struct device *dev, bool coherent);
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v7 03/17] arch/x86: mshyperv: Discover Confidential VMBus availability
From: Roman Kisel @ 2025-10-08 23:34 UTC (permalink / raw)
To: arnd, bp, bagasdotme, 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
In-Reply-To: <20251008233419.20372-1-romank@linux.microsoft.com>
Confidential VMBus requires enabling paravisor SynIC, and
the x86_64 guest has to inspect the Virtualization Stack (VS)
CPUID leaf to see if Confidential VMBus is available. If it is,
the guest shall enable the paravisor SynIC.
Read the relevant data from the VS CPUID leaf. Refactor the
code to avoid repeating CPUID and add flags to the struct
ms_hyperv_info. For ARM64, the flag for Confidential VMBus
is not set which provides the desired behaviour for now as
it is not available on ARM64 just yet. Once ARM64 CCA guests
are supported, this flag will be set unconditionally when
running such a guest.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
arch/x86/kernel/cpu/mshyperv.c | 28 +++++++++++++++-------------
include/asm-generic/mshyperv.h | 2 ++
2 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 25773af116bc..57163c7a000f 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -434,7 +434,7 @@ EXPORT_SYMBOL_GPL(hv_get_hypervisor_version);
static void __init ms_hyperv_init_platform(void)
{
- int hv_max_functions_eax;
+ int hv_max_functions_eax, eax;
#ifdef CONFIG_PARAVIRT
pv_info.name = "Hyper-V";
@@ -469,6 +469,19 @@ static void __init ms_hyperv_init_platform(void)
pr_info("Hyper-V: running on a nested hypervisor\n");
}
+ /*
+ * There is no check against the max function for HYPERV_CPUID_VIRT_STACK_* CPUID
+ * leaves as the hypervisor doesn't handle them. Even a nested root partition (L2
+ * root) will not get them because the nested (L1) hypervisor filters them out.
+ * These are handled through intercept processing by the Windows Hyper-V stack
+ * or the paravisor.
+ */
+ eax = cpuid_eax(HYPERV_CPUID_VIRT_STACK_PROPERTIES);
+ ms_hyperv.confidential_vmbus_available =
+ eax & HYPERV_VS_PROPERTIES_EAX_CONFIDENTIAL_VMBUS_AVAILABLE;
+ ms_hyperv.msi_ext_dest_id =
+ eax & HYPERV_VS_PROPERTIES_EAX_EXTENDED_IOAPIC_RTE;
+
if (ms_hyperv.features & HV_ACCESS_FREQUENCY_MSRS &&
ms_hyperv.misc_features & HV_FEATURE_FREQUENCY_MSRS_AVAILABLE) {
x86_platform.calibrate_tsc = hv_get_tsc_khz;
@@ -668,21 +681,10 @@ static bool __init ms_hyperv_x2apic_available(void)
* pci-hyperv host bridge.
*
* Note: for a Hyper-V root partition, this will always return false.
- * The hypervisor doesn't expose these HYPERV_CPUID_VIRT_STACK_* cpuids by
- * default, they are implemented as intercepts by the Windows Hyper-V stack.
- * Even a nested root partition (L2 root) will not get them because the
- * nested (L1) hypervisor filters them out.
*/
static bool __init ms_hyperv_msi_ext_dest_id(void)
{
- u32 eax;
-
- eax = cpuid_eax(HYPERV_CPUID_VIRT_STACK_INTERFACE);
- if (eax != HYPERV_VS_INTERFACE_EAX_SIGNATURE)
- return false;
-
- eax = cpuid_eax(HYPERV_CPUID_VIRT_STACK_PROPERTIES);
- return eax & HYPERV_VS_PROPERTIES_EAX_EXTENDED_IOAPIC_RTE;
+ return ms_hyperv.msi_ext_dest_id;
}
#ifdef CONFIG_AMD_MEM_ENCRYPT
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index 64ba6bc807d9..9049a9617324 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -62,6 +62,8 @@ struct ms_hyperv_info {
};
};
u64 shared_gpa_boundary;
+ bool msi_ext_dest_id;
+ bool confidential_vmbus_available;
};
extern struct ms_hyperv_info ms_hyperv;
extern bool hv_nested;
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v7 02/17] Drivers: hv: VMBus protocol version 6.0
From: Roman Kisel @ 2025-10-08 23:34 UTC (permalink / raw)
To: arnd, bp, bagasdotme, 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
In-Reply-To: <20251008233419.20372-1-romank@linux.microsoft.com>
The confidential VMBus is supported starting from the protocol
version 6.0 onwards.
Provide the required definitions. No functional changes.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/hv/hyperv_vmbus.h | 2 ++
drivers/hv/vmbus_drv.c | 12 +++++++
include/hyperv/hvgdk_mini.h | 1 +
include/linux/hyperv.h | 69 +++++++++++++++++++++++++++----------
4 files changed, 65 insertions(+), 19 deletions(-)
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 0b450e53161e..4a01797d4851 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -333,6 +333,8 @@ extern const struct vmbus_channel_message_table_entry
/* General vmbus interface */
+bool vmbus_is_confidential(void);
+
struct hv_device *vmbus_device_create(const guid_t *type,
const guid_t *instance,
struct vmbus_channel *channel);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 69591dc7bad2..3c414560fa5f 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -56,6 +56,18 @@ static long __percpu *vmbus_evt;
int vmbus_irq;
int vmbus_interrupt;
+/*
+ * If the Confidential VMBus is used, the data on the "wire" is not
+ * visible to either the host or the hypervisor.
+ */
+static bool is_confidential;
+
+bool vmbus_is_confidential(void)
+{
+ return is_confidential;
+}
+EXPORT_SYMBOL_GPL(vmbus_is_confidential);
+
/*
* The panic notifier below is responsible solely for unloading the
* vmbus connection, which is necessary in a panic event.
diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
index 77abddfc750e..7f730a0e54e6 100644
--- a/include/hyperv/hvgdk_mini.h
+++ b/include/hyperv/hvgdk_mini.h
@@ -260,6 +260,7 @@ union hv_hypervisor_version_info {
#define HYPERV_CPUID_VIRT_STACK_PROPERTIES 0x40000082
/* Support for the extended IOAPIC RTE format */
#define HYPERV_VS_PROPERTIES_EAX_EXTENDED_IOAPIC_RTE BIT(2)
+#define HYPERV_VS_PROPERTIES_EAX_CONFIDENTIAL_VMBUS_AVAILABLE BIT(3)
#define HYPERV_HYPERVISOR_PRESENT_BIT 0x80000000
#define HYPERV_CPUID_MIN 0x40000005
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 59826c89171c..dfc516c1c719 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -265,16 +265,18 @@ static inline u32 hv_get_avail_to_write_percent(
* Linux kernel.
*/
-#define VERSION_WS2008 ((0 << 16) | (13))
-#define VERSION_WIN7 ((1 << 16) | (1))
-#define VERSION_WIN8 ((2 << 16) | (4))
-#define VERSION_WIN8_1 ((3 << 16) | (0))
-#define VERSION_WIN10 ((4 << 16) | (0))
-#define VERSION_WIN10_V4_1 ((4 << 16) | (1))
-#define VERSION_WIN10_V5 ((5 << 16) | (0))
-#define VERSION_WIN10_V5_1 ((5 << 16) | (1))
-#define VERSION_WIN10_V5_2 ((5 << 16) | (2))
-#define VERSION_WIN10_V5_3 ((5 << 16) | (3))
+#define VMBUS_MAKE_VERSION(MAJ, MIN) ((((u32)MAJ) << 16) | (MIN))
+#define VERSION_WS2008 VMBUS_MAKE_VERSION(0, 13)
+#define VERSION_WIN7 VMBUS_MAKE_VERSION(1, 1)
+#define VERSION_WIN8 VMBUS_MAKE_VERSION(2, 4)
+#define VERSION_WIN8_1 VMBUS_MAKE_VERSION(3, 0)
+#define VERSION_WIN10 VMBUS_MAKE_VERSION(4, 0)
+#define VERSION_WIN10_V4_1 VMBUS_MAKE_VERSION(4, 1)
+#define VERSION_WIN10_V5 VMBUS_MAKE_VERSION(5, 0)
+#define VERSION_WIN10_V5_1 VMBUS_MAKE_VERSION(5, 1)
+#define VERSION_WIN10_V5_2 VMBUS_MAKE_VERSION(5, 2)
+#define VERSION_WIN10_V5_3 VMBUS_MAKE_VERSION(5, 3)
+#define VERSION_WIN10_V6_0 VMBUS_MAKE_VERSION(6, 0)
/* Make maximum size of pipe payload of 16K */
#define MAX_PIPE_DATA_PAYLOAD (sizeof(u8) * 16384)
@@ -335,14 +337,22 @@ struct vmbus_channel_offer {
} __packed;
/* Server Flags */
-#define VMBUS_CHANNEL_ENUMERATE_DEVICE_INTERFACE 1
-#define VMBUS_CHANNEL_SERVER_SUPPORTS_TRANSFER_PAGES 2
-#define VMBUS_CHANNEL_SERVER_SUPPORTS_GPADLS 4
-#define VMBUS_CHANNEL_NAMED_PIPE_MODE 0x10
-#define VMBUS_CHANNEL_LOOPBACK_OFFER 0x100
-#define VMBUS_CHANNEL_PARENT_OFFER 0x200
-#define VMBUS_CHANNEL_REQUEST_MONITORED_NOTIFICATION 0x400
-#define VMBUS_CHANNEL_TLNPI_PROVIDER_OFFER 0x2000
+#define VMBUS_CHANNEL_ENUMERATE_DEVICE_INTERFACE 0x0001
+/*
+ * This flag indicates that the channel is offered by the paravisor, and must
+ * use encrypted memory for the channel ring buffer.
+ */
+#define VMBUS_CHANNEL_CONFIDENTIAL_RING_BUFFER 0x0002
+/*
+ * This flag indicates that the channel is offered by the paravisor, and must
+ * use encrypted memory for GPA direct packets and additional GPADLs.
+ */
+#define VMBUS_CHANNEL_CONFIDENTIAL_EXTERNAL_MEMORY 0x0004
+#define VMBUS_CHANNEL_NAMED_PIPE_MODE 0x0010
+#define VMBUS_CHANNEL_LOOPBACK_OFFER 0x0100
+#define VMBUS_CHANNEL_PARENT_OFFER 0x0200
+#define VMBUS_CHANNEL_REQUEST_MONITORED_NOTIFICATION 0x0400
+#define VMBUS_CHANNEL_TLNPI_PROVIDER_OFFER 0x2000
struct vmpacket_descriptor {
u16 type;
@@ -621,6 +631,12 @@ struct vmbus_channel_relid_released {
u32 child_relid;
} __packed;
+/*
+ * Used by the paravisor only, means that the encrypted ring buffers and
+ * the encrypted external memory are supported
+ */
+#define VMBUS_FEATURE_FLAG_CONFIDENTIAL_CHANNELS 0x10
+
struct vmbus_channel_initiate_contact {
struct vmbus_channel_message_header header;
u32 vmbus_version_requested;
@@ -630,7 +646,8 @@ struct vmbus_channel_initiate_contact {
struct {
u8 msg_sint;
u8 msg_vtl;
- u8 reserved[6];
+ u8 reserved[2];
+ u32 feature_flags; /* VMBus version 6.0 */
};
};
u64 monitor_page1;
@@ -1003,6 +1020,10 @@ struct vmbus_channel {
/* boolean to control visibility of sysfs for ring buffer */
bool ring_sysfs_visible;
+ /* The ring buffer is encrypted */
+ bool co_ring_buffer;
+ /* The external memory is encrypted */
+ bool co_external_memory;
};
#define lock_requestor(channel, flags) \
@@ -1027,6 +1048,16 @@ u64 vmbus_request_addr_match(struct vmbus_channel *channel, u64 trans_id,
u64 rqst_addr);
u64 vmbus_request_addr(struct vmbus_channel *channel, u64 trans_id);
+static inline bool is_co_ring_buffer(const struct vmbus_channel_offer_channel *o)
+{
+ return !!(o->offer.chn_flags & VMBUS_CHANNEL_CONFIDENTIAL_RING_BUFFER);
+}
+
+static inline bool is_co_external_memory(const struct vmbus_channel_offer_channel *o)
+{
+ return !!(o->offer.chn_flags & VMBUS_CHANNEL_CONFIDENTIAL_EXTERNAL_MEMORY);
+}
+
static inline bool is_hvsock_offer(const struct vmbus_channel_offer_channel *o)
{
return !!(o->offer.chn_flags & VMBUS_CHANNEL_TLNPI_PROVIDER_OFFER);
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v7 01/17] Documentation: hyperv: Confidential VMBus
From: Roman Kisel @ 2025-10-08 23:34 UTC (permalink / raw)
To: arnd, bp, bagasdotme, 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
In-Reply-To: <20251008233419.20372-1-romank@linux.microsoft.com>
Define what the confidential VMBus is and describe what advantages
it offers on the capable hardware.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
Documentation/virt/hyperv/coco.rst | 139 ++++++++++++++++++++++++++++-
1 file changed, 138 insertions(+), 1 deletion(-)
diff --git a/Documentation/virt/hyperv/coco.rst b/Documentation/virt/hyperv/coco.rst
index c15d6fe34b4e..3231e51444da 100644
--- a/Documentation/virt/hyperv/coco.rst
+++ b/Documentation/virt/hyperv/coco.rst
@@ -178,7 +178,7 @@ These Hyper-V and VMBus memory pages are marked as decrypted:
* VMBus monitor pages
-* Synthetic interrupt controller (synic) related pages (unless supplied by
+* Synthetic interrupt controller (SynIC) related pages (unless supplied by
the paravisor)
* Per-cpu hypercall input and output pages (unless running with a paravisor)
@@ -232,6 +232,143 @@ with arguments explicitly describing the access. See
_hv_pcifront_read_config() and _hv_pcifront_write_config() and the
"use_calls" flag indicating to use hypercalls.
+Confidential VMBus
+------------------
+The confidential VMBus enables the confidential guest not to interact with
+the untrusted host partition and the untrusted hypervisor. Instead, the guest
+relies on the trusted paravisor to communicate with the devices processing
+sensitive data. The hardware (SNP or TDX) encrypts the guest memory and the
+register state while measuring the paravisor image using the platform security
+processor to ensure trusted and confidential computing.
+
+Confidential VMBus provides a secure communication channel between the guest
+and the paravisor, ensuring that sensitive data is protected from hypervisor-
+level access through memory encryption and register state isolation.
+
+Confidential VMBus is an extension of Confidential Computing (CoCo) VMs
+(a.k.a. "Isolated" VMs in Hyper-V terminology). Without Confidential VMBus,
+guest VMBus device drivers (the "VSC"s in VMBus terminology) communicate
+with VMBus servers (the VSPs) running on the Hyper-V host. The
+communication must be through memory that has been decrypted so the
+host can access it. With Confidential VMBus, one or more of the VSPs reside
+in the trusted paravisor layer in the guest VM. Since the paravisor layer also
+operates in encrypted memory, the memory used for communication with
+such VSPs does not need to be decrypted and thereby exposed to the
+Hyper-V host. The paravisor is responsible for communicating securely
+with the Hyper-V host as necessary.
+
+The data is transferred directly between the VM and a vPCI device (a.k.a.
+a PCI pass-thru device, see :doc:`vpci`) that is directly assigned to VTL2
+and that supports encrypted memory. In such a case, neither the host partition
+nor the hypervisor has any access to the data. The guest needs to establish
+a VMBus connection only with the paravisor for the channels that process
+sensitive data, and the paravisor abstracts the details of communicating
+with the specific devices away providing the guest with the well-established
+VSP (Virtual Service Provider) interface that has had support in the Hyper-V
+drivers for a decade.
+
+In the case the device does not support encrypted memory, the paravisor
+provides bounce-buffering, and although the data is not encrypted, the backing
+pages aren't mapped into the host partition through SLAT. While not impossible,
+it becomes much more difficult for the host partition to exfiltrate the data
+than it would be with a conventional VMBus connection where the host partition
+has direct access to the memory used for communication.
+
+Here is the data flow for a conventional VMBus connection (`C` stands for the
+client or VSC, `S` for the server or VSP, the `DEVICE` is a physical one, might
+be with multiple virtual functions)::
+
+ +---- GUEST ----+ +----- DEVICE ----+ +----- HOST -----+
+ | | | | | |
+ | | | | | |
+ | | | ========== |
+ | | | | | |
+ | | | | | |
+ | | | | | |
+ +----- C -------+ +-----------------+ +------- S ------+
+ || ||
+ || ||
+ +------||------------------ VMBus --------------------------||------+
+ | Interrupts, MMIO |
+ +-------------------------------------------------------------------+
+
+and the Confidential VMBus connection::
+
+ +---- GUEST --------------- VTL0 ------+ +-- DEVICE --+
+ | | | |
+ | +- PARAVISOR --------- VTL2 -----+ | | |
+ | | +-- VMBus Relay ------+ ====+================ |
+ | | | Interrupts, MMIO | | | | |
+ | | +-------- S ----------+ | | +------------+
+ | | || | |
+ | +---------+ || | |
+ | | Linux | || OpenHCL | |
+ | | kernel | || | |
+ | +---- C --+-----||---------------+ |
+ | || || |
+ +-------++------- C -------------------+ +------------+
+ || | HOST |
+ || +---- S -----+
+ +-------||----------------- VMBus ---------------------------||-----+
+ | Interrupts, MMIO |
+ +-------------------------------------------------------------------+
+
+An implementation of the VMBus relay that offers the Confidential VMBus
+channels is available in the OpenVMM project as a part of the OpenHCL
+paravisor. Please refer to
+
+ * https://openvmm.dev/, and
+ * https://github.com/microsoft/openvmm
+
+for more information about the OpenHCL paravisor.
+
+A guest that is running with a paravisor must determine at runtime if
+Confidential VMBus is supported by the current paravisor. The x86_64-specific
+approach relies on the CPUID Virtualization Stack leaf; the ARM64 implementation
+is expected to support the Confidential VMBus unconditionally when running
+ARM CCA guests.
+
+Confidential VMBus is a characteristic of the VMBus connection as a whole,
+and of each VMBus channel that is created. When a Confidential VMBus
+connection is established, the paravisor provides the guest the message-passing
+path that is used for VMBus device creation and deletion, and it provides a
+per-CPU synthetic interrupt controller (SynIC) just like the SynIC that is
+offered by the Hyper-V host. Each VMBus device that is offered to the guest
+indicates the degree to which it participates in Confidential VMBus. The offer
+indicates if the device uses encrypted ring buffers, and if the device uses
+encrypted memory for DMA that is done outside the ring buffer. These settings
+may be different for different devices using the same Confidential VMBus
+connection.
+
+Although these settings are separate, in practice it'll always be encrypted
+ring buffer only, or both encrypted ring buffer and external data. If a channel
+is offered by the paravisor with confidential VMBus, the ring buffer can always
+be encrypted since it's strictly for communication between the VTL2 paravisor
+and the VTL0 guest. However, other memory regions are often used for e.g. DMA,
+so they need to be accessible by the underlying hardware, and must be
+unencrypted (unless the device supports encrypted memory). Currently, there are
+not any VSPs in OpenHCL that support encrypted external memory, but future
+versions are expected to enable this capability.
+
+Because some devices on a Confidential VMBus may require decrypted ring buffers
+and DMA transfers, the guest must interact with two SynICs -- the one provided
+by the paravisor and the one provided by the Hyper-V host when Confidential
+VMBus is not offered. Interrupts are always signaled by the paravisor SynIC,
+but the guest must check for messages and for channel interrupts on both SynICs.
+
+In the case of a confidential VMBus, regular SynIC access by the guest is
+intercepted by the paravisor (this includes various MSRs such as the SIMP and
+SIEFP, as well as hypercalls like HvPostMessage and HvSignalEvent). If the
+guest actually wants to communicate with the hypervisor, it has to use special
+mechanisms (GHCB page on SNP, or tdcall on TDX). Messages can be of either
+kind: with confidential VMBus, messages use the paravisor SynIC, and if the
+guest chose to communicate directly to the hypervisor, they use the hypervisor
+SynIC. For interrupt signaling, some channels may be running on the host
+(non-confidential, using the VMBus relay) and use the hypervisor SynIC, and
+some on the paravisor and use its SynIC. The RelIDs are coordinated by the
+OpenHCL VMBus server and are guaranteed to be unique regardless of whether
+the channel originated on the host or the paravisor.
+
load_unaligned_zeropad()
------------------------
When transitioning memory between encrypted and decrypted, the caller of
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v7 00/17] Confidential VMBus
From: Roman Kisel @ 2025-10-08 23:34 UTC (permalink / raw)
To: arnd, bp, bagasdotme, 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
Hello all,
Enclosed is the 7th version of the patch series with all patches approved,
and the delta against the 6th version is few non-functional changes
suggested by Michael. Bagas also provided some feedback on how to use
a simpler cross-reference mechanism in the Documentation based on
some additional processing happening during "make htmldocs", and unfortunately
that wouldn't work for "make pdfdocs" so I kept the original approach
which is the standard :doc: role of Sphinx. I'll dig in into that anomaly
separately.
Since v5, the fallback mechanism for establishing the VMBus connection
is no longer used as the availability of the Confidential VMBus is
now indicated by a bit in the Virtualization Stack (VS) CPUID leaf.
The v6 patch series breaks that out into a separate patch seizing
the opportunity to refactor the code that uses the same leaf.
That is obviously an x86_64 specific technique. On ARM64, the
Confidential VMBus is expected to be required once support for ARM CCA is
implemented.
TLDR; is that these patches are for the Hyper-V guests, and the patches
allow to keep data flowing from physical devices into the guests encrypted
at the CPU level so that neither the root/host partition nor the hypervisor
can access the data being processed (they only "see" the encrypted/garbled
data) unless the guest decides to share it. The changes are backward compatible
with older systems, and their full potential is realized on hardware that
supports memory encryption.
These features also require running a paravisor, such as
OpenHCL (https://github.com/microsoft/openvmm) used in Azure. Another
implementation of the functionality available in this patch set is
available in the Hyper-V UEFI: https://github.com/microsoft/mu_msvm.
A more detailed description of the patches follows.
The guests running on Hyper-V can be confidential where the memory and the
register content are encrypted, provided that the hardware supports that
(currently support for AMD SEV-SNP and Intel TDX is implemented) and the guest
is capable of using these features. The confidential guests cannot be
introspected by the host nor the hypervisor without the guest sharing the
memory contents upon doing which the memory is decrypted.
In the confidential guests, neither the host nor the hypervisor need to be
trusted, and the guests processing sensitive data can take advantage of that.
Not trusting the host and the hypervisor (removing them from the Trusted
Computing Base aka TCB) necessitates that the method of communication
between the host and the guest be changed. Here is the data flow for a
conventional and the confidential VMBus connections (`C` stands for the
client or VSC, `S` for the server or VSP, the `DEVICE` is a physical one,
might be with multiple virtual functions):
1. Without the paravisor the devices are connected to the host, and the
host provides the device emulation or translation to the guest:
+---- GUEST ----+ +----- DEVICE ----+ +----- HOST -----+
| | | | | |
| | | | | |
| | | ========== |
| | | | | |
| | | | | |
| | | | | |
+----- C -------+ +-----------------+ +------- S ------+
|| ||
|| ||
+------||------------------ VMBus --------------------------||------+
| Interrupts, MMIO |
+-------------------------------------------------------------------+
2. With the paravisor, the devices are connected to the paravisor, and
the paravisor provides the device emulation or translation to the guest.
The guest doesn't communicate with the host directly, and the guest
communicates with the paravisor via the VMBus. The host is not trusted
in this model, and the paravisor is trusted:
+---- GUEST --------------- VTL0 ------+ +-- DEVICE --+
| | | |
| +- PARAVISOR --------- VTL2 -----+ | | |
| | +-- VMBus Relay ------+ ====+================ |
| | | Interrupts, MMIO | | | | |
| | +-------- S ----------+ | | +------------+
| | || | |
| +---------+ || | |
| | Linux | || OpenHCL | |
| | kernel | || | |
| +---- C --+-----||---------------+ |
| || || |
+-------++------- C -------------------+ +------------+
|| | HOST |
|| +---- S -----+
+-------||----------------- VMBus ---------------------------||-----+
| Interrupts, MMIO |
+-------------------------------------------------------------------+
Note that in the second case the guest doesn't need to share the memory
with the host as it communicates only with the paravisor within their
partition boundary. That is precisely the raison d'etre and the value
proposition of this patch series: equip the confidential guest to use
private (encrypted) memory and rely on the paravisor when this is
available to be more secure.
An implementation of the VMBus relay that offers the Confidential VMBus
channels is available in the OpenVMM project as a part of the OpenHCL
paravisor. Please refer to
* https://openvmm.dev/, and
* https://github.com/microsoft/openvmm
for more information about the OpenHCL paravisor. A VMBus client
that can work with the Confidential VMBus is available in the
open-source Hyper-V UEFI: https://github.com/microsoft/mu_msvm.
I'd like to thank the following people for their help with this
patch series:
* Dexuan for help with validation and the fruitful discussions,
* Easwar for reviewing the refactoring of the page allocating and
freeing in `hv.c`,
* John and Sven for the design,
* Mike for helping to avoid pitfalls when dealing with the GFP flags,
* Sven for blazing the trail and implementing the design in few
codebases.
I made sure to validate the patch series on
{TrustedLaunch(x86_64), OpenHCL} x
{SNP(x86_64), TDX(x86_64), No hardware isolation, No paravisor} x
{VMBus 5.0, VMBus 6.0} x
{arm64, x86_64}.
[V7]
- Removed the sentence about the fallible functions for setting/getting
registers of the paravisor SynIC. That was no longer valid in v6.
- Fixed grammar and whitespace in the patch for Documentation.
**Thank you, Michael!**
- Fixed the warning produced when building for i386 w/o Hyper-V.
**Thank you, kernel robot!**
**Thank you, Michael!**
[V6] https://lore.kernel.org/linux-hyperv/20251003222710.6257-1-romank@linux.microsoft.com/
- Rebased onto the latest hyperv-next tree.
- Gave another thought to the fallible routines for getting and setting
SynIC registers via paravisor introduced in the patch series, and after
Michael's feedback decided to make them infallible as now we have the
CPUID bit to indicate the availability of the Confidential VMBus. That
simplifies the code and makes it clearer and more robust - a reflection
of the improvements in the design throught the patch series iterations.
- Removed the sentence discussing the fallback mechanism in the Documentation
as it is no longer relevant.
**Thank you, Michael!**
- Avoided using the macro'es for (un)masking the proxy bit thanks to
`union hv_synic_sint`.
**Thank you, Wei!**
[V5] https://lore.kernel.org/linux-hyperv/20250828010557.123869-1-romank@linux.microsoft.com/
- Rebased onto the latest hyperv-next tree.
- Fixed build issues with the configs provided by the kernel robot.
**Thank you, kernel robot!**
- Fixed the potential NULL deref in a failure path.
**Thank you, Michael!**
- Removed the added blurb from the vmbus_drv.c with taxonomy of Hyper-V VMs
that was providing reasons for the trade-offs in the fallback code. That
code is no longer needed.
[V4] https://lore.kernel.org/linux-hyperv/20250714221545.5615-1-romank@linux.microsoft.com/
- Rebased the patch series on top of the latest hyperv-next branch,
applying changes as needed.
- Fixed typos and clarifications all around the patch series.
- Added clarifications in the patch 7 for `ms_hyperv.paravisor_present && !vmbus_is_confidential()`
and using hypercalls vs SNP or TDX specific protocols.
**Thank you, Alok!**
- Trim the Documentation changes to 80 columns.
**Thank you, Randy!**
- Make sure adhere to the RST format, actually built the PDF docs
and made sure the layout was correct.
**Thank you, Jon!**
- Better section order in Documentation.
- Fixed the commit descriptions where suggested.
- Moved EOI/EOM signaling for the confidential VMBus to the specialized function.
- Removed the unused `cpu` parameters.
- Clarified comments in the `hv_per_cpu_context` struct
- Explicitly test for NULL and only call `iounmap()` if non-NULL instead of
using `munmap()`.
- Don't deallocate SynIC pages in the CPU online and offline paths.
- Made sure the post page needs to be allocated for the future.
- Added comments to describe trade-offs.
**Thank you, Michael!**
[V3] https://lore.kernel.org/linux-hyperv/20250604004341.7194-1-romank@linux.microsoft.com/
- The patch series is rebased on top of the latest hyperv-next branch.
- Reworked the "wiring" diagram in the cover letter, added links to the
OpenVMM project and the OpenHCL paravisor.
- More precise wording in the comments and clearer code.
**Thank you, Alok!**
- Reworked the documentation patch.
- Split the patchset into much more granular patches.
- Various fixes and improvements throughout the patch series.
**Thank you, Michael!**
[V2] https://lore.kernel.org/linux-hyperv/20250511230758.160674-1-romank@linux.microsoft.com/
- The patch series is rebased on top of the latest hyperv-next branch.
- Better wording in the commit messages and the Documentation.
**Thank you, Alok and Wei!**
- Removed the patches 5 and 6 concerning turning bounce buffering off from
the previous version of the patch series as they were found to be
architecturally unsound. The value proposition of the patch series is not
diminished by this removal: these patches were an optimization and only for
the storage (for the simplicity sake) but not for the network. These changes
might be proposed in the future again after revolving the issues.
** Thanks you, Christoph, Dexuan, Dan, Michael, James, Robin! **
[V1] https://lore.kernel.org/linux-hyperv/20250409000835.285105-1-romank@linux.microsoft.com/
Roman Kisel (17):
Documentation: hyperv: Confidential VMBus
Drivers: hv: VMBus protocol version 6.0
arch/x86: mshyperv: Discover Confidential VMBus availability
arch: hyperv: Get/set SynIC synth.registers via paravisor
arch/x86: mshyperv: Trap on access for some synthetic MSRs
Drivers: hv: Rename fields for SynIC message and event pages
Drivers: hv: Allocate the paravisor SynIC pages when required
Drivers: hv: Post messages through the confidential VMBus if available
Drivers: hv: remove stale comment
Drivers: hv: Check message and event pages for non-NULL before
iounmap()
Drivers: hv: Rename the SynIC enable and disable routines
Drivers: hv: Functions for setting up and tearing down the paravisor
SynIC
Drivers: hv: Allocate encrypted buffers when requested
Drivers: hv: Free msginfo when the buffer fails to decrypt
Drivers: hv: Support confidential VMBus channels
Drivers: hv: Set the default VMBus version to 6.0
Drivers: hv: Support establishing the confidential VMBus connection
Documentation/virt/hyperv/coco.rst | 139 ++++++++++-
arch/x86/kernel/cpu/mshyperv.c | 77 ++++--
drivers/hv/channel.c | 73 ++++--
drivers/hv/channel_mgmt.c | 27 ++-
drivers/hv/connection.c | 6 +-
drivers/hv/hv.c | 372 +++++++++++++++++++----------
drivers/hv/hv_common.c | 16 ++
drivers/hv/hyperv_vmbus.h | 75 +++++-
drivers/hv/mshv_root.h | 2 +-
drivers/hv/mshv_synic.c | 6 +-
drivers/hv/ring_buffer.c | 5 +-
drivers/hv/vmbus_drv.c | 186 ++++++++++-----
include/asm-generic/mshyperv.h | 45 +---
include/hyperv/hvgdk_mini.h | 1 +
include/linux/hyperv.h | 69 ++++--
15 files changed, 793 insertions(+), 306 deletions(-)
base-commit: b595edcb24727e7f93e7962c3f6f971cc16dd29e
--
2.43.0
^ permalink raw reply
* Re: [PATCH hyperv-next v6 01/17] Documentation: hyperv: Confidential VMBus
From: Bagas Sanjaya @ 2025-10-08 23:20 UTC (permalink / raw)
To: Roman Kisel
Cc: benhill, bperkins, sunilmut, 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
In-Reply-To: <f1cd86d1-3a59-4bfa-ae97-3ab092a1f3d3@linux.microsoft.com>
[-- Attachment #1: Type: text/plain, Size: 2188 bytes --]
On Wed, Oct 08, 2025 at 03:11:35PM -0700, Roman Kisel wrote:
>
>
> On 10/7/2025 4:56 PM, Bagas Sanjaya wrote:
> > On Tue, Oct 07, 2025 at 01:38:02PM -0700, Roman Kisel wrote:
> > >
> > >
> > > On 10/6/2025 7:23 PM, Bagas Sanjaya wrote:
> > > > On Fri, Oct 03, 2025 at 03:26:54PM -0700, Roman Kisel wrote:
> > > > > +The data is transferred directly between the VM and a vPCI device (a.k.a.
> > > > > +a PCI pass-thru device, see :doc:`vpci`) that is directly assigned to VTL2
> > > > > +and that supports encrypted memory. In such a case, neither the host partition
> > > >
> > > > Nit: You can also write the cross-reference simply as vpci.rst.
> > > >
> > >
> > > Thanks for helping out! I could not find that way of cross-referencing
> > > in the Sphinx documentation though:
> > > https://www.sphinx-doc.org/en/master/usage/referencing.html#cross-referencing-documents
> >
> > That's kernel-specific extension (see Documentation/doc-guide/sphinx.rst).
> >
>
> Thanks, got it! So far, in my experience, that doesn't work for PDFs.
>
> > >
> > > I tried it out anyway. The suggestion worked out only for the HTML
> > > documentation, and would not work for the PDF one. Options attempted:
> > >
> > > 1. vpci
> > > 2. vpci.rst
> > > 3. Documentation/virt/hyperv/vpci
> > > 4. Documentation/virt/hyperv/vpci.rst
> > >
> > > and neither would produce a hyperlink inside virt.pdf. Options 2 & 4
> > > generated a hyperlink in HTML.
> >
> > That's it.
> >
> > Thanks.
> >
>
> I found in the document you referred to ("1.3.4 Cross-referencing") that
>
> "Cross-referencing from one documentation page to another can be done
> simply by writing the path to the document file, no special syntax
> required."
>
> From the document, that relies on some additional processing within the
> kernel tree (above you mentioned that, too), and that doesn't seem to
> work for PDFs. I'll stick to the :doc: syntax then used in the patch.
> I'll investigate separately why the additional processing that allows to
> simplify syntax works for HTMLs only.
OK, thanks!
--
An old man doll... just what I always wanted! - Clara
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [RFC PATCH --to=kys@microsoft.com,haiyangz@microsoft.com,wei.liu@kernel.org,decui@microsoft.com] Drivers: hv: Resolve ambiguity in hypervisor version log
From: Stanislav Kinsburskii @ 2025-10-08 22:43 UTC (permalink / raw)
To: linux-hyperv, linux-kernel
In-Reply-To: <175996333379.107949.887881974668560955.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net>
Please disregard
^ permalink raw reply
* [RFC PATCH] Drivers: hv: Resolve ambiguity in hypervisor version log
From: Stanislav Kinsburskii @ 2025-10-08 22:43 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui; +Cc: linux-hyperv, linux-kernel
Update the log message in hv_common_init to explicitly state that the
reported version is for the Microsoft Hypervisor, not the host OS.
Previously, this message was accurate for guests running on Windows
hosts, where the host and hypervisor versions matched. With support for
Linux hosts running the Hyper-V hypervisor, the host OS and hypervisor
versions may differ.
This change avoids confusion by making it clear that the version refers to
the Microsoft Hypervisor regardless of the host operating system.
Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
---
drivers/hv/hv_common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index e109a620c83fc..0289ee4ed5ebf 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -315,9 +315,9 @@ int __init hv_common_init(void)
int i;
union hv_hypervisor_version_info version;
- /* Get information about the Hyper-V host version */
+ /* Get information about the Microsoft Hypervisor version */
if (!hv_get_hypervisor_version(&version))
- pr_info("Hyper-V: Host Build %d.%d.%d.%d-%d-%d\n",
+ pr_info("Hyper-V: Hypervisor Build %d.%d.%d.%d-%d-%d\n",
version.major_version, version.minor_version,
version.build_number, version.service_number,
version.service_pack, version.service_branch);
^ permalink raw reply related
* [RFC PATCH --to=kys@microsoft.com,haiyangz@microsoft.com,wei.liu@kernel.org,decui@microsoft.com] Drivers: hv: Resolve ambiguity in hypervisor version log
From: Stanislav Kinsburskii @ 2025-10-08 22:42 UTC (permalink / raw)
Cc: linux-hyperv, linux-kernel
Update the log message in hv_common_init to explicitly state that the
reported version is for the Microsoft Hypervisor, not the host OS.
Previously, this message was accurate for guests running on Windows
hosts, where the host and hypervisor versions matched. With support for
Linux hosts running the Hyper-V hypervisor, the host OS and hypervisor
versions may differ.
This change avoids confusion by making it clear that the version refers to
the Microsoft Hypervisor regardless of the host operating system.
Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
---
drivers/hv/hv_common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index e109a620c83fc..0289ee4ed5ebf 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -315,9 +315,9 @@ int __init hv_common_init(void)
int i;
union hv_hypervisor_version_info version;
- /* Get information about the Hyper-V host version */
+ /* Get information about the Microsoft Hypervisor version */
if (!hv_get_hypervisor_version(&version))
- pr_info("Hyper-V: Host Build %d.%d.%d.%d-%d-%d\n",
+ pr_info("Hyper-V: Hypervisor Build %d.%d.%d.%d-%d-%d\n",
version.major_version, version.minor_version,
version.build_number, version.service_number,
version.service_pack, version.service_branch);
^ permalink raw reply related
* Re: [PATCH hyperv-next v6 01/17] Documentation: hyperv: Confidential VMBus
From: Roman Kisel @ 2025-10-08 22:11 UTC (permalink / raw)
To: Bagas Sanjaya
Cc: benhill, bperkins, sunilmut, 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
In-Reply-To: <aOWouGarxf0FB7ZR@archie.me>
On 10/7/2025 4:56 PM, Bagas Sanjaya wrote:
> On Tue, Oct 07, 2025 at 01:38:02PM -0700, Roman Kisel wrote:
>>
>>
>> On 10/6/2025 7:23 PM, Bagas Sanjaya wrote:
>>> On Fri, Oct 03, 2025 at 03:26:54PM -0700, Roman Kisel wrote:
>>>> +The data is transferred directly between the VM and a vPCI device (a.k.a.
>>>> +a PCI pass-thru device, see :doc:`vpci`) that is directly assigned to VTL2
>>>> +and that supports encrypted memory. In such a case, neither the host partition
>>>
>>> Nit: You can also write the cross-reference simply as vpci.rst.
>>>
>>
>> Thanks for helping out! I could not find that way of cross-referencing
>> in the Sphinx documentation though:
>> https://www.sphinx-doc.org/en/master/usage/referencing.html#cross-referencing-documents
>
> That's kernel-specific extension (see Documentation/doc-guide/sphinx.rst).
>
Thanks, got it! So far, in my experience, that doesn't work for PDFs.
>>
>> I tried it out anyway. The suggestion worked out only for the HTML
>> documentation, and would not work for the PDF one. Options attempted:
>>
>> 1. vpci
>> 2. vpci.rst
>> 3. Documentation/virt/hyperv/vpci
>> 4. Documentation/virt/hyperv/vpci.rst
>>
>> and neither would produce a hyperlink inside virt.pdf. Options 2 & 4
>> generated a hyperlink in HTML.
>
> That's it.
>
> Thanks.
>
I found in the document you referred to ("1.3.4 Cross-referencing") that
"Cross-referencing from one documentation page to another can be done
simply by writing the path to the document file, no special syntax
required."
From the document, that relies on some additional processing within the
kernel tree (above you mentioned that, too), and that doesn't seem to
work for PDFs. I'll stick to the :doc: syntax then used in the patch.
I'll investigate separately why the additional processing that allows to
simplify syntax works for HTMLs only.
Appreciate your help very much!
--
Thank you,
Roman
^ permalink raw reply
* RE: [PATCH] scsi: storvsc: Prefer returning channel with the same CPU as on the I/O issuing CPU
From: Michael Kelley @ 2025-10-08 18:24 UTC (permalink / raw)
To: Long Li, longli@linux.microsoft.com, KY Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, James E.J. Bottomley, Martin K. Petersen,
James Bottomley, linux-hyperv@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <DS3PR21MB573597BFA650534306FCA5CACEE1A@DS3PR21MB5735.namprd21.prod.outlook.com>
From: Long Li <longli@microsoft.com> Sent: Wednesday, October 8, 2025 10:20 AM
>
> > Subject: [EXTERNAL] RE: [PATCH] scsi: storvsc: Prefer returning channel with the
> > same CPU as on the I/O issuing CPU
> >
> > From: Long Li <longli@microsoft.com> Sent: Tuesday, October 7, 2025 5:56 PM
> > >
> > > > -----Original Message-----
> > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > Sent: Tuesday, October 7, 2025 8:42 AM
> > > > To: longli@linux.microsoft.com; KY Srinivasan <kys@microsoft.com>;
> > > > Haiyang Zhang <haiyangz@microsoft.com>; Wei Liu
> > > > <wei.liu@kernel.org>; Dexuan Cui <decui@microsoft.com>; James E.J.
> > > > Bottomley <James.Bottomley@HansenPartnership.com>; Martin K.
> > > > Petersen <martin.petersen@oracle.com>; James Bottomley
> > > > <JBottomley@Odin.com>; linux-hyperv@vger.kernel.org;
> > > > linux-scsi@vger.kernel.org; linux- kernel@vger.kernel.org
> > > > Cc: Long Li <longli@microsoft.com>
> > > > Subject: [EXTERNAL] RE: [PATCH] scsi: storvsc: Prefer returning
> > > > channel with the same CPU as on the I/O issuing CPU
> > > >
> > > > From: longli@linux.microsoft.com <longli@linux.microsoft.com> Sent:
> > > > Wednesday, October 1, 2025 10:06 PM
> > > > >
> > > > > When selecting an outgoing channel for I/O, storvsc tries to
> > > > > select a channel with a returning CPU that is not the same as
> > > > > issuing CPU. This worked well in the past, however it doesn't work
> > > > > well when the Hyper-V exposes a large number of channels (up to
> > > > > the number of all CPUs). Use a different CPU for returning channel is not efficient on Hyper-V.
> > > > >
> > > > > Change this behavior by preferring to the channel with the same
> > > > > CPU as the current I/O issuing CPU whenever possible.
> > > > >
> > > > > Tests have shown improvements in newer Hyper-V/Azure environment,
> > > > > and no regression with older Hyper-V/Azure environments.
> > > > >
> > > > > Tested-by: Raheel Abdul Faizy <rabdulfaizy@microsoft.com>
> > > > > Signed-off-by: Long Li <longli@microsoft.com>
> > > > > ---
> > > > > drivers/scsi/storvsc_drv.c | 96
> > > > > ++++++++++++++++++--------------------
> > > > > 1 file changed, 45 insertions(+), 51 deletions(-)
> > > > >
> > > > > diff --git a/drivers/scsi/storvsc_drv.c
> > > > > b/drivers/scsi/storvsc_drv.c index d9e59204a9c3..092939791ea0
> > > > > 100644
> > > > > --- a/drivers/scsi/storvsc_drv.c
> > > > > +++ b/drivers/scsi/storvsc_drv.c
> > > > > @@ -1406,14 +1406,19 @@ static struct vmbus_channel *get_og_chn(struct storvsc_device *stor_device,
> > > > > }
> > > > >
> > > > > /*
> > > > > - * Our channel array is sparsley populated and we
> > > > > + * Our channel array could be sparsley populated and we
> > > > > * initiated I/O on a processor/hw-q that does not
> > > > > * currently have a designated channel. Fix this.
> > > > > * The strategy is simple:
> > > > > - * I. Ensure NUMA locality
> > > > > - * II. Distribute evenly (best effort)
> > > > > + * I. Prefer the channel associated with the current CPU
> > > > > + * II. Ensure NUMA locality
> > > > > + * III. Distribute evenly (best effort)
> > > > > */
> > > > >
> > > > > + /* Prefer the channel on the I/O issuing processor/hw-q */
> > > > > + if (cpumask_test_cpu(q_num, &stor_device->alloced_cpus))
> > > > > + return stor_device->stor_chns[q_num];
> > > > > +
> > > >
> > > > Hmmm. When get_og_chn() is called, we know that stor_device-
> > > > >stor_chns[q_num] is NULL since storvsc_do_io() has already handled
> > > > >the non-
> > > > NULL case. And the checks are all done with stor_device->lock held,
> > > > so the stor_chns array can't change.
> > > > Hence the above code will return NULL, which will cause a NULL
> > > > reference when
> > > > storvsc_do_io() sends out the VMBus packet.
> > > >
> > > > My recollection is that get_og_chan() is called when there is no
> > > > channel that interrupts the current CPU (that's what it means for
> > > > stor_device-
> > > > >stor_chns[<current CPU>] to be NULL). So the algorithm must pick a
> > > > >channel
> > > > that interrupts some other CPU, preferably a CPU in the current NUMA node.
> > > > Adding code to prefer the channel associated with the current CPU
> > > > doesn't make sense in get_og_chn(), as get_og_chn() is only called
> > > > when it is already known that there is no such channel.
> > >
> > > The initial values for stor_chns[] and alloced_cpus are set in
> > > storvsc_channel_init() (for primary channel) and handle_sc_creation() (for
> > subchannels).
> >
> > OK, I agree that if the CPU bit in alloced_cpus is set, then the corresponding entry
> > in stor_chns[] will not be NULL. And if the entry in stor_chns[] is NULL, the CPU
> > bit in alloced_cpus is *not* set. All the places that manipulate these fields update
> > both so they are in sync with each other. Hence I'll agree the code you've added
> > in get_og_chn() will never return a NULL value.
> >
> > (However, FWIW the reverse is not true: If the entry in stor_chns[] is not NULL,
> > the corresponding CPU bit in alloced_cpus may or may not be set.)
> >
> > >
> > > As a result, the check for cpumask_test_cpu(q_num,
> > > &stor_device->alloced_cpus) will guarantee we are getting a channel.
> > > If the check fails, the code follows the old behavior to find a channel.
> > >
> > > This check is needed because storvsc supports
> > > change_target_cpu_callback() callback via vmbus.
> >
> > But look at the code in storvsc_do_io() where get_og_chn() is called. I've copied
> > the code here for discussion purposes. This is the only place that get_og_chn() is
> > called:
> >
> > spin_lock_irqsave(&stor_device->lock, flags);
> > outgoing_channel = stor_device->stor_chns[q_num];
> > if (outgoing_channel != NULL) {
> > spin_unlock_irqrestore(&stor_device->lock, flags);
> > goto found_channel;
> > }
> > outgoing_channel = get_og_chn(stor_device, q_num);
> > spin_unlock_irqrestore(&stor_device->lock, flags);
> >
> > The code gets the spin lock, then reads the stor_chns[] entry. If the entry is non-
> > NULL, then we've found a suitable channel and get_og_chn() is *not* called. The
> > only time get_og_chan() is called is when the stor_chn[] entry
> > *is* NULL, which also means that the CPU bit in alloced_cpus is *not* set.
> > So the check you've added in get_og_chn() can never be true and the check is not
> > needed. You said the check is needed because of change_target_cpu_callback(),
> > which will invoke storvsc_change_target_cpu(). But I don't see how that matters
> > given the checks done before get_og_chn() is called. The spin lock synchronizes
> > with any changes made by storvsc_change_target_cpu().
>
> I agree this check may not be necessary. Given the current patch is correct, how about I
> submit another patch to remove this check after more testing are done?
That's OK with me.
>
> >
> > Related, there are a couple of occurrences of code like this:
> >
> > for_each_cpu_wrap(tgt_cpu, &stor_device->alloced_cpus,
> > q_num + 1) {
> > channel = READ_ONCE(stor_device->stor_chns[tgt_cpu]);
> > if (!channel)
> > continue;
> >
> > Given your point that the alloced_cpus and stor_chns[] entries are always in sync
> > with each other, the check for channel being NULL is not necessary.
>
> I don't' agree with removing the check for NULL. This code follows the existing storvsc behavior
> on checking allocated CPUs. Looking at storvsc_change_target_cpu(), it changes stor_chns
> before changing alloced_cpus. It can run at the same time as this code. I think we may need
> to add some memory barriers in storvsc_change_target_cpu(), but that is for another topic.
Yes, you are right. The spin lock isn't held in these cases, so the checks for NULL
should stay.
Michael
^ permalink raw reply
* RE: [PATCH] scsi: storvsc: Prefer returning channel with the same CPU as on the I/O issuing CPU
From: Long Li @ 2025-10-08 17:20 UTC (permalink / raw)
To: Michael Kelley, longli@linux.microsoft.com, KY Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, James E.J. Bottomley,
Martin K. Petersen, James Bottomley, linux-hyperv@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <SN6PR02MB4157F816858A01D480FB203ED4E1A@SN6PR02MB4157.namprd02.prod.outlook.com>
> Subject: [EXTERNAL] RE: [PATCH] scsi: storvsc: Prefer returning channel with the
> same CPU as on the I/O issuing CPU
>
> From: Long Li <longli@microsoft.com> Sent: Tuesday, October 7, 2025 5:56 PM
> >
> > > -----Original Message-----
> > > From: Michael Kelley <mhklinux@outlook.com>
> > > Sent: Tuesday, October 7, 2025 8:42 AM
> > > To: longli@linux.microsoft.com; KY Srinivasan <kys@microsoft.com>;
> > > Haiyang Zhang <haiyangz@microsoft.com>; Wei Liu
> > > <wei.liu@kernel.org>; Dexuan Cui <decui@microsoft.com>; James E.J.
> > > Bottomley <James.Bottomley@HansenPartnership.com>; Martin K.
> > > Petersen <martin.petersen@oracle.com>; James Bottomley
> > > <JBottomley@Odin.com>; linux-hyperv@vger.kernel.org;
> > > linux-scsi@vger.kernel.org; linux- kernel@vger.kernel.org
> > > Cc: Long Li <longli@microsoft.com>
> > > Subject: [EXTERNAL] RE: [PATCH] scsi: storvsc: Prefer returning
> > > channel with the same CPU as on the I/O issuing CPU
> > >
> > > From: longli@linux.microsoft.com <longli@linux.microsoft.com> Sent:
> > > Wednesday, October 1, 2025 10:06 PM
> > > >
> > > > When selecting an outgoing channel for I/O, storvsc tries to
> > > > select a channel with a returning CPU that is not the same as
> > > > issuing CPU. This worked well in the past, however it doesn't work
> > > > well when the Hyper-V exposes a large number of channels (up to
> > > > the number of all CPUs). Use a different CPU for returning channel is not
> efficient on Hyper-V.
> > > >
> > > > Change this behavior by preferring to the channel with the same
> > > > CPU as the current I/O issuing CPU whenever possible.
> > > >
> > > > Tests have shown improvements in newer Hyper-V/Azure environment,
> > > > and no regression with older Hyper-V/Azure environments.
> > > >
> > > > Tested-by: Raheel Abdul Faizy <rabdulfaizy@microsoft.com>
> > > > Signed-off-by: Long Li <longli@microsoft.com>
> > > > ---
> > > > drivers/scsi/storvsc_drv.c | 96
> > > > ++++++++++++++++++--------------------
> > > > 1 file changed, 45 insertions(+), 51 deletions(-)
> > > >
> > > > diff --git a/drivers/scsi/storvsc_drv.c
> > > > b/drivers/scsi/storvsc_drv.c index d9e59204a9c3..092939791ea0
> > > > 100644
> > > > --- a/drivers/scsi/storvsc_drv.c
> > > > +++ b/drivers/scsi/storvsc_drv.c
> > > > @@ -1406,14 +1406,19 @@ static struct vmbus_channel
> *get_og_chn(struct storvsc_device *stor_device,
> > > > }
> > > >
> > > > /*
> > > > - * Our channel array is sparsley populated and we
> > > > + * Our channel array could be sparsley populated and we
> > > > * initiated I/O on a processor/hw-q that does not
> > > > * currently have a designated channel. Fix this.
> > > > * The strategy is simple:
> > > > - * I. Ensure NUMA locality
> > > > - * II. Distribute evenly (best effort)
> > > > + * I. Prefer the channel associated with the current CPU
> > > > + * II. Ensure NUMA locality
> > > > + * III. Distribute evenly (best effort)
> > > > */
> > > >
> > > > + /* Prefer the channel on the I/O issuing processor/hw-q */
> > > > + if (cpumask_test_cpu(q_num, &stor_device->alloced_cpus))
> > > > + return stor_device->stor_chns[q_num];
> > > > +
> > >
> > > Hmmm. When get_og_chn() is called, we know that stor_device-
> > > >stor_chns[q_num] is NULL since storvsc_do_io() has already handled
> > > >the non-
> > > NULL case. And the checks are all done with stor_device->lock held,
> > > so the stor_chns array can't change.
> > > Hence the above code will return NULL, which will cause a NULL
> > > reference when
> > > storvsc_do_io() sends out the VMBus packet.
> > >
> > > My recollection is that get_og_chan() is called when there is no
> > > channel that interrupts the current CPU (that's what it means for
> > > stor_device-
> > > >stor_chns[<current CPU>] to be NULL). So the algorithm must pick a
> > > >channel
> > > that interrupts some other CPU, preferably a CPU in the current NUMA node.
> > > Adding code to prefer the channel associated with the current CPU
> > > doesn't make sense in get_og_chn(), as get_og_chn() is only called
> > > when it is already known that there is no such channel.
> >
> > The initial values for stor_chns[] and alloced_cpus are set in
> > storvsc_channel_init() (for primary channel) and handle_sc_creation() (for
> subchannels).
>
> OK, I agree that if the CPU bit in alloced_cpus is set, then the corresponding entry
> in stor_chns[] will not be NULL. And if the entry in stor_chns[] is NULL, the CPU
> bit in alloced_cpus is *not* set. All the places that manipulate these fields update
> both so they are in sync with each other. Hence I'll agree the code you've added
> in get_og_chn() will never return a NULL value.
>
> (However, FWIW the reverse is not true: If the entry in stor_chns[] is not NULL,
> the corresponding CPU bit in alloced_cpus may or may not be set.)
>
> >
> > As a result, the check for cpumask_test_cpu(q_num,
> > &stor_device->alloced_cpus) will guarantee we are getting a channel.
> > If the check fails, the code follows the old behavior to find a channel.
> >
> > This check is needed because storvsc supports
> > change_target_cpu_callback() callback via vmbus.
>
> But look at the code in storvsc_do_io() where get_og_chn() is called. I've copied
> the code here for discussion purposes. This is the only place that get_og_chn() is
> called:
>
> spin_lock_irqsave(&stor_device->lock, flags);
> outgoing_channel = stor_device->stor_chns[q_num];
> if (outgoing_channel != NULL) {
> spin_unlock_irqrestore(&stor_device->lock, flags);
> goto found_channel;
> }
> outgoing_channel = get_og_chn(stor_device, q_num);
> spin_unlock_irqrestore(&stor_device->lock, flags);
>
> The code gets the spin lock, then reads the stor_chns[] entry. If the entry is non-
> NULL, then we've found a suitable channel and get_og_chn() is *not* called. The
> only time get_og_chan() is called is when the stor_chn[] entry
> *is* NULL, which also means that the CPU bit in alloced_cpus is *not* set.
> So the check you've added in get_og_chn() can never be true and the check is not
> needed. You said the check is needed because of change_target_cpu_callback(),
> which will invoke storvsc_change_target_cpu(). But I don't see how that matters
> given the checks done before get_og_chn() is called. The spin lock synchronizes
> with any changes made by storvsc_change_target_cpu().
I agree this check may not be necessary. Given the current patch is correct, how about I
submit another patch to remove this check after more testing are done?
>
> Related, there are a couple of occurrences of code like this:
>
> for_each_cpu_wrap(tgt_cpu, &stor_device->alloced_cpus,
> q_num + 1) {
> channel = READ_ONCE(stor_device->stor_chns[tgt_cpu]);
> if (!channel)
> continue;
>
> Given your point that the alloced_cpus and stor_chns[] entries are always in sync
> with each other, the check for channel being NULL is not necessary.
I don't' agree with removing the check for NULL. This code follows the existing storvsc behavior
on checking allocated CPUs. Looking at storvsc_change_target_cpu(), it changes stor_chns
before changing alloced_cpus. It can run at the same time as this code. I think we may need
to add some memory barriers in storvsc_change_target_cpu(), but that is for another topic.
Thanks,
Long
^ permalink raw reply
* RE: [PATCH] scsi: storvsc: Prefer returning channel with the same CPU as on the I/O issuing CPU
From: Michael Kelley @ 2025-10-08 15:30 UTC (permalink / raw)
To: Long Li, longli@linux.microsoft.com, KY Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, James E.J. Bottomley, Martin K. Petersen,
James Bottomley, linux-hyperv@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <DS3PR21MB573566DF7A81D555552DE8A7CEE1A@DS3PR21MB5735.namprd21.prod.outlook.com>
From: Long Li <longli@microsoft.com> Sent: Tuesday, October 7, 2025 5:56 PM
>
> > -----Original Message-----
> > From: Michael Kelley <mhklinux@outlook.com>
> > Sent: Tuesday, October 7, 2025 8:42 AM
> > To: longli@linux.microsoft.com; KY Srinivasan <kys@microsoft.com>; Haiyang
> > Zhang <haiyangz@microsoft.com>; Wei Liu <wei.liu@kernel.org>; Dexuan Cui
> > <decui@microsoft.com>; James E.J. Bottomley
> > <James.Bottomley@HansenPartnership.com>; Martin K. Petersen
> > <martin.petersen@oracle.com>; James Bottomley <JBottomley@Odin.com>;
> > linux-hyperv@vger.kernel.org; linux-scsi@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Cc: Long Li <longli@microsoft.com>
> > Subject: [EXTERNAL] RE: [PATCH] scsi: storvsc: Prefer returning channel with the
> > same CPU as on the I/O issuing CPU
> >
> > From: longli@linux.microsoft.com <longli@linux.microsoft.com> Sent:
> > Wednesday, October 1, 2025 10:06 PM
> > >
> > > When selecting an outgoing channel for I/O, storvsc tries to select a
> > > channel with a returning CPU that is not the same as issuing CPU. This
> > > worked well in the past, however it doesn't work well when the Hyper-V
> > > exposes a large number of channels (up to the number of all CPUs). Use
> > > a different CPU for returning channel is not efficient on Hyper-V.
> > >
> > > Change this behavior by preferring to the channel with the same CPU as
> > > the current I/O issuing CPU whenever possible.
> > >
> > > Tests have shown improvements in newer Hyper-V/Azure environment, and
> > > no regression with older Hyper-V/Azure environments.
> > >
> > > Tested-by: Raheel Abdul Faizy <rabdulfaizy@microsoft.com>
> > > Signed-off-by: Long Li <longli@microsoft.com>
> > > ---
> > > drivers/scsi/storvsc_drv.c | 96
> > > ++++++++++++++++++--------------------
> > > 1 file changed, 45 insertions(+), 51 deletions(-)
> > >
> > > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > > index d9e59204a9c3..092939791ea0 100644
> > > --- a/drivers/scsi/storvsc_drv.c
> > > +++ b/drivers/scsi/storvsc_drv.c
> > > @@ -1406,14 +1406,19 @@ static struct vmbus_channel *get_og_chn(struct storvsc_device *stor_device,
> > > }
> > >
> > > /*
> > > - * Our channel array is sparsley populated and we
> > > + * Our channel array could be sparsley populated and we
> > > * initiated I/O on a processor/hw-q that does not
> > > * currently have a designated channel. Fix this.
> > > * The strategy is simple:
> > > - * I. Ensure NUMA locality
> > > - * II. Distribute evenly (best effort)
> > > + * I. Prefer the channel associated with the current CPU
> > > + * II. Ensure NUMA locality
> > > + * III. Distribute evenly (best effort)
> > > */
> > >
> > > + /* Prefer the channel on the I/O issuing processor/hw-q */
> > > + if (cpumask_test_cpu(q_num, &stor_device->alloced_cpus))
> > > + return stor_device->stor_chns[q_num];
> > > +
> >
> > Hmmm. When get_og_chn() is called, we know that stor_device-
> > >stor_chns[q_num] is NULL since storvsc_do_io() has already handled the non-
> > NULL case. And the checks are all done with stor_device->lock held, so the
> > stor_chns array can't change.
> > Hence the above code will return NULL, which will cause a NULL reference when
> > storvsc_do_io() sends out the VMBus packet.
> >
> > My recollection is that get_og_chan() is called when there is no channel that
> > interrupts the current CPU (that's what it means for stor_device-
> > >stor_chns[<current CPU>] to be NULL). So the algorithm must pick a channel
> > that interrupts some other CPU, preferably a CPU in the current NUMA node.
> > Adding code to prefer the channel associated with the current CPU doesn't make
> > sense in get_og_chn(), as get_og_chn() is only called when it is already known
> > that there is no such channel.
>
> The initial values for stor_chns[] and alloced_cpus are set in storvsc_channel_init() (for
> primary channel) and handle_sc_creation() (for subchannels).
OK, I agree that if the CPU bit in alloced_cpus is set, then the corresponding entry in
stor_chns[] will not be NULL. And if the entry in stor_chns[] is NULL, the CPU bit in
alloced_cpus is *not* set. All the places that manipulate these fields update both so
they are in sync with each other. Hence I'll agree the code you've added in get_og_chn()
will never return a NULL value.
(However, FWIW the reverse is not true: If the entry in stor_chns[] is not NULL, the
corresponding CPU bit in alloced_cpus may or may not be set.)
>
> As a result, the check for cpumask_test_cpu(q_num, &stor_device->alloced_cpus) will
> guarantee we are getting a channel. If the check fails, the code follows the old behavior
> to find a channel.
>
> This check is needed because storvsc supports change_target_cpu_callback() callback
> via vmbus.
But look at the code in storvsc_do_io() where get_og_chn() is called. I've copied the
code here for discussion purposes. This is the only place that get_og_chn() is called:
spin_lock_irqsave(&stor_device->lock, flags);
outgoing_channel = stor_device->stor_chns[q_num];
if (outgoing_channel != NULL) {
spin_unlock_irqrestore(&stor_device->lock, flags);
goto found_channel;
}
outgoing_channel = get_og_chn(stor_device, q_num);
spin_unlock_irqrestore(&stor_device->lock, flags);
The code gets the spin lock, then reads the stor_chns[] entry. If the entry is
non-NULL, then we've found a suitable channel and get_og_chn() is *not*
called. The only time get_og_chan() is called is when the stor_chn[] entry
*is* NULL, which also means that the CPU bit in alloced_cpus is *not* set.
So the check you've added in get_og_chn() can never be true and the check
is not needed. You said the check is needed because of
change_target_cpu_callback(), which will invoke
storvsc_change_target_cpu(). But I don't see how that matters given the
checks done before get_og_chn() is called. The spin lock synchronizes with
any changes made by storvsc_change_target_cpu().
Related, there are a couple of occurrences of code like this:
for_each_cpu_wrap(tgt_cpu, &stor_device->alloced_cpus,
q_num + 1) {
channel = READ_ONCE(stor_device->stor_chns[tgt_cpu]);
if (!channel)
continue;
Given your point that the alloced_cpus and stor_chns[] entries are
always in sync with each other, the check for channel being NULL
is not necessary.
Michael
^ permalink raw reply
* Re: [PATCH net-next] net: mana: Linearize SKB if TX SGEs exceeds hardware limit
From: Aditya Garg @ 2025-10-08 15:28 UTC (permalink / raw)
To: Eric Dumazet
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, kuba, pabeni,
longli, kotaranov, horms, shradhagupta, ernis, dipayanroy,
shirazsaleem, linux-hyperv, netdev, linux-kernel, linux-rdma,
gargaditya, ssengar
In-Reply-To: <CANn89iKwHWdUaeAsdSuZUXG-W8XwyM2oppQL9spKkex0p9-Azw@mail.gmail.com>
On 08-10-2025 20:51, Eric Dumazet wrote:
> On Wed, Oct 8, 2025 at 8:16 AM Aditya Garg
> <gargaditya@linux.microsoft.com> wrote:
>>
>> On 03-10-2025 21:45, Eric Dumazet wrote:
>>> On Fri, Oct 3, 2025 at 8:47 AM Aditya Garg
>>> <gargaditya@linux.microsoft.com> 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)
>>>> + 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)) {
>>>
>>> This will fail in many cases.
>>>
>>> This sort of check is better done in ndo_features_check()
>>>
>>> Most probably this would occur for GSO packets, so can ask a software
>>> segmentation
>>> to avoid this big and risky kmalloc() by all means.
>>>
>>> Look at idpf_features_check() which has something similar.
>>
>> Hi Eric,
>> Thank you for your review. I understand your concerns regarding the use
>> of skb_linearize() in the xmit path, as it can fail under memory
>> pressure and introduces additional overhead in the transmit path. Based
>> on your input, I will work on a v2 that will move the SGE limit check to
>> the ndo_features_check() path and for GSO skbs exceding the hw limit
>> will disable the NETIF_F_GSO_MASK to enforce software segmentation in
>> kernel before the call to xmit.
>> Also for non GSO skb exceeding the SGE hw limit should we go for using
>> skb_linearize only then or would you suggest some other approach here?
>
> I think that for non GSO, the linearization attempt is fine.
>
> Note that this is extremely unlikely for non malicious users,
> and MTU being usually small (9K or less),
> the allocation will be much smaller than a GSO packet.
Okay. Will send a v2
^ permalink raw reply
* Re: [PATCH net-next] net: mana: Linearize SKB if TX SGEs exceeds hardware limit
From: Eric Dumazet @ 2025-10-08 15:21 UTC (permalink / raw)
To: Aditya Garg
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, kuba, pabeni,
longli, kotaranov, horms, shradhagupta, ernis, dipayanroy,
shirazsaleem, linux-hyperv, netdev, linux-kernel, linux-rdma,
gargaditya, ssengar
In-Reply-To: <9d886861-2e1f-4ea8-9f2c-604243bd751b@linux.microsoft.com>
On Wed, Oct 8, 2025 at 8:16 AM Aditya Garg
<gargaditya@linux.microsoft.com> wrote:
>
> On 03-10-2025 21:45, Eric Dumazet wrote:
> > On Fri, Oct 3, 2025 at 8:47 AM Aditya Garg
> > <gargaditya@linux.microsoft.com> 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)
> >> + 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)) {
> >
> > This will fail in many cases.
> >
> > This sort of check is better done in ndo_features_check()
> >
> > Most probably this would occur for GSO packets, so can ask a software
> > segmentation
> > to avoid this big and risky kmalloc() by all means.
> >
> > Look at idpf_features_check() which has something similar.
>
> Hi Eric,
> Thank you for your review. I understand your concerns regarding the use
> of skb_linearize() in the xmit path, as it can fail under memory
> pressure and introduces additional overhead in the transmit path. Based
> on your input, I will work on a v2 that will move the SGE limit check to
> the ndo_features_check() path and for GSO skbs exceding the hw limit
> will disable the NETIF_F_GSO_MASK to enforce software segmentation in
> kernel before the call to xmit.
> Also for non GSO skb exceeding the SGE hw limit should we go for using
> skb_linearize only then or would you suggest some other approach here?
I think that for non GSO, the linearization attempt is fine.
Note that this is extremely unlikely for non malicious users,
and MTU being usually small (9K or less),
the allocation will be much smaller than a GSO packet.
^ permalink raw reply
* Re: [PATCH net-next] net: mana: Linearize SKB if TX SGEs exceeds hardware limit
From: Aditya Garg @ 2025-10-08 15:18 UTC (permalink / raw)
To: Simon Horman
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: <20251004093805.GB3060232@horms.kernel.org>
On 04-10-2025 15:08, Simon Horman wrote:
> 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.
Hi Simon,
I will take care of yours and Eric's comment in v2 of this patch.
Regards,
Aditya
^ permalink raw reply
* Re: [PATCH net-next] net: mana: Linearize SKB if TX SGEs exceeds hardware limit
From: Aditya Garg @ 2025-10-08 15:16 UTC (permalink / raw)
To: Eric Dumazet
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, kuba, pabeni,
longli, kotaranov, horms, shradhagupta, ernis, dipayanroy,
shirazsaleem, linux-hyperv, netdev, linux-kernel, linux-rdma,
gargaditya, ssengar
In-Reply-To: <CANn89iJwkbxC5HvSKmk807K-3HY+YR1kt-LhcYwnoFLAaeVVow@mail.gmail.com>
On 03-10-2025 21:45, Eric Dumazet wrote:
> On Fri, Oct 3, 2025 at 8:47 AM Aditya Garg
> <gargaditya@linux.microsoft.com> 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)
>> + 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)) {
>
> This will fail in many cases.
>
> This sort of check is better done in ndo_features_check()
>
> Most probably this would occur for GSO packets, so can ask a software
> segmentation
> to avoid this big and risky kmalloc() by all means.
>
> Look at idpf_features_check() which has something similar.
Hi Eric,
Thank you for your review. I understand your concerns regarding the use
of skb_linearize() in the xmit path, as it can fail under memory
pressure and introduces additional overhead in the transmit path. Based
on your input, I will work on a v2 that will move the SGE limit check to
the ndo_features_check() path and for GSO skbs exceding the hw limit
will disable the NETIF_F_GSO_MASK to enforce software segmentation in
kernel before the call to xmit.
Also for non GSO skb exceeding the SGE hw limit should we go for using
skb_linearize only then or would you suggest some other approach here?
Regards,
Aditya
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox