On Mon, Feb 02, 2026 at 11:07:17AM -0800, Stanislav Kinsburskii wrote:
> On Mon, Feb 02, 2026 at 06:27:05PM +0000, Anirudh Rayabharam wrote:
> > From: Anirudh Rayabharam (Microsoft) <[email protected]>
> >
> > Rename mshv_synic_init() to mshv_synic_cpu_init() and
> > mshv_synic_cleanup() to mshv_synic_cpu_exit() to better reflect that
> > these functions handle per-cpu synic setup and teardown.
> >
> > Use mshv_synic_init/cleanup() to perform init/cleanup that is not per-cpu.
> > Move all the synic related setup from mshv_parent_partition_init.
> >
> > Move the reboot notifier to mshv_synic.c because it currently only
> > operates on the synic cpuhp state.
> >
> > Move out synic_pages from the global mshv_root since it's use is now
> > completely local to mshv_synic.c.
> >
> > This is in preparation for the next patch which will add more stuff to
> > mshv_synic_init().
> >
> > No functional change.
> >
> > Signed-off-by: Anirudh Rayabharam (Microsoft) <[email protected]>
> > ---
> > drivers/hv/mshv_root.h | 5 ++-
> > drivers/hv/mshv_root_main.c | 59 +++++-------------------------
> > drivers/hv/mshv_synic.c | 71 +++++++++++++++++++++++++++++++++----
> > 3 files changed, 75 insertions(+), 60 deletions(-)
> >
> > diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
> > index 3c1d88b36741..26e0320c8097 100644
> > --- a/drivers/hv/mshv_root.h
> > +++ b/drivers/hv/mshv_root.h
> > @@ -183,7 +183,6 @@ struct hv_synic_pages {
> > };
> >
> > struct mshv_root {
> > - struct hv_synic_pages __percpu *synic_pages;
> > spinlock_t pt_ht_lock;
> > DECLARE_HASHTABLE(pt_htable, MSHV_PARTITIONS_HASH_BITS);
> > struct hv_partition_property_vmm_capabilities vmm_caps;
> > @@ -242,8 +241,8 @@ int mshv_register_doorbell(u64 partition_id,
> > doorbell_cb_t doorbell_cb,
> > void mshv_unregister_doorbell(u64 partition_id, int doorbell_portid);
> >
> > void mshv_isr(void);
> > -int mshv_synic_init(unsigned int cpu);
> > -int mshv_synic_cleanup(unsigned int cpu);
> > +int mshv_synic_init(struct device *dev);
> > +void mshv_synic_cleanup(void);
> >
> > static inline bool mshv_partition_encrypted(struct mshv_partition
> > *partition)
> > {
> > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> > index 681b58154d5e..7c1666456e78 100644
> > --- a/drivers/hv/mshv_root_main.c
> > +++ b/drivers/hv/mshv_root_main.c
> > @@ -2035,7 +2035,6 @@ mshv_dev_release(struct inode *inode, struct file
> > *filp)
> > return 0;
> > }
> >
> > -static int mshv_cpuhp_online;
> > static int mshv_root_sched_online;
> >
> > static const char *scheduler_type_to_string(enum hv_scheduler_type type)
> > @@ -2198,40 +2197,14 @@ root_scheduler_deinit(void)
> > free_percpu(root_scheduler_output);
> > }
> >
> > -static int mshv_reboot_notify(struct notifier_block *nb,
> > - unsigned long code, void *unused)
> > -{
> > - cpuhp_remove_state(mshv_cpuhp_online);
> > - return 0;
> > -}
> > -
>
> Unrelated to the change, but it would be great to get rid of this
> notifier altogether and just do the cleanup in the device shutdown hook.
> This is a cleaner approach as this is a device driver and we do have the
> device in hands.
> Do you think you could make this change a part of this series?
That needs to more investigation. The notifier is there because it is
called during kexec. Whether device shutdown hook also works is
something I need to check. I will prefer it to be a separate patch.
Makes it easy to reason that this patch indeed has "No functional
changes".
>
> > -struct notifier_block mshv_reboot_nb = {
> > - .notifier_call = mshv_reboot_notify,
> > -};
> > -
> > static void mshv_root_partition_exit(void)
> > {
> > - unregister_reboot_notifier(&mshv_reboot_nb);
> > root_scheduler_deinit();
> > }
> >
> > static int __init mshv_root_partition_init(struct device *dev)
> > {
> > - int err;
> > -
> > - err = root_scheduler_init(dev);
> > - if (err)
> > - return err;
> > -
> > - err = register_reboot_notifier(&mshv_reboot_nb);
> > - if (err)
> > - goto root_sched_deinit;
> > -
> > - return 0;
> > -
> > -root_sched_deinit:
> > - root_scheduler_deinit();
> > - return err;
> > + return root_scheduler_init(dev);
> > }
> >
>
> This conflicts with the "mshv: Add support for integrated scheduler"
> patch out there.
> Perhaps we should ask Wei to merge that change first.
Sure, I'm okay with that ordering.
>
> > static void mshv_init_vmm_caps(struct device *dev)
> > @@ -2276,31 +2249,18 @@ static int __init mshv_parent_partition_init(void)
> > MSHV_HV_MAX_VERSION);
> > }
> >
> > - mshv_root.synic_pages = alloc_percpu(struct hv_synic_pages);
> > - if (!mshv_root.synic_pages) {
> > - dev_err(dev, "Failed to allocate percpu synic page\n");
> > - ret = -ENOMEM;
> > + ret = mshv_synic_init(dev);
> > + if (ret)
> > goto device_deregister;
> > - }
> > -
> > - ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "mshv_synic",
> > - mshv_synic_init,
> > - mshv_synic_cleanup);
> > - if (ret < 0) {
> > - dev_err(dev, "Failed to setup cpu hotplug state: %i\n", ret);
> > - goto free_synic_pages;
> > - }
> > -
> > - mshv_cpuhp_online = ret;
> >
> > ret = mshv_retrieve_scheduler_type(dev);
> > if (ret)
> > - goto remove_cpu_state;
> > + goto synic_cleanup;
> >
> > if (hv_root_partition())
> > ret = mshv_root_partition_init(dev);
> > if (ret)
> > - goto remove_cpu_state;
> > + goto synic_cleanup;
> >
> > mshv_init_vmm_caps(dev);
> >
> > @@ -2318,10 +2278,8 @@ static int __init mshv_parent_partition_init(void)
> > exit_partition:
> > if (hv_root_partition())
> > mshv_root_partition_exit();
> > -remove_cpu_state:
> > - cpuhp_remove_state(mshv_cpuhp_online);
> > -free_synic_pages:
> > - free_percpu(mshv_root.synic_pages);
> > +synic_cleanup:
> > + mshv_synic_cleanup();
> > device_deregister:
> > misc_deregister(&mshv_dev);
> > return ret;
> > @@ -2335,8 +2293,7 @@ static void __exit mshv_parent_partition_exit(void)
> > mshv_irqfd_wq_cleanup();
> > if (hv_root_partition())
> > mshv_root_partition_exit();
> > - cpuhp_remove_state(mshv_cpuhp_online);
> > - free_percpu(mshv_root.synic_pages);
> > + mshv_synic_cleanup();
> > }
> >
> > module_init(mshv_parent_partition_init);
> > diff --git a/drivers/hv/mshv_synic.c b/drivers/hv/mshv_synic.c
> > index f8b0337cdc82..98c58755846d 100644
> > --- a/drivers/hv/mshv_synic.c
> > +++ b/drivers/hv/mshv_synic.c
> > @@ -12,11 +12,16 @@
> > #include <linux/mm.h>
> > #include <linux/io.h>
> > #include <linux/random.h>
> > +#include <linux/cpuhotplug.h>
> > +#include <linux/reboot.h>
> > #include <asm/mshyperv.h>
> >
> > #include "mshv_eventfd.h"
> > #include "mshv.h"
> >
> > +static int synic_cpuhp_online;
> > +static struct hv_synic_pages __percpu *synic_pages;
> > +
> > static u32 synic_event_ring_get_queued_port(u32 sint_index)
> > {
> > struct hv_synic_event_ring_page **event_ring_page;
> > @@ -26,7 +31,7 @@ static u32 synic_event_ring_get_queued_port(u32
> > sint_index)
> > u32 message;
> > u8 tail;
> >
> > - spages = this_cpu_ptr(mshv_root.synic_pages);
> > + spages = this_cpu_ptr(synic_pages);
> > event_ring_page = &spages->synic_event_ring_page;
> > synic_eventring_tail = (u8 **)this_cpu_ptr(hv_synic_eventring_tail);
> >
> > @@ -393,7 +398,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_synic_pages *spages = this_cpu_ptr(synic_pages);
> > struct hv_message_page **msg_page = &spages->hyp_synic_message_page;
> > struct hv_message *msg;
> > bool handled;
> > @@ -446,7 +451,7 @@ void mshv_isr(void)
> > }
> > }
> >
> > -int mshv_synic_init(unsigned int cpu)
> > +static int mshv_synic_cpu_init(unsigned int cpu)
> > {
> > union hv_synic_simp simp;
> > union hv_synic_siefp siefp;
> > @@ -455,7 +460,7 @@ int mshv_synic_init(unsigned int cpu)
> > 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_synic_pages *spages = this_cpu_ptr(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;
> > @@ -542,14 +547,14 @@ int mshv_synic_init(unsigned int cpu)
> > return -EFAULT;
> > }
> >
> > -int mshv_synic_cleanup(unsigned int cpu)
> > +static int mshv_synic_cpu_exit(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_synic_pages *spages = this_cpu_ptr(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;
> > @@ -663,3 +668,57 @@ mshv_unregister_doorbell(u64 partition_id, int
> > doorbell_portid)
> >
> > mshv_portid_free(doorbell_portid);
> > }
> > +
> > +static int mshv_synic_reboot_notify(struct notifier_block *nb,
> > + unsigned long code, void *unused)
> > +{
> > + cpuhp_remove_state(synic_cpuhp_online);
> > + return 0;
> > +}
> > +
> > +static struct notifier_block mshv_synic_reboot_nb = {
> > + .notifier_call = mshv_synic_reboot_notify,
> > +};
> > +
> > +int __init mshv_synic_init(struct device *dev)
> > +{
> > + int ret = 0;
> > +
> > + synic_pages = alloc_percpu(struct hv_synic_pages);
> > + if (!synic_pages) {
> > + dev_err(dev, "Failed to allocate percpu synic page\n");
> > + return -ENOMEM;
> > + }
> > +
> > + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "mshv_synic",
> > + mshv_synic_cpu_init,
> > + mshv_synic_cpu_exit);
> > + if (ret < 0) {
> > + dev_err(dev, "Failed to setup cpu hotplug state: %i\n", ret);
> > + goto free_synic_pages;
> > + }
> > +
> > + synic_cpuhp_online = ret;
> > +
> > + if (hv_root_partition()) {
>
> Nit: it's probably better to branch in the notifier itself.
> It will introduce an additional object, but the branching will be in one
> palce instead of two and it will also make to code simpler and easier to
> read.
Maybe I introduce mshv_synic_root_partition_init/exit() which will have
branching inside? Similar to what we did in mshv_root_main.c. That will
avoid introducing the additional object. But I guess the branch will
still be in both init and exit functions...
Thanks,
Anirudh.
>
> Thanks
> Stanislav.
>
> > + ret = register_reboot_notifier(&mshv_synic_reboot_nb);
> > + if (ret)
> > + goto remove_cpuhp_state;
> > + }
> > +
> > + return 0;
> > +
> > +remove_cpuhp_state:
> > + cpuhp_remove_state(synic_cpuhp_online);
> > +free_synic_pages:
> > + free_percpu(synic_pages);
> > + return ret;
> > +}
> > +
> > +void mshv_synic_cleanup(void)
> > +{
> > + if (hv_root_partition())
> > + unregister_reboot_notifier(&mshv_synic_reboot_nb);
> > + cpuhp_remove_state(synic_cpuhp_online);
> > + free_percpu(synic_pages);
> > +}
> > --
> > 2.34.1
> >
