[PATCH for-6.0 0/2] arm: Make M-profile VTOR loads on reset handle memory aliasing

Peter Maydell posted 2 patches 3 years, 1 month ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210312172939.695-1-peter.maydell@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>, Stefan Hajnoczi <stefanha@redhat.com>, Bandan Das <bsd@redhat.com>, Thomas Huth <thuth@redhat.com>, Alexander Bulekov <alxndr@bu.edu>, Paolo Bonzini <pbonzini@redhat.com>, Laurent Vivier <lvivier@redhat.com>
include/exec/memory.h           |  4 +-
softmmu/memory.c                |  3 +-
target/arm/cpu.c                | 68 ++++++++++++++++++++++++++++++++-
tests/qtest/fuzz/generic_fuzz.c |  4 +-
4 files changed, 75 insertions(+), 4 deletions(-)
[PATCH for-6.0 0/2] arm: Make M-profile VTOR loads on reset handle memory aliasing
Posted by Peter Maydell 3 years, 1 month ago
For Arm M-profile CPUs, on reset the CPU must load its initial PC and
SP from a vector table in guest memory.  Because we can't guarantee
reset ordering, we have to handle the possibility that the ROM blob
loader's reset function has not yet run when the CPU resets, in which
case the data in an ELF file specified by the user won't be in guest
memory to be read yet.

We work around the reset ordering problem by checking whether the ROM
blob loader has any data for the address where the vector table is,
using rom_ptr().  Unfortunately this does not handle the possibility
of memory aliasing.  For many M-profile boards, memory can be accessed
via multiple possible physical addresses; if the board has the vector
table at address X but the user's ELF file loads data via a different
address Y which is an alias to the same underlying guest RAM then
rom_ptr() will not find it.

This series handles the possibility of aliasing by iterating through
the whole FlatView of the CPU's address space checking for other
mappings of the MemoryRegion corresponding to the location of the
vector table.  If we find any aliases we use rom_ptr() to see if the
ROM blob loader has any data there.

I still think that long-term the preferable option is going to be to
sort out our reset handling so that we can use three-phase-reset
everywhere and then have the rom blob loader write data in phase 2
which the CPU reset can read in phase 3. But that doesn't work today
because CPUs are not on any qbus and so they must be manually reset
outside the standard "system reset by resetting the qbus tree", using
qemu_register_reset(). The ROM blob loader also registers its reset
via qemu_register_reset(). Sadly qemu_register_reset() has no support
for three-phase-reset currently. That all adds up to "fixing reset
handling sufficiently for this approach to solve the problem is going
to take a long time", so for the moment this series is a reasonably
self-contained way to allow QEMU to run guest images linked to an
alias of the vector table address.

thanks
-- PMM

Peter Maydell (2):
  memory: Add offset_in_region to flatview_cb arguments
  target/arm: Make M-profile VTOR loads on reset handle memory aliasing

 include/exec/memory.h           |  4 +-
 softmmu/memory.c                |  3 +-
 target/arm/cpu.c                | 68 ++++++++++++++++++++++++++++++++-
 tests/qtest/fuzz/generic_fuzz.c |  4 +-
 4 files changed, 75 insertions(+), 4 deletions(-)

-- 
2.20.1


Re: [PATCH for-6.0 0/2] arm: Make M-profile VTOR loads on reset handle memory aliasing
Posted by Peter Maydell 3 years, 1 month ago
On Fri, 12 Mar 2021 at 17:29, Peter Maydell <peter.maydell@linaro.org> wrote:
> This series handles the possibility of aliasing by iterating through
> the whole FlatView of the CPU's address space checking for other
> mappings of the MemoryRegion corresponding to the location of the
> vector table.  If we find any aliases we use rom_ptr() to see if the
> ROM blob loader has any data there.

The other possible place we could put this code would be
to put it into rom_ptr() itself. You'd have to change the
callsites to pass an AddressSpace to rom_ptr(), but really
we ought to do that anyway, because a Rom has an AddressSpace
that we should be checking as well as the address.

-- PMM

Re: [PATCH for-6.0 0/2] arm: Make M-profile VTOR loads on reset handle memory aliasing
Posted by Richard Henderson 3 years, 1 month ago
On 3/12/21 12:59 PM, Peter Maydell wrote:
> On Fri, 12 Mar 2021 at 17:29, Peter Maydell <peter.maydell@linaro.org> wrote:
>> This series handles the possibility of aliasing by iterating through
>> the whole FlatView of the CPU's address space checking for other
>> mappings of the MemoryRegion corresponding to the location of the
>> vector table.  If we find any aliases we use rom_ptr() to see if the
>> ROM blob loader has any data there.
> 
> The other possible place we could put this code would be
> to put it into rom_ptr() itself. You'd have to change the
> callsites to pass an AddressSpace to rom_ptr(), but really
> we ought to do that anyway, because a Rom has an AddressSpace
> that we should be checking as well as the address.

I like this as the solution.


r~

Re: [PATCH for-6.0 0/2] arm: Make M-profile VTOR loads on reset handle memory aliasing
Posted by Peter Maydell 3 years, 1 month ago
On Sat, 13 Mar 2021 at 19:05, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 3/12/21 12:59 PM, Peter Maydell wrote:
> > On Fri, 12 Mar 2021 at 17:29, Peter Maydell <peter.maydell@linaro.org> wrote:
> >> This series handles the possibility of aliasing by iterating through
> >> the whole FlatView of the CPU's address space checking for other
> >> mappings of the MemoryRegion corresponding to the location of the
> >> vector table.  If we find any aliases we use rom_ptr() to see if the
> >> ROM blob loader has any data there.
> >
> > The other possible place we could put this code would be
> > to put it into rom_ptr() itself. You'd have to change the
> > callsites to pass an AddressSpace to rom_ptr(), but really
> > we ought to do that anyway, because a Rom has an AddressSpace
> > that we should be checking as well as the address.
>
> I like this as the solution.

I realized that checking against the Rom's AddressSpace
is likely to be a bad plan, though -- in some cases the AS is
used to allow the loader to say "load this image to AS
such-and-such", and we don't want to fail to find the ROM
blob because the rom_ptr() code is reading it from a different
AS that is an alternate view onto the same actual MemoryRegions
(eg Secure vs NonSecure and the RAM is in the same place.)

thanks
-- PMM