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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.11.16 at 17:24, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 24/11/16 14:31, Jan Beulich wrote:
>>>>> On 23.11.16 at 16:38, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> +#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.

Well, let's keep it that way then for now, but I think I will be tempted
to get rid of those zeros down the road.

> 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.

Right, I had tried to think of something like that too when
working out how that mkec() could be made work, but
couldn't think of anything. After all, us using the ellipsis is
pretty much heading into the opposite direction (and that part
of why I think dropping the zeros too would be a good idea,
because that will make more visible when we actually care
about a _specific_ error code).

>> 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?

Yes, it's basically all your work, so I think R-b is more appropriate.

Jan


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