bhyve/riscv: correctly handle empty masks and broadcast in SBI IPI/RFNC

[doonbsd_gmail.com]

Summary

Bhyve treats an empty cpu set as an error which triggers MPASS panic in
the guest. As per the SBI spec, empty masks should return SBI_SUCCESS,
and a base of -1UL specifies a broadcast to all harts. Fix both in vmm_sbi.c,
clear a1 on return, and skip the SBI ecall in sbi_ipi_pic_ipi_send() if the
mask is empty. Also add a NULL check in riscv_send_ipi().

Signed-off-by: Doongar Singh <[e-mail]>

[br]

Looks good and works great, only a minor issue

sys/riscv/vmm/vmm_sbi.c

96

could we use ffs() here and below?

[br]

another nit

sys/riscv/vmm/vmm_sbi.c

174

need to make this line shorter for 1 character. I usually remove one whitespace-character after comma, but it is up to you

[br]

This resolves a bug in the SBI IPI emulation when booting a guest OS in bhyve. The guest could request hart_mask == 0, in that case we indeed should return SBI_SUCCESS and treat the call as a no-op, as long as hart_mask_base != -1.
Thanks!

[doonbsd_gmail.com]

Few optimizations

• use of ffsl() instead of a for each bit loop
• in case when hart_mask = -1, use CPU_COPY to copy active_cpus instead of copying bit by bit
• 80 column fix

[br]

Looks good and tested with bhyve on SpacemiT k3 (using generic guest image).
SBI IPI works fine. Thanks!

sys/riscv/vmm/vmm_sbi.c

163

from style.9:
Either braces are used for all single-line statements (in a file), or they are used only where needed for clarity. Assuming this is for clarity

[jrtc27]

sys/riscv/riscv/sbi_ipi.c

77

Please don't conflate supervisor and hypervisor changes

sys/riscv/vmm/vmm_riscv.c

551

Surely this can't happen? The caller ensures only active CPUs are allowed, and I think we should maintain the internal contract that calling riscv_send_ipi with garbage in cpus, including mentioning ones that don't exist for the given struct hyp, is illegal and not something to guard against.

[jrtc27]

sys/riscv/vmm/vmm_sbi.c

100–102

This would be the minimal fix, right? (Plus changing the return value on line 106)

163–165

Similarly