device/pci: Rewrite PCI MMCONF with symbol reference

The effect of pointer aliasing on writes is that any data on CPU
registers that has been resolved from (non-const and non-volatile)
memory objects has to be discarded and resolved. In other words, the
compiler assumes that a pointer that does not have an absolute value
at build-time, and is of type 'void *' or 'char *', may write over
any memory object.

Using a unique datatype for MMIO writes makes the pointer to _not_
qualify for pointer aliasing with any other objects in memory. This
avoid constantly resolving the PCI MMCONF address, which is a derived
value from a 'struct device *'.

Change-Id: Id112aa5e729ffd8015bb806786bdee38783b7ea9
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31752
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>

src/arch/x86/include/arch/pci_mmio_cfg_romcc.h

@@ -0,0 +1,72 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2007-2009 coresystems GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _PCI_MMIO_CFG_ROMCC_H
+#define _PCI_MMIO_CFG_ROMCC_H
+
+#include <stdint.h>
+#include <device/mmio.h>
+#include <device/pci_type.h>
+
+
+static __always_inline
+uint8_t pci_mmio_read_config8(pci_devfn_t dev, uint16_t reg)
+{
+	void *addr;
+	addr = (void *)(uintptr_t)(CONFIG_MMCONF_BASE_ADDRESS | dev | reg);
+	return read8(addr);
+}
+
+static __always_inline
+uint16_t pci_mmio_read_config16(pci_devfn_t dev, uint16_t reg)
+{
+	void *addr;
+	addr = (void *)(uintptr_t)(CONFIG_MMCONF_BASE_ADDRESS | dev | (reg & ~1));
+	return read16(addr);
+}
+
+static __always_inline
+uint32_t pci_mmio_read_config32(pci_devfn_t dev, uint16_t reg)
+{
+	void *addr;
+	addr = (void *)(uintptr_t)(CONFIG_MMCONF_BASE_ADDRESS | dev | (reg & ~3));
+	return read32(addr);
+}
+
+static __always_inline
+void pci_mmio_write_config8(pci_devfn_t dev, uint16_t reg, uint8_t value)
+{
+	void *addr;
+	addr = (void *)(uintptr_t)(CONFIG_MMCONF_BASE_ADDRESS | dev | reg);
+	write8(addr, value);
+}
+
+static __always_inline
+void pci_mmio_write_config16(pci_devfn_t dev, uint16_t reg, uint16_t value)
+{
+	void *addr;
+	addr = (void *)(uintptr_t)(CONFIG_MMCONF_BASE_ADDRESS | dev | (reg & ~1));
+	write16(addr, value);
+}
+
+static __always_inline
+void pci_mmio_write_config32(pci_devfn_t dev, uint16_t reg, uint32_t value)
+{
+	void *addr;
+	addr = (void *)(uintptr_t)(CONFIG_MMCONF_BASE_ADDRESS | dev | (reg & ~3));
+	write32(addr, value);
+}
+
+#endif /* _PCI_MMIO_CFG_ROMCC_H */

src/arch/x86/include/arch/pci_ops.h

@@ -15,6 +15,12 @@
 #define ARCH_I386_PCI_OPS_H
 
 #include <arch/pci_io_cfg.h>
+
+#if defined(__ROMCC__)
+/* Must come before <device/pci_mmio_cfg.h> */
+#include <arch/pci_mmio_cfg_romcc.h>
+#endif
+
 #include <device/pci_mmio_cfg.h>
 
 #endif /* ARCH_I386_PCI_OPS_H */

src/device/Makefile.inc

@@ -26,9 +26,15 @@ postcar-y += pci_early.c
 
 ramstage-y += pci_class.c
 ramstage-y += pci_device.c
-ramstage-y += pci_ops.c
 ramstage-y += pci_rom.c
 
+bootblock-y += pci_ops.c
+verstage-y += pci_ops.c
+romstage-y += pci_ops.c
+postcar-y += pci_ops.c
+ramstage-y += pci_ops.c
+smm-y += pci_ops.c
+
 ramstage-$(CONFIG_HYPERTRANSPORT_PLUGIN_SUPPORT) += hypertransport.c
 ramstage-$(CONFIG_PCIX_PLUGIN_SUPPORT) += pcix_device.c
 ramstage-$(CONFIG_PCIEXP_PLUGIN_SUPPORT) += pciexp_device.c

src/device/pci_ops.c

@@ -10,3 +10,7 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  */
+
+#include <stdint.h>
+
+u8 *const pci_mmconf = (void *)(uintptr_t)CONFIG_MMCONF_BASE_ADDRESS;

src/include/device/pci_mmio_cfg.h

@@ -20,55 +20,74 @@
 #include <device/mmio.h>
 #include <device/pci_type.h>
 
+#if !defined(__ROMCC__)
+
+/* By not assigning this to CONFIG_MMCONF_BASE_ADDRESS here we
+ * prevent some sub-optimal constant folding. */
+extern u8 *const pci_mmconf;
+
+/* Using a unique datatype for MMIO writes makes the pointers to _not_
+ * qualify for pointer aliasing with any other objects in memory.
+ *
+ * MMIO offset is a value originally derived from 'struct device *'
+ * in ramstage. For the compiler to not discard this MMIO offset value
+ * from CPU registers after any MMIO writes, -fstrict-aliasing has to
+ * be also set for the build.
+ *
+ * Bottom 12 bits (4 KiB) are reserved to address the registers of a
+ * single PCI function. Declare the bank as a union to avoid some casting
+ * in the functions below.
+ */
+union pci_bank {
+	uint8_t reg8[4096];
+	uint16_t reg16[4096 / sizeof(uint16_t)];
+	uint32_t reg32[4096 / sizeof(uint32_t)];
+};
+
+static __always_inline
+volatile union pci_bank *pcicfg(pci_devfn_t dev)
+{
+	return (void *)&pci_mmconf[PCI_DEVFN_OFFSET(dev)];
+}
 
 static __always_inline
 uint8_t pci_mmio_read_config8(pci_devfn_t dev, uint16_t reg)
 {
-	void *addr;
+	return pcicfg(dev)->reg8[reg];
-	addr = (void *)(uintptr_t)(CONFIG_MMCONF_BASE_ADDRESS | dev | reg);
-	return read8(addr);
 }
 
 static __always_inline
 uint16_t pci_mmio_read_config16(pci_devfn_t dev, uint16_t reg)
 {
-	void *addr;
+	return pcicfg(dev)->reg16[reg / sizeof(uint16_t)];
-	addr = (void *)(uintptr_t)(CONFIG_MMCONF_BASE_ADDRESS | dev | (reg & ~1));
-	return read16(addr);
 }
 
 static __always_inline
 uint32_t pci_mmio_read_config32(pci_devfn_t dev, uint16_t reg)
 {
-	void *addr;
+	return pcicfg(dev)->reg32[reg / sizeof(uint32_t)];
-	addr = (void *)(uintptr_t)(CONFIG_MMCONF_BASE_ADDRESS | dev | (reg & ~3));
-	return read32(addr);
 }
 
 static __always_inline
 void pci_mmio_write_config8(pci_devfn_t dev, uint16_t reg, uint8_t value)
 {
-	void *addr;
+	pcicfg(dev)->reg8[reg] = value;
-	addr = (void *)(uintptr_t)(CONFIG_MMCONF_BASE_ADDRESS | dev | reg);
-	write8(addr, value);
 }
 
 static __always_inline
 void pci_mmio_write_config16(pci_devfn_t dev, uint16_t reg, uint16_t value)
 {
-	void *addr;
+	pcicfg(dev)->reg16[reg / sizeof(uint16_t)] = value;
-	addr = (void *)(uintptr_t)(CONFIG_MMCONF_BASE_ADDRESS | dev | (reg & ~1));
-	write16(addr, value);
 }
 
 static __always_inline
 void pci_mmio_write_config32(pci_devfn_t dev, uint16_t reg, uint32_t value)
 {
-	void *addr;
+	pcicfg(dev)->reg32[reg / sizeof(uint32_t)] = value;
-	addr = (void *)(uintptr_t)(CONFIG_MMCONF_BASE_ADDRESS | dev | (reg & ~3));
-	write32(addr, value);
 }
 
+#endif /* !defined(__ROMCC__) */
+
 #if CONFIG(MMCONF_SUPPORT)
 
 /* Avoid name collisions as different stages have different signature

src/include/device/pci_type.h

@@ -18,6 +18,10 @@
 
 typedef u32 pci_devfn_t;
 
+/* Convert pci_devfn_t to offset in MMCONF space.
+ * As it is one-to-one,  nothing needs to be done. */
+#define PCI_DEVFN_OFFSET(x) ((x))
+
 #define PCI_DEV(SEGBUS, DEV, FN) ( \
 	(((SEGBUS) & 0xFFF) << 20) | \
 	(((DEV) & 0x1F) << 15) | \