Re: [RFC] flash-kernel hook to prepend to boot script
On Thu, Jun 05, 2014 at 08:43:02AM +0100, Ian Campbell wrote:
> On Wed, 2014-06-04 at 11:40 -0600, dann frazier wrote:
> > On Fri, May 30, 2014 at 09:50:37AM +0100, Ian Campbell wrote:
> > > On Wed, 2014-05-28 at 16:30 -0600, dann frazier wrote:
> > > > On Tue, May 27, 2014 at 10:35:18AM -0600, dann frazier wrote:
> > > > > On Mon, May 26, 2014 at 11:54:31AM +0100, Ian Campbell wrote:
> > > > > > On Wed, 2014-05-21 at 14:59 -0600, dann frazier wrote:
> > > > > > > hey,
> > > > > > > A couple of projects we're working on at work require some
> > > > > > > tweaking of u-boot settings. These requirements can be summed up by:
> > > > > > >
> > > > > > > A) Maintain the console= setting, and ideally all userargs (the
> > > > > > > cmdline args after "--") after install. This seems like standard
> > > > > > > Debian functionality that exists on other architectures, but is
> > > > > > > currently missing on flash-kernel platforms.
> > > > > > >
> > > > > > > B) The ability to let a package change settings in the u-boot
> > > > > > > environment. We have a case where a u-boot envvar setting
> > > > > > > needs to differ depending on whether or not certain software
> > > > > > > will be used (their u-boot has special code that reconfigures
> > > > > > > the hardware depending on the setting of this variable).
> > > > > > >
> > > > > > > The systems we're dealing with use a boot.scr script generated by
> > > > > > > flash-kernel. So, we figure we could solve both by letting packages
> > > > > > > drop in u-boot code snippets that will be prepended to the
> > > > > > > boot.scr. To do this, we propose a scheme similar to initramfs-tools
> > > > > > > where packages can drop snippets in a path under /usr/share (solving
> > > > > > > B), and users can add their own new setings or override the /usr/share
> > > > > > > versions by dropping snippets under /etc. With this scheme in place,
> > > > > > > flash-kernel-installer could be extended to drop in a file in /etc
> > > > > > > that does a 'setenv bootargs $userargs' to solve (A). Comments?
> > > > > >
> > > > > > I think snippets like this are a useful idea in general, but I wonder if
> > > > > > something like the command line isn't deserving of "higher billing",
> > > > > > e.g. via a setting in /etc/defaults/flash-kernel:COMMAND_LINE?
> > > > >
> > > > > It looks like Ubuntu is carrying a patch that does this today:
> > > > >
> > > > > http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/utopic/flash-kernel/utopic/revision/443.2.7
> > > > >
> > > > > There the the variable is called "UBOOT_DEFAULTS". I think
> > > > > "KERNEL_CMDLINE" would be a more obvious name, but it would also be
> > > > > nice to be reducing their diff.
> > > >
> > > > Looking at grub as an example, I think we'll want to make the cmdline
> > > > paramaters a debconf setting, giving the user the option to modify the
> > > > proposed cmdline in expert mode.
> > > >
> > > > 1) f-k-i would use user-params to generate a reasonable default set
> > > > cmdline args and set flash-kernel/linux_cmdline.
> > > > 2) f-k/configure would prompt the user (priority=high) for changes to
> > > > this default during configuration.
> > > > 3) f-k/postinst would generate /etc/default/flash-kernel, and
> > > > presumably using ucf to manage it from there on
> > > >
> > > > Sound reasonable?
> > >
> > > It does.
> > >
> > > I'm travelling right now but I took a brief look through the patches, I
> > > think you should go ahead and push them.
> >
> > My target platform is actually an arm64 system, which I can't easily
> > test it with Debian's d-i (yet). But I did manage to find an armhf
> > system to test on this week. There were a few issues with my changes
> > that I found/fixed (multiline support for ubootenv stubs,
> > brokendebconf-set-selections call, etc), but it seems to be working as
> > expected now. I went ahead and also added support for the armhf test
> > system I used (Calxeda Highbank) and my target platform (APM Mustang),
> > which also required turning on arm64 support.
> >
> > > Reading the diffs in my mail
> > > client suggested there was some mixing of hard and soft tabs, butthat's
> > > only a minor thing.
> >
> > Should be fixed in the merged version.
> >
> > > I didn't see any boot.scr using this stuff, I suppose that is to
> > > come?
> >
> > Yeah - added this for both new platforms I've introduced.
> >
> > > I'll probably make the sunxi/cubietruck stuff use it at some point.
> >
> > Cool. In the meantime, I'll plan to do an f-k upload tomorrow, in case
> > anyone wants to take some time to review the merged changes.
>
> Mostly looks good too me.
>
> The xgene script is missing the extra bootenv marker.
Oh, good catch - obviously I haven't tested a Debian install on that
system yet :) Fix pushed.
> Bootloader-Sets-Incorrect-Root is marked "required" in the README but is
> omitted from X-gene. I think since 850c8fc3 the behaviour when missing
> is now to treat it as "no" which is the sane default, so perhaps the fix
> is to make the README reflect that rather than fiddle with the Xgene db
> entry.
Either works for me.
> flash-kernel upgrade rather than on install via di is a worry. Bootargs
> gets overridden to the default from /etc/default/flash-kernel which will
> be missing root= etc. On my cubietruck after dpkg -i of 3.20 I 've ended
> up with:
> setenv bootargs 'quiet'
> in my boot.scr, which isn't going to boot I think. (actually, I as
> discovered below it probably would, although missing console= is a
> concern...)
>
> This could just mean that we need to be a bit careful when adding
> @@LINUX_KERNEL_CMDLINE@@ to existing bootscripts, since we've not
> released with CT support I think it would be safe to do in that case.
> Other than that I'm at a loss what to suggest, perhaps something probed
> from /proc/cmdline when first installing a version >= 3.20?
That sounds reasonable/correct to me - I don't want to make anyone
unbootable, even in sid->sid upgrades. But my debconf foo is weak, so
I'm not sure what the best way to do this is. Maybe we could add a
mapping of model strings and the corresponding f-k version at which
each model started including L_K_C support to the postinst. When
crossing that boundary (version test), we could preload any
non-duplicate /proc/cmdline options to flash-kernel/linux_cmdline.
Here's an untested patch to explain using code:
diff --git a/debian/flash-kernel.postinst b/debian/flash-kernel.postinst
index 7ef963a..13a74a6 100755
--- a/debian/flash-kernel.postinst
+++ b/debian/flash-kernel.postinst
@@ -2,6 +2,45 @@
set -e
+# Transition from using a static bootargs
+# to one provided by /etc/flash/kernel:LINUX_KERNEL_CMDLINE
+transition_cmdline() {
+ local configver="$1"
+ local transver=""
+
+ . /usr/share/flash-kernel/functions
+
+ get_machine
+
+ case "$machine" in
+ "Cubietech Cubietruck")
+ transver=1.20 ;;
+ *)
+ ;;
+ esac
+
+ if [ -z "$transver" ]; then
+ return
+ fi
+
+ if dpkg --compare-versions "$configver" ge "$transver"; then
+ return
+ fi
+
+ local def_params=""
+ local new_params=""
+ db_get flash-kernel/linux_cmdline && def_params="$RET"
+
+ new_params="$def_params"
+
+ for param in $(cat /proc/cmdline); do
+ if ! echo "$def_params" | tr ' ' '\n' | grep -q "^${param}$"; then
+ new_params="${new_params:+$new_params }${param}"
+ fi
+ done
+ db_set flash-kernel/linux_cmdline "$new_params"
+}
+
# cargo-culted from grub
merge_debconf_into_conf() {
local tmpfile; tmpfile="$1"
@@ -19,10 +58,13 @@ merge_debconf_into_conf() {
fi
}
+configured_ver="$2"
+
case "$1" in
configure)
. /usr/share/debconf/confmodule
+ transition_cmdline "$configured_ver"
tmp_default_fk="$(mktemp -t flash-kernel.XXXXXXXXXX)"
trap "rm -f ${tmp_default_fk}" EXIT
cp -p /usr/share/flash-kernel/default/flash-kernel \
> Then I edited /etc/default/flash-kernel by hand to
> LINUX_KERNEL_CMDLINE="quiet root=/dev/sda2
> console=ttyS0,115200n"
>
> But:
> # flash-kernel
> Installing sun7i-a20-cubietruck.dtb 3.15-rc7-armmp into /boot
> flash-kernel: installing version 3.15-rc7-armmp
> Generating boot script u-boot image... sed: -e expression #2, char 40: unknown option to `s'
>
> Because of the / in /dev/sda2 perhaps. It turns out root= can be omitted
> here because it's also done in
> initramfs-tools/hooks/flash_kernel_set_rootm which sets the default in
> case the command line is missing it. That's OK, but it would be nice if
> we could be arranging for / to be allowed here -- someone is eventually
> going to add root= to /etc/defaults and break their system...
I pushed a fix for that, thanks!
-dann
> With root= gone from there things work on my CT.
>
> Cheers,
> Ian.
>
>
Reply to: