Re: [PATCH v2 4/6] mshv: limit SynIC management to MSHV-owned resources

[Anirudh Rayabharam]

On Fri, Apr 03, 2026 at 12:06:10PM -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

The redundant enable is probably a no-op from the hypervisor side so it
probably doesn't hurt us. The main problem is with the tear down.

> 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: leave to VMBus when it is active (L1VH
>   and nested root partition); on a non-nested root partition VMBus
>   doesn't run, so MSHV must enable/disable them
> 
> Signed-off-by: Jork Loeser <jloeser@xxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/hv/mshv_synic.c | 142 ++++++++++++++++++++++++++--------------
>  1 file changed, 94 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/hv/mshv_synic.c b/drivers/hv/mshv_synic.c
> index f8b0337cdc82..7d273766bdb5 100644
> --- a/drivers/hv/mshv_synic.c
> +++ b/drivers/hv/mshv_synic.c
> @@ -454,46 +454,72 @@ 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 =
>  			&spages->synic_event_flags_page;
>  	struct hv_synic_event_ring_page **event_ring_page =
>  			&spages->synic_event_ring_page;
> +	/* VMBus runs on L1VH and nested root; it owns SIMP/SIEFP/SCONTROL */
> +	bool vmbus_active = !hv_root_partition() || hv_nested;

An alternative approach could be: check if SIMP/SIEFP/SCONTROL is
already enabled. If so, don't enable it again. If not enabled, enable it
and keep track of what all stuf we have enabled. Then disable all of
them during cleanup. This approach makes less assumptions about the
behavior of the VMBUS driver and what stuff it does or doesn't use.

Thanks,
Anirudh.