From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
To: Jork Loeser <jloeser@linux.microsoft.com>
Cc: linux-hyperv@vger.kernel.org, x86@kernel.org,
"K . Y . Srinivasan" <kys@microsoft.com>,
Haiyang Zhang <haiyangz@microsoft.com>,
Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>,
Long Li <longli@microsoft.com>, Thomas Gleixner <tglx@kernel.org>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>,
"H . Peter Anvin" <hpa@zytor.com>, Arnd Bergmann <arnd@arndb.de>,
Roman Kisel <romank@linux.microsoft.com>,
Michael Kelley <mhklinux@outlook.com>,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Subject: Re: [PATCH 4/6] mshv: limit SynIC management to MSHV-owned resources
Date: Mon, 30 Mar 2026 14:51:29 -0700 [thread overview]
Message-ID: <acrwYf50SJnDwN3e@skinsburskii.localdomain> (raw)
In-Reply-To: <20260327201920.2100427-5-jloeser@linux.microsoft.com>
On Fri, Mar 27, 2026 at 01:19:15PM -0700, Jork Loeser wrote:
> The SynIC is shared between VMBus and MSHV. VMBus owns the message
> page (SIMP), event flags page (SIEFP), global enable (SCONTROL), and
> SINT2. MSHV adds SINT0, SINT5, and the event ring page (SIRBP).
>
> Currently mshv_synic_init() redundantly enables SIMP, SIEFP, and
> SCONTROL that VMBus already configured, and mshv_synic_cleanup()
> disables all of them. This is wrong because MSHV can be torn down
> while VMBus is still active. In particular, a kexec reboot notifier
> tears down MSHV first. Disabling SCONTROL, SIMP, and SIEFP out from
> under VMBus causes its later cleanup to write SynIC MSRs while SynIC
> is disabled, which the hypervisor does not tolerate.
>
> Restrict MSHV to managing only the resources it owns:
> - SINT0, SINT5: mask on cleanup, unmask on init
> - SIRBP: enable/disable as before
> - SIMP, SIEFP, SCONTROL: on L1VH leave entirely to VMBus (it
> already enabled them); on root partition VMBus doesn't run, so
> MSHV must enable/disable them
>
> Signed-off-by: Jork Loeser <jloeser@linux.microsoft.com>
> ---
> drivers/hv/mshv_synic.c | 109 ++++++++++++++++++++++++----------------
> 1 file changed, 67 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/hv/mshv_synic.c b/drivers/hv/mshv_synic.c
> index f8b0337cdc82..8a7d76a10dc3 100644
> --- a/drivers/hv/mshv_synic.c
> +++ b/drivers/hv/mshv_synic.c
> @@ -454,7 +454,6 @@ int mshv_synic_init(unsigned int cpu)
> #ifdef HYPERVISOR_CALLBACK_VECTOR
> union hv_synic_sint sint;
> #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->hyp_synic_message_page;
> struct hv_synic_event_flags_page **event_flags_page =
> @@ -462,28 +461,37 @@ int mshv_synic_init(unsigned int cpu)
> struct hv_synic_event_ring_page **event_ring_page =
> &spages->synic_event_ring_page;
>
> - /* Setup the Synic's message page */
> + /*
> + * Map the SYNIC message page. On root partition the hypervisor
> + * pre-provisions the SIMP GPA but may not set simp_enabled;
> + * on L1VH, VMBus already fully set it up. Enable it on root.
> + */
> simp.as_uint64 = hv_get_non_nested_msr(HV_MSR_SIMP);
> - simp.simp_enabled = true;
> + if (hv_root_partition()) {
Is it possible to split out the root partition logic to a separate
function(s) instead of weawing it into this function?
Ideally, there should be a generic function called by VMBUS and a
root partition-specific function called by MSHV if needed.
Thanks,
Stanislav
> + 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);
>
> if (!(*msg_page))
> - return -EFAULT;
> -
> - hv_set_non_nested_msr(HV_MSR_SIMP, simp.as_uint64);
> + goto cleanup_simp;
>
> - /* Setup the Synic's event flags page */
> + /*
> + * Map the event flags page. Same as SIMP: enable on root,
> + * already enabled by VMBus on L1VH.
> + */
> siefp.as_uint64 = hv_get_non_nested_msr(HV_MSR_SIEFP);
> - siefp.siefp_enabled = true;
> + if (hv_root_partition()) {
> + siefp.siefp_enabled = true;
> + hv_set_non_nested_msr(HV_MSR_SIEFP, siefp.as_uint64);
> + }
> *event_flags_page = memremap(siefp.base_siefp_gpa << PAGE_SHIFT,
> PAGE_SIZE, MEMREMAP_WB);
>
> if (!(*event_flags_page))
> - goto cleanup;
> -
> - hv_set_non_nested_msr(HV_MSR_SIEFP, siefp.as_uint64);
> + goto cleanup_siefp;
>
> /* Setup the Synic's event ring page */
> sirbp.as_uint64 = hv_get_non_nested_msr(HV_MSR_SIRBP);
> @@ -492,7 +500,7 @@ int mshv_synic_init(unsigned int cpu)
> PAGE_SIZE, MEMREMAP_WB);
>
> if (!(*event_ring_page))
> - goto cleanup;
> + goto cleanup_siefp;
>
> hv_set_non_nested_msr(HV_MSR_SIRBP, sirbp.as_uint64);
>
> @@ -515,28 +523,33 @@ int mshv_synic_init(unsigned int cpu)
> sint.as_uint64);
> #endif
>
> - /* Enable global synic bit */
> - sctrl.as_uint64 = hv_get_non_nested_msr(HV_MSR_SCONTROL);
> - sctrl.enable = 1;
> - hv_set_non_nested_msr(HV_MSR_SCONTROL, sctrl.as_uint64);
> + /*
> + * On L1VH, VMBus owns SCONTROL and has already enabled it.
> + * On root partition, VMBus doesn't run so we must enable it.
> + */
> + if (hv_root_partition()) {
> + union hv_synic_scontrol sctrl;
> +
> + sctrl.as_uint64 = hv_get_non_nested_msr(HV_MSR_SCONTROL);
> + sctrl.enable = 1;
> + hv_set_non_nested_msr(HV_MSR_SCONTROL, sctrl.as_uint64);
> + }
>
> return 0;
>
> -cleanup:
> - if (*event_ring_page) {
> - sirbp.sirbp_enabled = false;
> - hv_set_non_nested_msr(HV_MSR_SIRBP, sirbp.as_uint64);
> - memunmap(*event_ring_page);
> - }
> - if (*event_flags_page) {
> +cleanup_siefp:
> + if (*event_flags_page)
> + memunmap(*event_flags_page);
> + if (hv_root_partition()) {
> siefp.siefp_enabled = false;
> hv_set_non_nested_msr(HV_MSR_SIEFP, siefp.as_uint64);
> - memunmap(*event_flags_page);
> }
> - if (*msg_page) {
> +cleanup_simp:
> + if (*msg_page)
> + memunmap(*msg_page);
> + if (hv_root_partition()) {
> simp.simp_enabled = false;
> hv_set_non_nested_msr(HV_MSR_SIMP, simp.as_uint64);
> - memunmap(*msg_page);
> }
>
> return -EFAULT;
> @@ -545,10 +558,7 @@ int mshv_synic_init(unsigned int cpu)
> int mshv_synic_cleanup(unsigned int cpu)
> {
> union hv_synic_sint sint;
> - union hv_synic_simp simp;
> - union hv_synic_siefp siefp;
> 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->hyp_synic_message_page;
> struct hv_synic_event_flags_page **event_flags_page =
> @@ -568,28 +578,43 @@ int mshv_synic_cleanup(unsigned int cpu)
> hv_set_non_nested_msr(HV_MSR_SINT0 + HV_SYNIC_DOORBELL_SINT_INDEX,
> sint.as_uint64);
>
> - /* Disable Synic's event ring page */
> + /* Disable SYNIC event ring page owned by MSHV */
> sirbp.as_uint64 = hv_get_non_nested_msr(HV_MSR_SIRBP);
> sirbp.sirbp_enabled = false;
> hv_set_non_nested_msr(HV_MSR_SIRBP, sirbp.as_uint64);
> memunmap(*event_ring_page);
>
> - /* Disable Synic's event flags page */
> - siefp.as_uint64 = hv_get_non_nested_msr(HV_MSR_SIEFP);
> - siefp.siefp_enabled = false;
> - hv_set_non_nested_msr(HV_MSR_SIEFP, siefp.as_uint64);
> + /*
> + * Release our mappings of the message and event flags pages.
> + * On root partition, we enabled SIMP/SIEFP — disable them.
> + * On L1VH, VMBus owns the MSRs, leave them alone.
> + */
> memunmap(*event_flags_page);
> + if (hv_root_partition()) {
> + union hv_synic_simp simp;
> + union hv_synic_siefp siefp;
>
> - /* Disable Synic's message page */
> - simp.as_uint64 = hv_get_non_nested_msr(HV_MSR_SIMP);
> - simp.simp_enabled = false;
> - hv_set_non_nested_msr(HV_MSR_SIMP, simp.as_uint64);
> + siefp.as_uint64 = hv_get_non_nested_msr(HV_MSR_SIEFP);
> + siefp.siefp_enabled = false;
> + hv_set_non_nested_msr(HV_MSR_SIEFP, siefp.as_uint64);
> +
> + simp.as_uint64 = hv_get_non_nested_msr(HV_MSR_SIMP);
> + simp.simp_enabled = false;
> + hv_set_non_nested_msr(HV_MSR_SIMP, simp.as_uint64);
> + }
> memunmap(*msg_page);
>
> - /* Disable global synic bit */
> - sctrl.as_uint64 = hv_get_non_nested_msr(HV_MSR_SCONTROL);
> - sctrl.enable = 0;
> - hv_set_non_nested_msr(HV_MSR_SCONTROL, sctrl.as_uint64);
> + /*
> + * On root partition, we enabled SCONTROL in init — disable it.
> + * On L1VH, VMBus owns SCONTROL, leave it alone.
> + */
> + if (hv_root_partition()) {
> + union hv_synic_scontrol sctrl;
> +
> + sctrl.as_uint64 = hv_get_non_nested_msr(HV_MSR_SCONTROL);
> + sctrl.enable = 0;
> + hv_set_non_nested_msr(HV_MSR_SCONTROL, sctrl.as_uint64);
> + }
>
> return 0;
> }
> --
> 2.43.0
>
next prev parent reply other threads:[~2026-03-30 21:51 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-27 20:19 [PATCH 0/6] Hyper-V: kexec fixes for L1VH (mshv) Jork Loeser
2026-03-27 20:19 ` [PATCH 1/6] Drivers: hv: vmbus: fix hyperv_cpuhp_online variable shadowing Jork Loeser
2026-03-30 21:25 ` Stanislav Kinsburskii
2026-04-02 15:55 ` Anirudh Rayabharam
2026-03-27 20:19 ` [PATCH 2/6] x86/hyperv: move stimer cleanup to hv_machine_shutdown() Jork Loeser
2026-03-30 21:26 ` Stanislav Kinsburskii
2026-03-27 20:19 ` [PATCH 3/6] x86/hyperv: Skip LP/VP creation on kexec Jork Loeser
2026-03-30 21:30 ` Stanislav Kinsburskii
2026-03-27 20:19 ` [PATCH 4/6] mshv: limit SynIC management to MSHV-owned resources Jork Loeser
2026-03-30 21:51 ` Stanislav Kinsburskii [this message]
2026-04-01 17:43 ` Jork Loeser
2026-04-02 16:13 ` Anirudh Rayabharam
2026-04-02 21:21 ` Jork Loeser
2026-04-03 3:59 ` Anirudh Rayabharam
2026-04-03 4:34 ` Anirudh Rayabharam
2026-04-03 19:19 ` Jork Loeser
2026-03-27 20:19 ` [PATCH 5/6] mshv: clean up SynIC state on kexec for L1VH Jork Loeser
2026-03-30 21:52 ` Stanislav Kinsburskii
2026-03-27 20:19 ` [PATCH 6/6] mshv: unmap debugfs stats pages on kexec Jork Loeser
2026-03-30 21:54 ` Stanislav Kinsburskii
2026-04-01 17:47 ` Jork Loeser
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=acrwYf50SJnDwN3e@skinsburskii.localdomain \
--to=skinsburskii@linux.microsoft.com \
--cc=arnd@arndb.de \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=decui@microsoft.com \
--cc=haiyangz@microsoft.com \
--cc=hpa@zytor.com \
--cc=jloeser@linux.microsoft.com \
--cc=kys@microsoft.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=longli@microsoft.com \
--cc=mhklinux@outlook.com \
--cc=mingo@redhat.com \
--cc=romank@linux.microsoft.com \
--cc=tglx@kernel.org \
--cc=wei.liu@kernel.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox