[M] Change in coreboot[main]: security/intel: Use write{64,32,16,8}p and read{64,32,16,8}p

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80194?usp=email )

Change subject: security/intel: Use write{64,32,16,8}p and read{64,32,16,8}p
......................................................................

security/intel: Use write{64,32,16,8}p and read{64,32,16,8}p

Change-Id: I4bdfcd0cc0e2d9b5f884ea7275659c12488715e0
Signed-off-by: Elyes Haouas 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/80194
Reviewed-by: Eric Lai 
Tested-by: build bot (Jenkins) 
Reviewed-by: Felix Held 
---
M src/security/intel/txt/common.c
M src/security/intel/txt/logging.c
M src/security/intel/txt/ramstage.c
M src/security/intel/txt/romstage.c
M src/security/intel/txt/txtlib.c
5 files changed, 57 insertions(+), 57 deletions(-)

Approvals:
  Eric Lai: Looks good to me, approved
  build bot (Jenkins): Verified
  Felix Held: Looks good to me, approved

diff --git a/src/security/intel/txt/common.c b/src/security/intel/txt/common.c
index b78394a..6fba958 100644
--- a/src/security/intel/txt/common.c
+++ b/src/security/intel/txt/common.c
@@ -100,7 +100,7 @@

void intel_txt_log_spad(void)
 {
-	const uint64_t acm_status = read64((void *)TXT_SPAD);
+	const uint64_t acm_status = read64p(TXT_SPAD);

printk(BIOS_INFO, "TXT-STS: ACM verification ");

@@ -138,8 +138,8 @@
    if (!CONFIG(INTEL_TXT))
    	return false;

-	ret = (read8((void *)TXT_ESTS) & TXT_ESTS_WAKE_ERROR_STS) ||
-	      (read64((void *)TXT_E2STS) & TXT_E2STS_SECRET_STS);
+	ret = (read8p(TXT_ESTS) & TXT_ESTS_WAKE_ERROR_STS) ||
+	      (read64p(TXT_E2STS) & TXT_E2STS_SECRET_STS);

if (ret)
    	printk(BIOS_CRIT, "TXT-STS: Secrets in memory!\n");
@@ -154,10 +154,10 @@
     * Chapter B.1.7 and B.1.9
     * Intel TXT Software Development Guide (Document: 315168-015)
     */
-	uint32_t reg = read32((void *)TXT_VER_FSBIF);
+	uint32_t reg = read32p(TXT_VER_FSBIF);

if (reg == 0 || reg == UINT32_MAX)
-		reg = read32((void *)TXT_VER_QPIIF);
+		reg = read32p(TXT_VER_QPIIF);

return (reg & TXT_VER_PRODUCTION_FUSED) ? true : false;
 }
@@ -320,11 +320,11 @@
    	return;

/* FIXME: Do we need to program these two? */
-	//write32((void *)MCU_BASE_ADDR, 0xffe1a990);
-	//write32((void *)APINIT_ADDR, 0xfffffff0);
+	//write32p(MCU_BASE_ADDR, 0xffe1a990);
+	//write32p(APINIT_ADDR, 0xfffffff0);

-	write32((void *)BIOACM_ADDR, (uintptr_t)acm_data);
-	write32((void *)SEMAPHORE, 0);
+	write32p(BIOACM_ADDR, (uintptr_t)acm_data);
+	write32p(SEMAPHORE, 0);

/*
     * The time SCLEAN will take depends on the installed RAM size.
@@ -364,14 +364,14 @@

cbfs_unmap(acm_data);

-	const uint64_t acm_status = read64((void *)TXT_SPAD);
+	const uint64_t acm_status = read64p(TXT_SPAD);
    if (acm_status & ACMERROR_TXT_VALID) {
    	printk(BIOS_ERR, "TEE-TXT: FATAL ACM launch error !\n");
    	/*
    	 * WARNING !
    	 * To clear TXT.BIOSACM.ERRORCODE you must issue a cold reboot!
    	 */
-		intel_txt_log_acm_error(read32((void *)TXT_BIOSACM_ERRORCODE));
+		intel_txt_log_acm_error(read32p(TXT_BIOSACM_ERRORCODE));
    	return -1;
    }

@@ -460,7 +460,7 @@
    if ((eax & 0x7d) != 0x7d)
    	failure = true;

-	const uint64_t status = read64((void *)TXT_SPAD);
+	const uint64_t status = read64p(TXT_SPAD);

if (status & ACMSTS_TXT_DISABLED) {
    	printk(BIOS_INFO, "TEE-TXT: TXT disabled by BIOS policy in FIT.\n");
diff --git a/src/security/intel/txt/logging.c b/src/security/intel/txt/logging.c
index 6485b86..e1692cf 100644
--- a/src/security/intel/txt/logging.c
+++ b/src/security/intel/txt/logging.c
@@ -35,7 +35,7 @@
  */
 static void log_txt_error(const char *phase)
 {
-	const uint64_t txt_error = read64((void *)TXT_ERROR);
+	const uint64_t txt_error = read64p(TXT_ERROR);

if (txt_error & ACMERROR_TXT_VALID) {
    	printk(BIOS_ERR, "%s: Error occurred\n", phase);
@@ -63,9 +63,9 @@

printk(BIOS_INFO, "TEE-TXT: State of ACM and ucode update:\n");

-	bios_acm_error = read32((void *)TXT_BIOSACM_ERRORCODE);
-	acm_status = read64((void *)TXT_SPAD);
-	txt_error = read64((void *)TXT_ERROR);
+	bios_acm_error = read32p(TXT_BIOSACM_ERRORCODE);
+	acm_status = read64p(TXT_SPAD);
+	txt_error = read64p(TXT_ERROR);

/* Errors by BIOS ACM or FIT */
    if ((txt_error & ACMERROR_TXT_VALID) &&
@@ -81,10 +81,10 @@
    }

/* Check for fatal ACM error and TXT reset */
-	uint8_t error = read8((void *)TXT_ESTS);
+	uint8_t error = read8p(TXT_ESTS);
    if (error & TXT_ESTS_TXT_RESET_STS) {
    	printk(BIOS_CRIT, "TXT-STS: Intel TXT reset detected\n");
-		intel_txt_log_acm_error(read32((void *)TXT_ERROR));
+		intel_txt_log_acm_error(read32p(TXT_ERROR));
    }
 }

@@ -178,12 +178,12 @@
 {
    printk(BIOS_INFO, "TEE-TXT: Chipset Key Hash 0x");
    for (int i = 0; i < TXT_ACM_KEY_HASH_LEN; i++) {
-		printk(BIOS_INFO, "%llx", read64((void *)TXT_ACM_KEY_HASH +
+		printk(BIOS_INFO, "%llx", read64p(TXT_ACM_KEY_HASH +
    	       (i * sizeof(uint64_t))));
    }
    printk(BIOS_INFO, "\n");

-	printk(BIOS_INFO, "TEE-TXT: DIDVID 0x%x\n", read32((void *)TXT_DIDVID));
+	printk(BIOS_INFO, "TEE-TXT: DIDVID 0x%x\n", read32p(TXT_DIDVID));
    printk(BIOS_INFO, "TEE-TXT: production fused chipset: %s\n",
           intel_txt_chipset_is_production_fused() ? "true" : "false");
 }
@@ -199,18 +199,18 @@

uint64_t reg64;

-	reg64 = read64((void *)TXT_HEAP_BASE);
+	reg64 = read64p(TXT_HEAP_BASE);
    if ((reg64 != 0 && reg64 != ~0UL) &&
-	    (read64((void *)(uintptr_t)reg64) >= (sizeof(*bdr) + sizeof(uint64_t))))
+	    (read64p((uintptr_t)reg64) >= (sizeof(*bdr) + sizeof(uint64_t))))
    	bdr = (void *)((uintptr_t)reg64 + sizeof(uint64_t));

printk(BIOS_DEBUG, "TEE-TXT: TSEG 0x%lx, size %zu MiB\n", tseg_base, tseg_size / MiB);
-	printk(BIOS_DEBUG, "TEE-TXT: TXT.HEAP.BASE  0x%llx\n", read64((void *)TXT_HEAP_BASE));
-	printk(BIOS_DEBUG, "TEE-TXT: TXT.HEAP.SIZE  0x%llx\n", read64((void *)TXT_HEAP_SIZE));
-	printk(BIOS_DEBUG, "TEE-TXT: TXT.SINIT.BASE 0x%llx\n", read64((void *)TXT_SINIT_BASE));
-	printk(BIOS_DEBUG, "TEE-TXT: TXT.SINIT.SIZE 0x%llx\n", read64((void *)TXT_SINIT_SIZE));
-	printk(BIOS_DEBUG, "TEE-TXT: TXT.MSEG.BASE  0x%llx\n", read64((void *)TXT_MSEG_BASE));
-	printk(BIOS_DEBUG, "TEE-TXT: TXT.MSEG.SIZE  0x%llx\n", read64((void *)TXT_MSEG_SIZE));
+	printk(BIOS_DEBUG, "TEE-TXT: TXT.HEAP.BASE  0x%llx\n", read64p(TXT_HEAP_BASE));
+	printk(BIOS_DEBUG, "TEE-TXT: TXT.HEAP.SIZE  0x%llx\n", read64p(TXT_HEAP_SIZE));
+	printk(BIOS_DEBUG, "TEE-TXT: TXT.SINIT.BASE 0x%llx\n", read64p(TXT_SINIT_BASE));
+	printk(BIOS_DEBUG, "TEE-TXT: TXT.SINIT.SIZE 0x%llx\n", read64p(TXT_SINIT_SIZE));
+	printk(BIOS_DEBUG, "TEE-TXT: TXT.MSEG.BASE  0x%llx\n", read64p(TXT_MSEG_BASE));
+	printk(BIOS_DEBUG, "TEE-TXT: TXT.MSEG.SIZE  0x%llx\n", read64p(TXT_MSEG_SIZE));

if (bdr) {
    	printk(BIOS_DEBUG, "TEE-TXT: BiosDataRegion.bios_sinit_size 0x%x\n",
diff --git a/src/security/intel/txt/ramstage.c b/src/security/intel/txt/ramstage.c
index 8a266a4..5c03da9 100644
--- a/src/security/intel/txt/ramstage.c
+++ b/src/security/intel/txt/ramstage.c
@@ -20,8 +20,8 @@
 /* FIXME: Seems to work only on some platforms */
 static void log_ibb_measurements(void)
 {
-	const uint64_t mseg_size = read64((void *)TXT_MSEG_SIZE);
-	uint64_t mseg_base = read64((void *)TXT_MSEG_BASE);
+	const uint64_t mseg_size = read64p(TXT_MSEG_SIZE);
+	uint64_t mseg_base = read64p(TXT_MSEG_BASE);

if (!mseg_size || !mseg_base || mseg_size <= mseg_base)
    	return;
@@ -34,14 +34,14 @@

printk(BIOS_INFO, "TEE-TXT: IBB Hash 0x");
    for (; mseg_base < mseg_size; mseg_base++)
-		printk(BIOS_INFO, "%02X", read8((void *)(uintptr_t)mseg_base));
+		printk(BIOS_INFO, "%02X", read8p((uintptr_t)mseg_base));

printk(BIOS_INFO, "\n");
 }

void bootmem_platform_add_ranges(void)
 {
-	uint64_t status = read64((void *)TXT_SPAD);
+	uint64_t status = read64p(TXT_SPAD);

if (status & ACMSTS_TXT_DISABLED)
    	return;
@@ -67,7 +67,7 @@
    		  BM_MEM_RESERVED);

const union dpr_register dpr = {
-		.raw = read32((void *)TXT_DPR),
+		.raw = read32p(TXT_DPR),
    };

const uint32_t dpr_base = dpr.top - dpr.size * MiB;
@@ -78,13 +78,13 @@

static bool get_wake_error_status(void)
 {
-	const uint8_t error = read8((void *)TXT_ESTS);
+	const uint8_t error = read8p(TXT_ESTS);
    return !!(error & TXT_ESTS_WAKE_ERROR_STS);
 }

static void check_secrets_txt(void *unused)
 {
-	uint64_t status = read64((void *)TXT_SPAD);
+	uint64_t status = read64p(TXT_SPAD);

if (status & ACMSTS_TXT_DISABLED)
    	return;
@@ -102,7 +102,7 @@
    	intel_txt_run_bios_acm(ACMINPUT_CLEAR_SECRETS);

/* Should never reach this point ... */
-		intel_txt_log_acm_error(read32((void *)TXT_BIOSACM_ERRORCODE));
+		intel_txt_log_acm_error(read32p(TXT_BIOSACM_ERRORCODE));
    	die("Waiting for platform reset...\n");
    }
 }
@@ -121,7 +121,7 @@
  */
 static void init_intel_txt(void *unused)
 {
-	const uint64_t status = read64((void *)TXT_SPAD);
+	const uint64_t status = read64p(TXT_SPAD);

if (status & ACMSTS_TXT_DISABLED)
    	return;
@@ -250,10 +250,10 @@
    txt_heap_fill_common_bdr(&data.bdr);
    txt_heap_fill_bios_spec(&data.spec);

-	void *sinit_base = (void *)(uintptr_t)read64((void *)TXT_SINIT_BASE);
+	void *sinit_base = (void *)(uintptr_t)read64p(TXT_SINIT_BASE);
    data.bdr.bios_sinit_size = cbfs_load(CONFIG_INTEL_TXT_CBFS_SINIT_ACM,
    				     sinit_base,
-					     read64((void *)TXT_SINIT_SIZE));
+					     read64p(TXT_SINIT_SIZE));

/* Extended elements - ACM addresses */
    data.heap_acm.header.type = HEAP_EXTDATA_TYPE_ACM;
@@ -295,9 +295,9 @@
    txt_heap_fill_common_bdr(&data.bdr);
    txt_heap_fill_bios_spec(&data.spec);

-	void *sinit_base = (void *)(uintptr_t)read64((void *)TXT_SINIT_BASE);
+	void *sinit_base = (void *)(uintptr_t)read64p(TXT_SINIT_BASE);
    /* Clear SINIT ACM memory */
-	memset(sinit_base, 0, read64((void *)TXT_SINIT_SIZE));
+	memset(sinit_base, 0, read64p(TXT_SINIT_SIZE));

/* Extended elements - ACM addresses */
    data.heap_acm.header.type = HEAP_EXTDATA_TYPE_ACM;
@@ -318,7 +318,7 @@
 static void txt_initialize_heap(void)
 {
    /* Fill TXT.HEAP.BASE with 4 subregions */
-	u8 *heap_struct = (void *)((uintptr_t)read64((void *)TXT_HEAP_BASE));
+	u8 *heap_struct = (void *)((uintptr_t)read64p(TXT_HEAP_BASE));

/*
     * Since we may have either BIOS ACM or both BIOS and SINIT ACMs in
@@ -365,7 +365,7 @@
    if (skip_intel_txt_lockdown())
    	return;

-	const uint64_t status = read64((void *)TXT_SPAD);
+	const uint64_t status = read64p(TXT_SPAD);

uint32_t txt_feature_flags = 0;
    uintptr_t tseg_base;
@@ -401,7 +401,7 @@
     * Chapter 5.5.6.1 DMA Protection Memory Region
     */

-	const u8 dpr_capable = !!(read64((void *)TXT_CAPABILITIES) &
+	const u8 dpr_capable = !!(read64p(TXT_CAPABILITIES) &
    			  TXT_CAPABILITIES_DPR);
    printk(BIOS_INFO, "TEE-TXT: DPR capable %x\n", dpr_capable);

@@ -443,28 +443,28 @@
    	dpr.prs = 0;
    	dpr.epm = 0;

-		write64((void *)TXT_DPR, dpr.raw);
+		write64p(TXT_DPR, dpr.raw);

printk(BIOS_INFO, "TEE-TXT: TXT.DPR 0x%08x\n",
-		       read32((void *)TXT_DPR));
+		       read32p(TXT_DPR));
    }

/*
     * Document Number: 558294
     * Chapter 5.5.6.3 Intel TXT Heap Memory Region
     */
-	write64((void *)TXT_HEAP_SIZE, CONFIG_INTEL_TXT_HEAP_SIZE);
-	write64((void *)TXT_HEAP_BASE,
-		ALIGN_DOWN(tseg_base - read64((void *)TXT_HEAP_SIZE), 4096));
+	write64p(TXT_HEAP_SIZE, CONFIG_INTEL_TXT_HEAP_SIZE);
+	write64p(TXT_HEAP_BASE,
+		ALIGN_DOWN(tseg_base - read64p(TXT_HEAP_SIZE), 4096));

/*
     * Document Number: 558294
     * Chapter 5.5.6.2 SINIT Memory Region
     */
-	write64((void *)TXT_SINIT_SIZE, CONFIG_INTEL_TXT_SINIT_SIZE);
-	write64((void *)TXT_SINIT_BASE,
-		ALIGN_DOWN(read64((void *)TXT_HEAP_BASE) -
-			   read64((void *)TXT_SINIT_SIZE), 4096));
+	write64p(TXT_SINIT_SIZE, CONFIG_INTEL_TXT_SINIT_SIZE);
+	write64p(TXT_SINIT_BASE,
+		ALIGN_DOWN(read64p(TXT_HEAP_BASE) -
+			   read64p(TXT_SINIT_SIZE), 4096));

/*
     * FIXME: Server-TXT capable platforms need to install an STM in SMM and set up MSEG.
@@ -474,8 +474,8 @@
     * Chapter 5.10.1 SMM in the Intel TXT for Servers Environment
     * Disable MSEG.
     */
-	write64((void *)TXT_MSEG_SIZE, 0);
-	write64((void *)TXT_MSEG_BASE, 0);
+	write64p(TXT_MSEG_SIZE, 0);
+	write64p(TXT_MSEG_BASE, 0);

/* Only initialize the heap on regular boots */
    if (!acpi_is_wakeup_s3())
diff --git a/src/security/intel/txt/romstage.c b/src/security/intel/txt/romstage.c
index aa7cc2c..643e7a9 100644
--- a/src/security/intel/txt/romstage.c
+++ b/src/security/intel/txt/romstage.c
@@ -63,7 +63,7 @@
    	return;
    }

-	const uint8_t txt_ests = read8((void *)TXT_ESTS);
+	const uint8_t txt_ests = read8p(TXT_ESTS);

const bool establishment = is_establishment_bit_asserted();
    const bool is_wake_error = !!(txt_ests & TXT_ESTS_WAKE_ERROR_STS);
diff --git a/src/security/intel/txt/txtlib.c b/src/security/intel/txt/txtlib.c
index 5478206..6ce86a3 100644
--- a/src/security/intel/txt/txtlib.c
+++ b/src/security/intel/txt/txtlib.c
@@ -20,7 +20,7 @@
    stopwatch_init_msecs_expire(&timer, 50);

while (true) {
-		access = read8((void *)TPM_ACCESS_REG);
+		access = read8p(TPM_ACCESS_REG);

/* Register returns all ones if TPM is missing */
    	if (access == 0xff)