Changeset View
Changeset View
Standalone View
Standalone View
usr.sbin/bhyve/pci_passthru.c
| Show First 20 Lines • Show All 597 Lines • ▼ Show 20 Lines | cfginitbar(struct passthru_softc *sc) | |||||||||
| return (0); | return (0); | |||||||||
| } | } | |||||||||
| static int | static int | |||||||||
| cfginit(struct pci_devinst *pi, int bus, int slot, int func) | cfginit(struct pci_devinst *pi, int bus, int slot, int func) | |||||||||
| { | { | |||||||||
| int error; | int error; | |||||||||
| struct passthru_softc *sc; | struct passthru_softc *sc; | |||||||||
| uint8_t intline, intpin; | ||||||||||
| error = 1; | error = 1; | |||||||||
| sc = pi->pi_arg; | sc = pi->pi_arg; | |||||||||
| bzero(&sc->psc_sel, sizeof(struct pcisel)); | bzero(&sc->psc_sel, sizeof(struct pcisel)); | |||||||||
| sc->psc_sel.pc_bus = bus; | sc->psc_sel.pc_bus = bus; | |||||||||
| sc->psc_sel.pc_dev = slot; | sc->psc_sel.pc_dev = slot; | |||||||||
| sc->psc_sel.pc_func = func; | sc->psc_sel.pc_func = func; | |||||||||
| /* | ||||||||||
| * Copy physical PCI header to virtual config space. INTLINE and INTPIN | ||||||||||
jhb: Humm, I would maybe just use `int` or `u_int` instead of `uint32_t` for config register… | ||||||||||
Done Inline ActionsI 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. jhb: I would maybe use `int` or `u_int` instead of `uint32_t` for PCI config space addresses? If… | ||||||||||
| * shouldn't be aligned with their physical value and they are already set by | ||||||||||
Not Done Inline Actions
markj: | ||||||||||
| * pci_emul_init(). | ||||||||||
Not Done Inline Actions
markj: | ||||||||||
| */ | ||||||||||
| intline = pci_get_cfgdata8(pi, PCIR_INTLINE); | ||||||||||
| intpin = pci_get_cfgdata8(pi, PCIR_INTPIN); | ||||||||||
| for (int i = 0; i <= PCIR_MAXLAT; i += 4) { | ||||||||||
| pci_set_cfgdata32(pi, i, read_config(&sc->psc_sel, i, 4)); | ||||||||||
Done Inline ActionsHumm, 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? jhb: Humm, is it really safe to read all these as individual bytes? I would be tempted to read… | ||||||||||
| } | ||||||||||
| pci_set_cfgdata8(pi, PCIR_INTLINE, intline); | ||||||||||
| pci_set_cfgdata8(pi, PCIR_INTPIN, intpin); | ||||||||||
| if (cfginitmsi(sc) != 0) { | if (cfginitmsi(sc) != 0) { | |||||||||
| warnx("failed to initialize MSI for PCI %d/%d/%d", | warnx("failed to initialize MSI for PCI %d/%d/%d", | |||||||||
| bus, slot, func); | bus, slot, func); | |||||||||
| goto done; | goto done; | |||||||||
| } | } | |||||||||
| if (cfginitbar(sc) != 0) { | if (cfginitbar(sc) != 0) { | |||||||||
| warnx("failed to initialize BARs for PCI %d/%d/%d", | warnx("failed to initialize BARs for PCI %d/%d/%d", | |||||||||
| Show All 19 Lines | cfginit(struct pci_devinst *pi, int bus, int slot, int func) | |||||||||
| } | } | |||||||||
| error = 0; /* success */ | error = 0; /* success */ | |||||||||
| done: | done: | |||||||||
| return (error); | return (error); | |||||||||
| } | } | |||||||||
| int | int | |||||||||
| set_pcir_handler(struct passthru_softc *sc, int reg, int len, | set_pcir_handler(struct passthru_softc *sc, int reg, int len, | |||||||||
Done Inline Actions80 cols? jhb: 80 cols? | ||||||||||
| cfgread_handler rhandler, cfgwrite_handler whandler) | cfgread_handler rhandler, cfgwrite_handler whandler) | |||||||||
| { | { | |||||||||
| if (reg > PCI_REGMAX || reg + len > PCI_REGMAX + 1) | if (reg > PCI_REGMAX || reg + len > PCI_REGMAX + 1) | |||||||||
| return (-1); | return (-1); | |||||||||
| for (int i = reg; i < reg + len; ++i) { | for (int i = reg; i < reg + len; ++i) { | |||||||||
| assert(sc->psc_pcir_rhandler[i] == NULL || rhandler == NULL); | assert(sc->psc_pcir_rhandler[i] == NULL || rhandler == NULL); | |||||||||
| assert(sc->psc_pcir_whandler[i] == NULL || whandler == NULL); | assert(sc->psc_pcir_whandler[i] == NULL || whandler == NULL); | |||||||||
| ▲ Show 20 Lines • Show All 212 Lines • ▼ Show 20 Lines | #define GET_INT_CONFIG(var, name) do { \ | |||||||||
| if ((error = cfginit(pi, bus, slot, func)) != 0) | if ((error = cfginit(pi, bus, slot, func)) != 0) | |||||||||
| goto done; | goto done; | |||||||||
| /* initialize ROM */ | /* initialize ROM */ | |||||||||
| if ((error = passthru_init_rom(sc, | if ((error = passthru_init_rom(sc, | |||||||||
| get_config_value_node(nvl, "rom"))) != 0) | get_config_value_node(nvl, "rom"))) != 0) | |||||||||
| goto done; | goto done; | |||||||||
| /* Emulate most PCI header register. */ | ||||||||||
Done Inline ActionsMaybe "Default PCI config registers to accessing the config register of the physical device." jhb: Maybe "Default PCI config registers to accessing the config register of the physical device." | ||||||||||
| if ((error = set_pcir_handler(sc, 0, PCIR_MAXLAT + 1, | ||||||||||
Not Done Inline ActionsBut now we don't emulate accesses to fields below PCIR_BAR(0)? markj: But now we don't emulate accesses to fields below PCIR_BAR(0)? | ||||||||||
Not Done Inline ActionsI 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. corvink: I should improve my commit message.
Except the command and status register, the PCI header… | ||||||||||
| passthru_cfgread_emulate, passthru_cfgwrite_emulate)) != 0) | ||||||||||
| goto done; | ||||||||||
Done Inline ActionsThe whitespace seems inconsistent here and in the clauses below? jhb: The whitespace seems inconsistent here and in the clauses below? | ||||||||||
Done Inline ActionsNewlines before comments. Also, I would maybe reword this somewhat to be more like "Emulate most PCI header registers", jhb: Newlines before comments.
Also, I would maybe reword this somewhat to be more like "Emulate… | ||||||||||
| /* Allow access to the physical command and status register. */ | ||||||||||
| if ((error = set_pcir_handler(sc, PCIR_COMMAND, 0x04, NULL, NULL)) != 0) | ||||||||||
| goto done; | ||||||||||
| error = 0; /* success */ | error = 0; /* success */ | |||||||||
| done: | done: | |||||||||
| if (error) { | if (error) { | |||||||||
| free(sc); | free(sc); | |||||||||
| vm_unassign_pptdev(pi->pi_vmctx, bus, slot, func); | vm_unassign_pptdev(pi->pi_vmctx, bus, slot, func); | |||||||||
| } | } | |||||||||
| return (error); | return (error); | |||||||||
| } | } | |||||||||
| static int | static int | |||||||||
| bar_access(int coff) | ||||||||||
| { | ||||||||||
| if ((coff >= PCIR_BAR(0) && coff < PCIR_BAR(PCI_BARMAX + 1)) || | ||||||||||
| coff == PCIR_BIOS) | ||||||||||
| return (1); | ||||||||||
| else | ||||||||||
| return (0); | ||||||||||
| } | ||||||||||
| static int | ||||||||||
| msicap_access(struct passthru_softc *sc, int coff) | msicap_access(struct passthru_softc *sc, int coff) | |||||||||
| { | { | |||||||||
| int caplen; | int caplen; | |||||||||
| if (sc->psc_msi.capoff == 0) | if (sc->psc_msi.capoff == 0) | |||||||||
| return (0); | return (0); | |||||||||
| caplen = msi_caplen(sc->psc_msi.msgctrl); | caplen = msi_caplen(sc->psc_msi.msgctrl); | |||||||||
| Show All 14 Lines | return (coff >= sc->psc_msix.capoff && | |||||||||
| coff < sc->psc_msix.capoff + MSIX_CAPLEN); | coff < sc->psc_msix.capoff + MSIX_CAPLEN); | |||||||||
| } | } | |||||||||
| static int | static int | |||||||||
| passthru_cfgread_default(struct passthru_softc *sc, | passthru_cfgread_default(struct passthru_softc *sc, | |||||||||
| struct pci_devinst *pi __unused, int coff, int bytes, uint32_t *rv) | struct pci_devinst *pi __unused, int coff, int bytes, uint32_t *rv) | |||||||||
| { | { | |||||||||
| /* | /* | |||||||||
| * PCI BARs and MSI capability is emulated. | * MSI capability is emulated. | |||||||||
| */ | */ | |||||||||
| if (bar_access(coff) || msicap_access(sc, coff) || | if (msicap_access(sc, coff) || msixcap_access(sc, coff)) | |||||||||
| msixcap_access(sc, coff)) | ||||||||||
| return (-1); | return (-1); | |||||||||
Done Inline Actions()'s around the "argument" to return jhb: ()'s around the "argument" to return | ||||||||||
| #ifdef LEGACY_SUPPORT | ||||||||||
| /* | /* | |||||||||
Done Inline ActionsHummm, 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? jhb: Hummm, shouldn't these be handled by setting the ranges for these config registers to the… | ||||||||||
| * Emulate PCIR_CAP_PTR if this device does not support MSI capability | ||||||||||
| * natively. | ||||||||||
| */ | ||||||||||
| if (sc->psc_msi.emulated) { | ||||||||||
Done Inline ActionsHow 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? jhb: How is this handled now? Seems like you need to add a new call to set emulated hooks for this… | ||||||||||
| if (coff >= PCIR_CAP_PTR && coff < PCIR_CAP_PTR + 4) | ||||||||||
| return (-1); | ||||||||||
| } | ||||||||||
| #endif | ||||||||||
| /* | ||||||||||
| * Emulate the command register. If a single read reads both the | * Emulate the command register. If a single read reads both the | |||||||||
Done Inline ActionsI think it would be beneficial for this patch to move this into a separate function. jhb: I think it would be beneficial for this patch to move this into a separate function. | ||||||||||
| * command and status registers, read the status register from the | * command and status registers, read the status register from the | |||||||||
| * device's config space. | * device's config space. | |||||||||
| */ | */ | |||||||||
| if (coff == PCIR_COMMAND) { | if (coff == PCIR_COMMAND) { | |||||||||
| if (bytes <= 2) | if (bytes <= 2) | |||||||||
| return (-1); | return (-1); | |||||||||
| *rv = read_config(&sc->psc_sel, PCIR_STATUS, 2) << 16 | | *rv = read_config(&sc->psc_sel, PCIR_STATUS, 2) << 16 | | |||||||||
| pci_get_cfgdata16(pi, PCIR_COMMAND); | pci_get_cfgdata16(pi, PCIR_COMMAND); | |||||||||
| return (0); | return (0); | |||||||||
| } | } | |||||||||
| /* Everything else just read from the device's config space */ | /* Everything else just read from the device's config space */ | |||||||||
| *rv = read_config(&sc->psc_sel, coff, bytes); | *rv = read_config(&sc->psc_sel, coff, bytes); | |||||||||
| return (0); | return (0); | |||||||||
| } | } | |||||||||
| int | ||||||||||
| passthru_cfgread_emulate(struct passthru_softc *sc __unused, | ||||||||||
| struct pci_devinst *pi __unused, int coff __unused, int bytes __unused, | ||||||||||
| uint32_t *rv __unused) | ||||||||||
| { | ||||||||||
| return (-1); | ||||||||||
| } | ||||||||||
| static int | static int | |||||||||
| passthru_cfgread(struct pci_devinst *pi, int coff, int bytes, uint32_t *rv) | passthru_cfgread(struct pci_devinst *pi, int coff, int bytes, uint32_t *rv) | |||||||||
| { | { | |||||||||
| struct passthru_softc *sc; | struct passthru_softc *sc; | |||||||||
| sc = pi->pi_arg; | sc = pi->pi_arg; | |||||||||
| if (sc->psc_pcir_rhandler[coff] != NULL) | if (sc->psc_pcir_rhandler[coff] != NULL) | |||||||||
| return (sc->psc_pcir_rhandler[coff](sc, pi, coff, bytes, rv)); | return (sc->psc_pcir_rhandler[coff](sc, pi, coff, bytes, rv)); | |||||||||
| return (passthru_cfgread_default(sc, pi, coff, bytes, rv)); | return (passthru_cfgread_default(sc, pi, coff, bytes, rv)); | |||||||||
| } | } | |||||||||
| static int | static int | |||||||||
| passthru_cfgwrite_default(struct passthru_softc *sc, struct pci_devinst *pi, | passthru_cfgwrite_default(struct passthru_softc *sc, struct pci_devinst *pi, | |||||||||
| int coff, int bytes, uint32_t val) | int coff, int bytes, uint32_t val) | |||||||||
| { | { | |||||||||
| int error, msix_table_entries, i; | int error, msix_table_entries, i; | |||||||||
| uint16_t cmd_old; | uint16_t cmd_old; | |||||||||
| /* | /* | |||||||||
| * PCI BARs are emulated | ||||||||||
| */ | ||||||||||
| if (bar_access(coff)) | ||||||||||
| return (-1); | ||||||||||
| /* | ||||||||||
| * MSI capability is emulated | * MSI capability is emulated | |||||||||
| */ | */ | |||||||||
| if (msicap_access(sc, coff)) { | if (msicap_access(sc, coff)) { | |||||||||
| pci_emul_capwrite(pi, coff, bytes, val, sc->psc_msi.capoff, | pci_emul_capwrite(pi, coff, bytes, val, sc->psc_msi.capoff, | |||||||||
| PCIY_MSI); | PCIY_MSI); | |||||||||
| error = vm_setup_pptdev_msi(pi->pi_vmctx, sc->psc_sel.pc_bus, | error = vm_setup_pptdev_msi(pi->pi_vmctx, sc->psc_sel.pc_bus, | |||||||||
| sc->psc_sel.pc_dev, sc->psc_sel.pc_func, | sc->psc_sel.pc_dev, sc->psc_sel.pc_func, | |||||||||
| pi->pi_msi.addr, pi->pi_msi.msg_data, | pi->pi_msi.addr, pi->pi_msi.msg_data, | |||||||||
| ▲ Show 20 Lines • Show All 47 Lines • ▼ Show 20 Lines | if (coff == PCIR_COMMAND) { | |||||||||
| if (bytes == 1) | if (bytes == 1) | |||||||||
| pci_set_cfgdata8(pi, PCIR_COMMAND, val); | pci_set_cfgdata8(pi, PCIR_COMMAND, val); | |||||||||
| else if (bytes == 2) | else if (bytes == 2) | |||||||||
| pci_set_cfgdata16(pi, PCIR_COMMAND, val); | pci_set_cfgdata16(pi, PCIR_COMMAND, val); | |||||||||
| pci_emul_cmd_changed(pi, cmd_old); | pci_emul_cmd_changed(pi, cmd_old); | |||||||||
| } | } | |||||||||
| return (0); | return (0); | |||||||||
| } | ||||||||||
| int | ||||||||||
| passthru_cfgwrite_emulate(struct passthru_softc *sc __unused, | ||||||||||
| struct pci_devinst *pi __unused, int coff __unused, int bytes __unused, | ||||||||||
| uint32_t val __unused) | ||||||||||
| { | ||||||||||
| return (-1); | ||||||||||
| } | } | |||||||||
| static int | static int | |||||||||
| passthru_cfgwrite(struct pci_devinst *pi, int coff, int bytes, uint32_t val) | passthru_cfgwrite(struct pci_devinst *pi, int coff, int bytes, uint32_t val) | |||||||||
| { | { | |||||||||
| struct passthru_softc *sc; | struct passthru_softc *sc; | |||||||||
| sc = pi->pi_arg; | sc = pi->pi_arg; | |||||||||
| ▲ Show 20 Lines • Show All 197 Lines • Show Last 20 Lines | ||||||||||
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.