Page MenuHomeFreeBSD
Differential D33129 Diff 103715 usr.sbin/bhyve/pci_passthru.c

Changeset View

Standalone View

View Options

usr.sbin/bhyve/pci_passthru.c

Show All 33 Lines
#include <sys/param.h> #include <sys/param.h>
#ifndef WITHOUT_CAPSICUM #ifndef WITHOUT_CAPSICUM
#include <sys/capsicum.h> #include <sys/capsicum.h>
#endif #endif
#include <sys/types.h> #include <sys/types.h>
#include <sys/mman.h> #include <sys/mman.h>
#include <sys/pciio.h> #include <sys/pciio.h>
#include <sys/ioctl.h> #include <sys/ioctl.h>
#include <sys/stat.h>
#include <dev/io/iodev.h> #include <dev/io/iodev.h>
#include <dev/pci/pcireg.h> #include <dev/pci/pcireg.h>
#include <vm/vm.h> #include <vm/vm.h>
#include <machine/iodev.h> #include <machine/iodev.h>
#include <machine/vm.h> #include <machine/vm.h>
Show All 25 Lines
#define MSIX_TABLE_COUNT(ctrl) (((ctrl) & PCIM_MSIXCTRL_TABLE_SIZE) + 1) #define MSIX_TABLE_COUNT(ctrl) (((ctrl) & PCIM_MSIXCTRL_TABLE_SIZE) + 1)
#define MSIX_CAPLEN 12 #define MSIX_CAPLEN 12
static int pcifd = -1; static int pcifd = -1;
struct passthru_softc { struct passthru_softc {
struct pci_devinst *psc_pi; struct pci_devinst *psc_pi;
struct pcibar psc_bar[PCI_BARMAX + 1]; /* ROM is handled like a BAR */
struct pcibar psc_bar[PCI_BARMAX_WITH_ROM + 1];
struct { struct {
int capoff; int capoff;
int msgctrl; int msgctrl;
int emulated; int emulated;
} psc_msi; } psc_msi;
struct { struct {
int capoff; int capoff;
} psc_msix; } psc_msix;
▲ Show 20 Lines • Show All 562 Lines • ▼ Show 20 Linespassthru_legacy_config(nvlist_t *nvl, const char *opts)
} }
snprintf(value, sizeof(value), "%d", bus); snprintf(value, sizeof(value), "%d", bus);
set_config_value_node(nvl, "bus", value); set_config_value_node(nvl, "bus", value);
snprintf(value, sizeof(value), "%d", slot); snprintf(value, sizeof(value), "%d", slot);
set_config_value_node(nvl, "slot", value); set_config_value_node(nvl, "slot", value);
snprintf(value, sizeof(value), "%d", func); snprintf(value, sizeof(value), "%d", func);
set_config_value_node(nvl, "func", value); set_config_value_node(nvl, "func", value);
return (pci_parse_legacy_config(nvl, strchr(opts, ',')));
markjUnsubmitted
Done
Inline Actions
set_config_value_node(nvl, "func", value);
- return pci_parse_legacy_config(nvl, strchr(opts, ','));
+ return (pci_parse_legacy_config(nvl, strchr(opts, ',')));
}
static int
markj:
}
static int
passthru_init_rom(struct vmctx *const ctx, struct passthru_softc *const sc,
const char *const romfile)
{
if (romfile == NULL) {
return (0); return (0);
} }
const int fd = open(romfile, O_RDONLY);
if (fd < 0) {
warnx("%s: can't open romfile \"%s\"", __func__, romfile);
return (-1);
}
struct stat sbuf;
if (fstat(fd, &sbuf) < 0) {
markjUnsubmitted
Done
Inline Actions

Use fstat() to get the file size.

markj: Use fstat() to get the file size.
warnx("%s: can't fstat romfile \"%s\"", __func__, romfile);
close(fd);
return (-1);
}
const uint64_t rom_size = sbuf.st_size;
void *const rom_data = mmap(NULL, rom_size, PROT_READ, MAP_SHARED, fd,
markjUnsubmitted
Done
Inline Actions

Why the negative errno values?

markj: Why the negative errno values?
0);
if (rom_data == MAP_FAILED) {
warnx("%s: unable to mmap romfile \"%s\" (%d)", __func__,
romfile, errno);
close(fd);
return (-1);
}
void *rom_addr;
int error = pci_emul_alloc_rom(sc->psc_pi, rom_size, &rom_addr);
markjUnsubmitted
Done
Inline Actions

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

markj: Some of the error paths leak the fd and the temporary mapping.
if (error) {
warnx("%s: failed to alloc rom segment", __func__);
munmap(rom_data, rom_size);
close(fd);
return (error);
}
memcpy(rom_addr, rom_data, rom_size);
markjUnsubmitted
Done
Inline Actions

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

markj: I think you can just mmap the ROM file and copy it directly, no? Seems like that would be…
sc->psc_bar[PCI_ROM_IDX].type = PCIBAR_ROM;
sc->psc_bar[PCI_ROM_IDX].addr = (uint64_t)rom_addr;
sc->psc_bar[PCI_ROM_IDX].size = rom_size;
munmap(rom_data, rom_size);
close(fd);
return (0);
}
static int static int
passthru_init(struct vmctx *ctx, struct pci_devinst *pi, nvlist_t *nvl) passthru_init(struct vmctx *ctx, struct pci_devinst *pi, nvlist_t *nvl)
{ {
int bus, slot, func, error, memflags; int bus, slot, func, error, memflags;
struct passthru_softc *sc; struct passthru_softc *sc;
const char *value; const char *value;
sc = NULL; sc = NULL;
Show All 29 Lines#define GET_INT_CONFIG(var, name) do { \
} }
sc = calloc(1, sizeof(struct passthru_softc)); sc = calloc(1, sizeof(struct passthru_softc));
pi->pi_arg = sc; pi->pi_arg = sc;
sc->psc_pi = pi; sc->psc_pi = pi;
/* initialize config space */ /* initialize config space */
error = cfginit(ctx, pi, bus, slot, func); if ((error = cfginit(ctx, pi, bus, slot, func)) != 0)
goto done;
/* initialize ROM */
if ((error = passthru_init_rom(ctx, sc,
get_config_value_node(nvl, "rom"))) != 0)
markjUnsubmitted
Done
Inline Actions
if ((error = passthru_init_rom(ctx, sc,
- get_config_value_node(nvl, "rom"))) != 0)
+ get_config_value_node(nvl, "rom"))) != 0)
goto done;
markj:
corvinkAuthorUnsubmitted
Done
Inline Actions

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

corvink: Hmmm. I'm using autoformatting based on the `.clang-format` in current sources. It creates my…
goto done;
error = 0; /* success */
done: done:
if (error) { if (error) {
free(sc); free(sc);
vm_unassign_pptdev(ctx, bus, slot, func); vm_unassign_pptdev(ctx, bus, slot, func);
} }
return (error); return (error);
} }
static int static int
bar_access(int coff) bar_access(int coff)
{ {
if (coff >= PCIR_BAR(0) && coff < PCIR_BAR(PCI_BARMAX + 1)) if ((coff >= PCIR_BAR(0) && coff < PCIR_BAR(PCI_BARMAX + 1)) ||
coff == PCIR_BIOS)
return (1); return (1);
else else
return (0); return (0);
} }
static int static int
msicap_access(struct passthru_softc *sc, int coff) msicap_access(struct passthru_softc *sc, int coff)
{ {
▲ Show 20 Lines • Show All 275 Lines • ▼ Show 20 Lines if (vm_map_pptdev_mmio(ctx, sc->psc_sel.pc_bus,
sc->psc_sel.pc_func, address, sc->psc_sel.pc_func, address,
sc->psc_bar[baridx].size, sc->psc_bar[baridx].size,
sc->psc_bar[baridx].addr) != 0) sc->psc_bar[baridx].addr) != 0)
warnx("pci_passthru: map_pptdev_mmio failed"); warnx("pci_passthru: map_pptdev_mmio failed");
} }
} }
static void static void
passthru_addr_rom(struct pci_devinst *const pi, const int idx,
const int enabled)
{
const uint64_t addr = pi->pi_bar[idx].addr;
const uint64_t size = pi->pi_bar[idx].size;
if (!enabled) {
if (vm_munmap_memseg(pi->pi_vmctx, addr, size) != 0) {
errx(4, "%s: munmap_memseg @ [%016lx - %016lx] failed",
__func__, addr, addr + size);
markjUnsubmitted
Done
Inline Actions

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

markj: There's no error checking. Why is that ok?
markjUnsubmitted
Done
Inline Actions

Shouldn't it be a fatal error?

markj: Shouldn't it be a fatal error?
corvinkAuthorUnsubmitted
Done
Inline Actions

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?

corvink: I've used the same style like `passthru_mmio_addr`. I'm not sure if it should be a fatal error…
corvinkAuthorUnsubmitted
Done
Inline Actions

@markj Should I change it to a fatal error?

corvink: @markj Should I change it to a fatal error?
corvinkAuthorUnsubmitted
Done
Inline Actions

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.

corvink: It's a fatal error now. If someone has issue with that due to a buggy guest, it'll be easy to…
}
} else {
if (vm_mmap_memseg(pi->pi_vmctx, addr, VM_PCIROM,
pi->pi_romoffset, size, PROT_READ | PROT_EXEC) != 0) {
errx(4, "%s: mnmap_memseg @ [%016lx - %016lx] failed",
__func__, addr, addr + size);
}
}
}
static void
passthru_addr(struct vmctx *ctx, struct pci_devinst *pi, int baridx, passthru_addr(struct vmctx *ctx, struct pci_devinst *pi, int baridx,
int enabled, uint64_t address) int enabled, uint64_t address)
{ {
switch (pi->pi_bar[baridx].type) {
if (pi->pi_bar[baridx].type == PCIBAR_IO) case PCIBAR_IO:
return; /* IO BARs are emulated */
break;
case PCIBAR_ROM:
passthru_addr_rom(pi, baridx, enabled);
break;
case PCIBAR_MEM32:
case PCIBAR_MEM64:
if (baridx == pci_msix_table_bar(pi)) if (baridx == pci_msix_table_bar(pi))
passthru_msix_addr(ctx, pi, baridx, enabled, address); passthru_msix_addr(ctx, pi, baridx, enabled, address);
else else
passthru_mmio_addr(ctx, pi, baridx, enabled, address); passthru_mmio_addr(ctx, pi, baridx, enabled, address);
break;
default:
errx(4, "%s: invalid BAR type %d", __func__,
pi->pi_bar[baridx].type);
}
} }
struct pci_devemu passthru = { struct pci_devemu passthru = {
.pe_emu = "passthru", .pe_emu = "passthru",
.pe_init = passthru_init, .pe_init = passthru_init,
.pe_legacy_config = passthru_legacy_config, .pe_legacy_config = passthru_legacy_config,
.pe_cfgwrite = passthru_cfgwrite, .pe_cfgwrite = passthru_cfgwrite,
.pe_cfgread = passthru_cfgread, .pe_cfgread = passthru_cfgread,
.pe_barwrite = passthru_write, .pe_barwrite = passthru_write,
.pe_barread = passthru_read, .pe_barread = passthru_read,
.pe_baraddr = passthru_addr, .pe_baraddr = passthru_addr,
}; };
PCI_EMUL_SET(passthru); PCI_EMUL_SET(passthru);