Re: [Xen-devel] [PATCH] x86/HVM: adjust hvm_interrupt_blocked()

["Roger Pau Monné" <roger.pau@citrix.com>]

On Fri, Oct 12, 2018 at 09:58:46AM -0600, Jan Beulich wrote:
> First of all, hvm_intsrc_mce was not considered here at all, yet nothing
> blocks #MC (other than an already in-progress #MC, but dealing with this
> is not the purpose of this patch).
> 
> Additionally STI-shadow only blocks maskable interrupts, but not NMI.

I've found the Table 25-3 on Intel SDM vol3 quite helpful:

"Execution of STI with RFLAGS.IF = 0 blocks maskable interrupts on the
instruction boundary following its execution.1 Setting this bit
indicates that this blocking is in effect."

And:

"Execution of a MOV to SS or a POP to SS blocks or suppresses certain
debug exceptions as well as interrupts (maskable and nonmaskable) on
the instruction boundary following its execution."

Might be worth adding to the commit message IMO.

> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3771,19 +3771,24 @@ enum hvm_intblk hvm_interrupt_blocked(st
>              return intr;
>      }
>  
> -    if ( (intack.source != hvm_intsrc_nmi) &&
> -         !(guest_cpu_user_regs()->eflags & X86_EFLAGS_IF) )
> -        return hvm_intblk_rflags_ie;
> +    if ( intack.source == hvm_intsrc_mce )
> +        return hvm_intblk_none;

I've been wondering, why do we handle #MC here, instead of doing the
same as for other Traps/Exceptions and use hvm_inject_hw_exception()
directly?

>  
>      intr_shadow = hvm_funcs.get_interrupt_shadow(v);
>  
> -    if ( intr_shadow & (HVM_INTR_SHADOW_STI|HVM_INTR_SHADOW_MOV_SS) )
> +    if ( intr_shadow & HVM_INTR_SHADOW_MOV_SS )
>          return hvm_intblk_shadow;
>  
>      if ( intack.source == hvm_intsrc_nmi )
>          return ((intr_shadow & HVM_INTR_SHADOW_NMI) ?
>                  hvm_intblk_nmi_iret : hvm_intblk_none);
>  
> +    if ( intr_shadow & HVM_INTR_SHADOW_STI )
> +        return hvm_intblk_shadow;
> +
> +    if ( !(guest_cpu_user_regs()->eflags & X86_EFLAGS_IF) )
> +        return hvm_intblk_rflags_ie;

I do wonder whether this code would be clearer using a `switch (
intack.source )` construct, but that's out of the scope.

Thanks, Roger.