Most register of the PCI header are either constant values or require emulation anyway. The command and status register are the only exception which require hardware access. So, we're adding an emulation handler for all other register. As this emulation handler will be reused by some future features like GPU passthrough, we directly export it.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
| usr.sbin/bhyve/pci_passthru.c | ||
|---|---|---|
| 561 | Humm, I would maybe just use int or u_int instead of uint32_t for config register addresses throughout? The new function hooks all use int. | |
| 561 | I would maybe use int or u_int instead of uint32_t for PCI config space addresses? If you really wanted a fixed-width type, uint16_t would be the most accurate, but a simple int is what the new function hooks use already. Also, I would maybe use i <= PCIR_MAXLAT instead of adding a new constant. | |
| 568 | Humm, is it really safe to read all these as individual bytes? I would be tempted to read things like BARs as 32-bit values as I'm not sure if all hardware is going to tolerate byte reads? | |
| 592 | 80 cols? | |
| 693 | Maybe "Default PCI config registers to accessing the config register of the physical device." | |
| 695–696 | The whitespace seems inconsistent here and in the clauses below? | |
| 697 | Newlines before comments. Also, I would maybe reword this somewhat to be more like "Emulate most PCI header registers", | |
| 744 | How is this handled now? Seems like you need to add a new call to set emulated hooks for this range when this value is true? | |
| 761–770 | ()'s around the "argument" to return | |
| 775–776 | Hummm, shouldn't these be handled by setting the ranges for these config registers to the emulated hooks earlier? That is, I guess you need custom handlers for write, but having passthru_cfgwrite_msicap seems more consistent with the change you are making here? | |
| 779 | I think it would be beneficial for this patch to move this into a separate function. | |
| usr.sbin/bhyve/pci_passthru.c | ||
|---|---|---|
| 694 | But now we don't emulate accesses to fields below PCIR_BAR(0)? | |
| usr.sbin/bhyve/pci_passthru.c | ||
|---|---|---|
| 694 | I should improve my commit message. Except the command and status register, the PCI header contains fixed values or values which require emulation anyways. So, IMHO, we should always emulate the whole PCI header. It makes the code simpler and avoids hardware accesses. | |