Re: [PATCH RFC 1/9] x86/traps: Names for system descriptor types

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

On 15/05/14 10:48, Andrew Cooper wrote:
> Avoids some particularly obscure magic numbers
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>
> ---

Something I missed from the --- section.

I was toying with the naming scheme SYS_DESC_xxx instead as perhaps
being slightly more informative in context.

~Andrew

>  xen/arch/x86/crash.c        |    2 +-
>  xen/arch/x86/traps.c        |    8 ++++----
>  xen/arch/x86/x86_64/traps.c |    4 ++--
>  xen/include/asm-x86/desc.h  |    8 ++++++++
>  xen/include/asm-x86/ldt.h   |    2 +-
>  5 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/x86/crash.c b/xen/arch/x86/crash.c
> index aed3b3e..97adf02 100644
> --- a/xen/arch/x86/crash.c
> +++ b/xen/arch/x86/crash.c
> @@ -149,7 +149,7 @@ static void nmi_shootdown_cpus(void)
>               * This update is safe from a security point of view, as this pcpu 
>               * is never going to try to sysret back to a PV vcpu.
>               */
> -            _set_gate_lower(&idt_tables[i][TRAP_nmi], 14, 0, &trap_nop);
> +            _set_gate_lower(&idt_tables[i][TRAP_nmi], DESC_TYPE_irq_gate, 0, &trap_nop);
>              set_ist(&idt_tables[i][TRAP_machine_check], IST_NONE);
>          }
>          else
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index 40366f1..1de70af 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -3431,8 +3431,8 @@ static void __set_intr_gate(unsigned int n, uint32_t dpl, void *addr)
>      /* Keep secondary tables in sync with IRQ updates. */
>      for ( i = 1; i < nr_cpu_ids; i++ )
>          if ( idt_tables[i] != NULL )
> -            _set_gate(&idt_tables[i][n], 14, dpl, addr);
> -    _set_gate(&idt_table[n], 14, dpl, addr);
> +            _set_gate(&idt_tables[i][n], DESC_TYPE_irq_gate, dpl, addr);
> +    _set_gate(&idt_table[n], DESC_TYPE_irq_gate, dpl, addr);
>  }
>  
>  static void set_swint_gate(unsigned int n, void *addr)
> @@ -3457,12 +3457,12 @@ void load_TR(void)
>          this_cpu(gdt_table) + TSS_ENTRY - FIRST_RESERVED_GDT_ENTRY,
>          (unsigned long)tss,
>          offsetof(struct tss_struct, __cacheline_filler) - 1,
> -        9);
> +        DESC_TYPE_tss_avail);
>      _set_tssldt_desc(
>          this_cpu(compat_gdt_table) + TSS_ENTRY - FIRST_RESERVED_GDT_ENTRY,
>          (unsigned long)tss,
>          offsetof(struct tss_struct, __cacheline_filler) - 1,
> -        11);
> +        DESC_TYPE_tss_busy);
>  
>      /* Switch to non-compat GDT (which has B bit clear) to execute LTR. */
>      asm volatile (
> diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
> index 3a48478..cdaf1e6 100644
> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -394,10 +394,10 @@ void __devinit subarch_percpu_traps_init(void)
>           * The 32-on-64 hypercall entry vector is only accessible from ring 1.
>           * Also note that this is a trap gate, not an interrupt gate.
>           */
> -        _set_gate(idt_table+HYPERCALL_VECTOR, 15, 1, &compat_hypercall);
> +        _set_gate(idt_table+HYPERCALL_VECTOR, DESC_TYPE_trap_gate, 1, &compat_hypercall);
>  
>          /* Fast trap for int80 (faster than taking the #GP-fixup path). */
> -        _set_gate(idt_table+0x80, 15, 3, &int80_direct_trap);
> +        _set_gate(idt_table+0x80, DESC_TYPE_trap_gate, 3, &int80_direct_trap);
>      }
>  
>      stack_bottom = (char *)get_stack_bottom();
> diff --git a/xen/include/asm-x86/desc.h b/xen/include/asm-x86/desc.h
> index 4edb834..6b2094f 100644
> --- a/xen/include/asm-x86/desc.h
> +++ b/xen/include/asm-x86/desc.h
> @@ -98,6 +98,14 @@
>  
>  #ifndef __ASSEMBLY__
>  
> +/* System Descriptor types in the GDT */
> +#define DESC_TYPE_ldt       (2)
> +#define DESC_TYPE_tss_avail (9)
> +#define DESC_TYPE_tss_busy  (11)
> +#define DESC_TYPE_call_gate (12)
> +#define DESC_TYPE_irq_gate  (14)
> +#define DESC_TYPE_trap_gate (15)
> +
>  struct desc_struct {
>      u32 a, b;
>  };
> diff --git a/xen/include/asm-x86/ldt.h b/xen/include/asm-x86/ldt.h
> index b6f7beb..4ba6d32 100644
> --- a/xen/include/asm-x86/ldt.h
> +++ b/xen/include/asm-x86/ldt.h
> @@ -18,7 +18,7 @@ static inline void load_LDT(struct vcpu *v)
>          desc = (!is_pv_32on64_vcpu(v)
>                  ? this_cpu(gdt_table) : this_cpu(compat_gdt_table))
>                 + LDT_ENTRY - FIRST_RESERVED_GDT_ENTRY;
> -        _set_tssldt_desc(desc, LDT_VIRT_START(v), ents*8-1, 2);
> +        _set_tssldt_desc(desc, LDT_VIRT_START(v), ents*8-1, DESC_TYPE_ldt);
>          __asm__ __volatile__ ( "lldt %%ax" : : "a" (LDT_ENTRY << 3) );
>      }
>  }