Created attachment 179652 [details] cavium octeon 'bootoctlinux' argument support patch This patch adds support to set a few bootflags and kernel environment variables from the cavium 'bootoctlinux' uboot loader. With this patch I can do many of the loader things like booting to single user mode and setting an alternate root device. It's not 100% compatible with all loader features because many of them don't make sense on this platform. This was developed and tested on an EdgeRouter Lite v3. For reference, here is the u-boot version I have: U-Boot 1.1.1 (UBNT Build ID: 4670715-gbd7e2d7) (Build time: May 27 2014 - 11:16:22) BIST check passed. UBNT_E100 r1:2, r2:18, f:4/71, serial #: 802AA88F7448 MPR 13-00318-18 Core clock: 500 MHz, DDR clock: 266 MHz (532 Mhz data rate) DRAM: 512 MB Clearing DRAM....... done Flash: 4 MB Net: octeth0, octeth1, octeth2 This is the uboot environment I've been using: baudrate=115200 download_baudrate=115200 nuke_env=protect off $(env_addr) +$(env_size);erase $(env_addr) +$(env_size) autoload=n ethact=octeth0 bootdelay=10 kernel=kernel filesize=8EF8C8 fileaddr=0x9F00000 filename=kernel rootdev=ufs:/dev/da0s2a bootcmd=usb reset;fatload usb 0 ${loadaddr} ${kernel}; bootoctlinux ${loadaddr} coremask=0x3 vfs.root.mountfrom=${rootdev} loadaddr=0x9f00000 numcores=2 stdin=serial stdout=serial stderr=serial env_addr=0x1fbfe000 env_size=0x2000 flash_base_addr=0x1f800000 flash_size=0x400000 uboot_flash_addr=0x1f880000 uboot_flash_size=0x70000 flash_unused_addr=0x1f8f0000 flash_unused_size=0x310000 bootloader_flash_update=bootloaderupdate
There's parse-bootarg in mtk_machdep.c and jz4780_machdep.c that parses fill 'boothowto' switch set. Maybe one we can get this consolidated somehow and make use in your code too, if bootolclinux can accept -<blah>-like arguments directly?
'bootoctlinux' passes any supplied arguments verbatim into the machdep code. Specifying boothowto flags via short args as suggested will work. When I get some time in a few days I'll rework the patch to behave more like mtk_machdep.c. If someone suggests where a generic boothowto flag parser function should go besides cut & pasted into the octeon_machdep.c source I can look into it. Although, I was trying to be as non-intrusive as possible with this patch.
Created attachment 179816 [details] reworked cavium octeon 'bootoctlinux' argument support patch The patch has been reworked to support single letter boot flags and factors out the code that sets bits in the boothowto variable. Other machdep code that needs to set boothowto flags can pass the "-<bootflags>" argument it gets from the boot loader into the new "boothowto_parse" function. These are a few examples on how I use this on my edgerouter lite. Boot normally from U-Boot: Octeon ubnt_e100# bootoctlinux ${loadaddr} coremask=0x3 Boot to single user mode: Octeon ubnt_e100# bootoctlinux ${loadaddr} coremask=0x3 -s Specify kernel environment variables: Octeon ubnt_e100# bootoctlinux ${loadaddr} coremask=0x3 vfs.root.mountfrom=ufs:/dev/da0s2a foo=bar Combination of boot flags and kenv: Octeon ubnt_e100# bootoctlinux ${loadaddr} coremask=0x3 vfs.root.mountfrom=ufs:/dev/da0s2a -s
Has anyone had an opportunity to review the reworked patch yet or have any suggestions?
It looks OK to me.
(In reply to Dave Rush from comment #4) Do you need someone to commit this?
Hey Dave, A couple of minor thoughts on your patch: 1. Maybe don't assume the starting argument index. I'm pretty sure you can pass arguments just fine even if coremask is absent, or am I misremembering? It's been awhile since I had to deal with it, honestly. You could easily enough do strcmp on the n from strsep to ignore those parameters. 2. If you do want to add boothowto_parse, which is a fine idea in my opinion, please make the passed parameter const! Thanks very much for doing this!
Created attachment 180318 [details] reworked cavium octeon 'bootoctlinux' argument support patch
(In reply to Juli Mallett from comment #7) You are right about the coremask parameter. More experimentation shows it can be unspecified (defaults to 1 core) or placed anywhere in the command line. It seems like the most straight forward thing to do is just parse the command line from the beginning. I attached a new patch that addresses your feedback.
(In reply to Sean Bruno from comment #6) Yes please, assuming assuming everyone agrees it's ok.
Would somebody be willing to commit this patch for me?
If Sean does not do it by then, I will tomorrow evening.
(In reply to Alexander Kabaev from comment #12) Umm, a question - does argv[0] contain useful value? My userland habits protest against using argv[0] as command line parameter. + for (i = 0; i < app_desc_ptr->argc; i++) { Anyway, I am committing a slightly different patch that moves parse_boothowto back into octeon-specific machdep file. I was the one who requested the code to be shared, but since your change does not update other existing platform to use common functions, putting it in MI file defeats the purpose. If and when someone actually makes the function be used by more than one client, then it can be moved to MI code.
A commit references this bug: Author: kan Date: Thu Mar 16 00:49:59 UTC 2017 New revision: 315338 URL: https://svnweb.freebsd.org/changeset/base/315338 Log: Add cavium octeon 'bootoctlinux' boot argument support While there, parse u-boot provided command line arguments for supported switches and update boothowto appropriately. Also support setting kenv variables from the kernel comman line. PR: 216831 (modified) Changes: head/sys/mips/cavium/octeon_machdep.c
(In reply to Alexander Kabaev from comment #13) argv[0] contains the name of the u-boot applet that loads the kernel. Not totally useful but not harmful either. Thanks for the help.