Re: [PATCH] bcb: Android bootloader control block driver

[NeilBrown <neilb@suse.de>]

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

On Fri, 29 Jun 2012 21:37:36 -0700 Colin Cross <ccross@android.com> wrote:

> What's the point of the existing syscall option if it doesn't work on
> all platforms, or at least all platforms that want to support it?  It
> doesn't make sense to me to use REBOOT2 on some SoCs because they
> happen to use something that userspace cannot access, but use direct
> access from userspace and a different reboot syscall option on other
> SoCs.
> 
> >> <snip>
> >>
> >> >>
> >> >> diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig
> >> >> index 9a99238..c30fd20 100644
> >> >> --- a/drivers/staging/android/Kconfig
> >> >> +++ b/drivers/staging/android/Kconfig
> >> >> @@ -78,6 +78,17 @@ config ANDROID_INTF_ALARM_DEV
> >> >>         elapsed realtime, and a non-wakeup alarm on the monotonic clock.
> >> >>         Also exports the alarm interface to user-space.
> >> >>
> >> >> +config BOOTLOADER_CONTROL
> >> >> +     tristate "Bootloader Control Block module"
> >> >> +     default n
> >> >> +     help
> >> >> +       This driver installs a reboot hook, such that if reboot() is invoked
> >> >> +       with a string argument NNN, "bootonce-NNN" is copied to the command
> >> >> +       field in the Bootloader Control Block on the /misc partition, to be
> >> >> +       read by the bootloader. If the string matches one of the boot labels
> >> >> +       defined in its configuration, it will boot once into that label. The
> >> >> +       device and partition number are specified on the kernel command line.
> >> >> +
> >> >>  endif # if ANDROID
> >> >>
> >> >>  endmenu
> >>
> >> Most of this driver is not unique to Android.
> >
> > Do any other systems use it?
> 
> None that I'm aware of, but REBOOT2 existed long before Android, so I
> assume something must have used it.

Dangerous that - making assumptions.

I've just spent a while hunting though the code and the history.
The LINUX_REBOOT_CMD_RESTART2 option to sys_reboot - which is the only one
that uses the the 'arg' option - was added in 2.1.30.  This was the same time
that reboot notifiers were added and there seem to be steps towards a more
generic "machine_restart" call.
No code actually *used* the arg.

Looking through current code is rather time consuming as you have to follow
several levels of indirection to find code that might actually use the arg,
but I spend a while looking, trying to cover samples for all archs and driver
classes (there are lots of watchdogs -  I didn't check them all).
I only found *1* instance of code which actually used the arg.
This is arch/alpha/kernel/process.c which tests if the arg is NULL, and
selects between a "cold bootstrap" and a "warm bootstrap".

I think we would be well served by a patch that just removes it.  Or at
least, that ignores the value of the arg and just passes NULL or (void*)1.
And definitely don't pass it to the reboot notifiers - no code uses it there.

I can certainly see value in having a standard interface to say "the next
reboot should boot 'foo' rather than the default".  I don't think there is
any real need for the kernel to provide that interface.

I would suggest that you create
    /sbin/next-boot-image
(or some better name), which gets a different program installed depending on
what boot loader is in use.  Then your libcutils can just

  system("/sbin/next-boot-image foo")
  reboot(LINUX_REBOOT_CMD_RESTART);

and work on all platforms.
(Or use a loadable module, or a binder RPC to some service, or dbus or smoke
signals or whatever).

NeilBrown


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