Arthur Heymans has uploaded this change for review.

View Change

[UNTESTED]drivers/amd/agesa/s3_mtrr.c: Use a struct over pointer aritmetic

The size of the data used is fixed in this function so there is no
need for this aritmetic.

The function signature will be changed in a followup commit.

Change-Id: I3c6df8951d39695cddd4635360d6407d4d001b0a
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
---
M src/drivers/amd/agesa/s3_mtrr.c
1 file changed, 53 insertions(+), 85 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/44293/1
diff --git a/src/drivers/amd/agesa/s3_mtrr.c b/src/drivers/amd/agesa/s3_mtrr.c
index 6c7ba5f..672eb20 100644
--- a/src/drivers/amd/agesa/s3_mtrr.c
+++ b/src/drivers/amd/agesa/s3_mtrr.c
@@ -8,114 +8,82 @@
#include <string.h>
#include <northbridge/amd/agesa/agesa_helper.h>

-static void write_mtrr(u8 **p_nvram_pos, unsigned int idx)
-{
- msr_t msr_data;
- msr_data = rdmsr(idx);
-
- memcpy(*p_nvram_pos, &msr_data, sizeof(msr_data));
- *p_nvram_pos += sizeof(msr_data);
-}
+struct mtrr_backup {
+ msr_t mtrr_fix_64k_00000;
+ msr_t mtrr_fix_16k_80000;
+ msr_t mtrr_fix_16k_a0000;
+ msr_t mtrr_fix_4k_xx000[8]; /* 4K_C0000 -- 4K_F8000 */
+ msr_t mtrr_phys_base[8];
+ msr_t mtrr_phys_mask[8];
+ msr_t syscfg;
+ msr_t top_mem;
+ msr_t top_mem2;
+};

void backup_mtrr(void *mtrr_store, u32 *mtrr_store_size)
{
- u8 *nvram_pos = mtrr_store;
- msr_t msr_data;
- u32 i;
+ msr_t syscfg_msr;
+ struct mtrr_backup *mtrr_save = (struct mtrr_backup *)mtrr_store;
+ uint32_t i;

/* Enable access to AMD RdDram and WrDram extension bits */
- msr_data = rdmsr(SYSCFG_MSR);
- msr_data.lo |= SYSCFG_MSR_MtrrFixDramModEn;
- wrmsr(SYSCFG_MSR, msr_data);
+ syscfg_msr = rdmsr(SYSCFG_MSR);
+ syscfg_msr.lo |= SYSCFG_MSR_MtrrFixDramModEn;
+ wrmsr(SYSCFG_MSR, syscfg_msr);

- /* Fixed MTRRs */
- write_mtrr(&nvram_pos, MTRR_FIX_64K_00000);
- write_mtrr(&nvram_pos, MTRR_FIX_16K_80000);
- write_mtrr(&nvram_pos, MTRR_FIX_16K_A0000);
+ mtrr_save->mtrr_fix_64k_00000 = rdmsr(MTRR_FIX_64K_00000);
+ mtrr_save->mtrr_fix_16k_80000 = rdmsr(MTRR_FIX_16K_80000);
+ mtrr_save->mtrr_fix_16k_a0000 = rdmsr(MTRR_FIX_16K_A0000);

- for (i = MTRR_FIX_4K_C0000; i <= MTRR_FIX_4K_F8000; i++)
- write_mtrr(&nvram_pos, i);
+ for (i = 0; i < 8; i++)
+ mtrr_save->mtrr_fix_4k_xx000[i] = rdmsr(MTRR_FIX_4K_C0000 + i);

/* Disable access to AMD RdDram and WrDram extension bits */
- msr_data = rdmsr(SYSCFG_MSR);
- msr_data.lo &= ~SYSCFG_MSR_MtrrFixDramModEn;
- wrmsr(SYSCFG_MSR, msr_data);
+ syscfg_msr = rdmsr(SYSCFG_MSR);
+ syscfg_msr.lo &= ~SYSCFG_MSR_MtrrFixDramModEn;
+ wrmsr(SYSCFG_MSR, syscfg_msr);

- /* Variable MTRRs */
- for (i = MTRR_PHYS_BASE(0); i < MTRR_PHYS_BASE(8); i++)
- write_mtrr(&nvram_pos, i);
+ for (i = 0; i < 8; i++) {
+ mtrr_save->mtrr_phys_mask[i] = rdmsr(MTRR_PHYS_BASE(i));
+ mtrr_save->mtrr_phys_base[i] = rdmsr(MTRR_PHYS_BASE(i));
+ }

- /* SYSCFG_MSR */
- write_mtrr(&nvram_pos, SYSCFG_MSR);
- /* TOM */
- write_mtrr(&nvram_pos, TOP_MEM);
- /* TOM2 */
- write_mtrr(&nvram_pos, TOP_MEM2);
+ mtrr_save->syscfg = rdmsr(SYSCFG_MSR);
+ mtrr_save->top_mem = rdmsr(TOP_MEM);
+ mtrr_save->top_mem2 = rdmsr(TOP_MEM2);

- *mtrr_store_size = nvram_pos - (u8*) mtrr_store;
+ *mtrr_store_size = sizeof(struct mtrr_backup);
}

void restore_mtrr(void)
{
- volatile u32 *msrPtr = (u32 *) OemS3Saved_MTRR_Storage();
- u32 msr;
- msr_t msr_data;
-
- if (!msrPtr)
- return;
-
- disable_cache();
+ msr_t syscfg_msr;
+ struct mtrr_backup *mtrr_save = (struct mtrr_backup *)OemS3Saved_MTRR_Storage();
+ uint32_t i;

/* Enable access to AMD RdDram and WrDram extension bits */
- msr_data = rdmsr(SYSCFG_MSR);
- msr_data.lo |= SYSCFG_MSR_MtrrFixDramModEn;
- wrmsr(SYSCFG_MSR, msr_data);
+ syscfg_msr = rdmsr(SYSCFG_MSR);
+ syscfg_msr.lo |= SYSCFG_MSR_MtrrFixDramModEn;
+ wrmsr(SYSCFG_MSR, syscfg_msr);

- /* Now restore the Fixed MTRRs */
- msr_data.lo = *msrPtr;
- msrPtr ++;
- msr_data.hi = *msrPtr;
- msrPtr ++;
- wrmsr(MTRR_FIX_64K_00000, msr_data);
+ wrmsr(MTRR_FIX_64K_00000, mtrr_save->mtrr_fix_64k_00000);
+ wrmsr(MTRR_FIX_16K_80000, mtrr_save->mtrr_fix_16k_80000);
+ wrmsr(MTRR_FIX_16K_A0000, mtrr_save->mtrr_fix_16k_a0000);

- msr_data.lo = *msrPtr;
- msrPtr ++;
- msr_data.hi = *msrPtr;
- msrPtr ++;
- wrmsr(MTRR_FIX_16K_80000, msr_data);
-
- msr_data.lo = *msrPtr;
- msrPtr ++;
- msr_data.hi = *msrPtr;
- msrPtr ++;
- wrmsr(MTRR_FIX_16K_A0000, msr_data);
-
- for (msr = MTRR_FIX_4K_C0000; msr <= MTRR_FIX_4K_F8000; msr++) {
- msr_data.lo = *msrPtr;
- msrPtr ++;
- msr_data.hi = *msrPtr;
- msrPtr ++;
- wrmsr(msr, msr_data);
- }
+ for (i = 0; i < 8; i++)
+ wrmsr(MTRR_FIX_4K_C0000 + i, mtrr_save->mtrr_fix_4k_xx000[i]);

/* Disable access to AMD RdDram and WrDram extension bits */
- msr_data = rdmsr(SYSCFG_MSR);
- msr_data.lo &= ~SYSCFG_MSR_MtrrFixDramModEn;
- wrmsr(SYSCFG_MSR, msr_data);
+ syscfg_msr = rdmsr(SYSCFG_MSR);
+ syscfg_msr.lo &= ~SYSCFG_MSR_MtrrFixDramModEn;
+ wrmsr(SYSCFG_MSR, syscfg_msr);

- /* Restore the Variable MTRRs */
- for (msr = MTRR_PHYS_BASE(0); msr <= MTRR_PHYS_MASK(7); msr++) {
- msr_data.lo = *msrPtr;
- msrPtr ++;
- msr_data.hi = *msrPtr;
- msrPtr ++;
- wrmsr(msr, msr_data);
+ for (i = 0; i < 8; i++) {
+ wrmsr(MTRR_PHYS_BASE(i), mtrr_save->mtrr_phys_mask[i]);
+ wrmsr(MTRR_PHYS_BASE(i), mtrr_save->mtrr_phys_base[i]);
}

- /* Restore SYSCFG MTRR */
- msr_data.lo = *msrPtr;
- msrPtr ++;
- msr_data.hi = *msrPtr;
- msrPtr ++;
- wrmsr(SYSCFG_MSR, msr_data);
+ wrmsr(SYSCFG_MSR, mtrr_save->syscfg);
+ wrmsr(TOP_MEM, mtrr_save->top_mem);
+ wrmsr(TOP_MEM2, mtrr_save->top_mem2);
}

To view, visit change 44293. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3c6df8951d39695cddd4635360d6407d4d001b0a
Gerrit-Change-Number: 44293
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-MessageType: newchange