Re: [Qemu-devel] [PATCH] target-mips: Ignore unassigned accesses with KVM

[Paolo Bonzini]

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 <address@hidden>
>> Cc: Aurelien Jarno <address@hidden>
>> Cc: Peter Maydell <address@hidden>
>> Cc: Paolo Bonzini <address@hidden>
>> Cc: Gleb Natapov <address@hidden>
>> Cc: Christoffer Dall <address@hidden>
>> Cc: Sanjay Lal <address@hidden>
>> ---
>>  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 <address@hidden>
> 
> 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