[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<SN6PR02MB41570DFE89DFBC3476432562D49FA@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Fri, 30 Jan 2026 17:49:02 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Anirudh Rayabharam <anirudh@...rudhrb.com>
CC: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>,
"kys@...rosoft.com" <kys@...rosoft.com>, "haiyangz@...rosoft.com"
<haiyangz@...rosoft.com>, "wei.liu@...nel.org" <wei.liu@...nel.org>,
"decui@...rosoft.com" <decui@...rosoft.com>, "longli@...rosoft.com"
<longli@...rosoft.com>, "linux-hyperv@...r.kernel.org"
<linux-hyperv@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 2/2] mshv: Add support for integrated scheduler
From: Anirudh Rayabharam <anirudh@...rudhrb.com> Sent: Friday, January 30, 2026 9:37 AM
>
> On Fri, Jan 30, 2026 at 01:24:34AM +0000, Michael Kelley wrote:
> > From: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com> Sent: Thursday, January 29, 2026 11:10 AM
> > >
> > > On Thu, Jan 29, 2026 at 05:47:02PM +0000, Michael Kelley wrote:
> > > > From: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com> Sent: Wednesday, January 21, 2026 2:36 PM
> > >
> > > <snip>
> > >
> > > > > 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 err;
> > > > >
> > > > > return 0;
> > > >
> > > > This code is now:
> > > >
> > > > if (err)
> > > > return err;
> > > > return 0;
> > > >
> > > > which can be simplified to just:
> > > >
> > > > return err;
> > > >
> > > > Or drop the local variable 'err' and simplify the entire function to:
> > > >
> > > > return register_reboot_notifier(&mshv_reboot_nb);
> > > >
> > > > There's a tangential question here: Why is this reboot notifier
> > > > needed in the first place? All it does is remove the cpuhp state
> > > > that allocates/frees the per-cpu root_scheduler_input and
> > > > root_scheduler_output pages. Removing the state will free
> > > > the pages, but if Linux is rebooting, why bother?
> > > >
> > >
> > > This was originally done to support kexec.
> > > Here is the original commit message:
> > >
> > > mshv: perform synic cleanup during kexec
> > >
> > > Register a reboot notifier that performs synic cleanup when a kexec
> > > is in progress.
> > >
> > > One notable issue this commit fixes is one where after a kexec, virtio
> > > devices are not functional. Linux root partition receives MMIO doorbell
> > > events in the ring buffer in the SIRB synic page. The hypervisor maintains
> > > a head pointer where it writes new events into the ring buffer. The root
> > > partition maintains a tail pointer to read events from the buffer.
> > >
> > > Upon kexec reboot, all root data structures are re-initialized and thus the
> > > tail pointer gets reset to zero. The hypervisor on the other hand still
> > > retains the pre-kexec head pointer which could be non-zero. This means that
> > > when the hypervisor writes new events to the ring buffer, the root
> > > partition looks at the wrong place and doesn't find any events. So, future
> > > doorbell events never get delivered. As a result, virtqueue kicks never get
> > > delivered to the host.
> > >
> > > When the SIRB page is disabled the hypervisor resets the head pointer.
> >
> > FWIW, I don't see that commit message anywhere in a public source code
> > tree. The calls to register/unregister_reboot_notifier() were in the original
> > introduction of mshv_root_main.c in upstream commit 621191d709b14.
> > Evidently the code described by that commit message was not submitted
> > upstream. And of course, the kexec() topic is now being revisited ....
> >
> > So to clarify: Do you expect that in the future the reboot notifier will be
> > used for something that really is required for resetting hypervisor state
> > in the case of a kexec reboot?
>
> While that commit wasn't individually sent upstream but all the code
> from that commit did land upstream probably bundled with other commits
> when the mshv driver was introduced. So the reboot notifier is indeed
> currently used for resetting the synic correctly during kexec reboot.
>
Indeed, you are right. I confused the "mshv_root_sched_online" and
"mshv_cpuhp_online" cpuhp states. The reboot notifier removes the latter,
not the former. And the latter does substantive cleanup work on the SynIC
when the state is removed. Apologies for the confusion.
Michael
Powered by blists - more mailing lists