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

[Mark Cave-Ayland]

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.