Re: [PATCH] x86/LAPIC: drop support for non-integrated APIC

[Andrew Cooper <andrew.cooper3@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 7698 bytes --]

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 <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)
>  
>  int get_maxlvt(void)
>  {
> -    unsigned int v, ver, maxlvt;
> +    unsigned int v = apic_read(APIC_LVR);
>  
> -    v = apic_read(APIC_LVR);
> -    ver = GET_APIC_VERSION(v);
> -    /* 82489DXs do not report # of LVT entries. */
> -    maxlvt = APIC_INTEGRATED(ver) ? GET_APIC_MAXLVT(v) : 2;
> -    return maxlvt;
> +    return GET_APIC_MAXLVT(v);
>  }
>  
>  void clear_local_APIC(void)
> @@ -217,12 +213,9 @@ void clear_local_APIC(void)
>      if (maxlvt >= 6)
>          apic_write_around(APIC_CMCI, APIC_LVT_MASKED);
>  
> -    v = 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);
>  }
>  
>  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 = APIC_DM_NMI;
> -    if (!APIC_INTEGRATED(ver))              /* 82489DX */
> -        value |= APIC_LVT_LEVEL_TRIGGER;
> -    apic_write_around(APIC_LVT1, value);
> +    apic_write_around(APIC_LVT1, APIC_DM_NMI);
>  }
>  
>  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();
>  
>      /*
> @@ -671,11 +661,9 @@ void __devinit setup_local_APIC(void)
>          value = APIC_DM_NMI;
>      else
>          value = APIC_DM_NMI | APIC_LVT_MASKED;
> -    if (!APIC_INTEGRATED(ver))      /* 82489DX */
> -        value |= APIC_LVT_LEVEL_TRIGGER;
>      apic_write_around(APIC_LVT1, value);
>  
> -    if (APIC_INTEGRATED(ver) && !esr_disable) {        /* !82489DX */
> +    if (!esr_disable) {
>          maxlvt = 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)    
> -            /* 
> -             * 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");
> -        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");
>      }
>  
>      if (nmi_watchdog == NMI_LOCAL_APIC)
> @@ -1096,13 +1081,10 @@ static void __init wait_8254_wraparound(
>  
>  static void __setup_APIC_LVTT(unsigned int clocks)
>  {
> -    unsigned int lvtt_value, tmp_value, ver;
> +    unsigned int lvtt_value, tmp_value;
>  
> -    ver = GET_APIC_VERSION(apic_read(APIC_LVR));
>      /* NB. Xen uses local APIC timer in one-shot mode. */
>      lvtt_value = /*APIC_TIMER_MODE_PERIODIC |*/ LOCAL_TIMER_VECTOR;
> -    if (!APIC_INTEGRATED(ver))
> -        lvtt_value |= SET_APIC_TIMER_BASE(APIC_TIMER_BASE_DIV);
>  
>      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 = 0, accept_status = 0;
> -    int maxlvt, timeout, num_starts, i;
> +    int maxlvt, timeout, i;
>  
>      /*
>       * 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);
>  
>      Dprintk("Asserting INIT.\n");
>  
> @@ -448,20 +448,9 @@ static int wakeup_secondary_cpu(int phys
>          udelay(10);
>      }
>  
> -    /*
> -     * 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 = APIC_INTEGRATED(apic_version[phys_apicid]) ? 2 : 0;
> -
> -    /* Run STARTUP IPI loop. */
> -    Dprintk("#startup loops: %d.\n", num_starts);
> -
>      maxlvt = get_maxlvt();
>  
> -    for ( i = 0; i < num_starts; i++ )
> +    for ( i = 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
>      }
>  
>      /* 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
>  
> -#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
>  
> +/* Only available in x2APIC mode */
> +#define		APIC_SELF_IPI	0x3F0
> +
>  #define APIC_BASE (fix_to_virt(FIX_APIC_BASE))
>  
>  /* 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


[-- Attachment #1.2: Type: text/html, Size: 9539 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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