move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 774 bytes --]

KY,

if a hyperv VM crashes alot of work must be done to prepare the
environment for the kdump kernel. This approach is different compared to
all the other VM types, or baremetal. Since the just crashed kernel is
per definition unreliable all that work should be done within the kdump
kernel because I think a reliable environment exists only there.

Was it ever considered to do the CHANNELMSG_UNLOAD /
CHANNELMSG_UNLOAD_RESPONSE work during boot, instead of doing it before
starting the kexec/kdump kernel?

What would it take to prepare the runtime environment during boot?
Does the newly booted kernel need any info from the previous kernel,
something that cant be determined during boot? If yes, how can such info
be passed from the old kernel to the new kernel?

Olaf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

RE: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[KY Srinivasan]

> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Wednesday, December 7, 2016 12:51 AM
> To: KY Srinivasan <kys@microsoft.com>; vkuznets@redhat.com
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org
> Subject: move hyperv CHANNELMSG_UNLOAD from crashed kernel to
> kdump kernel
> 
> KY,
> 
> if a hyperv VM crashes alot of work must be done to prepare the
> environment for the kdump kernel. This approach is different compared to
> all the other VM types, or baremetal. Since the just crashed kernel is
> per definition unreliable all that work should be done within the kdump
> kernel because I think a reliable environment exists only there.
> 
> Was it ever considered to do the CHANNELMSG_UNLOAD /
> CHANNELMSG_UNLOAD_RESPONSE work during boot, instead of doing it
> before
> starting the kexec/kdump kernel?
> 
> What would it take to prepare the runtime environment during boot?
> Does the newly booted kernel need any info from the previous kernel,
> something that cant be determined during boot? If yes, how can such info
> be passed from the old kernel to the new kernel?

Yes; I had played with this approach a while ago. The issue is that the host knows about a 
bunch of in memory state that will be different in the kexec kernel. For instance if we did all
the cleanup as part of the boot sequence, we will need access to all the interrupt/messaging 
infrastructure that was set up in the previous kernel.

K. Y
> 
> Olaf

RE: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[Olaf Hering]

Am 7. Dezember 2016 16:04:29 MEZ, schrieb KY Srinivasan <kys@microsoft.com>:

>Yes; I had played with this approach a while ago. The issue is that the
>host knows about a 
>bunch of in memory state that will be different in the kexec kernel.
>For instance if we did all
>the cleanup as part of the boot sequence, we will need access to all
>the interrupt/messaging 
>infrastructure that was set up in the previous kernel.


Where is that stored?  Perhaps it should be put into one place,  so that the new kernel can find and use it. 

Olaf 

RE: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[KY Srinivasan]

> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Wednesday, December 7, 2016 7:47 AM
> To: KY Srinivasan <kys@microsoft.com>; vkuznets@redhat.com
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org
> Subject: RE: move hyperv CHANNELMSG_UNLOAD from crashed kernel to
> kdump kernel
> 
> Am 7. Dezember 2016 16:04:29 MEZ, schrieb KY Srinivasan
> <kys@microsoft.com>:
> 
> >Yes; I had played with this approach a while ago. The issue is that the
> >host knows about a
> >bunch of in memory state that will be different in the kexec kernel.
> >For instance if we did all
> >the cleanup as part of the boot sequence, we will need access to all
> >the interrupt/messaging
> >infrastructure that was set up in the previous kernel.
> 
> 
> Where is that stored?  Perhaps it should be put into one place,  so that the
> new kernel can find and use it.

All the state required to handle interrupts is needed as well as the state needed
to issue unload. Look at vmbus_isr() for the needed interrupt handling state. The entire
code path from the isr routine will have to be executed. Is there a mechanism for stashing away state
that can be retrieved in the context of the execed kernel.

K. Y
> 
> Olaf

Re: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 366 bytes --]

On Wed, Dec 07, KY Srinivasan wrote:

> Is there a mechanism for stashing away state that can be retrieved in
> the context of the execed kernel.

I have to find out. To simplify things the new approach may only be used
in the kdump case, which already passes various info in cmdline. Most
likely there is a way to preserve a few gpfns with the relevant data.

Olaf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

RE: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[KY Srinivasan]

> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Wednesday, December 7, 2016 8:19 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: vkuznets@redhat.com; gregkh@linuxfoundation.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org
> Subject: Re: move hyperv CHANNELMSG_UNLOAD from crashed kernel to
> kdump kernel
> 
> On Wed, Dec 07, KY Srinivasan wrote:
> 
> > Is there a mechanism for stashing away state that can be retrieved in
> > the context of the execed kernel.
> 
> I have to find out. To simplify things the new approach may only be used
> in the kdump case, which already passes various info in cmdline. Most
> likely there is a way to preserve a few gpfns with the relevant data.

May be a better solution might be to have a new mechanism to indicate to the
host that all state of the previous incarnation of the kernel needs to be cleaned up.
This will be close to what we have on hardware.

K. Y 
> 
> Olaf

Re: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 394 bytes --]

On Wed, Dec 07, KY Srinivasan wrote:

> May be a better solution might be to have a new mechanism to indicate
> to the host that all state of the previous incarnation of the kernel
> needs to be cleaned up.  This will be close to what we have on
> hardware.

That would be cool, but until this appears and until its deployed in
Azure and elsewhere I will likely lose the remaining hairs.

Olaf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

RE: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[KY Srinivasan]

> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Wednesday, December 7, 2016 8:40 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: vkuznets@redhat.com; gregkh@linuxfoundation.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org
> Subject: Re: move hyperv CHANNELMSG_UNLOAD from crashed kernel to
> kdump kernel
> 
> On Wed, Dec 07, KY Srinivasan wrote:
> 
> > May be a better solution might be to have a new mechanism to indicate
> > to the host that all state of the previous incarnation of the kernel
> > needs to be cleaned up.  This will be close to what we have on
> > hardware.
> 
> That would be cool, but until this appears and until its deployed in
> Azure and elsewhere I will likely lose the remaining hairs.

I think the code we have upstream works; I have not seen any issues reported.
I will try to get the host guys to implement this - hopefully it won't be too late!

K. Y
> 
> Olaf

Re: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[Vitaly Kuznetsov]

Olaf Hering <olaf@aepfle.de> writes:

> KY,
>
> if a hyperv VM crashes alot of work must be done to prepare the
> environment for the kdump kernel. This approach is different compared to
> all the other VM types, or baremetal. Since the just crashed kernel is
> per definition unreliable all that work should be done within the kdump
> kernel because I think a reliable environment exists only there.
>
> Was it ever considered to do the CHANNELMSG_UNLOAD /
> CHANNELMSG_UNLOAD_RESPONSE work during boot, instead of doing it before
> starting the kexec/kdump kernel?

Sorry guys I missed the discussion, I was on vacation.

I see a number of minor but at least one major issue against such move:
At least for some Hyper-V versions (2012R2 for example)
CHANNELMSG_UNLOAD_RESPONSE is delivered to the CPU which initially sent 
CHANNELMSG_REQUESTOFFERS and on kdump we may not have this CPU up as
we usually do kdump with nr_cpus=1 (and on the CPU which crashed). 

Minor issue is the necessity preserve the information about
message/events pages across kexec.

>
> What would it take to prepare the runtime environment during boot?
> Does the newly booted kernel need any info from the previous kernel,
> something that cant be determined during boot? If yes, how can such info
> be passed from the old kernel to the new kernel?
>
> Olaf
>

-- 
  Vitaly

Re: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

On Thu, Dec 15, Vitaly Kuznetsov wrote:

> I see a number of minor but at least one major issue against such move:
> At least for some Hyper-V versions (2012R2 for example)
> CHANNELMSG_UNLOAD_RESPONSE is delivered to the CPU which initially sent 
> CHANNELMSG_REQUESTOFFERS and on kdump we may not have this CPU up as
> we usually do kdump with nr_cpus=1 (and on the CPU which crashed). 

Since the kdump or kexec kernel will send the unload during boot I would
expect the response to arrive where it was sent, independent from the
number of cpus.

> Minor issue is the necessity preserve the information about
> message/events pages across kexec.

I guess this info is stored somewhere, and the relevant gfns can be
preserved across kernels, if we try really hard.


But after looking further at the involved code paths it seems that the
implemnted polling might be good enough to snatch the response. Was the
mdelay(10) just an arbitrary decision? I interpret the comments in
vmbus_signal_eom such that the host may overwrite the response. Perhaps
such thing may happen during the mdelay?


Olaf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

On Thu, Dec 15, Olaf Hering wrote:

> On Thu, Dec 15, Vitaly Kuznetsov wrote:
> 
> > I see a number of minor but at least one major issue against such move:
> > At least for some Hyper-V versions (2012R2 for example)
> > CHANNELMSG_UNLOAD_RESPONSE is delivered to the CPU which initially sent 
> > CHANNELMSG_REQUESTOFFERS and on kdump we may not have this CPU up as
> > we usually do kdump with nr_cpus=1 (and on the CPU which crashed). 
> 
> Since the kdump or kexec kernel will send the unload during boot I would
> expect the response to arrive where it was sent, independent from the
> number of cpus.

Wait, I just noticed that "REQUESTOFFERS" now. That might be a reason
why my suggestion will not work.

Olaf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[Vitaly Kuznetsov]

Olaf Hering <olaf@aepfle.de> writes:

> On Thu, Dec 15, Olaf Hering wrote:
>
>> On Thu, Dec 15, Vitaly Kuznetsov wrote:
>> 
>> > I see a number of minor but at least one major issue against such move:
>> > At least for some Hyper-V versions (2012R2 for example)
>> > CHANNELMSG_UNLOAD_RESPONSE is delivered to the CPU which initially sent 
>> > CHANNELMSG_REQUESTOFFERS and on kdump we may not have this CPU up as
>> > we usually do kdump with nr_cpus=1 (and on the CPU which crashed). 
>> 
>> Since the kdump or kexec kernel will send the unload during boot I would
>> expect the response to arrive where it was sent, independent from the
>> number of cpus.
>
> Wait, I just noticed that "REQUESTOFFERS" now. That might be a reason
> why my suggestion will not work.
>

Yep, that's what I meant to say.

-- 
  Vitaly

Re: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[Vitaly Kuznetsov]

Olaf Hering <olaf@aepfle.de> writes:

> On Thu, Dec 15, Vitaly Kuznetsov wrote:
>
>> I see a number of minor but at least one major issue against such move:
>> At least for some Hyper-V versions (2012R2 for example)
>> CHANNELMSG_UNLOAD_RESPONSE is delivered to the CPU which initially sent 
>> CHANNELMSG_REQUESTOFFERS and on kdump we may not have this CPU up as
>> we usually do kdump with nr_cpus=1 (and on the CPU which crashed). 
>
> Since the kdump or kexec kernel will send the unload during boot I would
> expect the response to arrive where it was sent, independent from the
> number of cpus.
>

We actually need to read the reply and empty the message slot to make
unload happen. And reading on a different CPU may not work, see:

http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2016-December/097330.html

>> Minor issue is the necessity preserve the information about
>> message/events pages across kexec.
>
> I guess this info is stored somewhere, and the relevant gfns can be
> preserved across kernels, if we try really hard.
>
> But after looking further at the involved code paths it seems that the
> implemnted polling might be good enough to snatch the response. Was the
> mdelay(10) just an arbitrary decision?

I observed delays up to several seconds (!) before
CHANNELMSG_UNLOAD_RESPONSE is delivered.

> I interpret the comments in  vmbus_signal_eom such that the host may
> overwrite the response. Perhaps such thing may happen during the mdelay?

No, (at least in theory) the host is never supposed to overwrite
messages, it waits for the guest to clean the slot and do wrmsr.

-- 
  Vitaly

Re: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

On Thu, Dec 15, Vitaly Kuznetsov wrote:

> We actually need to read the reply and empty the message slot to make
> unload happen. And reading on a different CPU may not work, see:
> 
> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2016-December/097330.html

Does the following sentences mean the vmbus_signal_eom in
vmbus_wait_for_unload is a noop because the wrmsrl() is expected to
happen on the other cpu instead of the current cpu?

...
- When we read the message we need to clear the slot and signal the fact
  to the hypervisor. In case there are more messages to this CPU pending
  the hypervisor will deliver the next message. The signaling is done by
  writing to an MSR so this can only be done on the appropriate CPU.
...

Olaf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[Vitaly Kuznetsov]

Olaf Hering <olaf@aepfle.de> writes:

> On Thu, Dec 15, Vitaly Kuznetsov wrote:
>
>> We actually need to read the reply and empty the message slot to make
>> unload happen. And reading on a different CPU may not work, see:
>> 
>> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2016-December/097330.html
>
> Does the following sentences mean the vmbus_signal_eom in
> vmbus_wait_for_unload is a noop because the wrmsrl() is expected to
> happen on the other cpu instead of the current cpu?
>
> ...
> - When we read the message we need to clear the slot and signal the fact
>   to the hypervisor. In case there are more messages to this CPU pending
>   the hypervisor will deliver the next message. The signaling is done by
>   writing to an MSR so this can only be done on the appropriate CPU.
> ...
>

-> K. Y., but these words were written before I implemented
vmbus_wait_for_unload(), to me they just explain how we read messages.

vmbus_wait_for_unload() may be receiving a message (not necessarily the
CHANNELMSG_UNLOAD_RESPONSE, we may see some other message) on the same
CPU it runs and in this case wrmsrl() makes sense. In other cases it
does nothing (neither good nor bad).

-- 
  Vitaly

Re: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 579 bytes --]

On Thu, Dec 15, Vitaly Kuznetsov wrote:

> vmbus_wait_for_unload() may be receiving a message (not necessarily the
> CHANNELMSG_UNLOAD_RESPONSE, we may see some other message) on the same
> CPU it runs and in this case wrmsrl() makes sense. In other cases it
> does nothing (neither good nor bad).

If that other cpu has interrupts disabled it may not process a pending
msg (the response may be stuck in the host queue?), and the loop can not
kick the other cpus queue if a wrmsrl is just valid for the current cpu.
If thats true, the response will not arrive in the loop.

Olaf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[Vitaly Kuznetsov]

Olaf Hering <olaf@aepfle.de> writes:

> On Thu, Dec 15, Vitaly Kuznetsov wrote:
>
>> vmbus_wait_for_unload() may be receiving a message (not necessarily the
>> CHANNELMSG_UNLOAD_RESPONSE, we may see some other message) on the same
>> CPU it runs and in this case wrmsrl() makes sense. In other cases it
>> does nothing (neither good nor bad).
>
> If that other cpu has interrupts disabled it may not process a pending
> msg (the response may be stuck in the host queue?), and the loop can not
> kick the other cpus queue if a wrmsrl is just valid for the current cpu.
> If thats true, the response will not arrive in the loop.
>

In case interrupts get permanently disabled on the CPU which is supposed
to receive the CHANNELMSG_UNLOAD_RESPONSE message *and* there is some
other message pedning in the slot for that CPU we'll hang. We may try to
overcome this by sending NMIs but this is getting more and more
complicated...

I'd like to see a simple fix from Hyper-V host team: always deliver
CHANNELMSG_UNLOAD_RESPONSE reply to the cpu which sent CHANNELMSG_UNLOAD
request. This would allow us to remove all the craziness.

-- 
  Vitaly

RE: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[KY Srinivasan]

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Thursday, December 15, 2016 6:32 AM
> To: Olaf Hering <olaf@aepfle.de>
> Cc: KY Srinivasan <kys@microsoft.com>; gregkh@linuxfoundation.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org
> Subject: Re: move hyperv CHANNELMSG_UNLOAD from crashed kernel to
> kdump kernel
> 
> Olaf Hering <olaf@aepfle.de> writes:
> 
> > On Thu, Dec 15, Vitaly Kuznetsov wrote:
> >
> >> vmbus_wait_for_unload() may be receiving a message (not necessarily
> the
> >> CHANNELMSG_UNLOAD_RESPONSE, we may see some other message)
> on the same
> >> CPU it runs and in this case wrmsrl() makes sense. In other cases it
> >> does nothing (neither good nor bad).
> >
> > If that other cpu has interrupts disabled it may not process a pending
> > msg (the response may be stuck in the host queue?), and the loop can not
> > kick the other cpus queue if a wrmsrl is just valid for the current cpu.
> > If thats true, the response will not arrive in the loop.
> >
> 
> In case interrupts get permanently disabled on the CPU which is supposed
> to receive the CHANNELMSG_UNLOAD_RESPONSE message *and* there is
> some
> other message pedning in the slot for that CPU we'll hang. We may try to
> overcome this by sending NMIs but this is getting more and more
> complicated...
> 
> I'd like to see a simple fix from Hyper-V host team: always deliver
> CHANNELMSG_UNLOAD_RESPONSE reply to the cpu which sent
> CHANNELMSG_UNLOAD
> request. This would allow us to remove all the craziness.

Agreed; I will give this feedback to the Hyper-V guys.

K. Y
> 
> --
>   Vitaly

Re: move hyperv CHANNELMSG_UNLOAD from crashed kernel to kdump kernel

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 491 bytes --]

On Thu, Dec 15, Vitaly Kuznetsov wrote:
> -> K. Y., but these words were written before I implemented
> vmbus_wait_for_unload(), to me they just explain how we read messages.

Another question for KY:
In my testing, while busy-looping in vmbus_wait_for_unload, I see a few
"message_type==1, hdr->msgtype==2" in the hv_context.synic_message_page
of the cpu which will deliver CHANNELMSG_UNLOAD_RESPONSE.
These values are not listed in their enum lists. Any idea what these
values mean?

Olaf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]