Re: [Xen-devel] [PATCH 05/15] x86/emul: Remove opencoded exception generation

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/11/16 14:31, Jan Beulich wrote:
>>>> On 23.11.16 at 16:38, <andrew.cooper3@xxxxxxxxxx> wrote:
>> +static inline int mkec(uint8_t e, int32_t ec, ...)
>> +{
>> +    return (e < 32 && (1u << e) & EXC_HAS_EC) ? ec : X86_EVENT_NO_EC;
> Please parenthesize the operands of &.

Fixed.

>
>> +}
>> +
>> +#define generate_exception_if(p, e, ec...)                                \
>>  ({  if ( (p) ) {                                                          \
>>          fail_if(ops->inject_hw_exception == NULL);                        \
>> -        rc = ops->inject_hw_exception(e, ec, ctxt) ? : X86EMUL_EXCEPTION; \
>> +        rc = ops->inject_hw_exception(e, mkec(e, ##ec, 0), ctxt)          \
> Did you notice that with the 0 used here, ...
>
>> @@ -1167,11 +1181,9 @@ static int ioport_access_check(
>>      if ( (rc = ops->read_segment(x86_seg_tr, &tr, ctxt)) != 0 )
>>          return rc;
>>  
>> -    /* Ensure that the TSS is valid and has an io-bitmap-offset field. */
>> -    if ( !tr.attr.fields.p ||
>> -         ((tr.attr.fields.type & 0xd) != 0x9) ||
>> -         (tr.limit < 0x67) )
>> -        goto raise_exception;
>> +    /* Ensure the TSS has an io-bitmap-offset field. */
>> +    generate_exception_if(tr.attr.fields.type != 0xb ||
>> +                          tr.limit < 0x67, EXC_GP, 0);
> ... invocations like this one don't really need their error code
> specified anymore either?

Yes, but I chose not to visually remove the error code from EXC_GP.

I attempted to get the compiler to force the presence or absence of the
error code parameter based on (e & EXC_HAS_EC) but failed to get
something working.  I doubt the C preprocessor is sufficiently
expressive for this use.

>
> With you having added my S-o-b (not really sure why), I'm not sure
> it makes a whole lot of sense to give my R-b as well (but feel free
> to add it).

Your code was originally the few bits replacing opencoded "goto
raise_exception" with generate_exception_if(), but the patch has morphed
a long way since then.  I am happy to exchange your S-o-B for R-b if you
would prefer?

~Andrew

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