This patchset converts the SPARC target away from the old
broken do_unassigned_access hook to the new (added in 2017...)
do_transaction_failed hook. In the process it fixes a number
of bugs in corner cases.

The SPARC ld/st-with-ASI helper functions are odd in that they
make use of the cpu_unassigned_access() function as a way of
raising an MMU fault. We start by making them just directly
call a new sparc_raise_mmu_fault() function so they don't go
through the hook function. This in-passing fixes a bug where
the hook was using GETPC(), which won't work inside a function
several levels deep in the callstack from a helper function.

The next four patches convert places that were using ld*_phys()
and st*_phys() and thus getting "implicitly causes an exception
via do_unassigned_access if it gets a bus error" behaviour.
We make them all use address_space_ld*/st* functions instead,
and explicitly handle the transaction-failure case. Variously:
 * for MMU passthrough, this doesn't change behaviour
 * for the MXCC stream source/destination ASI accesses,
   this doesn't change behaviour, but the current behaviour
   looks a bit weird, so a TODO comment is left in case anybody
   wants to chase up the actual right behaviour in future
 * for page table walks this fixes a bug where we would take
   an exception instead of reporting the page table walk failure
   with the correct fault status register information
 * for the page table walk in mmu_probe() this fixes a bug where
   we would take an exception when we are supposed to just report
   failure. Note that the spec says that MMU probe operations
   are supposed to set the fault status registers, but we do not;
   again I leave this pre-existing bug as a TODO note.
Next, we remove one entirely pointless and unused ldl_phys()
call from dump_mmu().

Finally, the last patch can do the very small amount of work to
change over to the new hook function. This will cause all the
"handle error" code paths in the preceding patches to become
active (when a do_unassigned_access hook is present the load
or store functions will never return an error, because the hook
will get called and throw an exception first).

I have tested that I can boot a sparc32 debian image, and
that sparc64 boots up to the firmware prompt, but this could
certainly use more extensive testing than I have given it.

(After SPARC, the only remaining unassigned-access-hook users
are RISCV, which crept in while I wasn't looking, and MIPS,
which is doing something complicated with the Jazz board that
I haven't yet investigated.)

thanks
--PMM

Peter Maydell (7):
  target/sparc: Factor out the body of sparc_cpu_unassigned_access()
  target/sparc: Check for transaction failures in MMU passthrough ASIs
  target/sparc: Check for transaction failures in MXCC stream ASI
    accesses
  target/sparc: Correctly handle bus errors in page table walks
  target/sparc: Handle bus errors in mmu_probe()
  target/sparc: Remove unused ldl_phys from dump_mmu()
  target/sparc: Switch to do_transaction_failed() hook

 target/sparc/cpu.h         |   8 +-
 target/sparc/cpu.c         |   2 +-
 target/sparc/ldst_helper.c | 319 +++++++++++++++++++++----------------
 target/sparc/mmu_helper.c  |  57 +++++--
 4 files changed, 238 insertions(+), 148 deletions(-)

-- 
2.20.1

Mark -- ping? Richard has reviewed this series; do you want
more time to test it, or should I just apply it to master
if you don't have any other pending sparc patches?

thanks
-- PMM

On Thu, 1 Aug 2019 at 19:30, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This patchset converts the SPARC target away from the old
> broken do_unassigned_access hook to the new (added in 2017...)
> do_transaction_failed hook. In the process it fixes a number
> of bugs in corner cases.
>
> The SPARC ld/st-with-ASI helper functions are odd in that they
> make use of the cpu_unassigned_access() function as a way of
> raising an MMU fault. We start by making them just directly
> call a new sparc_raise_mmu_fault() function so they don't go
> through the hook function. This in-passing fixes a bug where
> the hook was using GETPC(), which won't work inside a function
> several levels deep in the callstack from a helper function.
>
> The next four patches convert places that were using ld*_phys()
> and st*_phys() and thus getting "implicitly causes an exception
> via do_unassigned_access if it gets a bus error" behaviour.
> We make them all use address_space_ld*/st* functions instead,
> and explicitly handle the transaction-failure case. Variously:
>  * for MMU passthrough, this doesn't change behaviour
>  * for the MXCC stream source/destination ASI accesses,
>    this doesn't change behaviour, but the current behaviour
>    looks a bit weird, so a TODO comment is left in case anybody
>    wants to chase up the actual right behaviour in future
>  * for page table walks this fixes a bug where we would take
>    an exception instead of reporting the page table walk failure
>    with the correct fault status register information
>  * for the page table walk in mmu_probe() this fixes a bug where
>    we would take an exception when we are supposed to just report
>    failure. Note that the spec says that MMU probe operations
>    are supposed to set the fault status registers, but we do not;
>    again I leave this pre-existing bug as a TODO note.
> Next, we remove one entirely pointless and unused ldl_phys()
> call from dump_mmu().
>
> Finally, the last patch can do the very small amount of work to
> change over to the new hook function. This will cause all the
> "handle error" code paths in the preceding patches to become
> active (when a do_unassigned_access hook is present the load
> or store functions will never return an error, because the hook
> will get called and throw an exception first).
>
> I have tested that I can boot a sparc32 debian image, and
> that sparc64 boots up to the firmware prompt, but this could
> certainly use more extensive testing than I have given it.
>
> (After SPARC, the only remaining unassigned-access-hook users
> are RISCV, which crept in while I wasn't looking, and MIPS,
> which is doing something complicated with the Jazz board that
> I haven't yet investigated.)
>
> thanks
> --PMM
>
> Peter Maydell (7):
>   target/sparc: Factor out the body of sparc_cpu_unassigned_access()
>   target/sparc: Check for transaction failures in MMU passthrough ASIs
>   target/sparc: Check for transaction failures in MXCC stream ASI
>     accesses
>   target/sparc: Correctly handle bus errors in page table walks
>   target/sparc: Handle bus errors in mmu_probe()
>   target/sparc: Remove unused ldl_phys from dump_mmu()
>   target/sparc: Switch to do_transaction_failed() hook
>
>  target/sparc/cpu.h         |   8 +-
>  target/sparc/cpu.c         |   2 +-
>  target/sparc/ldst_helper.c | 319 +++++++++++++++++++++----------------
>  target/sparc/mmu_helper.c  |  57 +++++--
>  4 files changed, 238 insertions(+), 148 deletions(-)
>

On 03/09/2019 14:15, Peter Maydell wrote:

> Mark -- ping? Richard has reviewed this series; do you want
> more time to test it, or should I just apply it to master
> if you don't have any other pending sparc patches?

Yes, sorry - it has been on my TODO list for a while to look at this, but I've been
quite short of time these past few weeks. I should be able to run it through my SPARC
test images before the end of the week if that helps?


ATB,

Mark.

On Tue, 3 Sep 2019 at 18:00, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> On 03/09/2019 14:15, Peter Maydell wrote:
>
> > Mark -- ping? Richard has reviewed this series; do you want
> > more time to test it, or should I just apply it to master
> > if you don't have any other pending sparc patches?
>
> Yes, sorry - it has been on my TODO list for a while to look at this, but I've been
> quite short of time these past few weeks. I should be able to run it through my SPARC
> test images before the end of the week if that helps?

Yeah, that would be great. It's not hugely urgent, but I would
like to get this refactoring complete some time this
release cycle so we can drop the old hooks.

thanks
-- PMM

On 01/08/2019 19:30, Peter Maydell wrote:

> This patchset converts the SPARC target away from the old
> broken do_unassigned_access hook to the new (added in 2017...)
> do_transaction_failed hook. In the process it fixes a number
> of bugs in corner cases.
> 
> The SPARC ld/st-with-ASI helper functions are odd in that they
> make use of the cpu_unassigned_access() function as a way of
> raising an MMU fault. We start by making them just directly
> call a new sparc_raise_mmu_fault() function so they don't go
> through the hook function. This in-passing fixes a bug where
> the hook was using GETPC(), which won't work inside a function
> several levels deep in the callstack from a helper function.
> 
> The next four patches convert places that were using ld*_phys()
> and st*_phys() and thus getting "implicitly causes an exception
> via do_unassigned_access if it gets a bus error" behaviour.
> We make them all use address_space_ld*/st* functions instead,
> and explicitly handle the transaction-failure case. Variously:
>  * for MMU passthrough, this doesn't change behaviour
>  * for the MXCC stream source/destination ASI accesses,
>    this doesn't change behaviour, but the current behaviour
>    looks a bit weird, so a TODO comment is left in case anybody
>    wants to chase up the actual right behaviour in future
>  * for page table walks this fixes a bug where we would take
>    an exception instead of reporting the page table walk failure
>    with the correct fault status register information
>  * for the page table walk in mmu_probe() this fixes a bug where
>    we would take an exception when we are supposed to just report
>    failure. Note that the spec says that MMU probe operations
>    are supposed to set the fault status registers, but we do not;
>    again I leave this pre-existing bug as a TODO note.
> Next, we remove one entirely pointless and unused ldl_phys()
> call from dump_mmu().
> 
> Finally, the last patch can do the very small amount of work to
> change over to the new hook function. This will cause all the
> "handle error" code paths in the preceding patches to become
> active (when a do_unassigned_access hook is present the load
> or store functions will never return an error, because the hook
> will get called and throw an exception first).
> 
> I have tested that I can boot a sparc32 debian image, and
> that sparc64 boots up to the firmware prompt, but this could
> certainly use more extensive testing than I have given it.
> 
> (After SPARC, the only remaining unassigned-access-hook users
> are RISCV, which crept in while I wasn't looking, and MIPS,
> which is doing something complicated with the Jazz board that
> I haven't yet investigated.)
> 
> thanks
> --PMM
> 
> Peter Maydell (7):
>   target/sparc: Factor out the body of sparc_cpu_unassigned_access()
>   target/sparc: Check for transaction failures in MMU passthrough ASIs
>   target/sparc: Check for transaction failures in MXCC stream ASI
>     accesses
>   target/sparc: Correctly handle bus errors in page table walks
>   target/sparc: Handle bus errors in mmu_probe()
>   target/sparc: Remove unused ldl_phys from dump_mmu()
>   target/sparc: Switch to do_transaction_failed() hook
> 
>  target/sparc/cpu.h         |   8 +-
>  target/sparc/cpu.c         |   2 +-
>  target/sparc/ldst_helper.c | 319 +++++++++++++++++++++----------------
>  target/sparc/mmu_helper.c  |  57 +++++--
>  4 files changed, 238 insertions(+), 148 deletions(-)

I've just run this through my SPARC test images with a fairly recent git master and I
don't see any obvious regressions so:

Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Thanks for taking the time to do the conversion for the SPARC target.


ATB,

Mark.

On Fri, 6 Sep 2019 at 15:34, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> On 01/08/2019 19:30, Peter Maydell wrote:
>
> > This patchset converts the SPARC target away from the old
> > broken do_unassigned_access hook to the new (added in 2017...)
> > do_transaction_failed hook. In the process it fixes a number
> > of bugs in corner cases.

> > Peter Maydell (7):
> >   target/sparc: Factor out the body of sparc_cpu_unassigned_access()
> >   target/sparc: Check for transaction failures in MMU passthrough ASIs
> >   target/sparc: Check for transaction failures in MXCC stream ASI
> >     accesses
> >   target/sparc: Correctly handle bus errors in page table walks
> >   target/sparc: Handle bus errors in mmu_probe()
> >   target/sparc: Remove unused ldl_phys from dump_mmu()
> >   target/sparc: Switch to do_transaction_failed() hook
> >
> >  target/sparc/cpu.h         |   8 +-
> >  target/sparc/cpu.c         |   2 +-
> >  target/sparc/ldst_helper.c | 319 +++++++++++++++++++++----------------
> >  target/sparc/mmu_helper.c  |  57 +++++--
> >  4 files changed, 238 insertions(+), 148 deletions(-)
>
> I've just run this through my SPARC test images with a fairly recent git master and I
> don't see any obvious regressions so:
>
> Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>
> Thanks for taking the time to do the conversion for the SPARC target.

Thanks; I've applied the patchset to master.

-- PMM