x86: Distinguish Xen from non-Xen PVH boots

[cperciva]

Summary

The PVH boot protocol, introduced by Xen, is now used by some non-Xen
platforms (e.g. the Firecracker VM) as well. In order to accommodate
these, we use CPUID to detect Xen and only perform Xen-specific setup
when running on that platform.

The "isxen" function duplicates some work done by identcpu.c later in
the boot process; but we need it here since this is the very first C
code which runs when PVH booting (even before hammer_time).

In many places the existing code had

xc_printf(...);
HYPERVISOR_shutdown(SHUTDOWN_crash);

making use of Xen functionality to print a message and shut down; we
replace this idiom with a CRASH(...) macro which calls those in the
Xen case and halts in the non-Xen case.

Sponsored by: https://www.patreon.com/cperciva

[royger]

I wonder whether we might want to move part of the x86/xen/pv.c contents (IOW: those used by firecracker also) into a more generic file? Maybe sys/x86/x86/pvh.c?

sys/x86/xen/pv.c

124

I think we might want to somehow cache whether we are running on Xen or not, as a CPUID instruction will trigger a vmexit and that's costly. Using a static local variable would be fine IMO.

132

You need to check CPUID range 0x40000000 to 0x40010000, as when Xen is exposing Viridian (HyperV) extensions the signature at 0x40000000 will be the HyperV one. See xen_hvm_cpuid_base() in hvm.c

146

I guess there's no way to print any kind of message to the console in the firecracker case here?

160

You might want to rename to hammer_time_pvh() for clarity (since it's no longer Xen-specific)

302

This is very specific to how Xen loads the symtab for BSDs, which is unlikely to be useful for firecracker (as I guess the loader there doesn't load the symtab at all?)

334

You can join with the previous condition to avoid adding more indentation to the line.

340

Are you planning to use this function for firecracker? If so it would benefit from renaming also. Otherwise I would leave the xc_printf() calls as-is.

483

There no need to replace this one, as the previous call is already a Xen specific hypercall, so if you get here for other hypervisors it's likely you won't be able to reach the CRASH() anyway due to jumping into an uninitialized hypercall page.

[cperciva]

Update per review from royger

[cperciva]

sys/x86/xen/pv.c

124

I don't know that we call this enough times (assuming we're not crashing, between 2 and 5 times) to matter, but I've added the optimization.

132

Ugh, ok. I didn't realize that Xen could PVH boot it while pretending to be something other than Xen.

146

Correct, unless we want to assume there's a UART and do a bunch of outb blindly.

160

My plan is to get things working first, then rename functions / move stuff between files / make it compile without options XENHVM / etc. later.

302

At present I don't think the symbols get loaded at all. Theoretically a future Firecracker might load the entire kernel file including symbols? But yes, Firecracker currently hits the "Unable to load ELF symtab" and returns quickly.

334

Fixed.

340

I'm not sure. I'm using a two year old branch of Firecracker which added PVH support and I'm not 100% confident that they implemented everything. Right now we have modlist_paddr == 0 but I'm not sure that will always be the case after PVH support is merged into Firecracker's main branch.

(In any case, renaming will come later.)

483

Quite right. Sorry, that was a search-and-replace which went too far.

[cperciva]

@royger Is this version ok?

[royger]

LGTM, just one nit. Fell free to commit when that's done, no need for another round of review.

sys/x86/xen/pv.c

121

nit: bool would be better here, or the usage of the function below would have to use 'isxen() != 0' because coding style states: "Do not use ! for tests unless it is a boolean, ...".