mtrr: allow uncached type for PCI prefetchable memory
diff mbox series

Message ID 20181116084550.17167-1-alex.hung@canonical.com
State Accepted
Headers show
Series
  • mtrr: allow uncached type for PCI prefetchable memory
Related show

Commit Message

Alex Hung Nov. 16, 2018, 8:45 a.m. UTC
Buglink: https://bugs.launchpad.net/bugs/1781920

Intel® 64 and IA-32 Architectures Software Developer's Manual seems to
suggest uncachable memory can be used for memory-mapped I/O.

Details can be found in Section 11.3.2 - Choosing a Memory Type

Signed-off-by: Alex Hung <alex.hung@canonical.com>
---
 src/bios/mtrr/mtrr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Colin King Nov. 16, 2018, 9:06 a.m. UTC | #1
On 16/11/2018 08:45, Alex Hung wrote:
> Buglink: https://bugs.launchpad.net/bugs/1781920
> 
> Intel® 64 and IA-32 Architectures Software Developer's Manual seems to
> suggest uncachable memory can be used for memory-mapped I/O.
> 
> Details can be found in Section 11.3.2 - Choosing a Memory Type
> 
> Signed-off-by: Alex Hung <alex.hung@canonical.com>
> ---
>  src/bios/mtrr/mtrr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c
> index 4948b55a..a614ffbb 100644
> --- a/src/bios/mtrr/mtrr.c
> +++ b/src/bios/mtrr/mtrr.c
> @@ -351,7 +351,7 @@ static int guess_cache_type(
>  
>  		if (pref) {
>  			*must = 0;
> -			*mustnot = WRITE_BACK | UNCACHED;
> +			*mustnot = WRITE_BACK;
>  		} else {
>  			*must = UNCACHED;
>  			*mustnot = (~UNCACHED) & (~DEFAULT);
> 

Lets see if there is any fall-out from this. I agree it seems a correct
choice with respect to the manual, so I think it should be OK.

Acked-by: Colin Ian King <colin.king@canonical.com>
ivanhu Nov. 19, 2018, 2:53 a.m. UTC | #2
Actually, this piece of code is carried from Intel's Linux-ready
Firmware Developer Kit. No one know where is the requirement comes from.

And I got the clarification from Intel Graphics team,

"We do not have such requirement to keep GMAR BAR as "Write Combined".
We only need GMADR to be WC for GOP to optimize BIOS logo displaying, it
is as design to switch GMARD back to UC for other memory type
utilization before transferring control to OS. BIOS need to set the
GMADR to WC during POST for improving GOP performance, and restore it
back to UC before boot to reduce the MTRR variable usage."


No reason for checking not setting the MTRR to UC.

I'll also add the clarification to the commit before push the commit.

Acked-by: Ivan Hu <ivan.hu@canonical.com>

On 11/16/18 4:45 PM, Alex Hung wrote:
> Buglink: https://bugs.launchpad.net/bugs/1781920
>
> Intel® 64 and IA-32 Architectures Software Developer's Manual seems to
> suggest uncachable memory can be used for memory-mapped I/O.
>
> Details can be found in Section 11.3.2 - Choosing a Memory Type
>
> Signed-off-by: Alex Hung <alex.hung@canonical.com>
> ---
>  src/bios/mtrr/mtrr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c
> index 4948b55a..a614ffbb 100644
> --- a/src/bios/mtrr/mtrr.c
> +++ b/src/bios/mtrr/mtrr.c
> @@ -351,7 +351,7 @@ static int guess_cache_type(
>  
>  		if (pref) {
>  			*must = 0;
> -			*mustnot = WRITE_BACK | UNCACHED;
> +			*mustnot = WRITE_BACK;
>  		} else {
>  			*must = UNCACHED;
>  			*mustnot = (~UNCACHED) & (~DEFAULT);

Patch
diff mbox series

diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c
index 4948b55a..a614ffbb 100644
--- a/src/bios/mtrr/mtrr.c
+++ b/src/bios/mtrr/mtrr.c
@@ -351,7 +351,7 @@  static int guess_cache_type(
 
 		if (pref) {
 			*must = 0;
-			*mustnot = WRITE_BACK | UNCACHED;
+			*mustnot = WRITE_BACK;
 		} else {
 			*must = UNCACHED;
 			*mustnot = (~UNCACHED) & (~DEFAULT);