Re: [Xen-devel] [PATCH] x86/emul: Adjustments to exception error code handling

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.02.18 at 11:59, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -877,14 +877,12 @@ do {                                                    
> \
>      if ( rc ) goto done;                                \
>  } while (0)
>  
> -static inline int mkec(uint8_t e, int32_t ec, ...)
> -{
> -    return (e < 32 && ((1u << e) & EXC_HAS_EC)) ? ec : X86_EVENT_NO_EC;
> -}
> +/* CPP magic.  Chooses ec if not empty, otherwise X86_EVENT_NO_EC. */
> +#define mkec(ignore, x, ...) x
>  
>  #define generate_exception_if(p, e, ec...)                                \
>  ({  if ( (p) ) {                                                          \
> -        x86_emul_hw_exception(e, mkec(e, ##ec, 0), ctxt);                 \
> +        x86_emul_hw_exception(e, mkec(X, ##ec, X86_EVENT_NO_EC), ctxt);   \
>          rc = X86EMUL_EXCEPTION;                                           \
>          goto done;                                                        \
>      }                                                                     \

This orphans EXC_HAS_EC, which makes me wonder what assertion
you're talking about in the description. The way things are before
your change means that at least an exception with error code will
be delivered properly (the error code will be zero then) if it wasn't
specified in the invocation (which, as you may recall, I actually
consider useful, but you did object to making this an "officially"
allowed mechanism). With your change in place, an assertion will
supposedly trigger (wherever that is), killing the host or (in a
release build) leading to some other behavior that's likely fatal to
a guest. Would the guest perhaps get to see an error code of all
ones? If, otoh, we could know at build time that something is wrong,
I would be quite a bit more in agreement with doing such a change,
most importantly because those exception raising paths are rarely
hit, and are mostly (if not entirely) untested by the test harness.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel