This series adds some helper functions for working with PCI BARs (Base Address Registers), and it then updates all the drivers to use these helper functions. The new helper functions perform sanity checks on the BARs and make sure the appropriate pci command register flags are enabled. The existing driver code was inconsistent with these checks.
At the end of this series, the pci->have_driver flag is also consistently set on any PCI device that there is a SeaBIOS driver for. This will make sure SeaBIOS does not attempt to load an option rom for a device that SeaBIOS already has a native driver for.
This series is also available at:
https://github.com/KevinOConnor/seabios/tree/testing
-Kevin
Kevin O'Connor (14): pci: Add helper functions for internal driver BAR handling ahci: Convert to new PCI BAR helper functions ata: Convert to new PCI BAR helper functions esp-scsi: Convert to new PCI BAR helper functions lsi-scsi: Convert to new PCI BAR helper functions megasas: Convert to new PCI BAR helper functions pvscsi: Convert to new PCI BAR helper functions sdcard: Convert to new PCI BAR helper functions ehci: Convert to new PCI BAR helper functions ohci: Convert to new PCI BAR helper functions uhci: Convert to new PCI BAR helper functions xhci: Convert to new PCI BAR helper functions virtio: Convert to new PCI BAR helper functions pci: Consistently set pci->have_drivers for devices with internal drivers
src/hw/ahci.c | 35 ++++++++++++++---------------- src/hw/ahci.h | 3 +-- src/hw/ata.c | 26 +++++++++++------------ src/hw/esp-scsi.c | 13 ++++++------ src/hw/lsi-scsi.c | 13 ++++++------ src/hw/megasas.c | 18 +++++++--------- src/hw/pci.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++ src/hw/pci.h | 3 +++ src/hw/pvscsi.c | 17 +++++++-------- src/hw/sdcard.c | 15 ++++++------- src/hw/usb-ehci.c | 12 +++++------ src/hw/usb-ohci.c | 17 +++++++-------- src/hw/usb-uhci.c | 16 ++++++++------ src/hw/usb-xhci.c | 27 ++++++++++++------------ src/hw/virtio-pci.c | 36 +++++++++++++++++-------------- src/hw/virtio-pci.h | 32 ++++++++++++++++------------ 16 files changed, 203 insertions(+), 141 deletions(-)
Add functions to verify and obtain PCI BARs (Base Address Registers). These new functions check that the requested BAR is of the right type and appears valid.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/pci.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/hw/pci.h | 3 +++ 2 files changed, 61 insertions(+)
diff --git a/src/hw/pci.c b/src/hw/pci.c index a241d06..86b7d54 100644 --- a/src/hw/pci.c +++ b/src/hw/pci.c @@ -10,6 +10,7 @@ #include "pci.h" // pci_config_writel #include "pci_regs.h" // PCI_VENDOR_ID #include "romfile.h" // romfile_loadint +#include "stacks.h" // wait_preempt #include "string.h" // memset #include "util.h" // udelay #include "x86.h" // outl @@ -271,6 +272,63 @@ int pci_bridge_has_region(struct pci_device *pci, return pci_config_readb(pci->bdf, base) != 0; }
+// Enable PCI bus-mastering (ie, DMA) support on a pci device +void +pci_enable_busmaster(struct pci_device *pci) +{ + ASSERT32FLAT(); + wait_preempt(); + pci_config_maskw(pci->bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER); +} + +// Verify an IO bar and return it to the caller +u16 +pci_enable_iobar(struct pci_device *pci, u32 addr) +{ + ASSERT32FLAT(); + wait_preempt(); + u32 bar = pci_config_readl(pci->bdf, addr); + if (!(bar & PCI_BASE_ADDRESS_SPACE_IO)) { + warn_internalerror(); + return 0; + } + bar &= PCI_BASE_ADDRESS_IO_MASK; + if (bar == 0 || bar > 0xffff) { + warn_internalerror(); + return 0; + } + pci_config_maskw(pci->bdf, PCI_COMMAND, 0, PCI_COMMAND_IO); + return bar; +} + +// Verify a memory bar and return it to the caller +void * +pci_enable_membar(struct pci_device *pci, u32 addr) +{ + ASSERT32FLAT(); + wait_preempt(); + u32 bar = pci_config_readl(pci->bdf, addr); + if (bar & PCI_BASE_ADDRESS_SPACE_IO) { + warn_internalerror(); + return NULL; + } + if (bar & PCI_BASE_ADDRESS_MEM_TYPE_64) { + u32 high = pci_config_readl(pci->bdf, addr+4); + if (high) { + dprintf(1, "Can not map memory bar over 4Gig\n"); + return NULL; + } + } + bar &= PCI_BASE_ADDRESS_MEM_MASK; + if (bar + 4*1024*1024 < 20*1024*1024) { + // Bar doesn't look valid (it is in last 4M or first 16M) + warn_internalerror(); + return NULL; + } + pci_config_maskw(pci->bdf, PCI_COMMAND, 0, PCI_COMMAND_MEMORY); + return (void*)bar; +} + void pci_reboot(void) { diff --git a/src/hw/pci.h b/src/hw/pci.h index fc5e7b9..8e39753 100644 --- a/src/hw/pci.h +++ b/src/hw/pci.h @@ -126,6 +126,9 @@ struct pci_device *pci_find_init_device(const struct pci_device_id *ids u8 pci_find_capability(struct pci_device *pci, u8 cap_id, u8 cap); int pci_bridge_has_region(struct pci_device *pci, enum pci_region_type region_type); +void pci_enable_busmaster(struct pci_device *pci); +u16 pci_enable_iobar(struct pci_device *pci, u32 addr); +void *pci_enable_membar(struct pci_device *pci, u32 addr); void pci_reboot(void);
#endif
Use the pci_enable_x() functions.
This patch also converts cntl->iobase from a 'u32' to a 'void*' so that it is clear that the address is a virtual memory address.
After this change, the AHCI driver will no longer enable PCI_COMMAND_IO io accesses, as the AHCI driver doesn't actually attempt IO accesses to the device.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/ahci.c | 35 ++++++++++++++++------------------- src/hw/ahci.h | 3 +-- 2 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/src/hw/ahci.c b/src/hw/ahci.c index 83b747c..5401cca 100644 --- a/src/hw/ahci.c +++ b/src/hw/ahci.c @@ -71,14 +71,12 @@ static void sata_prep_atapi(struct sata_cmd_fis *fis, u16 blocksize) // ahci register access helpers static u32 ahci_ctrl_readl(struct ahci_ctrl_s *ctrl, u32 reg) { - u32 addr = ctrl->iobase + reg; - return readl((void*)addr); + return readl(ctrl->iobase + reg); }
static void ahci_ctrl_writel(struct ahci_ctrl_s *ctrl, u32 reg, u32 val) { - u32 addr = ctrl->iobase + reg; - writel((void*)addr, val); + writel(ctrl->iobase + reg, val); }
static u32 ahci_port_to_ctrl(u32 pnr, u32 port_reg) @@ -567,31 +565,30 @@ ahci_port_detect(void *data) static void ahci_controller_setup(struct pci_device *pci) { - struct ahci_ctrl_s *ctrl = malloc_fseg(sizeof(*ctrl)); struct ahci_port_s *port; - u16 bdf = pci->bdf; u32 val, pnr, max;
- if (!ctrl) { - warn_noalloc(); + if (create_bounce_buf() < 0) return; - }
- if (create_bounce_buf() < 0) { + void *iobase = pci_enable_membar(pci, PCI_BASE_ADDRESS_5); + if (!iobase) + return; + + struct ahci_ctrl_s *ctrl = malloc_fseg(sizeof(*ctrl)); + if (!ctrl) { warn_noalloc(); - free(ctrl); return; }
ctrl->pci_tmp = pci; - ctrl->pci_bdf = bdf; - ctrl->iobase = pci_config_readl(bdf, PCI_BASE_ADDRESS_5); - ctrl->irq = pci_config_readb(bdf, PCI_INTERRUPT_LINE); - dprintf(1, "AHCI controller at %02x.%x, iobase %x, irq %d\n", - bdf >> 3, bdf & 7, ctrl->iobase, ctrl->irq); - - pci_config_maskw(bdf, PCI_COMMAND, 0, - PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER); + ctrl->iobase = iobase; + ctrl->irq = pci_config_readb(pci->bdf, PCI_INTERRUPT_LINE); + dprintf(1, "AHCI controller at %02x:%02x.%x, iobase %p, irq %d\n" + , pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf) + , pci_bdf_to_fn(pci->bdf), ctrl->iobase, ctrl->irq); + + pci_enable_busmaster(pci);
val = ahci_ctrl_readl(ctrl, HOST_CTL); ahci_ctrl_writel(ctrl, HOST_CTL, val | HOST_CTL_AHCI_EN); diff --git a/src/hw/ahci.h b/src/hw/ahci.h index fa11d66..5c4f6e1 100644 --- a/src/hw/ahci.h +++ b/src/hw/ahci.h @@ -30,9 +30,8 @@ struct sata_cmd_fis {
struct ahci_ctrl_s { struct pci_device *pci_tmp; - u16 pci_bdf; u8 irq; - u32 iobase; + void *iobase; u32 caps; u32 ports; };
Use the pci_enable_x() functions.
The ATA controller code will now explicitly set PCI_COMMAND_IO instead of assuming it has already been enabled.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/ata.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/src/hw/ata.c b/src/hw/ata.c index fbbbbc1..7aaf2f1 100644 --- a/src/hw/ata.c +++ b/src/hw/ata.c @@ -946,24 +946,23 @@ static void init_pciata(struct pci_device *pci, u8 prog_if) { pci->have_driver = 1; - u16 bdf = pci->bdf; - u8 pciirq = pci_config_readb(bdf, PCI_INTERRUPT_LINE); + u8 pciirq = pci_config_readb(pci->bdf, PCI_INTERRUPT_LINE); int master = 0; if (CONFIG_ATA_DMA && prog_if & 0x80) { // Check for bus-mastering. - u32 bar = pci_config_readl(bdf, PCI_BASE_ADDRESS_4); + u32 bar = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_4); if (bar & PCI_BASE_ADDRESS_SPACE_IO) { - master = bar & PCI_BASE_ADDRESS_IO_MASK; - pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER); + master = pci_enable_iobar(pci, PCI_BASE_ADDRESS_4); + pci_enable_busmaster(pci); } }
u32 port1, port2, irq; if (prog_if & 1) { - port1 = (pci_config_readl(bdf, PCI_BASE_ADDRESS_0) - & PCI_BASE_ADDRESS_IO_MASK); - port2 = (pci_config_readl(bdf, PCI_BASE_ADDRESS_1) - & PCI_BASE_ADDRESS_IO_MASK); + port1 = pci_enable_iobar(pci, PCI_BASE_ADDRESS_0); + port2 = pci_enable_iobar(pci, PCI_BASE_ADDRESS_1); + if (!port1 || !port2) + return; irq = pciirq; } else { port1 = PORT_ATA1_CMD_BASE; @@ -973,10 +972,10 @@ init_pciata(struct pci_device *pci, u8 prog_if) init_controller(pci, 0, irq, port1, port2, master);
if (prog_if & 4) { - port1 = (pci_config_readl(bdf, PCI_BASE_ADDRESS_2) - & PCI_BASE_ADDRESS_IO_MASK); - port2 = (pci_config_readl(bdf, PCI_BASE_ADDRESS_3) - & PCI_BASE_ADDRESS_IO_MASK); + port1 = pci_enable_iobar(pci, PCI_BASE_ADDRESS_2); + port2 = pci_enable_iobar(pci, PCI_BASE_ADDRESS_3); + if (!port1 || !port2) + return; irq = pciirq; } else { port1 = PORT_ATA2_CMD_BASE;
Use the pci_enable_x() functions.
The esp-scsi controller code will now explicitly set PCI_COMMAND_IO instead of assuming it has already been enabled.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/esp-scsi.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/src/hw/esp-scsi.c b/src/hw/esp-scsi.c index d4e47e3..cf2cc11 100644 --- a/src/hw/esp-scsi.c +++ b/src/hw/esp-scsi.c @@ -192,15 +192,14 @@ esp_scsi_scan_target(struct pci_device *pci, u32 iobase, u8 target) static void init_esp_scsi(struct pci_device *pci) { - u16 bdf = pci->bdf; - u32 iobase = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0) - & PCI_BASE_ADDRESS_IO_MASK; + u32 iobase = pci_enable_iobar(pci, PCI_BASE_ADDRESS_0); + if (!iobase) + return; + pci_enable_busmaster(pci);
dprintf(1, "found esp at %02x:%02x.%x, io @ %x\n", - pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), - pci_bdf_to_fn(bdf), iobase); - - pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER); + pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf), + pci_bdf_to_fn(pci->bdf), iobase);
// reset outb(ESP_CMD_RESET, iobase + ESP_CMD);
Use the pci_enable_x() functions.
The lsi-scsi controller code will now explicitly set PCI_COMMAND_IO instead of assuming it has already been enabled.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/lsi-scsi.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/src/hw/lsi-scsi.c b/src/hw/lsi-scsi.c index ad33528..2629b5f 100644 --- a/src/hw/lsi-scsi.c +++ b/src/hw/lsi-scsi.c @@ -172,15 +172,14 @@ lsi_scsi_scan_target(struct pci_device *pci, u32 iobase, u8 target) static void init_lsi_scsi(struct pci_device *pci) { - u16 bdf = pci->bdf; - u32 iobase = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0) - & PCI_BASE_ADDRESS_IO_MASK; - - pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER); + u32 iobase = pci_enable_iobar(pci, PCI_BASE_ADDRESS_0); + if (!iobase) + return; + pci_enable_busmaster(pci);
dprintf(1, "found lsi53c895a at %02x:%02x.%x, io @ %x\n", - pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), - pci_bdf_to_fn(bdf), iobase); + pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf), + pci_bdf_to_fn(pci->bdf), iobase);
// reset outb(LSI_ISTAT0_SRST, iobase + LSI_REG_ISTAT0);
Use the pci_enable_x() functions.
After this change, the megasas driver will no longer enable PCI_COMMAND_MEMORY accesses, as the megasas driver doesn't actually map any BARs as memory.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/megasas.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/src/hw/megasas.c b/src/hw/megasas.c index cb1a2a6..31426e7 100644 --- a/src/hw/megasas.c +++ b/src/hw/megasas.c @@ -359,20 +359,18 @@ static int megasas_transition_to_ready(struct pci_device *pci, u32 ioaddr) static void init_megasas(struct pci_device *pci) { - u16 bdf = pci->bdf; - u32 iobase = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_2) - & PCI_BASE_ADDRESS_IO_MASK; - + u32 bar = PCI_BASE_ADDRESS_2; + if (!(pci_config_readl(pci->bdf, bar) & PCI_BASE_ADDRESS_IO_MASK)) + bar = PCI_BASE_ADDRESS_0; + u32 iobase = pci_enable_iobar(pci, bar); if (!iobase) - iobase = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0) - & PCI_BASE_ADDRESS_IO_MASK; + return; + pci_enable_busmaster(pci);
dprintf(1, "found MegaRAID SAS at %02x:%02x.%x, io @ %x\n", - pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), - pci_bdf_to_fn(bdf), iobase); + pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf), + pci_bdf_to_fn(pci->bdf), iobase);
- pci_config_maskw(pci->bdf, PCI_COMMAND, 0, - PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER); // reset if (megasas_transition_to_ready(pci, iobase) == 0) megasas_scan_target(pci, iobase);
Use the pci_enable_x() functions.
The pvscsi controller code will now explicitly set PCI_COMMAND_MEMORY instead of assuming it has already been enabled.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/pvscsi.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/src/hw/pvscsi.c b/src/hw/pvscsi.c index fa20efe..101a9c5 100644 --- a/src/hw/pvscsi.c +++ b/src/hw/pvscsi.c @@ -298,21 +298,20 @@ pvscsi_scan_target(struct pci_device *pci, void *iobase, static void init_pvscsi(struct pci_device *pci) { - struct pvscsi_ring_dsc_s *ring_dsc = NULL; - int i; - u16 bdf = pci->bdf; - void *iobase = (void*)(pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0) - & PCI_BASE_ADDRESS_MEM_MASK); - - pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER); + void *iobase = pci_enable_membar(pci, PCI_BASE_ADDRESS_0); + if (!iobase) + return; + pci_enable_busmaster(pci);
dprintf(1, "found pvscsi at %02x:%02x.%x, io @ %p\n", - pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), - pci_bdf_to_fn(bdf), iobase); + pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf), + pci_bdf_to_fn(pci->bdf), iobase);
pvscsi_write_cmd_desc(iobase, PVSCSI_CMD_ADAPTER_RESET, NULL, 0);
+ struct pvscsi_ring_dsc_s *ring_dsc = NULL; pvscsi_init_rings(iobase, &ring_dsc); + int i; for (i = 0; i < 7; i++) pvscsi_scan_target(pci, iobase, ring_dsc, i);
Use the pci_enable_x() functions.
After this change, the sdcard driver will no longer enable PCI_COMMAND_IO or PCI_COMMAND_MASTER accesses, as the sdcard driver doesn't actually use IO BARs or implement DMA.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/sdcard.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/hw/sdcard.c b/src/hw/sdcard.c index db3bbe1..1524b51 100644 --- a/src/hw/sdcard.c +++ b/src/hw/sdcard.c @@ -7,11 +7,11 @@ #include "block.h" // struct drive_s #include "malloc.h" // malloc_fseg #include "output.h" // znprintf -#include "pci.h" // pci_config_readl +#include "pci.h" // foreachpci #include "pci_ids.h" // PCI_CLASS_SYSTEM_SDHCI #include "pci_regs.h" // PCI_BASE_ADDRESS_0 #include "romfile.h" // romfile_findprefix -#include "stacks.h" // wait_preempt +#include "stacks.h" // yield #include "std/disk.h" // DISK_RET_SUCCESS #include "string.h" // memset #include "util.h" // boot_add_hd @@ -525,14 +525,12 @@ static void sdcard_pci_setup(void *data) { struct pci_device *pci = data; - wait_preempt(); // Avoid pci_config_readl when preempting // XXX - bars dependent on slot index register in pci config space - u32 regs = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0); - regs &= PCI_BASE_ADDRESS_MEM_MASK; - pci_config_maskw(pci->bdf, PCI_COMMAND, 0, - PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER); + struct sdhci_s *regs = pci_enable_membar(pci, PCI_BASE_ADDRESS_0); + if (!regs) + return; int prio = bootprio_find_pci_device(pci); - sdcard_controller_setup((void*)regs, prio); + sdcard_controller_setup(regs, prio); }
static void
Use the pci_enable_x() functions.
The ehci controller code will now explicitly set PCI_COMMAND_MEMORY instead of assuming it has already been enabled.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/usb-ehci.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/hw/usb-ehci.c b/src/hw/usb-ehci.c index a519455..57ce5a8 100644 --- a/src/hw/usb-ehci.c +++ b/src/hw/usb-ehci.c @@ -295,9 +295,9 @@ fail: static void ehci_controller_setup(struct pci_device *pci) { - u16 bdf = pci->bdf; - u32 baseaddr = pci_config_readl(bdf, PCI_BASE_ADDRESS_0); - struct ehci_caps *caps = (void*)(baseaddr & PCI_BASE_ADDRESS_MEM_MASK); + struct ehci_caps *caps = pci_enable_membar(pci, PCI_BASE_ADDRESS_0); + if (!caps) + return; u32 hcc_params = readl(&caps->hccparams);
struct usb_ehci_s *cntl = malloc_tmphigh(sizeof(*cntl)); @@ -316,10 +316,10 @@ ehci_controller_setup(struct pci_device *pci) PendingEHCI++;
dprintf(1, "EHCI init on dev %02x:%02x.%x (regs=%p)\n" - , pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf) - , pci_bdf_to_fn(bdf), cntl->regs); + , pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf) + , pci_bdf_to_fn(pci->bdf), cntl->regs);
- pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER); + pci_enable_busmaster(pci);
// XXX - check for and disable SMM control?
Use the pci_enable_x() functions.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/usb-ohci.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/src/hw/usb-ohci.c b/src/hw/usb-ohci.c index 0c0bf60..3d55d48 100644 --- a/src/hw/usb-ohci.c +++ b/src/hw/usb-ohci.c @@ -268,6 +268,10 @@ free: static void ohci_controller_setup(struct pci_device *pci) { + struct ohci_regs *regs = pci_enable_membar(pci, PCI_BASE_ADDRESS_0); + if (!regs) + return; + struct usb_ohci_s *cntl = malloc_tmphigh(sizeof(*cntl)); if (!cntl) { warn_noalloc(); @@ -276,18 +280,13 @@ ohci_controller_setup(struct pci_device *pci) memset(cntl, 0, sizeof(*cntl)); cntl->usb.pci = pci; cntl->usb.type = USB_TYPE_OHCI; - - u16 bdf = pci->bdf; - u32 baseaddr = pci_config_readl(bdf, PCI_BASE_ADDRESS_0); - cntl->regs = (void*)(baseaddr & PCI_BASE_ADDRESS_MEM_MASK); + cntl->regs = regs;
dprintf(1, "OHCI init on dev %02x:%02x.%x (regs=%p)\n" - , pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf) - , pci_bdf_to_fn(bdf), cntl->regs); + , pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf) + , pci_bdf_to_fn(pci->bdf), cntl->regs);
- // Enable bus mastering and memory access. - pci_config_maskw(bdf, PCI_COMMAND - , 0, PCI_COMMAND_MASTER|PCI_COMMAND_MEMORY); + pci_enable_busmaster(pci);
// XXX - check for and disable SMM control?
Use the pci_enable_x() functions.
The uhci controller code will now explicitly set PCI_COMMAND_IO instead of assuming it has already been enabled.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/usb-uhci.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/hw/usb-uhci.c b/src/hw/usb-uhci.c index 7a11510..3b949e4 100644 --- a/src/hw/usb-uhci.c +++ b/src/hw/usb-uhci.c @@ -244,7 +244,10 @@ fail: static void uhci_controller_setup(struct pci_device *pci) { - u16 bdf = pci->bdf; + u16 iobase = pci_enable_iobar(pci, PCI_BASE_ADDRESS_4); + if (!iobase) + return; + struct usb_uhci_s *cntl = malloc_tmphigh(sizeof(*cntl)); if (!cntl) { warn_noalloc(); @@ -253,16 +256,15 @@ uhci_controller_setup(struct pci_device *pci) memset(cntl, 0, sizeof(*cntl)); cntl->usb.pci = pci; cntl->usb.type = USB_TYPE_UHCI; - cntl->iobase = (pci_config_readl(bdf, PCI_BASE_ADDRESS_4) - & PCI_BASE_ADDRESS_IO_MASK); + cntl->iobase = iobase;
dprintf(1, "UHCI init on dev %02x:%02x.%x (io=%x)\n" - , pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf) - , pci_bdf_to_fn(bdf), cntl->iobase); + , pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf) + , pci_bdf_to_fn(pci->bdf), cntl->iobase);
- pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER); + pci_enable_busmaster(pci);
- reset_uhci(cntl, bdf); + reset_uhci(cntl, pci->bdf);
run_thread(configure_uhci, cntl); }
Use the pci_enable_x() functions.
The xhci controller code will now explicitly set PCI_COMMAND_MEMORY instead of assuming it has already been enabled.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/usb-xhci.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/src/hw/usb-xhci.c b/src/hw/usb-xhci.c index ad541ab..af09592 100644 --- a/src/hw/usb-xhci.c +++ b/src/hw/usb-xhci.c @@ -230,7 +230,6 @@ struct usb_xhci_s { struct usb_s usb;
/* devinfo */ - u32 baseaddr; u32 xcap; u32 ports; u32 slots; @@ -527,20 +526,21 @@ fail: static void xhci_controller_setup(struct pci_device *pci) { + void *baseaddr = pci_enable_membar(pci, PCI_BASE_ADDRESS_0); + if (!baseaddr) + return; + struct usb_xhci_s *xhci = malloc_high(sizeof(*xhci)); if (!xhci) { warn_noalloc(); return; } memset(xhci, 0, sizeof(*xhci)); - - xhci->baseaddr = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0) - & PCI_BASE_ADDRESS_MEM_MASK; - xhci->caps = (void*)(xhci->baseaddr); - xhci->op = (void*)(xhci->baseaddr + readb(&xhci->caps->caplength)); - xhci->pr = (void*)(xhci->baseaddr + readb(&xhci->caps->caplength) + 0x400); - xhci->db = (void*)(xhci->baseaddr + readl(&xhci->caps->dboff)); - xhci->ir = (void*)(xhci->baseaddr + readl(&xhci->caps->rtsoff) + 0x20); + xhci->caps = baseaddr; + xhci->op = baseaddr + readb(&xhci->caps->caplength); + xhci->pr = baseaddr + readb(&xhci->caps->caplength) + 0x400; + xhci->db = baseaddr + readl(&xhci->caps->dboff); + xhci->ir = baseaddr + readl(&xhci->caps->rtsoff) + 0x20;
u32 hcs1 = readl(&xhci->caps->hcsparams1); u32 hcc = readl(&xhci->caps->hccparams); @@ -559,9 +559,10 @@ xhci_controller_setup(struct pci_device *pci) , xhci->ports, xhci->slots, xhci->context64 ? 64 : 32);
if (xhci->xcap) { - u32 off, addr = xhci->baseaddr + xhci->xcap; + u32 off; + void *addr = baseaddr + xhci->xcap; do { - struct xhci_xcap *xcap = (void*)addr; + struct xhci_xcap *xcap = addr; u32 ports, name, cap = readl(&xcap->cap); switch (cap & 0xff) { case 0x02: @@ -580,7 +581,7 @@ xhci_controller_setup(struct pci_device *pci) , ports >> 16); break; default: - dprintf(1, "XHCI extcap 0x%x @ %x\n", cap & 0xff, addr); + dprintf(1, "XHCI extcap 0x%x @ %p\n", cap & 0xff, addr); break; } off = (cap >> 8) & 0xff; @@ -596,7 +597,7 @@ xhci_controller_setup(struct pci_device *pci) return; }
- pci_config_maskw(pci->bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER); + pci_enable_busmaster(pci);
run_thread(configure_xhci, xhci); }
Use the pci_enable_x() functions.
This patch also converts cap->addr from a 'u32' to a union storing a 'u32' or a 'void*'. This makes it more clear when the address is a virtual memory address.
The virtio controller code will now explicitly set PCI_COMMAND_MEMORY and/or PCI_COMMAND_IO instead of assuming it has already been enabled.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/virtio-pci.c | 36 ++++++++++++++++++++---------------- src/hw/virtio-pci.h | 32 ++++++++++++++++++-------------- 2 files changed, 38 insertions(+), 30 deletions(-)
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index 6df5194..f1e30a2 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -100,16 +100,14 @@ void vp_reset(struct vp_device *vp) void vp_notify(struct vp_device *vp, struct vring_virtqueue *vq) { if (vp->use_modern) { - u32 addr = vp->notify.addr + - vq->queue_notify_off * - vp->notify_off_multiplier; + u32 offset = vq->queue_notify_off * vp->notify_off_multiplier; if (vp->notify.is_io) { - outw(vq->queue_index, addr); + outw(vq->queue_index, vp->notify.ioaddr + offset); } else { - writew((void*)addr, vq->queue_index); + writew(vp->notify.memaddr + offset, vq->queue_index); } dprintf(9, "vp notify %x (%d) -- 0x%x\n", - addr, 2, vq->queue_index); + vp->notify.ioaddr, 2, vq->queue_index); } else { vp_write(&vp->legacy, virtio_pci_legacy, queue_notify, vq->queue_index); } @@ -208,7 +206,7 @@ void vp_init_simple(struct vp_device *vp, struct pci_device *pci) { u8 cap = pci_find_capability(pci, PCI_CAP_ID_VNDR, 0); struct vp_cap *vp_cap; - u32 addr, offset, mul; + u32 offset, mul; u8 type;
memset(vp, 0, sizeof(*vp)); @@ -240,19 +238,24 @@ void vp_init_simple(struct vp_device *vp, struct pci_device *pci) offsetof(struct virtio_pci_cap, bar)); offset = pci_config_readl(pci->bdf, cap + offsetof(struct virtio_pci_cap, offset)); - addr = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0 + 4 * vp_cap->bar); - if (addr & PCI_BASE_ADDRESS_SPACE_IO) { + u32 bar = PCI_BASE_ADDRESS_0 + 4 * vp_cap->bar; + if (pci_config_readl(pci->bdf, bar) & PCI_BASE_ADDRESS_SPACE_IO) { vp_cap->is_io = 1; - addr &= PCI_BASE_ADDRESS_IO_MASK; + u32 addr = pci_enable_iobar(pci, bar); + if (!addr) + return; + vp_cap->ioaddr = addr + offset; } else { vp_cap->is_io = 0; - addr &= PCI_BASE_ADDRESS_MEM_MASK; + void *addr = pci_enable_membar(pci, bar); + if (!addr) + return; + vp_cap->memaddr = addr + offset; } - vp_cap->addr = addr + offset; dprintf(3, "pci dev %x:%x virtio cap at 0x%x type %d " "bar %d at 0x%08x off +0x%04x [%s]\n", pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf), - vp_cap->cap, type, vp_cap->bar, addr, offset, + vp_cap->cap, type, vp_cap->bar, vp_cap->ioaddr, offset, vp_cap->is_io ? "io" : "mmio"); }
@@ -267,13 +270,14 @@ void vp_init_simple(struct vp_device *vp, struct pci_device *pci) dprintf(1, "pci dev %x:%x using legacy (0.9.5) virtio mode\n", pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf)); vp->legacy.bar = 0; - vp->legacy.addr = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0) & - PCI_BASE_ADDRESS_IO_MASK; + vp->legacy.ioaddr = pci_enable_iobar(pci, PCI_BASE_ADDRESS_0); + if (!vp->legacy.ioaddr) + return; vp->legacy.is_io = 1; }
vp_reset(vp); - pci_config_maskw(pci->bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER); + pci_enable_busmaster(pci); vp_set_status(vp, VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER ); } diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index b11c355..ff8454e 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -86,7 +86,10 @@ typedef struct virtio_pci_isr { /* --- driver structs ----------------------------------------------- */
struct vp_cap { - u32 addr; + union { + void *memaddr; + u32 ioaddr; + }; u8 cap; u8 bar; u8 is_io; @@ -100,10 +103,10 @@ struct vp_device {
static inline u64 _vp_read(struct vp_cap *cap, u32 offset, u8 size) { - u32 addr = cap->addr + offset; u64 var;
if (cap->is_io) { + u32 addr = cap->ioaddr + offset; switch (size) { case 8: var = inl(addr); @@ -122,34 +125,34 @@ static inline u64 _vp_read(struct vp_cap *cap, u32 offset, u8 size) var = 0; } } else { + void *addr = cap->memaddr + offset; switch (size) { case 8: - var = readl((void*)addr); - var |= (u64)readl((void*)(addr+4)) << 32; + var = readl(addr); + var |= (u64)readl(addr+4) << 32; break; case 4: - var = readl((void*)addr); + var = readl(addr); break; case 2: - var = readw((void*)addr); + var = readw(addr); break; case 1: - var = readb((void*)addr); + var = readb(addr); break; default: var = 0; } } - dprintf(9, "vp read %x (%d) -> 0x%llx\n", addr, size, var); + dprintf(9, "vp read %x (%d) -> 0x%llx\n", cap->ioaddr + offset, size, var); return var; }
static inline void _vp_write(struct vp_cap *cap, u32 offset, u8 size, u64 var) { - u32 addr = cap->addr + offset; - - dprintf(9, "vp write %x (%d) <- 0x%llx\n", addr, size, var); + dprintf(9, "vp write %x (%d) <- 0x%llx\n", cap->ioaddr + offset, size, var); if (cap->is_io) { + u32 addr = cap->ioaddr + offset; switch (size) { case 4: outl(var, addr); @@ -162,15 +165,16 @@ static inline void _vp_write(struct vp_cap *cap, u32 offset, u8 size, u64 var) break; } } else { + void *addr = cap->memaddr + offset; switch (size) { case 4: - writel((void*)addr, var); + writel(addr, var); break; case 2: - writew((void*)addr, var); + writew(addr, var); break; case 1: - writeb((void*)addr, var); + writeb(addr, var); break; } }
Set the pci->have_drivers flag for any device that calls pci_enable_x() to ensure that the flag is consistently set on any device with an internal driver. Setting this flag prevents an option rom on the device from being executed.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/ata.c | 1 - src/hw/pci.c | 3 +++ src/hw/sdcard.c | 1 + 3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/hw/ata.c b/src/hw/ata.c index 7aaf2f1..12dab96 100644 --- a/src/hw/ata.c +++ b/src/hw/ata.c @@ -945,7 +945,6 @@ init_controller(struct pci_device *pci, int chanid, int irq static void init_pciata(struct pci_device *pci, u8 prog_if) { - pci->have_driver = 1; u8 pciirq = pci_config_readb(pci->bdf, PCI_INTERRUPT_LINE); int master = 0; if (CONFIG_ATA_DMA && prog_if & 0x80) { diff --git a/src/hw/pci.c b/src/hw/pci.c index 86b7d54..76c293c 100644 --- a/src/hw/pci.c +++ b/src/hw/pci.c @@ -279,6 +279,7 @@ pci_enable_busmaster(struct pci_device *pci) ASSERT32FLAT(); wait_preempt(); pci_config_maskw(pci->bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER); + pci->have_driver = 1; }
// Verify an IO bar and return it to the caller @@ -298,6 +299,7 @@ pci_enable_iobar(struct pci_device *pci, u32 addr) return 0; } pci_config_maskw(pci->bdf, PCI_COMMAND, 0, PCI_COMMAND_IO); + pci->have_driver = 1; return bar; }
@@ -326,6 +328,7 @@ pci_enable_membar(struct pci_device *pci, u32 addr) return NULL; } pci_config_maskw(pci->bdf, PCI_COMMAND, 0, PCI_COMMAND_MEMORY); + pci->have_driver = 1; return (void*)bar; }
diff --git a/src/hw/sdcard.c b/src/hw/sdcard.c index 1524b51..68a5e56 100644 --- a/src/hw/sdcard.c +++ b/src/hw/sdcard.c @@ -562,6 +562,7 @@ sdcard_setup(void) if (pci->class != PCI_CLASS_SYSTEM_SDHCI || pci->prog_if >= 2) // Not an SDHCI controller following SDHCI spec continue; + pci->have_driver = 1; // Note driver present prior to starting thread run_thread(sdcard_pci_setup, pci); } }
On Tue, Feb 02, 2016 at 11:18:43PM -0500, Kevin O'Connor wrote:
This series adds some helper functions for working with PCI BARs (Base Address Registers), and it then updates all the drivers to use these helper functions. The new helper functions perform sanity checks on the BARs and make sure the appropriate pci command register flags are enabled. The existing driver code was inconsistent with these checks.
At the end of this series, the pci->have_driver flag is also consistently set on any PCI device that there is a SeaBIOS driver for. This will make sure SeaBIOS does not attempt to load an option rom for a device that SeaBIOS already has a native driver for.
FYI, I committed this series.
-Kevin
Dear Kevin,
Am Samstag, den 06.02.2016, 13:00 -0500 schrieb Kevin O'Connor:
On Tue, Feb 02, 2016 at 11:18:43PM -0500, Kevin O'Connor wrote:
This series adds some helper functions for working with PCI BARs (Base Address Registers), and it then updates all the drivers to use these helper functions. The new helper functions perform sanity checks on the BARs and make sure the appropriate pci command register flags are enabled. The existing driver code was inconsistent with these checks.
At the end of this series, the pci->have_driver flag is also consistently set on any PCI device that there is a SeaBIOS driver for. This will make sure SeaBIOS does not attempt to load an option rom for a device that SeaBIOS already has a native driver for.
FYI, I committed this series.
Just a note, that I successfully tested this on the ASRock E350M1.
Thank you for cleaning up the code!
Thanks,
Paul
-
Kevin O'Connor -
Paul Menzel