Skip to content

Commit da24a12

Browse files
otischungjserv
andcommitted
pci: preserve full BAR layout across guest sizing
This replaces bar_is_io_space[6] with bar_layout[6] so the full PCI_BASE_ADDRESS_* bitmask written by pci_set_bar() survives a guest BAR write. Previously only bit 0 (space) was retained; MEM_TYPE and PREFETCH bits were silently dropped on the first config-space probe, which meant pci_set_bar()'s new `layout` parameter was a partial lie. pci_config_bar() now restores all non-address bits via (bar_layout[bar] & ~mask) and assigns space_dev[bar].base to the masked address only, fixing a latent I/O BAR routing bug where the SPACE_IO bit in 'base' made bus_find_dev() miss the first port. Header parameter renamed is_io_space -> layout to match the implementation, and the magic 0x1U mask in pci_set_bar() now uses the standard PCI_BASE_ADDRESS_SPACE_IO constant. 64-bit BARs still need a paired adjacent slot for the upper dword; that limitation is now documented in pci_set_bar(). Co-authored-by: Jim Huang <jserv@ccns.ncku.edu.tw>
1 parent 1a8f3b9 commit da24a12

3 files changed

Lines changed: 18 additions & 38 deletions

File tree

src/pci.c

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,9 @@ static void pci_command_bar(struct pci_dev *dev)
5252
bool enable_mem =
5353
PCI_HDR_READ(dev->hdr, PCI_COMMAND, 16) & PCI_COMMAND_MEMORY;
5454
for (int i = 0; i < PCI_STD_NUM_BARS; i++) {
55-
struct bus *bus = dev->bar_is_io_space[i] ? dev->io_bus : dev->mmio_bus;
56-
bool enable = dev->bar_is_io_space[i] ? enable_io : enable_mem;
55+
bool is_io = dev->bar_layout[i] & PCI_BASE_ADDRESS_SPACE_IO;
56+
struct bus *bus = is_io ? dev->io_bus : dev->mmio_bus;
57+
bool enable = is_io ? enable_io : enable_mem;
5758

5859
if (enable)
5960
pci_activate_bar(dev, i, bus);
@@ -71,9 +72,9 @@ static void pci_config_bar(struct pci_dev *dev, uint8_t bar)
7172
{
7273
uint32_t mask = ~(dev->bar_size[bar] - 1);
7374
uint32_t old_bar = PCI_HDR_READ(dev->hdr, PCI_BAR_OFFSET(bar), 32);
74-
uint32_t new_bar = (old_bar & mask) | dev->bar_is_io_space[bar];
75+
uint32_t new_bar = (old_bar & mask) | (dev->bar_layout[bar] & ~mask);
7576
PCI_HDR_WRITE(dev->hdr, PCI_BAR_OFFSET(bar), new_bar, 32);
76-
dev->space_dev[bar].base = new_bar;
77+
dev->space_dev[bar].base = new_bar & mask;
7778
}
7879

7980
static void pci_config_write(struct pci_dev *dev,
@@ -148,10 +149,14 @@ void pci_set_bar(struct pci_dev *dev,
148149
uint32_t layout,
149150
dev_io_fn do_io)
150151
{
151-
/* FIXME: bar_size must be power of 2 */
152+
/*
153+
* FIXME: bar_size must be a power of two.
154+
* TODO: 64-bit BARs need a second adjacent slot for the upper dword;
155+
* only 32-bit memory and I/O BARs are wired up here.
156+
*/
152157
PCI_HDR_WRITE(dev->hdr, PCI_BAR_OFFSET(bar), layout, 32);
153158
dev->bar_size[bar] = bar_size;
154-
dev->bar_is_io_space[bar] = layout & 0x1U; // Get the bit[0] of layout
159+
dev->bar_layout[bar] = layout;
155160
dev_init(&dev->space_dev[bar], 0, bar_size, dev, do_io);
156161
}
157162

src/pci.h

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ struct pci_dev {
3030
void *hdr;
3131
uint32_t bar_size[6];
3232
bool bar_active[6];
33-
bool bar_is_io_space[6];
33+
uint32_t bar_layout[6];
3434
struct dev space_dev[6];
3535
struct dev config_dev;
3636
struct bus *io_bus;
@@ -46,39 +46,15 @@ struct pci {
4646
struct dev pci_mmio_dev;
4747
};
4848

49-
/**
50-
* @brief Configure and initialize a PCI Base Address Register (BAR).
51-
*
52-
* This writes the caller-provided layout bitmask into the BAR register
53-
* in the PCI configuration header, records the region size and I/O type,
54-
* and sets up the address space (MMIO or port I/O) with the specified
55-
* callback.
56-
*
57-
* @param dev Pointer to the pci_dev representing the device.
58-
* @param bar BAR index to program (0–5 in a standard PCI header).
59-
* @param bar_size Size of the BAR region in bytes (must be a power of two).
60-
* @param layout Bitmask of PCI_BASE_ADDRESS_* flags defined in
61-
* `/usr/include/linux/pci_regs.h`:
62-
* - Bit 0: I/O space (1) vs. memory space (0)
63-
* (`PCI_BASE_ADDRESS_SPACE_IO` or
64-
* `PCI_BASE_ADDRESS_SPACE_MEMORY`)
65-
* - Bits [2:1]: Memory decoding type
66-
* (`PCI_BASE_ADDRESS_MEM_TYPE_32` or
67-
* `PCI_BASE_ADDRESS_MEM_TYPE_64`)
68-
* - Bit 3: Prefetchable flag for memory
69-
* (`PCI_BASE_ADDRESS_MEM_PREFETCH`)
70-
* @param do_io Callback (dev_io_fn) invoked on accesses within
71-
* the BAR region.
72-
*
73-
* @note bar_size must be a power of two for correct decoding by the
74-
* PCI framework.
75-
* @note For 64-bit memory BARs, callers must reserve the next BAR index
76-
* (n+1) for the high 32 bits if required by the platform.
49+
/*
50+
* Configure a PCI BAR. @layout is a bitmask of PCI_BASE_ADDRESS_* flags
51+
* from <linux/pci_regs.h> (space, mem type, prefetch). The non-address
52+
* bits are preserved across guest BAR sizing probes.
7753
*/
7854
void pci_set_bar(struct pci_dev *dev,
7955
uint8_t bar,
8056
uint32_t bar_size,
81-
uint32_t is_io_space,
57+
uint32_t layout,
8258
dev_io_fn do_io);
8359
void pci_set_status(struct pci_dev *dev, uint16_t status);
8460
void pci_dev_register(struct pci_dev *dev);

src/virtio-pci.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,7 @@ void virtio_pci_init(struct virtio_pci_dev *dev,
269269
PCI_HDR_WRITE(dev->pci_dev.hdr, PCI_INTERRUPT_PIN, 1, 8);
270270
pci_set_status(&dev->pci_dev, PCI_STATUS_CAP_LIST | PCI_STATUS_INTERRUPT);
271271
pci_set_bar(&dev->pci_dev, 0, 0x100,
272-
PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32
273-
/* | PCI_BASE_ADDRESS_MEM_PREFETCH */,
272+
PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
274273
virtio_pci_space_io);
275274
virtio_pci_set_cap(dev, cap_list);
276275
dev->device_feature |=

0 commit comments

Comments
 (0)