[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3] dp8393x: don't force 32-bit register access
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v3] dp8393x: don't force 32-bit register access |
Date: |
Mon, 5 Jul 2021 23:36:23 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 |
On 7/5/21 9:33 PM, Mark Cave-Ayland wrote:
> On 05/07/2021 08:52, Mark Cave-Ayland wrote:
>
>> I think the problem is because of the interaction of
>> .impl.max_access_size = 2 and the it_shift property specifying a
>> stride of 4 bytes: when the 4 byte access is split into 2 x 2 byte
>> accesses then for a read reg = addr >> s->it_shift causes the second
>> access to be a duplicate of the first rather than containing zeros.
I believe this is something I tried to fix earlier, see:
"hw/misc: Add support for interleaved memory accesses"
https://www.mail-archive.com/qemu-devel@nongnu.org/msg730721.html
(in particular:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg730725.html)
and
"hw/misc: Add memory_region_add_subregion_aliased() helper"
https://www.mail-archive.com/qemu-block@nongnu.org/msg83742.html
I plan to respin during next dev cycle...
> After some more experiments I'm fairly confident this is the issue: if
> you rely on applying it_shift within the MMIO access itself then you
> lose the zero extension of the result to the access size as done by the
> memory API.
>
> I'll come up with a new version which I shall send as part of an updated
> and tested v2 of Phil's housekeeping patchset, since the endian changes
> were really helpful when studying the descriptors.
I'm fine if we take the patch Finn has tested (I'll amend a comment
explaining the limitations) and we fix the details in the next cycle.