diff mbox

target-mips: Ignore unassigned accesses with KVM

Message ID 1406547470-22766-1-git-send-email-james.hogan@imgtec.com (mailing list archive)
State New, archived
Headers show

Commit Message

James Hogan July 28, 2014, 11:37 a.m. UTC
MIPS registers an unassigned access handler which raises a guest bus
error exception. However this causes QEMU to crash when KVM is enabled
as it isn't called from the main execution loop so longjmp() gets called
without a corresponding setjmp().

Until the KVM API can be updated to trigger a guest exception in
response to an MMIO exit, prevent the bus error exception being raised
from mips_cpu_unassigned_access() if KVM is enabled.

The check is at run time since the do_unassigned_access callback is
initialised before it is known whether KVM will be enabled.

The problem can be triggered with Malta emulation by making the guest
write to the reset region at physical address 0x1bf00000, since it is
marked read-only which is treated as unassigned for writes.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@redhat.com>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 target-mips/op_helper.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Aurelien Jarno July 28, 2014, 9:36 p.m. UTC | #1
On Mon, Jul 28, 2014 at 12:37:50PM +0100, James Hogan wrote:
> MIPS registers an unassigned access handler which raises a guest bus
> error exception. However this causes QEMU to crash when KVM is enabled
> as it isn't called from the main execution loop so longjmp() gets called
> without a corresponding setjmp().
> 
> Until the KVM API can be updated to trigger a guest exception in
> response to an MMIO exit, prevent the bus error exception being raised
> from mips_cpu_unassigned_access() if KVM is enabled.
> 
> The check is at run time since the do_unassigned_access callback is
> initialised before it is known whether KVM will be enabled.
> 
> The problem can be triggered with Malta emulation by making the guest
> write to the reset region at physical address 0x1bf00000, since it is
> marked read-only which is treated as unassigned for writes.
> 
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Gleb Natapov <gleb@redhat.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Sanjay Lal <sanjayl@kymasys.com>
> ---
>  target-mips/op_helper.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index 27651a4a00c1..df97b35f8701 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -21,6 +21,7 @@
>  #include "qemu/host-utils.h"
>  #include "exec/helper-proto.h"
>  #include "exec/cpu_ldst.h"
> +#include "sysemu/kvm.h"
>  
>  #ifndef CONFIG_USER_ONLY
>  static inline void cpu_mips_tlb_flush (CPUMIPSState *env, int flush_global);
> @@ -2168,6 +2169,16 @@ void mips_cpu_unassigned_access(CPUState *cs, hwaddr addr,
>      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) {
>          helper_raise_exception(env, EXCP_IBE);
>      } else {

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

Note that even if the test is added for each exception, it is light
enough compared to triggering and handling an exception so that it has
no impact on performance.

Paolo, do you want to take this patch in your kvm tree?
Paolo Bonzini July 29, 2014, 8:55 a.m. UTC | #2
Il 28/07/2014 23:36, Aurelien Jarno ha scritto:
> On Mon, Jul 28, 2014 at 12:37:50PM +0100, James Hogan wrote:
>> MIPS registers an unassigned access handler which raises a guest bus
>> error exception. However this causes QEMU to crash when KVM is enabled
>> as it isn't called from the main execution loop so longjmp() gets called
>> without a corresponding setjmp().
>>
>> Until the KVM API can be updated to trigger a guest exception in
>> response to an MMIO exit, prevent the bus error exception being raised
>> from mips_cpu_unassigned_access() if KVM is enabled.
>>
>> The check is at run time since the do_unassigned_access callback is
>> initialised before it is known whether KVM will be enabled.
>>
>> The problem can be triggered with Malta emulation by making the guest
>> write to the reset region at physical address 0x1bf00000, since it is
>> marked read-only which is treated as unassigned for writes.
>>
>> Signed-off-by: James Hogan <james.hogan@imgtec.com>
>> Cc: Aurelien Jarno <aurelien@aurel32.net>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Gleb Natapov <gleb@redhat.com>
>> Cc: Christoffer Dall <christoffer.dall@linaro.org>
>> Cc: Sanjay Lal <sanjayl@kymasys.com>
>> ---
>>  target-mips/op_helper.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
>> index 27651a4a00c1..df97b35f8701 100644
>> --- a/target-mips/op_helper.c
>> +++ b/target-mips/op_helper.c
>> @@ -21,6 +21,7 @@
>>  #include "qemu/host-utils.h"
>>  #include "exec/helper-proto.h"
>>  #include "exec/cpu_ldst.h"
>> +#include "sysemu/kvm.h"
>>  
>>  #ifndef CONFIG_USER_ONLY
>>  static inline void cpu_mips_tlb_flush (CPUMIPSState *env, int flush_global);
>> @@ -2168,6 +2169,16 @@ void mips_cpu_unassigned_access(CPUState *cs, hwaddr addr,
>>      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) {
>>          helper_raise_exception(env, EXCP_IBE);
>>      } else {
> 
> Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
> 
> Note that even if the test is added for each exception, it is light
> enough compared to triggering and handling an exception so that it has
> no impact on performance.
> 
> Paolo, do you want to take this patch in your kvm tree?

Sure, I'll include it for 2.2.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 27651a4a00c1..df97b35f8701 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -21,6 +21,7 @@ 
 #include "qemu/host-utils.h"
 #include "exec/helper-proto.h"
 #include "exec/cpu_ldst.h"
+#include "sysemu/kvm.h"
 
 #ifndef CONFIG_USER_ONLY
 static inline void cpu_mips_tlb_flush (CPUMIPSState *env, int flush_global);
@@ -2168,6 +2169,16 @@  void mips_cpu_unassigned_access(CPUState *cs, hwaddr addr,
     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) {
         helper_raise_exception(env, EXCP_IBE);
     } else {