Re: [PULL 22/49] hw/pci-host/raven.c: Mark raven_io_ops as implementing unaligned accesses
On 2/19/24 15:55, Peter Maydell wrote: On Mon, 19 Feb 2024 at 14:53, Cédric Le Goater wrote: On 2/19/24 15:49, BALATON Zoltan wrote: On Mon, 19 Feb 2024, Nicholas Piggin wrote: From: Peter Maydell 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") Reviewed-by: Cédric Le Goater Tested-by: Cédric Le Goater Signed-off-by: Peter Maydell Signed-off-by: Nicholas Piggin Hm, this seems to be missing the actual patch. It's merged already and git knows how to handle this. Mmm, though this is the result of "rebased onto a tree that already had the commit" rather than "two merges both contain the commit", so we end up with a genuinely empty commit upstream, which is a bit odd looking, though harmless. git rebase -i db5f7f9e3ceb and dropping the first patch would cleanup the empty patch. C.
Re: [PULL 22/49] hw/pci-host/raven.c: Mark raven_io_ops as implementing unaligned accesses
On Mon, 19 Feb 2024 at 14:53, Cédric Le Goater wrote: > > On 2/19/24 15:49, BALATON Zoltan wrote: > > On Mon, 19 Feb 2024, Nicholas Piggin wrote: > >> From: Peter Maydell > >> > >> 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") > >> Reviewed-by: Cédric Le Goater > >> Tested-by: Cédric Le Goater > >> Signed-off-by: Peter Maydell > >> Signed-off-by: Nicholas Piggin > > > > Hm, this seems to be missing the actual patch. > > It's merged already and git knows how to handle this. Mmm, though this is the result of "rebased onto a tree that already had the commit" rather than "two merges both contain the commit", so we end up with a genuinely empty commit upstream, which is a bit odd looking, though harmless. -- PMM
Re: [PULL 22/49] hw/pci-host/raven.c: Mark raven_io_ops as implementing unaligned accesses
On 2/19/24 15:49, BALATON Zoltan wrote: On Mon, 19 Feb 2024, Nicholas Piggin wrote: From: Peter Maydell 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") Reviewed-by: Cédric Le Goater Tested-by: Cédric Le Goater Signed-off-by: Peter Maydell Signed-off-by: Nicholas Piggin Hm, this seems to be missing the actual patch. It's merged already and git knows how to handle this. Thanks, C.
Re: [PULL 22/49] hw/pci-host/raven.c: Mark raven_io_ops as implementing unaligned accesses
On Mon, 19 Feb 2024, Nicholas Piggin wrote: From: Peter Maydell 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") Reviewed-by: Cédric Le Goater Tested-by: Cédric Le Goater Signed-off-by: Peter Maydell Signed-off-by: Nicholas Piggin Hm, this seems to be missing the actual patch. Regards, BALATON Zoltan
[PULL 22/49] hw/pci-host/raven.c: Mark raven_io_ops as implementing unaligned accesses
From: Peter Maydell 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") Reviewed-by: Cédric Le Goater Tested-by: Cédric Le Goater Signed-off-by: Peter Maydell Signed-off-by: Nicholas Piggin -- 2.42.0