Boot parameter parsing

[Warner Losh]

On Jun 11, 2012, at 1:11 PM, Ian Lepore wrote:

> On Thu, 2012-06-07 at 00:26 -0600, Warner Losh wrote:
>> Greetings,
>> 
>> Please find enclosed a small patch to the arm port.
>> 
>> For too long parsing boot args in arm land has suffered from cut and paste code.  This is inefficient and inflexible.  This patch does something to fix that.  First, it modifies all the arm ports to call parse_boot_param passing in the boot parameters from initarm as the first thing in each platform's implementation of that function.  This is done really super early, importantly before we start using memory outside of the loaded kernel's text, data, and bss areas.  I'd thought of moving this even earlier, into __start just before the call to initarm, but wasn't completely sure was quite right (it would be more code deleted, however, if I do that: please comment).  The down side is that initarm was the only function we called in __start apart from mundane things like memcpy and that would change that, but that's kinda a weak argument I think.
>> 
>> I've created a weak alias to tying this function to fake_preload_metadata.  All but one of the ports do this now, and this moves them to a common standard that could be more easily changed.
>> 
>> For most ports, it replaces the call to fake_preload_metadata.  As such, I've modified the signature of fake_preload_metadata to be the same as parse_boot_param and made it a weak alias.  So, if you don't define one, you'll get the current behavior.
>> 
>> In a future patch, I'll likely be moving the mv platform's code into this routine (I'll create a default_parse_boot_param and create a weak alias pointing to that instead).  I'll need to modify the mv port to then get the dtb blob via the metadata, or possibly create a new global for it so that other platforms might make use of that also.
>> 
>> In a patch after that, I may add a kernel option to enable creation of FreeBSD /boot/loader metadata from Linux's standard boot stuff.  This will allow platforms to get more data from the Linux boot loader without going through the intermediate /boot/loader.  But it should preserve a unified interface by having it behave just like /boot/loader, but without anything setup by its more advanced features like kernel environment variables or loadable modules.
>> 
>> If I've done things right by this point, then any ARM port can take advantage of these new features, not just the target I'm aiming at.  In addition, anybody can use their own boot loader, if they so choose, and be able to write custom code that parses the args from it in whatever appropriate way might arise for their board.  I know of at least one FreeBSD/arm user that has a heavily hacked boot2 boot loader that passes things into the kernel in a non-standard way.  This will accommodate them, and others like them, while still providing the project with useful functionality.
>> 
>> Comments?
>> 
>> Warner
>> 
>> P.S.  You can also find this at http://people.freebsd.org/~imp/parse_boot_param.diff in case the mailing list eats this for lunch.
>> 
>> 
> 
> I like everything about this except the names.  I've always thought
> fake_preload_metadata() was a bad name.  Now having it be the default
> implementation of parse_boot_param() seems even-more-bad.  There's
> nothing about the name (or the new comment block) that makes it clear
> that if you supply a parse_boot_param() routine, it must do everything
> that fake_preload_metadata() does.

The next commit in my tree actually creates a new default_parse_boot_param() that will check to see if the FreeBSD data is present and parse the usual suspects and fall back to the fake_preload_metadata() routine that's used everywhere.  Right now only mv uses it, and we really should have everybody use it.

After that, there is an option to parse the Linux boot loader stuff into the metadata and then have the normal metadata code do the right thing.  this would be an optional thing, but the good news here is that we can tell the difference between a FreeBSD /boot/loader and a Linux/Uboot load automatically.

> The attached patch changes just the arm/machdep.c and machdep.h part of
> your changes.  The fake_preload_metadata() is renamed to
> add_kernel_preload_metadata() and it becomes a helper routine that
> board-specific implementations of parse_boot_param() can use to generate
> the preload metadata when it doesn't come in with the boot params data.
> It also adds a new routine default_parse_boot_param() which is just a
> thin wrapper around add_kernel_preload_metadata().

Yea, I have that in a new patch in my tree.  I don't really want to change fake_preload_metadata() just yet, since there are out of tree ports that use it and I don't want to cut that cord yet.

> This patch applies to -current on top of the recent changes from Andrew
> Turner, and instead of (not in addition to) your patch.


Thanks.

Warner