Re: [Xen-devel] [PATCH v2] x86/HVM: fix forwarding of internally cached requests

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.03.16 at 09:28, <changjzh@xxxxxxxxx> wrote:
> 2016-03-29 18:39 GMT+08:00 Jan Beulich <JBeulich@xxxxxxxx>:
>> Forwarding entire batches to the device model when an individual
>> iteration of them got rejected by internal device emulation handlers
>> with X86EMUL_UNHANDLEABLE is wrong: The device model would then handle
>> all iterations, without the internal handler getting to see any past
>> the one it returned failure for. This causes misbehavior in at least
>> the MSI-X and VGA code, which want to see all such requests for
>> internal tracking/caching purposes. But note that this does not apply
>> to buffered I/O requests.
>>
>> This in turn means that the condition in hvm_process_io_intercept() of
>> when to crash the domain was wrong: Since X86EMUL_UNHANDLEABLE can
>> validly be returned by the individual device handlers, we mustn't
>> blindly crash the domain if such occurs on other than the initial
>> iteration. Instead we need to distinguish hvm_copy_*_guest_phys()
>> failures from device specific ones, and then the former need to always
>> be fatal to the domain (i.e. also on the first iteration), since
>> otherwise we again would end up forwarding a request to qemu which the
>> internal handler didn't get to see.
>>
>> The adjustment should be okay even for stdvga's MMIO handling:
>> - if it is not caching then the accept function would have failed so we
>>   won't get into hvm_process_io_intercept(),
>> - if it issued the buffered ioreq then we only get to the p->count
>>   reduction if hvm_send_ioreq() actually encountered an error (in which
>>   we don't care about the request getting split up).
>>
>> Also commit 4faffc41d ("x86/hvm: limit reps to avoid the need to handle
>> retry") went too far in removing code from hvm_process_io_intercept():
>> When there were successfully handled iterations, the function should
>> continue to return success with a clipped repeat count.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> Cc: Chang Jianzhong <changjzh@xxxxxxxxx>
>> ---
>> v2: Make hvm_process_io_intercept() always return a valid X86EMUL_*
>>     code by moving the domain_crash() invocations up. Add a stdvga
>>     related paragraph to the commit message.
>> ---
>> I assume this also addresses the issue which
>> http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg03189.html 
>> attempted to deal with in a not really acceptable way.
>>
>> I hope this issue is resolved, but it still exists.

Coming back to this: Have you made any progress with the analysis?

Jan


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