On 18 Dec 2017 3:45 p.m., "Guillaume Tucker" guillaume.tucker@collabora.com wrote:
On 18/12/17 11:45, Neil Williams wrote:
On 14 December 2017 at 09:47, Guillaume Tucker < guillaume.tucker@collabora.com> wrote:
On 07/12/17 17:16, Neil Williams wrote:
On 7 December 2017 at 16:20, Guillaume Tucker <
guillaume.tucker@collabora.com> wrote:
A change was sent a while ago to add support for the Coreboot /
Depthcharge bootloader which is used on Chromebook devices. This is useful in particular to avoid having to install U-Boot on Chromebook devices. See this Gerrit review here for previous history:
https://review.linaro.org/#/c/15203/I'm now opening this case again to try and get this resolved, there seem to be several issues with the original patch that would need to be clarified. Also, some things might have changed since then in LAVA or Coreboot which could potentially lead to a different approach - any feedback on this would be welcome.
Thanks for picking this up.
You're welcome. I've now uploaded a new version which generates the command line file but not the FIT image, it expects the kernel image to be already in this format. Still the same Gerrit number:
https://review.linaro.org/#/c/15203/
I've also made a patch to add the rk3288-veyron-jaq as a "depthcharge" device type:
https://review.linaro.org/#/c/22992/
So as a next step, it would be convenient to find a way to have the FIT image generated as part of the LAVA job with a given kernel image, dtb, maybe the .its file and optionally a ramdisk.
For the reference:
http://git.denx.de/?p=u-boot.git%3Ba=blob%3Bf=doc/uImage.FIT/how to.txt;hb=master
To start with, I understand that running mkimage on the
dispatcher is not a valid thing to do, it should receive a
FIT (flattened image tree) kernel image ready to be booted. This complicates things a bit for projects like kernelci.org where only a plain kernel image is built and ramdisks are served separately, but it's fair enough to say that LAVA is not meant to be packaging kernel images on the fly.
We've come up with a method in the meantime, although it does mean using
LXC but that makes it completely generic. It's principally designed for boards which need to munge a kernel and other files into an image to be transferred to the device using tools like fastboot. This is how KernelCI will be able to submit boot tests on devices like HiKey and db410c. Sadly, the example test job is suffering because the db410c devices have a different problem which is keeping them offline. Matt has been looking into this.
https://staging.validation.linaro.org/scheduler/job/203317/definition
https://staging.validation.linaro.org/static/docs/v2/actions -deploy.html#index-25
Thanks for the pointers, seems worth investigating.
On the other hand, creating the FIT image is a similar process to that of uImage, which is currently being done directly on the dispatcher:
https://git.linaro.org/lava/lava-dispatcher.git/tree/lava_di spatcher/actions/deploy/prepare.py#n79
So would it make sense to add some code there to support FIT?
What is an example command line to mkimage to do this?
mkimage -D "-I dts -O dtb -p 2048" -f rk3288-veyron-jaq.its arch/arm/boot/vmlinuz
Is the its file really needed? I added the ramdisk parameter precisely so lava doesn't need to generate one.
Regards,
Tomeu
Are any external configuration files required?
Everything should be in the .its file, and it should also be possible to generate it on the fly using a template and the LAVA device properties (kernel load address etc...). If this proves to not be flexible enough in practice, then I suppose the .its file could be downloaded although I think we should avoid doing this if we can.
Then I believe creating the command line file in LAVA should be
fine, although it probably makes more sense to have both the FIT
image and cmdline file generated by the same build system. In any case, both files would need to be served from the dispatcher TFTP server to the target device running Coreboot / Depthcharge.
That bit is fine, the problem is why this cannot use the existing
temporary paths which all the other TFTP devices use. Is it just to do some mangling of the files?
This is resolved now with the version I sent yesterday.
That makes this review much better, thanks.
Great, thanks for confirming.
So the idea was basically to have an option in Coreboot /
Depthcharge to interactively tell it where to find these files
for the current job to run, say:
<JOB_NUMBER>/tftp-deploy-<RANDOM>/kernel/vmlinuz <JOB_NUMBER>/tftp-deploy-<RANDOM>/kernel/cmdlineIt looks like the current patch in Gerrit relies on this location to be hard-coded in the bootloader, which works fine for a private development set-up but not for LAVA.
That makes very little sense because the whole point of TFTP is that
everything after the SERVER_IP is just a relative path from the TFTP base directory which is handled by the TFTP daemon itself.
Ditto.
To recap, my understanding is that the "depthcharge" boot support
code in LAVA would need to:
* maybe create the cmdline file with basically the kernel command line split up with one argument per lineAlternatively, do whatever operations are required in a test shell in
the LXC and then pass those files to the device - entirely within the test shell support.
That, or maybe run mkimage on the dispatcher like for uImage...
The cmdline file is now generated on the dispatcher.
* or just download the cmdline file along with the vmlinuz FITThe ready-made FIT kernel image is now downloaded with the
version I sent yesterday.
* place both the cmdline and vmlinuz FIT files in the job'sTFTP directory on the dispatcher* turn on the device and open the serial console... * interactively pass at least the path to the job TFTP directory on the serial console (and if possible the server IP address as well, and maybe even the individual file names rather than hard-coded vmlinuz and cmdline)Isn't this equivalent to what U-Boot already does with TFTP?
Almost. This part is now all implemented in the last patch I sent. One thing though is that the NFS rootfs parameters are stored in the kernel cmdline file and not set interactively in the bootloader shell.
How can these be extended by test writers? We do see requests to add arguments to the NFS parameters but adding options to the kernel command line itself is all but essential for most testing.
This can be done using the {{ extra_kernel_args }} template variable, see the other change to add base-depthcharge.jinja2:
https://review.linaro.org/#/c/22992/1/lava_scheduler_app/tes ts/device-types/base-depthcharge.jinja2
If anything more special ever needs to be done with some parameters such as inserting some IP address, it can be done in DepthchargeCommandOverlay where the command line file is generated.
The only command sent is to start the tftp
boot with the server IP and the relative paths to the kernel and cmdline files.
On this topic, the changes to add the tftpboot command in Depthcharge are still under review:
https://chromium-review.googlesource.com/c/chromiumos/platfo rm/depthcharge/+/451382
So I think it would actually be wiser to not merge base-depthcharge.jinja2 until the review above has been merged in case the command line syntax needs to be adjusted.
* look for a bootloader message to know when the kernel starts
to load and hand over to the next action (login...)Done as well, I've now got the veyron-jaq device booting fine
with NFS rootfs. There was an issue with adding a ramdisk to the FIT image as it was to big to boot on the device, will investigate this part to add "ramdisk" boot commands.
Please let me know if this sounds reasonable or if we should be
doing anything differently. I think it would be good to have
some agreement and a clear understanding of how this is going to be implemented before starting to work on the code again.
Best wishes, Guillaume _______________________________________________ Lava-users mailing list Lava-users@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lava-users
Hi Tomeu,
On 19/12/17 09:41, Tomeu Vizoso wrote:
On 18 Dec 2017 3:45 p.m., "Guillaume Tucker" guillaume.tucker@collabora.com wrote:
On 18/12/17 11:45, Neil Williams wrote:
On 14 December 2017 at 09:47, Guillaume Tucker < guillaume.tucker@collabora.com> wrote:
On 07/12/17 17:16, Neil Williams wrote:
On 7 December 2017 at 16:20, Guillaume Tucker <
guillaume.tucker@collabora.com> wrote:
A change was sent a while ago to add support for the Coreboot /
Depthcharge bootloader which is used on Chromebook devices. This is useful in particular to avoid having to install U-Boot on Chromebook devices. See this Gerrit review here for previous history:
https://review.linaro.org/#/c/15203/I'm now opening this case again to try and get this resolved, there seem to be several issues with the original patch that would need to be clarified. Also, some things might have changed since then in LAVA or Coreboot which could potentially lead to a different approach - any feedback on this would be welcome.
Thanks for picking this up.
You're welcome. I've now uploaded a new version which generates the command line file but not the FIT image, it expects the kernel image to be already in this format. Still the same Gerrit number:
https://review.linaro.org/#/c/15203/I've also made a patch to add the rk3288-veyron-jaq as a "depthcharge" device type:
https://review.linaro.org/#/c/22992/So as a next step, it would be convenient to find a way to have the FIT image generated as part of the LAVA job with a given kernel image, dtb, maybe the .its file and optionally a ramdisk.
For the reference:
http://git.denx.de/?p=u-boot.git;a=blob;f=doc/uImage.FIT/howto.txt;hb=master
To start with, I understand that running mkimage on the
dispatcher is not a valid thing to do, it should receive a
FIT (flattened image tree) kernel image ready to be booted. This complicates things a bit for projects like kernelci.org where only a plain kernel image is built and ramdisks are served separately, but it's fair enough to say that LAVA is not meant to be packaging kernel images on the fly.
We've come up with a method in the meantime, although it does mean using
LXC but that makes it completely generic. It's principally designed for boards which need to munge a kernel and other files into an image to be transferred to the device using tools like fastboot. This is how KernelCI will be able to submit boot tests on devices like HiKey and db410c. Sadly, the example test job is suffering because the db410c devices have a different problem which is keeping them offline. Matt has been looking into this.
https://staging.validation.linaro.org/scheduler/job/203317/definition
https://staging.validation.linaro.org/static/docs/v2/actions -deploy.html#index-25
Thanks for the pointers, seems worth investigating.
On the other hand, creating the FIT image is a similar process to that of uImage, which is currently being done directly on the dispatcher:
https://git.linaro.org/lava/lava-dispatcher.git/tree/lava_dispatcher/actions/deploy/prepare.py#n79
So would it make sense to add some code there to support FIT?
What is an example command line to mkimage to do this?
mkimage -D "-I dts -O dtb -p 2048" -f rk3288-veyron-jaq.its arch/arm/boot/vmlinuz
Is the its file really needed? I added the ramdisk parameter precisely so lava doesn't need to generate one.
The file is needed by mkimage, but it shouldn't need to be downloaded. If you carry on reading a few lines further:
Everything should be in the .its file, and it should also be possible to generate it on the fly using a template and the LAVA device properties (kernel load address etc...).
Are any external configuration files required?
Everything should be in the .its file, and it should also be possible to generate it on the fly using a template and the LAVA device properties (kernel load address etc...). If this proves to not be flexible enough in practice, then I suppose the .its file could be downloaded although I think we should avoid doing this if we can.
Then I believe creating the command line file in LAVA should be
fine, although it probably makes more sense to have both the FIT
image and cmdline file generated by the same build system. In any case, both files would need to be served from the dispatcher TFTP server to the target device running Coreboot / Depthcharge.
That bit is fine, the problem is why this cannot use the existing
temporary paths which all the other TFTP devices use. Is it just to do some mangling of the files?
This is resolved now with the version I sent yesterday.
That makes this review much better, thanks.
Great, thanks for confirming.
So the idea was basically to have an option in Coreboot /
Depthcharge to interactively tell it where to find these files
for the current job to run, say:
<JOB_NUMBER>/tftp-deploy-<RANDOM>/kernel/vmlinuz <JOB_NUMBER>/tftp-deploy-<RANDOM>/kernel/cmdlineIt looks like the current patch in Gerrit relies on this location to be hard-coded in the bootloader, which works fine for a private development set-up but not for LAVA.
That makes very little sense because the whole point of TFTP is that
everything after the SERVER_IP is just a relative path from the TFTP base directory which is handled by the TFTP daemon itself.
Ditto.
To recap, my understanding is that the "depthcharge" boot support
code in LAVA would need to:
* maybe create the cmdline file with basically the kernel command line split up with one argument per lineAlternatively, do whatever operations are required in a test shell in
the LXC and then pass those files to the device - entirely within the test shell support.
That, or maybe run mkimage on the dispatcher like for uImage...
The cmdline file is now generated on the dispatcher.
* or just download the cmdline file along with the vmlinuz FITThe ready-made FIT kernel image is now downloaded with the
version I sent yesterday.
* place both the cmdline and vmlinuz FIT files in the job'sTFTP directory on the dispatcher* turn on the device and open the serial console... * interactively pass at least the path to the job TFTP directory on the serial console (and if possible the server IP address as well, and maybe even the individual file names rather than hard-coded vmlinuz and cmdline)Isn't this equivalent to what U-Boot already does with TFTP?
Almost. This part is now all implemented in the last patch I sent. One thing though is that the NFS rootfs parameters are stored in the kernel cmdline file and not set interactively in the bootloader shell.
How can these be extended by test writers? We do see requests to add arguments to the NFS parameters but adding options to the kernel command line itself is all but essential for most testing.
This can be done using the {{ extra_kernel_args }} template variable, see the other change to add base-depthcharge.jinja2:
https://review.linaro.org/#/c/22992/1/lava_scheduler_app/tes ts/device-types/base-depthcharge.jinja2
If anything more special ever needs to be done with some parameters such as inserting some IP address, it can be done in DepthchargeCommandOverlay where the command line file is generated.
The only command sent is to start the tftpboot with the server IP and the relative paths to the kernel and cmdline files.
On this topic, the changes to add the tftpboot command in Depthcharge are still under review:
https://chromium-review.googlesource.com/c/chromiumos/platfo rm/depthcharge/+/451382
So I think it would actually be wiser to not merge base-depthcharge.jinja2 until the review above has been merged in case the command line syntax needs to be adjusted.
* look for a bootloader message to know when the kernel startsto load and hand over to the next action (login...)Done as well, I've now got the veyron-jaq device booting fine
with NFS rootfs. There was an issue with adding a ramdisk to the FIT image as it was to big to boot on the device, will investigate this part to add "ramdisk" boot commands.
Please let me know if this sounds reasonable or if we should be
doing anything differently. I think it would be good to have
some agreement and a clear understanding of how this is going to be implemented before starting to work on the code again.
Best wishes, Guillaume _______________________________________________ Lava-users mailing list Lava-users@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lava-users
On 19/12/17 09:47, Guillaume Tucker wrote:
mkimage -D "-I dts -O dtb -p 2048" -f rk3288-veyron-jaq.its arch/arm/boot/vmlinuz
Is the its file really needed? I added the ramdisk parameter precisely so lava doesn't need to generate one.
The file is needed by mkimage, but it shouldn't need to be downloaded. If you carry on reading a few lines further:
Everything should be in the .its file, and it should also be possible to generate it on the fly using a template and the LAVA device properties (kernel load address etc...).
Oh I see, you mean with this command:
cmd = "mkimage -f auto -A arm -O linux -T kernel -C None -d %s -b %s -i %s %s" % (kernel_filename, dtb_filename, ramdisk_filename, itb_path)
Fine, I'll take a look at this approach and compare with creating the .its file.
Guillaume
-
Guillaume Tucker -
Tomeu Vizoso