Re: [Xen-devel] [PATCH] xen/pciback: Update data filter intersection logic.

[Andrey Grodzovsky <andrey2805@xxxxxxxxx>]

On Tue, Jun 21, 2016 at 5:23 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:

>>> On 21.06.16 at 11:04, <JBeulich@xxxxxxxx> wrote:
>>>> On 20.06.16 at 18:23, <andrey2805@xxxxxxxxx> wrote:
>> Here is printouts with applying the new logic
>>
>> [  382.222213] xen-pciback: 0000:06:00.0: write request 4 bytes at 0xc =
> 4000
>> [  382.222214] xen-pciback: 0000:06:00.0: read 1 bytes at 0xc
>> [  382.222228] xen-pciback: 0000:06:00.0: read 1 bytes at 0xc = 0
>> [  382.222238] xen-pciback: 0000:06:00.0: read 1 bytes at 0xd
>> [  382.222281] xen-pciback: 0000:06:00.0: read 1 bytes at 0xd = 0
>> [  382.222313] xen-pciback: 0000:06:00.0: read 1 bytes at 0xf
>> [  382.222341] xen-pciback: 0000:06:00.0: read 1 bytes at 0xf = 0
>>
>> So from prints the logic is not equivalent. 0xd is included in this case.
>>
>> In original logic field 0xd is excluded because
>> Not if ((req_start >= field_start && req_start < field_end)
>> Nor (req_end > field_start && req_end <= field_end))  evaluate to true.
>> Where request would be 4b segment starting with 0xc
>
> Oh, I see - the current _expression_ is screwed in two ways: For one
> it has req_* and field_* the wrong way round (or should I better
> say their uses are not symmetric, which for any kind of overlap
> check they of course should be), and then it uses || instead of &&
> (i.e. kind of, but not really checking that req is contained in field,
> whereas for the purpose here we'd need the other direction). So
> yes, your change is not just a simplification, but a correction.
>
> But then on top of the previously mentioned change to your patch
> you should also fix the read path in a similar manner. And the
> description should of course accurately reflect the change (i.e.
> no talk of quirks and overlapping filters, and a proper explanation
> of what's wrong with the current _expression_).

Sent updated patch for review.

 

Yet then what I don't understand: How does this change help you?
There's still not going to be any actual write to the Latency Timer
register, as conf_space_write() - even if now getting called - won't
do anything for it.

 

Jan

Not sure, seems to me sometime the reset sequence in the net driver doesn't 

actually erase the conf space so it masks the issue

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