Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1612000 > unrolled thread

[RFC PATCH 3/6] x86/apic: Extract APIC timer related code from apic_bsp_setup()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC PATCH 3/6] x86/apic: Extract APIC timer related code from apic_bsp_setup() Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-03-29 17:00 +0200
    Re: [RFC PATCH 3/6] x86/apic: Extract APIC timer related code from  apic_bsp_setup() Thomas Gleixner <tglx@linutronix.de> - 2017-04-05 14:00 +0200
      Re: [RFC PATCH 3/6] x86/apic: Extract APIC timer related code from  apic_bsp_setup() Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-04-07 10:00 +0200

#1612000 — [RFC PATCH 3/6] x86/apic: Extract APIC timer related code from apic_bsp_setup()

The apic_bsp_setup() contains the APIC timer related code, which
leads to hard reuse the local APIC and I/O APIC setup independently.

Extract the related code to a single function for setuping APIC in
advance.

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
 arch/x86/include/asm/io_apic.h |  2 ++
 arch/x86/kernel/apic/apic.c    | 28 +++++++++++++++++++++-------
 arch/x86/kernel/apic/io_apic.c |  4 +---
 3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 6cbf2cf..535ca00 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -189,6 +189,7 @@ static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
 	return x86_io_apic_ops.read(apic, reg);
 }
 
+extern void check_timer(void);
 extern void setup_IO_APIC(void);
 extern void enable_IO_APIC(void);
 extern void disable_IO_APIC(void);
@@ -230,6 +231,7 @@ static inline void io_apic_init_mappings(void) { }
 #define native_io_apic_read		NULL
 #define native_disable_io_apic		NULL
 
+static inline void check_timer(void) { }
 static inline void setup_IO_APIC(void) { }
 static inline void enable_IO_APIC(void) { }
 static inline void setup_ioapic_dest(void) { }
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index bf4ccd0..0ba8a85 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2324,6 +2324,25 @@ static void __init apic_bsp_up_setup(void)
 	physid_set_mask_of_physid(boot_cpu_physical_apicid, &phys_cpu_present_map);
 }
 
+/* Setup local APIC timer and get the Id*/
+static int __init apic_bsp_timer_setup(void)
+{
+	int id;
+
+	if (x2apic_mode)
+		id = apic_read(APIC_LDR);
+	else
+		id = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
+
+	if (!skip_ioapic_setup && nr_ioapics && nr_legacy_irqs())
+		check_timer();
+
+	/* Setup local timer */
+	x86_init.timers.setup_percpu_clockev();
+
+	return id;
+}
+
 /**
  * apic_bsp_setup - Setup function for local apic and io-apic
  * @upmode:		Force UP mode (for APIC_init_uniprocessor)
@@ -2340,17 +2359,12 @@ int __init apic_bsp_setup(bool upmode)
 		apic_bsp_up_setup();
 	setup_local_APIC();
 
-	if (x2apic_mode)
-		id = apic_read(APIC_LDR);
-	else
-		id = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
-
 	enable_IO_APIC();
 	end_local_APIC_setup();
 	irq_remap_enable_fault_handling();
 	setup_IO_APIC();
-	/* Setup local timer */
-	x86_init.timers.setup_percpu_clockev();
+
+	id = apic_bsp_timer_setup();
 	return id;
 }
 
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 347bb9f..e19b88f 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2047,7 +2047,7 @@ static int mp_alloc_timer_irq(int ioapic, int pin)
  *
  * FIXME: really need to revamp this for all platforms.
  */
-static inline void __init check_timer(void)
+void __init check_timer(void)
 {
 	struct irq_data *irq_data = irq_get_irq_data(0);
 	struct mp_chip_data *data = irq_data->chip_data;
@@ -2278,8 +2278,6 @@ void __init setup_IO_APIC(void)
 	sync_Arb_IDs();
 	setup_IO_APIC_irqs();
 	init_IO_APIC_traps();
-	if (nr_legacy_irqs())
-		check_timer();
 
 	ioapic_initialized = 1;
 }
-- 
2.5.5

[toc] | [next] | [standalone]

#1616870 — Re: [RFC PATCH 3/6] x86/apic: Extract APIC timer related code from apic_bsp_setup()

On Wed, 29 Mar 2017, Dou Liyang wrote:
> +/* Setup local APIC timer and get the Id*/
> +static int __init apic_bsp_timer_setup(void)

This does not make sense. The id and the timers have nothing to do with
each other. 

> +{
> +	int id;
> +
> +	if (x2apic_mode)
> +		id = apic_read(APIC_LDR);
> +	else
> +		id = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
> +
> +	if (!skip_ioapic_setup && nr_ioapics && nr_legacy_irqs())
> +		check_timer();

Why are you moving this to the APIC? check_timer() has absolutely nothing
to do with the apic timer. It's a IOAPIC only issue and required to test
that the IRQ0 interrupt delivery works through the IOAPIC.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1618555 — Re: [RFC PATCH 3/6] x86/apic: Extract APIC timer related code from apic_bsp_setup()

Hi Thomas,

At 04/05/2017 07:56 PM, Thomas Gleixner wrote:
> On Wed, 29 Mar 2017, Dou Liyang wrote:
>> +/* Setup local APIC timer and get the Id*/
>> +static int __init apic_bsp_timer_setup(void)
>
> This does not make sense. The id and the timers have nothing to do with
> each other.

Yes, Indeed. Here is more like the rest work for APIC setup, which
can't be executed earlier, The name is not suitable. I will refactor
it.

>> +{
>> +	int id;
>> +
>> +	if (x2apic_mode)
>> +		id = apic_read(APIC_LDR);
>> +	else
>> +		id = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
>> +
>> +	if (!skip_ioapic_setup && nr_ioapics && nr_legacy_irqs())
>> +		check_timer();
>
> Why are you moving this to the APIC? check_timer() has absolutely nothing

Because, the jiffies is used in check_timer() for checking timer irq 
works(timer_irq_works()) and can't work at the beginning.

So, keep check_timer() in IOAPIC setup function (setup_IO_APIC()) which
has been moved to init_IRQ() will make the kernel not boot up.

> to do with the apic timer. It's a IOAPIC only issue and required to test
> that the IRQ0 interrupt delivery works through the IOAPIC.
>

Yes, I see, split check_timer() is not a good way, and move the APIC
initialization to the end of init_IRQ() is also not well.

Thanks,
	Liyang

> Thanks,
>
> 	tglx
>
>
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web