| Message ID | 20201117033847.365918-1-alex.hung@canonical.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mtrr: refine memory type above 4GB on AMD platforms | expand |
On 17/11/2020 03:38, Alex Hung wrote: > Buglink: https://bugs.launchpad.net/bugs/1901146 > > 1. amd_Tom2ForceMemTypeWB only works from 4GB and top of memory 2 (TOM2) > 2. amd_Tom2ForceMemTypeWB can be overridden by MTRR > 3. amd_Tom2ForceMemTypeWB overrides MTRR_DEF_TYPE > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > src/bios/mtrr/mtrr.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c > index e064f9c4..c9b77941 100644 > --- a/src/bios/mtrr/mtrr.c > +++ b/src/bios/mtrr/mtrr.c > @@ -51,8 +51,10 @@ static fwts_cpuinfo_x86 *fwts_cpuinfo; > > #define MTRR_DEF_TYPE_MSR 0x2FF > #define AMD_SYS_CFG_MSR 0xC0010010 > +#define AMD_TOM2_MSR 0xC001001D > > static uint64_t mtrr_default; > +static uint64_t amd_tom2_addr; > static bool amd_Tom2ForceMemTypeWB = false; > > struct mtrr_entry { > @@ -195,6 +197,9 @@ static int get_default_mtrr(fwts_framework *fw) > if (fwts_cpu_readmsr(fw, 0, AMD_SYS_CFG_MSR, &amd_sys_conf) == FWTS_OK) > if (amd_sys_conf & 0x200000) > amd_Tom2ForceMemTypeWB = true; > + > + if (fwts_cpu_readmsr(fw, 0, AMD_TOM2_MSR, &amd_tom2_addr) != FWTS_OK) > + amd_tom2_addr = 0x100000000; // this should above 4G I'd rather not have // style comments in fwts if that's OK. > } > > if (fwts_cpu_readmsr(fw, 0, MTRR_DEF_TYPE_MSR, &mtrr_default) == FWTS_OK) { > @@ -229,12 +234,6 @@ static int cache_types(uint64_t start, uint64_t end) > struct mtrr_entry *entry; > int type = 0; > > - /* On AMD platforms, Tom2ForceMemTypeWB overwrites other memory types */ > - if (amd_Tom2ForceMemTypeWB && start >= 0x100000000) { > - type = WRITE_BACK; > - return type; > - } > - > fwts_list_foreach(item, mtrr_list) { > entry = fwts_list_data(struct mtrr_entry*, item); > > @@ -242,6 +241,11 @@ static int cache_types(uint64_t start, uint64_t end) > type |= entry->type; > } > > + /* On AMD platforms, Tom2ForceMemTypeWB overwrites MTRRdefType */ > + if (type == 0 && (strstr(fwts_cpuinfo->vendor_id, "AMD") || strstr(fwts_cpuinfo->vendor_id, "Hygon"))) > + if (amd_Tom2ForceMemTypeWB && start >= 0x100000000 && start <= amd_tom2_addr) > + return WRITE_BACK; > + The second if statement could be replaced with a further && clause, something like: if ((type == 0) && (strstr(fwts_cpuinfo->vendor_id, "AMD") || strstr(fwts_cpuinfo->vendor_id, "Hygon")) && (amd_Tom2ForceMemTypeWB && (start >= 0x100000000) && (start <= amd_tom2_addr))) return WRITE_BACK; > /* > * now to see if there is any part of the range that isn't > * covered by an mtrr, since it's UNCACHED if so >
On 2020-11-17 4:29 a.m., Colin Ian King wrote: > On 17/11/2020 03:38, Alex Hung wrote: >> Buglink: https://bugs.launchpad.net/bugs/1901146 >> >> 1. amd_Tom2ForceMemTypeWB only works from 4GB and top of memory 2 (TOM2) >> 2. amd_Tom2ForceMemTypeWB can be overridden by MTRR >> 3. amd_Tom2ForceMemTypeWB overrides MTRR_DEF_TYPE >> >> Signed-off-by: Alex Hung <alex.hung@canonical.com> >> --- >> src/bios/mtrr/mtrr.c | 16 ++++++++++------ >> 1 file changed, 10 insertions(+), 6 deletions(-) >> >> diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c >> index e064f9c4..c9b77941 100644 >> --- a/src/bios/mtrr/mtrr.c >> +++ b/src/bios/mtrr/mtrr.c >> @@ -51,8 +51,10 @@ static fwts_cpuinfo_x86 *fwts_cpuinfo; >> >> #define MTRR_DEF_TYPE_MSR 0x2FF >> #define AMD_SYS_CFG_MSR 0xC0010010 >> +#define AMD_TOM2_MSR 0xC001001D >> >> static uint64_t mtrr_default; >> +static uint64_t amd_tom2_addr; >> static bool amd_Tom2ForceMemTypeWB = false; >> >> struct mtrr_entry { >> @@ -195,6 +197,9 @@ static int get_default_mtrr(fwts_framework *fw) >> if (fwts_cpu_readmsr(fw, 0, AMD_SYS_CFG_MSR, &amd_sys_conf) == FWTS_OK) >> if (amd_sys_conf & 0x200000) >> amd_Tom2ForceMemTypeWB = true; >> + >> + if (fwts_cpu_readmsr(fw, 0, AMD_TOM2_MSR, &amd_tom2_addr) != FWTS_OK) >> + amd_tom2_addr = 0x100000000; // this should above 4G > > I'd rather not have // style comments in fwts if that's OK. Thanks for pointing this out. > >> } >> >> if (fwts_cpu_readmsr(fw, 0, MTRR_DEF_TYPE_MSR, &mtrr_default) == FWTS_OK) { >> @@ -229,12 +234,6 @@ static int cache_types(uint64_t start, uint64_t end) >> struct mtrr_entry *entry; >> int type = 0; >> >> - /* On AMD platforms, Tom2ForceMemTypeWB overwrites other memory types */ >> - if (amd_Tom2ForceMemTypeWB && start >= 0x100000000) { >> - type = WRITE_BACK; >> - return type; >> - } >> - >> fwts_list_foreach(item, mtrr_list) { >> entry = fwts_list_data(struct mtrr_entry*, item); >> >> @@ -242,6 +241,11 @@ static int cache_types(uint64_t start, uint64_t end) >> type |= entry->type; >> } >> >> + /* On AMD platforms, Tom2ForceMemTypeWB overwrites MTRRdefType */ >> + if (type == 0 && (strstr(fwts_cpuinfo->vendor_id, "AMD") || strstr(fwts_cpuinfo->vendor_id, "Hygon"))) >> + if (amd_Tom2ForceMemTypeWB && start >= 0x100000000 && start <= amd_tom2_addr) >> + return WRITE_BACK; >> + > > The second if statement could be replaced with a further && clause, > something like: > > if ((type == 0) && > (strstr(fwts_cpuinfo->vendor_id, "AMD") || > strstr(fwts_cpuinfo->vendor_id, "Hygon")) && > (amd_Tom2ForceMemTypeWB && (start >= 0x100000000) && (start > <= amd_tom2_addr))) I just realized that CPU vendor IDs aren't necessary because amd_Tom2ForceMemTypeWB will never be set without them in the first place, and the variable name is also clear about this too. Therefore, the following condition will be sufficient: if (type == 0 && amd_Tom2ForceMemTypeWB && start >= 0x100000000 && start <= amd_tom2_addr) > return WRITE_BACK; > > > >> /* >> * now to see if there is any part of the range that isn't >> * covered by an mtrr, since it's UNCACHED if so >> >
diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c index e064f9c4..c9b77941 100644 --- a/src/bios/mtrr/mtrr.c +++ b/src/bios/mtrr/mtrr.c @@ -51,8 +51,10 @@ static fwts_cpuinfo_x86 *fwts_cpuinfo; #define MTRR_DEF_TYPE_MSR 0x2FF #define AMD_SYS_CFG_MSR 0xC0010010 +#define AMD_TOM2_MSR 0xC001001D static uint64_t mtrr_default; +static uint64_t amd_tom2_addr; static bool amd_Tom2ForceMemTypeWB = false; struct mtrr_entry { @@ -195,6 +197,9 @@ static int get_default_mtrr(fwts_framework *fw) if (fwts_cpu_readmsr(fw, 0, AMD_SYS_CFG_MSR, &amd_sys_conf) == FWTS_OK) if (amd_sys_conf & 0x200000) amd_Tom2ForceMemTypeWB = true; + + if (fwts_cpu_readmsr(fw, 0, AMD_TOM2_MSR, &amd_tom2_addr) != FWTS_OK) + amd_tom2_addr = 0x100000000; // this should above 4G } if (fwts_cpu_readmsr(fw, 0, MTRR_DEF_TYPE_MSR, &mtrr_default) == FWTS_OK) { @@ -229,12 +234,6 @@ static int cache_types(uint64_t start, uint64_t end) struct mtrr_entry *entry; int type = 0; - /* On AMD platforms, Tom2ForceMemTypeWB overwrites other memory types */ - if (amd_Tom2ForceMemTypeWB && start >= 0x100000000) { - type = WRITE_BACK; - return type; - } - fwts_list_foreach(item, mtrr_list) { entry = fwts_list_data(struct mtrr_entry*, item); @@ -242,6 +241,11 @@ static int cache_types(uint64_t start, uint64_t end) type |= entry->type; } + /* On AMD platforms, Tom2ForceMemTypeWB overwrites MTRRdefType */ + if (type == 0 && (strstr(fwts_cpuinfo->vendor_id, "AMD") || strstr(fwts_cpuinfo->vendor_id, "Hygon"))) + if (amd_Tom2ForceMemTypeWB && start >= 0x100000000 && start <= amd_tom2_addr) + return WRITE_BACK; + /* * now to see if there is any part of the range that isn't * covered by an mtrr, since it's UNCACHED if so
Buglink: https://bugs.launchpad.net/bugs/1901146 1. amd_Tom2ForceMemTypeWB only works from 4GB and top of memory 2 (TOM2) 2. amd_Tom2ForceMemTypeWB can be overridden by MTRR 3. amd_Tom2ForceMemTypeWB overrides MTRR_DEF_TYPE Signed-off-by: Alex Hung <alex.hung@canonical.com> --- src/bios/mtrr/mtrr.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)