Re: [Xen-devel] [PATCH v9 08/13] Add IOREQ_TYPE_VMWARE_PORT

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.02.15 at 20:52, <dslutz@xxxxxxxxxxx> wrote:
> On 02/26/15 03:07, Jan Beulich wrote:
>>>>> On 25.02.15 at 21:20, <dslutz@xxxxxxxxxxx> wrote:
>>> On 02/24/15 10:34, Jan Beulich wrote:
>>>>>>> On 17.02.15 at 00:05, <dslutz@xxxxxxxxxxx> wrote:
>>>>> @@ -2474,7 +2594,12 @@ struct hvm_ioreq_server 
>>>>> *hvm_select_ioreq_server(struct domain *d,
>>>>>          BUILD_BUG_ON(IOREQ_TYPE_PIO != HVMOP_IO_RANGE_PORT);
>>>>>          BUILD_BUG_ON(IOREQ_TYPE_COPY != HVMOP_IO_RANGE_MEMORY);
>>>>>          BUILD_BUG_ON(IOREQ_TYPE_PCI_CONFIG != HVMOP_IO_RANGE_PCI);
>>>>> +        BUILD_BUG_ON(IOREQ_TYPE_VMWARE_PORT != 
>>>>> HVMOP_IO_RANGE_VMWARE_PORT);
>>>>> +        BUILD_BUG_ON(IOREQ_TYPE_TIMEOFFSET != HVMOP_IO_RANGE_TIMEOFFSET);
>>>>> +        BUILD_BUG_ON(IOREQ_TYPE_INVALIDATE != HVMOP_IO_RANGE_INVALIDATE);
>>>>>          r = s->range[type];
>>>>> +        if ( !r )
>>>>> +            continue;
>>>>
>>>> Why, all of the sudden?
>>>>
>>>
>>> This is the replacement for the deleted "if" above.  Continue will lead
>>> to the same return that was remove above (it is at the end).  They are
>>> currently the same because all ioreq servers have the same set of
>>> ranges.  But if it would help, I can change "continue" into the "return
>>> default".
>> 
>> So further down you talk of the "special range 1" (see there for
>> further remarks in this regard) - how would r be NULL here in the
>> first place?
> 
> Since there is a hole in the #defines 0,1,2,7,8 (currently) range[6] is
> where r will be NULL for example.  However no current code should be
> able to get here.  So if you want me to I can drop the "if".

That's where ASSERT() comes in handy.

>> I understand all this is non-trivial and not necessarily obvious. But
>> as said before - the x86 instruction emulator should please remain
>> something acting along _only_ architectural specifications. Any
>> extensions to support things like what you want to do here should
>> be added using neutral hooks, responsible for keeping state they
>> need on their own.
>> 
> 
> 
> How does (the incorrectly formatted for a smaller diff):

Quite a bit better imo!

Jan


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