brand/bhyve: Add support for virtio-scsi configuration.

[hrosenfeld]

This code was written by Patrick van der Linden of EFit Partners. I made a significant overhaul and wrote the new manpage content.

Virtio-SCSI is a bit different from other device emulations, as there needs to be a configuration for the virtio-scsi HBA and then one for each of the targets assigned to it. It's possible to have multiple HBAs with multiple targets on each:

add attr
    set name="scsi0"
    set type="string"
    set value="backend=uscsi"
end
add attr
    set name="scsi0-target4"
    set type="string"
    set value="/dev/rdsk/c0t600144F0766CCA0600006A5BDD420005d0p0"
end
add attr
    set name="scsi0-target5"
    set type="string"
    set value="/dev/rdsk/c0t600144F0766CCA0600006A5BDD430006d0p0"
end

[citrus-it]

I wonder if we should be setting a bootoption here? It will depend on what the UEFI firmware detects and adds to the EFI variables.

[noengo]

I have modified our local implementation to comply with what was agreed upon in this request.
...
add attr
set name="scsi0"
set type="string"
set value="backend=uscsi"
end
...
add attr
set name="scsi0-target4"
set type="string"
set value="/dev/rdsk/c0t600144F0766CCA0600006A5BDD420005d0p0"
end
add attr
set name="scsi0-target5"
set type="string"
set value="/dev/rdsk/c0t600144F0766CCA0600006A5BDD430006d0p0"
end
....

This is indeed better than my previous implementation. More concise and clearer
Already in use in our production environments

[citrus-it]

Thanks for the rework, the separate scsiN/scsiN-targetM namespacing is nice, and we no longer disturb PCI function assignment.
Could you please update the PR description to match the new implementation?

[citrus-it]

The man page says [0, 255] so the 255 here needs to be 256, or the man page needs adjusting.

[citrus-it]

The list() here is a no-op and could be dropped.

[citrus-it]

The reworked code still doesn't call add_bootoption() so one cannot reference virtio-scsi targets from bootorder/bootnext. If that's tricky to wire up, can we note in the man page that scsi targets can't currently be used in the boot order attributes?

[hrosenfeld]

I'm not sure I truly understand how the boot options work. So far every disk is on its own PCI function, and they each add a boot option.

It would be quite easy to add a boot option for each virtio-scsi HBA, and probably a bit more involved if we want one for each target. Also, should it still be "disk", given the various device types that a target could be? Would the support code building the UEFI variables even understand anything else than that?

[citrus-it]

The names are just aliases - "disk", "net", "cdrom" and so on are only
used to resolve a bootorder/bootnext attribute value to a key. add_bootoption()
records name -> ('pci', 'slot.func') in boot.py, and uefi/vars.py builds the reverse
map by parsing the firmware's Boot#### entries and extracting the PCI node from
each one's device path. So a new alias family such as scsi0-target4 is fine, nothing
needs to understand "disk".

The catch is that vars.py currently keys only on the PCI node and, because
the reverse map is a dict, when several Boot entries share a PCI node the
last one wins. A per-HBA option would be ambiguous when the HBA has more
than one target.

A per-target option needs vars.py to also pick up the SCSI messaging node that
follows the PCI node in the firmware's device path (type 3, subtype 2,
4 bytes of data: 16-bit target ID then 16-bit LUN) and include it in the key,
e.g. ('pci', '16.0', 'scsi', '4.0'), plus one add_bootoption() per target in boot.py.
I'm partly guessing - we'd also need to check what the firmware actually
writes.

Let's leave this for a follow-up and just add a sentence in the man page saying
that scsi targets can't currently be referenced from bootorder/bootnext.

[citrus-it]

Suggest the simpler:

Suggested change

	','.join(list(map(lambda x: 'target=' + x , v['targets']))))
	','.join(f'target={x}' for x in v['targets'])

[citrus-it]

Thanks for the updates on this.