[coreboot-gerrit] Change in ...coreboot[master]: soc/amd/common: Add errors for invalid AcpiMmio access

[Marshall Dawson (Code Review)]

Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32934

Change subject: soc/amd/common: Add errors for invalid AcpiMmio access
......................................................................

soc/amd/common: Add errors for invalid AcpiMmio access

Print an error when accessing a reserved or unexpected AcpiMmio
register block.  At a minimum, require the soc to indicate in its
iomap.h file which register blocks it allows coreboot to use.  It
may optionally indicate all that are supported/documented or may
treat unused blocks as reserved.

This patch attempts to dissuade accessing unsupported blocks without
requiring the complexity of structures or reinitializing at the
beginning of a new stage.

TEST=inspect grunt build with objdump with supported and
     unsupported blocks
BUG=b:131682806

Change-Id: I2121df108fd3caf07e5588bc3201bcdd8dcaaa00
Signed-off-by: Marshall Dawson 
---
M src/soc/amd/common/block/acpimmio/mmio_util.c
M src/soc/amd/common/block/include/amdblocks/acpimmio.h
M src/soc/amd/stoneyridge/include/soc/iomap.h
3 files changed, 185 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/32934/1

diff --git a/src/soc/amd/common/block/acpimmio/mmio_util.c b/src/soc/amd/common/block/acpimmio/mmio_util.c
index c8c6988..89edf396 100644
--- a/src/soc/amd/common/block/acpimmio/mmio_util.c
+++ b/src/soc/amd/common/block/acpimmio/mmio_util.c
@@ -16,6 +16,7 @@
 #include <types.h>
 #include <arch/io.h>
 #include <device/mmio.h>
+#include <console/console.h>
 #include <amdblocks/acpimmio_map.h>
 #include <amdblocks/acpimmio.h>

@@ -65,37 +66,55 @@
    pm_io_write16(reg + sizeof(uint16_t), value & 0xffff);
 }

+static void print_error(uintptr_t base, uint8_t reg)
+{
+	printk(BIOS_ERR, "Error: attempting to access offset 0x%x of unsupported AcpiMmio at 0x%lx\n",
+			reg, base);
+}
+
 /* smbus pci read/write - access registers at 0xfed80000 - currently unused */

/* smi read/write - access registers at 0xfed80200 */

uint8_t smi_read8(uint8_t reg)
 {
+	if (!SUPPORTS_ACPIMMIO_SMI_BASE)
+		print_error(ACPIMMIO_SMI_BASE, reg);
    return read8((void *)(ACPIMMIO_SMI_BASE + reg));
 }

uint16_t smi_read16(uint8_t reg)
 {
+	if (!SUPPORTS_ACPIMMIO_SMI_BASE)
+		print_error(ACPIMMIO_SMI_BASE, reg);
    return read16((void *)(ACPIMMIO_SMI_BASE + reg));
 }

uint32_t smi_read32(uint8_t reg)
 {
+	if (!SUPPORTS_ACPIMMIO_SMI_BASE)
+		print_error(ACPIMMIO_SMI_BASE, reg);
    return read32((void *)(ACPIMMIO_SMI_BASE + reg));
 }

void smi_write8(uint8_t reg, uint8_t value)
 {
+	if (!SUPPORTS_ACPIMMIO_SMI_BASE)
+		print_error(ACPIMMIO_SMI_BASE, reg);
    write8((void *)(ACPIMMIO_SMI_BASE + reg), value);
 }

void smi_write16(uint8_t reg, uint16_t value)
 {
+	if (!SUPPORTS_ACPIMMIO_SMI_BASE)
+		print_error(ACPIMMIO_SMI_BASE, reg);
    write16((void *)(ACPIMMIO_SMI_BASE + reg), value);
 }

void smi_write32(uint8_t reg, uint32_t value)
 {
+	if (!SUPPORTS_ACPIMMIO_SMI_BASE)
+		print_error(ACPIMMIO_SMI_BASE, reg);
    write32((void *)(ACPIMMIO_SMI_BASE + reg), value);
 }

@@ -103,31 +122,43 @@

u8 pm_read8(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_PMIO_BASE)
+		print_error(ACPIMMIO_PMIO_BASE, reg);
    return read8((void *)(ACPIMMIO_PMIO_BASE + reg));
 }

u16 pm_read16(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_PMIO_BASE)
+		print_error(ACPIMMIO_PMIO_BASE, reg);
    return read16((void *)(ACPIMMIO_PMIO_BASE + reg));
 }

u32 pm_read32(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_PMIO_BASE)
+		print_error(ACPIMMIO_PMIO_BASE, reg);
    return read32((void *)(ACPIMMIO_PMIO_BASE + reg));
 }

void pm_write8(u8 reg, u8 value)
 {
+	if (!SUPPORTS_ACPIMMIO_PMIO_BASE)
+		print_error(ACPIMMIO_PMIO_BASE, reg);
    write8((void *)(ACPIMMIO_PMIO_BASE + reg), value);
 }

void pm_write16(u8 reg, u16 value)
 {
+	if (!SUPPORTS_ACPIMMIO_PMIO_BASE)
+		print_error(ACPIMMIO_PMIO_BASE, reg);
    write16((void *)(ACPIMMIO_PMIO_BASE + reg), value);
 }

void pm_write32(u8 reg, u32 value)
 {
+	if (!SUPPORTS_ACPIMMIO_PMIO_BASE)
+		print_error(ACPIMMIO_PMIO_BASE, reg);
    write32((void *)(ACPIMMIO_PMIO_BASE + reg), value);
 }

@@ -137,6 +168,8 @@

uint8_t biosram_read8(uint8_t reg)
 {
+	if (!SUPPORTS_ACPIMMIO_BIOSRAM_BASE)
+		print_error(ACPIMMIO_BIOSRAM_BASE, reg); /* could see 1/2/4x */
    return read8((void *)(ACPIMMIO_BIOSRAM_BASE + reg));
 }

@@ -153,6 +186,8 @@

void biosram_write8(uint8_t reg, uint8_t value)
 {
+	if (!SUPPORTS_ACPIMMIO_BIOSRAM_BASE)
+		print_error(ACPIMMIO_BIOSRAM_BASE, reg); /* could see 1/2/4x */
    write8((void *)(ACPIMMIO_BIOSRAM_BASE + reg), value);
 }

@@ -178,31 +213,43 @@

u8 acpi_read8(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_ACPI_BASE)
+		print_error(ACPIMMIO_ACPI_BASE, reg);
    return read8((void *)(ACPIMMIO_ACPI_BASE + reg));
 }

u16 acpi_read16(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_ACPI_BASE)
+		print_error(ACPIMMIO_ACPI_BASE, reg);
    return read16((void *)(ACPIMMIO_ACPI_BASE + reg));
 }

u32 acpi_read32(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_ACPI_BASE)
+		print_error(ACPIMMIO_ACPI_BASE, reg);
    return read32((void *)(ACPIMMIO_ACPI_BASE + reg));
 }

void acpi_write8(u8 reg, u8 value)
 {
+	if (!SUPPORTS_ACPIMMIO_ACPI_BASE)
+		print_error(ACPIMMIO_ACPI_BASE, reg);
    write8((void *)(ACPIMMIO_ACPI_BASE + reg), value);
 }

void acpi_write16(u8 reg, u16 value)
 {
+	if (!SUPPORTS_ACPIMMIO_ACPI_BASE)
+		print_error(ACPIMMIO_ACPI_BASE, reg);
    write16((void *)(ACPIMMIO_ACPI_BASE + reg), value);
 }

void acpi_write32(u8 reg, u32 value)
 {
+	if (!SUPPORTS_ACPIMMIO_ACPI_BASE)
+		print_error(ACPIMMIO_ACPI_BASE, reg);
    write32((void *)(ACPIMMIO_ACPI_BASE + reg), value);
 }

@@ -210,21 +257,29 @@

u8 asf_read8(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_ASF_BASE)
+		print_error(ACPIMMIO_ASF_BASE, reg);
    return read8((void *)(ACPIMMIO_ASF_BASE + reg));
 }

u16 asf_read16(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_ASF_BASE)
+		print_error(ACPIMMIO_ASF_BASE, reg);
    return read16((void *)(ACPIMMIO_ASF_BASE + reg));
 }

void asf_write8(u8 reg, u8 value)
 {
+	if (!SUPPORTS_ACPIMMIO_ASF_BASE)
+		print_error(ACPIMMIO_ASF_BASE, reg);
    write8((void *)(ACPIMMIO_ASF_BASE + reg), value);
 }

void asf_write16(u8 reg, u16 value)
 {
+	if (!SUPPORTS_ACPIMMIO_ASF_BASE)
+		print_error(ACPIMMIO_ASF_BASE, reg);
    write16((void *)(ACPIMMIO_ASF_BASE + reg), value);
 }

@@ -232,21 +287,29 @@

u8 smbus_read8(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_SMBUS_BASE)
+		print_error(ACPIMMIO_SMBUS_BASE, reg);
    return read8((void *)(ACPIMMIO_SMBUS_BASE + reg));
 }

u16 smbus_read16(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_SMBUS_BASE)
+		print_error(ACPIMMIO_SMBUS_BASE, reg);
    return read16((void *)(ACPIMMIO_SMBUS_BASE + reg));
 }

void smbus_write8(u8 reg, u8 value)
 {
+	if (!SUPPORTS_ACPIMMIO_SMBUS_BASE)
+		print_error(ACPIMMIO_SMBUS_BASE, reg);
    write8((void *)(ACPIMMIO_SMBUS_BASE + reg), value);
 }

void smbus_write16(u8 reg, u16 value)
 {
+	if (!SUPPORTS_ACPIMMIO_SMBUS_BASE)
+		print_error(ACPIMMIO_SMBUS_BASE, reg);
    write16((void *)(ACPIMMIO_SMBUS_BASE + reg), value);
 }

@@ -258,31 +321,43 @@

u8 iomux_read8(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_IOMUX_BASE)
+		print_error(ACPIMMIO_IOMUX_BASE, reg);
    return read8((void *)(ACPIMMIO_IOMUX_BASE + reg));
 }

u16 iomux_read16(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_IOMUX_BASE)
+		print_error(ACPIMMIO_IOMUX_BASE, reg);
    return read16((void *)(ACPIMMIO_IOMUX_BASE + reg));
 }

u32 iomux_read32(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_IOMUX_BASE)
+		print_error(ACPIMMIO_IOMUX_BASE, reg);
    return read32((void *)(ACPIMMIO_IOMUX_BASE + reg));
 }

void iomux_write8(u8 reg, u8 value)
 {
+	if (!SUPPORTS_ACPIMMIO_IOMUX_BASE)
+		print_error(ACPIMMIO_IOMUX_BASE, reg);
    write8((void *)(ACPIMMIO_IOMUX_BASE + reg), value);
 }

void iomux_write16(u8 reg, u16 value)
 {
+	if (!SUPPORTS_ACPIMMIO_IOMUX_BASE)
+		print_error(ACPIMMIO_IOMUX_BASE, reg);
    write16((void *)(ACPIMMIO_IOMUX_BASE + reg), value);
 }

void iomux_write32(u8 reg, u32 value)
 {
+	if (!SUPPORTS_ACPIMMIO_IOMUX_BASE)
+		print_error(ACPIMMIO_IOMUX_BASE, reg);
    write32((void *)(ACPIMMIO_IOMUX_BASE + reg), value);
 }

@@ -290,31 +365,43 @@

u8 misc_read8(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_MISC_BASE)
+		print_error(ACPIMMIO_MISC_BASE, reg);
    return read8((void *)(ACPIMMIO_MISC_BASE + reg));
 }

u16 misc_read16(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_MISC_BASE)
+		print_error(ACPIMMIO_MISC_BASE, reg);
    return read16((void *)(ACPIMMIO_MISC_BASE + reg));
 }

u32 misc_read32(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_MISC_BASE)
+		print_error(ACPIMMIO_MISC_BASE, reg);
    return read32((void *)(ACPIMMIO_MISC_BASE + reg));
 }

void misc_write8(u8 reg, u8 value)
 {
+	if (!SUPPORTS_ACPIMMIO_MISC_BASE)
+		print_error(ACPIMMIO_MISC_BASE, reg);
    write8((void *)(ACPIMMIO_MISC_BASE + reg), value);
 }

void misc_write16(u8 reg, u16 value)
 {
+	if (!SUPPORTS_ACPIMMIO_MISC_BASE)
+		print_error(ACPIMMIO_MISC_BASE, reg);
    write16((void *)(ACPIMMIO_MISC_BASE + reg), value);
 }

void misc_write32(u8 reg, u32 value)
 {
+	if (!SUPPORTS_ACPIMMIO_MISC_BASE)
+		print_error(ACPIMMIO_MISC_BASE, reg);
    write32((void *)(ACPIMMIO_MISC_BASE + reg), value);
 }

@@ -328,31 +415,43 @@

uint8_t xhci_pm_read8(uint8_t reg)
 {
+	if (!SUPPORTS_ACPIMMIO_XHCIPM_BASE)
+		print_error(ACPIMMIO_XHCIPM_BASE, reg);
    return read8((void *)(ACPIMMIO_XHCIPM_BASE + reg));
 }

uint16_t xhci_pm_read16(uint8_t reg)
 {
+	if (!SUPPORTS_ACPIMMIO_XHCIPM_BASE)
+		print_error(ACPIMMIO_XHCIPM_BASE, reg);
    return read16((void *)(ACPIMMIO_XHCIPM_BASE + reg));
 }

uint32_t xhci_pm_read32(uint8_t reg)
 {
+	if (!SUPPORTS_ACPIMMIO_XHCIPM_BASE)
+		print_error(ACPIMMIO_XHCIPM_BASE, reg);
    return read32((void *)(ACPIMMIO_XHCIPM_BASE + reg));
 }

void xhci_pm_write8(uint8_t reg, uint8_t value)
 {
+	if (!SUPPORTS_ACPIMMIO_XHCIPM_BASE)
+		print_error(ACPIMMIO_XHCIPM_BASE, reg);
    write8((void *)(ACPIMMIO_XHCIPM_BASE + reg), value);
 }

void xhci_pm_write16(uint8_t reg, uint16_t value)
 {
+	if (!SUPPORTS_ACPIMMIO_XHCIPM_BASE)
+		print_error(ACPIMMIO_XHCIPM_BASE, reg);
    write16((void *)(ACPIMMIO_XHCIPM_BASE + reg), value);
 }

void xhci_pm_write32(uint8_t reg, uint32_t value)
 {
+	if (!SUPPORTS_ACPIMMIO_XHCIPM_BASE)
+		print_error(ACPIMMIO_XHCIPM_BASE, reg);
    write32((void *)(ACPIMMIO_XHCIPM_BASE + reg), value);
 }

@@ -362,10 +461,14 @@

u8 aoac_read8(u8 reg)
 {
+	if (!SUPPORTS_ACPIMMIO_AOAC_BASE)
+		print_error(ACPIMMIO_AOAC_BASE, reg);
    return read8((void *)(ACPIMMIO_AOAC_BASE + reg));
 }

void aoac_write8(u8 reg, u8 value)
 {
+	if (!SUPPORTS_ACPIMMIO_AOAC_BASE)
+		print_error(ACPIMMIO_AOAC_BASE, reg);
    write8((void *)(ACPIMMIO_AOAC_BASE + reg), value);
 }
diff --git a/src/soc/amd/common/block/include/amdblocks/acpimmio.h b/src/soc/amd/common/block/include/amdblocks/acpimmio.h
index 6be856b..32da867 100644
--- a/src/soc/amd/common/block/include/amdblocks/acpimmio.h
+++ b/src/soc/amd/common/block/include/amdblocks/acpimmio.h
@@ -18,6 +18,69 @@
 #ifndef __AMDBLOCKS_ACPIMMIO_H__
 #define __AMDBLOCKS_ACPIMMIO_H__

+/* iomap.h must indicate if the device uses a block, optional if unused. */
+#include <soc/iomap.h>
+#ifndef SUPPORTS_ACPIMMIO_SMI_BASE
+  #define SUPPORTS_ACPIMMIO_SMI_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_PMIO_BASE
+  #define SUPPORTS_ACPIMMIO_PMIO_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_PMIO2_BASE
+  #define SUPPORTS_ACPIMMIO_PMIO2_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_BIOSRAM_BASE
+  #define SUPPORTS_ACPIMMIO_BIOSRAM_BASE	0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_CMOSRAM_BASE
+  #define SUPPORTS_ACPIMMIO_CMOSRAM_BASE	0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_CMOS_BASE
+  #define SUPPORTS_ACPIMMIO_CMOS_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_ACPI_BASE
+  #define SUPPORTS_ACPIMMIO_ACPI_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_ASF_BASE
+  #define SUPPORTS_ACPIMMIO_ASF_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_SMBUS_BASE
+  #define SUPPORTS_ACPIMMIO_SMBUS_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_WDT_BASE
+  #define SUPPORTS_ACPIMMIO_WDT_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_HPET_BASE
+  #define SUPPORTS_ACPIMMIO_HPET_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_IOMUX_BASE
+  #define SUPPORTS_ACPIMMIO_IOMUX_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_MISC_BASE
+  #define SUPPORTS_ACPIMMIO_MISC_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_DPVGA_BASE
+  #define SUPPORTS_ACPIMMIO_DPVGA_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_GPIO0_BASE
+  #define SUPPORTS_ACPIMMIO_GPIO0_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_GPIO1_BASE
+  #define SUPPORTS_ACPIMMIO_GPIO1_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_GPIO2_BASE
+  #define SUPPORTS_ACPIMMIO_GPIO2_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_XHCIPM_BASE
+  #define SUPPORTS_ACPIMMIO_XHCIPM_BASE		0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_ACDCTMR_BASE
+  #define SUPPORTS_ACPIMMIO_ACDCTMR_BASE	0
+#endif
+#ifndef SUPPORTS_ACPIMMIO_AOAC_BASE
+  #define SUPPORTS_ACPIMMIO_AOAC_BASE		0
+#endif
+
 /*
  * The following AcpiMmio register block mapping represents definitions
  * that have been documented in AMD publications.  All blocks aren't
diff --git a/src/soc/amd/stoneyridge/include/soc/iomap.h b/src/soc/amd/stoneyridge/include/soc/iomap.h
index 3856a22..da8906a 100644
--- a/src/soc/amd/stoneyridge/include/soc/iomap.h
+++ b/src/soc/amd/stoneyridge/include/soc/iomap.h
@@ -22,8 +22,26 @@
 #define SPI_BASE_ADDRESS		0xfec10000
 #define IO_APIC2_ADDR			0xfec20000

-/* AcpiMmio blocks are at fixed offsets from FED8_0000h, enabled in PMx04[1] */
+/*
+ * AcpiMmio blocks are at fixed offsets from FED8_0000h, enabled in PMx04[1].
+ * All ranges not specified as supported below may, or may not, be listed in
+ * any documentation but should be considered reserved through FED8_1FFFh.
+ */
 #include <amdblocks/acpimmio_map.h>
+#define SUPPORTS_ACPIMMIO_SMI_BASE	1 /* 0xfed80100 */
+#define SUPPORTS_ACPIMMIO_PMIO_BASE	1 /* 0xfed80300 */
+#define SUPPORTS_ACPIMMIO_PMIO2_BASE	1 /* 0xfed80400 */
+#define SUPPORTS_ACPIMMIO_BIOSRAM_BASE	1 /* 0xfed80500 */
+#define SUPPORTS_ACPIMMIO_ACPI_BASE	1 /* 0xfed80800 */
+#define SUPPORTS_ACPIMMIO_ASF_BASE	1 /* 0xfed80900 */
+#define SUPPORTS_ACPIMMIO_SMBUS_BASE	1 /* 0xfed80a00 */
+#define SUPPORTS_ACPIMMIO_IOMUX_BASE	1 /* 0xfed80d00 */
+#define SUPPORTS_ACPIMMIO_MISC_BASE	1 /* 0xfed80e00 */
+#define SUPPORTS_ACPIMMIO_GPIO0_BASE	1 /* 0xfed81500 */
+#define SUPPORTS_ACPIMMIO_GPIO1_BASE	1 /* 0xfed81800 */
+#define SUPPORTS_ACPIMMIO_GPIO2_BASE	1 /* 0xfed81700 */
+#define SUPPORTS_ACPIMMIO_XHCIPM_BASE	1 /* 0xfed81c00 */
+#define SUPPORTS_ACPIMMIO_AOAC_BASE	1 /* 0xfed81e00 */

#define ALINK_AHB_ADDRESS		0xfedc0000