Re: [Qemu-devel] [PATCH v3 16/16] [RFC] dma/rc4030: do multiple calls to address_space_rw when doing DMA transfers

[Peter Maydell <peter.maydell@linaro.org>]

On 10 June 2015 at 20:37, Hervé Poussineau <hpoussin@reactos.org> wrote:
> Hi Leon,
>
>
> Le 10/06/2015 14:00, Leon Alrae a écrit :
>>
>> Hi Hervé,
>>
>> On 03/06/2015 21:45, Hervé Poussineau wrote:
>>>
>>> This fixes Windows NT 4.0/MIPS, which was always bugchecking with
>>> IRQL_NOT_LESS_OR_EQUAL.
>>>
>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>> ---
>>>   hw/dma/rc4030.c | 14 ++++++++++++--
>>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
>>> index 3efa6de..deac0a8 100644
>>> --- a/hw/dma/rc4030.c
>>> +++ b/hw/dma/rc4030.c
>>> @@ -681,6 +681,7 @@ static void rc4030_do_dma(void *opaque, int n,
>>> uint8_t *buf, int len, int is_wri
>>>       rc4030State *s = opaque;
>>>       hwaddr dma_addr;
>>>       int dev_to_mem;
>>> +    int i;
>>>
>>>       s->dma_regs[n][DMA_REG_ENABLE] &= ~(DMA_FLAG_TC_INTR |
>>> DMA_FLAG_MEM_INTR | DMA_FLAG_ADDR_INTR);
>>>
>>> @@ -699,8 +700,17 @@ static void rc4030_do_dma(void *opaque, int n,
>>> uint8_t *buf, int len, int is_wri
>>>       dma_addr = s->dma_regs[n][DMA_REG_ADDRESS];
>>>
>>>       /* Read/write data at right place */
>>> -    address_space_rw(&s->dma_as, dma_addr, MEMTXATTRS_UNSPECIFIED,
>>> -                     buf, len, is_write);
>>> +    for (i = 0; i < len; ) {
>>> +        int ncpy = DMA_PAGESIZE - (dma_addr & (DMA_PAGESIZE - 1));
>>> +        if (ncpy > len - i) {
>>> +            ncpy = len - i;
>>> +        }
>>> +        address_space_rw(&s->dma_as, dma_addr, MEMTXATTRS_UNSPECIFIED,
>>> +                         buf + i, ncpy, is_write);
>>> +
>>> +        dma_addr += ncpy;
>>> +        i += ncpy;
>>> +    }
>>>
>>>       s->dma_regs[n][DMA_REG_ENABLE] |= DMA_FLAG_TC_INTR;
>>>       s->dma_regs[n][DMA_REG_COUNT] -= len;
>>>
>>
>> I'm sending out current target-mips queue soon and I can include this
>> series,
>> but I'm not quite sure what to do with this RFC patch. I presume you put
>> RFC
>> here because it's a workaround for a bug, but it's not clear where the
>> actual
>> bug is?
>
>
> Indeed, that's a workaround for a bug that I failed to identify. You can
> still take the RFC patch as a patch, as I didn't see any objection (yet).
> I think we can defer the bug correction for later.

At a minimum, we need a comment saying that we're doing this weird
thing as a workaround (and ideally with enough info in the commit
message to reproduce so that at some later date we can confirm whether
we've really fixed the underlying issue and can revert this).

-- PMM