1. Patchew
  2. Xen
  3. View series

[PATCH] x86/APIC: Remove esr_disable

Andrew Cooper posted 1 patch 2 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230829154621.3565971-1-andrew.cooper3@citrix.com
xen/arch/x86/apic.c                           | 50 ++++++-------------
.../x86/include/asm/mach-generic/mach_apic.h  |  3 --
2 files changed, 16 insertions(+), 37 deletions(-)
[PATCH] x86/APIC: Remove esr_disable
Posted by Andrew Cooper 2 years, 5 months ago 
It is unconditionally 0 in Xen, and was deleted in Linux somewhere between 2.5
and 2.6.

Remove it in Xen too.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>

I got bored waiting for `git log` to tell where it was disabled in Linux...
---
 xen/arch/x86/apic.c                           | 50 ++++++-------------
 .../x86/include/asm/mach-generic/mach_apic.h  |  3 --
 2 files changed, 16 insertions(+), 37 deletions(-)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 41879230ec90..5c6935ba42db 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -496,14 +496,6 @@ void setup_local_APIC(bool bsp)
     unsigned long oldvalue, value, maxlvt;
     int i, j;
 
-    /* Pound the ESR really hard over the head with a big hammer - mbligh */
-    if (esr_disable) {
-        apic_write(APIC_ESR, 0);
-        apic_write(APIC_ESR, 0);
-        apic_write(APIC_ESR, 0);
-        apic_write(APIC_ESR, 0);
-    }
-
     BUILD_BUG_ON((SPURIOUS_APIC_VECTOR & 0x0f) != 0x0f);
 
     /*
@@ -628,33 +620,23 @@ void setup_local_APIC(bool bsp)
         value = APIC_DM_NMI | APIC_LVT_MASKED;
     apic_write(APIC_LVT1, value);
 
-    if (!esr_disable) {
-        maxlvt = get_maxlvt();
-        if (maxlvt > 3)     /* Due to the Pentium erratum 3AP. */
-            apic_write(APIC_ESR, 0);
-        oldvalue = apic_read(APIC_ESR);
+    maxlvt = get_maxlvt();
+    if (maxlvt > 3)     /* Due to the Pentium erratum 3AP. */
+        apic_write(APIC_ESR, 0);
+    oldvalue = apic_read(APIC_ESR);
 
-        value = ERROR_APIC_VECTOR;      // enables sending errors
-        apic_write(APIC_LVTERR, value);
-        /*
-         * spec says clear errors after enabling vector.
-         */
-        if (maxlvt > 3)
-            apic_write(APIC_ESR, 0);
-        value = apic_read(APIC_ESR);
-        if (value != oldvalue)
-            apic_printk(APIC_VERBOSE, "ESR value before enabling "
-                        "vector: %#lx  after: %#lx\n",
-                        oldvalue, value);
-    } else {
-        /*
-         * Something untraceble is creating bad interrupts on
-         * secondary quads ... for the moment, just leave the
-         * ESR disabled - we can't do anything useful with the
-         * errors anyway - mbligh
-         */
-        printk("Leaving ESR disabled.\n");
-    }
+    value = ERROR_APIC_VECTOR;      // enables sending errors
+    apic_write(APIC_LVTERR, value);
+    /*
+     * spec says clear errors after enabling vector.
+     */
+    if (maxlvt > 3)
+        apic_write(APIC_ESR, 0);
+    value = apic_read(APIC_ESR);
+    if (value != oldvalue)
+        apic_printk(APIC_VERBOSE,
+                    "ESR value before enabling vector: %#lx  after: %#lx\n",
+                    oldvalue, value);
 
     if (nmi_watchdog == NMI_LOCAL_APIC && !bsp)
         setup_apic_nmi_watchdog();
diff --git a/xen/arch/x86/include/asm/mach-generic/mach_apic.h b/xen/arch/x86/include/asm/mach-generic/mach_apic.h
index b6f6361c6046..cf8b31b6e09e 100644
--- a/xen/arch/x86/include/asm/mach-generic/mach_apic.h
+++ b/xen/arch/x86/include/asm/mach-generic/mach_apic.h
@@ -6,9 +6,6 @@
 #include <asm/genapic.h>
 #include <asm/smp.h>
 
-/* ESR was originally disabled in Linux for NUMA-Q. Do we really need to? */
-#define esr_disable (0)
-
 /* The following are dependent on APIC delivery mode (logical vs. physical). */
 #define INT_DELIVERY_MODE (genapic.int_delivery_mode)
 #define INT_DEST_MODE (genapic.int_dest_mode)

base-commit: 067f18c3a72d8f0acccab831083b8518f0832d81
-- 
2.30.2
Re: [PATCH] x86/APIC: Remove esr_disable
Posted by Roger Pau Monné 2 years, 5 months ago 
On Tue, Aug 29, 2023 at 04:46:21PM +0100, Andrew Cooper wrote:
> It is unconditionally 0 in Xen, and was deleted in Linux somewhere between 2.5
> and 2.6.
> 
> Remove it in Xen too.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> 
> I got bored waiting for `git log` to tell where it was disabled in Linux...
> ---
>  xen/arch/x86/apic.c                           | 50 ++++++-------------
>  .../x86/include/asm/mach-generic/mach_apic.h  |  3 --
>  2 files changed, 16 insertions(+), 37 deletions(-)
> 
> diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
> index 41879230ec90..5c6935ba42db 100644
> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -496,14 +496,6 @@ void setup_local_APIC(bool bsp)
>      unsigned long oldvalue, value, maxlvt;
>      int i, j;
>  
> -    /* Pound the ESR really hard over the head with a big hammer - mbligh */
> -    if (esr_disable) {
> -        apic_write(APIC_ESR, 0);
> -        apic_write(APIC_ESR, 0);
> -        apic_write(APIC_ESR, 0);
> -        apic_write(APIC_ESR, 0);
> -    }
> -
>      BUILD_BUG_ON((SPURIOUS_APIC_VECTOR & 0x0f) != 0x0f);
>  
>      /*
> @@ -628,33 +620,23 @@ void setup_local_APIC(bool bsp)
>          value = APIC_DM_NMI | APIC_LVT_MASKED;
>      apic_write(APIC_LVT1, value);
>  
> -    if (!esr_disable) {
> -        maxlvt = get_maxlvt();
> -        if (maxlvt > 3)     /* Due to the Pentium erratum 3AP. */
> -            apic_write(APIC_ESR, 0);
> -        oldvalue = apic_read(APIC_ESR);
> +    maxlvt = get_maxlvt();
> +    if (maxlvt > 3)     /* Due to the Pentium erratum 3AP. */
> +        apic_write(APIC_ESR, 0);
> +    oldvalue = apic_read(APIC_ESR);
>  
> -        value = ERROR_APIC_VECTOR;      // enables sending errors
> -        apic_write(APIC_LVTERR, value);
> -        /*
> -         * spec says clear errors after enabling vector.
> -         */
> -        if (maxlvt > 3)
> -            apic_write(APIC_ESR, 0);
> -        value = apic_read(APIC_ESR);
> -        if (value != oldvalue)
> -            apic_printk(APIC_VERBOSE, "ESR value before enabling "
> -                        "vector: %#lx  after: %#lx\n",
> -                        oldvalue, value);
> -    } else {
> -        /*
> -         * Something untraceble is creating bad interrupts on
> -         * secondary quads ... for the moment, just leave the
> -         * ESR disabled - we can't do anything useful with the
> -         * errors anyway - mbligh
> -         */
> -        printk("Leaving ESR disabled.\n");
> -    }
> +    value = ERROR_APIC_VECTOR;      // enables sending errors
> +    apic_write(APIC_LVTERR, value);
> +    /*
> +     * spec says clear errors after enabling vector.

Is it worth making this a one-line comment while adjusting padding?

Regardless:

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.
Re: [PATCH] x86/APIC: Remove esr_disable
Posted by Andrew Cooper 2 years, 5 months ago 
On 29/08/2023 5:00 pm, Roger Pau Monné wrote:
> On Tue, Aug 29, 2023 at 04:46:21PM +0100, Andrew Cooper wrote:
>> It is unconditionally 0 in Xen, and was deleted in Linux somewhere between 2.5
>> and 2.6.
>>
>> Remove it in Xen too.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Wei Liu <wl@xen.org>
>>
>> I got bored waiting for `git log` to tell where it was disabled in Linux...
>> ---
>>  xen/arch/x86/apic.c                           | 50 ++++++-------------
>>  .../x86/include/asm/mach-generic/mach_apic.h  |  3 --
>>  2 files changed, 16 insertions(+), 37 deletions(-)
>>
>> diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
>> index 41879230ec90..5c6935ba42db 100644
>> --- a/xen/arch/x86/apic.c
>> +++ b/xen/arch/x86/apic.c
>> @@ -496,14 +496,6 @@ void setup_local_APIC(bool bsp)
>>      unsigned long oldvalue, value, maxlvt;
>>      int i, j;
>>  
>> -    /* Pound the ESR really hard over the head with a big hammer - mbligh */
>> -    if (esr_disable) {
>> -        apic_write(APIC_ESR, 0);
>> -        apic_write(APIC_ESR, 0);
>> -        apic_write(APIC_ESR, 0);
>> -        apic_write(APIC_ESR, 0);
>> -    }
>> -
>>      BUILD_BUG_ON((SPURIOUS_APIC_VECTOR & 0x0f) != 0x0f);
>>  
>>      /*
>> @@ -628,33 +620,23 @@ void setup_local_APIC(bool bsp)
>>          value = APIC_DM_NMI | APIC_LVT_MASKED;
>>      apic_write(APIC_LVT1, value);
>>  
>> -    if (!esr_disable) {
>> -        maxlvt = get_maxlvt();
>> -        if (maxlvt > 3)     /* Due to the Pentium erratum 3AP. */
>> -            apic_write(APIC_ESR, 0);
>> -        oldvalue = apic_read(APIC_ESR);
>> +    maxlvt = get_maxlvt();
>> +    if (maxlvt > 3)     /* Due to the Pentium erratum 3AP. */
>> +        apic_write(APIC_ESR, 0);
>> +    oldvalue = apic_read(APIC_ESR);
>>  
>> -        value = ERROR_APIC_VECTOR;      // enables sending errors
>> -        apic_write(APIC_LVTERR, value);
>> -        /*
>> -         * spec says clear errors after enabling vector.
>> -         */
>> -        if (maxlvt > 3)
>> -            apic_write(APIC_ESR, 0);
>> -        value = apic_read(APIC_ESR);
>> -        if (value != oldvalue)
>> -            apic_printk(APIC_VERBOSE, "ESR value before enabling "
>> -                        "vector: %#lx  after: %#lx\n",
>> -                        oldvalue, value);
>> -    } else {
>> -        /*
>> -         * Something untraceble is creating bad interrupts on
>> -         * secondary quads ... for the moment, just leave the
>> -         * ESR disabled - we can't do anything useful with the
>> -         * errors anyway - mbligh
>> -         */
>> -        printk("Leaving ESR disabled.\n");
>> -    }
>> +    value = ERROR_APIC_VECTOR;      // enables sending errors
>> +    apic_write(APIC_LVTERR, value);
>> +    /*
>> +     * spec says clear errors after enabling vector.
> Is it worth making this a one-line comment while adjusting padding?

Yeah ok.

>
> Regardless:
>
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

~Andrew
Re: [PATCH] x86/APIC: Remove esr_disable
Posted by Jan Beulich 2 years, 5 months ago 
On 29.08.2023 17:46, Andrew Cooper wrote:
> It is unconditionally 0 in Xen, and was deleted in Linux somewhere between 2.5
> and 2.6.
> 
> Remove it in Xen too.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.