diff mbox series

[2/3] target/mips: Switch to do_transaction_failed() hook

Message ID 20190802160458.25681-3-peter.maydell@linaro.org
State Accepted
Headers show
Series target/mips: Convert to do_transaction_failed hook | expand

Commit Message

Peter Maydell Aug. 2, 2019, 4:04 p.m. UTC
Switch the MIPS target from the old unassigned_access hook to the new
do_transaction_failed hook.

Unlike the old hook, do_transaction_failed is only ever called from
the TCG memory access paths, so there is no need for the "ignore this
if we're using KVM" hack that we were previously using to work around
the way unassigned_access was called for all kinds of memory accesses
to unassigned physical addresses.

The MIPS target does not ever do direct memory reads by physical
address (via either ldl_phys etc or address_space_ldl etc), so the
only memory accesses this affects are the 'normal' guest loads and
stores, which will be handled by the new hook; their behaviour is
unchanged.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
 target/mips/internal.h  |  8 +++++---
 target/mips/cpu.c       |  2 +-
 target/mips/op_helper.c | 24 ++++++++----------------
 3 files changed, 14 insertions(+), 20 deletions(-)

-- 
2.20.1

Comments

Philippe Mathieu-Daudé Aug. 2, 2019, 4:29 p.m. UTC | #1
Cc'ing James Hogan.

On 8/2/19 6:04 PM, Peter Maydell wrote:
> Switch the MIPS target from the old unassigned_access hook to the new

> do_transaction_failed hook.

> 

> Unlike the old hook, do_transaction_failed is only ever called from

> the TCG memory access paths, so there is no need for the "ignore this

> if we're using KVM" hack that we were previously using to work around

> the way unassigned_access was called for all kinds of memory accesses

> to unassigned physical addresses.

> 

> The MIPS target does not ever do direct memory reads by physical

> address (via either ldl_phys etc or address_space_ldl etc), so the

> only memory accesses this affects are the 'normal' guest loads and

> stores, which will be handled by the new hook; their behaviour is

> unchanged.

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

>  target/mips/internal.h  |  8 +++++---

>  target/mips/cpu.c       |  2 +-

>  target/mips/op_helper.c | 24 ++++++++----------------

>  3 files changed, 14 insertions(+), 20 deletions(-)

> 

> diff --git a/target/mips/internal.h b/target/mips/internal.h

> index b2b41a51ab4..26533bb937d 100644

> --- a/target/mips/internal.h

> +++ b/target/mips/internal.h

> @@ -138,9 +138,11 @@ void r4k_helper_tlbinv(CPUMIPSState *env);

>  void r4k_helper_tlbinvf(CPUMIPSState *env);

>  void r4k_invalidate_tlb(CPUMIPSState *env, int idx, int use_extra);

>  

> -void mips_cpu_unassigned_access(CPUState *cpu, hwaddr addr,

> -                                bool is_write, bool is_exec, int unused,

> -                                unsigned size);

> +void mips_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,

> +                                    vaddr addr, unsigned size,

> +                                    MMUAccessType access_type,

> +                                    int mmu_idx, MemTxAttrs attrs,

> +                                    MemTxResult response, uintptr_t retaddr);

>  hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address,

>                                    int rw);

>  #endif

> diff --git a/target/mips/cpu.c b/target/mips/cpu.c

> index 39eafafc5cd..a79badcb1a6 100644

> --- a/target/mips/cpu.c

> +++ b/target/mips/cpu.c

> @@ -197,7 +197,7 @@ static void mips_cpu_class_init(ObjectClass *c, void *data)

>      cc->gdb_read_register = mips_cpu_gdb_read_register;

>      cc->gdb_write_register = mips_cpu_gdb_write_register;

>  #ifndef CONFIG_USER_ONLY

> -    cc->do_unassigned_access = mips_cpu_unassigned_access;

> +    cc->do_transaction_failed = mips_cpu_do_transaction_failed;

>      cc->do_unaligned_access = mips_cpu_do_unaligned_access;

>      cc->get_phys_page_debug = mips_cpu_get_phys_page_debug;

>      cc->vmsd = &vmstate_mips_cpu;

> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c

> index 9e2e02f8586..65ff9f5b935 100644

> --- a/target/mips/op_helper.c

> +++ b/target/mips/op_helper.c

> @@ -2666,27 +2666,19 @@ void mips_cpu_do_unaligned_access(CPUState *cs, vaddr addr,

>      do_raise_exception_err(env, excp, error_code, retaddr);

>  }

>  

> -void mips_cpu_unassigned_access(CPUState *cs, hwaddr addr,

> -                                bool is_write, bool is_exec, int unused,

> -                                unsigned size)

> +void mips_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,

> +                                    vaddr addr, unsigned size,

> +                                    MMUAccessType access_type,

> +                                    int mmu_idx, MemTxAttrs attrs,

> +                                    MemTxResult response, uintptr_t retaddr)

>  {

>      MIPSCPU *cpu = MIPS_CPU(cs);

>      CPUMIPSState *env = &cpu->env;

>  

> -    /*

> -     * Raising an exception with KVM enabled will crash because it won't be from

> -     * the main execution loop so the longjmp won't have a matching setjmp.

> -     * Until we can trigger a bus error exception through KVM lets just ignore

> -     * the access.

> -     */

> -    if (kvm_enabled()) {

> -        return;


Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>


> -    }

> -

> -    if (is_exec) {

> -        raise_exception(env, EXCP_IBE);

> +    if (access_type == MMU_INST_FETCH) {

> +        do_raise_exception(env, EXCP_IBE, retaddr);

>      } else {

> -        raise_exception(env, EXCP_DBE);

> +        do_raise_exception(env, EXCP_DBE, retaddr);

>      }

>  }

>  #endif /* !CONFIG_USER_ONLY */

>
Hervé Poussineau Sept. 9, 2019, 9:41 p.m. UTC | #2
Le 02/08/2019 à 18:04, Peter Maydell a écrit :
> Switch the MIPS target from the old unassigned_access hook to the new

> do_transaction_failed hook.

> 

> Unlike the old hook, do_transaction_failed is only ever called from

> the TCG memory access paths, so there is no need for the "ignore this

> if we're using KVM" hack that we were previously using to work around

> the way unassigned_access was called for all kinds of memory accesses

> to unassigned physical addresses.

> 

> The MIPS target does not ever do direct memory reads by physical

> address (via either ldl_phys etc or address_space_ldl etc), so the

> only memory accesses this affects are the 'normal' guest loads and

> stores, which will be handled by the new hook; their behaviour is

> unchanged.

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

>   target/mips/internal.h  |  8 +++++---

>   target/mips/cpu.c       |  2 +-

>   target/mips/op_helper.c | 24 ++++++++----------------

>   3 files changed, 14 insertions(+), 20 deletions(-)


Tested-by: Hervé Poussineau <hpoussin@reactos.org>
diff mbox series

Patch

diff --git a/target/mips/internal.h b/target/mips/internal.h
index b2b41a51ab4..26533bb937d 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -138,9 +138,11 @@  void r4k_helper_tlbinv(CPUMIPSState *env);
 void r4k_helper_tlbinvf(CPUMIPSState *env);
 void r4k_invalidate_tlb(CPUMIPSState *env, int idx, int use_extra);
 
-void mips_cpu_unassigned_access(CPUState *cpu, hwaddr addr,
-                                bool is_write, bool is_exec, int unused,
-                                unsigned size);
+void mips_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
+                                    vaddr addr, unsigned size,
+                                    MMUAccessType access_type,
+                                    int mmu_idx, MemTxAttrs attrs,
+                                    MemTxResult response, uintptr_t retaddr);
 hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address,
                                   int rw);
 #endif
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 39eafafc5cd..a79badcb1a6 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -197,7 +197,7 @@  static void mips_cpu_class_init(ObjectClass *c, void *data)
     cc->gdb_read_register = mips_cpu_gdb_read_register;
     cc->gdb_write_register = mips_cpu_gdb_write_register;
 #ifndef CONFIG_USER_ONLY
-    cc->do_unassigned_access = mips_cpu_unassigned_access;
+    cc->do_transaction_failed = mips_cpu_do_transaction_failed;
     cc->do_unaligned_access = mips_cpu_do_unaligned_access;
     cc->get_phys_page_debug = mips_cpu_get_phys_page_debug;
     cc->vmsd = &vmstate_mips_cpu;
diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index 9e2e02f8586..65ff9f5b935 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -2666,27 +2666,19 @@  void mips_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
     do_raise_exception_err(env, excp, error_code, retaddr);
 }
 
-void mips_cpu_unassigned_access(CPUState *cs, hwaddr addr,
-                                bool is_write, bool is_exec, int unused,
-                                unsigned size)
+void mips_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
+                                    vaddr addr, unsigned size,
+                                    MMUAccessType access_type,
+                                    int mmu_idx, MemTxAttrs attrs,
+                                    MemTxResult response, uintptr_t retaddr)
 {
     MIPSCPU *cpu = MIPS_CPU(cs);
     CPUMIPSState *env = &cpu->env;
 
-    /*
-     * Raising an exception with KVM enabled will crash because it won't be from
-     * the main execution loop so the longjmp won't have a matching setjmp.
-     * Until we can trigger a bus error exception through KVM lets just ignore
-     * the access.
-     */
-    if (kvm_enabled()) {
-        return;
-    }
-
-    if (is_exec) {
-        raise_exception(env, EXCP_IBE);
+    if (access_type == MMU_INST_FETCH) {
+        do_raise_exception(env, EXCP_IBE, retaddr);
     } else {
-        raise_exception(env, EXCP_DBE);
+        do_raise_exception(env, EXCP_DBE, retaddr);
     }
 }
 #endif /* !CONFIG_USER_ONLY */