From c4b325e659e7cceebaed6b39b2eb3d6796b2f5e0 Mon Sep 17 00:00:00 2001 From: Otis Chung Date: Wed, 11 Jun 2025 19:29:44 +0800 Subject: [PATCH 1/2] Replace BAR flags with unified `layout` parameter This change updates pci_set_bar() to accept a single `layout` bitmask instead of separate boolean flags, simplifying BAR configuration. The new `layout` argument encodes I/O vs. memory space, 32-/64-bit decoding, and prefetchable settings via standard PCI_BASE_ADDRESS_* macros. Existing callers can now pass any combination of: - PCI_BASE_ADDRESS_SPACE_IO or PCI_BASE_ADDRESS_SPACE_MEMORY - PCI_BASE_ADDRESS_MEM_TYPE_32 or PCI_BASE_ADDRESS_MEM_TYPE_64 - PCI_BASE_ADDRESS_MEM_PREFETCH The function writes the full layout to the BAR, derives `bar_is_io_space` from bit[0], and initializes the region with the provided callback. Docstrings updated with Doxygen examples illustrating MMIO and port I/O. BREAKING CHANGE: pci_set_bar() signature changed; call sites must be updated to pass the new `layout` parameter rather than separate flags. --- src/pci.c | 7 +++---- src/pci.h | 31 ++++++++++++++++++++++++++++++- src/virtio-pci.c | 4 +++- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/pci.c b/src/pci.c index 2b59ded..aa4dccf 100644 --- a/src/pci.c +++ b/src/pci.c @@ -145,14 +145,13 @@ static void pci_mmio_io(void *owner, void pci_set_bar(struct pci_dev *dev, uint8_t bar, uint32_t bar_size, - bool is_io_space, + uint32_t layout, dev_io_fn do_io) { - /* TODO: mem type, prefetch */ /* FIXME: bar_size must be power of 2 */ - PCI_HDR_WRITE(dev->hdr, PCI_BAR_OFFSET(bar), is_io_space, 32); + PCI_HDR_WRITE(dev->hdr, PCI_BAR_OFFSET(bar), layout, 32); dev->bar_size[bar] = bar_size; - dev->bar_is_io_space[bar] = is_io_space; + dev->bar_is_io_space[bar] = layout & 0x1U; // Get the bit[0] of layout dev_init(&dev->space_dev[bar], 0, bar_size, dev, do_io); } diff --git a/src/pci.h b/src/pci.h index ff37be7..ca9d322 100644 --- a/src/pci.h +++ b/src/pci.h @@ -46,10 +46,39 @@ struct pci { struct dev pci_mmio_dev; }; +/** + * @brief Configure and initialize a PCI Base Address Register (BAR). + * + * This writes the caller-provided layout bitmask into the BAR register + * in the PCI configuration header, records the region size and I/O type, + * and sets up the address space (MMIO or port I/O) with the specified + * callback. + * + * @param dev Pointer to the pci_dev representing the device. + * @param bar BAR index to program (0–5 in a standard PCI header). + * @param bar_size Size of the BAR region in bytes (must be a power of two). + * @param layout Bitmask of PCI_BASE_ADDRESS_* flags defined in + * `/usr/include/linux/pci_regs.h`: + * - Bit 0: I/O space (1) vs. memory space (0) + * (`PCI_BASE_ADDRESS_SPACE_IO` or + * `PCI_BASE_ADDRESS_SPACE_MEMORY`) + * - Bits [2:1]: Memory decoding type + * (`PCI_BASE_ADDRESS_MEM_TYPE_32` or + * `PCI_BASE_ADDRESS_MEM_TYPE_64`) + * - Bit 3: Prefetchable flag for memory + * (`PCI_BASE_ADDRESS_MEM_PREFETCH`) + * @param do_io Callback (dev_io_fn) invoked on accesses within + * the BAR region. + * + * @note bar_size must be a power of two for correct decoding by the + * PCI framework. + * @note For 64-bit memory BARs, callers must reserve the next BAR index + * (n+1) for the high 32 bits if required by the platform. + */ void pci_set_bar(struct pci_dev *dev, uint8_t bar, uint32_t bar_size, - bool is_io_space, + uint32_t is_io_space, dev_io_fn do_io); void pci_set_status(struct pci_dev *dev, uint16_t status); void pci_dev_register(struct pci_dev *dev); diff --git a/src/virtio-pci.c b/src/virtio-pci.c index 289abb8..f712e8c 100644 --- a/src/virtio-pci.c +++ b/src/virtio-pci.c @@ -268,7 +268,9 @@ void virtio_pci_init(struct virtio_pci_dev *dev, PCI_HDR_WRITE(dev->pci_dev.hdr, PCI_HEADER_TYPE, PCI_HEADER_TYPE_NORMAL, 8); PCI_HDR_WRITE(dev->pci_dev.hdr, PCI_INTERRUPT_PIN, 1, 8); pci_set_status(&dev->pci_dev, PCI_STATUS_CAP_LIST | PCI_STATUS_INTERRUPT); - pci_set_bar(&dev->pci_dev, 0, 0x100, PCI_BASE_ADDRESS_SPACE_MEMORY, + pci_set_bar(&dev->pci_dev, 0, 0x100, + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32 + /* | PCI_BASE_ADDRESS_MEM_PREFETCH */, virtio_pci_space_io); virtio_pci_set_cap(dev, cap_list); dev->device_feature |= From 82a6713d9625a6bc981b24c573c3eee9eec88047 Mon Sep 17 00:00:00 2001 From: Otis Chung Date: Thu, 17 Jul 2025 17:46:10 +0800 Subject: [PATCH 2/2] Simplify comments and clarify prefetch flag usage - Remove verbose Doxygen-style documentation for pci_set_bar() to match the existing concise comment style used throughout the codebase - Update PCI_BASE_ADDRESS_MEM_PREFETCH comment to clarify it's an optional flag that users can uncomment if prefetchable memory is needed, rather than implying it's technically not supported The original verbose comment was inconsistent with the project's comment style, which favors brief, direct explanations. The prefetch flag comment now better explains that it's a user choice rather than a limitation. --- src/pci.h | 30 ++---------------------------- src/virtio-pci.c | 2 +- 2 files changed, 3 insertions(+), 29 deletions(-) diff --git a/src/pci.h b/src/pci.h index ca9d322..c9ac9b6 100644 --- a/src/pci.h +++ b/src/pci.h @@ -46,34 +46,8 @@ struct pci { struct dev pci_mmio_dev; }; -/** - * @brief Configure and initialize a PCI Base Address Register (BAR). - * - * This writes the caller-provided layout bitmask into the BAR register - * in the PCI configuration header, records the region size and I/O type, - * and sets up the address space (MMIO or port I/O) with the specified - * callback. - * - * @param dev Pointer to the pci_dev representing the device. - * @param bar BAR index to program (0–5 in a standard PCI header). - * @param bar_size Size of the BAR region in bytes (must be a power of two). - * @param layout Bitmask of PCI_BASE_ADDRESS_* flags defined in - * `/usr/include/linux/pci_regs.h`: - * - Bit 0: I/O space (1) vs. memory space (0) - * (`PCI_BASE_ADDRESS_SPACE_IO` or - * `PCI_BASE_ADDRESS_SPACE_MEMORY`) - * - Bits [2:1]: Memory decoding type - * (`PCI_BASE_ADDRESS_MEM_TYPE_32` or - * `PCI_BASE_ADDRESS_MEM_TYPE_64`) - * - Bit 3: Prefetchable flag for memory - * (`PCI_BASE_ADDRESS_MEM_PREFETCH`) - * @param do_io Callback (dev_io_fn) invoked on accesses within - * the BAR region. - * - * @note bar_size must be a power of two for correct decoding by the - * PCI framework. - * @note For 64-bit memory BARs, callers must reserve the next BAR index - * (n+1) for the high 32 bits if required by the platform. +/* Configure PCI BAR with layout flags from linux/pci_regs.h. + * FIXME: bar_size must be a power of two. */ void pci_set_bar(struct pci_dev *dev, uint8_t bar, diff --git a/src/virtio-pci.c b/src/virtio-pci.c index f712e8c..28c4a71 100644 --- a/src/virtio-pci.c +++ b/src/virtio-pci.c @@ -270,7 +270,7 @@ void virtio_pci_init(struct virtio_pci_dev *dev, pci_set_status(&dev->pci_dev, PCI_STATUS_CAP_LIST | PCI_STATUS_INTERRUPT); pci_set_bar(&dev->pci_dev, 0, 0x100, PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32 - /* | PCI_BASE_ADDRESS_MEM_PREFETCH */, + /* | PCI_BASE_ADDRESS_MEM_PREFETCH: uncomment if prefetchable */, virtio_pci_space_io); virtio_pci_set_cap(dev, cap_list); dev->device_feature |=