* Mike Travis <tra...@sgi.com> wrote:

> This patch allows enabling or disabling of the "EXTRA APIC ID BITS"
> workaround found only on UV100/UV1000 systems.  This workaround was
> required by the Intel Nahelm Processor (used in that architecture)
> because it could only support a limited number of CPU threads within
> the SSI.  This was due to only having 8 APIC ID bits thus a workaround
> was added to add and remove extra bits to the APIC ID.  It also required
> a specialized UV1 only apic driver.
> 
> Signed-off-by: Mike Travis <tra...@sgi.com>
> Tested-by: Nathan Zimmer <nzim...@sgi.com>
> ---
>  arch/x86/Kconfig                   |   14 ++++++++++++++
>  arch/x86/include/asm/uv/uv_hub.h   |   11 ++++-------
>  arch/x86/include/asm/uv/uv_mmrs.h  |    2 ++
>  arch/x86/kernel/apic/x2apic_uv_x.c |   26 +++++++++++++++++++++++---
>  4 files changed, 43 insertions(+), 10 deletions(-)
> 
> --- linux.orig/arch/x86/Kconfig
> +++ linux/arch/x86/Kconfig
> @@ -489,6 +489,20 @@ config X86_UV
>         This option is needed in order to support SGI Ultraviolet systems.
>         If you don't have one of these, you should say N here.
>  
> +config X86_UV1_SUPPORTED
> +     bool "SGI Ultraviolet Series 1 Supported"
> +     depends on X86_UV
> +     default n
> +     ---help---
> +       Set this option if you have a UV100/UV1000 system.  By setting
> +       this option extra execution time and space is introduced for
> +       workarounds designed for processors limited to only 8 apicid bits.
> +       This limited the number of processors that could be supported in
> +       an SSI.  With the Intel release of the SandyBridge Processor (used
> +       in UV2000 systems), the "x2apic" mode was introduced to extend
> +       the number of apicid bits.  Thus more processors are supported
> +       without these workarounds and the specialized UV1 only apic driver.
> +
>  # Following is an alphabetically sorted list of 32 bit extended platforms
>  # Please maintain the alphabetic order if and when there are additions
>  
> --- linux.orig/arch/x86/include/asm/uv/uv_hub.h
> +++ linux/arch/x86/include/asm/uv/uv_hub.h
> @@ -289,25 +289,21 @@ union uvh_apicid {
>  #define UV4_GLOBAL_MMR32_SIZE                (16UL * 1024 * 1024)
>  
>  #define UV_LOCAL_MMR_BASE            (                               \
> -                                     is_uv1_hub() ? UV1_LOCAL_MMR_BASE : \
>                                       is_uv2_hub() ? UV2_LOCAL_MMR_BASE : \
>                                       is_uv3_hub() ? UV3_LOCAL_MMR_BASE : \
>                                       /*is_uv4_hub*/ UV4_LOCAL_MMR_BASE)
>  
>  #define UV_GLOBAL_MMR32_BASE         (                               \
> -                                     is_uv1_hub() ? UV1_GLOBAL_MMR32_BASE : \
>                                       is_uv2_hub() ? UV2_GLOBAL_MMR32_BASE : \
>                                       is_uv3_hub() ? UV3_GLOBAL_MMR32_BASE : \
>                                       /*is_uv4_hub*/ UV4_GLOBAL_MMR32_BASE)
>  
>  #define UV_LOCAL_MMR_SIZE            (                               \
> -                                     is_uv1_hub() ? UV1_LOCAL_MMR_SIZE : \
>                                       is_uv2_hub() ? UV2_LOCAL_MMR_SIZE : \
>                                       is_uv3_hub() ? UV3_LOCAL_MMR_SIZE : \
>                                       /*is_uv4_hub*/ UV4_LOCAL_MMR_SIZE)
>  
>  #define UV_GLOBAL_MMR32_SIZE         (                               \
> -                                     is_uv1_hub() ? UV1_GLOBAL_MMR32_SIZE : \
>                                       is_uv2_hub() ? UV2_GLOBAL_MMR32_SIZE : \
>                                       is_uv3_hub() ? UV3_GLOBAL_MMR32_SIZE : \
>                                       /*is_uv4_hub*/ UV4_GLOBAL_MMR32_SIZE)

Hm, are you sure this can be removed?


> @@ -445,9 +441,6 @@ static inline int uv_apicid_to_pnode(int
>   */
>  static inline int uv_apicid_to_socket(int apicid)
>  {
> -     if (is_uv1_hub())
> -             return (apicid >> (uv_hub_info->apic_pnode_shift - 1)) & 1;
> -     else
>               return 0;
>  }
>  
> @@ -704,7 +697,11 @@ static inline void uv_set_cpu_scir_bits(
>       }
>  }
>  
> +#ifdef       UV1_HUB_IS_SUPPORTED
>  extern unsigned int uv_apicid_hibits;
> +#else
> +#define      uv_apicid_hibits        0
> +#endif
>  static unsigned long uv_hub_ipi_value(int apicid, int vector, int mode)
>  {
>       apicid |= uv_apicid_hibits;
> --- linux.orig/arch/x86/include/asm/uv/uv_mmrs.h
> +++ linux/arch/x86/include/asm/uv/uv_mmrs.h
> @@ -95,7 +95,9 @@
>  #define UV4_HUB_PART_NUMBER  0x99a1
>  
>  /* Compat: Indicate which UV Hubs are supported. */
> +#ifdef       CONFIG_X86_UV1_SUPPORTED
>  #define UV1_HUB_IS_SUPPORTED 1
> +#endif
>  #define UV2_HUB_IS_SUPPORTED 1
>  #define UV3_HUB_IS_SUPPORTED 1
>  #define UV4_HUB_IS_SUPPORTED 1
> --- linux.orig/arch/x86/kernel/apic/x2apic_uv_x.c
> +++ linux/arch/x86/kernel/apic/x2apic_uv_x.c
> @@ -39,7 +39,9 @@
>  #include <asm/x86_init.h>
>  #include <asm/nmi.h>
>  
> +#ifdef       UV1_HUB_IS_SUPPORTED
>  DEFINE_PER_CPU(int, x2apic_extra_bits);
> +#endif
>  
>  #define PR_DEVEL(fmt, args...)       pr_devel("%s: " fmt, __func__, args)
>  
> @@ -50,8 +52,10 @@ static u64 gru_dist_lmask, gru_dist_umas
>  static union uvh_apicid uvh_apicid;
>  int uv_min_hub_revision_id;
>  EXPORT_SYMBOL_GPL(uv_min_hub_revision_id);
> +#ifdef       UV1_HUB_IS_SUPPORTED
>  unsigned int uv_apicid_hibits;
>  EXPORT_SYMBOL_GPL(uv_apicid_hibits);
> +#endif
>  
>  static struct apic apic_x2apic_uv_x;
>  
> @@ -145,6 +149,7 @@ static void __init early_get_apic_pnode_
>   * interrupts potentially passing through the UV HUB.  This prevents
>   * a deadlock between interrupts and IO port operations.
>   */
> +#ifdef       UV1_HUB_IS_SUPPORTED
>  static void __init uv_set_apicid_hibit(void)
>  {
>       union uv1h_lb_target_physical_apic_id_mask_u apicid_mask;
> @@ -156,6 +161,7 @@ static void __init uv_set_apicid_hibit(v
>                       apicid_mask.s1.bit_enables & UV_APICID_HIBIT_MASK;
>       }
>  }
> +#endif
>  
>  static int __init uv_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
>  {
> @@ -190,13 +196,14 @@ static int __init uv_acpi_madt_oem_check
>               uv_system_type = UV_X2APIC;
>               uv_apic = 0;
>  
> +#ifdef       UV1_HUB_IS_SUPPORTED
>       } else if (!strcmp(oem_table_id, "UVH")) {      /* only UV1 systems */
>               uv_system_type = UV_NON_UNIQUE_APIC;
>               __this_cpu_write(x2apic_extra_bits,
>                       pnodeid << uvh_apicid.s.pnode_shift);
>               uv_set_apicid_hibit();
>               uv_apic = 1;
> -
> +#endif
>       } else  if (!strcmp(oem_table_id, "UVL")) {     /* only used for */
>               uv_system_type = UV_LEGACY_APIC;        /* very small systems */
>               uv_apic = 0;
> @@ -252,7 +259,6 @@ static int uv_wakeup_secondary(int phys_
>       int pnode;
>  
>       pnode = uv_apicid_to_pnode(phys_apicid);
> -     phys_apicid |= uv_apicid_hibits;
>       val = (1UL << UVH_IPI_INT_SEND_SHFT) |
>           (phys_apicid << UVH_IPI_INT_APIC_ID_SHFT) |
>           ((start_rip << UVH_IPI_INT_VECTOR_SHFT) >> 12) |
> @@ -344,7 +350,7 @@ uv_cpu_mask_to_apicid_and(const struct c
>       }
>  
>       if (likely(cpu < nr_cpu_ids)) {
> -             *apicid = per_cpu(x86_cpu_to_apicid, cpu) | uv_apicid_hibits;
> +             *apicid = per_cpu(x86_cpu_to_apicid, cpu);
>               return 0;
>       }
>  
> @@ -353,21 +359,29 @@ uv_cpu_mask_to_apicid_and(const struct c
>  
>  static unsigned int x2apic_get_apic_id(unsigned long x)
>  {
> +#ifdef       UV1_HUB_IS_SUPPORTED
>       unsigned int id;
>  
>       WARN_ON(preemptible() && num_online_cpus() > 1);
>       id = x | __this_cpu_read(x2apic_extra_bits);
>  
>       return id;
> +#else
> +     return x;
> +#endif
>  }
>  
>  static unsigned long set_apic_id(unsigned int id)
>  {
> +#ifdef       UV1_HUB_IS_SUPPORTED
>       unsigned long x;
>  
>       /* maskout x2apic_extra_bits ? */
>       x = id;
>       return x;
> +#else
> +     return id;
> +#endif
>  }

Hm, this hunk does not appear to be doing anything.

>  
>  static unsigned int uv_read_apic_id(void)
> @@ -442,10 +456,16 @@ static struct apic __refdata apic_x2apic
>       .safe_wait_icr_idle             = native_safe_x2apic_wait_icr_idle,
>  };
>  
> +#ifdef       UV1_HUB_IS_SUPPORTED
>  static void set_x2apic_extra_bits(int pnode)
>  {
>       __this_cpu_write(x2apic_extra_bits, pnode << uvh_apicid.s.pnode_shift);
>  }
> +#else
> +static inline void set_x2apic_extra_bits(int pnode)
> +{
> +}
> +#endif

So Kconfig values and #ifdefs are not really user friendly nor developer 
friendly. 
Wouldn't it be possible to eliminate all the #ifdefs and simply use something 
like 
this:

  static void set_x2apic_extra_bits(int pnode)
  {
        if (unlikely(uv_apic))
                __this_cpu_write(x2apic_extra_bits, pnode << 
uvh_apicid.s.pnode_shift);
  }

etc. The unlikely() will make sure this never impacts any fast path. You can 
also 
mark 'uv_apic' with __read_mostly, to make sure it's nicely cached.

(You could also rename 'uv_apic' to 'legacy_uv1_apic' to make it really clear 
that 
this is only for older systems.)

Hm?

Thanks,

        Ingo

Reply via email to