diff mbox series

[for-6.2,07/43] target/ppc: Set fault address in ppc_cpu_do_unaligned_access

Message ID 20210729004647.282017-8-richard.henderson@linaro.org
State New
Headers show
Series Unaligned accesses for user-only | expand

Commit Message

Richard Henderson July 29, 2021, 12:46 a.m. UTC
We ought to have been recording the virtual address for reporting
to the guest trap handler.

Cc: qemu-ppc@nongnu.org
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 target/ppc/excp_helper.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.25.1

Comments

Peter Maydell July 29, 2021, 1:44 p.m. UTC | #1
On Thu, 29 Jul 2021 at 01:51, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> We ought to have been recording the virtual address for reporting

> to the guest trap handler.

>

> Cc: qemu-ppc@nongnu.org

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  target/ppc/excp_helper.c | 2 ++

>  1 file changed, 2 insertions(+)

>

> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c

> index a79a0ed465..0b2c6de442 100644

> --- a/target/ppc/excp_helper.c

> +++ b/target/ppc/excp_helper.c

> @@ -1503,6 +1503,8 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,

>      CPUPPCState *env = cs->env_ptr;

>      uint32_t insn;

>

> +    env->spr[SPR_DAR] = vaddr;

> +


Is this the right SPR for all PPC variants? For instance the
kernel's code in arch/powerpc/kernel/exceptions-64e.S looks
in SPRN_DEAR, which is our SPR_BOOKE_DEAR or SPR_40x_DEAR.

-- PMM
Richard Henderson July 29, 2021, 6:05 p.m. UTC | #2
On 7/29/21 3:44 AM, Peter Maydell wrote:
> On Thu, 29 Jul 2021 at 01:51, Richard Henderson

> <richard.henderson@linaro.org> wrote:

>>

>> We ought to have been recording the virtual address for reporting

>> to the guest trap handler.

>>

>> Cc: qemu-ppc@nongnu.org

>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

>> ---

>>   target/ppc/excp_helper.c | 2 ++

>>   1 file changed, 2 insertions(+)

>>

>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c

>> index a79a0ed465..0b2c6de442 100644

>> --- a/target/ppc/excp_helper.c

>> +++ b/target/ppc/excp_helper.c

>> @@ -1503,6 +1503,8 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,

>>       CPUPPCState *env = cs->env_ptr;

>>       uint32_t insn;

>>

>> +    env->spr[SPR_DAR] = vaddr;

>> +

> 

> Is this the right SPR for all PPC variants? For instance the

> kernel's code in arch/powerpc/kernel/exceptions-64e.S looks

> in SPRN_DEAR, which is our SPR_BOOKE_DEAR or SPR_40x_DEAR.


I have no idea.  I glanced through a handful of the mmu's, and looked at the current BookS 
docs, but that's certainly not all.

I'll note that if we do need to set different regs for different mmus, we'll probably want 
to standardize on this one for user-only, like we did for the user-only copy of 
ppc_cpu_tlb_fill.


r~
Cédric Le Goater July 30, 2021, 4:58 p.m. UTC | #3
On 7/29/21 2:46 AM, Richard Henderson wrote:
> We ought to have been recording the virtual address for reporting

> to the guest trap handler.

> 

> Cc: qemu-ppc@nongnu.org

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Reviewed-by: Cédric Le Goater <clg@kaod.org>



> ---

>  target/ppc/excp_helper.c | 2 ++

>  1 file changed, 2 insertions(+)

> 

> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c

> index a79a0ed465..0b2c6de442 100644

> --- a/target/ppc/excp_helper.c

> +++ b/target/ppc/excp_helper.c

> @@ -1503,6 +1503,8 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,

>      CPUPPCState *env = cs->env_ptr;

>      uint32_t insn;

>  

> +    env->spr[SPR_DAR] = vaddr;

> +

>      /* Restore state and reload the insn we executed, for filling in DSISR.  */

>      cpu_restore_state(cs, retaddr, true);

>      insn = cpu_ldl_code(env, env->nip);

>
Cédric Le Goater July 30, 2021, 5:13 p.m. UTC | #4
On 7/29/21 8:05 PM, Richard Henderson wrote:
> On 7/29/21 3:44 AM, Peter Maydell wrote:

>> On Thu, 29 Jul 2021 at 01:51, Richard Henderson

>> <richard.henderson@linaro.org> wrote:

>>>

>>> We ought to have been recording the virtual address for reporting

>>> to the guest trap handler.

>>>

>>> Cc: qemu-ppc@nongnu.org

>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

>>> ---

>>>   target/ppc/excp_helper.c | 2 ++

>>>   1 file changed, 2 insertions(+)

>>>

>>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c

>>> index a79a0ed465..0b2c6de442 100644

>>> --- a/target/ppc/excp_helper.c

>>> +++ b/target/ppc/excp_helper.c

>>> @@ -1503,6 +1503,8 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,

>>>       CPUPPCState *env = cs->env_ptr;

>>>       uint32_t insn;

>>>

>>> +    env->spr[SPR_DAR] = vaddr;

>>> +

>>

>> Is this the right SPR for all PPC variants? For instance the

>> kernel's code in arch/powerpc/kernel/exceptions-64e.S looks

>> in SPRN_DEAR, which is our SPR_BOOKE_DEAR or SPR_40x_DEAR.


Indeed :/

> I have no idea.  I glanced through a handful of the mmu's, and looked at the current BookS docs, but that's certainly not all.


I took a look at some more and for instance, e300 uses DAR and e500, 405, 476 use DEAR. 

DAR should be consistent over the server processors.

C.

> 

> I'll note that if we do need to set different regs for different mmus, we'll probably want to standardize on this one for user-only, like we did for the user-only copy of ppc_cpu_tlb_fill.

> 

> 

> r~

>
Cédric Le Goater July 30, 2021, 5:23 p.m. UTC | #5
On 7/30/21 7:13 PM, Cédric Le Goater wrote:
> On 7/29/21 8:05 PM, Richard Henderson wrote:

>> On 7/29/21 3:44 AM, Peter Maydell wrote:

>>> On Thu, 29 Jul 2021 at 01:51, Richard Henderson

>>> <richard.henderson@linaro.org> wrote:

>>>>

>>>> We ought to have been recording the virtual address for reporting

>>>> to the guest trap handler.

>>>>

>>>> Cc: qemu-ppc@nongnu.org

>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

>>>> ---

>>>>   target/ppc/excp_helper.c | 2 ++

>>>>   1 file changed, 2 insertions(+)

>>>>

>>>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c

>>>> index a79a0ed465..0b2c6de442 100644

>>>> --- a/target/ppc/excp_helper.c

>>>> +++ b/target/ppc/excp_helper.c

>>>> @@ -1503,6 +1503,8 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,

>>>>       CPUPPCState *env = cs->env_ptr;

>>>>       uint32_t insn;

>>>>

>>>> +    env->spr[SPR_DAR] = vaddr;

>>>> +

>>>

>>> Is this the right SPR for all PPC variants? For instance the

>>> kernel's code in arch/powerpc/kernel/exceptions-64e.S looks

>>> in SPRN_DEAR, which is our SPR_BOOKE_DEAR or SPR_40x_DEAR.

> 

> Indeed :/

> 

>> I have no idea.  I glanced through a handful of the mmu's, and looked at the current BookS docs, but that's certainly not all.

> 

> I took a look at some more and for instance, e300 uses DAR and e500, 405, 476 use DEAR. 

> 

> DAR should be consistent over the server processors.



and  is_book3s_arch2x(env) is a good way to test.

Thanks,

C.
diff mbox series

Patch

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index a79a0ed465..0b2c6de442 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1503,6 +1503,8 @@  void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
     CPUPPCState *env = cs->env_ptr;
     uint32_t insn;
 
+    env->spr[SPR_DAR] = vaddr;
+
     /* Restore state and reload the insn we executed, for filling in DSISR.  */
     cpu_restore_state(cs, retaddr, true);
     insn = cpu_ldl_code(env, env->nip);