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

[Xiaotian Feng <dfeng@redhat.com>]

On 12/01/2009 07:21 AM, Suresh Siddha wrote:
> 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 for the detailed explanation, I'm clear now, thank you very much.
>
> 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;
>
>