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

Message ID 20210729004647.282017-15-richard.henderson@linaro.org
State New
Headers show
Series
  • Unaligned accesses for user-only
Related show

Commit Message

Richard Henderson July 29, 2021, 12:46 a.m.
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(+)

-- 
2.25.1

Comments

Peter Maydell July 29, 2021, 2:51 p.m. | #1
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.

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().

-- PMM
Mark Cave-Ayland Aug. 1, 2021, 3:56 p.m. | #2
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.
Peter Maydell Aug. 1, 2021, 3:59 p.m. | #3
On Sun, 1 Aug 2021 at 16:56, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>

> 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.


Do you know what 32-bit CPUs do? The Linux kernel sources don't help
here because they don't bother to report the fault address...

thanks
-- PMM
Mark Cave-Ayland Aug. 1, 2021, 4:13 p.m. | #4
On 01/08/2021 16:59, Peter Maydell wrote:

> On Sun, 1 Aug 2021 at 16:56, Mark Cave-Ayland

> <mark.cave-ayland@ilande.co.uk> wrote:

>>

>> 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.

> 

> Do you know what 32-bit CPUs do? The Linux kernel sources don't help

> here because they don't bother to report the fault address...


The SFSR and SFAR for the 32-bit sun4m machines is described in the Sun4m 
Architecture manual section 4.4 "Synchronous Fault Registers". Unaligned access 
behaviour isn't explicitly mentioned AFAICS but fault type 1 is "Invalid Address 
Error" which seems like a possibility.


ATB,

Mark.

Patch

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