grehan-freebsd / grub2-bhyve

GNU General Public License v3.0
51 stars 21 forks source link

Update OpenBSD boot arguments for console device. #16

Closed yuichiro-naito closed 10 months ago

yuichiro-naito commented 1 year ago

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

https://github.com/Openbsd/src/commit/745c2f60e98fd1f418c104960a567e120624d705

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.

grehan-freebsd commented 1 year ago

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

yuichiro-naito commented 1 year ago

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 commented 11 months ago

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

yuichiro-naito commented 11 months ago

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 commented 11 months ago

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

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

grehan-freebsd commented 11 months ago

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

grehan-freebsd commented 10 months ago

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 :)