Re: [PATCH for-6.2 14/43] target/sparc: Set fault address in sparc_cpu_do_unaligned_access

[Mark Cave-Ayland]

On 29/07/2021 15:51, Peter Maydell wrote:

> On Thu, 29 Jul 2021 at 02:01, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> > We ought to have been recording the virtual address for reporting
> > to the guest trap handler.  Mirror the SFSR FIXME from the sparc64
> > version of get_physical_address_data.
> >
> > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >   target/sparc/ldst_helper.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> >
> > diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
> > index 974afea041..7367b48c8b 100644
> > --- a/target/sparc/ldst_helper.c
> > +++ b/target/sparc/ldst_helper.c
> > @@ -1963,6 +1963,14 @@ void QEMU_NORETURN 
> > sparc_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> >       SPARCCPU *cpu = SPARC_CPU(cs);
> >       CPUSPARCState *env = &cpu->env;
> >
> > +#ifdef TARGET_SPARC64
> > +    /* FIXME: ASI field in SFSR must be set */
> > +    env->dmmu.sfsr = SFSR_VALID_BIT; /* Fault status register */
> > +    env->dmmu.sfar = addr;           /* Fault address register */
> > +#else
> > +    env->mmuregs[4] = addr;
> > +#endif
> > +
> >       cpu_raise_exception_ra(env, TT_UNALIGNED, retaddr);
> >   }
> >   #endif
>
> The architecture manual seems to be gratuitously opaque about
> whether and where the fault address for an alignment fault gets
> recorded, but Linux at least for 64-bit seems to pull it out of the
> sfar, so I guess that's right.

Yeah, this part is actually contained within the UltraSPARC II specification - it can 
be found in section 6.4 "MMU-Related Faults and Traps" table 6-3 which indicates that 
for *_mem_address_not_aligned traps the D-SFSR and SFAR registers within the MMU are 
updated.

> We probably ought to have the "if SFSR_VALID_BIT already set,
> set the OW bit". MMUAccessType and mmu_idx give us enough to
> set the CT bits and the WRITE bit in the same way we do at
> the start of get_physical_address_data().

This sounds correct too from reading over section 6.9.4 "I-/D-MMU Synchronous Fault 
Status Registers (SFSR)". I'm going to be fairly busy with $DAYJOB for the next 
couple of weeks so it's going to be a little while before I can take a look at this 
in more detail.


ATB,

Mark.