-
-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SeaBIOS support #574
base: develop
Are you sure you want to change the base?
SeaBIOS support #574
Conversation
@macpijan I wonder how far I should go with that, now results are as follows: % ./scripts/ci/qemu-self-test-seabios.sh
==============================================================================
Setup-And-Boot-Seabios :: This suite verifies the correct operation of keyw...
==============================================================================
Enter Boot Menu SeaBIOS :: Test Enter Boot Menu kwd | PASS |
------------------------------------------------------------------------------
Enter Boot Menu SeaBIOS And Return Construction :: Test Enter Boot... | PASS |
------------------------------------------------------------------------------
Enter sortbootorder :: Test Enter sortbootorder kwd | PASS |
------------------------------------------------------------------------------
Enter TPM Configuration :: Test Enter TPM Configuration kwd | PASS |
------------------------------------------------------------------------------
Enter iPXE :: Test Enter iPXE kwd | FAIL |
No match found for 'autoboot' in 10 seconds. Output:
------------------------------------------------------------------------------
Setup-And-Boot-Seabios :: This suite verifies the correct operatio... | FAIL |
5 tests, 4 passed, 1 failed
==============================================================================
Output: /home/user/src/3mdeb/dasharo/open-source-firmware-validation/logs/2024.11.12_14.29.14/setup-and-boot-seabios/output.xml
Log: /home/user/src/3mdeb/dasharo/open-source-firmware-validation/logs/2024.11.12_14.29.14/setup-and-boot-seabios/log.html
Report: /home/user/src/3mdeb/dasharo/open-source-firmware-validation/logs/2024.11.12_14.29.14/setup-and-boot-seabios/report.html
zsh: exit 1 ./scripts/ci/qemu-self-test-seabios.sh Of course, I can satisfy all the checks despite the self-test change, and if my approach is correct, I would probably need your approval. Regarding the SeaBIOS self-test, my concern is that I'm entering the land of testing |
Another set of changes: % ./scripts/ci/qemu-self-test-seabios.sh
==============================================================================
Setup-And-Boot-Seabios :: This suite verifies the correct operation of keyw...
==============================================================================
Enter Boot Menu SeaBIOS :: Test Enter Boot Menu kwd | PASS |
------------------------------------------------------------------------------
Enter Boot Menu SeaBIOS And Return Construction :: Test Enter Boot... | PASS |
------------------------------------------------------------------------------
Enter sortbootorder :: Test Enter sortbootorder kwd | PASS |
------------------------------------------------------------------------------
Get sortbootorder Menu Construction :: Get sortbootorder Menu Cons... | PASS |
------------------------------------------------------------------------------
Get Option State :: Test Get Option State kwd | PASS |
------------------------------------------------------------------------------
Enter Menu From Snapshot and Return sortbootorder Consruction :: T... | PASS |
------------------------------------------------------------------------------
Enter TPM Configuration :: Test Enter TPM Configuration kwd | PASS |
------------------------------------------------------------------------------
Enter iPXE :: Test Enter iPXE kwd | FAIL |
No match found for 'autoboot' in 10 seconds. Output:
------------------------------------------------------------------------------
Setup-And-Boot-Seabios :: This suite verifies the correct operatio... | FAIL |
8 tests, 7 passed, 1 failed
==============================================================================
Output: /home/user/src/3mdeb/dasharo/open-source-firmware-validation/logs/2024.11.14_00.54.30/setup-and-boot-seabios/output.xml
Log: /home/user/src/3mdeb/dasharo/open-source-firmware-validation/logs/2024.11.14_00.54.30/setup-and-boot-seabios/log.html
Report: /home/user/src/3mdeb/dasharo/open-source-firmware-validation/logs/2024.11.14_00.54.30/setup-and-boot-seabios/report.html
zsh: exit 1 ./scripts/ci/qemu-self-test-seabios.sh |
% ./scripts/ci/qemu-self-test-seabios.sh
==============================================================================
Setup-And-Boot-Seabios :: This suite verifies the correct operation of keyw...
==============================================================================
Enter Boot Menu SeaBIOS :: Test Enter Boot Menu kwd | PASS |
------------------------------------------------------------------------------
Enter Boot Menu SeaBIOS And Return Construction :: Test Enter Boot... | PASS |
------------------------------------------------------------------------------
Enter sortbootorder :: Test Enter sortbootorder kwd | PASS |
------------------------------------------------------------------------------
Get sortbootorder Menu Construction :: Get sortbootorder Menu Cons... | PASS |
------------------------------------------------------------------------------
Get Option State :: Test Get Option State kwd | PASS |
------------------------------------------------------------------------------
Enter Menu From Snapshot and Return sortbootorder Consruction :: T... | PASS |
------------------------------------------------------------------------------
Enter TPM Configuration :: Test Enter TPM Configuration kwd | PASS |
------------------------------------------------------------------------------
Enable Network Boot :: Test Enable Network/PXE boot | PASS |
------------------------------------------------------------------------------
Enter iPXE :: Test Enter iPXE kwd | PASS |
------------------------------------------------------------------------------
Setup-And-Boot-Seabios :: This suite verifies the correct operatio... | PASS |
9 tests, 9 passed, 0 failed
============================================================================== |
2da749d
to
e0d5520
Compare
@macpijan what would you need here to merge it as a base for further development? |
a0cf13a
to
f86cb7f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think more code can be reused, rather than copied in two places.
First thought would be to have:
- lib/bios/common.robot - some common keywords, that can be reused across differnt bios versions
- lib/bios/seabios.robot - seabios keywords - either entirely new ones, or calling ones from common. robot, maybe with some extra arguments
- lib/bios/edk2.robot (?) - for dasharo uefi menus based on edk2
@@ -0,0 +1,53 @@ | |||
name: Keywords self-tests for Dasharo (coreboot+SeaBIOS) on QEMU Q35 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of this is copied from existing file, so we could avoid duplication: https://docs.github.com/en/actions/sharing-automations/reusing-workflows
Not a high priority, we can also leave it in issues.
I can also imagine, we would like to reuse this workflow not only in this repo, but use very similar config in e.g. coreboot or edk2 repos in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should stick to GH's actions. The question is if this would be reliable. Sooner or later, we will exhaust the pool of free actions and storage. I don't see ourselves switching to paid GH, so I would switch to our Woodpecker CI instead.
But I get your point about reusability. My gh-actions-fu was not good enough to be aware of that possibility. There is no space for such work for now, but we should track it as a potential improvement. Based on your comment, I will create an issue for the following PC Engines release.
lib/bios/seabios.robot
Outdated
${menu}= Get Setup Menu Construction | ||
RETURN ${menu} | ||
|
||
Get RTC Clock Submenu Construction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not know if RTC is the only submenu right now, but this kwd would work for submenus in general, not only this specific one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it is not the only one; there are at least two more, but those are "hidden":
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed in b435739 as unused
lib/bios/seabios.robot
Outdated
[Documentation] This keyword saves introduced changes | ||
Write Bare Into Terminal s | ||
|
||
Get IPXE Boot Menu Construction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same as UEFI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Please ping me once you believe you have a complete list of keywords you need, or at least remove those that are not needed at the time, so we do not waste time discussing something to be dropped anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pietrushnic I saw a new MR, do you consider this one finalized per your current needs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@macpijan Yes, I need to figure out how to extend it without starting to enable new tests. If you think something is missing here, considering there is minimal support and new PRs coming, please let me know, and I will try to improve.
Feedback on whether the approach is clean enough would also be helpful.
I like those ideas and will work on implementing them. However, I'm worried that PR will grow indefinitely and never be merged. I have time to push now, but I may return to it in 2-3 months. Nobody will pick improvements, and we will face various issues. I would be glad to divide it into smaller PRs, which consist of changes that can already be accepted and will not destroy anything. Still, I will slowly move the development of SeaBIOS forward, delivering something each time. |
Let's see what can be done in this iteration. Move the rest to the next one. |
lib/bios/seabios.robot
Outdated
# Title line, such as: Dasharo System Features | ||
# BOTTOM: | ||
# Help line, such as: F9=Reset to Defaults Esc=Exit | ||
${submenu}= Get Menu Construction ${checkpoint} ${lines_top} ${lines_bot} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This keyword is not defined aywhere? Since Get RTC Clock Submenu Construction
uses undefined keyword, I assume it has not been tested.
Which keyword from this file do you actually need to execute the tests, and which have been tested @pietrushnic ?
lib/bios/seabios.robot
Outdated
|
||
RETURN ${value}[0] | ||
|
||
Set Option State And Return Construction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also not used in self-tests, or in any tests so far - unless I am mistaken and you have run some tests with these keywords. Has it been tested? I am confused which keywords are really needed for seabios operation. Please confirm this.
Ideally, we test all of the keywords from seabios.robot in self- tests. That was possible for uefi, it's not possible in seabios ?
This commit starts standard lib: c3fc40a |
b3e6195
to
77e2b9a
Compare
Signed-off-by: Piotr Król <[email protected]>
Signed-off-by: Piotr Król <[email protected]> Signed-off-by: Piotr Król <[email protected]>
…pends on menus.robot and conflict seabios Signed-off-by: Piotr Król <[email protected]>
…ed by seabios Signed-off-by: Piotr Król <[email protected]>
…env var Signed-off-by: Piotr Król <[email protected]>
Signed-off-by: Piotr Król <[email protected]>
Signed-off-by: Piotr Król <[email protected]>
It appers to me that the selftest config should have more menus enabled, than the "release" config, supporting only "reasonable" menus. Signed-off-by: Maciej Pijanowski <[email protected]>
…,robot Signed-off-by: Maciej Pijanowski <[email protected]>
Signed-off-by: Piotr Król <[email protected]>
Signed-off-by: Piotr Król <[email protected]>
…y in qemu seabios config Signed-off-by: Piotr Król <[email protected]>
…ot resource Signed-off-by: Piotr Król <[email protected]>
…ibility Signed-off-by: Piotr Król <[email protected]>
Signed-off-by: Piotr Król <[email protected]>
Signed-off-by: Piotr Król <[email protected]>
Signed-off-by: Piotr Król <[email protected]>
This commit replace: - Enter Boot Menu Tianocore And Return Construction - Enter Boot Menu SeaBIOS And Return Construction with one keyword Enter Boot Menu and Return Construction, which is provided by lib/bios/common.robot It also move Parse Menu Snapshot Into Construction to common lib. Signed-off-by: Piotr Król <[email protected]>
Signed-off-by: Piotr Król <[email protected]>
Signed-off-by: Piotr Król <[email protected]>
Signed-off-by: Piotr Król <[email protected]>
…pper Signed-off-by: Piotr Król <[email protected]>
Signed-off-by: Piotr Król <[email protected]>
…tation Signed-off-by: Piotr Król <[email protected]>
Signed-off-by: Piotr Król <[email protected]>
Signed-off-by: Piotr Król <[email protected]>
10abb86
to
c7f0aeb
Compare
Signed-off-by: Filip Lewiński <[email protected]>
Signed-off-by: Filip Lewiński <[email protected]>
Signed-off-by: Filip Lewiński <[email protected]>
@filipleple, how was the "Boot System ..." keyword tested? I guess you didn't automatically install OS because of this #274 ? |
We install operating systems (typically Windows and Ubuntu) when setting up the platforms using configs from: https://github.com/Dasharo/preseeds |
@macpijan, so "setting up the platform" is not automated? I'm not sure what that means. Is this Assembly in Dasharo Lab or preparation before running regression/release tests? I want to leverage I prefer an automatic installation mechanism that can be kicked as a test, but the difference between those approaches may be negligible. In that case, the remaining arguments for automatic installation in tests would be:
We used to have automatic OS installation, and I used that in legacy PC Engines tests. I would like to know how we plan to address this topic better to boost release cycle time. Shouldn't we have tests where we install over netboot.xyz with our preseed? |
We typically install Windows -> Ubuntu using automated installers from preseeds repo when setting up platform in the lab.It must be in this order to install them both on one disk.
You could, or you could test the proposal from Daniil for Ubuntu Server installer. For PC Engines you are probably not that interested in the Ubuntu Desktop installer. Also, you do not have the limitations caused by Windows here. |
Yes, I've used a platform with pre-installed OS'es, as we usually do with regression/testing tasks Here's the full log for dmidecode for reference: https://cloud.3mdeb.com/index.php/s/mALHsbN7DgWeSpx |
Please let me know how to split those changes to merge most of them. The results are far from production, but I'm working on fixes.
The two most important fixes support for flash in sortbootorder and figuring out how to run iPXE in QEMU, so QEMU would not use OptionROM, but SeaBIOS will load builtin one.