Page MenuHomeFreeBSD

bhyvectl: Add an arm64 port
ClosedPublic

Authored by markj on Apr 24 2024, 10:29 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 12, 7:00 PM
Unknown Object (File)
Mon, Nov 4, 6:13 AM
Unknown Object (File)
Mon, Nov 4, 6:12 AM
Unknown Object (File)
Mon, Nov 4, 6:12 AM
Unknown Object (File)
Mon, Nov 4, 6:12 AM
Unknown Object (File)
Mon, Nov 4, 5:47 AM
Unknown Object (File)
Sat, Oct 19, 9:53 AM
Unknown Object (File)
Oct 1 2024, 5:55 AM

Details

Summary

Right now this implementation doesn't provide any MD functionality, but
is enough for one to perform basic VM management tasks on arm64. In
particular, one can use it to destroy VMs and force a reboot.

As in bhyve, the approach is to move MD logic into a bhyvectl_machdep.c.
Each copy of this file has the ability to register command line options,
add flags to the usage message, and implement functionality via
bhyvectl_md_main().

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Can this patch be split into multiple smaller ones to make it easier for review?

Can this patch be split into multiple smaller ones to make it easier for review?

Most of the patch is simply moving code from one file to the other. I looked for ways to split it up but don't really see one. I could add arm64 in a separate patch at least, do you think it's enough? How else would you propose splitting up the patch?

First of all, looks good. Splitting the commit is just a suggestion!

A single commit makes it hard to see which code was simply moved and which was modified. So, ideally one commit makes some changes and another one just moves code. In this case, one commit making changes to bhyvectl, one commit moving code to amd64/bhyvectl and one commit adding arm64 might make sense.

Btw. this commit includes some format changes which have a similar issue. They make it hard to see the "real" change. For that reason, I'm mostly try to avoid fixing style issues or I'm adding a separate commit for fixing style.

I'm not sure there's a useful way to split this up into multiple commits. Maybe one to do the amd64 split and a second to add arm64. (In particular, we may soon get a RISC-V port so having "add arm64" as a standalone commit might be nice for RISC-V to pull from.)

usr.sbin/bhyvectl/bhyvectl.c
89 ↗(On Diff #137633)

Do you want to add a OPT_LAST here and then add a static assert that OPT_LAST < OPT_START_MD?

143 ↗(On Diff #137633)

Maybe leave it called usage to minimize diffs?

markj marked 2 inline comments as done.

Apply suggestions from jhb.

Before pushing, I'll split this into two commits, one moving MD code into amd64/ and the other adding the arm64 bits.

This revision is now accepted and ready to land.Apr 30 2024, 10:01 PM