[patch 0/4] PAT Misc updates

[patch 0/4] PAT Misc updates

[venkatesh.pallipadi]

-- 

[patch 1/4] x86: PAT proper tracking of set_memory_uc and friends

[venkatesh.pallipadi]

[-- Attachment #1: set_memory_pa.patch --]
[-- Type: text/plain, Size: 1476 bytes --]

Big thinko in pat memtype tracking code. reserve_memtype should be called
with physical address and not virtual address. 

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>

---
 arch/x86/mm/pageattr.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: tip/arch/x86/mm/pageattr.c
===================================================================
--- tip.orig/arch/x86/mm/pageattr.c	2008-08-20 14:25:18.000000000 -0700
+++ tip/arch/x86/mm/pageattr.c	2008-08-20 14:26:15.000000000 -0700
@@ -862,7 +862,7 @@ int set_memory_uc(unsigned long addr, in
 	/*
 	 * for now UC MINUS. see comments in ioremap_nocache()
 	 */
-	if (reserve_memtype(addr, addr + numpages * PAGE_SIZE,
+	if (reserve_memtype(__pa(addr), __pa(addr) + numpages * PAGE_SIZE,
 			    _PAGE_CACHE_UC_MINUS, NULL))
 		return -EINVAL;
 
@@ -905,7 +905,7 @@ int set_memory_wc(unsigned long addr, in
 	if (!pat_enabled)
 		return set_memory_uc(addr, numpages);
 
-	if (reserve_memtype(addr, addr + numpages * PAGE_SIZE,
+	if (reserve_memtype(__pa(addr), __pa(addr) + numpages * PAGE_SIZE,
 		_PAGE_CACHE_WC, NULL))
 		return -EINVAL;
 
@@ -921,7 +921,7 @@ int _set_memory_wb(unsigned long addr, i
 
 int set_memory_wb(unsigned long addr, int numpages)
 {
-	free_memtype(addr, addr + numpages * PAGE_SIZE);
+	free_memtype(__pa(addr), __pa(addr) + numpages * PAGE_SIZE);
 
 	return _set_memory_wb(addr, numpages);
 }

-- 

[patch 2/4] x86: PAT Change /dev/mem mmap with O_SYNC to use UC_MINUS

[venkatesh.pallipadi]

[-- Attachment #1: osync_as_ucminus.patch --]
[-- Type: text/plain, Size: 2332 bytes --]

All kernel mappings like ioremap(), etc uses UC_MINUS as the type. /dev/mem
mappings with /dev/mem being opened with O_SYNC however was using UC,
resulting in a conflict with /dev/mem mmap failing. This seems to be
affecting some apps (one being flashrom) which are using O_SYNC and which were
working before.

Switch /dev/mem with O_SYNC also to UC_MINUS.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>

---
 arch/x86/mm/pat.c |   17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

Index: tip/arch/x86/mm/pat.c
===================================================================
--- tip.orig/arch/x86/mm/pat.c	2008-08-20 14:25:18.000000000 -0700
+++ tip/arch/x86/mm/pat.c	2008-08-20 14:26:35.000000000 -0700
@@ -390,14 +390,6 @@ int free_memtype(u64 start, u64 end)
 }
 
 
-/*
- * /dev/mem mmap interface. The memtype used for mapping varies:
- * - Use UC for mappings with O_SYNC flag
- * - Without O_SYNC flag, if there is any conflict in reserve_memtype,
- *   inherit the memtype from existing mapping.
- * - Else use UC_MINUS memtype (for backward compatibility with existing
- *   X drivers.
- */
 pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
 				unsigned long size, pgprot_t vma_prot)
 {
@@ -435,14 +427,14 @@ int phys_mem_access_prot_allowed(struct 
 				unsigned long size, pgprot_t *vma_prot)
 {
 	u64 offset = ((u64) pfn) << PAGE_SHIFT;
-	unsigned long flags = _PAGE_CACHE_UC_MINUS;
+	unsigned long flags = -1;
 	int retval;
 
 	if (!range_is_allowed(pfn, size))
 		return 0;
 
 	if (file->f_flags & O_SYNC) {
-		flags = _PAGE_CACHE_UC;
+		flags = _PAGE_CACHE_UC_MINUS;
 	}
 
 #ifdef CONFIG_X86_32
@@ -465,13 +457,14 @@ int phys_mem_access_prot_allowed(struct 
 #endif
 
 	/*
-	 * With O_SYNC, we can only take UC mapping. Fail if we cannot.
+	 * With O_SYNC, we can only take UC_MINUS mapping. Fail if we cannot.
+	 *
 	 * Without O_SYNC, we want to get
 	 * - WB for WB-able memory and no other conflicting mappings
 	 * - UC_MINUS for non-WB-able memory with no other conflicting mappings
 	 * - Inherit from confliting mappings otherwise
 	 */
-	if (flags != _PAGE_CACHE_UC_MINUS) {
+	if (flags != -1) {
 		retval = reserve_memtype(offset, offset + size, flags, NULL);
 	} else {
 		retval = reserve_memtype(offset, offset + size, -1, &flags);

-- 

[patch 3/4] x86: PAT Update validate_pat_support for intel CPUs

[venkatesh.pallipadi]

[-- Attachment #1: intel_pat_whitelist.patch --]
[-- Type: text/plain, Size: 1550 bytes --]

Pentium III and Core Solo/Duo CPUs have an erratum
" Page with PAT set to WC while associated MTRR is UC may consolidate to UC "
which can result in WC setting in PAT to be ineffective. We will disable
PAT on such CPUs, so that we can continue to use MTRR WC setting.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>

---
 arch/x86/kernel/cpu/addon_cpuid_features.c |   17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Index: tip/arch/x86/kernel/cpu/addon_cpuid_features.c
===================================================================
--- tip.orig/arch/x86/kernel/cpu/addon_cpuid_features.c	2008-08-20 14:25:18.000000000 -0700
+++ tip/arch/x86/kernel/cpu/addon_cpuid_features.c	2008-08-20 14:26:39.000000000 -0700
@@ -56,9 +56,22 @@ void __cpuinit validate_pat_support(stru
 
 	switch (c->x86_vendor) {
 	case X86_VENDOR_INTEL:
-		if (c->x86 == 0xF || (c->x86 == 6 && c->x86_model >= 15))
+		/*
+		 * There is a known erratum on Pentium III and Core Solo
+		 * and Core Duo CPUs.
+		 * " Page with PAT set to WC while associated MTRR is UC
+		 *   may consolidate to UC "
+		 * Because of this erratum, it is better to stick with
+		 * setting WC in MTRR rather than using PAT on these CPUs.
+		 *
+		 * Enable PAT WC only on P4, Core 2 or later CPUs.
+		 */
+		if (c->x86 > 0x6 || (c->x86 == 6 && c->x86_model >= 15))
 			return;
-		break;
+
+		pat_disable("PAT WC disabled due to known CPU erratum.");
+		return;
+
 	case X86_VENDOR_AMD:
 	case X86_VENDOR_CENTAUR:
 	case X86_VENDOR_TRANSMETA:

-- 

[patch 4/4] x86: PAT documentation updates with debug info

[venkatesh.pallipadi]

[-- Attachment #1: pat_doc_updates.patch --]
[-- Type: text/plain, Size: 6058 bytes --]

Documentation update for PAT. Reflect the latest API details.
Also, adds details about ways to get more info in order to debug PAT.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>

---
 Documentation/x86/pat.txt |   54 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 45 insertions(+), 9 deletions(-)

Index: tip/Documentation/x86/pat.txt
===================================================================
--- tip.orig/Documentation/x86/pat.txt	2008-08-20 14:25:16.000000000 -0700
+++ tip/Documentation/x86/pat.txt	2008-08-20 14:26:43.000000000 -0700
@@ -14,6 +14,10 @@ PAT allows for different types of memory
 ones that will be supported at this time are Write-back, Uncached,
 Write-combined and Uncached Minus.
 
+
+PAT APIs
+--------
+
 There are many different APIs in the kernel that allows setting of memory
 attributes at the page level. In order to avoid aliasing, these interfaces
 should be used thoughtfully. Below is a table of interfaces available,
@@ -26,38 +30,38 @@ address range to avoid any aliasing.
 API                    |    RAM   |  ACPI,...  |  Reserved/Holes  |
 -----------------------|----------|------------|------------------|
                        |          |            |                  |
-ioremap                |    --    |    UC      |       UC         |
+ioremap                |    --    |    UC-     |       UC-        |
                        |          |            |                  |
 ioremap_cache          |    --    |    WB      |       WB         |
                        |          |            |                  |
-ioremap_nocache        |    --    |    UC      |       UC         |
+ioremap_nocache        |    --    |    UC-     |       UC-        |
                        |          |            |                  |
 ioremap_wc             |    --    |    --      |       WC         |
                        |          |            |                  |
-set_memory_uc          |    UC    |    --      |       --         |
+set_memory_uc          |    UC-   |    --      |       --         |
  set_memory_wb         |          |            |                  |
                        |          |            |                  |
 set_memory_wc          |    WC    |    --      |       --         |
  set_memory_wb         |          |            |                  |
                        |          |            |                  |
-pci sysfs resource     |    --    |    --      |       UC         |
+pci sysfs resource     |    --    |    --      |       UC-        |
                        |          |            |                  |
 pci sysfs resource_wc  |    --    |    --      |       WC         |
  is IORESOURCE_PREFETCH|          |            |                  |
                        |          |            |                  |
-pci proc               |    --    |    --      |       UC         |
+pci proc               |    --    |    --      |       UC-        |
  !PCIIOC_WRITE_COMBINE |          |            |                  |
                        |          |            |                  |
 pci proc               |    --    |    --      |       WC         |
  PCIIOC_WRITE_COMBINE  |          |            |                  |
                        |          |            |                  |
-/dev/mem               |    --    |    UC      |       UC         |
+/dev/mem               |    --    |  WB/WC/UC- |    WB/WC/UC-     |
  read-write            |          |            |                  |
                        |          |            |                  |
-/dev/mem               |    --    |    UC      |       UC         |
+/dev/mem               |    --    |    UC-     |       UC-        |
  mmap SYNC flag        |          |            |                  |
                        |          |            |                  |
-/dev/mem               |    --    |  WB/WC/UC  |    WB/WC/UC      |
+/dev/mem               |    --    |  WB/WC/UC- |    WB/WC/UC-     |
  mmap !SYNC flag       |          |(from exist-|  (from exist-    |
  and                   |          |  ing alias)|    ing alias)    |
  any alias to this area|          |            |                  |
@@ -68,7 +72,7 @@ pci proc               |    --    |    -
  and                   |          |            |                  |
  MTRR says WB          |          |            |                  |
                        |          |            |                  |
-/dev/mem               |    --    |    --      |    UC_MINUS      |
+/dev/mem               |    --    |    --      |       UC-        |
  mmap !SYNC flag       |          |            |                  |
  no alias to this area |          |            |                  |
  and                   |          |            |                  |
@@ -98,3 +102,35 @@ types.
 
 Drivers should use set_memory_[uc|wc] to set access type for RAM ranges.
 
+
+PAT debugging
+-------------
+
+With CONFIG_DEBUG_FS enabled, PAT memtype list can be examined by
+
+# mount -t debugfs debugfs /sys/kernel/debug
+# cat /sys/kernel/debug/x86/pat_memtype_list
+PAT memtype list:
+uncached-minus @ 0x7fadf000-0x7fae0000
+uncached-minus @ 0x7fb19000-0x7fb1a000
+uncached-minus @ 0x7fb1a000-0x7fb1b000
+uncached-minus @ 0x7fb1b000-0x7fb1c000
+uncached-minus @ 0x7fb1c000-0x7fb1d000
+uncached-minus @ 0x7fb1d000-0x7fb1e000
+uncached-minus @ 0x7fb1e000-0x7fb25000
+uncached-minus @ 0x7fb25000-0x7fb26000
+uncached-minus @ 0x7fb26000-0x7fb27000
+uncached-minus @ 0x7fb27000-0x7fb28000
+uncached-minus @ 0x7fb28000-0x7fb2e000
+uncached-minus @ 0x7fb2e000-0x7fb2f000
+uncached-minus @ 0x7fb2f000-0x7fb30000
+uncached-minus @ 0x7fb31000-0x7fb32000
+uncached-minus @ 0x80000000-0x90000000
+
+This list shows physical address ranges and various PAT settings used to
+access those physical address ranges.
+
+Another, more verbose way of getting PAT related debug messages is with
+"debugpat" boot parameter. With this parameter, various debug messages are
+printed to dmesg log.
+

-- 

Re: [patch 0/4] PAT Misc updates

[Ingo Molnar]

applied the first 3 patches to tip/x86/urgent and the last patch to 
tip/x86/doc. Thanks Venki,

	Ingo

Re: [patch 3/4] x86: PAT Update validate_pat_support for intel CPUs

[Dave Airlie]

On Thu, Aug 21, 2008 at 9:45 AM,  <venkatesh.pallipadi@intel.com> wrote:
> Pentium III and Core Solo/Duo CPUs have an erratum
> " Page with PAT set to WC while associated MTRR is UC may consolidate to UC "
> which can result in WC setting in PAT to be ineffective. We will disable
> PAT on such CPUs, so that we can continue to use MTRR WC setting.
>

IMHO this seems like a bit hammer with which to squash this nut.

I really need PAT support for upcoming GPU stuff, esp where I have
pages of RAM allocated into a
GART and I want write-combined access to them. These pages will
physically me under a write-back MTRR, with a WC PAT entry
on the PTE mappings.

Can we not just be smarter and fix this when we know we have a UC MTRR
and a WC PAT mapping inside it?

Dave.

> Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
>
> ---
>  arch/x86/kernel/cpu/addon_cpuid_features.c |   17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> Index: tip/arch/x86/kernel/cpu/addon_cpuid_features.c
> ===================================================================
> --- tip.orig/arch/x86/kernel/cpu/addon_cpuid_features.c 2008-08-20 14:25:18.000000000 -0700
> +++ tip/arch/x86/kernel/cpu/addon_cpuid_features.c      2008-08-20 14:26:39.000000000 -0700
> @@ -56,9 +56,22 @@ void __cpuinit validate_pat_support(stru
>
>        switch (c->x86_vendor) {
>        case X86_VENDOR_INTEL:
> -               if (c->x86 == 0xF || (c->x86 == 6 && c->x86_model >= 15))
> +               /*
> +                * There is a known erratum on Pentium III and Core Solo
> +                * and Core Duo CPUs.
> +                * " Page with PAT set to WC while associated MTRR is UC
> +                *   may consolidate to UC "
> +                * Because of this erratum, it is better to stick with
> +                * setting WC in MTRR rather than using PAT on these CPUs.
> +                *
> +                * Enable PAT WC only on P4, Core 2 or later CPUs.
> +                */
> +               if (c->x86 > 0x6 || (c->x86 == 6 && c->x86_model >= 15))
>                        return;
> -               break;
> +
> +               pat_disable("PAT WC disabled due to known CPU erratum.");
> +               return;
> +
>        case X86_VENDOR_AMD:
>        case X86_VENDOR_CENTAUR:
>        case X86_VENDOR_TRANSMETA:
>
> --
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

RE: [patch 3/4] x86: PAT Update validate_pat_support for intel CPUs

[Pallipadi, Venkatesh]

The problem here is both with MTRR entry of UC and even with
default MTRR being UC.

Default MTRR being UC for all reserved regions and drivers
wanting WC is very common situation and performance will be
affected when that ends up being UC access.
With pat disabled, drivers can set MTRR with WC in this case
and get the expected performance.

Yes. We can still allow WC mappings only for the RAM
regions and disable PAT WC for all the reserved regions. I will
take a look at that option and see how cleanly we can
enable pat only for set_memory_wc and disable PAT for
ioremap_wc and pci regions.

Thanks,
Venki



>-----Original Message-----
>From: Dave Airlie [mailto:airlied@gmail.com]
>Sent: Tuesday, August 26, 2008 12:06 AM
>To: Pallipadi, Venkatesh
>Cc: mingo@elte.hu; tglx@linutronix.de; hpa@zytor.com;
>linux-kernel@vger.kernel.org; Siddha, Suresh B
>Subject: Re: [patch 3/4] x86: PAT Update validate_pat_support
>for intel CPUs
>
>On Thu, Aug 21, 2008 at 9:45 AM,
><venkatesh.pallipadi@intel.com> wrote:
>> Pentium III and Core Solo/Duo CPUs have an erratum
>> " Page with PAT set to WC while associated MTRR is UC may
>consolidate to UC "
>> which can result in WC setting in PAT to be ineffective. We
>will disable
>> PAT on such CPUs, so that we can continue to use MTRR WC setting.
>>
>
>IMHO this seems like a bit hammer with which to squash this nut.
>
>I really need PAT support for upcoming GPU stuff, esp where I have
>pages of RAM allocated into a
>GART and I want write-combined access to them. These pages will
>physically me under a write-back MTRR, with a WC PAT entry
>on the PTE mappings.
>
>Can we not just be smarter and fix this when we know we have a UC MTRR
>and a WC PAT mapping inside it?
>
>Dave.
>
>> Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
>>
>> ---
>>  arch/x86/kernel/cpu/addon_cpuid_features.c |   17 +++++++++++++++--
>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> Index: tip/arch/x86/kernel/cpu/addon_cpuid_features.c
>> ===================================================================
>> --- tip.orig/arch/x86/kernel/cpu/addon_cpuid_features.c
>2008-08-20 14:25:18.000000000 -0700
>> +++ tip/arch/x86/kernel/cpu/addon_cpuid_features.c
>2008-08-20 14:26:39.000000000 -0700
>> @@ -56,9 +56,22 @@ void __cpuinit validate_pat_support(stru
>>
>>        switch (c->x86_vendor) {
>>        case X86_VENDOR_INTEL:
>> -               if (c->x86 == 0xF || (c->x86 == 6 &&
>c->x86_model >= 15))
>> +               /*
>> +                * There is a known erratum on Pentium III
>and Core Solo
>> +                * and Core Duo CPUs.
>> +                * " Page with PAT set to WC while
>associated MTRR is UC
>> +                *   may consolidate to UC "
>> +                * Because of this erratum, it is better to
>stick with
>> +                * setting WC in MTRR rather than using PAT
>on these CPUs.
>> +                *
>> +                * Enable PAT WC only on P4, Core 2 or later CPUs.
>> +                */
>> +               if (c->x86 > 0x6 || (c->x86 == 6 &&
>c->x86_model >= 15))
>>                        return;
>> -               break;
>> +
>> +               pat_disable("PAT WC disabled due to known
>CPU erratum.");
>> +               return;
>> +
>>        case X86_VENDOR_AMD:
>>        case X86_VENDOR_CENTAUR:
>>        case X86_VENDOR_TRANSMETA:
>>
>> --
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>>
>

Re: [patch 3/4] x86: PAT Update validate_pat_support for intel CPUs

[Brice Goglin]

Pallipadi, Venkatesh wrote:
> Default MTRR being UC for all reserved regions and drivers
> wanting WC is very common situation and performance will be
> affected when that ends up being UC access.
> With pat disabled, drivers can set MTRR with WC in this case
> and get the expected performance.
>   

Are you saying that drivers are supposed to check if PAT is disabled
before deciding whether they need to setup a MTRR WC? If so, how to do
we check that? Unless we get a way to know whether ioremap_wc() returned
WC or UC, I will always keep the MTRR WC in myri10ge.

Brice

RE: [patch 3/4] x86: PAT Update validate_pat_support for intel CPUs

[Pallipadi, Venkatesh]

>-----Original Message-----
>From: Brice Goglin [mailto:brice@myri.com]
>Sent: Tuesday, August 26, 2008 10:30 PM
>To: Pallipadi, Venkatesh
>Cc: Dave Airlie; mingo@elte.hu; tglx@linutronix.de;
>hpa@zytor.com; linux-kernel@vger.kernel.org; Siddha, Suresh B
>Subject: Re: [patch 3/4] x86: PAT Update validate_pat_support
>for intel CPUs
>
>Pallipadi, Venkatesh wrote:
>> Default MTRR being UC for all reserved regions and drivers
>> wanting WC is very common situation and performance will be
>> affected when that ends up being UC access.
>> With pat disabled, drivers can set MTRR with WC in this case
>> and get the expected performance.
>>
>
>Are you saying that drivers are supposed to check if PAT is disabled
>before deciding whether they need to setup a MTRR WC? If so, how to do
>we check that? Unless we get a way to know whether
>ioremap_wc() returned
>WC or UC, I will always keep the MTRR WC in myri10ge.
>

Agreed. There is no way for driver to cleanly know whether it got
WC mapping or not. I guess it is possibly simpler for kernel to
ignore MTRR writes when WC is working, rather than each driver
checking the return value. But, that will come later,
once we have PAT code stable.

Thanks,
Venki