Style is wrong.

Use fstat() to get the file size.

Why the negative errno values?

I think you can just mmap the ROM file and copy it directly, no? Seems like that would be simpler.

There's no error checking. Why is that ok?

Let's avoid conflating uint64_t with pointer types. This will not work on CHERI, where pointers store additional metadata that is lost when downcasting to uint64_t. Use a pointer here and return it to the caller, which can cast when storing the address in psc_bar.

Again here, it seems better to print a warning closer to the source of the error (note you can use warnc() to give a better hint) rather than returning a negative errno value that gets converted to -1 anyway.

Some of the error paths leak the fd and the temporary mapping.

Shouldn't it be a fatal error?

Hmmm. I'm using autoformatting based on the .clang-format in current sources. It creates my version.

I've used the same style like passthru_mmio_addr.
I'm not sure if it should be a fatal error or not. The guest could set an invalid rom address. Should bhyve be killed in that case?

@markj Should I change it to a fatal error?

It's a fatal error now. If someone has issue with that due to a buggy guest, it'll be easy to change it later. It's much harder to fix issues when it's ignored.

Porting this over to illumos, and the compiler is saying:

pci_emul.c:835:24: error: 'addr' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  819 |  pdi->pi_bar[idx].addr = addr;

Seems right - do we need to set addr = 0; in this case block?

Porting to illumos, I'm getting:

pci_emul.c:2123 pci_cfgrw() error: buffer overflow 'pi->pi_bar' 7 <= 8

and several more.

I think line 2118 should be:

if (coff < PCIR_BIOS)

You're right. I think I've missed it because guests usually write to the ROM register before reading it. We should set addr = 0 in this case block.

Writes that aren't 4 byte aligned, are ignored. For that reason, if (coff != PCIR_BIOS) works as intented. However, making it more clear might be a good idea.

Coverity also flagged this -- CID 1486777.

D34688

D34689