qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC 0/2] macio: split out unaligned DMA access into se


From: John Snow
Subject: Re: [Qemu-devel] [RFC 0/2] macio: split out unaligned DMA access into separate functions
Date: Tue, 19 May 2015 17:01:42 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0


On 05/19/2015 04:50 PM, Mark Cave-Ayland wrote:
> On 18/05/15 20:54, John Snow wrote:
> 
>> On 03/09/2015 06:24 PM, Mark Cave-Ayland wrote:
>>> This patchset attempts to separate out the IDE/ATAPI logic from the 
>>> unaligned
>>> DMA access logic for macio which provides the following benefits:
>>>
>>> 1) Reduced code complexity
>>>
>>> The existing macio IDE/ATAPI functions were becoming extremely difficult to
>>> follow through the various callbacks. By splitting up the functions in this
>>> way it becomes much easier to follow the DMA-specific sections of code.
>>>
>>> 2) Future-proofing
>>>
>>> If/when the block layer becomes able to handle unaligned DMA accesses 
>>> directly
>>> then it should be possible to switch out pmac_dma_read() and 
>>> pmac_dma_write()
>>> with their unaligned-capable bdrv_*() equivalents without having to change 
>>> any
>>> other logic.
>>>
>>> 3) Fix intermittent CDROM detection under -M g3beige
>>>
>>> The code refactoring now correctly handles non-block ATAPI transfers which
>>> fixes the problem with intermittent CDROM detection with Darwin under
>>> -M g3beige.
>>>
>>> Signed-off-by: Mark Cave-Ayland <address@hidden>
>>>
>>>
>>> Mark Cave-Ayland (2):
>>>   macio: move unaligned DMA read code into separate pmac_dma_read()
>>>     function
>>>   macio: move unaligned DMA write code into separate pmac_dma_write()
>>>     function
>>>
>>>  hw/ide/macio.c             |  487 
>>> +++++++++++++++++++++++---------------------
>>>  include/hw/ppc/mac_dbdma.h |    4 -
>>>  2 files changed, 254 insertions(+), 237 deletions(-)
>>>
>>
>> I haven't been able to produce meaningful debug info for the problem
>> this patchset tries to help highlight (yet?), but it is an improvement
>> nonetheless, so given that it doesn't appear to make anything worse, and
>> this version is much nicer to follow, I'll stage this for the IDE branch.
>>
>> Acked-by: John Snow <address@hidden>
> 
> Thanks John. Even though you haven't managed to figure out the problem
> the patchset attempts to solve, were you at least able to reproduce the
> image corruption locally?
> 
> That said, the patchset is still worth including just for the fact that
> it fixes the flaky CDROM detection here.
> 
> 
> ATB,
> 
> Mark.
> 

Yeah, I reproduced the problem you're describing and spent a chunk of my
time debugging it and trying to figure out a section of the trace that
coincides with "the problem," but was unable to find anything of
particular interest.

I do notice that sometimes we appear to start a new transfer almost
immediately after one completes, but the code in place to sleep that
action until the guest finishes programming the DMA command seems to
catch it and nothing gets maliciously perturbed.

I still wonder somewhat that with the move to async and the strange
order of how darwin appears to program DMA transfers that we're hitting
some weird race, but I think that how reliably I hit the exact same
problem means that I should think again :)

I'll keep poking.

--js



reply via email to

[Prev in Thread] Current Thread [Next in Thread]