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

Update OpenBSD boot arguments for console device. #16

Conversation

yuichiro-naito
Copy link
Contributor

OpenBSD has dropped supporing old console device structure by the following commit.

openbsd/src@745c2f6

This commit changes to use new console device structure by default. However if the loading kernel is older than '7.3' release, use the old console device structure for compatibility.

The version number is written in the value of 'osrelease' symbol in the OpenBSD
kernel. All the previous releases contain symbol tables, so no problem.
If users strip the OpenBSD kernel, reading the version number will fail. I added 'kopenbsd -l'
option to force to use the old console device structure.

Reading symbol tables is already implemented in 'grub_openbsd_find_ramdisk'
function. I added reading the value of 'osrelease' symbol in this function to
share the symbol lookup code. And building console device structure is
implemented in 'grub_cmd_openbsd' function, but it is called earlier than
'grub_openbsd_find_ramdisk' function. So 'grub_cmd_openbsd' function never
knows the version number. I changed to build both the new and old console device
structures in 'grub_cmd_openbsd'. They are added to 'tag' list and
'grub_openbsd_boot' function reads the 'tag' list and build stack frame that is
passed to OpenBSD kernel. I changed the building stack frame code to ignore
unnecessary console device structure checked by the 'osrelease' value.

I also added 'openbsd_force_legacy_console' global variable. It's a flag to
force to use the old console device structure. This flag is set by '-l' option.

I confirmed this patch booted OpenBSD kernel from 6.9 to 7.3 and Current successfully.
And also the installers of OpenBSD 6.9 - 7.3 was booted successfully.

OpenBSD has dropped supporing old console device structure
by the following commit.

openbsd/src@745c2f6

This commit changes to use new console device structure by default.
However if the loading kernel is older than '7.3' release, use the
old console device structure for compatibility.
@grehan-freebsd
Copy link
Owner

Thanks: this looks great work. I'll do a quick build and test before merging.

@yuichiro-naito
Copy link
Contributor Author

I fixed use after free bug in my patch. The memories for the elf file header must be kept until 'osrelease' symbol value is parsed. I'm sorry if this issue is suffering to you.

@brycv
Copy link

brycv commented Nov 30, 2023

Any updates on getting this merged? This is pretty important for OpenBSD 7.4 and later.

@yuichiro-naito
Copy link
Contributor Author

Hi, I updated my patch to simplify the logic. The new console structure 'grub_openbsd_bootarg_console' is always assigned for the boot argument. If the OpenBSD kernel version is prior to 7.3, it is replaced by the legacy console structure. The new structure is bigger than the legacy one. So, there is no need to reallocate the memory. We can simply rearrange the memory data image to the legacy structure.

@yuichiro-naito
Copy link
Contributor Author

And also I reported to the upstream project, GNU Grub2. But no response yet...

https://savannah.gnu.org/bugs/index.php?64919

@grehan-freebsd
Copy link
Owner

Thanks for the update. Was just testing with the prior patch but will cut over.

@grehan-freebsd
Copy link
Owner

I've tested this with 5.6/6.5/7.3 and 7.4, both i386 and amd64, and verified that both the new and patched grub-bhyve work correctly where expected, and that the patched version correctly boots 7.4/amd64 whereas the original can't.

So, I'll merge this, update the readme to document the new "-l" option, and file a PR against the port to be updated.

Thanks very much for this hard work - debugging the serial console and low-level bootcode is not an easy task :)

@grehan-freebsd grehan-freebsd merged commit 91f06e5 into grehan-freebsd:master Dec 18, 2023
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.

None yet

3 participants