[kvm-unit-tests PATCH v2] x86: pci: Support unaligned register access in PCI config read/write

[kvm-unit-tests PATCH v2] x86: pci: Support unaligned register access in PCI config read/write

[Irene Wang]

Per the PCI Local Bus Specification (Section 3.2.2.3.2, "Configuration
Mechanism #1"), port 0xCF8 (CONFIG_ADDRESS) requires a DWORD-aligned
register offset (bits [1:0] = 00b), while byte and word offsets within
the DWORD must be selected via data port 0xCFC + (reg & 3).

Previously, PCI_CONF1_ADDRESS did not clear bits [1:0], and 8-bit /
16-bit helpers always accessed base port 0xCFC directly. This worked
in QEMU because its PCI host bridge emulation preserves unaligned bits
in CONFIG_ADDRESS and uses them during CONFIG_DATA accesses. However,
in strictly spec-compliant VMMs (and potentially real hardware), bits
[1:0] of 0xCF8 are ignored, causing non-aligned reads/writes at port
0xCFC to erroneously target byte 0 of the DWORD. In practice, this
causes pci_find_dev() to read the vendor ID twice instead of reading
the vendor ID and device ID.

Fix this by masking `reg` with `~3` in PCI_CONF1_ADDRESS and adding
the `(reg & 3)` offset to the CONFIG_DATA port for 8-bit and 16-bit
accessors, ensuring compatibility across QEMU and other VMMs. Additionally,
assert that 16-bit accesses do not use an offset of 3, as reading/writing a
word across DWORD boundaries (port 0xCFC + 3) is invalid per spec.

Assisted-by: Gemini:gemini-3.6-flash
Signed-off-by: Irene Wang <yiranirenewang@gmail.com>
---
v1 -> v2:
 - Enclose macro parameter `dev` in parentheses in PCI_CONF1_ADDRESS.
 - Add assert((reg & 3) != 3) in pci_config_readw() and pci_config_writew()
   to guard against illegal 16-bit accesses crossing DWORD boundaries.

Link to v1: https://lore.kernel.org/kvm/20260730182053.905335-1-yiranirenewang@gmail.com/

 lib/x86/asm/pci.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/lib/x86/asm/pci.h b/lib/x86/asm/pci.h
index 03e55c27..da0b59a3 100644
--- a/lib/x86/asm/pci.h
+++ b/lib/x86/asm/pci.h
@@ -9,18 +9,19 @@
 #include "pci.h"
 #include "x86/asm/io.h"
 
-#define PCI_CONF1_ADDRESS(dev, reg)	((0x1 << 31) | (dev << 8) | reg)
+#define PCI_CONF1_ADDRESS(dev, reg)	((0x1 << 31) | ((dev) << 8) | ((reg) & ~3))
 
 static inline uint8_t pci_config_readb(pcidevaddr_t dev, uint8_t reg)
 {
     outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
-    return inb(0xCFC);
+    return inb(0xCFC + (reg & 3));
 }
 
 static inline uint16_t pci_config_readw(pcidevaddr_t dev, uint8_t reg)
 {
+    assert((reg & 3) != 3);
     outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
-    return inw(0xCFC);
+    return inw(0xCFC + (reg & 3));
 }
 
 static inline uint32_t pci_config_readl(pcidevaddr_t dev, uint8_t reg)
@@ -33,14 +34,15 @@ static inline void pci_config_writeb(pcidevaddr_t dev, uint8_t reg,
                                      uint8_t val)
 {
     outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
-    outb(val, 0xCFC);
+    outb(val, 0xCFC + (reg & 3));
 }
 
 static inline void pci_config_writew(pcidevaddr_t dev, uint8_t reg,
                                      uint16_t val)
 {
+    assert((reg & 3) != 3);
     outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
-    outw(val, 0xCFC);
+    outw(val, 0xCFC + (reg & 3));
 }
 
 static inline void pci_config_writel(pcidevaddr_t dev, uint8_t reg,
-- 
2.55.0.571.g244d577d93-goog

Re: [kvm-unit-tests PATCH v2] x86: pci: Support unaligned register access in PCI config read/write

[Jim Mattson]

On Tue, Aug 4, 2026 at 4:28 PM Irene Wang <yiranirenewang@gmail.com> wrote:
>
> Per the PCI Local Bus Specification (Section 3.2.2.3.2, "Configuration
> Mechanism #1"), port 0xCF8 (CONFIG_ADDRESS) requires a DWORD-aligned
> register offset (bits [1:0] = 00b), while byte and word offsets within
> the DWORD must be selected via data port 0xCFC + (reg & 3).
>
> Previously, PCI_CONF1_ADDRESS did not clear bits [1:0], and 8-bit /
> 16-bit helpers always accessed base port 0xCFC directly. This worked
> in QEMU because its PCI host bridge emulation preserves unaligned bits
> in CONFIG_ADDRESS and uses them during CONFIG_DATA accesses. However,
> in strictly spec-compliant VMMs (and potentially real hardware), bits
> [1:0] of 0xCF8 are ignored, causing non-aligned reads/writes at port
> 0xCFC to erroneously target byte 0 of the DWORD. In practice, this
> causes pci_find_dev() to read the vendor ID twice instead of reading
> the vendor ID and device ID.
>
> Fix this by masking `reg` with `~3` in PCI_CONF1_ADDRESS and adding
> the `(reg & 3)` offset to the CONFIG_DATA port for 8-bit and 16-bit
> accessors, ensuring compatibility across QEMU and other VMMs. Additionally,
> assert that 16-bit accesses do not use an offset of 3, as reading/writing a
> word across DWORD boundaries (port 0xCFC + 3) is invalid per spec.
>
> Assisted-by: Gemini:gemini-3.6-flash
> Signed-off-by: Irene Wang <yiranirenewang@gmail.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

Re: [kvm-unit-tests PATCH v2] x86: pci: Support unaligned register access in PCI config read/write

[David Matlack]

On 2026-08-04 11:23 PM, Irene Wang wrote:
> Per the PCI Local Bus Specification (Section 3.2.2.3.2, "Configuration
> Mechanism #1"), port 0xCF8 (CONFIG_ADDRESS) requires a DWORD-aligned
> register offset (bits [1:0] = 00b), while byte and word offsets within
> the DWORD must be selected via data port 0xCFC + (reg & 3).
> 
> Previously, PCI_CONF1_ADDRESS did not clear bits [1:0], and 8-bit /
> 16-bit helpers always accessed base port 0xCFC directly. This worked
> in QEMU because its PCI host bridge emulation preserves unaligned bits
> in CONFIG_ADDRESS and uses them during CONFIG_DATA accesses. However,
> in strictly spec-compliant VMMs (and potentially real hardware), bits
> [1:0] of 0xCF8 are ignored, causing non-aligned reads/writes at port
> 0xCFC to erroneously target byte 0 of the DWORD. In practice, this
> causes pci_find_dev() to read the vendor ID twice instead of reading
> the vendor ID and device ID.
> 
> Fix this by masking `reg` with `~3` in PCI_CONF1_ADDRESS and adding
> the `(reg & 3)` offset to the CONFIG_DATA port for 8-bit and 16-bit
> accessors, ensuring compatibility across QEMU and other VMMs. Additionally,
> assert that 16-bit accesses do not use an offset of 3, as reading/writing a
> word across DWORD boundaries (port 0xCFC + 3) is invalid per spec.
> 
> Assisted-by: Gemini:gemini-3.6-flash
> Signed-off-by: Irene Wang <yiranirenewang@gmail.com>
> ---
> v1 -> v2:
>  - Enclose macro parameter `dev` in parentheses in PCI_CONF1_ADDRESS.
>  - Add assert((reg & 3) != 3) in pci_config_readw() and pci_config_writew()
>    to guard against illegal 16-bit accesses crossing DWORD boundaries.
> 
> Link to v1: https://lore.kernel.org/kvm/20260730182053.905335-1-yiranirenewang@gmail.com/
> 
>  lib/x86/asm/pci.h | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/x86/asm/pci.h b/lib/x86/asm/pci.h
> index 03e55c27..da0b59a3 100644
> --- a/lib/x86/asm/pci.h
> +++ b/lib/x86/asm/pci.h
> @@ -9,18 +9,19 @@
>  #include "pci.h"
>  #include "x86/asm/io.h"
>  
> -#define PCI_CONF1_ADDRESS(dev, reg)	((0x1 << 31) | (dev << 8) | reg)
> +#define PCI_CONF1_ADDRESS(dev, reg)	((0x1 << 31) | ((dev) << 8) | ((reg) & ~3))

A comment here would be useful. Spec-compliant host bridges (including
emulations) should ignore the lower 2 bits so clearing it should
technically be unnecessary. The reason we have to clear it is because
QEMU is not spec-compliant and will consume those 2 bits.

>  
>  static inline uint8_t pci_config_readb(pcidevaddr_t dev, uint8_t reg)
>  {
>      outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
> -    return inb(0xCFC);
> +    return inb(0xCFC + (reg & 3));
>  }
>  
>  static inline uint16_t pci_config_readw(pcidevaddr_t dev, uint8_t reg)
>  {
> +    assert((reg & 3) != 3);
>      outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
> -    return inw(0xCFC);
> +    return inw(0xCFC + (reg & 3));
>  }
>  
>  static inline uint32_t pci_config_readl(pcidevaddr_t dev, uint8_t reg)
> @@ -33,14 +34,15 @@ static inline void pci_config_writeb(pcidevaddr_t dev, uint8_t reg,
>                                       uint8_t val)
>  {
>      outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
> -    outb(val, 0xCFC);
> +    outb(val, 0xCFC + (reg & 3));
>  }
>  
>  static inline void pci_config_writew(pcidevaddr_t dev, uint8_t reg,
>                                       uint16_t val)
>  {
> +    assert((reg & 3) != 3);

Should assert((reg & 3) == 0) be added to the readl/writel routines as
well?

>      outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
> -    outw(val, 0xCFC);
> +    outw(val, 0xCFC + (reg & 3));
>  }
>  
>  static inline void pci_config_writel(pcidevaddr_t dev, uint8_t reg,
> -- 
> 2.55.0.571.g244d577d93-goog
> 

[kvm-unit-tests PATCH v3] x86: pci: Support unaligned register access in PCI config read/write

[Irene Wang]

Per the PCI Local Bus Specification (Section 3.2.2.3.2, "Configuration
Mechanism #1"), port 0xCF8 (CONFIG_ADDRESS) requires a DWORD-aligned
register offset (bits [1:0] = 00b), while byte and word offsets within
the DWORD must be selected via data port 0xCFC + (reg & 3).

Previously, PCI_CONF1_ADDRESS did not clear bits [1:0], and 8-bit /
16-bit helpers always accessed base port 0xCFC directly. This worked
in QEMU because its PCI host bridge emulation preserves unaligned bits
in CONFIG_ADDRESS and uses them during CONFIG_DATA accesses. However,
in strictly spec-compliant VMMs (and potentially real hardware), bits
[1:0] of 0xCF8 are ignored, causing non-aligned reads/writes at port
0xCFC to erroneously target byte 0 of the DWORD. In practice, this
causes pci_find_dev() to read the vendor ID twice instead of reading
the vendor ID and device ID.

Fix this by masking `reg` with `~3` in PCI_CONF1_ADDRESS and adding
the `(reg & 3)` offset to the CONFIG_DATA port for 8-bit and 16-bit
accessors, ensuring compatibility across QEMU and other VMMs. Additionally,
add an ASSERT_PCI_CONF1_VALID macro to guard against illegal accesses that
cross DWORD boundaries.

Assisted-by: Gemini:gemini-3.6-flash
Reviewed-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Irene Wang <yiranirenewang@gmail.com>
---
v2 -> v3:
 - Add comment in PCI_CONF1_ADDRESS explicitly explaining that clearing
   bits [1:0] prevents QEMU from double-offsetting accesses (David)
 - Introduce ASSERT_PCI_CONF1_VALID macro checking that access offset +
   access size do not read/write past 4-byte window (David)

Link to v2: https://lore.kernel.org/kvm/20260804232344.2976694-2-yiranirenewang@gmail.com/
Link to v1: https://lore.kernel.org/kvm/20260730182053.905335-1-yiranirenewang@gmail.com/
---
 lib/x86/asm/pci.h | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/lib/x86/asm/pci.h b/lib/x86/asm/pci.h
index 03e55c27..144a4459 100644
--- a/lib/x86/asm/pci.h
+++ b/lib/x86/asm/pci.h
@@ -9,22 +9,35 @@
 #include "pci.h"
 #include "x86/asm/io.h"
 
-#define PCI_CONF1_ADDRESS(dev, reg)	((0x1 << 31) | (dev << 8) | reg)
+/*
+ * Bits [1:0] offset into data port, not address port. Spec compliant
+ * host bridges ignore them in CONFIG_ADDRESS, but QEMU does not; mask them out
+ * here so QEMU doesn't double-offset the access.
+ */
+#define PCI_CONF1_ADDRESS(dev, reg)	((0x1 << 31) | ((dev) << 8) | ((reg) & ~3))
+
+/*
+ * Ensure access offset + size stays within the 4-byte (DWORD) boundary.
+ */
+#define ASSERT_PCI_CONF1_VALID(reg, type) \
+	assert(((reg) & 3) + sizeof(type) <= 4)
 
 static inline uint8_t pci_config_readb(pcidevaddr_t dev, uint8_t reg)
 {
     outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
-    return inb(0xCFC);
+    return inb(0xCFC + (reg & 3));
 }
 
 static inline uint16_t pci_config_readw(pcidevaddr_t dev, uint8_t reg)
 {
+    ASSERT_PCI_CONF1_VALID(reg, uint16_t);
     outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
-    return inw(0xCFC);
+    return inw(0xCFC + (reg & 3));
 }
 
 static inline uint32_t pci_config_readl(pcidevaddr_t dev, uint8_t reg)
 {
+    ASSERT_PCI_CONF1_VALID(reg, uint32_t);
     outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
     return inl(0xCFC);
 }
@@ -33,19 +46,21 @@ static inline void pci_config_writeb(pcidevaddr_t dev, uint8_t reg,
                                      uint8_t val)
 {
     outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
-    outb(val, 0xCFC);
+    outb(val, 0xCFC + (reg & 3));
 }
 
 static inline void pci_config_writew(pcidevaddr_t dev, uint8_t reg,
                                      uint16_t val)
 {
+    ASSERT_PCI_CONF1_VALID(reg, uint16_t);
     outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
-    outw(val, 0xCFC);
+    outw(val, 0xCFC + (reg & 3));
 }
 
 static inline void pci_config_writel(pcidevaddr_t dev, uint8_t reg,
                                      uint32_t val)
 {
+    ASSERT_PCI_CONF1_VALID(reg, uint32_t);
     outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
     outl(val, 0xCFC);
 }
-- 
2.55.0.691.gc56d675ccc-goog

Re: [kvm-unit-tests PATCH v3] x86: pci: Support unaligned register access in PCI config read/write

[David Matlack]

On 2026-08-13 08:57 PM, Irene Wang wrote:
> Per the PCI Local Bus Specification (Section 3.2.2.3.2, "Configuration
> Mechanism #1"), port 0xCF8 (CONFIG_ADDRESS) requires a DWORD-aligned
> register offset (bits [1:0] = 00b), while byte and word offsets within
> the DWORD must be selected via data port 0xCFC + (reg & 3).
> 
> Previously, PCI_CONF1_ADDRESS did not clear bits [1:0], and 8-bit /
> 16-bit helpers always accessed base port 0xCFC directly. This worked
> in QEMU because its PCI host bridge emulation preserves unaligned bits
> in CONFIG_ADDRESS and uses them during CONFIG_DATA accesses. However,
> in strictly spec-compliant VMMs (and potentially real hardware), bits
> [1:0] of 0xCF8 are ignored, causing non-aligned reads/writes at port
> 0xCFC to erroneously target byte 0 of the DWORD. In practice, this
> causes pci_find_dev() to read the vendor ID twice instead of reading
> the vendor ID and device ID.
> 
> Fix this by masking `reg` with `~3` in PCI_CONF1_ADDRESS and adding
> the `(reg & 3)` offset to the CONFIG_DATA port for 8-bit and 16-bit
> accessors, ensuring compatibility across QEMU and other VMMs. Additionally,
> add an ASSERT_PCI_CONF1_VALID macro to guard against illegal accesses that
> cross DWORD boundaries.
> 
> Assisted-by: Gemini:gemini-3.6-flash
> Reviewed-by: Jim Mattson <jmattson@google.com>
> Signed-off-by: Irene Wang <yiranirenewang@gmail.com>

Looks good aside from the comment nits.

Reviewed-by: David Matlack <dmatlack@google.com>

> ---
> v2 -> v3:
>  - Add comment in PCI_CONF1_ADDRESS explicitly explaining that clearing
>    bits [1:0] prevents QEMU from double-offsetting accesses (David)
>  - Introduce ASSERT_PCI_CONF1_VALID macro checking that access offset +
>    access size do not read/write past 4-byte window (David)
> 
> Link to v2: https://lore.kernel.org/kvm/20260804232344.2976694-2-yiranirenewang@gmail.com/
> Link to v1: https://lore.kernel.org/kvm/20260730182053.905335-1-yiranirenewang@gmail.com/
> ---
>  lib/x86/asm/pci.h | 25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/x86/asm/pci.h b/lib/x86/asm/pci.h
> index 03e55c27..144a4459 100644
> --- a/lib/x86/asm/pci.h
> +++ b/lib/x86/asm/pci.h
> @@ -9,22 +9,35 @@
>  #include "pci.h"
>  #include "x86/asm/io.h"
>  
> -#define PCI_CONF1_ADDRESS(dev, reg)	((0x1 << 31) | (dev << 8) | reg)
> +/*
> + * Bits [1:0] offset into data port, not address port. Spec compliant
> + * host bridges ignore them in CONFIG_ADDRESS, but QEMU does not; mask them out
> + * here so QEMU doesn't double-offset the access.
> + */

nit: Wrap at 80 chars.

/*
 * Bits [1:0] offset into data port, not address port. Spec compliant host
 * bridges ignore them in CONFIG_ADDRESS, but QEMU does not; mask them out here
 * so QEMU doesn't double-offset the access.
 */

> +#define PCI_CONF1_ADDRESS(dev, reg)	((0x1 << 31) | ((dev) << 8) | ((reg) & ~3))
> +
> +/*
> + * Ensure access offset + size stays within the 4-byte (DWORD) boundary.
> + */

nit: Use /* ... */ style for one-line comments.

/* Ensure access offset + size stays within the 4-byte (DWORD) boundary. */

> +#define ASSERT_PCI_CONF1_VALID(reg, type) \
> +	assert(((reg) & 3) + sizeof(type) <= 4)
>  
>  static inline uint8_t pci_config_readb(pcidevaddr_t dev, uint8_t reg)
>  {
>      outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
> -    return inb(0xCFC);
> +    return inb(0xCFC + (reg & 3));
>  }
>  
>  static inline uint16_t pci_config_readw(pcidevaddr_t dev, uint8_t reg)
>  {
> +    ASSERT_PCI_CONF1_VALID(reg, uint16_t);
>      outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
> -    return inw(0xCFC);
> +    return inw(0xCFC + (reg & 3));
>  }
>  
>  static inline uint32_t pci_config_readl(pcidevaddr_t dev, uint8_t reg)
>  {
> +    ASSERT_PCI_CONF1_VALID(reg, uint32_t);
>      outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
>      return inl(0xCFC);
>  }
> @@ -33,19 +46,21 @@ static inline void pci_config_writeb(pcidevaddr_t dev, uint8_t reg,
>                                       uint8_t val)
>  {
>      outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
> -    outb(val, 0xCFC);
> +    outb(val, 0xCFC + (reg & 3));
>  }
>  
>  static inline void pci_config_writew(pcidevaddr_t dev, uint8_t reg,
>                                       uint16_t val)
>  {
> +    ASSERT_PCI_CONF1_VALID(reg, uint16_t);
>      outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
> -    outw(val, 0xCFC);
> +    outw(val, 0xCFC + (reg & 3));
>  }
>  
>  static inline void pci_config_writel(pcidevaddr_t dev, uint8_t reg,
>                                       uint32_t val)
>  {
> +    ASSERT_PCI_CONF1_VALID(reg, uint32_t);
>      outl(PCI_CONF1_ADDRESS(dev, reg), 0xCF8);
>      outl(val, 0xCFC);
>  }
> -- 
> 2.55.0.691.gc56d675ccc-goog
>