Page MenuHomeFreeBSD

bhyve: add helper to create a bootorder
ClosedPublic

Authored by corvink on Mar 27 2023, 10:29 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 3 2024, 12:29 PM
Unknown Object (File)
Jan 3 2024, 1:13 AM
Unknown Object (File)
Dec 20 2023, 5:00 AM
Unknown Object (File)
Dec 12 2023, 8:17 AM
Unknown Object (File)
Nov 11 2023, 8:48 AM
Unknown Object (File)
Oct 10 2023, 7:52 AM
Unknown Object (File)
Aug 18 2023, 7:54 AM
Unknown Object (File)
Jun 24 2023, 5:53 AM

Details

Summary

Qemu's fwcfg allows to define a bootorder. Therefore, the hypervisor has
to create a fwcfg item named bootorder, which has a newline seperated
list of boot entries. Qemu's OVMF will pick up the bootorder and applies
it.

Add the moment, bhyve's OVMF doesn't support a custom bootorder by
qemu's fwcfg. However, in the future bhyve will gain support for qemu's
OVMF. Additonally, we can port relevant parts from qemu to bhyve's OVMF
implementation if desired.

Diff Detail

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

Event Timeline

rew added inline comments.
usr.sbin/bhyve/pci_emul.c
977

why negative error here?

1443–1444
1577

why not return the error instead?

usr.sbin/bhyve/pci_emul.c
975

Note that style(9) also really wants you to declare all the variables within a block up front.

982

A TAILQ is fine. If the logic is simpler, a growable array of pointers might be the other option to consider as then checking for a conflict is simpler I think. In the iteration case you just have to skip NULL pointers in the array.

1421

Have you considered using open_memstream() to construct this string? It will do all the realloc + NUL terminate for you letting you do something like:

struct boot_device *device;
FILE *fp;
char *bootorder;
size_t bootorder_len;

if (TAILQ_EMPTY(&boot_devices))
    return (0);

fp = open_memstream(&fp, &bootorder_len);
TAILQ_FOREACH(device, &boot_devices, boot_device_chain) {
    fwrite(fp, "/pci@i0cf8/pci@%d,%d\n", device->pdi->pdi_slot,
        device->pdi->pdi_func);
}
fclose(cp);

return (qemu_fwcfg_add_file("bootorder", bootorder_len, bootorder));
1573

The comment mostly duplicates the function name so you can probably omit it.

  • fix style issues
  • use memstream to build a bootorder fwcfg item

Just one more suggestion. If you decide to use fprintf, you can still count it as reviewed.

usr.sbin/bhyve/pci_emul.c
1426

Is the NAME_MAX restriction part of the fwcfg API for this node? If not, maybe just use fprintf directly without the bootpath temporary variable?

This revision is now accepted and ready to land.Jun 19 2023, 5:21 PM
markj added inline comments.
usr.sbin/bhyve/pci_emul.c
989
1573
This revision was automatically updated to reflect the committed changes.