[U-Boot] [PATCH 1/3] x86: Add missing DECLARE_GLOBAL_DATA_PTR for mtrr.c
arch/x86/cpu/mtrr.c has access to the U-Boot global data thus DECLARE_GLOBAL_DATA_PTR is needed.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
arch/x86/cpu/mtrr.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c index d5a825d..ac8765f 100644 --- a/arch/x86/cpu/mtrr.c +++ b/arch/x86/cpu/mtrr.c @@ -17,6 +17,8 @@ #include <asm/msr.h> #include <asm/mtrr.h>
+DECLARE_GLOBAL_DATA_PTR; + /* Prepare to adjust MTRRs */ void mtrr_open(struct mtrr_state *state) {
CPUID (EAX 01H) returns MTRR support flag in EDX bit 12. Probe this flag in x86_cpu_init_f() and save it in global data.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
arch/x86/cpu/cpu.c | 7 +++++++ arch/x86/include/asm/global_data.h | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c index 30e5069..ed7905c 100644 --- a/arch/x86/cpu/cpu.c +++ b/arch/x86/cpu/cpu.c @@ -223,6 +223,11 @@ static bool has_cpuid(void) return flag_is_changeable_p(X86_EFLAGS_ID); }
+static bool has_mtrr(void) +{ + return cpuid_edx(0x00000001) & (1 << 12) ? true : false; +} + static int build_vendor_name(char *vendor_name) { struct cpuid_result result; @@ -318,6 +323,8 @@ int x86_cpu_init_f(void) gd->arch.x86_model = c.x86_model; gd->arch.x86_mask = c.x86_mask; gd->arch.x86_device = cpu.device; + + gd->arch.has_mtrr = has_mtrr(); }
return 0; diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h index 24e3052..6499b9e 100644 --- a/arch/x86/include/asm/global_data.h +++ b/arch/x86/include/asm/global_data.h @@ -63,7 +63,8 @@ struct arch_global_data { void *hob_list; /* FSP HOB list */ #endif struct mtrr_request mtrr_req[MAX_MTRR_REQUESTS]; - int mtrr_req_count; + int mtrr_req_count; + int has_mtrr; };
#endif
On 19 January 2015 at 22:01, Bin Meng bmeng.cn@gmail.com wrote:
CPUID (EAX 01H) returns MTRR support flag in EDX bit 12. Probe this flag in x86_cpu_init_f() and save it in global data.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/cpu.c | 7 +++++++ arch/x86/include/asm/global_data.h | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-)
Acked-by: Simon Glass sjg@chromium.org
int mtrr_req_count;
int mtrr_req_count;int has_mtrr;
But I don't think we need the tabs after 'int'.
Regards, Simon
Hi Simon,
On Thu, Jan 22, 2015 at 12:05 AM, Simon Glass sjg@chromium.org wrote:
On 19 January 2015 at 22:01, Bin Meng bmeng.cn@gmail.com wrote:
CPUID (EAX 01H) returns MTRR support flag in EDX bit 12. Probe this flag in x86_cpu_init_f() and save it in global data.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/cpu.c | 7 +++++++ arch/x86/include/asm/global_data.h | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-)
Acked-by: Simon Glass sjg@chromium.org
int mtrr_req_count;
int mtrr_req_count;int has_mtrr;But I don't think we need the tabs after 'int'.
Looks that there is a tab before hob_list, so I chose tab to indent these two. I will send a v2 patch to fix this.
Regards, Bin
On some x86 processors (like Intel Quark) the MTRR registers are not supported. This is reflected by the CPUID (EAX 01H) result EDX[12]. Accessing the MTRR registers on such processors will cause #GP so we must test the support flag before accessing MTRR MSRs.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
arch/x86/cpu/mtrr.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c index ac8765f..68f1b04 100644 --- a/arch/x86/cpu/mtrr.c +++ b/arch/x86/cpu/mtrr.c @@ -22,6 +22,9 @@ DECLARE_GLOBAL_DATA_PTR; /* Prepare to adjust MTRRs */ void mtrr_open(struct mtrr_state *state) { + if (!gd->arch.has_mtrr) + return; + state->enable_cache = dcache_status();
if (state->enable_cache) @@ -33,6 +36,9 @@ void mtrr_open(struct mtrr_state *state) /* Clean up after adjusting MTRRs, and enable them */ void mtrr_close(struct mtrr_state *state) { + if (!gd->arch.has_mtrr) + return; + wrmsrl(MTRR_DEF_TYPE_MSR, state->deftype | MTRR_DEF_TYPE_EN); if (state->enable_cache) enable_caches(); @@ -45,6 +51,9 @@ int mtrr_commit(bool do_caches) uint64_t mask; int i;
+ if (!gd->arch.has_mtrr) + return 0; + mtrr_open(&state); for (i = 0; i < gd->arch.mtrr_req_count; i++, req++) { mask = ~(req->size - 1); @@ -66,6 +75,9 @@ int mtrr_add_request(int type, uint64_t start, uint64_t size) struct mtrr_request *req; uint64_t mask;
+ if (!gd->arch.has_mtrr) + return 0; + if (gd->arch.mtrr_req_count == MAX_MTRR_REQUESTS) return -ENOSPC; req = &gd->arch.mtrr_req[gd->arch.mtrr_req_count++];
Hi Bin,
On 19 January 2015 at 22:01, Bin Meng bmeng.cn@gmail.com wrote:
On some x86 processors (like Intel Quark) the MTRR registers are not supported. This is reflected by the CPUID (EAX 01H) result EDX[12]. Accessing the MTRR registers on such processors will cause #GP so we must test the support flag before accessing MTRR MSRs.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/mtrr.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c index ac8765f..68f1b04 100644 --- a/arch/x86/cpu/mtrr.c +++ b/arch/x86/cpu/mtrr.c @@ -22,6 +22,9 @@ DECLARE_GLOBAL_DATA_PTR; /* Prepare to adjust MTRRs */ void mtrr_open(struct mtrr_state *state) {
if (!gd->arch.has_mtrr)return;state->enable_cache = dcache_status(); if (state->enable_cache)@@ -33,6 +36,9 @@ void mtrr_open(struct mtrr_state *state) /* Clean up after adjusting MTRRs, and enable them */ void mtrr_close(struct mtrr_state *state) {
if (!gd->arch.has_mtrr)return;wrmsrl(MTRR_DEF_TYPE_MSR, state->deftype | MTRR_DEF_TYPE_EN); if (state->enable_cache) enable_caches();@@ -45,6 +51,9 @@ int mtrr_commit(bool do_caches) uint64_t mask; int i;
if (!gd->arch.has_mtrr)return 0;mtrr_open(&state); for (i = 0; i < gd->arch.mtrr_req_count; i++, req++) { mask = ~(req->size - 1);@@ -66,6 +75,9 @@ int mtrr_add_request(int type, uint64_t start, uint64_t size) struct mtrr_request *req; uint64_t mask;
if (!gd->arch.has_mtrr)return 0;
Shouldn't this (and the above) return -ENOSYS?
if (gd->arch.mtrr_req_count == MAX_MTRR_REQUESTS) return -ENOSPC; req = &gd->arch.mtrr_req[gd->arch.mtrr_req_count++];-- 1.8.2.1
Regards, Simon
Hi Simon,
On Thu, Jan 22, 2015 at 12:06 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 19 January 2015 at 22:01, Bin Meng bmeng.cn@gmail.com wrote:
On some x86 processors (like Intel Quark) the MTRR registers are not supported. This is reflected by the CPUID (EAX 01H) result EDX[12]. Accessing the MTRR registers on such processors will cause #GP so we must test the support flag before accessing MTRR MSRs.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/mtrr.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c index ac8765f..68f1b04 100644 --- a/arch/x86/cpu/mtrr.c +++ b/arch/x86/cpu/mtrr.c @@ -22,6 +22,9 @@ DECLARE_GLOBAL_DATA_PTR; /* Prepare to adjust MTRRs */ void mtrr_open(struct mtrr_state *state) {
if (!gd->arch.has_mtrr)return;state->enable_cache = dcache_status(); if (state->enable_cache)@@ -33,6 +36,9 @@ void mtrr_open(struct mtrr_state *state) /* Clean up after adjusting MTRRs, and enable them */ void mtrr_close(struct mtrr_state *state) {
if (!gd->arch.has_mtrr)return;wrmsrl(MTRR_DEF_TYPE_MSR, state->deftype | MTRR_DEF_TYPE_EN); if (state->enable_cache) enable_caches();@@ -45,6 +51,9 @@ int mtrr_commit(bool do_caches) uint64_t mask; int i;
if (!gd->arch.has_mtrr)return 0;mtrr_open(&state); for (i = 0; i < gd->arch.mtrr_req_count; i++, req++) { mask = ~(req->size - 1);@@ -66,6 +75,9 @@ int mtrr_add_request(int type, uint64_t start, uint64_t size) struct mtrr_request *req; uint64_t mask;
if (!gd->arch.has_mtrr)return 0;Shouldn't this (and the above) return -ENOSYS?
If returning non-zero, the init_cache_f_r() will fail as it checks the return value. Do you think we should ignore the return value there? But if ignored, there is no meaning of returning error codes here.
if (gd->arch.mtrr_req_count == MAX_MTRR_REQUESTS) return -ENOSPC; req = &gd->arch.mtrr_req[gd->arch.mtrr_req_count++];-- 1.8.2.1
Regards, Simon
Regards, Bin
Hi Bin,
On 21 January 2015 at 09:19, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Thu, Jan 22, 2015 at 12:06 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 19 January 2015 at 22:01, Bin Meng bmeng.cn@gmail.com wrote:
On some x86 processors (like Intel Quark) the MTRR registers are not supported. This is reflected by the CPUID (EAX 01H) result EDX[12]. Accessing the MTRR registers on such processors will cause #GP so we must test the support flag before accessing MTRR MSRs.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/mtrr.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/arch/x86/cpu/mtrr.c b/arch/x86/cpu/mtrr.c index ac8765f..68f1b04 100644 --- a/arch/x86/cpu/mtrr.c +++ b/arch/x86/cpu/mtrr.c @@ -22,6 +22,9 @@ DECLARE_GLOBAL_DATA_PTR; /* Prepare to adjust MTRRs */ void mtrr_open(struct mtrr_state *state) {
if (!gd->arch.has_mtrr)return;state->enable_cache = dcache_status(); if (state->enable_cache)@@ -33,6 +36,9 @@ void mtrr_open(struct mtrr_state *state) /* Clean up after adjusting MTRRs, and enable them */ void mtrr_close(struct mtrr_state *state) {
if (!gd->arch.has_mtrr)return;wrmsrl(MTRR_DEF_TYPE_MSR, state->deftype | MTRR_DEF_TYPE_EN); if (state->enable_cache) enable_caches();@@ -45,6 +51,9 @@ int mtrr_commit(bool do_caches) uint64_t mask; int i;
if (!gd->arch.has_mtrr)return 0;mtrr_open(&state); for (i = 0; i < gd->arch.mtrr_req_count; i++, req++) { mask = ~(req->size - 1);@@ -66,6 +75,9 @@ int mtrr_add_request(int type, uint64_t start, uint64_t size) struct mtrr_request *req; uint64_t mask;
if (!gd->arch.has_mtrr)return 0;Shouldn't this (and the above) return -ENOSYS?
If returning non-zero, the init_cache_f_r() will fail as it checks the return value. Do you think we should ignore the return value there? But if ignored, there is no meaning of returning error codes here.
Yes, I understand, but I think it is better to ignore (just the -ENOSYS) return value in the caller (add a comment in the code if you like) than return an incorrect value indicating that the action was taken.
if (gd->arch.mtrr_req_count == MAX_MTRR_REQUESTS) return -ENOSPC; req = &gd->arch.mtrr_req[gd->arch.mtrr_req_count++];-- 1.8.2.1
Regards, Simon
On 19 January 2015 at 22:01, Bin Meng bmeng.cn@gmail.com wrote:
arch/x86/cpu/mtrr.c has access to the U-Boot global data thus DECLARE_GLOBAL_DATA_PTR is needed.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/mtrr.c | 2 ++ 1 file changed, 2 insertions(+)
Acked-by: Simon Glass sjg@chromium.org
participants (2)
-
Bin Meng -
Simon Glass