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

Groups > linux.kernel > #1612011 > unrolled thread

[RFC PATCH 1/6] x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_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 1/6] x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup() Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-03-29 17:00 +0200
    Re: [RFC PATCH 1/6] x86/apic: Replace init_bsp_APIC() with  apic_virture_wire_mode_setup() Thomas Gleixner <tglx@linutronix.de> - 2017-04-05 12:50 +0200
      Re: [RFC PATCH 1/6] x86/apic: Replace init_bsp_APIC() with  apic_virture_wire_mode_setup() Thomas Gleixner <tglx@linutronix.de> - 2017-04-05 13:00 +0200
        Re: [RFC PATCH 1/6] x86/apic: Replace init_bsp_APIC() with  apic_virture_wire_mode_setup() Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-04-06 03:40 +0200
      Re: [RFC PATCH 1/6] x86/apic: Replace init_bsp_APIC() with  apic_virture_wire_mode_setup() Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-04-06 03:40 +0200

#1612011 — [RFC PATCH 1/6] x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup()

The init_bsp_APIC() setups the virtual wire mode through the local
APIC.

The function name is unsuitable which might imply that the BSP's
APIC will be initialized here, actually, where it will be done is
almost at the end of start_kernel(). And the CONFIG X86_64 is also
imply the X86_LOCAL_APIC is y.

Clarify it, also remove the redundant macros to increase
readability

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

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 730ef65..20ac73c 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -127,6 +127,7 @@ extern void disconnect_bsp_APIC(int virt_wire_setup);
 extern void disable_local_APIC(void);
 extern void lapic_shutdown(void);
 extern void sync_Arb_IDs(void);
+extern void apic_virture_wire_mode_setup(void);
 extern void init_bsp_APIC(void);
 extern void setup_local_APIC(void);
 extern void init_apic_mappings(void);
@@ -170,6 +171,7 @@ static inline void disable_local_APIC(void) { }
 # define setup_boot_APIC_clock x86_init_noop
 # define setup_secondary_APIC_clock x86_init_noop
 static inline void lapic_update_tsc_freq(void) { }
+static inline void apic_virture_wire_mode_setup(void) {}
 #endif /* !CONFIG_X86_LOCAL_APIC */
 
 #ifdef CONFIG_X86_X2APIC
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 8ccb7ef..f4fc949 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1154,9 +1154,9 @@ void __init sync_Arb_IDs(void)
 }
 
 /*
- * An initial setup of the virtual wire mode.
+ * Setup the through-local-APIC virtual wire mode.
  */
-void __init init_bsp_APIC(void)
+void apic_virture_wire_mode_setup(void)
 {
 	unsigned int value;
 
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index 1423ab1..b6ef4ea 100644
--- a/arch/x86/kernel/irqinit.c
+++ b/arch/x86/kernel/irqinit.c
@@ -72,9 +72,8 @@ void __init init_ISA_irqs(void)
 	struct irq_chip *chip = legacy_pic->chip;
 	int i;
 
-#if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC)
-	init_bsp_APIC();
-#endif
+	apic_virture_wire_mode_setup();
+
 	legacy_pic->init(0);
 
 	for (i = 0; i < nr_legacy_irqs(); i++)
-- 
2.5.5

[toc] | [next] | [standalone]

#1616832 — Re: [RFC PATCH 1/6] x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup()

On Wed, 29 Mar 2017, Dou Liyang wrote:

> The init_bsp_APIC() setups the virtual wire mode through the local
> APIC.
> 
> The function name is unsuitable which might imply that the BSP's
> APIC will be initialized here, actually, where it will be done is
> almost at the end of start_kernel(). And the CONFIG X86_64 is also
> imply the X86_LOCAL_APIC is y.

Correct, but X86_32 can have X86_LOCAL_APIC=n. And by removing the ifdefs
you break that.

>  /*
> - * An initial setup of the virtual wire mode.
> + * Setup the through-local-APIC virtual wire mode.
>   */
> -void __init init_bsp_APIC(void)
> +void apic_virture_wire_mode_setup(void)

s/virture/virtual/ ?

Why is this function not longer marked __init ?

Thanks,

	tglx

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

#1616834 — Re: [RFC PATCH 1/6] x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup()

On Wed, 5 Apr 2017, Thomas Gleixner wrote:

> On Wed, 29 Mar 2017, Dou Liyang wrote:
> 
> > The init_bsp_APIC() setups the virtual wire mode through the local
> > APIC.
> > 
> > The function name is unsuitable which might imply that the BSP's
> > APIC will be initialized here, actually, where it will be done is
> > almost at the end of start_kernel(). And the CONFIG X86_64 is also
> > imply the X86_LOCAL_APIC is y.
> 
> Correct, but X86_32 can have X86_LOCAL_APIC=n. And by removing the ifdefs
> you break that.

Oops. Sorry, the function is replaced by an empty stub for the APIC=n
case. So that's correct.

> >  /*
> > - * An initial setup of the virtual wire mode.
> > + * Setup the through-local-APIC virtual wire mode.
> >   */
> > -void __init init_bsp_APIC(void)
> > +void apic_virture_wire_mode_setup(void)
> 
> s/virture/virtual/ ?
> 
> Why is this function not longer marked __init ?
> 
> Thanks,
> 
> 	tglx
> 

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

#1617462 — Re: [RFC PATCH 1/6] x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup()

Hi Thomas,

At 04/05/2017 06:50 PM, Thomas Gleixner wrote:
> On Wed, 5 Apr 2017, Thomas Gleixner wrote:
>
>> On Wed, 29 Mar 2017, Dou Liyang wrote:
>>
>>> The init_bsp_APIC() setups the virtual wire mode through the local
>>> APIC.
>>>
>>> The function name is unsuitable which might imply that the BSP's
>>> APIC will be initialized here, actually, where it will be done is
>>> almost at the end of start_kernel(). And the CONFIG X86_64 is also
>>> imply the X86_LOCAL_APIC is y.
>>
>> Correct, but X86_32 can have X86_LOCAL_APIC=n. And by removing the ifdefs
>> you break that.
>
> Oops. Sorry, the function is replaced by an empty stub for the APIC=n
> case. So that's correct.
>

I forgot to clear it in the change log. I will add this comment to the
change log.

Thanks,
	Liyang

>>>  /*
>>> - * An initial setup of the virtual wire mode.
>>> + * Setup the through-local-APIC virtual wire mode.
>>>   */
>>> -void __init init_bsp_APIC(void)
>>> +void apic_virture_wire_mode_setup(void)
>>
>> s/virture/virtual/ ?
>>
>> Why is this function not longer marked __init ?
>>
>> Thanks,
>>
>> 	tglx
>>
>
>
>

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

#1617460 — Re: [RFC PATCH 1/6] x86/apic: Replace init_bsp_APIC() with apic_virture_wire_mode_setup()

Hi Thomas,

Thank you very much!

At 04/05/2017 06:43 PM, Thomas Gleixner wrote:
> On Wed, 29 Mar 2017, Dou Liyang wrote:
>
>> The init_bsp_APIC() setups the virtual wire mode through the local
>> APIC.
>>
>> The function name is unsuitable which might imply that the BSP's
>> APIC will be initialized here, actually, where it will be done is
>> almost at the end of start_kernel(). And the CONFIG X86_64 is also
>> imply the X86_LOCAL_APIC is y.
>
> Correct, but X86_32 can have X86_LOCAL_APIC=n. And by removing the ifdefs
> you break that.
>
>>  /*
>> - * An initial setup of the virtual wire mode.
>> + * Setup the through-local-APIC virtual wire mode.
>>   */
>> -void __init init_bsp_APIC(void)
>> +void apic_virture_wire_mode_setup(void)
>
> s/virture/virtual/ ?

Yes, It is.

>
> Why is this function not longer marked __init ?
>

Oops. The __init is necessary. I will modify it.

Thanks,
	Liyang.


> Thanks,
>
> 	tglx
>
>
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web