From 61ef776fedab5f6e87eeae20e3d39b59e978e314 Mon Sep 17 00:00:00 2001 From: David Guillen Fandos Date: Fri, 10 Dec 2021 18:32:04 +0100 Subject: [PATCH] [x86] Fix CPSR store bug in LR register There's a race condition on CPSR store (only if mode is changed) where, if an IRQ is pending, the IRQ will be served, but the saved LR value will be wrong (will skip the return instruction). Fixed this and improved the logic a bit to make it faster and not use unnecessary save slots. --- x86/x86_emit.h | 25 ++++++++++--------------- x86/x86_stub.S | 13 +++++++++---- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/x86/x86_emit.h b/x86/x86_emit.h index fad2d36..f08bfc1 100644 --- a/x86/x86_emit.h +++ b/x86/x86_emit.h @@ -1365,23 +1365,18 @@ u32 function_cc execute_spsr_restore(u32 address) execute_read_##psr_reg(rv); \ generate_store_reg(rv, rd) \ -// store_mask and address are stored in the SAVE slots, since there's no real -// register space to nicely pass them. - +// Does mode-change magic (including IRQ checks) u32 execute_store_cpsr_body() { - if(reg[REG_SAVE] & 0xFF) + set_cpu_mode(cpu_modes[reg[REG_CPSR] & 0x1F]); + if((io_registers[REG_IE] & io_registers[REG_IF]) && + io_registers[REG_IME] && ((reg[REG_CPSR] & 0x80) == 0)) { - set_cpu_mode(cpu_modes[reg[REG_CPSR] & 0x1F]); - if((io_registers[REG_IE] & io_registers[REG_IF]) && - io_registers[REG_IME] && ((reg[REG_CPSR] & 0x80) == 0)) - { - reg_mode[MODE_IRQ][6] = reg[REG_SAVE2] + 4; - spsr[MODE_IRQ] = reg[REG_CPSR]; - reg[REG_CPSR] = (reg[REG_CPSR] & 0xFFFFFF00) | 0xD2; - set_cpu_mode(MODE_IRQ); - return 0x00000018; - } + reg_mode[MODE_IRQ][6] = reg[REG_PC] + 4; + spsr[MODE_IRQ] = reg[REG_CPSR]; + reg[REG_CPSR] = (reg[REG_CPSR] & 0xFFFFFF00) | 0xD2; + set_cpu_mode(MODE_IRQ); + return 0x00000018; } return 0; @@ -1397,7 +1392,7 @@ u32 execute_store_cpsr_body() #define execute_store_cpsr() \ generate_load_imm(a1, psr_masks[psr_field]); \ - generate_store_reg_i32(pc + 4, REG_SAVE2); \ + generate_store_reg_i32(pc, REG_PC); \ generate_function_call(execute_store_cpsr) \ /* spsr[reg[CPU_MODE]] = (new_spsr & store_mask) | (old_spsr & (~store_mask))*/ diff --git a/x86/x86_stub.S b/x86/x86_stub.S index 8681510..0153539 100644 --- a/x86/x86_stub.S +++ b/x86/x86_stub.S @@ -448,7 +448,7 @@ load_stubs( s8, movsbl, ~0, 0, read_memory8s) # arg0 (%eax) = new_cpsr # arg1 (%edx) = store_mask defsymbl(execute_store_cpsr) - mov %edx, REG_SAVE(REG_BASE) # save store_mask + mov %edx, %esi # save store_mask for later mov %eax, %ecx # ecx = new_cpsr and %edx, %ecx # ecx = new_cpsr & store_mask @@ -458,13 +458,18 @@ defsymbl(execute_store_cpsr) or %ecx, %eax # eax = new cpsr combined with old mov %eax, REG_CPSR(REG_BASE) # save new cpsr to register + # Check whether any side effects (IRQ) could have happened + test $0xff, %esi + jnz 1f + extract_flags # pull out flag vars from new CPSR + ret +1: CALL_FUNC(execute_store_cpsr_body) # do the dirty work in this C function extract_flags # pull out flag vars from new CPSR - cmp $0, %eax # see if return value is 0 - jnz 1f # might have changed the PC + jnz 2f # might have changed the PC ret # return -1: # PC has changed, due to IRQ triggered +2: # PC has changed, due to IRQ triggered mov %eax, CARG1_REG # Returned addr from C function CALL_FUNC(block_lookup_address_arm) # lookup new PC add $ADDR_SIZE_BYTES, STACK_REG # get rid of current return address