On 05/02/18 13:32, Jan Beulich wrote:
>>>> On 05.02.18 at 11:59, <andrew.coop...@citrix.com> 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.
{pv,hvm}_inject_event()
> 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).
It also meant that programming errors go completely unnoticed, which is
worse.
> 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?
In a release builds, it depends how vicious the vmentry checks are.
> 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.
I was originally aiming for a build time check, but the check_fpu_exn()
and protmode_load_seg() paths at least have non-constant exceptions.
We could force a constant exception by BUILD_BUG_ON(e >= 32), and
opencode the result of check_fpu_exn() (which is the only case which
can't be converted to a constant exception) to use
x86_emul_hw_exception() directly with suitable auditing.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
