* [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 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 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 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 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 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 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 13/17] Drivers: hv: Allocate encrypted buffers when requested
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 is built around using buffers not shared with
the host.
Support allocating encrypted buffers when requested.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Tianyu Lan <tiala@microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/hv/channel.c | 49 +++++++++++++++++++++++----------------
drivers/hv/hyperv_vmbus.h | 3 ++-
drivers/hv/ring_buffer.c | 5 ++--
3 files changed, 34 insertions(+), 23 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 162d6aeece7b..d69713201bef 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -444,20 +444,23 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
return ret;
}
- /*
- * Set the "decrypted" flag to true for the set_memory_decrypted()
- * success case. In the failure case, the encryption state of the
- * memory is unknown. Leave "decrypted" as true to ensure the
- * memory will be leaked instead of going back on the free list.
- */
- gpadl->decrypted = true;
- ret = set_memory_decrypted((unsigned long)kbuffer,
- PFN_UP(size));
- if (ret) {
- dev_warn(&channel->device_obj->device,
- "Failed to set host visibility for new GPADL %d.\n",
- ret);
- return ret;
+ gpadl->decrypted = !((channel->co_external_memory && type == HV_GPADL_BUFFER) ||
+ (channel->co_ring_buffer && type == HV_GPADL_RING));
+ if (gpadl->decrypted) {
+ /*
+ * The "decrypted" flag being true assumes that set_memory_decrypted() succeeds.
+ * But if it fails, the encryption state of the memory is unknown. In that case,
+ * leave "decrypted" as true to ensure the memory is leaked instead of going back
+ * on the free list.
+ */
+ ret = set_memory_decrypted((unsigned long)kbuffer,
+ PFN_UP(size));
+ if (ret) {
+ dev_warn(&channel->device_obj->device,
+ "Failed to set host visibility for new GPADL %d.\n",
+ ret);
+ return ret;
+ }
}
init_completion(&msginfo->waitevent);
@@ -545,8 +548,10 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
* left as true so the memory is leaked instead of being
* put back on the free list.
*/
- if (!set_memory_encrypted((unsigned long)kbuffer, PFN_UP(size)))
- gpadl->decrypted = false;
+ if (gpadl->decrypted) {
+ if (!set_memory_encrypted((unsigned long)kbuffer, PFN_UP(size)))
+ gpadl->decrypted = false;
+ }
}
return ret;
@@ -677,12 +682,13 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
goto error_clean_ring;
err = hv_ringbuffer_init(&newchannel->outbound,
- page, send_pages, 0);
+ page, send_pages, 0, newchannel->co_ring_buffer);
if (err)
goto error_free_gpadl;
err = hv_ringbuffer_init(&newchannel->inbound, &page[send_pages],
- recv_pages, newchannel->max_pkt_size);
+ recv_pages, newchannel->max_pkt_size,
+ newchannel->co_ring_buffer);
if (err)
goto error_free_gpadl;
@@ -863,8 +869,11 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, struct vmbus_gpadl *gpad
kfree(info);
- ret = set_memory_encrypted((unsigned long)gpadl->buffer,
- PFN_UP(gpadl->size));
+ if (gpadl->decrypted)
+ ret = set_memory_encrypted((unsigned long)gpadl->buffer,
+ PFN_UP(gpadl->size));
+ else
+ ret = 0;
if (ret)
pr_warn("Fail to set mem host visibility in GPADL teardown %d.\n", ret);
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 552ed782bcfc..f7fc2630c054 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -201,7 +201,8 @@ extern int hv_synic_cleanup(unsigned int cpu);
void hv_ringbuffer_pre_init(struct vmbus_channel *channel);
int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
- struct page *pages, u32 pagecnt, u32 max_pkt_size);
+ struct page *pages, u32 pagecnt, u32 max_pkt_size,
+ bool confidential);
void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info);
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 23ce1fb70de1..3c421a7f78c0 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -184,7 +184,8 @@ void hv_ringbuffer_pre_init(struct vmbus_channel *channel)
/* Initialize the ring buffer. */
int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
- struct page *pages, u32 page_cnt, u32 max_pkt_size)
+ struct page *pages, u32 page_cnt, u32 max_pkt_size,
+ bool confidential)
{
struct page **pages_wraparound;
int i;
@@ -208,7 +209,7 @@ int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
ring_info->ring_buffer = (struct hv_ring_buffer *)
vmap(pages_wraparound, page_cnt * 2 - 1, VM_MAP,
- pgprot_decrypted(PAGE_KERNEL));
+ confidential ? PAGE_KERNEL : pgprot_decrypted(PAGE_KERNEL));
kfree(pages_wraparound);
if (!ring_info->ring_buffer)
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v7 14/17] Drivers: hv: Free msginfo when the buffer fails to decrypt
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 early failure path in __vmbus_establish_gpadl() doesn't deallocate
msginfo if the buffer fails to decrypt.
Fix the leak by breaking out the cleanup code into a separate function
and calling it where required.
Fixes: d4dccf353db80 ("Drivers: hv: vmbus: Mark vmbus ring buffer visible to host in Isolation VM")
Reported-by: Michael Kelley <mkhlinux@outlook.com>
Closes: https://lore.kernel.org/linux-hyperv/SN6PR02MB41573796F9787F67E0E97049D472A@SN6PR02MB4157.namprd02.prod.outlook.com
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/hv/channel.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index d69713201bef..88485d255a42 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -410,6 +410,21 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
return 0;
}
+static void vmbus_free_channel_msginfo(struct vmbus_channel_msginfo *msginfo)
+{
+ struct vmbus_channel_msginfo *submsginfo, *tmp;
+
+ if (!msginfo)
+ return;
+
+ list_for_each_entry_safe(submsginfo, tmp, &msginfo->submsglist,
+ msglistentry) {
+ kfree(submsginfo);
+ }
+
+ kfree(msginfo);
+}
+
/*
* __vmbus_establish_gpadl - Establish a GPADL for a buffer or ringbuffer
*
@@ -429,7 +444,7 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
struct vmbus_channel_gpadl_header *gpadlmsg;
struct vmbus_channel_gpadl_body *gpadl_body;
struct vmbus_channel_msginfo *msginfo = NULL;
- struct vmbus_channel_msginfo *submsginfo, *tmp;
+ struct vmbus_channel_msginfo *submsginfo;
struct list_head *curr;
u32 next_gpadl_handle;
unsigned long flags;
@@ -459,6 +474,7 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
dev_warn(&channel->device_obj->device,
"Failed to set host visibility for new GPADL %d.\n",
ret);
+ vmbus_free_channel_msginfo(msginfo);
return ret;
}
}
@@ -535,12 +551,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
list_del(&msginfo->msglistentry);
spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
- list_for_each_entry_safe(submsginfo, tmp, &msginfo->submsglist,
- msglistentry) {
- kfree(submsginfo);
- }
- kfree(msginfo);
+ vmbus_free_channel_msginfo(msginfo);
if (ret) {
/*
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v7 15/17] Drivers: hv: Support confidential VMBus channels
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>
To make use of Confidential VMBus channels, initialize the
co_ring_buffers and co_external_memory fields of the channel
structure.
Advertise support upon negotiating the version and compute
values for those fields and initialize them.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/hv/channel_mgmt.c | 19 +++++++++++++++++++
drivers/hv/connection.c | 3 +++
2 files changed, 22 insertions(+)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 6d66cbc9030b..74fed2c073d4 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1022,6 +1022,7 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
struct vmbus_channel_offer_channel *offer;
struct vmbus_channel *oldchannel, *newchannel;
size_t offer_sz;
+ bool co_ring_buffer, co_external_memory;
offer = (struct vmbus_channel_offer_channel *)hdr;
@@ -1034,6 +1035,22 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
return;
}
+ co_ring_buffer = is_co_ring_buffer(offer);
+ co_external_memory = is_co_external_memory(offer);
+ if (!co_ring_buffer && co_external_memory) {
+ pr_err("Invalid offer relid=%d: the ring buffer isn't encrypted\n",
+ offer->child_relid);
+ return;
+ }
+ if (co_ring_buffer || co_external_memory) {
+ if (vmbus_proto_version < VERSION_WIN10_V6_0 || !vmbus_is_confidential()) {
+ pr_err("Invalid offer relid=%d: no support for confidential VMBus\n",
+ offer->child_relid);
+ atomic_dec(&vmbus_connection.offer_in_progress);
+ return;
+ }
+ }
+
oldchannel = find_primary_channel_by_offer(offer);
if (oldchannel != NULL) {
@@ -1112,6 +1129,8 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
pr_err("Unable to allocate channel object\n");
return;
}
+ newchannel->co_ring_buffer = co_ring_buffer;
+ newchannel->co_external_memory = co_external_memory;
vmbus_setup_channel_state(newchannel, offer);
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 1fe3573ae52a..5ac9232396f7 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -105,6 +105,9 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID;
}
+ if (vmbus_is_confidential() && version >= VERSION_WIN10_V6_0)
+ msg->feature_flags = VMBUS_FEATURE_FLAG_CONFIDENTIAL_CHANNELS;
+
/*
* shared_gpa_boundary is zero in non-SNP VMs, so it's safe to always
* bitwise OR it
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v7 16/17] Drivers: hv: Set the default VMBus version to 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 by the protocol version
6.0 onwards.
Attempt to establish the VMBus 6.0 connection thus enabling
the confidential VMBus features when available.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/hv/connection.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 5ac9232396f7..5d9cb5bf2d62 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -51,6 +51,7 @@ EXPORT_SYMBOL_GPL(vmbus_proto_version);
* Linux guests and are not listed.
*/
static __u32 vmbus_versions[] = {
+ VERSION_WIN10_V6_0,
VERSION_WIN10_V5_3,
VERSION_WIN10_V5_2,
VERSION_WIN10_V5_1,
@@ -65,7 +66,7 @@ static __u32 vmbus_versions[] = {
* Maximal VMBus protocol version guests can negotiate. Useful to cap the
* VMBus version for testing and debugging purpose.
*/
-static uint max_version = VERSION_WIN10_V5_3;
+static uint max_version = VERSION_WIN10_V6_0;
module_param(max_version, uint, S_IRUGO);
MODULE_PARM_DESC(max_version,
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v7 17/17] Drivers: hv: Support establishing the confidential VMBus connection
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>
To establish the confidential VMBus connection the CoCo VM, the guest
first checks on the confidential VMBus availability, and then proceeds
to initializing the communication stack.
Implement that in the VMBus driver initialization.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/hv/vmbus_drv.c | 168 ++++++++++++++++++++++++++---------------
1 file changed, 106 insertions(+), 62 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 2b5bf672c467..0dc4692b411a 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1057,12 +1057,9 @@ static void vmbus_onmessage_work(struct work_struct *work)
kfree(ctx);
}
-void vmbus_on_msg_dpc(unsigned long data)
+static void __vmbus_on_msg_dpc(void *message_page_addr)
{
- struct hv_per_cpu_context *hv_cpu = (void *)data;
- void *page_addr = hv_cpu->hyp_synic_message_page;
- struct hv_message msg_copy, *msg = (struct hv_message *)page_addr +
- VMBUS_MESSAGE_SINT;
+ struct hv_message msg_copy, *msg;
struct vmbus_channel_message_header *hdr;
enum vmbus_channel_message_type msgtype;
const struct vmbus_channel_message_table_entry *entry;
@@ -1070,6 +1067,10 @@ void vmbus_on_msg_dpc(unsigned long data)
__u8 payload_size;
u32 message_type;
+ if (!message_page_addr)
+ return;
+ msg = (struct hv_message *)message_page_addr + VMBUS_MESSAGE_SINT;
+
/*
* 'enum vmbus_channel_message_type' is supposed to always be 'u32' as
* it is being used in 'struct vmbus_channel_message_header' definition
@@ -1195,6 +1196,14 @@ void vmbus_on_msg_dpc(unsigned long data)
vmbus_signal_eom(msg, message_type);
}
+void vmbus_on_msg_dpc(unsigned long data)
+{
+ struct hv_per_cpu_context *hv_cpu = (void *)data;
+
+ __vmbus_on_msg_dpc(hv_cpu->hyp_synic_message_page);
+ __vmbus_on_msg_dpc(hv_cpu->para_synic_message_page);
+}
+
#ifdef CONFIG_PM_SLEEP
/*
* Fake RESCIND_CHANNEL messages to clean up hv_sock channels by force for
@@ -1233,21 +1242,19 @@ static void vmbus_force_channel_rescinded(struct vmbus_channel *channel)
#endif /* CONFIG_PM_SLEEP */
/*
- * Schedule all channels with events pending
+ * Schedule all channels with events pending.
+ * The event page can be directly checked to get the id of
+ * the channel that has the interrupt pending.
*/
-static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
+static void vmbus_chan_sched(void *event_page_addr)
{
unsigned long *recv_int_page;
u32 maxbits, relid;
+ union hv_synic_event_flags *event;
- /*
- * The event page can be directly checked to get the id of
- * the channel that has the interrupt pending.
- */
- void *page_addr = hv_cpu->hyp_synic_event_page;
- union hv_synic_event_flags *event
- = (union hv_synic_event_flags *)page_addr +
- VMBUS_MESSAGE_SINT;
+ if (!event_page_addr)
+ return;
+ event = (union hv_synic_event_flags *)event_page_addr + VMBUS_MESSAGE_SINT;
maxbits = HV_EVENT_FLAGS_COUNT;
recv_int_page = event->flags;
@@ -1255,6 +1262,11 @@ static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
if (unlikely(!recv_int_page))
return;
+ /*
+ * Suggested-by: Michael Kelley <mhklinux@outlook.com>
+ * One possible optimization would be to keep track of the largest relID that's in use,
+ * and only scan up to that relID.
+ */
for_each_set_bit(relid, recv_int_page, maxbits) {
void (*callback_fn)(void *context);
struct vmbus_channel *channel;
@@ -1318,26 +1330,35 @@ static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
}
}
-static void vmbus_isr(void)
+static void vmbus_message_sched(struct hv_per_cpu_context *hv_cpu, void *message_page_addr)
{
- struct hv_per_cpu_context *hv_cpu
- = this_cpu_ptr(hv_context.cpu_context);
- void *page_addr;
struct hv_message *msg;
- vmbus_chan_sched(hv_cpu);
-
- page_addr = hv_cpu->hyp_synic_message_page;
- msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT;
+ if (!message_page_addr)
+ return;
+ msg = (struct hv_message *)message_page_addr + VMBUS_MESSAGE_SINT;
/* Check if there are actual msgs to be processed */
if (msg->header.message_type != HVMSG_NONE) {
if (msg->header.message_type == HVMSG_TIMER_EXPIRED) {
hv_stimer0_isr();
vmbus_signal_eom(msg, HVMSG_TIMER_EXPIRED);
- } else
+ } else {
tasklet_schedule(&hv_cpu->msg_dpc);
+ }
}
+}
+
+static void vmbus_isr(void)
+{
+ struct hv_per_cpu_context *hv_cpu
+ = this_cpu_ptr(hv_context.cpu_context);
+
+ vmbus_chan_sched(hv_cpu->hyp_synic_event_page);
+ vmbus_chan_sched(hv_cpu->para_synic_event_page);
+
+ vmbus_message_sched(hv_cpu, hv_cpu->hyp_synic_message_page);
+ vmbus_message_sched(hv_cpu, hv_cpu->para_synic_message_page);
add_interrupt_randomness(vmbus_interrupt);
}
@@ -1355,6 +1376,59 @@ static void vmbus_percpu_work(struct work_struct *work)
hv_synic_init(cpu);
}
+static int vmbus_alloc_synic_and_connect(void)
+{
+ int ret, cpu;
+ struct work_struct __percpu *works;
+ int hyperv_cpuhp_online;
+
+ ret = hv_synic_alloc();
+ if (ret < 0)
+ goto err_alloc;
+
+ works = alloc_percpu(struct work_struct);
+ if (!works) {
+ ret = -ENOMEM;
+ goto err_alloc;
+ }
+
+ /*
+ * Initialize the per-cpu interrupt state and stimer state.
+ * Then connect to the host.
+ */
+ cpus_read_lock();
+ for_each_online_cpu(cpu) {
+ struct work_struct *work = per_cpu_ptr(works, cpu);
+
+ INIT_WORK(work, vmbus_percpu_work);
+ schedule_work_on(cpu, work);
+ }
+
+ for_each_online_cpu(cpu)
+ flush_work(per_cpu_ptr(works, cpu));
+
+ /* Register the callbacks for possible CPU online/offline'ing */
+ ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN, "hyperv/vmbus:online",
+ hv_synic_init, hv_synic_cleanup);
+ cpus_read_unlock();
+ free_percpu(works);
+ if (ret < 0)
+ goto err_alloc;
+ hyperv_cpuhp_online = ret;
+
+ ret = vmbus_connect();
+ if (ret)
+ goto err_connect;
+ return 0;
+
+err_connect:
+ cpuhp_remove_state(hyperv_cpuhp_online);
+ return -ENODEV;
+err_alloc:
+ hv_synic_free();
+ return -ENOMEM;
+}
+
/*
* vmbus_bus_init -Main vmbus driver initialization routine.
*
@@ -1365,8 +1439,7 @@ static void vmbus_percpu_work(struct work_struct *work)
*/
static int vmbus_bus_init(void)
{
- int ret, cpu;
- struct work_struct __percpu *works;
+ int ret;
ret = hv_init();
if (ret != 0) {
@@ -1401,41 +1474,15 @@ static int vmbus_bus_init(void)
}
}
- ret = hv_synic_alloc();
- if (ret)
- goto err_alloc;
-
- works = alloc_percpu(struct work_struct);
- if (!works) {
- ret = -ENOMEM;
- goto err_alloc;
- }
-
/*
- * Initialize the per-cpu interrupt state and stimer state.
- * Then connect to the host.
+ * Cache the value as getting it involves a VM exit on x86(_64), and
+ * doing that on each VP while initializing SynIC's wastes time.
*/
- cpus_read_lock();
- for_each_online_cpu(cpu) {
- struct work_struct *work = per_cpu_ptr(works, cpu);
-
- INIT_WORK(work, vmbus_percpu_work);
- schedule_work_on(cpu, work);
- }
-
- for_each_online_cpu(cpu)
- flush_work(per_cpu_ptr(works, cpu));
-
- /* Register the callbacks for possible CPU online/offline'ing */
- ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN, "hyperv/vmbus:online",
- hv_synic_init, hv_synic_cleanup);
- cpus_read_unlock();
- free_percpu(works);
- if (ret < 0)
- goto err_alloc;
- hyperv_cpuhp_online = ret;
-
- ret = vmbus_connect();
+ is_confidential = ms_hyperv.confidential_vmbus_available;
+ if (is_confidential)
+ pr_info("Establishing connection to the confidential VMBus\n");
+ hv_para_set_sint_proxy(!is_confidential);
+ ret = vmbus_alloc_synic_and_connect();
if (ret)
goto err_connect;
@@ -1451,9 +1498,6 @@ static int vmbus_bus_init(void)
return 0;
err_connect:
- cpuhp_remove_state(hyperv_cpuhp_online);
-err_alloc:
- hv_synic_free();
if (vmbus_irq == -1) {
hv_remove_vmbus_handler();
} else {
--
2.43.0
^ permalink raw reply related
* [PATCH 0/2] Add support for clean shutdown with MSHV
From: Praveen K Paladugu @ 2025-10-09 15:58 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, tglx, mingo, linux-hyperv,
linux-kernel, bp, dave.hansen, x86, hpa, arnd
Cc: anbelski, prapal
Add support for clean shutdown of the root partition when running on MSHV
hypervisor.
Praveen K Paladugu (2):
hyperv: Add definitions for MSHV sleep state configuration
hyperv: Enable clean shutdown for root partition with MSHV
arch/x86/hyperv/hv_init.c | 7 ++
drivers/hv/hv_common.c | 118 +++++++++++++++++++++++++++++++++
include/asm-generic/mshyperv.h | 1 +
include/hyperv/hvgdk_mini.h | 4 +-
include/hyperv/hvhdk_mini.h | 33 +++++++++
5 files changed, 162 insertions(+), 1 deletion(-)
--
2.51.0
^ permalink raw reply
* [PATCH 1/2] hyperv: Add definitions for MSHV sleep state configuration
From: Praveen K Paladugu @ 2025-10-09 15:58 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, tglx, mingo, linux-hyperv,
linux-kernel, bp, dave.hansen, x86, hpa, arnd
Cc: anbelski, prapal
In-Reply-To: <20251009160501.6356-1-prapal@linux.microsoft.com>
Add the definitions required to configure sleep states in mshv hypervsior.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Co-developed-by: Anatol Belski <anbelski@linux.microsoft.com>
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
---
include/hyperv/hvgdk_mini.h | 4 +++-
include/hyperv/hvhdk_mini.h | 33 +++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
index 77abddfc750e..943df5d292f2 100644
--- a/include/hyperv/hvgdk_mini.h
+++ b/include/hyperv/hvgdk_mini.h
@@ -464,18 +464,20 @@ union hv_vp_assist_msr_contents { /* HV_REGISTER_VP_ASSIST_PAGE */
#define HVCALL_RESET_DEBUG_SESSION 0x006b
#define HVCALL_MAP_STATS_PAGE 0x006c
#define HVCALL_UNMAP_STATS_PAGE 0x006d
+#define HVCALL_SET_SYSTEM_PROPERTY 0x006f
#define HVCALL_ADD_LOGICAL_PROCESSOR 0x0076
#define HVCALL_GET_SYSTEM_PROPERTY 0x007b
#define HVCALL_MAP_DEVICE_INTERRUPT 0x007c
#define HVCALL_UNMAP_DEVICE_INTERRUPT 0x007d
#define HVCALL_RETARGET_INTERRUPT 0x007e
+#define HVCALL_ENTER_SLEEP_STATE 0x0084
#define HVCALL_NOTIFY_PORT_RING_EMPTY 0x008b
#define HVCALL_REGISTER_INTERCEPT_RESULT 0x0091
#define HVCALL_ASSERT_VIRTUAL_INTERRUPT 0x0094
#define HVCALL_CREATE_PORT 0x0095
#define HVCALL_CONNECT_PORT 0x0096
#define HVCALL_START_VP 0x0099
-#define HVCALL_GET_VP_INDEX_FROM_APIC_ID 0x009a
+#define HVCALL_GET_VP_INDEX_FROM_APIC_ID 0x009a
#define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
#define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
#define HVCALL_SIGNAL_EVENT_DIRECT 0x00c0
diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
index 858f6a3925b3..8fa86c014c25 100644
--- a/include/hyperv/hvhdk_mini.h
+++ b/include/hyperv/hvhdk_mini.h
@@ -114,10 +114,24 @@ enum hv_snp_status {
enum hv_system_property {
/* Add more values when needed */
+ HV_SYSTEM_PROPERTY_SLEEP_STATE = 3,
HV_SYSTEM_PROPERTY_SCHEDULER_TYPE = 15,
HV_DYNAMIC_PROCESSOR_FEATURE_PROPERTY = 21,
};
+enum hv_sleep_state {
+ HV_SLEEP_STATE_S1 = 1,
+ HV_SLEEP_STATE_S2 = 2,
+ HV_SLEEP_STATE_S3 = 3,
+ HV_SLEEP_STATE_S4 = 4,
+ HV_SLEEP_STATE_S5 = 5,
+ /*
+ * After hypervisor has received this, any follow up sleep
+ * state registration requests will be rejected.
+ */
+ HV_SLEEP_STATE_LOCK = 6
+};
+
enum hv_dynamic_processor_feature_property {
/* Add more values when needed */
HV_X64_DYNAMIC_PROCESSOR_FEATURE_MAX_ENCRYPTED_PARTITIONS = 13,
@@ -145,6 +159,25 @@ struct hv_output_get_system_property {
};
} __packed;
+struct hv_sleep_state_info {
+ u32 sleep_state; /* enum hv_sleep_state */
+ u8 pm1a_slp_typ;
+ u8 pm1b_slp_typ;
+} __packed;
+
+struct hv_input_set_system_property {
+ u32 property_id; /* enum hv_system_property */
+ u32 reserved;
+ union {
+ /* More fields to be filled in when needed */
+ struct hv_sleep_state_info set_sleep_state_info;
+ };
+} __packed;
+
+struct hv_input_enter_sleep_state { /* HV_INPUT_ENTER_SLEEP_STATE */
+ u32 sleep_state; /* enum hv_sleep_state */
+} __packed;
+
struct hv_input_map_stats_page {
u32 type; /* enum hv_stats_object_type */
u32 padding;
--
2.51.0
^ permalink raw reply related
* [PATCH 2/2] hyperv: Enable clean shutdown for root partition with MSHV
From: Praveen K Paladugu @ 2025-10-09 15:58 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, tglx, mingo, linux-hyperv,
linux-kernel, bp, dave.hansen, x86, hpa, arnd
Cc: anbelski, prapal
In-Reply-To: <20251009160501.6356-1-prapal@linux.microsoft.com>
This commit enables the root partition to perform a clean shutdown when
running with MSHV hypervisor.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Co-developed-by: Anatol Belski <anbelski@linux.microsoft.com>
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
---
arch/x86/hyperv/hv_init.c | 7 ++
drivers/hv/hv_common.c | 118 +++++++++++++++++++++++++++++++++
include/asm-generic/mshyperv.h | 1 +
3 files changed, 126 insertions(+)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index afdbda2dd7b7..57bd96671ead 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -510,6 +510,13 @@ void __init hyperv_init(void)
memunmap(src);
hv_remap_tsc_clocksource();
+ /*
+ * The notifier registration might fail at various hops.
+ * Corresponding error messages will land in dmesg. There is
+ * otherwise nothing that can be specifically done to handle
+ * failures here.
+ */
+ (void)hv_sleep_notifiers_register();
} else {
hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg);
wrmsrq(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index e109a620c83f..c5165deb5278 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -837,3 +837,121 @@ const char *hv_result_to_string(u64 status)
return "Unknown";
}
EXPORT_SYMBOL_GPL(hv_result_to_string);
+
+/*
+ * Corresponding sleep states have to be initialized, in order for a subsequent
+ * HVCALL_ENTER_SLEEP_STATE call to succeed. Currently only S5 state as per
+ * ACPI 6.4 chapter 7.4.2 is relevant, while S1, S2 and S3 can be supported.
+ *
+ * ACPI should be initialized and should support S5 sleep state when this method
+ * is called, so that, it can extract correct PM values and pass them to hv.
+ */
+static int hv_initialize_sleep_states(void)
+{
+ u64 status;
+ unsigned long flags;
+ struct hv_input_set_system_property *in;
+ acpi_status acpi_status;
+ u8 sleep_type_a, sleep_type_b;
+
+ if (!acpi_sleep_state_supported(ACPI_STATE_S5)) {
+ pr_err("%s: S5 sleep state not supported.\n", __func__);
+ return -ENODEV;
+ }
+
+ acpi_status = acpi_get_sleep_type_data(ACPI_STATE_S5,
+ &sleep_type_a, &sleep_type_b);
+ if (ACPI_FAILURE(acpi_status))
+ return -ENODEV;
+
+ local_irq_save(flags);
+ in = (struct hv_input_set_system_property *)(*this_cpu_ptr(
+ hyperv_pcpu_input_arg));
+
+ in->property_id = HV_SYSTEM_PROPERTY_SLEEP_STATE;
+ in->set_sleep_state_info.sleep_state = HV_SLEEP_STATE_S5;
+ in->set_sleep_state_info.pm1a_slp_typ = sleep_type_a;
+ in->set_sleep_state_info.pm1b_slp_typ = sleep_type_b;
+
+ status = hv_do_hypercall(HVCALL_SET_SYSTEM_PROPERTY, in, NULL);
+ local_irq_restore(flags);
+
+ if (!hv_result_success(status)) {
+ pr_err("%s: %s\n", __func__, hv_result_to_string(status));
+ return hv_result_to_errno(status);
+ }
+
+ return 0;
+}
+
+static int hv_call_enter_sleep_state(u32 sleep_state)
+{
+ u64 status;
+ int ret;
+ unsigned long flags;
+ struct hv_input_enter_sleep_state *in;
+
+ ret = hv_initialize_sleep_states();
+ if (ret)
+ return ret;
+
+ local_irq_save(flags);
+ in = (struct hv_input_enter_sleep_state *)
+ (*this_cpu_ptr(hyperv_pcpu_input_arg));
+ in->sleep_state = (enum hv_sleep_state)sleep_state;
+
+ status = hv_do_hypercall(HVCALL_ENTER_SLEEP_STATE, in, NULL);
+ local_irq_restore(flags);
+
+ if (!hv_result_success(status)) {
+ pr_err("%s: %s\n", __func__, hv_result_to_string(status));
+ return hv_result_to_errno(status);
+ }
+
+ return 0;
+}
+
+static int hv_reboot_notifier_handler(struct notifier_block *this,
+ unsigned long code, void *another)
+{
+ int ret = 0;
+
+ if (SYS_HALT == code || SYS_POWER_OFF == code)
+ ret = hv_call_enter_sleep_state(HV_SLEEP_STATE_S5);
+
+ return ret ? NOTIFY_DONE : NOTIFY_OK;
+}
+
+static struct notifier_block hv_reboot_notifier = {
+ .notifier_call = hv_reboot_notifier_handler,
+};
+
+static int hv_acpi_sleep_handler(u8 sleep_state, u32 pm1a_cnt, u32 pm1b_cnt)
+{
+ int ret = 0;
+
+ if (sleep_state == ACPI_STATE_S5)
+ ret = hv_call_enter_sleep_state(HV_SLEEP_STATE_S5);
+
+ return ret == 0 ? 1 : -1;
+}
+
+static int hv_acpi_extended_sleep_handler(u8 sleep_state, u32 val_a, u32 val_b)
+{
+ return hv_acpi_sleep_handler(sleep_state, val_a, val_b);
+}
+
+int hv_sleep_notifiers_register(void)
+{
+ int ret;
+
+ acpi_os_set_prepare_sleep(&hv_acpi_sleep_handler);
+ acpi_os_set_prepare_extended_sleep(&hv_acpi_extended_sleep_handler);
+
+ ret = register_reboot_notifier(&hv_reboot_notifier);
+ if (ret)
+ pr_err("%s: cannot register reboot notifier %d\n",
+ __func__, ret);
+
+ return ret;
+}
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index 64ba6bc807d9..903d089aba82 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -339,6 +339,7 @@ u64 hv_tdx_hypercall(u64 control, u64 param1, u64 param2);
void hyperv_cleanup(void);
bool hv_query_ext_cap(u64 cap_query);
void hv_setup_dma_ops(struct device *dev, bool coherent);
+int hv_sleep_notifiers_register(void);
#else /* CONFIG_HYPERV */
static inline void hv_identify_partition_type(void) {}
static inline bool hv_is_hyperv_initialized(void) { return false; }
--
2.51.0
^ permalink raw reply related
* Re: [PATCH net-next v6 9/9] selftests/vsock: add namespace tests
From: Bobby Eshleman @ 2025-10-09 16:59 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, berrange,
Bobby Eshleman
In-Reply-To: <5kgnein4cq2ymeq3cozevld3ppbzbv62usavfigpi33krqjqde@k3sn3giizzvr>
On Tue, Sep 30, 2025 at 10:58:46AM +0200, Stefano Garzarella wrote:
> On Tue, Sep 16, 2025 at 04:43:53PM -0700, Bobby Eshleman wrote:
> > From: Bobby Eshleman <bobbyeshleman@meta.com>
> >
> > Add tests for namespace support in vsock. Use socat for basic connection
> > failure tests and vsock_test for full functionality tests when
> > communication is expected to succeed. vsock_test is not used for failure
> > cases because in theory vsock_test could allow connection and some
> > traffic flow but fail on some other case (e.g., fail on MSG_ZEROCOPY).
> >
> > Tests cover all cases of clients and servers being in all variants of
> > local ns, global ns, host process, and VM process.
> >
> > Legacy tests are retained and executed in the init ns.
> >
> > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
> >
> > ---
> > Changes in v6:
> > - check for namespace support in vmtest.sh
> >
> > Changes in v5:
> > - use /proc/sys/net/vsock/ns_mode
> > - clarify logic of tests that reuse the same VM and tests that require
> > netns setup
> > - fix unassigned BUILD bug
> > ---
> > tools/testing/selftests/vsock/vmtest.sh | 954 ++++++++++++++++++++++++++++----
> > 1 file changed, 849 insertions(+), 105 deletions(-)
> >
> > diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
> > index 5e36d1068f6f..59621b32cf1a 100755
> > --- a/tools/testing/selftests/vsock/vmtest.sh
> > +++ b/tools/testing/selftests/vsock/vmtest.sh
> > @@ -7,6 +7,7 @@
> > # * virtme-ng
> > # * busybox-static (used by virtme-ng)
> > # * qemu (used by virtme-ng)
> > +# * socat
> >
> > readonly SCRIPT_DIR="$(cd -P -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)"
> > readonly KERNEL_CHECKOUT=$(realpath "${SCRIPT_DIR}"/../../../../)
> > @@ -23,7 +24,7 @@ readonly VSOCK_CID=1234
> > readonly WAIT_PERIOD=3
> > readonly WAIT_PERIOD_MAX=60
> > readonly WAIT_TOTAL=$(( WAIT_PERIOD * WAIT_PERIOD_MAX ))
> > -readonly QEMU_PIDFILE=$(mktemp /tmp/qemu_vsock_vmtest_XXXX.pid)
> > +readonly WAIT_QEMU=5
> >
> > # virtme-ng offers a netdev for ssh when using "--ssh", but we also need a
> > # control port forwarded for vsock_test. Because virtme-ng doesn't support
> > @@ -33,23 +34,146 @@ readonly QEMU_PIDFILE=$(mktemp /tmp/qemu_vsock_vmtest_XXXX.pid)
> > # add the kernel cmdline options that virtme-init uses to setup the interface.
> > readonly QEMU_TEST_PORT_FWD="hostfwd=tcp::${TEST_HOST_PORT}-:${TEST_GUEST_PORT}"
> > readonly QEMU_SSH_PORT_FWD="hostfwd=tcp::${SSH_HOST_PORT}-:${SSH_GUEST_PORT}"
> > -readonly QEMU_OPTS="\
> > - -netdev user,id=n0,${QEMU_TEST_PORT_FWD},${QEMU_SSH_PORT_FWD} \
> > - -device virtio-net-pci,netdev=n0 \
> > - -device vhost-vsock-pci,guest-cid=${VSOCK_CID} \
> > - --pidfile ${QEMU_PIDFILE} \
> > -"
>
> I expected this patch to only add new tests, but we are changing a few things.
> Are they related, or can they be moved to another preparation patch?
>
Unfortunately they are related. For example, this pidfile change is
because this patch introduces testing two VMs up at once (to test if
their CIDs collide), so they each need unique pidfiles.
Definitely can be moved to a preparation patch, will do next rev.
> > readonly KERNEL_CMDLINE="\
> > virtme.dhcp net.ifnames=0 biosdevname=0 \
> > virtme.ssh virtme_ssh_channel=tcp virtme_ssh_user=$USER \
> > "
> > readonly LOG=$(mktemp /tmp/vsock_vmtest_XXXX.log)
> > -readonly TEST_NAMES=(vm_server_host_client vm_client_host_server vm_loopback)
> > +readonly TEST_NAMES=(
> > + vm_server_host_client
> > + vm_client_host_server
> > + vm_loopback
> > + host_vsock_ns_mode_ok
>
> can we use a `ns_` prefix for all ns related tests?
>
Yep, np.
> > + host_vsock_ns_mode_write_once_ok
> > + global_same_cid_fails
> > + local_same_cid_ok
> > + global_local_same_cid_ok
> > + local_global_same_cid_ok
> > + diff_ns_global_host_connect_to_global_vm_ok
> > + diff_ns_global_host_connect_to_local_vm_fails
> > + diff_ns_global_vm_connect_to_global_host_ok
> > + diff_ns_global_vm_connect_to_local_host_fails
> > + diff_ns_local_host_connect_to_local_vm_fails
> > + diff_ns_local_vm_connect_to_local_host_fails
> > + diff_ns_global_to_local_loopback_local_fails
> > + diff_ns_local_to_global_loopback_fails
> > + diff_ns_local_to_local_loopback_fails
> > + diff_ns_global_to_global_loopback_ok
> > + same_ns_local_loopback_ok
> > + same_ns_local_host_connect_to_local_vm_ok
> > + same_ns_local_vm_connect_to_local_host_ok
> > +)
> > +
> > readonly TEST_DESCS=(
> > + # vm_server_host_client
> > "Run vsock_test in server mode on the VM and in client mode on the host."
> > +
> > + # vm_client_host_server
> > "Run vsock_test in client mode on the VM and in server mode on the host."
> > +
> > + # vm_loopback
> > "Run vsock_test using the loopback transport in the VM."
> > +
> > + # host_vsock_ns_mode_ok
> > + "Check /proc/sys/net/vsock/ns_mode strings on the host."
> > +
> > + # host_vsock_ns_mode_write_once_ok
> > + "Check /proc/sys/net/vsock/ns_mode is write-once on the host."
> > +
> > + # global_same_cid_fails
> > + "Check QEMU fails to start two VMs with same CID in two different global namespaces."
> > +
> > + # local_same_cid_ok
> > + "Check QEMU successfully starts two VMs with same CID in two different local namespaces."
> > +
> > + # global_local_same_cid_ok
> > + "Check QEMU successfully starts one VM in a global ns and then another VM in a local ns with the same CID."
> > +
> > + # local_global_same_cid_ok
> > + "Check QEMU successfully starts one VM in a local ns and then another VM in a global ns with the same CID."
> > +
> > + # diff_ns_global_host_connect_to_global_vm_ok
> > + "Run vsock_test client in global ns with server in VM in another global ns."
> > +
> > + # diff_ns_global_host_connect_to_local_vm_fails
> > + "Run socat to test a process in a global ns fails to connect to a VM in a local ns."
> > +
> > + # diff_ns_global_vm_connect_to_global_host_ok
> > + "Run vsock_test client in VM in a global ns with server in another global ns."
> > +
> > + # diff_ns_global_vm_connect_to_local_host_fails
> > + "Run socat to test a VM in a global ns fails to connect to a host process in a local ns."
> > +
> > + # diff_ns_local_host_connect_to_local_vm_fails
> > + "Run socat to test a host process in a local ns fails to connect to a VM in another local ns."
> > +
> > + # diff_ns_local_vm_connect_to_local_host_fails
> > + "Run socat to test a VM in a local ns fails to connect to a host process in another local ns."
> > +
> > + # diff_ns_global_to_local_loopback_local_fails
> > + "Run socat to test a loopback vsock in a global ns fails to connect to a vsock in a local ns."
> > +
> > + # diff_ns_local_to_global_loopback_fails
> > + "Run socat to test a loopback vsock in a local ns fails to connect to a vsock in a global ns."
> > +
> > + # diff_ns_local_to_local_loopback_fails
> > + "Run socat to test a loopback vsock in a local ns fails to connect to a vsock in another local ns."
> > +
> > + # diff_ns_global_to_global_loopback_ok
> > + "Run socat to test a loopback vsock in a global ns successfully connects to a vsock in another global ns."
> > +
> > + # same_ns_local_loopback_ok
> > + "Run socat to test a loopback vsock in a local ns successfully connects to a vsock in the same ns."
> > +
> > + # same_ns_local_host_connect_to_local_vm_ok
> > + "Run vsock_test client in a local ns with server in VM in same ns."
> > +
> > + # same_ns_local_vm_connect_to_local_host_ok
> > + "Run vsock_test client in VM in a local ns with server in same ns."
>
> Should we run some test to check edge cases like namespace deletion
> during active connections or changing ns mode from global to local while
> running.
>
Sgtm!
> > +)
> > +
> > +readonly USE_SHARED_VM=(vm_server_host_client vm_client_host_server vm_loopback)
> > +readonly USE_INIT_NETNS=(
> > + global_same_cid_fails
> > + local_same_cid_ok
> > + global_local_same_cid_ok
> > + local_global_same_cid_ok
> > + diff_ns_global_host_connect_to_global_vm_ok
> > + diff_ns_global_host_connect_to_local_vm_fails
> > + diff_ns_global_vm_connect_to_global_host_ok
> > + diff_ns_global_vm_connect_to_local_host_fails
> > + diff_ns_local_host_connect_to_local_vm_fails
> > + diff_ns_local_vm_connect_to_local_host_fails
> > + diff_ns_global_to_local_loopback_local_fails
> > + diff_ns_local_to_global_loopback_fails
> > + diff_ns_local_to_local_loopback_fails
> > + diff_ns_global_to_global_loopback_ok
> > + same_ns_local_loopback_ok
> > + same_ns_local_host_connect_to_local_vm_ok
> > + same_ns_local_vm_connect_to_local_host_ok
> > +)
> > +readonly REQUIRES_NETNS=(
> > + host_vsock_ns_mode_ok
> > + host_vsock_ns_mode_write_once_ok
> > + global_same_cid_fails
> > + local_same_cid_ok
> > + global_local_same_cid_ok
> > + local_global_same_cid_ok
> > + diff_ns_global_host_connect_to_global_vm_ok
> > + diff_ns_global_host_connect_to_local_vm_fails
> > + diff_ns_global_vm_connect_to_global_host_ok
> > + diff_ns_global_vm_connect_to_local_host_fails
> > + diff_ns_local_host_connect_to_local_vm_fails
> > + diff_ns_local_vm_connect_to_local_host_fails
> > + diff_ns_global_to_local_loopback_local_fails
> > + diff_ns_local_to_global_loopback_fails
> > + diff_ns_local_to_local_loopback_fails
> > + diff_ns_global_to_global_loopback_ok
> > + same_ns_local_loopback_ok
> > + same_ns_local_host_connect_to_local_vm_ok
> > + same_ns_local_vm_connect_to_local_host_ok
> > )
> > +readonly MODES=("local" "global")
>
> What about NS_MODES ?
>
Roger that.
> >
> > readonly LOG_LEVEL_DEBUG=0
> > readonly LOG_LEVEL_INFO=1
> > @@ -58,6 +182,12 @@ readonly LOG_LEVEL_ERROR=3
> >
> > VERBOSE="${LOG_LEVEL_WARN}"
> >
> > +# Test pass/fail counters
> > +cnt_pass=0
> > +cnt_fail=0
> > +cnt_skip=0
> > +cnt_total=0
> > +
> > usage() {
> > local name
> > local desc
> > @@ -77,7 +207,7 @@ usage() {
> > for ((i = 0; i < ${#TEST_NAMES[@]}; i++)); do
> > name=${TEST_NAMES[${i}]}
> > desc=${TEST_DESCS[${i}]}
> > - printf "\t%-35s%-35s\n" "${name}" "${desc}"
> > + printf "\t%-55s%-35s\n" "${name}" "${desc}"
> > done
> > echo
> >
> > @@ -89,21 +219,87 @@ die() {
> > exit "${KSFT_FAIL}"
> > }
> >
> > +add_namespaces() {
> > + # add namespaces local0, local1, global0, and global1
> > + for mode in "${MODES[@]}"; do
> > + ip netns add "${mode}0" 2>/dev/null
> > + ip netns add "${mode}1" 2>/dev/null
> > + done
> > +}
> > +
> > +init_namespaces() {
> > + for mode in "${MODES[@]}"; do
> > + ns_set_mode "${mode}0" "${mode}"
> > + ns_set_mode "${mode}1" "${mode}"
> > +
> > + log_host "set ns ${mode}0 to mode ${mode}"
> > + log_host "set ns ${mode}1 to mode ${mode}"
> > +
> > + # we need lo for qemu port forwarding
> > + ip netns exec "${mode}0" ip link set dev lo up
> > + ip netns exec "${mode}1" ip link set dev lo up
> > + done
> > +}
> > +
> > +del_namespaces() {
> > + for mode in "${MODES[@]}"; do
> > + ip netns del "${mode}0"
> > + ip netns del "${mode}1"
> > + log_host "removed ns ${mode}0"
> > + log_host "removed ns ${mode}1"
> > + done &>/dev/null
> > +}
> > +
> > +ns_set_mode() {
> > + local ns=$1
> > + local mode=$2
> > +
> > + echo "${mode}" | ip netns exec "${ns}" \
> > + tee /proc/sys/net/vsock/ns_mode &>/dev/null
> > +}
> > +
> > vm_ssh() {
> > - ssh -q -o UserKnownHostsFile=/dev/null -p ${SSH_HOST_PORT} localhost "$@"
> > + local ns_exec
> > +
> > + if [[ "${1}" == none ]]; then
> > + local ns_exec=""
> > + else
> > + local ns_exec="ip netns exec ${1}"
> > + fi
> > +
> > + shift
> > +
> > + ${ns_exec} ssh -q -o UserKnownHostsFile=/dev/null -p ${SSH_HOST_PORT} localhost $*
> > +
> > return $?
> > }
> >
> > cleanup() {
> > - if [[ -s "${QEMU_PIDFILE}" ]]; then
> > - pkill -SIGTERM -F "${QEMU_PIDFILE}" > /dev/null 2>&1
> > - fi
> > + del_namespaces
> > +}
> >
> > - # If failure occurred during or before qemu start up, then we need
> > - # to clean this up ourselves.
> > - if [[ -e "${QEMU_PIDFILE}" ]]; then
> > - rm "${QEMU_PIDFILE}"
> > - fi
> > +terminate_pidfiles() {
> > + local pidfile
> > +
> > + for pidfile in "$@"; do
> > + if [[ -s "${pidfile}" ]]; then
> > + pkill -SIGTERM -F "${pidfile}" 2>&1 > /dev/null
> > + fi
> > +
> > + # If failure occurred during or before qemu start up, then we need
> > + # to clean this up ourselves.
> > + if [[ -e "${pidfile}" ]]; then
> > + rm -f "${pidfile}"
> > + fi
> > + done
> > +}
> > +
> > +terminate_pids() {
> > + local pid
> > +
> > + for pid in "$@"; do
> > + kill -SIGTERM "${pid}" &>/dev/null || :
> > + done
> > }
> >
> > check_args() {
> > @@ -133,7 +329,7 @@ check_args() {
> > }
> >
> > check_deps() {
> > - for dep in vng ${QEMU} busybox pkill ssh; do
> > + for dep in vng ${QEMU} busybox pkill ssh socat; do
> > if [[ ! -x $(command -v "${dep}") ]]; then
> > echo -e "skip: dependency ${dep} not found!\n"
> > exit "${KSFT_SKIP}"
> > @@ -147,6 +343,20 @@ check_deps() {
> > fi
> > }
> >
> > +check_test_deps() {
> > + local tname=$1
> > +
> > + # If the test requires NS support, check if NS support exists
> > + # using /proc/self/ns
> > + if [[ "${tname}" =~ "${REQUIRES_NETNS[@]}" ]] &&
> > + [[ ! -e /proc/self/ns ]]; then
> > + log_host "No NS support detected for test ${tname}"
> > + return 1
> > + fi
> > +
> > + return 0
> > +}
> > +
> > check_vng() {
> > local tested_versions
> > local version
> > @@ -170,6 +380,20 @@ check_vng() {
> > fi
> > }
> >
> > +check_socat() {
> > + local support_string
> > +
> > + support_string="$(socat -V)"
> > +
> > + if [[ "${support_string}" != *"WITH_VSOCK 1"* ]]; then
> > + die "err: socat is missing vsock support"
> > + fi
> > +
> > + if [[ "${support_string}" != *"WITH_UNIX 1"* ]]; then
> > + die "err: socat is missing unix support"
> > + fi
> > +}
> > +
> > handle_build() {
> > if [[ ! "${BUILD}" -eq 1 ]]; then
> > return
> > @@ -194,9 +418,14 @@ handle_build() {
> > }
> >
> > vm_start() {
> > + local cid=$1
> > + local ns=$2
> > + local pidfile=$3
> > local logfile=/dev/null
> > local verbose_opt=""
> > + local qemu_opts=""
> > local kernel_opt=""
> > + local ns_exec=""
> > local qemu
> >
> > qemu=$(command -v "${QEMU}")
> > @@ -206,27 +435,37 @@ vm_start() {
> > logfile=/dev/stdout
> > fi
> >
> > + qemu_opts="\
> > + -netdev user,id=n0,${QEMU_TEST_PORT_FWD},${QEMU_SSH_PORT_FWD} \
> > + -device virtio-net-pci,netdev=n0 \
> > + ${QEMU_OPTS} -device vhost-vsock-pci,guest-cid=${cid} \
>
> Have we removed QEMU_OPTS, right?
>
> (I still prefer to have it defined on top, but maybe there is a reason
> to remove it)
>
> > + --pidfile ${pidfile}
> > + "
> > +
> > if [[ "${BUILD}" -eq 1 ]]; then
> > kernel_opt="${KERNEL_CHECKOUT}"
> > fi
> >
> > - vng \
> > + if [[ "${ns}" != "none" ]]; then
> > + ns_exec="ip netns exec ${ns}"
> > + fi
> > +
> > + ${ns_exec} vng \
> > --run \
> > ${kernel_opt} \
> > ${verbose_opt} \
> > - --qemu-opts="${QEMU_OPTS}" \
> > + --qemu-opts="${qemu_opts}" \
> > --qemu="${qemu}" \
> > --user root \
> > --append "${KERNEL_CMDLINE}" \
> > --rw &> ${logfile} &
> >
> > - if ! timeout ${WAIT_TOTAL} \
>
> So WAIT_TOTAL is now unused, right?
>
Yes, its definition can be dropped.
> Can you explain better this change?
>
It turned out that WAIT_TOTAL (three minutes) was very long wait time to
just see if qemu places the pidfile or not. It's a suitable wait time
for VM boot up, but qemu creates the pidfile far before that. Waiting
three minutes made the cases where we expect the VM to fail bootup to
take a a very long time. Waiting only 5 seconds to check the pidfile
offers a lot of savings.
Sounds like this should also be a different patch.
[..]
>
> Sorry, there are too many changes and reviewing it is complicated. Can you
> at least divide it into patches to fix pre-existing bugs, patches to support
> namespaces (and use init_ns for the ones we already have), and patches to
> add tests?
>
> Thanks,
> Stefano
>
No problem, totally understandable looking at it in hindsight. I'll
break it up and send that out after the window opens.
Thanks again for your patient review!
Best,
Bobby
^ permalink raw reply
* Re: [PATCH 2/2] hyperv: Enable clean shutdown for root partition with MSHV
From: Easwar Hariharan @ 2025-10-09 17:30 UTC (permalink / raw)
To: Praveen K Paladugu
Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, linux-hyperv,
linux-kernel, bp, dave.hansen, x86, hpa, arnd, easwar.hariharan,
anbelski
In-Reply-To: <20251009160501.6356-3-prapal@linux.microsoft.com>
On 10/9/2025 8:58 AM, Praveen K Paladugu wrote:
> This commit enables the root partition to perform a clean shutdown when
> running with MSHV hypervisor.
No "This commit..." please
>
> Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
> Co-developed-by: Anatol Belski <anbelski@linux.microsoft.com>
> Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
> ---
> arch/x86/hyperv/hv_init.c | 7 ++
> drivers/hv/hv_common.c | 118 +++++++++++++++++++++++++++++++++
> include/asm-generic/mshyperv.h | 1 +
> 3 files changed, 126 insertions(+)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index afdbda2dd7b7..57bd96671ead 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -510,6 +510,13 @@ void __init hyperv_init(void)
> memunmap(src);
>
> hv_remap_tsc_clocksource();
> + /*
> + * The notifier registration might fail at various hops.
> + * Corresponding error messages will land in dmesg. There is
> + * otherwise nothing that can be specifically done to handle
> + * failures here.
> + */
> + (void)hv_sleep_notifiers_register();
> } else {
> hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg);
> wrmsrq(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index e109a620c83f..c5165deb5278 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -837,3 +837,121 @@ const char *hv_result_to_string(u64 status)
> return "Unknown";
> }
> EXPORT_SYMBOL_GPL(hv_result_to_string);
> +
> +/*
> + * Corresponding sleep states have to be initialized, in order for a subsequent
> + * HVCALL_ENTER_SLEEP_STATE call to succeed. Currently only S5 state as per
> + * ACPI 6.4 chapter 7.4.2 is relevant, while S1, S2 and S3 can be supported.
> + *
> + * ACPI should be initialized and should support S5 sleep state when this method
> + * is called, so that, it can extract correct PM values and pass them to hv.
Nit: No need for this ^ comma, i.e. "...when this method is called, so that it can..."
> + */
> +static int hv_initialize_sleep_states(void)
> +{
> + u64 status;
> + unsigned long flags;
> + struct hv_input_set_system_property *in;
> + acpi_status acpi_status;
> + u8 sleep_type_a, sleep_type_b;
> +
> + if (!acpi_sleep_state_supported(ACPI_STATE_S5)) {
> + pr_err("%s: S5 sleep state not supported.\n", __func__);
> + return -ENODEV;
> + }
> +
> + acpi_status = acpi_get_sleep_type_data(ACPI_STATE_S5,
> + &sleep_type_a, &sleep_type_b);
> + if (ACPI_FAILURE(acpi_status))
> + return -ENODEV;
> +
> + local_irq_save(flags);
> + in = (struct hv_input_set_system_property *)(*this_cpu_ptr(
> + hyperv_pcpu_input_arg));
Other users don't have these casts, why is it necessary here?
> +
> + in->property_id = HV_SYSTEM_PROPERTY_SLEEP_STATE;
> + in->set_sleep_state_info.sleep_state = HV_SLEEP_STATE_S5;
> + in->set_sleep_state_info.pm1a_slp_typ = sleep_type_a;
> + in->set_sleep_state_info.pm1b_slp_typ = sleep_type_b;
> +
> + status = hv_do_hypercall(HVCALL_SET_SYSTEM_PROPERTY, in, NULL);
> + local_irq_restore(flags);
> +
> + if (!hv_result_success(status)) {
> + pr_err("%s: %s\n", __func__, hv_result_to_string(status));
> + return hv_result_to_errno(status);
> + }
> +
> + return 0;
> +}
> +
> +static int hv_call_enter_sleep_state(u32 sleep_state)
> +{
> + u64 status;
> + int ret;
> + unsigned long flags;
> + struct hv_input_enter_sleep_state *in;
> +
> + ret = hv_initialize_sleep_states();
> + if (ret)
> + return ret;
> +
> + local_irq_save(flags);
> + in = (struct hv_input_enter_sleep_state *)
> + (*this_cpu_ptr(hyperv_pcpu_input_arg));
> + in->sleep_state = (enum hv_sleep_state)sleep_state;
> +
More casts...
> + status = hv_do_hypercall(HVCALL_ENTER_SLEEP_STATE, in, NULL);
> + local_irq_restore(flags);
> +
> + if (!hv_result_success(status)) {
> + pr_err("%s: %s\n", __func__, hv_result_to_string(status));
> + return hv_result_to_errno(status);
> + }
> +
> + return 0;
> +}
> +
> +static int hv_reboot_notifier_handler(struct notifier_block *this,
> + unsigned long code, void *another)
> +{
> + int ret = 0;
> +
> + if (SYS_HALT == code || SYS_POWER_OFF == code)
Usually the variable is on the left of the comparison with the constant
> + ret = hv_call_enter_sleep_state(HV_SLEEP_STATE_S5);
> +
> + return ret ? NOTIFY_DONE : NOTIFY_OK;
> +}
> +
> +static struct notifier_block hv_reboot_notifier = {
> + .notifier_call = hv_reboot_notifier_handler,
> +};
> +
> +static int hv_acpi_sleep_handler(u8 sleep_state, u32 pm1a_cnt, u32 pm1b_cnt)
> +{
> + int ret = 0;
> +
> + if (sleep_state == ACPI_STATE_S5)
> + ret = hv_call_enter_sleep_state(HV_SLEEP_STATE_S5);
> +
> + return ret == 0 ? 1 : -1;
> +}
> +
> +static int hv_acpi_extended_sleep_handler(u8 sleep_state, u32 val_a, u32 val_b)
> +{
> + return hv_acpi_sleep_handler(sleep_state, val_a, val_b);
> +}
> +
> +int hv_sleep_notifiers_register(void)
> +{
> + int ret;
> +
> + acpi_os_set_prepare_sleep(&hv_acpi_sleep_handler);
> + acpi_os_set_prepare_extended_sleep(&hv_acpi_extended_sleep_handler);
> +
> + ret = register_reboot_notifier(&hv_reboot_notifier);
> + if (ret)
> + pr_err("%s: cannot register reboot notifier %d\n",
> + __func__, ret);
> +
> + return ret;
> +}
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index 64ba6bc807d9..903d089aba82 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -339,6 +339,7 @@ u64 hv_tdx_hypercall(u64 control, u64 param1, u64 param2);
> void hyperv_cleanup(void);
> bool hv_query_ext_cap(u64 cap_query);
> void hv_setup_dma_ops(struct device *dev, bool coherent);
> +int hv_sleep_notifiers_register(void);
Does this still work when CONFIG_HYPERV = n, i.e. do we need a stub below? Also, this looks
like it's only implemented for x86, so perhaps this declaration should be in arch/x86/include/asm/mshyperv.h
instead of asm-generic?
> #else /* CONFIG_HYPERV */
> static inline void hv_identify_partition_type(void) {}
> static inline bool hv_is_hyperv_initialized(void) { return false; }
^ permalink raw reply
* Re: [PATCH 1/2] hyperv: Add definitions for MSHV sleep state configuration
From: Easwar Hariharan @ 2025-10-09 17:30 UTC (permalink / raw)
To: Praveen K Paladugu
Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, linux-hyperv,
linux-kernel, bp, dave.hansen, x86, hpa, arnd, easwar.hariharan,
anbelski
In-Reply-To: <20251009160501.6356-2-prapal@linux.microsoft.com>
On 10/9/2025 8:58 AM, Praveen K Paladugu wrote:
> Add the definitions required to configure sleep states in mshv hypervsior.
>
> Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
> Co-developed-by: Anatol Belski <anbelski@linux.microsoft.com>
> Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
> ---
> include/hyperv/hvgdk_mini.h | 4 +++-
> include/hyperv/hvhdk_mini.h | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+), 1 deletion(-)
Reviewed-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>
^ permalink raw reply
* Re: [PATCH v2] Drivers: hv: Use better errno matches for HV_STATUS values
From: Nuno Das Neves @ 2025-10-09 18:53 UTC (permalink / raw)
To: Easwar Hariharan, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Stanislav Kinsburskii,
open list:Hyper-V/Azure CORE AND DRIVERS, open list
In-Reply-To: <20251006230821.275642-1-easwar.hariharan@linux.microsoft.com>
On 10/6/2025 4:08 PM, Easwar Hariharan wrote:
> Use a better mapping of hypervisor status codes to errno values and
> disambiguate the catch-all -EIO value. While here, remove the duplicate
> INVALID_LP_INDEX and INVALID_REGISTER_VALUES hypervisor status entries.
>
To be honest, in retrospect the idea of 'translating' the hypercall error
codes is a bit pointless. hv_result_to_errno() allows the hypercall helper
functions to be a bit cleaner, but that's about it. When debugging you
almost always want to know the actual hypercall error code. Translating
it imperfectly is often useless, and at worst creates a red
herring/obfuscates the true source of the error.
With that in mind, updating the errno mappings to be more accurate feels
like unnecessary churn. It might even be better to remove the errno mappings
altogether and just translate HV_STATUS_SUCCESS to 0 and any other error
to -EIO or some other 'signal' error code to make it more obvious that
a *hypercall* error occurred and not some other Linux error. We'd still
want to keep the table in some form because it's also used for the error
strings.
The cleanup removing the duplicates in the table is welcome.
Nuno
> Fixes: 3817854ba89201 ("hyperv: Log hypercall status codes as strings")
> Signed-off-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>
> ---
> Changes in v2: Change more values, delete duplicated entries
> v1: https://lore.kernel.org/all/20251002221347.402320-1-easwar.hariharan@linux.microsoft.com/
> ---
> drivers/hv/hv_common.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 49898d10fafff..bb32471a53d68 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -758,32 +758,30 @@ static const struct hv_status_info hv_status_infos[] = {
> _STATUS_INFO(HV_STATUS_SUCCESS, 0),
> _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_CODE, -EINVAL),
> _STATUS_INFO(HV_STATUS_INVALID_HYPERCALL_INPUT, -EINVAL),
> - _STATUS_INFO(HV_STATUS_INVALID_ALIGNMENT, -EIO),
> + _STATUS_INFO(HV_STATUS_INVALID_ALIGNMENT, -EINVAL),
> _STATUS_INFO(HV_STATUS_INVALID_PARAMETER, -EINVAL),
> - _STATUS_INFO(HV_STATUS_ACCESS_DENIED, -EIO),
> - _STATUS_INFO(HV_STATUS_INVALID_PARTITION_STATE, -EIO),
> - _STATUS_INFO(HV_STATUS_OPERATION_DENIED, -EIO),
> + _STATUS_INFO(HV_STATUS_ACCESS_DENIED, -EACCES),
> + _STATUS_INFO(HV_STATUS_INVALID_PARTITION_STATE, -EINVAL),
> + _STATUS_INFO(HV_STATUS_OPERATION_DENIED, -EACCES),
> _STATUS_INFO(HV_STATUS_UNKNOWN_PROPERTY, -EIO),
> - _STATUS_INFO(HV_STATUS_PROPERTY_VALUE_OUT_OF_RANGE, -EIO),
> + _STATUS_INFO(HV_STATUS_PROPERTY_VALUE_OUT_OF_RANGE, -ERANGE),
> _STATUS_INFO(HV_STATUS_INSUFFICIENT_MEMORY, -ENOMEM),
> _STATUS_INFO(HV_STATUS_INVALID_PARTITION_ID, -EINVAL),
> _STATUS_INFO(HV_STATUS_INVALID_VP_INDEX, -EINVAL),
> _STATUS_INFO(HV_STATUS_NOT_FOUND, -EIO),
> _STATUS_INFO(HV_STATUS_INVALID_PORT_ID, -EINVAL),
> _STATUS_INFO(HV_STATUS_INVALID_CONNECTION_ID, -EINVAL),
> - _STATUS_INFO(HV_STATUS_INSUFFICIENT_BUFFERS, -EIO),
> - _STATUS_INFO(HV_STATUS_NOT_ACKNOWLEDGED, -EIO),
> - _STATUS_INFO(HV_STATUS_INVALID_VP_STATE, -EIO),
> + _STATUS_INFO(HV_STATUS_INSUFFICIENT_BUFFERS, -ENOBUFS),
> + _STATUS_INFO(HV_STATUS_NOT_ACKNOWLEDGED, -EBUSY),
> + _STATUS_INFO(HV_STATUS_INVALID_VP_STATE, -EINVAL),
> _STATUS_INFO(HV_STATUS_NO_RESOURCES, -EIO),
> _STATUS_INFO(HV_STATUS_PROCESSOR_FEATURE_NOT_SUPPORTED, -EIO),
> _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EINVAL),
> _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EINVAL),
> - _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX, -EIO),
> - _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE, -EIO),
> _STATUS_INFO(HV_STATUS_OPERATION_FAILED, -EIO),
> - _STATUS_INFO(HV_STATUS_TIME_OUT, -EIO),
> + _STATUS_INFO(HV_STATUS_TIME_OUT, -ETIMEDOUT),
> _STATUS_INFO(HV_STATUS_CALL_PENDING, -EIO),
> - _STATUS_INFO(HV_STATUS_VTL_ALREADY_ENABLED, -EIO),
> + _STATUS_INFO(HV_STATUS_VTL_ALREADY_ENABLED, -EBUSY),
> #undef _STATUS_INFO
> };
>
^ permalink raw reply
* Re: [PATCH v2] Drivers: hv: Use better errno matches for HV_STATUS values
From: Easwar Hariharan @ 2025-10-09 19:10 UTC (permalink / raw)
To: Nuno Das Neves
Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Stanislav Kinsburskii, open list:Hyper-V/Azure CORE AND DRIVERS,
open list, easwar.hariharan
In-Reply-To: <2768abf3-6974-49e6-9ea2-5e5e04f533a7@linux.microsoft.com>
On 10/9/2025 11:53 AM, Nuno Das Neves wrote:
> On 10/6/2025 4:08 PM, Easwar Hariharan wrote:
>> Use a better mapping of hypervisor status codes to errno values and
>> disambiguate the catch-all -EIO value. While here, remove the duplicate
>> INVALID_LP_INDEX and INVALID_REGISTER_VALUES hypervisor status entries.
>>
>
> To be honest, in retrospect the idea of 'translating' the hypercall error
> codes is a bit pointless. hv_result_to_errno() allows the hypercall helper
> functions to be a bit cleaner, but that's about it. When debugging you
> almost always want to know the actual hypercall error code. Translating
> it imperfectly is often useless, and at worst creates a red
> herring/obfuscates the true source of the error.
I feel like you're thinking from the perspective of Microsoft engineers working with
the hypervisor. IMHO, the translation is useful for the rest of the kernel to understand
and possibly handle differently the possible errors. EBUSY for example is
meant to indicate that the operation is already done, or that the target (device/hypervisor)
is busy with something else. Timeouts may be handled by a retry, perhaps with a backoff period.
>
> With that in mind, updating the errno mappings to be more accurate feels
> like unnecessary churn. It might even be better to remove the errno mappings
> altogether and just translate HV_STATUS_SUCCESS to 0 and any other error
> to -EIO or some other 'signal' error code to make it more obvious that
> a *hypercall* error occurred and not some other Linux error. We'd still
> want to keep the table in some form because it's also used for the error
> strings.
IMHO, an arbitrarily chosen 'signal' error code wouldn't tell support or the rest of the community that
a hypercall error occurred, without a print or trace closest to the hypercall location.
>
> The cleanup removing the duplicates in the table is welcome.
>
> Nuno
>
<snip>
^ permalink raw reply
* Re: [PATCH 2/2] hyperv: Enable clean shutdown for root partition with MSHV
From: Nuno Das Neves @ 2025-10-09 19:18 UTC (permalink / raw)
To: Praveen K Paladugu, kys, haiyangz, wei.liu, decui, tglx, mingo,
linux-hyperv, linux-kernel, bp, dave.hansen, x86, hpa, arnd
Cc: anbelski
In-Reply-To: <20251009160501.6356-3-prapal@linux.microsoft.com>
On 10/9/2025 8:58 AM, Praveen K Paladugu wrote:
> This commit enables the root partition to perform a clean shutdown when
> running with MSHV hypervisor.
>
Commit message could briefly explain what the current problem is - what is
wrong with the current shutdown and how does this fix it?
> Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
> Co-developed-by: Anatol Belski <anbelski@linux.microsoft.com>
> Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
> ---
> arch/x86/hyperv/hv_init.c | 7 ++
> drivers/hv/hv_common.c | 118 +++++++++++++++++++++++++++++++++
> include/asm-generic/mshyperv.h | 1 +
> 3 files changed, 126 insertions(+)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index afdbda2dd7b7..57bd96671ead 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -510,6 +510,13 @@ void __init hyperv_init(void)
> memunmap(src);
>
> hv_remap_tsc_clocksource();
> + /*
> + * The notifier registration might fail at various hops.
> + * Corresponding error messages will land in dmesg. There is
> + * otherwise nothing that can be specifically done to handle
> + * failures here.
> + */
> + (void)hv_sleep_notifiers_register();
> } else {
> hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg);
> wrmsrq(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index e109a620c83f..c5165deb5278 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -837,3 +837,121 @@ const char *hv_result_to_string(u64 status)
> return "Unknown";
> }
> EXPORT_SYMBOL_GPL(hv_result_to_string);
> +
> +/*
> + * Corresponding sleep states have to be initialized, in order for a subsequent
The comma in the above line is not needed.
> + * HVCALL_ENTER_SLEEP_STATE call to succeed. Currently only S5 state as per
> + * ACPI 6.4 chapter 7.4.2 is relevant, while S1, S2 and S3 can be supported.
> + *
> + * ACPI should be initialized and should support S5 sleep state when this method
> + * is called, so that, it can extract correct PM values and pass them to hv.
> + */
> +static int hv_initialize_sleep_states(void)
> +{
> + u64 status;
> + unsigned long flags;
> + struct hv_input_set_system_property *in;
> + acpi_status acpi_status;
> + u8 sleep_type_a, sleep_type_b;
> +
> + if (!acpi_sleep_state_supported(ACPI_STATE_S5)) {
> + pr_err("%s: S5 sleep state not supported.\n", __func__);
> + return -ENODEV;
> + }
> +
> + acpi_status = acpi_get_sleep_type_data(ACPI_STATE_S5,
> + &sleep_type_a, &sleep_type_b);
> + if (ACPI_FAILURE(acpi_status))
> + return -ENODEV;
> +
> + local_irq_save(flags);
> + in = (struct hv_input_set_system_property *)(*this_cpu_ptr(
> + hyperv_pcpu_input_arg));
> +
The input struct contains a reserved field that should be zero.
You could either set it to zero explicitly or preferably just zero the whole
struct here. Doing that gives confidence to a reader that no field is left
uninitialized even if they're not familiar with the struct.
> + in->property_id = HV_SYSTEM_PROPERTY_SLEEP_STATE;
> + in->set_sleep_state_info.sleep_state = HV_SLEEP_STATE_S5;
> + in->set_sleep_state_info.pm1a_slp_typ = sleep_type_a;
> + in->set_sleep_state_info.pm1b_slp_typ = sleep_type_b;
> +
> + status = hv_do_hypercall(HVCALL_SET_SYSTEM_PROPERTY, in, NULL);
> + local_irq_restore(flags);
> +
> + if (!hv_result_success(status)) {
> + pr_err("%s: %s\n", __func__, hv_result_to_string(status));
Use hv_status_err(status, "\n");
> + return hv_result_to_errno(status);
> + }
> +
> + return 0;
> +}
> +
> +static int hv_call_enter_sleep_state(u32 sleep_state)
> +{
> + u64 status;
> + int ret;
> + unsigned long flags;
> + struct hv_input_enter_sleep_state *in;
> +
> + ret = hv_initialize_sleep_states();
> + if (ret)
> + return ret;
> +
> + local_irq_save(flags);
> + in = (struct hv_input_enter_sleep_state *)
> + (*this_cpu_ptr(hyperv_pcpu_input_arg));
> + in->sleep_state = (enum hv_sleep_state)sleep_state;
> +
> + status = hv_do_hypercall(HVCALL_ENTER_SLEEP_STATE, in, NULL);
> + local_irq_restore(flags);
> +
> + if (!hv_result_success(status)) {
> + pr_err("%s: %s\n", __func__, hv_result_to_string(status));
Use hv_status_err(status, "\n");
> + return hv_result_to_errno(status);
> + }
> +
> + return 0;
> +}
> +
> +static int hv_reboot_notifier_handler(struct notifier_block *this,
> + unsigned long code, void *another)
> +{
> + int ret = 0;
> +
> + if (SYS_HALT == code || SYS_POWER_OFF == code)
> + ret = hv_call_enter_sleep_state(HV_SLEEP_STATE_S5);
> +
> + return ret ? NOTIFY_DONE : NOTIFY_OK;
> +}
> +
> +static struct notifier_block hv_reboot_notifier = {
> + .notifier_call = hv_reboot_notifier_handler,
> +};
> +
> +static int hv_acpi_sleep_handler(u8 sleep_state, u32 pm1a_cnt, u32 pm1b_cnt)
> +{
> + int ret = 0;
> +
> + if (sleep_state == ACPI_STATE_S5)
> + ret = hv_call_enter_sleep_state(HV_SLEEP_STATE_S5);
> +
> + return ret == 0 ? 1 : -1;
> +}
> +
> +static int hv_acpi_extended_sleep_handler(u8 sleep_state, u32 val_a, u32 val_b)
> +{
> + return hv_acpi_sleep_handler(sleep_state, val_a, val_b);
> +}
> +
> +int hv_sleep_notifiers_register(void)
> +{
> + int ret;
> +
> + acpi_os_set_prepare_sleep(&hv_acpi_sleep_handler);
> + acpi_os_set_prepare_extended_sleep(&hv_acpi_extended_sleep_handler);
> +
> + ret = register_reboot_notifier(&hv_reboot_notifier);
> + if (ret)
> + pr_err("%s: cannot register reboot notifier %d\n",
> + __func__, ret);
> +
> + return ret;
> +}
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index 64ba6bc807d9..903d089aba82 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -339,6 +339,7 @@ u64 hv_tdx_hypercall(u64 control, u64 param1, u64 param2);
> void hyperv_cleanup(void);
> bool hv_query_ext_cap(u64 cap_query);
> void hv_setup_dma_ops(struct device *dev, bool coherent);
> +int hv_sleep_notifiers_register(void);
> #else /* CONFIG_HYPERV */
> static inline void hv_identify_partition_type(void) {}
> static inline bool hv_is_hyperv_initialized(void) { return false; }
^ permalink raw reply
* Re: [PATCH 1/2] hyperv: Add definitions for MSHV sleep state configuration
From: Nuno Das Neves @ 2025-10-09 19:22 UTC (permalink / raw)
To: Praveen K Paladugu, kys, haiyangz, wei.liu, decui, tglx, mingo,
linux-hyperv, linux-kernel, bp, dave.hansen, x86, hpa, arnd
Cc: anbelski
In-Reply-To: <20251009160501.6356-2-prapal@linux.microsoft.com>
On 10/9/2025 8:58 AM, Praveen K Paladugu wrote:
> Add the definitions required to configure sleep states in mshv hypervsior.
>
> Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
> Co-developed-by: Anatol Belski <anbelski@linux.microsoft.com>
> Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
> ---
> include/hyperv/hvgdk_mini.h | 4 +++-
> include/hyperv/hvhdk_mini.h | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
> index 77abddfc750e..943df5d292f2 100644
> --- a/include/hyperv/hvgdk_mini.h
> +++ b/include/hyperv/hvgdk_mini.h
> @@ -464,18 +464,20 @@ union hv_vp_assist_msr_contents { /* HV_REGISTER_VP_ASSIST_PAGE */
> #define HVCALL_RESET_DEBUG_SESSION 0x006b
> #define HVCALL_MAP_STATS_PAGE 0x006c
> #define HVCALL_UNMAP_STATS_PAGE 0x006d
> +#define HVCALL_SET_SYSTEM_PROPERTY 0x006f
> #define HVCALL_ADD_LOGICAL_PROCESSOR 0x0076
> #define HVCALL_GET_SYSTEM_PROPERTY 0x007b
> #define HVCALL_MAP_DEVICE_INTERRUPT 0x007c
> #define HVCALL_UNMAP_DEVICE_INTERRUPT 0x007d
> #define HVCALL_RETARGET_INTERRUPT 0x007e
> +#define HVCALL_ENTER_SLEEP_STATE 0x0084
> #define HVCALL_NOTIFY_PORT_RING_EMPTY 0x008b
> #define HVCALL_REGISTER_INTERCEPT_RESULT 0x0091
> #define HVCALL_ASSERT_VIRTUAL_INTERRUPT 0x0094
> #define HVCALL_CREATE_PORT 0x0095
> #define HVCALL_CONNECT_PORT 0x0096
> #define HVCALL_START_VP 0x0099
> -#define HVCALL_GET_VP_INDEX_FROM_APIC_ID 0x009a
> +#define HVCALL_GET_VP_INDEX_FROM_APIC_ID 0x009a
> #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
> #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
> #define HVCALL_SIGNAL_EVENT_DIRECT 0x00c0
> diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
> index 858f6a3925b3..8fa86c014c25 100644
> --- a/include/hyperv/hvhdk_mini.h
> +++ b/include/hyperv/hvhdk_mini.h
> @@ -114,10 +114,24 @@ enum hv_snp_status {
>
> enum hv_system_property {
> /* Add more values when needed */
> + HV_SYSTEM_PROPERTY_SLEEP_STATE = 3,
> HV_SYSTEM_PROPERTY_SCHEDULER_TYPE = 15,
> HV_DYNAMIC_PROCESSOR_FEATURE_PROPERTY = 21,
> };
>
> +enum hv_sleep_state {
> + HV_SLEEP_STATE_S1 = 1,
> + HV_SLEEP_STATE_S2 = 2,
> + HV_SLEEP_STATE_S3 = 3,
> + HV_SLEEP_STATE_S4 = 4,
> + HV_SLEEP_STATE_S5 = 5,
> + /*
> + * After hypervisor has received this, any follow up sleep
> + * state registration requests will be rejected.
> + */
> + HV_SLEEP_STATE_LOCK = 6
> +};
> +
> enum hv_dynamic_processor_feature_property {
> /* Add more values when needed */
> HV_X64_DYNAMIC_PROCESSOR_FEATURE_MAX_ENCRYPTED_PARTITIONS = 13,
> @@ -145,6 +159,25 @@ struct hv_output_get_system_property {
> };
> } __packed;
>
> +struct hv_sleep_state_info {
> + u32 sleep_state; /* enum hv_sleep_state */
> + u8 pm1a_slp_typ;
> + u8 pm1b_slp_typ;
> +} __packed;
> +
> +struct hv_input_set_system_property {
> + u32 property_id; /* enum hv_system_property */
> + u32 reserved;
> + union {
> + /* More fields to be filled in when needed */
> + struct hv_sleep_state_info set_sleep_state_info;
> + };
> +} __packed;
> +
> +struct hv_input_enter_sleep_state { /* HV_INPUT_ENTER_SLEEP_STATE */
> + u32 sleep_state; /* enum hv_sleep_state */
> +} __packed;
> +
> struct hv_input_map_stats_page {
> u32 type; /* enum hv_stats_object_type */
> u32 padding;
Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
^ permalink raw reply
* Re: [PATCH 0/2] Add support for clean shutdown with MSHV
From: Stanislav Kinsburskii @ 2025-10-09 22:26 UTC (permalink / raw)
To: Praveen K Paladugu
Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, linux-hyperv,
linux-kernel, bp, dave.hansen, x86, hpa, arnd, anbelski
In-Reply-To: <20251009160501.6356-1-prapal@linux.microsoft.com>
On Thu, Oct 09, 2025 at 10:58:49AM -0500, Praveen K Paladugu wrote:
> Add support for clean shutdown of the root partition when running on MSHV
> hypervisor.
>
> Praveen K Paladugu (2):
> hyperv: Add definitions for MSHV sleep state configuration
> hyperv: Enable clean shutdown for root partition with MSHV
>
There is no need to split this logic to two patches: the first one
doesn't make sense without the second one, so it would be better to
squash them.
Thanks,
Stanislav
> arch/x86/hyperv/hv_init.c | 7 ++
> drivers/hv/hv_common.c | 118 +++++++++++++++++++++++++++++++++
> include/asm-generic/mshyperv.h | 1 +
> include/hyperv/hvgdk_mini.h | 4 +-
> include/hyperv/hvhdk_mini.h | 33 +++++++++
> 5 files changed, 162 insertions(+), 1 deletion(-)
>
> --
> 2.51.0
>
^ permalink raw reply
* Re: [PATCH v4 5/5] Drivers: hv: Add support for movable memory regions
From: kernel test robot @ 2025-10-10 17:48 UTC (permalink / raw)
To: Stanislav Kinsburskii, kys, haiyangz, wei.liu, decui
Cc: llvm, oe-kbuild-all, linux-hyperv, linux-kernel
In-Reply-To: <175976319844.16834.4747024333732752980.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net>
Hi Stanislav,
kernel test robot noticed the following build warnings:
[auto build test WARNING on linus/master]
[also build test WARNING on next-20251010]
[cannot apply to v6.17]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Stanislav-Kinsburskii/Drivers-hv-Refactor-and-rename-memory-region-handling-functions/20251010-111917
base: linus/master
patch link: https://lore.kernel.org/r/175976319844.16834.4747024333732752980.stgit%40skinsburskii-cloud-desktop.internal.cloudapp.net
patch subject: [PATCH v4 5/5] Drivers: hv: Add support for movable memory regions
config: x86_64-randconfig-006-20251010 (https://download.01.org/0day-ci/archive/20251011/202510110134.RmOV83Fz-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251011/202510110134.RmOV83Fz-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510110134.RmOV83Fz-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/hv/mshv_root_main.c:1410:11: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
1410 | else if (!mutex_trylock(®ion->mutex))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/hv/mshv_root_main.c:1434:12: note: uninitialized use occurs here
1434 | WARN_ONCE(ret,
| ^~~
include/asm-generic/bug.h:152:18: note: expanded from macro 'WARN_ONCE'
152 | DO_ONCE_LITE_IF(condition, WARN, 1, format)
| ^~~~~~~~~
include/linux/once_lite.h:28:27: note: expanded from macro 'DO_ONCE_LITE_IF'
28 | bool __ret_do_once = !!(condition); \
| ^~~~~~~~~
drivers/hv/mshv_root_main.c:1410:7: note: remove the 'if' if its condition is always false
1410 | else if (!mutex_trylock(®ion->mutex))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1411 | goto out_fail;
| ~~~~~~~~~~~~~
drivers/hv/mshv_root_main.c:1406:9: note: initialize the variable 'ret' to silence this warning
1406 | int ret;
| ^
| = 0
1 warning generated.
vim +1410 drivers/hv/mshv_root_main.c
1377
1378 /**
1379 * mshv_region_interval_invalidate - Invalidate a range of memory region
1380 * @mni: Pointer to the mmu_interval_notifier structure
1381 * @range: Pointer to the mmu_notifier_range structure
1382 * @cur_seq: Current sequence number for the interval notifier
1383 *
1384 * This function invalidates a memory region by remapping its pages with
1385 * no access permissions. It locks the region's mutex to ensure thread safety
1386 * and updates the sequence number for the interval notifier. If the range
1387 * is blockable, it uses a blocking lock; otherwise, it attempts a non-blocking
1388 * lock and returns false if unsuccessful.
1389 *
1390 * NOTE: Failure to invalidate a region is a serious error, as the pages will
1391 * be considered freed while they are still mapped by the hypervisor.
1392 * Any attempt to access such pages will likely crash the system.
1393 *
1394 * Return: true if the region was successfully invalidated, false otherwise.
1395 */
1396 static bool
1397 mshv_region_interval_invalidate(struct mmu_interval_notifier *mni,
1398 const struct mmu_notifier_range *range,
1399 unsigned long cur_seq)
1400 {
1401 struct mshv_mem_region *region = container_of(mni,
1402 struct mshv_mem_region,
1403 mni);
1404 u64 page_offset, page_count;
1405 unsigned long mstart, mend;
1406 int ret;
1407
1408 if (mmu_notifier_range_blockable(range))
1409 mutex_lock(®ion->mutex);
> 1410 else if (!mutex_trylock(®ion->mutex))
1411 goto out_fail;
1412
1413 mmu_interval_set_seq(mni, cur_seq);
1414
1415 mstart = max(range->start, region->start_uaddr);
1416 mend = min(range->end, region->start_uaddr +
1417 (region->nr_pages << HV_HYP_PAGE_SHIFT));
1418
1419 page_offset = HVPFN_DOWN(mstart - region->start_uaddr);
1420 page_count = HVPFN_DOWN(mend - mstart);
1421
1422 ret = mshv_region_remap_pages(region, HV_MAP_GPA_NO_ACCESS,
1423 page_offset, page_count);
1424 if (ret)
1425 goto out_fail;
1426
1427 mshv_region_invalidate_pages(region, page_offset, page_count);
1428
1429 mutex_unlock(®ion->mutex);
1430
1431 return true;
1432
1433 out_fail:
1434 WARN_ONCE(ret,
1435 "Failed to invalidate region %#llx-%#llx (range %#lx-%#lx, event: %u, pages %#llx-%#llx, mm: %#llx): %d\n",
1436 region->start_uaddr,
1437 region->start_uaddr + (region->nr_pages << HV_HYP_PAGE_SHIFT),
1438 range->start, range->end, range->event,
1439 page_offset, page_offset + page_count - 1, (u64)range->mm, ret);
1440 return false;
1441 }
1442
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v4 4/5] mshv: Allocate vp state page for HVCALL_MAP_VP_STATE_PAGE on L1VH
From: Nuno Das Neves @ 2025-10-10 19:08 UTC (permalink / raw)
To: Michael Kelley, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, prapal@linux.microsoft.com,
easwar.hariharan@linux.microsoft.com, tiala@microsoft.com,
anirudh@anirudhrb.com, paekkaladevi@linux.microsoft.com,
skinsburskii@linux.microsoft.com
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, Jinank Jain
In-Reply-To: <SN6PR02MB4157F9804CC7F22A22E01682D4E5A@SN6PR02MB4157.namprd02.prod.outlook.com>
On 10/4/2025 8:25 AM, Michael Kelley wrote:
> From: Michael Kelley <mhklinux@outlook.com> Sent: Wednesday, October 1, 2025 5:03 PM
>>
>> From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Wednesday, October 1, 2025 3:56 PM
>>>
>>> On 9/29/2025 10:56 AM, Michael Kelley wrote:
>>>> From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Friday, September 26, 2025 9:23 AM
>>>>>
>>>>> From: Jinank Jain <jinankjain@linux.microsoft.com>
>>>>>
>>>>> Introduce mshv_use_overlay_gpfn() to check if a page needs to be
>>>>> allocated and passed to the hypervisor to map VP state pages. This is
>>>>> only needed on L1VH, and only on some (newer) versions of the
>>>>> hypervisor, hence the need to check vmm_capabilities.
>>>>>
>>>>> Introduce functions hv_map/unmap_vp_state_page() to handle the
>>>>> allocation and freeing.
>>>>>
>>>>> Signed-off-by: Jinank Jain <jinankjain@linux.microsoft.com>
>>>>> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>>>>> Reviewed-by: Praveen K Paladugu <prapal@linux.microsoft.com>
>>>>> Reviewed-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>
>>>>> Reviewed-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
>>>>> Reviewed-by: Anirudh Rayabharam <anirudh@anirudhrb.com>
>>>>> ---
>>>>> drivers/hv/mshv_root.h | 11 ++---
>>>>> drivers/hv/mshv_root_hv_call.c | 61 ++++++++++++++++++++++++---
>>>>> drivers/hv/mshv_root_main.c | 76 +++++++++++++++++-----------------
>>>>> 3 files changed, 98 insertions(+), 50 deletions(-)
>>>>>
>>>>> diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
>>>>> index 0cb1e2589fe1..dbe2d1d0b22f 100644
>>>>> --- a/drivers/hv/mshv_root.h
>>>>> +++ b/drivers/hv/mshv_root.h
>>>>> @@ -279,11 +279,12 @@ int hv_call_set_vp_state(u32 vp_index, u64 partition_id,
>>>>> /* Choose between pages and bytes */
>>>>> struct hv_vp_state_data state_data, u64 page_count,
>>>>> struct page **pages, u32 num_bytes, u8 *bytes);
>>>>> -int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>>>>> - union hv_input_vtl input_vtl,
>>>>> - struct page **state_page);
>>>>> -int hv_call_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>>>>> - union hv_input_vtl input_vtl);
>>>>> +int hv_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>>>>> + union hv_input_vtl input_vtl,
>>>>> + struct page **state_page);
>>>>> +int hv_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>>>>> + struct page *state_page,
>>>>> + union hv_input_vtl input_vtl);
>>>>> int hv_call_create_port(u64 port_partition_id, union hv_port_id port_id,
>>>>> u64 connection_partition_id, struct hv_port_info *port_info,
>>>>> u8 port_vtl, u8 min_connection_vtl, int node);
>>>>> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
>>>>> index 3fd3cce23f69..98c6278ff151 100644
>>>>> --- a/drivers/hv/mshv_root_hv_call.c
>>>>> +++ b/drivers/hv/mshv_root_hv_call.c
>>>>> @@ -526,9 +526,9 @@ int hv_call_set_vp_state(u32 vp_index, u64 partition_id,
>>>>> return ret;
>>>>> }
>>>>>
>>>>> -int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>>>>> - union hv_input_vtl input_vtl,
>>>>> - struct page **state_page)
>>>>> +static int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>>>>> + union hv_input_vtl input_vtl,
>>>>> + struct page **state_page)
>>>>> {
>>>>> struct hv_input_map_vp_state_page *input;
>>>>> struct hv_output_map_vp_state_page *output;
>>>>> @@ -547,7 +547,14 @@ int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>>>>> input->type = type;
>>>>> input->input_vtl = input_vtl;
>>>>>
>>>>> - status = hv_do_hypercall(HVCALL_MAP_VP_STATE_PAGE, input, output);
>>>>
>>>> This function must zero the input area before using it. Otherwise,
>>>> flags.map_location_provided is uninitialized when *state_page is NULL. It will
>>>> have whatever value was left by the previous user of hyperv_pcpu_input_arg,
>>>> potentially producing bizarre results. And there's a reserved field that won't be
>>>> set to zero.
>>>>
>>> Good catch, will add a memset().
>>>
>>>>> + if (*state_page) {
>>>>> + input->flags.map_location_provided = 1;
>>>>> + input->requested_map_location =
>>>>> + page_to_pfn(*state_page);
>>>>
>>>> Technically, this should be page_to_hvpfn() since the PFN value is being sent to
>>>> Hyper-V. I know root (and L1VH?) partitions must run with the same page size
>>>> as the Hyper-V host, but it's better to not leave code buried here that will blow
>>>> up if the "same page size requirement" should ever change.
>>>>
>>> Good point...I could change these calls, but the other way doesn't work, see below.
>>>
>>>> And after making the hypercall, there's an invocation of pfn_to_page(), which
>>>> should account for the same. Unfortunately, there's not an existing hvpfn_to_page()
>>>> function.
>>>>
>>> This seems like a tricky scenario to get right. In the root partition case, the
>>> hypervisor allocates the page. That pfn could be some page within a larger Linux page.
>>> Converting that to a Linux pfn (naively) means losing the original hvpfn since it gets
>>> truncated, which is no good if we want to unmap it later. Also page_address() would
>>> give the wrong virtual address.
>>>
>>> In other words, we'd have to completely change how we track these pages in order to
>>> support this scenario, and the same goes for various other hypervisor APIs where the
>>> hypervisor does the allocating. I think it's out of scope to try and address that
>>> here, even in part, especially since we will be making assumptions about something
>>> that may never happen.
>>
>> OK, yes the hypervisor allocating the page is a problem when Linux tracks it
>> as a struct page. I'll agree it's out of current scope to change this.
>>
>> It makes me think about hv_synic_enable_regs() where the paravisor or hypervisor
>> allocates the synic_message_page and synic_event_page. But that case should work
>> OK with a regular guest with page size greater than 4K because the pages are tracked
>> based on the guest kernel virtual address, not the PFN. So hv_synic_enable_regs()
>> should work on ARM64 Linux guests with 64K page size and a paravisor, as well as
>> for my postulated root partition with page size greater than 4K.
>>
>> When it matters, cases where the hypervisor or paravisor allocate pages to give
>> to the guest will require careful handling to ensure they work for guest page sizes
>> greater than 4K. That's useful information for future consideration. Thanks for the
>> discussion.
>>
>
> Upon further reflection, I think there's a subtle bug in the case where the
> hypervisor supplies the state page. This bug is present in the existing code, and
> persists after these patches.
>
> I'm assuming that the hypervisor supplied page is *not* part of the memory
> assigned to the root partition when it was created. I.e., the supplied page is part
> of the hypervisor's private memory. Or does the root partition Linux "give"
> the hypervisor some memory for it to later return as one of these state pages?
>
It is a regular page that Linux deposits to that particular guest partition so it
can be used for this or other purposes (bookkeeping, page tables, stats pages,
etc...). It would have been deposited earlier, e.g. one of the pages deposited
just before HVCALL_INITIALIZE_PARTITION.
The hypervisor's pre-reserved pages which Linux doesn't touch are only used for a
few things, and never for guests as far as I know.
> Assuming the page did *not* originate in Linux, then the page provided by the
> hypervisor doesn't actually have a "struct page" entry in the root partition Linux
> instance. Doing pfn_to_page() works because it just does some address
> arithmetic, but the resulting "struct page *" value points somewhere off the
> end of the root partition's "struct page" array.
>
> Then page_to_virt() is done on this somewhat bogus "struct page *" value.
> page_to_virt() also just does some address arithmetic, so it "works". But it
> assumes that the "struct page *" input value is good, and that Linux has a valid
> virtual-to-physical direct mapping for the physical memory represented by
> that input value. Unfortunately, that assumption might not always be true. I
> think it works most of the time because Linux uses huge page mappings for
> the direct map, and they may extend far enough beyond the root partition's
> actual memory to cover this hypervisor page. Or maybe there's something
> else going on that I'm not aware of and that allows the current code to work.
> So please check my thinking.
>
> The robust fix is to do like hv_synic_enable_regs(), where a page returned
> by the hypervisor/paravisor is explicitly mapped in order to have a valid
> virtual address in the root partition Linux.
>
> Note that on ARM64, when CONFIG_DEBUG_VIRTUAL is set to catch errors
> like this, page_to_virt() does additional checks on its input value, and would
> generate a WARN_ON() in this case. x86/x64 does not do the additional checks.
>
> Michael
>
>>
>>>
>>>>> + }
>>>>> +
>>>>> + status = hv_do_hypercall(HVCALL_MAP_VP_STATE_PAGE, input,
>>>>> + output);
>>>>>
>>>>> if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY) {
>>>>> if (hv_result_success(status))
^ permalink raw reply
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