On 7/29/19 5:32 PM, Paolo Bonzini wrote:
> On 29/07/19 21:45, John Snow wrote:
>> Next, we'll unschedule the BH if there is one. I think the only case
>> where there is one is the reschedule_dma case of dma_blk_cb. (I'm not
>> too familiar with these DMA helpers: in what cases do we expect the iov
>> to be empty?)
>
> When there is another I/O that is using the DMA bounce buffer (the one
> case that comes to mind in which you do DMA from MMIO areas is
> loading/saving VGA RAM).
>
>> So it looks like this cancellation will produce one of two effects,
>> depending on when it's invoked:
>>
>> 1) We'll stall the DMA permanently by deleting that BH, because
>> dma_complete will never get invoked and therefore nobody will ever call
>> ide_dma_cb with any return value of any kind. The IDE state machine
>> likely just hangs waiting for the DMA to finish until the guest OS
>> decides to reset the errant controller.
>>
>> 2) The DMA will continue blissfully unaware it was canceled, because the
>> lower AIOCB has no cancel method, and so will finish, call back to
>> dma_blk_cb, and continue the transfer loop unaware.
>>
>>
>> ... Does your reading align with mine?
>>
>>
>> If it does -- if there are indeed no places in the code today that
>> artificially inject -ECANCELED -- I need to remove these special stanzas
>> from the IDE code and allow the IDE state machine to handle these errors
>> as true errors.
>
> The bug is that there is no place to inject -ECANCELED in the dbs->bh
> case. I've sent an obviously^W untested patch.
>
Where does it inject -ECANCELED in the non-dbs->bh case?
--js
