[PATCH] Remove Barcelona Thermal Throttling

[PATCH] Remove Barcelona Thermal Throttling

[Russell Leidich]

[-- Attachment #1: Type: text/plain, Size: 1145 bytes --]

All,

You can imagine my dismay when I recently learned that, after all our
collective effort, hardware thermal throttling does not work reliably
on Barcelona, according to AMD.  Due to NDA restrictions, I am unable
to provide further details.

But the effort has not been a waste; whenever AMD manages to make a
reliable hardware thermal sensor, this code should "just work" along
with all the other northbridge initialization associated with the
device IDs in k8_northbridges[].

This patch Blacklists Barcelona as non-hardware-thermal-throttlable
(in addition to RevF, which was already blacklisted).  On the
recommendation of my colleagues, it also contains these politeness
enhancements: (1) warns the user if the CPU is _not_ blacklisted, but
the BIOS neglected to enable thermal throttling (as a safety measure)
and (2) uses the PCI_SLOT macro instead of the more obnoxious
shift-right-by-3 to compute PCI device numbers.

This patch assumes that it is being applied _after_ my previous patch
has already been integrated.  I mailed out that patch on 1/17/2008
with subject "Re: [PATCH] AMD Thermal Interrupt Support".
-- 
Russell Leidich

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 2369 bytes --]

Signed-off-by: Russell Leidich <rml@google.com>
--- linux-2.6.24-rc5.orig/arch/x86/kernel/cpu/mcheck/mce_amd_64	2008-02-03 21:17:58.186613000 -0800
+++ linux-2.6.24-rc5/arch/x86/kernel/cpu/mcheck/mce_amd_64.c	2008-02-03 21:51:38.810989000 -0800
@@ -742,13 +742,20 @@ static void smp_thermal_northbridge_init
 		 */
 		pci_read_config_dword(k8_northbridges[nb_num],
 					THERM_CTL_F3X64, &therm_ctl_f3x64);
-		therm_ctl_f3x64 |= PSL_APIC_HI_EN | HTC_EN;
+		therm_ctl_f3x64 |= PSL_APIC_HI_EN;
 		therm_ctl_f3x64 &= (~PSL_APIC_LO_EN);
 		pci_write_config_dword(k8_northbridges[nb_num],
 					THERM_CTL_F3X64, therm_ctl_f3x64);
-		printk(KERN_INFO "CPU thermal throttling interrupts enabled in "
-			"PCI bus 0x0, device 0x%x, function 0x3, register 0x64."
-			"\n", k8_northbridges[nb_num]->devfn >> 3);
+		if (therm_ctl_f3x64 & HTC_EN)
+			printk(KERN_INFO "CPU thermal throttling interrupts "
+				"enabled in PCI bus 0x0, device 0x%x, function "
+				"0x3, register 0x64.\n",
+				PCI_SLOT(k8_northbridges[nb_num]->devfn));
+		else
+			printk(KERN_WARNING "CPU thermal throttling disabled by"
+				" the BIOS in PCI bus 0x0, device 0x%x, "
+				"function 0x3, register 0x64, bit 0x0.\n",
+				PCI_SLOT(k8_northbridges[nb_num]->devfn));
 	}
 }
 
@@ -835,7 +842,7 @@ static void smp_thermal_early_throttle_c
  * corresponding APIC initialization on each core.
  *
  * Due to an erratum involving the thermal sensor, this code does not work on
- * any CPU prior to Family 0x10.
+ * CPUs with certain northbridge device IDs, as coded below.
  */
 static int smp_thermal_interrupt_init(void)
 {
@@ -844,13 +851,15 @@ static int smp_thermal_interrupt_init(vo
 	if (num_k8_northbridges == 0)
 		goto out;
 	/*
-	 * If any of the northbridges has PCI ID 0x1103, then its thermal
-	 * hardware suffers from an erratum which prevents this code from
-	 * working, so abort.  (This implies that it only works on Family
-	 * 0x10.)
+	 * If any of the northbridges has any of the device IDs in the following
+	 * loop, then the hardware suffers from an erratum which prevents
+	 * this code from working, so abort.
 	 */
 	for (nb_num = 0; nb_num < num_k8_northbridges; nb_num++) {
-		if ((k8_northbridges[nb_num]->device) == 0x1103)
+		__u32 device_id;
+
+		device_id = k8_northbridges[nb_num]->device;
+		if ((device_id == 0x1103) || (device_id == 0x1203))
 			goto out;
 	}
 	/*

Re: [PATCH] Remove Barcelona Thermal Throttling

[Russell Leidich]

P.S. This patch also moves the responsibility for enabling hardware
thermal control from the OS to the BIOS.

On Feb 3, 2008 10:13 PM, Russell Leidich <rml@google.com> wrote:
> All,
>
> You can imagine my dismay when I recently learned that, after all our
> collective effort, hardware thermal throttling does not work reliably
> on Barcelona, according to AMD.  Due to NDA restrictions, I am unable
> to provide further details.
>
> But the effort has not been a waste; whenever AMD manages to make a
> reliable hardware thermal sensor, this code should "just work" along
> with all the other northbridge initialization associated with the
> device IDs in k8_northbridges[].
>
> This patch Blacklists Barcelona as non-hardware-thermal-throttlable
> (in addition to RevF, which was already blacklisted).  On the
> recommendation of my colleagues, it also contains these politeness
> enhancements: (1) warns the user if the CPU is _not_ blacklisted, but
> the BIOS neglected to enable thermal throttling (as a safety measure)
> and (2) uses the PCI_SLOT macro instead of the more obnoxious
> shift-right-by-3 to compute PCI device numbers.
>
> This patch assumes that it is being applied _after_ my previous patch
> has already been integrated.  I mailed out that patch on 1/17/2008
> with subject "Re: [PATCH] AMD Thermal Interrupt Support".
> --
> Russell Leidich
>



-- 
Russell Leidich

Re: [PATCH] Remove Barcelona Thermal Throttling

[Andi Kleen]

On Sun, Feb 03, 2008 at 10:13:53PM -0800, Russell Leidich wrote:
> All,
> 
> You can imagine my dismay when I recently learned that, after all our
> collective effort, hardware thermal throttling does not work reliably
> on Barcelona, according to AMD.  Due to NDA restrictions, I am unable
> to provide further details.

Too bad. Hopefully that code will work then on the hypothetic
future parts with fixed throttling. Perhaps it would be better
to just remove it now and readd later.

All this reminds me you need to adjust the Kconfig dependencies
for K8_NB too to be always enabled with MCE_AMD.

Ideally on top of this patch with a || MCE_AMD

-Andi

---

Fix and simplify k8.c Kconfig dependencies

- Check for K8_NUMA instead of NUMA && PCI
- No need to check for x86_64 explicitely

Signed-off-by: Andi Kleen <ak@suse.de>

Index: linux/arch/x86/Kconfig
===================================================================
--- linux.orig/arch/x86/Kconfig
+++ linux/arch/x86/Kconfig
@@ -1540,7 +1540,7 @@ endif # X86_32
 
 config K8_NB
 	def_bool y
-	depends on AGP_AMD64 || (X86_64 && (GART_IOMMU || (PCI && NUMA)))
+	depends on AGP_AMD64 || GART_IOMMU || K8_NUMA
 
 source "drivers/pcmcia/Kconfig"
 

Re: [PATCH] Remove Barcelona Thermal Throttling

[Russell Leidich]

On Feb 3, 2008 11:20 PM, Andi Kleen <andi@firstfloor.org> wrote:
> On Sun, Feb 03, 2008 at 10:13:53PM -0800, Russell Leidich wrote:
> > All,
> >
> > You can imagine my dismay when I recently learned that, after all our
> > collective effort, hardware thermal throttling does not work reliably
> > on Barcelona, according to AMD.  Due to NDA restrictions, I am unable
> > to provide further details.
>
> Too bad. Hopefully that code will work then on the hypothetic
> future parts with fixed throttling. Perhaps it would be better
> to just remove it now and readd later.

I really think it's best to leave it in.  It's harmless at the moment,
and readding it later will just create a massive amount of duplicate
work.    Sooner or later, I'm have little doubt that AMD can make this
feature work.

>
> All this reminds me you need to adjust the Kconfig dependencies
> for K8_NB too to be always enabled with MCE_AMD.
>
> Ideally on top of this patch with a || MCE_AMD

OK, if you'll take the patch, I'd be happy to do this :).  I'm
assuming that you mean I should force K8_NB = "yes" if MCE_AMD = "yes"
in .config.  Is that what you're asking for?

>
> -Andi
>
> ---
>
> Fix and simplify k8.c Kconfig dependencies
>
> - Check for K8_NUMA instead of NUMA && PCI
> - No need to check for x86_64 explicitely
>
> Signed-off-by: Andi Kleen <ak@suse.de>
>
> Index: linux/arch/x86/Kconfig
> ===================================================================
> --- linux.orig/arch/x86/Kconfig
> +++ linux/arch/x86/Kconfig
> @@ -1540,7 +1540,7 @@ endif # X86_32
>
>  config K8_NB
>         def_bool y
> -       depends on AGP_AMD64 || (X86_64 && (GART_IOMMU || (PCI && NUMA)))
> +       depends on AGP_AMD64 || GART_IOMMU || K8_NUMA
>
>  source "drivers/pcmcia/Kconfig"
>
>



-- 
Russell Leidich

Re: [PATCH] Remove Barcelona Thermal Throttling

[Russell Leidich]

[-- Attachment #1: Type: text/plain, Size: 1421 bytes --]

All,

Sorry this took so long.  I have attached the Kconfig changes
requested by Andi, appended to the original patch.

On Feb 4, 2008 11:22 AM, Russell Leidich <rml@google.com> wrote:
> On Feb 3, 2008 11:20 PM, Andi Kleen <andi@firstfloor.org> wrote:
> > On Sun, Feb 03, 2008 at 10:13:53PM -0800, Russell Leidich wrote:
> > > All,
> > >
> > > You can imagine my dismay when I recently learned that, after all our
> > > collective effort, hardware thermal throttling does not work reliably
> > > on Barcelona, according to AMD.  Due to NDA restrictions, I am unable
> > > to provide further details.
> >
> > Too bad. Hopefully that code will work then on the hypothetic
> > future parts with fixed throttling. Perhaps it would be better
> > to just remove it now and readd later.
>
> I really think it's best to leave it in.  It's harmless at the moment,
> and readding it later will just create a massive amount of duplicate
> work.    Sooner or later, I'm have little doubt that AMD can make this
> feature work.
>
> >
> > All this reminds me you need to adjust the Kconfig dependencies
> > for K8_NB too to be always enabled with MCE_AMD.
> >
> > Ideally on top of this patch with a || MCE_AMD
>
> OK, if you'll take the patch, I'd be happy to do this :).  I'm
> assuming that you mean I should force K8_NB = "yes" if MCE_AMD = "yes"
> in .config.  Is that what you're asking for?
>
> >
> > -Andi

-- 
Russell Leidich

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3048 bytes --]

Signed-off-by: Russell Leidich <rml@google.com>
--- linux-2.6.24-rc5.orig/arch/x86/kernel/cpu/mcheck/mce_amd_64.c	2008-02-03 21:17:58.186613000 -0800
+++ linux-2.6.24-rc5/arch/x86/kernel/cpu/mcheck/mce_amd_64.c	2008-02-03 21:51:38.810989000 -0800
@@ -742,13 +742,20 @@ static void smp_thermal_northbridge_init
 		 */
 		pci_read_config_dword(k8_northbridges[nb_num],
 					THERM_CTL_F3X64, &therm_ctl_f3x64);
-		therm_ctl_f3x64 |= PSL_APIC_HI_EN | HTC_EN;
+		therm_ctl_f3x64 |= PSL_APIC_HI_EN;
 		therm_ctl_f3x64 &= (~PSL_APIC_LO_EN);
 		pci_write_config_dword(k8_northbridges[nb_num],
 					THERM_CTL_F3X64, therm_ctl_f3x64);
-		printk(KERN_INFO "CPU thermal throttling interrupts enabled in "
-			"PCI bus 0x0, device 0x%x, function 0x3, register 0x64."
-			"\n", k8_northbridges[nb_num]->devfn >> 3);
+		if (therm_ctl_f3x64 & HTC_EN)
+			printk(KERN_INFO "CPU thermal throttling interrupts "
+				"enabled in PCI bus 0x0, device 0x%x, function "
+				"0x3, register 0x64.\n",
+				PCI_SLOT(k8_northbridges[nb_num]->devfn));
+		else
+			printk(KERN_WARNING "CPU thermal throttling disabled by"
+				" the BIOS in PCI bus 0x0, device 0x%x, "
+				"function 0x3, register 0x64, bit 0x0.\n",
+				PCI_SLOT(k8_northbridges[nb_num]->devfn));
 	}
 }
 
@@ -835,7 +842,7 @@ static void smp_thermal_early_throttle_c
  * corresponding APIC initialization on each core.
  *
  * Due to an erratum involving the thermal sensor, this code does not work on
- * any CPU prior to Family 0x10.
+ * CPUs with certain northbridge device IDs, as coded below.
  */
 static int smp_thermal_interrupt_init(void)
 {
@@ -844,13 +851,15 @@ static int smp_thermal_interrupt_init(vo
 	if (num_k8_northbridges == 0)
 		goto out;
 	/*
-	 * If any of the northbridges has PCI ID 0x1103, then its thermal
-	 * hardware suffers from an erratum which prevents this code from
-	 * working, so abort.  (This implies that it only works on Family
-	 * 0x10.)
+	 * If any of the northbridges has any of the device IDs in the following
+	 * loop, then the hardware suffers from an erratum which prevents
+	 * this code from working, so abort.
 	 */
 	for (nb_num = 0; nb_num < num_k8_northbridges; nb_num++) {
-		if ((k8_northbridges[nb_num]->device) == 0x1103)
+		__u32 device_id;
+
+		device_id = k8_northbridges[nb_num]->device;
+		if ((device_id == 0x1103) || (device_id == 0x1203))
 			goto out;
 	}
 	/*
--- linux-2.6.24-rc5.orig/arch/x86/Kconfig	2007-12-11 14:30:52.708983000 -0800
+++ linux-2.6.24-rc5/arch/x86/Kconfig	2008-02-12 11:01:24.860123000 -0800
@@ -564,6 +564,7 @@ config X86_MCE_INTEL
 config X86_MCE_AMD
 	bool "AMD MCE features"
 	depends on X86_64 && X86_MCE && X86_LOCAL_APIC
+	select K8_NB
 	default y
 	help
 	   Additional support for AMD specific MCE features such as
@@ -1559,8 +1560,7 @@ endif # X86_32
 
 config K8_NB
 	def_bool y
-	depends on AGP_AMD64 || (X86_64 && (GART_IOMMU || (PCI && NUMA)))
-
+	depends on AGP_AMD64 || (X86_64 && (GART_IOMMU || (PCI && NUMA))) || X86_MCE_AMD
 source "drivers/pcmcia/Kconfig"
 
 source "drivers/pci/hotplug/Kconfig"