All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] v2: x86,APIC: Detect lapic_is_integrated() once - use on and  on
@ 2009-05-26  1:20 Rakib Mullick
  2009-05-26  2:11 ` Yinghai Lu
  0 siblings, 1 reply; 6+ messages in thread
From: Rakib Mullick @ 2009-05-26  1:20 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Thomas Gleixner, H. Peter Anvin, macro, Cyrill Gorcunov,
	Andrew Morton, LKML

 This patch was sent previously (http://lkml.org/lkml/2009/5/22/240) -
but that one contains flaw as that one prevents compiler to optimize
the X86_64 case out. Thomas Gleixner (so thanks Thomas) shows how to
prevent that flaw. Also it helps to avoid cache line pollution. So, I
think with this patch everyone would be happy.
		
Thanks,
--
Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>

--- linus/arch/x86/kernel/apic/apic.c	2009-05-25 00:20:25.000000000 +0600
+++ rakib/arch/x86/kernel/apic/apic.c	2009-05-25 00:50:08.333910872 +0600
@@ -127,6 +127,13 @@ early_param("nox2apic", setup_nox2apic);

 unsigned long mp_lapic_addr;
 int disable_apic;
+
+#ifdef	CONFIG_X86_64
+# define integrated_lapic (1)
+#else
+static int integrated_lapic __read_mostly;
+#endif
+
 /* Disable local APIC timer from the kernel commandline or via dmi quirk */
 static int disable_apic_timer __cpuinitdata;
 /* Local APIC timer works in C2 */
@@ -188,12 +195,10 @@ static inline int lapic_get_version(void
 /*
  * Check, if the APIC is integrated or a separate chip
  */
-static inline int lapic_is_integrated(void)
+static inline void lapic_is_integrated(void)
 {
-#ifdef CONFIG_X86_64
-	return 1;
-#else
-	return APIC_INTEGRATED(lapic_get_version());
+#ifdef CONFIG_X86_32
+	integrated_lapic = APIC_INTEGRATED(lapic_get_version());
 #endif
 }

@@ -258,7 +263,7 @@ void __cpuinit enable_NMI_through_LVT0(v
 	v = APIC_DM_NMI;

 	/* Level triggered for 82489DX (32bit mode) */
-	if (!lapic_is_integrated())
+	if (!integrated_lapic)
 		v |= APIC_LVT_LEVEL_TRIGGER;

 	apic_write(APIC_LVT0, v);
@@ -313,7 +318,7 @@ static void __setup_APIC_LVTT(unsigned i
 	lvtt_value = LOCAL_TIMER_VECTOR;
 	if (!oneshot)
 		lvtt_value |= APIC_LVT_TIMER_PERIODIC;
-	if (!lapic_is_integrated())
+	if (!integrated_lapic)
 		lvtt_value |= SET_APIC_TIMER_BASE(APIC_TIMER_BASE_DIV);

 	if (!irqen)
@@ -869,7 +874,7 @@ void clear_local_APIC(void)
 		apic_write(APIC_LVTPC, APIC_LVT_MASKED);

 	/* Integrated APIC (!82489DX) ? */
-	if (lapic_is_integrated()) {
+	if (integrated_lapic) {
 		if (maxlvt > 3)
 			/* Clear ESR due to Pentium errata 3AP and 11AP */
 			apic_write(APIC_ESR, 0);
@@ -1064,7 +1069,7 @@ void __init init_bsp_APIC(void)
 	 */
 	apic_write(APIC_LVT0, APIC_DM_EXTINT);
 	value = APIC_DM_NMI;
-	if (!lapic_is_integrated())		/* 82489DX */
+	if (!integrated_lapic)		/* 82489DX */
 		value |= APIC_LVT_LEVEL_TRIGGER;
 	apic_write(APIC_LVT1, value);
 }
@@ -1073,7 +1078,7 @@ static void __cpuinit lapic_setup_esr(vo
 {
 	unsigned int oldvalue, value, maxlvt;

-	if (!lapic_is_integrated()) {
+	if (!integrated_lapic) {
 		pr_info("No ESR for 82489DX.\n");
 		return;
 	}
@@ -1126,7 +1131,7 @@ void __cpuinit setup_local_APIC(void)

 #ifdef CONFIG_X86_32
 	/* Pound the ESR really hard over the head with a big hammer - mbligh */
-	if (lapic_is_integrated() && apic->disable_esr) {
+	if (integrated_lapic && apic->disable_esr) {
 		apic_write(APIC_ESR, 0);
 		apic_write(APIC_ESR, 0);
 		apic_write(APIC_ESR, 0);
@@ -1251,7 +1256,7 @@ void __cpuinit setup_local_APIC(void)
 		value = APIC_DM_NMI;
 	else
 		value = APIC_DM_NMI | APIC_LVT_MASKED;
-	if (!lapic_is_integrated())		/* 82489DX */
+	if (!integrated_lapic)		/* 82489DX */
 		value |= APIC_LVT_LEVEL_TRIGGER;
 	apic_write(APIC_LVT1, value);

@@ -1599,6 +1604,8 @@ int __init APIC_init_uniprocessor(void)
 		clear_cpu_cap(&boot_cpu_data, X86_FEATURE_APIC);
 		return -1;
 	}
+	/* Determine APIC is integrated or not.	*/
+	lapic_is_integrated();
 #endif

 	enable_IR_x2apic();

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] v2: x86,APIC: Detect lapic_is_integrated() once - use on  and on
  2009-05-26  1:20 [PATCH] v2: x86,APIC: Detect lapic_is_integrated() once - use on and on Rakib Mullick
@ 2009-05-26  2:11 ` Yinghai Lu
  2009-05-27  2:49   ` Rakib Mullick
  0 siblings, 1 reply; 6+ messages in thread
From: Yinghai Lu @ 2009-05-26  2:11 UTC (permalink / raw)
  To: Rakib Mullick
  Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, macro,
	Cyrill Gorcunov, Andrew Morton, LKML

On Mon, May 25, 2009 at 6:20 PM, Rakib Mullick <rakib.mullick@gmail.com> wrote:
>  This patch was sent previously (http://lkml.org/lkml/2009/5/22/240) -
> but that one contains flaw as that one prevents compiler to optimize
> the X86_64 case out. Thomas Gleixner (so thanks Thomas) shows how to
> prevent that flaw. Also it helps to avoid cache line pollution. So, I
> think with this patch everyone would be happy.
>
> Thanks,
> --
> Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
>
> --- linus/arch/x86/kernel/apic/apic.c   2009-05-25 00:20:25.000000000 +0600
> +++ rakib/arch/x86/kernel/apic/apic.c   2009-05-25 00:50:08.333910872 +0600
> @@ -127,6 +127,13 @@ early_param("nox2apic", setup_nox2apic);
>
>  unsigned long mp_lapic_addr;
>  int disable_apic;
> +
> +#ifdef CONFIG_X86_64
> +# define integrated_lapic (1)
> +#else
> +static int integrated_lapic __read_mostly;
> +#endif
> +
>  /* Disable local APIC timer from the kernel commandline or via dmi quirk */
>  static int disable_apic_timer __cpuinitdata;
>  /* Local APIC timer works in C2 */
> @@ -188,12 +195,10 @@ static inline int lapic_get_version(void
>  /*
>  * Check, if the APIC is integrated or a separate chip
>  */
> -static inline int lapic_is_integrated(void)
> +static inline void lapic_is_integrated(void)
>  {
> -#ifdef CONFIG_X86_64
> -       return 1;
> -#else
> -       return APIC_INTEGRATED(lapic_get_version());
> +#ifdef CONFIG_X86_32
> +       integrated_lapic = APIC_INTEGRATED(lapic_get_version());
>  #endif
>  }
>
> @@ -258,7 +263,7 @@ void __cpuinit enable_NMI_through_LVT0(v
>        v = APIC_DM_NMI;
>
>        /* Level triggered for 82489DX (32bit mode) */
> -       if (!lapic_is_integrated())
> +       if (!integrated_lapic)
>                v |= APIC_LVT_LEVEL_TRIGGER;
>
>        apic_write(APIC_LVT0, v);
> @@ -313,7 +318,7 @@ static void __setup_APIC_LVTT(unsigned i
>        lvtt_value = LOCAL_TIMER_VECTOR;
>        if (!oneshot)
>                lvtt_value |= APIC_LVT_TIMER_PERIODIC;
> -       if (!lapic_is_integrated())
> +       if (!integrated_lapic)
>                lvtt_value |= SET_APIC_TIMER_BASE(APIC_TIMER_BASE_DIV);
>
>        if (!irqen)
> @@ -869,7 +874,7 @@ void clear_local_APIC(void)
>                apic_write(APIC_LVTPC, APIC_LVT_MASKED);
>
>        /* Integrated APIC (!82489DX) ? */
> -       if (lapic_is_integrated()) {
> +       if (integrated_lapic) {
>                if (maxlvt > 3)
>                        /* Clear ESR due to Pentium errata 3AP and 11AP */
>                        apic_write(APIC_ESR, 0);
> @@ -1064,7 +1069,7 @@ void __init init_bsp_APIC(void)
>         */
>        apic_write(APIC_LVT0, APIC_DM_EXTINT);
>        value = APIC_DM_NMI;
> -       if (!lapic_is_integrated())             /* 82489DX */
> +       if (!integrated_lapic)          /* 82489DX */
>                value |= APIC_LVT_LEVEL_TRIGGER;
>        apic_write(APIC_LVT1, value);
>  }
> @@ -1073,7 +1078,7 @@ static void __cpuinit lapic_setup_esr(vo
>  {
>        unsigned int oldvalue, value, maxlvt;
>
> -       if (!lapic_is_integrated()) {
> +       if (!integrated_lapic) {
>                pr_info("No ESR for 82489DX.\n");
>                return;
>        }
> @@ -1126,7 +1131,7 @@ void __cpuinit setup_local_APIC(void)
>
>  #ifdef CONFIG_X86_32
>        /* Pound the ESR really hard over the head with a big hammer - mbligh */
> -       if (lapic_is_integrated() && apic->disable_esr) {
> +       if (integrated_lapic && apic->disable_esr) {
>                apic_write(APIC_ESR, 0);
>                apic_write(APIC_ESR, 0);
>                apic_write(APIC_ESR, 0);
> @@ -1251,7 +1256,7 @@ void __cpuinit setup_local_APIC(void)
>                value = APIC_DM_NMI;
>        else
>                value = APIC_DM_NMI | APIC_LVT_MASKED;
> -       if (!lapic_is_integrated())             /* 82489DX */
> +       if (!integrated_lapic)          /* 82489DX */
>                value |= APIC_LVT_LEVEL_TRIGGER;
>        apic_write(APIC_LVT1, value);
>
> @@ -1599,6 +1604,8 @@ int __init APIC_init_uniprocessor(void)
>                clear_cpu_cap(&boot_cpu_data, X86_FEATURE_APIC);
>                return -1;
>        }
> +       /* Determine APIC is integrated or not. */
> +       lapic_is_integrated();
>  #endif
>
>        enable_IR_x2apic();
> --

do you need to call lapic_is_integrated() for 32bit smp?

YH

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] v2: x86,APIC: Detect lapic_is_integrated() once - use on  and on
  2009-05-26  2:11 ` Yinghai Lu
@ 2009-05-27  2:49   ` Rakib Mullick
  2009-06-10 11:24     ` Rakib Mullick
  0 siblings, 1 reply; 6+ messages in thread
From: Rakib Mullick @ 2009-05-27  2:49 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, macro,
	Cyrill Gorcunov, Andrew Morton, LKML

On 5/26/09, Yinghai Lu <yhlu.kernel@gmail.com> wrote:

> do you need to call lapic_is_integrated() for 32bit smp?

 Actually, external APIC were used in older version of intel
processors ( probably some P4 and it's earlier version - if i'm not
wrong). So I don't think it's  required. And current implementation
doesn't separate the case of smp or non-smp - it just deals with 32 or
64 bit processors. Is there anything that i'm missing ?

Thanks,
>
> YH
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] v2: x86,APIC: Detect lapic_is_integrated() once - use on  and on
  2009-05-27  2:49   ` Rakib Mullick
@ 2009-06-10 11:24     ` Rakib Mullick
  2009-06-10 11:34       ` Thomas Gleixner
  0 siblings, 1 reply; 6+ messages in thread
From: Rakib Mullick @ 2009-06-10 11:24 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner
  Cc: yhlu.kernel, H. Peter Anvin, macro, Cyrill Gorcunov, Andrew Morton, LKML

Hi Ingo, Thomas and everyone, would you please take a look at this
patch and comment ? I think Thomas was agreed with this one. Is there
anything that i couldn't deliver? If i'm wrong please notice thus i
can fix myself.

Thanks,
Rakib

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] v2: x86,APIC: Detect lapic_is_integrated() once - use on and on
  2009-06-10 11:24     ` Rakib Mullick
@ 2009-06-10 11:34       ` Thomas Gleixner
  2009-06-14 13:16         ` Rakib Mullick
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Gleixner @ 2009-06-10 11:34 UTC (permalink / raw)
  To: Rakib Mullick
  Cc: Ingo Molnar, yhlu.kernel, H. Peter Anvin, macro, Cyrill Gorcunov,
	Andrew Morton, LKML

On Wed, 10 Jun 2009, Rakib Mullick wrote:

> Hi Ingo, Thomas and everyone, would you please take a look at this
> patch and comment ? I think Thomas was agreed with this one. Is there
> anything that i couldn't deliver? If i'm wrong please notice thus i
> can fix myself.

The patch is fine, but it lacks a proper patch description.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] v2: x86,APIC: Detect lapic_is_integrated() once - use on  and on
  2009-06-10 11:34       ` Thomas Gleixner
@ 2009-06-14 13:16         ` Rakib Mullick
  0 siblings, 0 replies; 6+ messages in thread
From: Rakib Mullick @ 2009-06-14 13:16 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Ingo Molnar, yhlu.kernel, H. Peter Anvin, macro, Cyrill Gorcunov,
	Andrew Morton, LKML

On Wed, Jun 10, 2009 at 5:34 PM, Thomas Gleixner<tglx@linutronix.de> wrote:
> On Wed, 10 Jun 2009, Rakib Mullick wrote:
>
>> Hi Ingo, Thomas and everyone, would you please take a look at this
>> patch and comment ? I think Thomas was agreed with this one. Is there
>> anything that i couldn't deliver? If i'm wrong please notice thus i
>> can fix myself.
>
> The patch is fine, but it lacks a proper patch description.
>
> Thanks,
>
Thanks Thomas. I understatnd my problem. I'm resending the patch with
proper patch description.

Thanks
>        tglx
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-06-14 13:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-05-26  1:20 [PATCH] v2: x86,APIC: Detect lapic_is_integrated() once - use on and on Rakib Mullick
2009-05-26  2:11 ` Yinghai Lu
2009-05-27  2:49   ` Rakib Mullick
2009-06-10 11:24     ` Rakib Mullick
2009-06-10 11:34       ` Thomas Gleixner
2009-06-14 13:16         ` Rakib Mullick

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.