This is a tiny cleanup patchset for setup_local_APIC().
Dou Liyang (3):
x86/apic: Move pending intr check code into it's own function
x86/apic: Replace common tools with new ones
x86/apic: Drop the logical_smp_processor_id()
arch/x86/include/asm/smp.h | 10 -----
arch/x86/kernel/apic/apic.c | 107 ++++++++++++++++++++++++--------------------
2 files changed, 58 insertions(+), 59 deletions(-)
--
2.14.3
The pending interrupt check code is old, update the following.
-Replace for-if pair with for_each_set_bit()
-Replace printk() with pr_err()
Also merge the printk's code in one line and make curly braces balanced
Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: Dou Liyang <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
arch/x86/kernel/apic/apic.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 69d2936e3154..7a347d7450b6 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1412,7 +1412,8 @@ static void apic_pending_intr_clear(void)
{
long long max_loops = cpu_khz ? cpu_khz : 1000000;
unsigned long long tsc = 0, ntsc;
- unsigned int value, queued;
+ unsigned int queued;
+ unsigned long value;
int i, j, acked = 0;
if (boot_cpu_has(X86_FEATURE_TSC))
@@ -1435,24 +1436,22 @@ static void apic_pending_intr_clear(void)
for (i = APIC_ISR_NR - 1; i >= 0; i--) {
value = apic_read(APIC_ISR + i*0x10);
- for (j = 31; j >= 0; j--) {
- if (value & (1<<j)) {
- ack_APIC_irq();
- acked++;
- }
+ for_each_set_bit(j, &value, 32) {
+ ack_APIC_irq();
+ acked++;
}
}
if (acked > 256) {
- printk(KERN_ERR "LAPIC pending interrupts after %d EOI\n",
- acked);
+ pr_err("LAPIC pending interrupts after %d EOI\n", acked);
break;
}
if (queued) {
if (boot_cpu_has(X86_FEATURE_TSC) && cpu_khz) {
ntsc = rdtsc();
max_loops = (cpu_khz << 10) - (ntsc - tsc);
- } else
+ } else {
max_loops--;
+ }
}
} while (queued && max_loops > 0);
WARN_ON(max_loops <= 0);
--
2.14.3
The logical_smp_processor_id() which is only called in setup_local_APIC()
on x86_32 system looks redundant.
Drop it, then directly use GET_APIC_LOGICAL_ID() marco and more suitable
variable name for readability
Signed-off-by: Dou Liyang <[email protected]>
---
arch/x86/include/asm/smp.h | 10 ----------
arch/x86/kernel/apic/apic.c | 10 +++++-----
2 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 461f53d27708..e2057780d67f 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -176,16 +176,6 @@ static inline int wbinvd_on_all_cpus(void)
extern unsigned disabled_cpus;
#ifdef CONFIG_X86_LOCAL_APIC
-
-#ifndef CONFIG_X86_64
-static inline int logical_smp_processor_id(void)
-{
- /* we don't want to mark this access volatile - bad code generation */
- return GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
-}
-
-#endif
-
extern int hard_smp_processor_id(void);
#else /* CONFIG_X86_LOCAL_APIC */
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 7a347d7450b6..ce2066373922 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1468,7 +1468,7 @@ static void setup_local_APIC(void)
int cpu = smp_processor_id();
unsigned int value;
#ifdef CONFIG_X86_32
- int i;
+ int logical_apicid, ldr_apicid;
#endif
@@ -1507,11 +1507,11 @@ static void setup_local_APIC(void)
* initialized during get_smp_config(), make sure it matches the
* actual value.
*/
- i = early_per_cpu(x86_cpu_to_logical_apicid, cpu);
- WARN_ON(i != BAD_APICID && i != logical_smp_processor_id());
+ logical_apicid = early_per_cpu(x86_cpu_to_logical_apicid, cpu);
+ ldr_apicid = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
+ WARN_ON(logical_apicid != BAD_APICID && logical_apicid != ldr_apicid);
/* always use the value from LDR */
- early_per_cpu(x86_cpu_to_logical_apicid, cpu) =
- logical_smp_processor_id();
+ early_per_cpu(x86_cpu_to_logical_apicid, cpu) = ldr_apicid;
#endif
/*
--
2.14.3
the pending interrupt check code is mixed with the local APIC setup code,
that looks messy.
Extract the related code, move it into a new function named
apic_pending_intr_clear().
Signed-off-by: Dou Liyang <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
changelog:
v4 --> v5:
- Fix undeclared 'i' error reported by LKP
arch/x86/kernel/apic/apic.c | 100 ++++++++++++++++++++++++--------------------
1 file changed, 55 insertions(+), 45 deletions(-)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 2ceac9f6c5d7..69d2936e3154 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1408,6 +1408,56 @@ static void lapic_setup_esr(void)
oldvalue, value);
}
+static void apic_pending_intr_clear(void)
+{
+ long long max_loops = cpu_khz ? cpu_khz : 1000000;
+ unsigned long long tsc = 0, ntsc;
+ unsigned int value, queued;
+ int i, j, acked = 0;
+
+ if (boot_cpu_has(X86_FEATURE_TSC))
+ tsc = rdtsc();
+ /*
+ * After a crash, we no longer service the interrupts and a pending
+ * interrupt from previous kernel might still have ISR bit set.
+ *
+ * Most probably by now CPU has serviced that pending interrupt and
+ * it might not have done the ack_APIC_irq() because it thought,
+ * interrupt came from i8259 as ExtInt. LAPIC did not get EOI so it
+ * does not clear the ISR bit and cpu thinks it has already serivced
+ * the interrupt. Hence a vector might get locked. It was noticed
+ * for timer irq (vector 0x31). Issue an extra EOI to clear ISR.
+ */
+ do {
+ queued = 0;
+ for (i = APIC_ISR_NR - 1; i >= 0; i--)
+ queued |= apic_read(APIC_IRR + i*0x10);
+
+ for (i = APIC_ISR_NR - 1; i >= 0; i--) {
+ value = apic_read(APIC_ISR + i*0x10);
+ for (j = 31; j >= 0; j--) {
+ if (value & (1<<j)) {
+ ack_APIC_irq();
+ acked++;
+ }
+ }
+ }
+ if (acked > 256) {
+ printk(KERN_ERR "LAPIC pending interrupts after %d EOI\n",
+ acked);
+ break;
+ }
+ if (queued) {
+ if (boot_cpu_has(X86_FEATURE_TSC) && cpu_khz) {
+ ntsc = rdtsc();
+ max_loops = (cpu_khz << 10) - (ntsc - tsc);
+ } else
+ max_loops--;
+ }
+ } while (queued && max_loops > 0);
+ WARN_ON(max_loops <= 0);
+}
+
/**
* setup_local_APIC - setup the local APIC
*
@@ -1417,13 +1467,11 @@ static void lapic_setup_esr(void)
static void setup_local_APIC(void)
{
int cpu = smp_processor_id();
- unsigned int value, queued;
- int i, j, acked = 0;
- unsigned long long tsc = 0, ntsc;
- long long max_loops = cpu_khz ? cpu_khz : 1000000;
+ unsigned int value;
+#ifdef CONFIG_X86_32
+ int i;
+#endif
- if (boot_cpu_has(X86_FEATURE_TSC))
- tsc = rdtsc();
if (disable_apic) {
disable_ioapic_support();
@@ -1475,45 +1523,7 @@ static void setup_local_APIC(void)
value &= ~APIC_TPRI_MASK;
apic_write(APIC_TASKPRI, value);
- /*
- * After a crash, we no longer service the interrupts and a pending
- * interrupt from previous kernel might still have ISR bit set.
- *
- * Most probably by now CPU has serviced that pending interrupt and
- * it might not have done the ack_APIC_irq() because it thought,
- * interrupt came from i8259 as ExtInt. LAPIC did not get EOI so it
- * does not clear the ISR bit and cpu thinks it has already serivced
- * the interrupt. Hence a vector might get locked. It was noticed
- * for timer irq (vector 0x31). Issue an extra EOI to clear ISR.
- */
- do {
- queued = 0;
- for (i = APIC_ISR_NR - 1; i >= 0; i--)
- queued |= apic_read(APIC_IRR + i*0x10);
-
- for (i = APIC_ISR_NR - 1; i >= 0; i--) {
- value = apic_read(APIC_ISR + i*0x10);
- for (j = 31; j >= 0; j--) {
- if (value & (1<<j)) {
- ack_APIC_irq();
- acked++;
- }
- }
- }
- if (acked > 256) {
- printk(KERN_ERR "LAPIC pending interrupts after %d EOI\n",
- acked);
- break;
- }
- if (queued) {
- if (boot_cpu_has(X86_FEATURE_TSC) && cpu_khz) {
- ntsc = rdtsc();
- max_loops = (cpu_khz << 10) - (ntsc - tsc);
- } else
- max_loops--;
- }
- } while (queued && max_loops > 0);
- WARN_ON(max_loops <= 0);
+ apic_pending_intr_clear();
/*
* Now that we are all set up, enable the APIC
--
2.14.3
Commit-ID: 9b217f33017715903d0956dfc58f82d2a2d00e63
Gitweb: https://git.kernel.org/tip/9b217f33017715903d0956dfc58f82d2a2d00e63
Author: Dou Liyang <[email protected]>
AuthorDate: Thu, 1 Mar 2018 13:59:28 +0800
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 1 Mar 2018 10:12:20 +0100
x86/apic: Move pending interrupt check code into it's own function
The pending interrupt check code is mixed with the local APIC setup code,
that looks messy.
Extract the related code, move it into a new function named
apic_pending_intr_clear().
Signed-off-by: Dou Liyang <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Cc: [email protected]
Cc: [email protected]
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/kernel/apic/apic.c | 100 ++++++++++++++++++++++++--------------------
1 file changed, 55 insertions(+), 45 deletions(-)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 2ceac9f6c5d7..69d2936e3154 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1408,6 +1408,56 @@ static void lapic_setup_esr(void)
oldvalue, value);
}
+static void apic_pending_intr_clear(void)
+{
+ long long max_loops = cpu_khz ? cpu_khz : 1000000;
+ unsigned long long tsc = 0, ntsc;
+ unsigned int value, queued;
+ int i, j, acked = 0;
+
+ if (boot_cpu_has(X86_FEATURE_TSC))
+ tsc = rdtsc();
+ /*
+ * After a crash, we no longer service the interrupts and a pending
+ * interrupt from previous kernel might still have ISR bit set.
+ *
+ * Most probably by now CPU has serviced that pending interrupt and
+ * it might not have done the ack_APIC_irq() because it thought,
+ * interrupt came from i8259 as ExtInt. LAPIC did not get EOI so it
+ * does not clear the ISR bit and cpu thinks it has already serivced
+ * the interrupt. Hence a vector might get locked. It was noticed
+ * for timer irq (vector 0x31). Issue an extra EOI to clear ISR.
+ */
+ do {
+ queued = 0;
+ for (i = APIC_ISR_NR - 1; i >= 0; i--)
+ queued |= apic_read(APIC_IRR + i*0x10);
+
+ for (i = APIC_ISR_NR - 1; i >= 0; i--) {
+ value = apic_read(APIC_ISR + i*0x10);
+ for (j = 31; j >= 0; j--) {
+ if (value & (1<<j)) {
+ ack_APIC_irq();
+ acked++;
+ }
+ }
+ }
+ if (acked > 256) {
+ printk(KERN_ERR "LAPIC pending interrupts after %d EOI\n",
+ acked);
+ break;
+ }
+ if (queued) {
+ if (boot_cpu_has(X86_FEATURE_TSC) && cpu_khz) {
+ ntsc = rdtsc();
+ max_loops = (cpu_khz << 10) - (ntsc - tsc);
+ } else
+ max_loops--;
+ }
+ } while (queued && max_loops > 0);
+ WARN_ON(max_loops <= 0);
+}
+
/**
* setup_local_APIC - setup the local APIC
*
@@ -1417,13 +1467,11 @@ static void lapic_setup_esr(void)
static void setup_local_APIC(void)
{
int cpu = smp_processor_id();
- unsigned int value, queued;
- int i, j, acked = 0;
- unsigned long long tsc = 0, ntsc;
- long long max_loops = cpu_khz ? cpu_khz : 1000000;
+ unsigned int value;
+#ifdef CONFIG_X86_32
+ int i;
+#endif
- if (boot_cpu_has(X86_FEATURE_TSC))
- tsc = rdtsc();
if (disable_apic) {
disable_ioapic_support();
@@ -1475,45 +1523,7 @@ static void setup_local_APIC(void)
value &= ~APIC_TPRI_MASK;
apic_write(APIC_TASKPRI, value);
- /*
- * After a crash, we no longer service the interrupts and a pending
- * interrupt from previous kernel might still have ISR bit set.
- *
- * Most probably by now CPU has serviced that pending interrupt and
- * it might not have done the ack_APIC_irq() because it thought,
- * interrupt came from i8259 as ExtInt. LAPIC did not get EOI so it
- * does not clear the ISR bit and cpu thinks it has already serivced
- * the interrupt. Hence a vector might get locked. It was noticed
- * for timer irq (vector 0x31). Issue an extra EOI to clear ISR.
- */
- do {
- queued = 0;
- for (i = APIC_ISR_NR - 1; i >= 0; i--)
- queued |= apic_read(APIC_IRR + i*0x10);
-
- for (i = APIC_ISR_NR - 1; i >= 0; i--) {
- value = apic_read(APIC_ISR + i*0x10);
- for (j = 31; j >= 0; j--) {
- if (value & (1<<j)) {
- ack_APIC_irq();
- acked++;
- }
- }
- }
- if (acked > 256) {
- printk(KERN_ERR "LAPIC pending interrupts after %d EOI\n",
- acked);
- break;
- }
- if (queued) {
- if (boot_cpu_has(X86_FEATURE_TSC) && cpu_khz) {
- ntsc = rdtsc();
- max_loops = (cpu_khz << 10) - (ntsc - tsc);
- } else
- max_loops--;
- }
- } while (queued && max_loops > 0);
- WARN_ON(max_loops <= 0);
+ apic_pending_intr_clear();
/*
* Now that we are all set up, enable the APIC
Commit-ID: 3ea9e7ae1a2038b9fdff729861c9b4af0087024f
Gitweb: https://git.kernel.org/tip/3ea9e7ae1a2038b9fdff729861c9b4af0087024f
Author: Dou Liyang <[email protected]>
AuthorDate: Thu, 1 Mar 2018 13:59:29 +0800
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 1 Mar 2018 10:12:20 +0100
x86/apic: Modernize the pending interrupt code
The pending interrupt check code is old, update the following:
- Use for_each_set_bit() instead of open coding it
- Replace printk() with pr_err()
- Get rid of printk line breaks
- Make curly braces balanced
Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: Dou Liyang <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Cc: [email protected]
Cc: [email protected]
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/kernel/apic/apic.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 69d2936e3154..7a347d7450b6 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1412,7 +1412,8 @@ static void apic_pending_intr_clear(void)
{
long long max_loops = cpu_khz ? cpu_khz : 1000000;
unsigned long long tsc = 0, ntsc;
- unsigned int value, queued;
+ unsigned int queued;
+ unsigned long value;
int i, j, acked = 0;
if (boot_cpu_has(X86_FEATURE_TSC))
@@ -1435,24 +1436,22 @@ static void apic_pending_intr_clear(void)
for (i = APIC_ISR_NR - 1; i >= 0; i--) {
value = apic_read(APIC_ISR + i*0x10);
- for (j = 31; j >= 0; j--) {
- if (value & (1<<j)) {
- ack_APIC_irq();
- acked++;
- }
+ for_each_set_bit(j, &value, 32) {
+ ack_APIC_irq();
+ acked++;
}
}
if (acked > 256) {
- printk(KERN_ERR "LAPIC pending interrupts after %d EOI\n",
- acked);
+ pr_err("LAPIC pending interrupts after %d EOI\n", acked);
break;
}
if (queued) {
if (boot_cpu_has(X86_FEATURE_TSC) && cpu_khz) {
ntsc = rdtsc();
max_loops = (cpu_khz << 10) - (ntsc - tsc);
- } else
+ } else {
max_loops--;
+ }
}
} while (queued && max_loops > 0);
WARN_ON(max_loops <= 0);
Commit-ID: 8f1561680f42a5491b371b513f1ab8197f31fd62
Gitweb: https://git.kernel.org/tip/8f1561680f42a5491b371b513f1ab8197f31fd62
Author: Dou Liyang <[email protected]>
AuthorDate: Thu, 1 Mar 2018 13:59:30 +0800
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 1 Mar 2018 10:12:21 +0100
x86/apic: Drop logical_smp_processor_id() inline
The logical_smp_processor_id() inline which is only called in
setup_local_APIC() on x86_32 systems has no real value.
Drop it and directly use GET_APIC_LOGICAL_ID() at the call site and use a
more suitable variable name for readability
Signed-off-by: Dou Liyang <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/include/asm/smp.h | 10 ----------
arch/x86/kernel/apic/apic.c | 10 +++++-----
2 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 461f53d27708..e2057780d67f 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -176,16 +176,6 @@ static inline int wbinvd_on_all_cpus(void)
extern unsigned disabled_cpus;
#ifdef CONFIG_X86_LOCAL_APIC
-
-#ifndef CONFIG_X86_64
-static inline int logical_smp_processor_id(void)
-{
- /* we don't want to mark this access volatile - bad code generation */
- return GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
-}
-
-#endif
-
extern int hard_smp_processor_id(void);
#else /* CONFIG_X86_LOCAL_APIC */
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 7a347d7450b6..ce2066373922 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1468,7 +1468,7 @@ static void setup_local_APIC(void)
int cpu = smp_processor_id();
unsigned int value;
#ifdef CONFIG_X86_32
- int i;
+ int logical_apicid, ldr_apicid;
#endif
@@ -1507,11 +1507,11 @@ static void setup_local_APIC(void)
* initialized during get_smp_config(), make sure it matches the
* actual value.
*/
- i = early_per_cpu(x86_cpu_to_logical_apicid, cpu);
- WARN_ON(i != BAD_APICID && i != logical_smp_processor_id());
+ logical_apicid = early_per_cpu(x86_cpu_to_logical_apicid, cpu);
+ ldr_apicid = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
+ WARN_ON(logical_apicid != BAD_APICID && logical_apicid != ldr_apicid);
/* always use the value from LDR */
- early_per_cpu(x86_cpu_to_logical_apicid, cpu) =
- logical_smp_processor_id();
+ early_per_cpu(x86_cpu_to_logical_apicid, cpu) = ldr_apicid;
#endif
/*