PCI ops: MMCONF_SUPPORT_DEFAULT is required

Doing PCI config operations via MMIO window by default is a
requirement, if supported by the platform. This means chipset
or CPU code must enable MMCONF operations early in bootblock
already, or before platform-specific romstage entry.

Platforms are allowed to have NO_MMCONF_SUPPORT only in the
case it is actually not implemented in the silicon.

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

src/Kconfig

@@ -550,15 +550,6 @@ config MAX_CPUS
 	int
 	default 1
 
-config MMCONF_SUPPORT_DEFAULT
-	bool
-	default n
-
-config MMCONF_SUPPORT
-	bool
-	default y if MMCONF_SUPPORT_DEFAULT
-	default n
-
 source "src/console/Kconfig"
 
 config HAVE_ACPI_RESUME

src/arch/x86/include/arch/io.h

@@ -249,7 +249,7 @@ typedef u32 device_t;
 static inline __attribute__((always_inline))
 uint8_t pci_read_config8(pci_devfn_t dev, unsigned int where)
 {
-	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT_DEFAULT))
+	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT))
 		return pci_mmio_read_config8(dev, where);
 	else
 		return pci_io_read_config8(dev, where);
@@ -258,7 +258,7 @@ uint8_t pci_read_config8(pci_devfn_t dev, unsigned int where)
 static inline __attribute__((always_inline))
 uint16_t pci_read_config16(pci_devfn_t dev, unsigned int where)
 {
-	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT_DEFAULT))
+	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT))
 		return pci_mmio_read_config16(dev, where);
 	else
 		return pci_io_read_config16(dev, where);
@@ -267,7 +267,7 @@ uint16_t pci_read_config16(pci_devfn_t dev, unsigned int where)
 static inline __attribute__((always_inline))
 uint32_t pci_read_config32(pci_devfn_t dev, unsigned int where)
 {
-	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT_DEFAULT))
+	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT))
 		return pci_mmio_read_config32(dev, where);
 	else
 		return pci_io_read_config32(dev, where);
@@ -276,7 +276,7 @@ uint32_t pci_read_config32(pci_devfn_t dev, unsigned int where)
 static inline __attribute__((always_inline))
 void pci_write_config8(pci_devfn_t dev, unsigned int where, uint8_t value)
 {
-	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT_DEFAULT))
+	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT))
 		pci_mmio_write_config8(dev, where, value);
 	else
 		pci_io_write_config8(dev, where, value);
@@ -285,7 +285,7 @@ void pci_write_config8(pci_devfn_t dev, unsigned int where, uint8_t value)
 static inline __attribute__((always_inline))
 void pci_write_config16(pci_devfn_t dev, unsigned int where, uint16_t value)
 {
-	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT_DEFAULT))
+	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT))
 		pci_mmio_write_config16(dev, where, value);
 	else
 		pci_io_write_config16(dev, where, value);
@@ -294,7 +294,7 @@ void pci_write_config16(pci_devfn_t dev, unsigned int where, uint16_t value)
 static inline __attribute__((always_inline))
 void pci_write_config32(pci_devfn_t dev, unsigned where, uint32_t value)
 {
-	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT_DEFAULT))
+	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT))
 		pci_mmio_write_config32(dev, where, value);
 	else
 		pci_io_write_config32(dev, where, value);

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

@@ -17,10 +17,7 @@
 #ifndef __SIMPLE_DEVICE__
 
 extern const struct pci_bus_operations pci_cf8_conf1;
-
-#if CONFIG_MMCONF_SUPPORT
 extern const struct pci_bus_operations pci_ops_mmconf;
-#endif
 
 const struct pci_bus_operations *pci_bus_default_ops(device_t dev);
 

src/device/Kconfig

@@ -217,31 +217,42 @@ config PCI
 	bool
 	default n
 
+if PCI
+
+config NO_MMCONF_SUPPORT
+	bool
+	default !MMCONF_SUPPORT_DEFAULT
+
+config MMCONF_SUPPORT
+	bool
+	default MMCONF_SUPPORT_DEFAULT
+
+config MMCONF_SUPPORT_DEFAULT
+	bool
+	default n
+
 config HYPERTRANSPORT_PLUGIN_SUPPORT
 	bool
-	depends on PCI
 	default n
 
 config PCIX_PLUGIN_SUPPORT
 	bool
-	depends on PCI
 	default y
 
 config CARDBUS_PLUGIN_SUPPORT
 	bool
-	depends on PCI
 	default y
 
 config AZALIA_PLUGIN_SUPPORT
 	bool
-	depends on PCI
 	default n
 
 config PCIEXP_PLUGIN_SUPPORT
 	bool
-	depends on PCI
 	default y
 
+endif # PCI
+
 if PCIEXP_PLUGIN_SUPPORT
 
 config PCIEXP_COMMON_CLOCK
@@ -268,7 +279,7 @@ config PCIEXP_CLK_PM
 config PCIEXP_L1_SUB_STATE
 	prompt "Enable PCIe ASPM L1 SubState"
 	bool
-	depends on (MMCONF_SUPPORT_DEFAULT || PCI_IO_CFG_EXT)
+	depends on (MMCONF_SUPPORT || PCI_IO_CFG_EXT)
 	default n
 	help
 	  Detect and enable ASPM on PCIe links.

src/device/pci_ops.c

@@ -22,11 +22,10 @@
 
 const struct pci_bus_operations *pci_bus_default_ops(device_t dev)
 {
-#if CONFIG_MMCONF_SUPPORT_DEFAULT
+	if (IS_ENABLED(CONFIG_NO_MMCONF_SUPPORT))
+		return &pci_cf8_conf1;
+
 	return &pci_ops_mmconf;
-#else
-	return &pci_cf8_conf1;
-#endif
 }
 
 static const struct pci_bus_operations *pci_bus_ops(struct bus *bus, struct device *dev)

src/mainboard/emulation/qemu-q35/bootblock.c

@@ -10,12 +10,12 @@ static void bootblock_northbridge_init(void)
 
 	/*
 	 * The "io" variant of the config access is explicitly used to
-	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT_DEFAULT is set to
+	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT is set to
 	 * to true. That way all subsequent non-explicit config accesses use
 	 * MCFG. This code also assumes that bootblock_northbridge_init() is
 	 * the first thing called in the non-asm boot block code. The final
 	 * assumption is that no assembly code is using the
-	 * CONFIG_MMCONF_SUPPORT_DEFAULT option to do PCI config acceses.
+	 * CONFIG_MMCONF_SUPPORT option to do PCI config acceses.
 	 *
 	 * The PCIEXBAR is assumed to live in the memory mapped IO space under
 	 * 4GiB.

src/mainboard/google/beltino/Kconfig

@@ -9,7 +9,6 @@ config BOARD_GOOGLE_BASEBOARD_BELTINO
 	select HAVE_ACPI_TABLES
 	select HAVE_OPTION_TABLE
 	select HAVE_ACPI_RESUME
-	select MMCONF_SUPPORT
 	select HAVE_SMI_HANDLER
 	select MAINBOARD_HAS_CHROMEOS
 	select MAINBOARD_HAS_LPC_TPM

src/northbridge/amd/amdfam10/northbridge.c

@@ -740,19 +740,18 @@ static void amdfam10_domain_read_resources(device_t dev)
 
 	pci_domain_read_resources(dev);
 
-	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT)) {
+	/* We have MMCONF_SUPPORT, create the resource window. */
-		struct resource *res = new_resource(dev, 0xc0010058);
+	struct resource *res = new_resource(dev, 0xc0010058);
-		res->base = CONFIG_MMCONF_BASE_ADDRESS;
+	res->base = CONFIG_MMCONF_BASE_ADDRESS;
-		res->size = CONFIG_MMCONF_BUS_NUMBER * 1024 * 1024;	/* Each bus needs 1M */
+	res->size = CONFIG_MMCONF_BUS_NUMBER * 1024 * 1024;	/* Each bus needs 1M */
-		res->align = log2(res->size);
+	res->align = log2(res->size);
-		res->gran = log2(res->size);
+	res->gran = log2(res->size);
-		res->limit = 0xffffffffffffffffULL;			/* 64-bit location allowed */
+	res->limit = 0xffffffffffffffffULL;			/* 64-bit location allowed */
-		res->flags = IORESOURCE_MEM | IORESOURCE_RESERVE |
+	res->flags = IORESOURCE_MEM | IORESOURCE_RESERVE |
-			IORESOURCE_FIXED | IORESOURCE_STORED |  IORESOURCE_ASSIGNED;
+		IORESOURCE_FIXED | IORESOURCE_STORED |  IORESOURCE_ASSIGNED;
 
-		/* Reserve lower DRAM region to force PCI MMIO region to correct location above 0xefffffff */
+	/* Reserve lower DRAM region to force PCI MMIO region to correct location above 0xefffffff */
-		ram_resource(dev, 7, 0, rdmsr(TOP_MEM).lo >> 10);
+	ram_resource(dev, 7, 0, rdmsr(TOP_MEM).lo >> 10);
-	}
 
 	if (is_fam15h()) {
 		enable_cc6 = 0;

src/northbridge/amd/pi/00630F01/northbridge.c

@@ -335,8 +335,7 @@ static void read_resources(device_t dev)
 	 * It is not honored by the coreboot resource allocator if it is in
 	 * the CPU_CLUSTER.
 	 */
-	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT))
+	enable_mmconf_resource(dev);
-		enable_mmconf_resource(dev);
 }
 
 static void set_resource(device_t dev, struct resource *resource, u32 nodeid)

src/northbridge/amd/pi/00660F01/northbridge.c

@@ -330,8 +330,7 @@ static void read_resources(device_t dev)
 	 * It is not honored by the coreboot resource allocator if it is in
 	 * the CPU_CLUSTER.
 	 */
-	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT))
+	enable_mmconf_resource(dev);
-		enable_mmconf_resource(dev);
 }
 
 static void set_resource(device_t dev, struct resource *resource, u32 nodeid)

src/northbridge/amd/pi/00670F00/northbridge.c

@@ -330,8 +330,7 @@ static void read_resources(device_t dev)
 	 * It is not honored by the coreboot resource allocator if it is in
 	 * the CPU_CLUSTER.
 	 */
-	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT))
+	enable_mmconf_resource(dev);
-		enable_mmconf_resource(dev);
 }
 
 static void set_resource(device_t dev, struct resource *resource, u32 nodeid)

src/northbridge/amd/pi/00730F01/northbridge.c

@@ -338,8 +338,7 @@ static void read_resources(device_t dev)
 	 * It is not honored by the coreboot resource allocator if it is in
 	 * the CPU_CLUSTER.
 	 */
-	if (IS_ENABLED(CONFIG_MMCONF_SUPPORT))
+	enable_mmconf_resource(dev);
-		enable_mmconf_resource(dev);
 }
 
 static void set_resource(device_t dev, struct resource *resource, u32 nodeid)

src/northbridge/intel/fsp_sandybridge/bootblock.c

@@ -9,12 +9,12 @@ static void bootblock_northbridge_init(void)
 
 	/*
 	 * The "io" variant of the config access is explicitly used to
-	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT_DEFAULT is set to
+	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT is set to
 	 * to true. That way all subsequent non-explicit config accesses use
 	 * MCFG. This code also assumes that bootblock_northbridge_init() is
 	 * the first thing called in the non-asm boot block code. The final
 	 * assumption is that no assembly code is using the
-	 * CONFIG_MMCONF_SUPPORT_DEFAULT option to do PCI config acceses.
+	 * CONFIG_MMCONF_SUPPORT option to do PCI config acceses.
 	 *
 	 * The PCIEXBAR is assumed to live in the memory mapped IO space under
 	 * 4GiB.

src/northbridge/intel/gm45/bootblock.c

@@ -10,12 +10,12 @@ static void bootblock_northbridge_init(void)
 
 	/*
 	 * The "io" variant of the config access is explicitly used to
-	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT_DEFAULT is set to
+	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT is set to
 	 * to true. That way all subsequent non-explicit config accesses use
 	 * MCFG. This code also assumes that bootblock_northbridge_init() is
 	 * the first thing called in the non-asm boot block code. The final
 	 * assumption is that no assembly code is using the
-	 * CONFIG_MMCONF_SUPPORT_DEFAULT option to do PCI config accesses.
+	 * CONFIG_MMCONF_SUPPORT option to do PCI config accesses.
 	 *
 	 * The PCIEXBAR is assumed to live in the memory mapped IO space under
 	 * 4GiB.

src/northbridge/intel/haswell/bootblock.c

@@ -9,12 +9,12 @@ static void bootblock_northbridge_init(void)
 
 	/*
 	 * The "io" variant of the config access is explicitly used to
-	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT_DEFAULT is set to
+	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT is set to
 	 * to true. That way all subsequent non-explicit config accesses use
 	 * MCFG. This code also assumes that bootblock_northbridge_init() is
 	 * the first thing called in the non-asm boot block code. The final
 	 * assumption is that no assembly code is using the
-	 * CONFIG_MMCONF_SUPPORT_DEFAULT option to do PCI config acceses.
+	 * CONFIG_MMCONF_SUPPORT option to do PCI config acceses.
 	 *
 	 * The PCIEXBAR is assumed to live in the memory mapped IO space under
 	 * 4GiB.

src/northbridge/intel/i5000/bootblock.c

@@ -4,12 +4,12 @@ static void bootblock_northbridge_init(void)
 {
 	/*
 	 * The "io" variant of the config access is explicitly used to
-	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT_DEFAULT is set to
+	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT is set to
 	 * to true. That way all subsequent non-explicit config accesses use
 	 * MCFG. This code also assumes that bootblock_northbridge_init() is
 	 * the first thing called in the non-asm boot block code. The final
 	 * assumption is that no assembly code is using the
-	 * CONFIG_MMCONF_SUPPORT_DEFAULT option to do PCI config acceses.
+	 * CONFIG_MMCONF_SUPPORT option to do PCI config acceses.
 	 *
 	 * The PCIEXBAR is assumed to live in the memory mapped IO space under
 	 * 4GiB.

src/northbridge/intel/i945/bootblock.c

@@ -9,12 +9,12 @@ static void bootblock_northbridge_init(void)
 
 	/*
 	 * The "io" variant of the config access is explicitly used to
-	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT_DEFAULT is set to
+	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT is set to
 	 * to true. That way all subsequent non-explicit config accesses use
 	 * MCFG. This code also assumes that bootblock_northbridge_init() is
 	 * the first thing called in the non-asm boot block code. The final
 	 * assumption is that no assembly code is using the
-	 * CONFIG_MMCONF_SUPPORT_DEFAULT option to do PCI config acceses.
+	 * CONFIG_MMCONF_SUPPORT option to do PCI config acceses.
 	 *
 	 * The PCIEXBAR is assumed to live in the memory mapped IO space under
 	 * 4GiB.

src/northbridge/intel/sandybridge/bootblock.c

@@ -9,12 +9,12 @@ static void bootblock_northbridge_init(void)
 
 	/*
 	 * The "io" variant of the config access is explicitly used to
-	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT_DEFAULT is set to
+	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT is set to
 	 * to true. That way all subsequent non-explicit config accesses use
 	 * MCFG. This code also assumes that bootblock_northbridge_init() is
 	 * the first thing called in the non-asm boot block code. The final
 	 * assumption is that no assembly code is using the
-	 * CONFIG_MMCONF_SUPPORT_DEFAULT option to do PCI config acceses.
+	 * CONFIG_MMCONF_SUPPORT option to do PCI config acceses.
 	 *
 	 * The PCIEXBAR is assumed to live in the memory mapped IO space under
 	 * 4GiB.

src/northbridge/via/vx900/early_vx900.c

@@ -40,14 +40,13 @@ void vx900_enable_pci_config_space(void)
 	 * accessed */
 	pci_io_write_config8(HOST_CTR, 0x4f, 0x01);
 
-#if CONFIG_MMCONF_SUPPORT
 	/* COOL, now enable MMCONF */
 	u8 reg8 = pci_io_read_config8(TRAF_CTR, 0x60);
 	reg8 |= 3;
 	pci_io_write_config8(TRAF_CTR, 0x60, reg8);
+
 	reg8 = CONFIG_MMCONF_BASE_ADDRESS >> 28;
 	pci_io_write_config8(TRAF_CTR, 0x61, reg8);
-#endif
 }
 
 /**

src/soc/intel/broadwell/bootblock/systemagent.c

@@ -23,12 +23,12 @@ static void bootblock_northbridge_init(void)
 
 	/*
 	 * The "io" variant of the config access is explicitly used to
-	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT_DEFAULT is set to
+	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT is set to
 	 * to true. That way all subsequent non-explicit config accesses use
 	 * MCFG. This code also assumes that bootblock_northbridge_init() is
 	 * the first thing called in the non-asm boot block code. The final
 	 * assumption is that no assembly code is using the
-	 * CONFIG_MMCONF_SUPPORT_DEFAULT option to do PCI config accesses.
+	 * CONFIG_MMCONF_SUPPORT option to do PCI config accesses.
 	 *
 	 * The PCIEXBAR is assumed to live in the memory mapped IO space under
 	 * 4GiB.

src/soc/intel/skylake/bootblock/systemagent.c

@@ -26,12 +26,12 @@ void bootblock_systemagent_early_init(void)
 
 	/*
 	 * The "io" variant of the config access is explicitly used to
-	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT_DEFAULT is set to
+	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT is set to
 	 * to true. That way all subsequent non-explicit config accesses use
 	 * MCFG. This code also assumes that bootblock_northbridge_init() is
 	 * the first thing called in the non-asm boot block code. The final
 	 * assumption is that no assembly code is using the
-	 * CONFIG_MMCONF_SUPPORT_DEFAULT option to do PCI config acceses.
+	 * CONFIG_MMCONF_SUPPORT option to do PCI config acceses.
 	 *
 	 * The PCIEXBAR is assumed to live in the memory mapped IO space under
 	 * 4GiB.

src/southbridge/intel/i82801gx/i82801gx.c

@@ -20,10 +20,6 @@
 #include "i82801gx.h"
 #include "sata.h"
 
-#if !CONFIG_MMCONF_SUPPORT_DEFAULT
-#error ICH7 requires CONFIG_MMCONF_SUPPORT_DEFAULT
-#endif
-
 void i82801gx_enable(device_t dev)
 {
 	u32 reg32;

src/southbridge/intel/i82801ix/i82801ix.c

@@ -23,10 +23,6 @@
 #include <console/console.h>
 #include "i82801ix.h"
 
-#if !CONFIG_MMCONF_SUPPORT_DEFAULT
-#error ICH9 requires CONFIG_MMCONF_SUPPORT_DEFAULT
-#endif
-
 typedef struct southbridge_intel_i82801ix_config config_t;
 
 static void i82801ix_enable_device(device_t dev)