Message ID | 20240219082938.238302-1-npiggin@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
From: Peter Maydell <peter.maydell@linaro.org> 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 <clg@redhat.com> Tested-by: Cédric Le Goater <clg@redhat.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
On Mon, 19 Feb 2024, Nicholas Piggin wrote: > From: Peter Maydell <peter.maydell@linaro.org> > > 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 <clg@redhat.com> > Tested-by: Cédric Le Goater <clg@redhat.com> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Hm, this seems to be missing the actual patch. Regards, BALATON Zoltan
On 2/19/24 15:49, BALATON Zoltan wrote: > On Mon, 19 Feb 2024, Nicholas Piggin wrote: >> From: Peter Maydell <peter.maydell@linaro.org> >> >> 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 <clg@redhat.com> >> Tested-by: Cédric Le Goater <clg@redhat.com> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > Hm, this seems to be missing the actual patch. It's merged already and git knows how to handle this. Thanks, C.
On Mon, 19 Feb 2024 at 14:53, Cédric Le Goater <clg@kaod.org> wrote: > > On 2/19/24 15:49, BALATON Zoltan wrote: > > On Mon, 19 Feb 2024, Nicholas Piggin wrote: > >> From: Peter Maydell <peter.maydell@linaro.org> > >> > >> 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 <clg@redhat.com> > >> Tested-by: Cédric Le Goater <clg@redhat.com> > >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > > > 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
On 2/19/24 15:55, Peter Maydell wrote: > On Mon, 19 Feb 2024 at 14:53, Cédric Le Goater <clg@kaod.org> wrote: >> >> On 2/19/24 15:49, BALATON Zoltan wrote: >>> On Mon, 19 Feb 2024, Nicholas Piggin wrote: >>>> From: Peter Maydell <peter.maydell@linaro.org> >>>> >>>> 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 <clg@redhat.com> >>>> Tested-by: Cédric Le Goater <clg@redhat.com> >>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >>> >>> 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.
On Mon, 19 Feb 2024 at 08:31, Nicholas Piggin <npiggin@gmail.com> wrote: > > The following changes since commit da96ad4a6a2ef26c83b15fa95e7fceef5147269c: > > Merge tag 'hw-misc-20240215' of https://github.com/philmd/qemu into staging (2024-02-16 11:05:14 +0000) > > are available in the Git repository at: > > https://gitlab.com/npiggin/qemu.git tags/pull-ppc-for-9.0-20240219 > > for you to fetch changes up to 922e408e12315121d3e09304b8b8f462ea051af1: > > target/ppc: optimise ppcemb_tlb_t flushing (2024-02-19 18:09:19 +1000) > > ---------------------------------------------------------------- > * Avocado tests for ppc64 to boot FreeBSD, run guests with emulated > or nested hypervisor facilities, among other things. > * Update ppc64 CPU defaults to Power10. > * Add a new powernv10-rainier machine to better capture differences > between the different Power10 systems. > * Implement more device models for powernv. > * 4xx TLB flushing performance and correctness improvements. > * Correct gdb implementation to access some important SPRs. > * Misc cleanups and bug fixes. > > I dropped the BHRB patches, they are very close but minor issue only > noticed recently held them up. Hopefully we can get those and a bunch > of other outstanding submissions in for 9.0 but this PR was taking too > long as it was. > Peter Maydell (1): > hw/pci-host/raven.c: Mark raven_io_ops as implementing unaligned accesses Hi Nick -- this commit went upstream via a different route, and so it now appears in this pullrequest as a commit with a commit message but no contents. Could I ask you to respin the pullreq with that commit dropped, please? thanks -- PMM
On Tue Feb 20, 2024 at 3:06 AM AEST, Peter Maydell wrote: > On Mon, 19 Feb 2024 at 08:31, Nicholas Piggin <npiggin@gmail.com> wrote: > > > > The following changes since commit da96ad4a6a2ef26c83b15fa95e7fceef5147269c: > > > > Merge tag 'hw-misc-20240215' of https://github.com/philmd/qemu into staging (2024-02-16 11:05:14 +0000) > > > > are available in the Git repository at: > > > > https://gitlab.com/npiggin/qemu.git tags/pull-ppc-for-9.0-20240219 > > > > for you to fetch changes up to 922e408e12315121d3e09304b8b8f462ea051af1: > > > > target/ppc: optimise ppcemb_tlb_t flushing (2024-02-19 18:09:19 +1000) > > > > ---------------------------------------------------------------- > > * Avocado tests for ppc64 to boot FreeBSD, run guests with emulated > > or nested hypervisor facilities, among other things. > > * Update ppc64 CPU defaults to Power10. > > * Add a new powernv10-rainier machine to better capture differences > > between the different Power10 systems. > > * Implement more device models for powernv. > > * 4xx TLB flushing performance and correctness improvements. > > * Correct gdb implementation to access some important SPRs. > > * Misc cleanups and bug fixes. > > > > I dropped the BHRB patches, they are very close but minor issue only > > noticed recently held them up. Hopefully we can get those and a bunch > > of other outstanding submissions in for 9.0 but this PR was taking too > > long as it was. > > > Peter Maydell (1): > > hw/pci-host/raven.c: Mark raven_io_ops as implementing unaligned accesses > > Hi Nick -- this commit went upstream via a different route, and > so it now appears in this pullrequest as a commit with a commit > message but no contents. Could I ask you to respin the pullreq > with that commit dropped, please? Yeah, sorry about that :( I think I noticed it when rebasing but did not check that I'd fixed it. It's nice to keep gunk out of the upstream so I agree, I will respin it. Thanks, Nick