Re: [PATCH] hw/pci-host/raven.c: Mark raven_io_ops as implementing unaligned accesses

["Cédric Le Goater" <clegoate@redhat.com>]

On 2/1/24 14:32, Peter Maydell wrote:
> On Tue, 23 Jan 2024 at 11:03, Cédric Le Goater <clegoate@redhat.com> wrote:
>>
>> On 1/12/24 14:46, Peter Maydell wrote:
>>> The raven_io_ops MemoryRegionOps is the only one in the source tree
>>> which sets .valid.unaligned to indicate that it should support
>>> unaligned accesses and which does not also set .impl.unaligned to
>>> indicate that its read and write functions can do the unaligned
>>> handling themselves.  This is a problem, because at the moment the
>>> core memory system does not implement the support for handling
>>> unaligned accesses by doing a series of aligned accesses and
>>> combining them (system/memory.c:access_with_adjusted_size() has a
>>> TODO comment noting this).
>>>
>>> Fortunately raven_io_read() and raven_io_write() will correctly deal
>>> with the case of being passed an unaligned address, so we can fix the
>>> missing unaligned access support by setting .impl.unaligned in the
>>> MemoryRegionOps struct.
>>>
>>> Fixes: 9a1839164c9c8f06 ("raven: Implement non-contiguous I/O region")
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>> Spotted by code inspection: I was looking for devices whose behaviour
>>> might be changed by a patch I'm reviewing that adds that missing
>>> support for unaligned accesses in the core memory system. But even
>>> if we do implement it there, it's more efficient for the raven MR
>>> to correctly mark it as handling unaligned accesses itself.
>>>
>>> Tested with 'make check' and 'make check-avocado' only.
>>
>> It doesn't affect the prep machine boot with OpenBIOS and a
>> "Debian GNU/Linux 3.0 6015" image.
>>
>> Tested-by: Cédric Le Goater <clg@redhat.com>
>> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> 
> Thanks for the review -- is this patch going to go via a
> ppc queue, or should I throw it in with my upcoming
> target-arm pullreq?

Please take it through target-arm. PPC is in low power state AFAICT.

Thanks,

C.