Re: [PATCH v4 9/9] mshv: set up own SynIC registers on a nested root partition
From: sashiko-bot@kernel.org
Date: 2026-08-31 12:09:42
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Missing memory decryption for locally allocated L1VH SIRBP in Confidential VMs. -- commit 0a5809fa9d9b76912e599a03beec81fda2326b1b Author: Wei Hu [off-list ref] mshv: set up own SynIC registers on a nested root partition This patch modifies mshv_synic to set up its own SynIC registers on nested root partitions instead of assuming VMBus has provisioned them. It also updates the mapping of hypervisor-provided SIMP, SIEFP, and root SIRBP pages to use MEMREMAP_DEC to support confidential hosts.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hv/mshv_synic.c b/drivers/hv/mshv_synic.c index 0fdbae1e053c3..8470cf958f21b 100644 --- a/drivers/hv/mshv_synic.c +++ b/drivers/hv/mshv_synic.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -511,8 +524,8 @@ static int mshv_synic_cpu_init(unsigned int cpu) sirbp.as_uint64 = hv_get_non_nested_msr(HV_MSR_SIRBP); if (hv_root_partition()) { - event_ring_page = memremap(sirbp.base_sirbp_gpa << HV_HYP_PAGE_SHIFT, - HV_HYP_PAGE_SIZE, MEMREMAP_WB); + event_ring_page = + mshv_synic_map_shared_page(sirbp.base_sirbp_gpa); if (!event_ring_page) goto cleanup_siefp;
[Severity: High]
Since this patch adds support for confidential hosts by mapping
hypervisor-provided pages with MEMREMAP_DEC, what happens to the locally
allocated SIRBP in the else branch for L1VH partitions?
If we look further down in mshv_synic_cpu_init():
} else {
/*
* On L1VH the hypervisor does not provide a SIRBP page.
* Allocate one and program its GPA into the MSR.
*/
event_ring_page = (struct hv_synic_event_ring_page *)
get_zeroed_page(GFP_KERNEL);
In a Confidential VM, memory allocated via get_zeroed_page() is encrypted by
default. Because set_memory_decrypted() is never called here, will the
hypervisor be unable to access the event ring, causing a regression in event
delivery for L1VH?
For comparison, drivers/hv/hv.c decrypts its locally allocated SynIC pages
via hv_alloc_page(..., decrypt=true, ...).
[ ... ]
quoted hunk ↗ jump to hunk
@@ -595,8 +608,7 @@ static int mshv_synic_cpu_exit(unsigned int cpu) struct hv_message_page *msg_page; struct hv_synic_event_flags_page *event_flags_page; struct hv_synic_event_ring_page *event_ring_page; - /* VMBus owns SIMP/SIEFP/SCONTROL when it is active */ - bool vmbus_active = hv_vmbus_exists(); + bool vmbus_active = mshv_synic_vmbus_owns_registers(); msg_page = READ_ONCE(spages->hyp_synic_message_page); event_flags_page = READ_ONCE(spages->synic_event_flags_page);
[Severity: High]
If the allocation in mshv_synic_cpu_init() is updated to decrypt the page,
does the cleanup path in mshv_synic_cpu_exit() need a corresponding change?
Looking at the cleanup path in mshv_synic_cpu_exit():
if (event_ring_page) {
if (hv_root_partition())
memunmap(event_ring_page);
else
free_page((unsigned long)event_ring_page);
}
If the page is decrypted during initialization, will freeing it here without
first calling set_memory_encrypted() return a decrypted page to the kernel
allocator and cause a memory corruption regression?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260831112704.2851147-1-weh@linux.microsoft.com?part=9