Re: [RFC PATCH -tip] x86/pat: fix pat_x_mtrr_type to its essentials

[Suresh Siddha <suresh.b.siddha@intel.com>]

Xiaotian,

We look through the mtrr's only when the incoming request type is WB
(which is mostly the case for legacy /dev/mem mmap mapping). And only in
this scenario we use more conservative UC- instead of WC, if mtrr says
WC.

This will ensure the scenarios like ioremap() (which defaults to UC-) by
legacy drivers, before and after the user level mmap are consistent and
don't fail because of inconsistent mappings etc.

There is no reason why we need to use WC when especially the incoming
request was not strict enough and specified WB.

Also BTW,  your current effective return values mentioned in the below
table () is wrong.

i.e., for example, when we use UC- for PAT and mtrr has WC, effective
memory type used by cpu will be WC.

thanks,
suresh

On Thu, 2009-11-26 at 04:15 -0800, Xiaotian Feng wrote:
> Due to the IA-32 SDM vol 3a "Effective Memory Type" tables, the PAT
> and mtrr relations should be:
> 	|	UC	UC-	WC	WB	[PAT]
> --------+--------------------------------------
> UC	|	UC	UC(UC-)	WC	UC(UC-)
> WC	|	UC	WC(UC-)	WC	WC(UC-)
> WB	|	UC	UC(UC-)	WC	WB
> [MTRR]
> * In () is the current return value.
> 
> So, commit b6ff32, Fix pat_x_mtrr_type() to use UC_MINUS when the
> mtrr type return UC. But it also made mtrr type WC use UC_MINUS.
> This is not reasonable.
> 
> Signed-off-by: Xiaotian Feng <dfeng@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> ---
>  arch/x86/mm/pat.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
> index c37fd51..92e9627 100644
> --- a/arch/x86/mm/pat.c
> +++ b/arch/x86/mm/pat.c
> @@ -228,10 +228,11 @@ static unsigned long pat_x_mtrr_type(u64 start, u64 end, unsigned long req_type)
>  		u8 mtrr_type;
>  
>  		mtrr_type = mtrr_type_lookup(start, end);
> -		if (mtrr_type != MTRR_TYPE_WRBACK)
> -			return _PAGE_CACHE_UC_MINUS;
> -
> -		return _PAGE_CACHE_WB;
> +		if (mtrr_type == MTRR_TYPE_WRBACK)
> +			return _PAGE_CACHE_WB;
> +		else if (mtrr_type == MTRR_TYPE_WRCOMB)
> +			return _PAGE_CACHE_WC;
> +		return _PAGE_CACHE_UC_MINUS;
>  	}
>  
>  	return req_type;