Skip to content
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

Open
wants to merge 33 commits into
base: develop
Choose a base branch
from
Open

SeaBIOS support #574

wants to merge 33 commits into from

Conversation

pietrushnic
Copy link
Contributor

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.

% ./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                   | FAIL |
No match found for 'Save configuration and exit' in 10 seconds. Output:
SeaBIOS (version rel-1.16.0.1-23-g5b5fd6ffPCI: XHCI at 00:04.0 (mmio 0xfcf64000)
XHCI init: regs @ 0xfcf64000, 8 ports, 64 slots, 32 byte contexts
XHCI    protocol USB  2.00, 4 ports (offset 5), def 0
XHCI    protocol USB  3.00, 4 ports (offset 1), def 0
/3ed38000\ Start thread
AHCI controller at 00:1f.2, iobase 0xfcf61000, irq 10
AHCI: cap 0xc0141f05, ports_impl 0x3f
/3ed36000\ Start thread
|3ed36000| AHCI/0: probing
/3ed35000\ Start thread
|3ed35000| AHCI/1: probing
/3ed33000\ Start thread
|3ed33000| AHCI/2: probing
|3ed33000| AHCI/2: link up
|3ed33000| Searching bootorder for: /pci@i0cf8/*@1f,2/drive@2/disk@0
|3ed33000| Searching bios-geometry for: /pci@i0cf8/*@1f,2/drive@2/disk@0
/3ed32000\ Start thread
|3ed32000| AHCI/3: probing
    [ Message content over the limit has been removed. ]
Searching bootorder for: /pci@i0cf8/*@2
Unable to find VPD table
Unable to find VPD table

Press F10 key now for boot menu

Searching bootorder for: HALT
Space available for UMB: dc800-ed000, f69e0-f70e0
Returned 16773120 bytes of ZoneHigh
e820 map has 7 items:
  0: 0000000000000000 - 000000000009fc00 = 1 RAM
  1: 000000000009fc00 - 00000000000a0000 = 2 RESERVED
  2: 00000000000f0000 - 0000000000100000 = 2 RESERVED
  3: 0000000000100000 - 000000003fd45000 = 1 RAM
  4: 000000003fd45000 - 0000000040000000 = 2 RESERVED
  5: 00000000b0000000 - 00000000c0000000 = 2 RESERVED
  6: 00000000fed40000 - 00000000fed45000 = 2 RESERVED
enter handle_19:

Booting from ROM..Booting from ca00:0385
.
iPXE starting execution...ok
iPXE initialising devices...unimplemented handle_1aXX:258:
    [ Message content over the limit has been removed. ]
Trying next device: 2
Booting from DVD/CD..AHCI/2: ... finished, status 0x41, ERROR 0x20
Device reports MEDIUM NOT PRESENT - 2 tries left
AHCI/2: ... finished, status 0x41, ERROR 0x20
Device reports MEDIUM NOT PRESENT - 1 tries left
AHCI/2: ... finished, status 0x41, ERROR 0x20
Device reports MEDIUM NOT PRESENT - 0 tries left
AHCI/2: ... finished, status 0x41, ERROR 0x20
.
Boot failed: Could not read from CDROM (code 0003enter handle_18:
  NULL
)
Trying next device: enter handle_18:
  NULL
3
Trying next device: enter handle_18:
  NULL
4
Trying next device: 5
No bootable device.  Retrying in 60 seconds.
------------------------------------------------------------------------------
Setup-And-Boot-Seabios :: This suite verifies the correct operatio... | FAIL |
5 tests, 3 passed, 2 failed
==============================================================================

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.

@pietrushnic pietrushnic requested a review from macpijan November 8, 2024 14:05
@pietrushnic
Copy link
Contributor Author

@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 sortbootorder; some tests exist in the legacy framework, and I could start porting those, but then the question arises: what is the difference between the keyword that should be tested and test itself. I think we should validate iPXE and network booting comprehensively, but to enter iPXE you need to enable network, which means you have to teach OSFV how to walk into sortbootorder (already done ) and change some option (probably done in legacy test suite).

@pietrushnic
Copy link
Contributor Author

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

@pietrushnic
Copy link
Contributor Author

% ./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
==============================================================================

@pietrushnic pietrushnic force-pushed the seabios-support branch 3 times, most recently from 2da749d to e0d5520 Compare November 25, 2024 23:51
@pietrushnic pietrushnic changed the title WIP: SeaBIOS support SeaBIOS support Nov 26, 2024
@pietrushnic
Copy link
Contributor Author

@macpijan what would you need here to merge it as a base for further development?

Copy link
Contributor

@macpijan macpijan left a 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

scripts/ci/qemu-run.sh Outdated Show resolved Hide resolved
@@ -0,0 +1,53 @@
name: Keywords self-tests for Dasharo (coreboot+SeaBIOS) on QEMU Q35
Copy link
Contributor

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.

Copy link
Contributor Author

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.

keywords.robot Outdated Show resolved Hide resolved
lib/bios/seabios.robot Outdated Show resolved Hide resolved
${menu}= Get Setup Menu Construction
RETURN ${menu}

Get RTC Clock Submenu Construction
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

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

[Documentation] This keyword saves introduced changes
Write Bare Into Terminal s

Get IPXE Boot Menu Construction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as UEFI

Copy link
Contributor Author

@pietrushnic pietrushnic Nov 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@macpijan there are too many leftovers I have clean that up: a9ac47c - this keyword is not used by me anywhere.

Copy link
Contributor

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.

Copy link
Contributor

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?

Copy link
Contributor Author

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.

scripts/ci/qemu-self-test.sh Outdated Show resolved Hide resolved
platform-configs/qemu-selftests-seabios.robot Show resolved Hide resolved
scripts/ci/qemu-run.sh Outdated Show resolved Hide resolved
scripts/ci/qemu-self-test-seabios.sh Outdated Show resolved Hide resolved
@pietrushnic
Copy link
Contributor Author

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

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.

@macpijan
Copy link
Contributor

Let's see what can be done in this iteration. Move the rest to the next one.

# 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}
Copy link
Contributor

@macpijan macpijan Nov 27, 2024

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 ?


RETURN ${value}[0]

Set Option State And Return Construction
Copy link
Contributor

@macpijan macpijan Nov 27, 2024

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 ?

@pietrushnic
Copy link
Contributor Author

  • lib/bios/edk2.robot (?) - for dasharo uefi menus based on edk2

@macpijan fd2d51f

@pietrushnic
Copy link
Contributor Author

  • lib/bios/common.robot - some common keywords, that can be reused across differnt bios versions

This commit starts standard lib: c3fc40a
I moved Enter Boot Menu {Tianocore,SeaBIOS} to Enter Boot Menu and parameterized with FW_STRING instead of {SEABIOS,TIANOCORE}_STRING.

@pietrushnic pietrushnic force-pushed the seabios-support branch 2 times, most recently from b3e6195 to 77e2b9a Compare November 29, 2024 23:14
macpijan and others added 19 commits December 2, 2024 15:35
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]>
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]>
@pietrushnic
Copy link
Contributor Author

@filipleple, how was the "Boot System ..." keyword tested? I guess you didn't automatically install OS because of this #274 ?

@macpijan
Copy link
Contributor

@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

@pietrushnic
Copy link
Contributor Author

@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 os_install option from qemu-run.sh and perform automatic tests on top of Dasharo (coreboot+SeaBIOS) for QEMU. Are you suggesting I use create_image.sh and then provide it as INSTALLER_PATH?

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:

  • excluding humans from the process to limit potential errors,
  • limit additional steps,
  • decrease maintenance overhead (we should update to 24.04.1) and keep adjustments in one place (OSFV) instead of OSFV and pressed repo

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?

@macpijan
Copy link
Contributor

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?

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.

I want to leverage os_install option from qemu-run.sh and perform automatic tests on top of Dasharo (coreboot+SeaBIOS) for QEMU. Are you suggesting I use create_image.sh and then provide it as INSTALLER_PATH?

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.

@filipleple
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants