Re: [PATCH V3 10/11] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers
On 11/11/2022 20:16, Wei Liu wrote: > On Fri, Nov 11, 2022 at 10:47:17PM +, Wei Liu wrote: >> On Thu, Nov 10, 2022 at 06:32:54PM -0300, Guilherme G. Piccoli wrote: >>> [Trimming long CC list] >>> >>> Hi Wei / Michael, just out of curiosity, did this (and patch 9) ended-up >>> being queued in hyperv-next? >>> >> >> No. They are not queued. >> >> I didn't notice Michael's email and thought they would be picked up by >> someone else while dealing with the whole series. >> >> I will pick them up later. > > They are now queued to hyperv-next. Thanks. Thanks a lot Wei and Michael, much appreciated =) ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH V3 10/11] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers
On Fri, Nov 11, 2022 at 10:47:17PM +, Wei Liu wrote: > On Thu, Nov 10, 2022 at 06:32:54PM -0300, Guilherme G. Piccoli wrote: > > [Trimming long CC list] > > > > Hi Wei / Michael, just out of curiosity, did this (and patch 9) ended-up > > being queued in hyperv-next? > > > > No. They are not queued. > > I didn't notice Michael's email and thought they would be picked up by > someone else while dealing with the whole series. > > I will pick them up later. They are now queued to hyperv-next. Thanks. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH V3 10/11] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers
On Thu, Nov 10, 2022 at 06:32:54PM -0300, Guilherme G. Piccoli wrote: > [Trimming long CC list] > > Hi Wei / Michael, just out of curiosity, did this (and patch 9) ended-up > being queued in hyperv-next? > No. They are not queued. I didn't notice Michael's email and thought they would be picked up by someone else while dealing with the whole series. I will pick them up later. Thanks, Wei. > Thanks in advance, > > > Guilherme > > On 17/10/2022 12:26, Michael Kelley (LINUX) wrote: > > From: Guilherme G. Piccoli Sent: Tuesday, October 4, > > 2022 10:20 AM > >> > >> On 04/10/2022 13:24, Michael Kelley (LINUX) wrote: > >>> [...] > >>> > >>> Tested this patch in combination with Patch 9 in this series. Verified > >>> that both the panic and die paths work correctly with notification to > >>> Hyper-V via hyperv_report_panic() or via hv_kmsg_dump(). Hyper-V > >>> framebuffer is updated as expected, though I did not reproduce > >>> a case where the ring buffer lock is held. vmbus_initiate_unload() runs > >>> as expected. > >>> > >>> Tested-by: Michael Kelley > >>> > >> > >> Thanks a lot for the tests/review Michael! > >> > >> Do you think Hyper-V folks could add both patches in hv tree? If you > >> prefer, I can re-send them individually. > >> > > > > Wei Liu: Could you pick up Patch 9 and Patch 10 from this series in the > > hyperv-next tree? > > > > Michael > > ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH V3 10/11] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers
[Trimming long CC list] Hi Wei / Michael, just out of curiosity, did this (and patch 9) ended-up being queued in hyperv-next? Thanks in advance, Guilherme On 17/10/2022 12:26, Michael Kelley (LINUX) wrote: > From: Guilherme G. Piccoli Sent: Tuesday, October 4, > 2022 10:20 AM >> >> On 04/10/2022 13:24, Michael Kelley (LINUX) wrote: >>> [...] >>> >>> Tested this patch in combination with Patch 9 in this series. Verified >>> that both the panic and die paths work correctly with notification to >>> Hyper-V via hyperv_report_panic() or via hv_kmsg_dump(). Hyper-V >>> framebuffer is updated as expected, though I did not reproduce >>> a case where the ring buffer lock is held. vmbus_initiate_unload() runs >>> as expected. >>> >>> Tested-by: Michael Kelley >>> >> >> Thanks a lot for the tests/review Michael! >> >> Do you think Hyper-V folks could add both patches in hv tree? If you >> prefer, I can re-send them individually. >> > > Wei Liu: Could you pick up Patch 9 and Patch 10 from this series in the > hyperv-next tree? > > Michael > ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
RE: [PATCH V3 10/11] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers
From: Guilherme G. Piccoli Sent: Tuesday, October 4, 2022 10:20 AM > > On 04/10/2022 13:24, Michael Kelley (LINUX) wrote: > > [...] > > > > Tested this patch in combination with Patch 9 in this series. Verified > > that both the panic and die paths work correctly with notification to > > Hyper-V via hyperv_report_panic() or via hv_kmsg_dump(). Hyper-V > > framebuffer is updated as expected, though I did not reproduce > > a case where the ring buffer lock is held. vmbus_initiate_unload() runs > > as expected. > > > > Tested-by: Michael Kelley > > > > Thanks a lot for the tests/review Michael! > > Do you think Hyper-V folks could add both patches in hv tree? If you > prefer, I can re-send them individually. > Wei Liu: Could you pick up Patch 9 and Patch 10 from this series in the hyperv-next tree? Michael ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH V3 10/11] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers
On 04/10/2022 13:24, Michael Kelley (LINUX) wrote: > [...] > > Tested this patch in combination with Patch 9 in this series. Verified > that both the panic and die paths work correctly with notification to > Hyper-V via hyperv_report_panic() or via hv_kmsg_dump(). Hyper-V > framebuffer is updated as expected, though I did not reproduce > a case where the ring buffer lock is held. vmbus_initiate_unload() runs > as expected. > > Tested-by: Michael Kelley > Thanks a lot for the tests/review Michael! Do you think Hyper-V folks could add both patches in hv tree? If you prefer, I can re-send them individually. Cheers, Guilherme ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
RE: [PATCH V3 10/11] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers
From: Guilherme G. Piccoli Sent: Friday, August 19, 2022
3:18 PM
>
> Currently Hyper-V guests are among the most relevant users of the panic
> infrastructure, like panic notifiers, kmsg dumpers, etc. The reasons rely
> both in cleaning-up procedures (closing hypervisor <-> guest connection,
> disabling some paravirtualized timer) as well as to data collection
> (sending panic information to the hypervisor) and framebuffer management.
>
> The thing is: some notifiers are related to others, ordering matters, some
> functionalities are duplicated and there are lots of conditionals behind
> sending panic information to the hypervisor. As part of an effort to
> clean-up the panic notifiers mechanism and better document things, we
> hereby address some of the issues/complexities of Hyper-V panic handling
> through the following changes:
>
> (a) We have die and panic notifiers on vmbus_drv.c and both have goals of
> sending panic information to the hypervisor, though the panic notifier is
> also responsible for a cleaning-up procedure.
>
> This commit clears the code by splitting the panic notifier in two, one
> for closing the vmbus connection whereas the other is only for sending
> panic info to hypervisor. With that, it was possible to merge the die and
> panic notifiers in a single/well-documented function, and clear some
> conditional complexities on sending such information to the hypervisor.
>
> (b) There is a Hyper-V framebuffer panic notifier, which relies in doing
> a vmbus operation that demands a valid connection. So, we must order this
> notifier with the panic notifier from vmbus_drv.c, to guarantee that the
> framebuffer code executes before the vmbus connection is unloaded.
>
> Also, this commit removes a useless header.
>
> Although there is code rework and re-ordering, we expect that this change
> has no functional regressions but instead optimize the path and increase
> panic reliability on Hyper-V. This was tested on Hyper-V with success.
>
> Cc: Andrea Parri (Microsoft)
> Cc: Dexuan Cui
> Cc: Haiyang Zhang
> Cc: "K. Y. Srinivasan"
> Cc: Petr Mladek
> Cc: Stephen Hemminger
> Cc: Tianyu Lan
> Cc: Wei Liu
> Reviewed-by: Michael Kelley
> Tested-by: Fabio A M Martins
> Signed-off-by: Guilherme G. Piccoli
>
> ---
>
> V3:
> - Added Michael's review tag - thanks!
>
> V2:
> - Unfortunately we cannot rely in the crash shutdown (custom) handler
> to perform the vmbus unload - arm64 architecture doesn't have this
> "feature" [0]. So, in V2 we kept the notifier behavior and always
> unload the vmbus connection, no matter what - thanks Michael for
> pointing that;
>
> - Removed the Fixes tags as per Michael suggestion;
>
> - As per Petr suggestion, we abandoned the idea of distinguish among
> notifiers using an id - so, in V2 we rely in the old and good address
> comparison for that. Thanks Petr for the enriching discussion!
>
> [0]
> https://lore.kernel.org/lkml/427a8277-49f0-4317-d6c3-4a15d7070...@igalia.com/
>
>
> drivers/hv/vmbus_drv.c | 109 +++-
> drivers/video/fbdev/hyperv_fb.c | 8 +++
> 2 files changed, 74 insertions(+), 43 deletions(-)
>
Tested this patch in combination with Patch 9 in this series. Verified
that both the panic and die paths work correctly with notification to
Hyper-V via hyperv_report_panic() or via hv_kmsg_dump(). Hyper-V
framebuffer is updated as expected, though I did not reproduce
a case where the ring buffer lock is held. vmbus_initiate_unload() runs
as expected.
Tested-by: Michael Kelley
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 23c680d1a0f5..0a77e2bb0b70 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -25,7 +25,6 @@
> #include
>
> #include
> -#include
> #include
> #include
> #include
> @@ -69,53 +68,74 @@ static int hyperv_report_reg(void)
> return !sysctl_record_panic_msg || !hv_panic_page;
> }
>
> -static int hyperv_panic_event(struct notifier_block *nb, unsigned long val,
> +/*
> + * The panic notifier below is responsible solely for unloading the
> + * vmbus connection, which is necessary in a panic event.
> + *
> + * Notice an intrincate relation of this notifier with Hyper-V
> + * framebuffer panic notifier exists - we need vmbus connection alive
> + * there in order to succeed, so we need to order both with each other
> + * [see hvfb_on_panic()] - this is done using notifiers' priorities.
> + */
> +static int hv_panic_vmbus_unload(struct notifier_block *nb, unsigned long
> val,
> void *args)
> +{
> + vmbus_initiate_unload(true);
> + return NOTIFY_DONE;
> +}
> +static struct notifier_block hyperv_panic_vmbus_unload_block = {
> + .notifier_call = hv_panic_vmbus_unload,
> + .priority = INT_MIN + 1, /* almost the latest one to execute */
> +};
> +
> +static int hv_die_panic_notify_crash(struct notifier_block *self,
> + unsigned long
[PATCH V3 10/11] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers
Currently Hyper-V guests are among the most relevant users of the panic
infrastructure, like panic notifiers, kmsg dumpers, etc. The reasons rely
both in cleaning-up procedures (closing hypervisor <-> guest connection,
disabling some paravirtualized timer) as well as to data collection
(sending panic information to the hypervisor) and framebuffer management.
The thing is: some notifiers are related to others, ordering matters, some
functionalities are duplicated and there are lots of conditionals behind
sending panic information to the hypervisor. As part of an effort to
clean-up the panic notifiers mechanism and better document things, we
hereby address some of the issues/complexities of Hyper-V panic handling
through the following changes:
(a) We have die and panic notifiers on vmbus_drv.c and both have goals of
sending panic information to the hypervisor, though the panic notifier is
also responsible for a cleaning-up procedure.
This commit clears the code by splitting the panic notifier in two, one
for closing the vmbus connection whereas the other is only for sending
panic info to hypervisor. With that, it was possible to merge the die and
panic notifiers in a single/well-documented function, and clear some
conditional complexities on sending such information to the hypervisor.
(b) There is a Hyper-V framebuffer panic notifier, which relies in doing
a vmbus operation that demands a valid connection. So, we must order this
notifier with the panic notifier from vmbus_drv.c, to guarantee that the
framebuffer code executes before the vmbus connection is unloaded.
Also, this commit removes a useless header.
Although there is code rework and re-ordering, we expect that this change
has no functional regressions but instead optimize the path and increase
panic reliability on Hyper-V. This was tested on Hyper-V with success.
Cc: Andrea Parri (Microsoft)
Cc: Dexuan Cui
Cc: Haiyang Zhang
Cc: "K. Y. Srinivasan"
Cc: Petr Mladek
Cc: Stephen Hemminger
Cc: Tianyu Lan
Cc: Wei Liu
Reviewed-by: Michael Kelley
Tested-by: Fabio A M Martins
Signed-off-by: Guilherme G. Piccoli
---
V3:
- Added Michael's review tag - thanks!
V2:
- Unfortunately we cannot rely in the crash shutdown (custom) handler
to perform the vmbus unload - arm64 architecture doesn't have this
"feature" [0]. So, in V2 we kept the notifier behavior and always
unload the vmbus connection, no matter what - thanks Michael for
pointing that;
- Removed the Fixes tags as per Michael suggestion;
- As per Petr suggestion, we abandoned the idea of distinguish among
notifiers using an id - so, in V2 we rely in the old and good address
comparison for that. Thanks Petr for the enriching discussion!
[0]
https://lore.kernel.org/lkml/427a8277-49f0-4317-d6c3-4a15d7070...@igalia.com/
drivers/hv/vmbus_drv.c | 109 +++-
drivers/video/fbdev/hyperv_fb.c | 8 +++
2 files changed, 74 insertions(+), 43 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 23c680d1a0f5..0a77e2bb0b70 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -25,7 +25,6 @@
#include
#include
-#include
#include
#include
#include
@@ -69,53 +68,74 @@ static int hyperv_report_reg(void)
return !sysctl_record_panic_msg || !hv_panic_page;
}
-static int hyperv_panic_event(struct notifier_block *nb, unsigned long val,
+/*
+ * The panic notifier below is responsible solely for unloading the
+ * vmbus connection, which is necessary in a panic event.
+ *
+ * Notice an intrincate relation of this notifier with Hyper-V
+ * framebuffer panic notifier exists - we need vmbus connection alive
+ * there in order to succeed, so we need to order both with each other
+ * [see hvfb_on_panic()] - this is done using notifiers' priorities.
+ */
+static int hv_panic_vmbus_unload(struct notifier_block *nb, unsigned long val,
void *args)
+{
+ vmbus_initiate_unload(true);
+ return NOTIFY_DONE;
+}
+static struct notifier_block hyperv_panic_vmbus_unload_block = {
+ .notifier_call = hv_panic_vmbus_unload,
+ .priority = INT_MIN + 1, /* almost the latest one to execute */
+};
+
+static int hv_die_panic_notify_crash(struct notifier_block *self,
+unsigned long val, void *args);
+
+static struct notifier_block hyperv_die_report_block = {
+ .notifier_call = hv_die_panic_notify_crash,
+};
+static struct notifier_block hyperv_panic_report_block = {
+ .notifier_call = hv_die_panic_notify_crash,
+};
+
+/*
+ * The following callback works both as die and panic notifier; its
+ * goal is to provide panic information to the hypervisor unless the
+ * kmsg dumper is used [see hv_kmsg_dump()], which provides more
+ * information but isn't always available.
+ *
+ * Notice that both the panic/die report notifiers are registered only
+ * if we have the capability HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE set.
+ */
+static
