Thread (61 messages) flat view 61 messages, 3 authors, 2d ago

Re: [PATCH v3 7/7] mshv: set up own SynIC registers on a nested root partition

From: sashiko-bot@kernel.org
Date: 2026-08-25 04:20:08

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing MEMREMAP_DEC flag and shared GPA boundary masking when mapping hypervisor-shared SynIC pages.
- [High] Missing pointer nullification on the error path of mshv_synic_cpu_init leaves a dangling pointer in a per-cpu variable.
--
--- Patch [7]: [PATCH v3 7/7] mshv: set up own SynIC registers on a nested root partition ---
commit 04d92759c80f66439e3b646d824c274dc6edc45b
Author: Wei Hu [off-list ref]

mshv: set up own SynIC registers on a nested root partition

The commit fixes SynIC register setup for nested root partitions by programming
non-nested MSRs instead of relying on VMBus's nested MSR setup, restoring
correct intercept and async-hypercall delivery.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hv/mshv_synic.c b/drivers/hv/mshv_synic.c
index 7c168e5a740dd..7d714ee5d2267 100644
--- a/drivers/hv/mshv_synic.c
+++ b/drivers/hv/mshv_synic.c
@@ -472,7 +472,13 @@ static int mshv_synic_cpu_init(unsigned int cpu)
 	 * VMBus owns SIMP/SIEFP/SCONTROL when it is active.
 	 * See hv_hyp_synic_enable_regs() for that initialization.
 	 */
-	bool vmbus_active = hv_vmbus_exists();
+	/*
+	 * On a nested root partition VMBus programs the *nested* SynIC MSRs
+	 * (SIMP/SIEFP/SCONTROL) via hv_set_msr()'s nested remap, whereas the
+	 * code below reads/writes the non-nested SynIC MSRs. So when nested,
+	 * do not defer SynIC setup to VMBus -- set up our own registers.
+	 */
+	bool vmbus_active = hv_vmbus_exists() && !hv_nested;
 
 	/*
 	 * Map the SYNIC message page. When VMBus is not active the
 	 * hypervisor pre-provisions the SIMP GPA but may not set
 	 * simp_enabled  enable it here.
 	 */
 	simp.as_uint64 = hv_get_non_nested_msr(HV_MSR_SIMP);
 	if (!vmbus_active) {
 		simp.simp_enabled = true;
 		hv_set_non_nested_msr(HV_MSR_SIMP, simp.as_uint64);
 	}
 	*msg_page = memremap(simp.base_simp_gpa << HV_HYP_PAGE_SHIFT,
 			     HV_HYP_PAGE_SIZE,
 			     MEMREMAP_WB);
[Severity: High]
This isn't a bug introduced by this patch, but does this mapping need to use 
MEMREMAP_DEC and mask the shared_gpa_boundary bit?

If the root partition is running as a confidential VM (such as AMD SEV-SNP or 
Intel TDX), hypervisor-shared pages need to be mapped as decrypted.

Mapping them as encrypted memory could cause the driver to read garbage 
intercept messages and miss doorbell events.

[ ... ]
 cleanup_simp:
 	if (*msg_page)
 		memunmap(*msg_page);
[Severity: High]
This is a pre-existing issue, but does this leave a dangling pointer in the 
per-cpu synic_pages structure?

If memremap() fails for the event flags or ring pages during initialization, 
the error path unmaps *msg_page but doesn't set the per-CPU pointer to NULL. 

Since the Hyper-V callback vector can be shared globally, a shared interrupt 
(like from VMBus) could fire on this CPU after a failure.

If the IDT handler unconditionally calls mshv_isr(), could it bypass a NULL 
check and dereference the unmapped pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825040505.826600-1-weh@linux.microsoft.com?part=7
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help