[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 19/30] panic: Add the panic hypervisor notifier list
- To: "Guilherme G. Piccoli" <gpiccoli@xxxxxxxxxx>
- From: Evan Green <evgreen@xxxxxxxxxxxx>
- Date: Mon, 16 May 2022 09:02:10 -0700
- Cc: Petr Mladek <pmladek@xxxxxxxx>, David Gow <davidgow@xxxxxxxxxx>, Julius Werner <jwerner@xxxxxxxxxxxx>, Scott Branden <scott.branden@xxxxxxxxxxxx>, bcm-kernel-feedback-list@xxxxxxxxxxxx, Sebastian Reichel <sre@xxxxxxxxxx>, Linux PM <linux-pm@xxxxxxxxxxxxxxx>, Florian Fainelli <f.fainelli@xxxxxxxxx>, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>, bhe@xxxxxxxxxx, kexec@xxxxxxxxxxxxxxxxxxx, LKML <linux-kernel@xxxxxxxxxxxxxxx>, linuxppc-dev@xxxxxxxxxxxxxxxx, linux-alpha@xxxxxxxxxxxxxxx, linux-arm Mailing List <linux-arm-kernel@xxxxxxxxxxxxxxxxxxx>, linux-edac@xxxxxxxxxxxxxxx, linux-hyperv@xxxxxxxxxxxxxxx, linux-leds@xxxxxxxxxxxxxxx, linux-mips@xxxxxxxxxxxxxxx, linux-parisc@xxxxxxxxxxxxxxx, linux-remoteproc@xxxxxxxxxxxxxxx, linux-s390@xxxxxxxxxxxxxxx, linux-tegra@xxxxxxxxxxxxxxx, linux-um@xxxxxxxxxxxxxxxxxxx, linux-xtensa@xxxxxxxxxxxxxxxx, netdev@xxxxxxxxxxxxxxx, openipmi-developer@xxxxxxxxxxxxxxxxxxxxx, rcu@xxxxxxxxxxxxxxx, sparclinux@xxxxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx, x86@xxxxxxxxxx, kernel-dev@xxxxxxxxxx, kernel@xxxxxxxxxxxx, halves@xxxxxxxxxxxxx, fabiomirmar@xxxxxxxxx, alejandro.j.jimenez@xxxxxxxxxx, Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>, Arnd Bergmann <arnd@xxxxxxxx>, Borislav Petkov <bp@xxxxxxxxx>, Jonathan Corbet <corbet@xxxxxxx>, d.hatayama@xxxxxxxxxxxxxx, dave.hansen@xxxxxxxxxxxxxxx, dyoung@xxxxxxxxxx, feng.tang@xxxxxxxxx, Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>, mikelley@xxxxxxxxxxxxx, hidehiro.kawai.ez@xxxxxxxxxxx, jgross@xxxxxxxx, john.ogness@xxxxxxxxxxxxx, Kees Cook <keescook@xxxxxxxxxxxx>, luto@xxxxxxxxxx, mhiramat@xxxxxxxxxx, mingo@xxxxxxxxxx, paulmck@xxxxxxxxxx, peterz@xxxxxxxxxxxxx, rostedt@xxxxxxxxxxx, senozhatsky@xxxxxxxxxxxx, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>, Thomas Gleixner <tglx@xxxxxxxxxxxxx>, vgoyal@xxxxxxxxxx, vkuznets@xxxxxxxxxx, Will Deacon <will@xxxxxxxxxx>, Alexander Gordeev <agordeev@xxxxxxxxxxxxx>, Andrea Parri <parri.andrea@xxxxxxxxx>, Ard Biesheuvel <ardb@xxxxxxxxxx>, Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>, Brian Norris <computersforpeace@xxxxxxxxx>, Christian Borntraeger <borntraeger@xxxxxxxxxxxxx>, Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>, "David S. Miller" <davem@xxxxxxxxxxxxx>, Dexuan Cui <decui@xxxxxxxxxxxxx>, Doug Berger <opendmb@xxxxxxxxx>, Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>, Hari Bathini <hbathini@xxxxxxxxxxxxx>, Heiko Carstens <hca@xxxxxxxxxxxxx>, Justin Chen <justinpopo6@xxxxxxxxx>, "K. Y. Srinivasan" <kys@xxxxxxxxxxxxx>, Lee Jones <lee.jones@xxxxxxxxxx>, Markus Mayer <mmayer@xxxxxxxxxxxx>, Michael Ellerman <mpe@xxxxxxxxxxxxxx>, Mihai Carabas <mihai.carabas@xxxxxxxxxx>, Nicholas Piggin <npiggin@xxxxxxxxx>, Paul Mackerras <paulus@xxxxxxxxx>, Pavel Machek <pavel@xxxxxx>, Shile Zhang <shile.zhang@xxxxxxxxxxxxxxxxx>, Stephen Hemminger <sthemmin@xxxxxxxxxxxxx>, Sven Schnelle <svens@xxxxxxxxxxxxx>, Thomas Bogendoerfer <tsbogend@xxxxxxxxxxxxxxxx>, Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx>, Vasily Gorbik <gor@xxxxxxxxxxxxx>, Wang ShaoBo <bobo.shaobowang@xxxxxxxxxx>, Wei Liu <wei.liu@xxxxxxxxxx>, zhenwei pi <pizhenwei@xxxxxxxxxxxxx>, Stephen Boyd <swboyd@xxxxxxxxxxxx>
- Delivery-date: Mon, 16 May 2022 16:11:40 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Mon, May 16, 2022 at 8:07 AM Guilherme G. Piccoli
<gpiccoli@xxxxxxxxxx> wrote:
>
> Thanks for the review!
>
> I agree with the blinking stuff, I can rework and add all LED/blinking
> stuff into the loop list, it does make sense. I'll comment a bit in the
> others below...
>
> On 16/05/2022 11:01, Petr Mladek wrote:
> > [...]
> >> --- a/arch/mips/sgi-ip22/ip22-reset.c
> >> +++ b/arch/mips/sgi-ip22/ip22-reset.c
> >> @@ -195,7 +195,7 @@ static int __init reboot_setup(void)
> >> }
> >>
> >> timer_setup(&blink_timer, blink_timeout, 0);
> >> - atomic_notifier_chain_register(&panic_notifier_list, &panic_block);
> >> + atomic_notifier_chain_register(&panic_hypervisor_list, &panic_block);
> >
> > This notifier enables blinking. It is not much safe. It calls
> > mod_timer() that takes a lock internally.
> >
> > This kind of functionality should go into the last list called
> > before panic() enters the infinite loop. IMHO, all the blinking
> > stuff should go there.
> > [...]
> >> --- a/arch/mips/sgi-ip32/ip32-reset.c
> >> +++ b/arch/mips/sgi-ip32/ip32-reset.c
> >> @@ -145,7 +144,7 @@ static __init int ip32_reboot_setup(void)
> >> pm_power_off = ip32_machine_halt;
> >>
> >> timer_setup(&blink_timer, blink_timeout, 0);
> >> - atomic_notifier_chain_register(&panic_notifier_list, &panic_block);
> >> + atomic_notifier_chain_register(&panic_hypervisor_list, &panic_block);
> >
> > Same here. Should be done only before the "loop".
> > [...]
>
> Ack.
>
>
> >> --- a/drivers/firmware/google/gsmi.c
> >> +++ b/drivers/firmware/google/gsmi.c
> >> @@ -1034,7 +1034,7 @@ static __init int gsmi_init(void)
> >>
> >> register_reboot_notifier(&gsmi_reboot_notifier);
> >> register_die_notifier(&gsmi_die_notifier);
> >> - atomic_notifier_chain_register(&panic_notifier_list,
> >> + atomic_notifier_chain_register(&panic_hypervisor_list,
> >> &gsmi_panic_notifier);
> >
> > I am not sure about this one. It looks like some logging or
> > pre_reboot stuff.
> >
>
> Disagree here. I'm looping Google maintainers, so they can comment.
> (CCed Evan, David, Julius)
>
> This notifier is clearly a hypervisor notification mechanism. I've fixed
> a locking stuff there (in previous patch), I feel it's low-risk but even
> if it's mid-risk, the class of such callback remains a perfect fit with
> the hypervisor list IMHO.
This logs a panic to our "eventlog", a tiny logging area in SPI flash
for critical and power-related events. In some cases this ends up
being the only clue we get in a Chromebook feedback report that a
panic occurred, so from my perspective moving it to the front of the
line seems like a good idea.
-Evan
|