[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <X7UJtlLqJ0ZljhXG@alley>
Date: Wed, 18 Nov 2020 12:47:02 +0100
From: Petr Mladek <pmladek@...e.com>
To: Matteo Croce <mcroce@...ux.microsoft.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Nathan Chancellor <natechancellor@...il.com>,
Arnd Bergmann <arnd@...db.de>,
Kees Cook <keescook@...omium.org>,
linux-kernel@...r.kernel.org, Guenter Roeck <linux@...ck-us.net>,
Pavel Tatashin <pasha.tatashin@...een.com>,
Mike Rapoport <rppt@...nel.org>,
Tyler Hicks <tyhicks@...ux.microsoft.com>,
Nick Desaulniers <ndesaulniers@...gle.com>,
clang-built-linux@...glegroups.com
Subject: Re: [PATCH] reboot: Fix variable assignments in type_store
On Fri 2020-11-13 22:28:18, Matteo Croce wrote:
> On Fri, Nov 13, 2020 at 9:06 PM Petr Mladek <pmladek@...e.com> wrote:
> >
> > On Fri 2020-11-13 03:58:49, Matteo Croce wrote:
> > > On Fri, Nov 13, 2020 at 3:46 AM Andrew Morton <akpm@...ux-foundation.org> wrote:
> > > >
> > > > On Fri, 13 Nov 2020 02:38:18 +0100 Matteo Croce <mcroce@...ux.microsoft.com> wrote:
> > > >
> > > > > At this point, since 'pci' enables BOOT_CF9_FORCE type and
> > > > > BOOT_CF9_SAFE is not user selectable, should I simply leave only
> > > > > 'pci'?
> > > > >
> > > > > This way, we'll have the same set of options for both sysfs and kernel cmdline.
> > > >
> > > > Well, you're the reboot expert ;)
> > > >
> > >
> > > So honored! :)
> > >
> > > > But my $0.02 is yes, let's keep the command-line and sysfs interfaces
> > > > in sync and cover it all in documentation. It would of course be
> > > > problematic to change the existing reboot= interface.
> > > >
> > > > I assume that means doing this?
> > > >
> > > > - #define BOOT_CF9_FORCE_STR "cf9_force"
> > > > + #define BOOT_CF9_FORCE_STR "pci"
> > > > - #define BOOT_CF9_SAFE_STR "cf9_safe"
> > >
> > > Either BOOT_PCI_STR or BOOT_CF9_FORCE_STR, I have no strong preference.
> > >
> > > The syntax is 'pci' while the enum BOOT_CF9_FORCE, so we can't please both.
> >
> > The question is whether we should modify/allow to set these values at
> > all.
> >
> > Anyway, we must prevent them on non-x86 architectures because
> > the reboot behavior would be undefined there. They could probably
> > make a mess even on many x86-architectures.
> >
>
> That's right, but the same can be obtained by passing 'reboot=pci' on
> non x86 machines: the cmdline parsing is generic and will set
> reboot_type on all arches.
Fair enough. Ah, I mixed reboot_type and reboot_mode and looked
into wrong part of reboot_setup().
> I looked at the reboot_type usage, there isn't any reference outside
> arch/x86. In fact, the parameter is just ignored:
>
> # uname -m
> aarch64
> # cat /proc/cmdline
> console=ttyS0,115200n8 reboot=pci
> # reboot -ff
> Rebooting.
> [ 43.893833] reboot: Restarting system
Good to know. Thanks for checking.
> The same applies for reboot_force, the only flags available on
> different architectures are reboot_mode and reboot_cpu.
> We could hide some handlers for some architectures. We save some
> space, and avoid letting the user set flags which do nothing.
I am fine with the current patchset after all. We could always make
it more safe when people hit it in the real life. All these
worries were because I thought that this interface allowed
to set values that were not possible before.
> > Anyway, we should get input from some x86-experts about the BOOT_CF9
> > values.
>
> Sure, x86@...nel.org ?
Yes but I do not resist on it any longer. Just if you were going to send
another version just by chance then it would be nice to CC x86.
Best Regards,
Petr
Powered by blists - more mailing lists