Re: [Xen-devel] [PATCH 1 of 2 V3] x86/IST: Create set_ist() helper function

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.12.12 at 22:42, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> ... to save using open-coded bitwise operations, and update all IST
> manipulation sites to use the function.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
> --
> 
> I am not overly happy with the name set_ist(), and certainly not tied to
> it.  However, I am unable to think of a better name. set_idt_ist() is
> wrong, as is set_irq_ist(), while set_idt_entry_ist() just seems to
> cludgy.  The comment and parameter types do explicitly state what is
> expected t be passed, but suggestions welcome for a better name.
> 
> diff -r bc624b00d6d6 -r 43f86afe90be xen/arch/x86/hvm/svm/svm.c
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -869,9 +869,9 @@ static void svm_ctxt_switch_from(struct 
>      svm_vmload(per_cpu(root_vmcb, cpu));
>  
>      /* Resume use of ISTs now that the host TR is reinstated. */
> -    idt_tables[cpu][TRAP_double_fault].a  |= IST_DF << 32;
> -    idt_tables[cpu][TRAP_nmi].a           |= IST_NMI << 32;
> -    idt_tables[cpu][TRAP_machine_check].a |= IST_MCE << 32;
> +    set_ist(&idt_tables[cpu][TRAP_double_fault],  IST_DF);
> +    set_ist(&idt_tables[cpu][TRAP_nmi],           IST_NMI);
> +    set_ist(&idt_tables[cpu][TRAP_machine_check], IST_MCE);
>  }
>  
>  static void svm_ctxt_switch_to(struct vcpu *v)
> @@ -893,9 +893,9 @@ static void svm_ctxt_switch_to(struct vc
>       * Cannot use ISTs for NMI/#MC/#DF while we are running with the guest 
> TR.
>       * But this doesn't matter: the IST is only req'd to handle 
> SYSCALL/SYSRET.
>       */
> -    idt_tables[cpu][TRAP_double_fault].a  &= ~(7UL << 32);
> -    idt_tables[cpu][TRAP_nmi].a           &= ~(7UL << 32);
> -    idt_tables[cpu][TRAP_machine_check].a &= ~(7UL << 32);
> +    set_ist(&idt_tables[cpu][TRAP_double_fault],  IST_NONE);
> +    set_ist(&idt_tables[cpu][TRAP_nmi],           IST_NONE);
> +    set_ist(&idt_tables[cpu][TRAP_machine_check], IST_NONE);
>  
>      svm_restore_dr(v);
>  
> diff -r bc624b00d6d6 -r 43f86afe90be xen/arch/x86/x86_64/traps.c
> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -370,9 +370,9 @@ void __devinit subarch_percpu_traps_init
>      {
>          /* Specify dedicated interrupt stacks for NMI, #DF, and #MC. */
>          set_intr_gate(TRAP_double_fault, &double_fault);
> -        idt_table[TRAP_double_fault].a  |= IST_DF << 32;
> -        idt_table[TRAP_nmi].a           |= IST_NMI << 32;
> -        idt_table[TRAP_machine_check].a |= IST_MCE << 32;
> +        set_ist(&idt_table[TRAP_double_fault],  IST_DF);
> +        set_ist(&idt_table[TRAP_nmi],           IST_NMI);
> +        set_ist(&idt_table[TRAP_machine_check], IST_MCE);
>  
>          /*
>           * The 32-on-64 hypercall entry vector is only accessible from ring 
> 1.
> diff -r bc624b00d6d6 -r 43f86afe90be xen/include/asm-x86/processor.h
> --- a/xen/include/asm-x86/processor.h
> +++ b/xen/include/asm-x86/processor.h
> @@ -425,10 +425,20 @@ struct tss_struct {
>      u8 __cacheline_filler[24];
>  } __cacheline_aligned __attribute__((packed));
>  
> -#define IST_DF  1UL
> -#define IST_NMI 2UL
> -#define IST_MCE 3UL
> -#define IST_MAX 3UL
> +#define IST_NONE 0UL
> +#define IST_DF   1UL
> +#define IST_NMI  2UL
> +#define IST_MCE  3UL
> +#define IST_MAX  3UL
> +
> +/* Set the interrupt stack table used by a particular interrupt
> + * descriptor table entry. */
> +static always_inline void set_ist(idt_entry_t * idt, unsigned long ist)
> +{
> +    /* ist is a 3 bit field, 32 bits into the idt entry. */
> +    ASSERT( ist < 8 );

This ought to check against IST_MAX.

> +    idt->a = ( idt->a & ~(7UL << 32) ) | ( (ist & 7UL) << 32 );

And with the check above, the right most & is pretty pointless.

Jan

> +}
>  
>  #define IDT_ENTRIES 256
>  extern idt_entry_t idt_table[];
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx 
> http://lists.xen.org/xen-devel 




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