From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] x86/LAPIC: drop support for non-integrated APIC Date: Wed, 24 Sep 2014 17:18:57 +0100 Message-ID: <5422EEF1.3030305@citrix.com> References: <542301E902000078000386E5@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5938684003730852550==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XWpHW-0006vT-RF for xen-devel@lists.xenproject.org; Wed, 24 Sep 2014 16:19:07 +0000 In-Reply-To: <542301E902000078000386E5@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , xen-devel Cc: Keir Fraser List-Id: xen-devel@lists.xenproject.org --===============5938684003730852550== Content-Type: multipart/alternative; boundary="------------030305060306050105010706" --------------030305060306050105010706 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: quoted-printable On 24/09/14 16:39, Jan Beulich wrote: > We never really supported such, even in the 32-bit days. > > As a minor extra thing move the APIC_SELF_IPI definition out of the > middle of Divider COnfiguration Register ones. Did you intend to capitalise CO ? > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper > > --- a/xen/arch/x86/apic.c > +++ b/xen/arch/x86/apic.c > @@ -145,13 +145,9 @@ int get_physical_broadcast(void) > =20 > int get_maxlvt(void) > { > - unsigned int v, ver, maxlvt; > + unsigned int v =3D apic_read(APIC_LVR); > =20 > - v =3D apic_read(APIC_LVR); > - ver =3D GET_APIC_VERSION(v); > - /* 82489DXs do not report # of LVT entries. */ > - maxlvt =3D APIC_INTEGRATED(ver) ? GET_APIC_MAXLVT(v) : 2; > - return maxlvt; > + return GET_APIC_MAXLVT(v); > } > =20 > void clear_local_APIC(void) > @@ -217,12 +213,9 @@ void clear_local_APIC(void) > if (maxlvt >=3D 6) > apic_write_around(APIC_CMCI, APIC_LVT_MASKED); > =20 > - v =3D GET_APIC_VERSION(apic_read(APIC_LVR)); > - if (APIC_INTEGRATED(v)) { /* !82489DX */ > - if (maxlvt > 3) /* Due to Pentium errata 3AP and 11AP. = */ > - apic_write(APIC_ESR, 0); > - apic_read(APIC_ESR); > - } > + if (maxlvt > 3) /* Due to Pentium errata 3AP and 11AP. */ > + apic_write(APIC_ESR, 0); > + apic_read(APIC_ESR); > } > =20 > void __init connect_bsp_APIC(void) > @@ -477,10 +470,7 @@ void __init init_bsp_APIC(void) > * Set up the virtual wire mode. > */ > apic_write_around(APIC_LVT0, APIC_DM_EXTINT); > - value =3D APIC_DM_NMI; > - if (!APIC_INTEGRATED(ver)) /* 82489DX */ > - value |=3D APIC_LVT_LEVEL_TRIGGER; > - apic_write_around(APIC_LVT1, value); > + apic_write_around(APIC_LVT1, APIC_DM_NMI); > } > =20 > static void apic_pm_activate(void) > @@ -555,7 +545,7 @@ void __devinit setup_local_APIC(void) > /* > * Double-check whether this APIC is really registered. > */ > - if (!apic_id_registered()) > + if (!APIC_INTEGRATED(ver) || !apic_id_registered()) > BUG(); > =20 > /* > @@ -671,11 +661,9 @@ void __devinit setup_local_APIC(void) > value =3D APIC_DM_NMI; > else > value =3D APIC_DM_NMI | APIC_LVT_MASKED; > - if (!APIC_INTEGRATED(ver)) /* 82489DX */ > - value |=3D APIC_LVT_LEVEL_TRIGGER; > apic_write_around(APIC_LVT1, value); > =20 > - if (APIC_INTEGRATED(ver) && !esr_disable) { /* !82489DX */ > + if (!esr_disable) { > maxlvt =3D get_maxlvt(); > if (maxlvt > 3) /* Due to the Pentium erratum 3AP. */ > apic_write(APIC_ESR, 0); > @@ -694,16 +682,13 @@ void __devinit setup_local_APIC(void) > "vector: %#lx after: %#lx\n", > oldvalue, value); > } else { > - if (esr_disable) =20 > - /*=20 > - * Something untraceble is creating bad interrupts on=20 > - * 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"); > - else > - printk("No ESR for 82489DX.\n"); > + /* > + * 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"); > } > =20 > if (nmi_watchdog =3D=3D NMI_LOCAL_APIC) > @@ -1096,13 +1081,10 @@ static void __init wait_8254_wraparound( > =20 > static void __setup_APIC_LVTT(unsigned int clocks) > { > - unsigned int lvtt_value, tmp_value, ver; > + unsigned int lvtt_value, tmp_value; > =20 > - ver =3D GET_APIC_VERSION(apic_read(APIC_LVR)); > /* NB. Xen uses local APIC timer in one-shot mode. */ > lvtt_value =3D /*APIC_TIMER_MODE_PERIODIC |*/ LOCAL_TIMER_VECTOR; > - if (!APIC_INTEGRATED(ver)) > - lvtt_value |=3D SET_APIC_TIMER_BASE(APIC_TIMER_BASE_DIV); > =20 > if ( tdt_enabled ) > { > --- a/xen/arch/x86/smpboot.c > +++ b/xen/arch/x86/smpboot.c > @@ -391,17 +391,17 @@ extern void *stack_start; > static int wakeup_secondary_cpu(int phys_apicid, unsigned long start_e= ip) > { > unsigned long send_status =3D 0, accept_status =3D 0; > - int maxlvt, timeout, num_starts, i; > + int maxlvt, timeout, i; > =20 > /* > * Be paranoid about clearing APIC errors. > */ > - if ( APIC_INTEGRATED(apic_version[phys_apicid]) ) > - { > - apic_read_around(APIC_SPIV); > - apic_write(APIC_ESR, 0); > - apic_read(APIC_ESR); > - } > + if ( !APIC_INTEGRATED(apic_version[phys_apicid]) ) > + return -ENODEV; > + > + apic_read_around(APIC_SPIV); > + apic_write(APIC_ESR, 0); > + apic_read(APIC_ESR); > =20 > Dprintk("Asserting INIT.\n"); > =20 > @@ -448,20 +448,9 @@ static int wakeup_secondary_cpu(int phys > udelay(10); > } > =20 > - /* > - * Should we send STARTUP IPIs ? > - * > - * Determine this based on the APIC version. > - * If we don't have an integrated APIC, don't send the STARTUP IPI= s. > - */ > - num_starts =3D APIC_INTEGRATED(apic_version[phys_apicid]) ? 2 : 0;= > - > - /* Run STARTUP IPI loop. */ > - Dprintk("#startup loops: %d.\n", num_starts); > - > maxlvt =3D get_maxlvt(); > =20 > - for ( i =3D 0; i < num_starts; i++ ) > + for ( i =3D 0; i < 2; i++ ) > { > Dprintk("Sending STARTUP #%d.\n", i+1); > apic_read_around(APIC_SPIV); > @@ -761,8 +750,8 @@ void __init smp_prepare_cpus(unsigned in > } > =20 > /* If we couldn't find a local APIC, then get out of here now! */ > - if ( APIC_INTEGRATED(apic_version[boot_cpu_physical_apicid]) > - && !cpu_has_apic ) > + if ( !APIC_INTEGRATED(apic_version[boot_cpu_physical_apicid]) > + || !cpu_has_apic ) > { > printk(KERN_ERR "BIOS bug, local APIC #%d not detected!...\n",= > boot_cpu_physical_apicid); > --- a/xen/include/asm-x86/apicdef.h > +++ b/xen/include/asm-x86/apicdef.h > @@ -84,12 +84,6 @@ > #define APIC_LVT0 0x350 > #define APIC_CMCI 0x2F0 > =20 > -#define APIC_LVT_TIMER_BASE_MASK (0x3<<18) > -#define GET_APIC_TIMER_BASE(x) (((x)>>18)&0x3) > -#define SET_APIC_TIMER_BASE(x) (((x)<<18)) > -#define APIC_TIMER_BASE_CLKIN 0x0 > -#define APIC_TIMER_BASE_TMBASE 0x1 > -#define APIC_TIMER_BASE_DIV 0x2 > #define APIC_TIMER_MODE_MASK (0x3<<17) > #define APIC_TIMER_MODE_ONESHOT (0x0<<17) > #define APIC_TIMER_MODE_PERIODIC (0x1<<17) > @@ -110,10 +104,6 @@ > #define APIC_TMICT 0x380 > #define APIC_TMCCT 0x390 > #define APIC_TDCR 0x3E0 > - > -/* Only available in x2APIC mode */ > -#define APIC_SELF_IPI 0x3F0 > - > #define APIC_TDR_DIV_TMBASE (1<<2) > #define APIC_TDR_DIV_1 0xB > #define APIC_TDR_DIV_2 0x0 > @@ -124,6 +114,9 @@ > #define APIC_TDR_DIV_64 0x9 > #define APIC_TDR_DIV_128 0xA > =20 > +/* Only available in x2APIC mode */ > +#define APIC_SELF_IPI 0x3F0 > + > #define APIC_BASE (fix_to_virt(FIX_APIC_BASE)) > =20 > /* It's only used in x2APIC mode of an x2APIC unit. */ > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------030305060306050105010706 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: quoted-printable
On 24/09/14 16:39, Jan Beulich wrote: 
We never really supported such, even in the 32-bit day=
s.

As a minor extra thing move the APIC_SELF_IPI definition out of the
middle of Divider COnfiguration Register ones.

Did you intend to capitalise CO ?


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

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>


--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -145,13 +145,9 @@ int get_physical_broadcast(void)
=20
 int get_maxlvt(void)
 {
-    unsigned int v, ver, maxlvt;
+    unsigned int v =3D apic_read(APIC_LVR);
=20
-    v =3D apic_read(APIC_LVR);
-    ver =3D GET_APIC_VERSION(v);
-    /* 82489DXs do not report # of LVT entries. */
-    maxlvt =3D APIC_INTEGRATED(ver) ? GET_APIC_MAXLVT(v) : 2;
-    return maxlvt;
+    return GET_APIC_MAXLVT(v);
 }
=20
 void clear_local_APIC(void)
@@ -217,12 +213,9 @@ void clear_local_APIC(void)
     if (maxlvt >=3D 6)
         apic_write_around(APIC_CMCI, APIC_LVT_MASKED);
=20
-    v =3D GET_APIC_VERSION(apic_read(APIC_LVR));
-    if (APIC_INTEGRATED(v)) {  /* !82489DX */
-        if (maxlvt > 3)        /* Due to Pentium errata 3AP and 11AP. *=
/
-            apic_write(APIC_ESR, 0);
-        apic_read(APIC_ESR);
-    }
+    if (maxlvt > 3)        /* Due to Pentium errata 3AP and 11AP. */
+        apic_write(APIC_ESR, 0);
+    apic_read(APIC_ESR);
 }
=20
 void __init connect_bsp_APIC(void)
@@ -477,10 +470,7 @@ void __init init_bsp_APIC(void)
      * Set up the virtual wire mode.
      */
     apic_write_around(APIC_LVT0, APIC_DM_EXTINT);
-    value =3D APIC_DM_NMI;
-    if (!APIC_INTEGRATED(ver))              /* 82489DX */
-        value |=3D APIC_LVT_LEVEL_TRIGGER;
-    apic_write_around(APIC_LVT1, value);
+    apic_write_around(APIC_LVT1, APIC_DM_NMI);
 }
=20
 static void apic_pm_activate(void)
@@ -555,7 +545,7 @@ void __devinit setup_local_APIC(void)
     /*
      * Double-check whether this APIC is really registered.
      */
-    if (!apic_id_registered())
+    if (!APIC_INTEGRATED(ver) || !apic_id_registered())
         BUG();
=20
     /*
@@ -671,11 +661,9 @@ void __devinit setup_local_APIC(void)
         value =3D APIC_DM_NMI;
     else
         value =3D APIC_DM_NMI | APIC_LVT_MASKED;
-    if (!APIC_INTEGRATED(ver))      /* 82489DX */
-        value |=3D APIC_LVT_LEVEL_TRIGGER;
     apic_write_around(APIC_LVT1, value);
=20
-    if (APIC_INTEGRATED(ver) && !esr_disable) {        /* !82489DX=
 */
+    if (!esr_disable) {
         maxlvt =3D get_maxlvt();
         if (maxlvt > 3)     /* Due to the Pentium erratum 3AP. */
             apic_write(APIC_ESR, 0);
@@ -694,16 +682,13 @@ void __devinit setup_local_APIC(void)
                         "vector: %#lx  after: %#lx\n",
                         oldvalue, value);
     } else {
-        if (esr_disable)   =20
-            /*=20
-             * Something untraceble is creating bad interrupts on=20
-             * 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");
-        else
-            printk("No ESR for 82489DX.\n");
+        /*
+         * 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");
     }
=20
     if (nmi_watchdog =3D=3D NMI_LOCAL_APIC)
@@ -1096,13 +1081,10 @@ static void __init wait_8254_wraparound(
=20
 static void __setup_APIC_LVTT(unsigned int clocks)
 {
-    unsigned int lvtt_value, tmp_value, ver;
+    unsigned int lvtt_value, tmp_value;
=20
-    ver =3D GET_APIC_VERSION(apic_read(APIC_LVR));
     /* NB. Xen uses local APIC timer in one-shot mode. */
     lvtt_value =3D /*APIC_TIMER_MODE_PERIODIC |*/ LOCAL_TIMER_VECTOR;
-    if (!APIC_INTEGRATED(ver))
-        lvtt_value |=3D SET_APIC_TIMER_BASE(APIC_TIMER_BASE_DIV);
=20
     if ( tdt_enabled )
     {
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -391,17 +391,17 @@ extern void *stack_start;
 static int wakeup_secondary_cpu(int phys_apicid, unsigned long start_eip)
 {
     unsigned long send_status =3D 0, accept_status =3D 0;
-    int maxlvt, timeout, num_starts, i;
+    int maxlvt, timeout, i;
=20
     /*
      * Be paranoid about clearing APIC errors.
      */
-    if ( APIC_INTEGRATED(apic_version[phys_apicid]) )
-    {
-        apic_read_around(APIC_SPIV);
-        apic_write(APIC_ESR, 0);
-        apic_read(APIC_ESR);
-    }
+    if ( !APIC_INTEGRATED(apic_version[phys_apicid]) )
+        return -ENODEV;
+
+    apic_read_around(APIC_SPIV);
+    apic_write(APIC_ESR, 0);
+    apic_read(APIC_ESR);
=20
     Dprintk("Asserting INIT.\n");
=20
@@ -448,20 +448,9 @@ static int wakeup_secondary_cpu(int phys
         udelay(10);
     }
=20
-    /*
-     * Should we send STARTUP IPIs ?
-     *
-     * Determine this based on the APIC version.
-     * If we don't have an integrated APIC, don't send the STARTUP IPIs.
-     */
-    num_starts =3D APIC_INTEGRATED(apic_version[phys_apicid]) ? 2 : 0;
-
-    /* Run STARTUP IPI loop. */
-    Dprintk("#startup loops: %d.\n", num_starts);
-
     maxlvt =3D get_maxlvt();
=20
-    for ( i =3D 0; i < num_starts; i++ )
+    for ( i =3D 0; i < 2; i++ )
     {
         Dprintk("Sending STARTUP #%d.\n", i+1);
         apic_read_around(APIC_SPIV);
@@ -761,8 +750,8 @@ void __init smp_prepare_cpus(unsigned in
     }
=20
     /* If we couldn't find a local APIC, then get out of here now! */
-    if ( APIC_INTEGRATED(apic_version[boot_cpu_physical_apicid])
-         && !cpu_has_apic )
+    if ( !APIC_INTEGRATED(apic_version[boot_cpu_physical_apicid])
+         || !cpu_has_apic )
     {
         printk(KERN_ERR "BIOS bug, local APIC #%d not detected!...\n",
                boot_cpu_physical_apicid);
--- a/xen/include/asm-x86/apicdef.h
+++ b/xen/include/asm-x86/apicdef.h
@@ -84,12 +84,6 @@
 #define		APIC_LVT0	0x350
 #define		APIC_CMCI	0x2F0
=20
-#define			APIC_LVT_TIMER_BASE_MASK	(0x3<<18)
-#define			GET_APIC_TIMER_BASE(x)		(((x)>>18)&0x3)
-#define			SET_APIC_TIMER_BASE(x)		(((x)<<18))
-#define			APIC_TIMER_BASE_CLKIN		0x0
-#define			APIC_TIMER_BASE_TMBASE		0x1
-#define			APIC_TIMER_BASE_DIV		0x2
 #define			APIC_TIMER_MODE_MASK		(0x3<<17)
 #define			APIC_TIMER_MODE_ONESHOT		(0x0<<17)
 #define			APIC_TIMER_MODE_PERIODIC	(0x1<<17)
@@ -110,10 +104,6 @@
 #define		APIC_TMICT	0x380
 #define		APIC_TMCCT	0x390
 #define		APIC_TDCR	0x3E0
-
-/* Only available in x2APIC mode */
-#define		APIC_SELF_IPI	0x3F0
-
 #define			APIC_TDR_DIV_TMBASE	(1<<2)
 #define			APIC_TDR_DIV_1		0xB
 #define			APIC_TDR_DIV_2		0x0
@@ -124,6 +114,9 @@
 #define			APIC_TDR_DIV_64		0x9
 #define			APIC_TDR_DIV_128	0xA
=20
+/* Only available in x2APIC mode */
+#define		APIC_SELF_IPI	0x3F0
+
 #define APIC_BASE (fix_to_virt(FIX_APIC_BASE))
=20
 /* It's only used in x2APIC mode of an x2APIC unit. */





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
=
http://lists.xen.org/xen-devel

--------------030305060306050105010706-- --===============5938684003730852550== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============5938684003730852550==--