Re: [U-Boot] [PATCH 3/3] x86: Test mtrr support flag before accessing mtrr msr
Hi Simon,
On Thu, Jan 22, 2015 at 12:06 AM, Simon Glass s...@chromium.org wrote:
Hi Bin,
On 19 January 2015 at 22:01, Bin Meng bmeng...@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...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/3] x86: Test mtrr support flag before accessing mtrr msr
Hi Bin,
On 19 January 2015 at 22:01, Bin Meng bmeng...@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...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/3] x86: Test mtrr support flag before accessing mtrr msr
Hi Bin,
On 21 January 2015 at 09:19, Bin Meng bmeng...@gmail.com wrote:
Hi Simon,
On Thu, Jan 22, 2015 at 12:06 AM, Simon Glass s...@chromium.org wrote:
Hi Bin,
On 19 January 2015 at 22:01, Bin Meng bmeng...@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...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 3/3] x86: Test mtrr support flag before accessing mtrr msr
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...@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++];
--
1.8.2.1
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
