From 3912df61de73bb55c4c8b9afddf7e422f3ef9eb8 Mon Sep 17 00:00:00 2001 From: "Takacs, Philipp" Date: Mon, 21 Oct 2024 15:08:28 +0200 Subject: [PATCH] disable notdirty_write for self modifying code When SMC access the memory region more then once the tb must be rebuild multible times. fixes #2029 --- qemu/accel/tcg/cputlb.c | 23 ++++++++++------------- tests/unit/test_x86.c | 21 +++++++++++++++++++++ 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/qemu/accel/tcg/cputlb.c b/qemu/accel/tcg/cputlb.c index b2d8ac97bf..0f4ef351b0 100644 --- a/qemu/accel/tcg/cputlb.c +++ b/qemu/accel/tcg/cputlb.c @@ -1188,9 +1188,13 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr) static void notdirty_write(CPUState *cpu, vaddr mem_vaddr, unsigned size, CPUIOTLBEntry *iotlbentry, uintptr_t retaddr, - MemoryRegion *mr) + CPUTLBEntry *tlbe) { +#ifdef TARGET_ARM + struct uc_struct *uc = cpu->uc; +#endif ram_addr_t ram_addr = mem_vaddr + iotlbentry->addr; + MemoryRegion *mr = cpu->uc->memory_mapping(cpu->uc, tlbe->paddr | (mem_vaddr & ~TARGET_PAGE_MASK)); if (mr && (mr->perms & UC_PROT_EXEC) != 0) { struct page_collection *pages @@ -1205,7 +1209,8 @@ static void notdirty_write(CPUState *cpu, vaddr mem_vaddr, unsigned size, // - or doing snapshot // , then never clean the tlb if (!(!mr || mr->priority < cpu->uc->snapshot_level) && - !(HOOK_EXISTS(cpu->uc, UC_HOOK_MEM_READ) || HOOK_EXISTS(cpu->uc, UC_HOOK_MEM_WRITE))) { + !(HOOK_EXISTS(cpu->uc, UC_HOOK_MEM_READ) || HOOK_EXISTS(cpu->uc, UC_HOOK_MEM_WRITE)) && + !(tlbe->addr_code != -1)) { tlb_set_dirty(cpu, mem_vaddr); } } @@ -1228,8 +1233,6 @@ void *probe_access(CPUArchState *env, target_ulong addr, int size, target_ulong tlb_addr; size_t elt_ofs = 0; int wp_access = 0; - MemoryRegion *mr = NULL; - target_ulong paddr; #ifdef _MSC_VER g_assert(((target_ulong)0 - (addr | TARGET_PAGE_MASK)) >= size); @@ -1286,9 +1289,7 @@ void *probe_access(CPUArchState *env, target_ulong addr, int size, /* Handle clean RAM pages. */ if (tlb_addr & TLB_NOTDIRTY) { - paddr = entry->paddr | (addr & ~TARGET_PAGE_MASK); - mr = env->uc->memory_mapping(env->uc, paddr); - notdirty_write(env_cpu(env), addr, size, iotlbentry, retaddr, mr); + notdirty_write(env_cpu(env), addr, size, iotlbentry, retaddr, entry); } } @@ -1362,8 +1363,6 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr, int a_bits = get_alignment_bits(mop); int s_bits = mop & MO_SIZE; void *hostaddr; - MemoryRegion *mr; - target_ulong paddr; /* Adjust the given return address. */ retaddr -= GETPC_ADJ; @@ -1415,10 +1414,8 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr, hostaddr = (void *)((uintptr_t)addr + tlbe->addend); if (unlikely(tlb_addr & TLB_NOTDIRTY)) { - paddr = tlbe->paddr | (addr & ~TARGET_PAGE_MASK); - mr = env->uc->memory_mapping(env->uc, paddr); notdirty_write(env_cpu(env), addr, 1 << s_bits, - &env_tlb(env)->d[mmu_idx].iotlb[index], retaddr, mr); + &env_tlb(env)->d[mmu_idx].iotlb[index], retaddr, tlbe); } return hostaddr; @@ -2277,7 +2274,7 @@ store_helper(CPUArchState *env, target_ulong addr, uint64_t val, /* Handle clean RAM pages. */ if (tlb_addr & TLB_NOTDIRTY) { - notdirty_write(env_cpu(env), addr, size, iotlbentry, retaddr, mr); + notdirty_write(env_cpu(env), addr, size, iotlbentry, retaddr, entry); } haddr = (void *)((uintptr_t)addr + entry->addend); diff --git a/tests/unit/test_x86.c b/tests/unit/test_x86.c index c3d0b15776..f969290dda 100644 --- a/tests/unit/test_x86.c +++ b/tests/unit/test_x86.c @@ -624,6 +624,26 @@ static void test_x86_smc_xor(void) OK(uc_close(uc)); } +static void test_x86_smc_add(void) +{ + uc_engine *uc; + uint64_t stack_base = 0x20000; + int r_rsp; + /* + * mov qword ptr [rip+0x10], rax + * mov word ptr [rip], 0x0548 + * [orig] mov eax, dword ptr [rax + 0x12345678]; [after SMC] 480578563412 add rax, 0x12345678 + * hlt + */ + char code[] = "\x48\x89\x05\x10\x00\x00\x00\x66\xc7\x05\x00\x00\x00\x00\x48\x05\x8b\x80\x78\x56\x34\x12\xf4"; + uc_common_setup(&uc, UC_ARCH_X86, UC_MODE_64, code, sizeof(code) - 1); + + OK(uc_mem_map(uc, stack_base, 0x2000, UC_PROT_ALL)); + r_rsp = stack_base + 0x1800; + OK(uc_reg_write(uc, UC_X86_REG_RSP, &r_rsp)); + OK(uc_emu_start(uc, code_start, -1, 0, 0)); +} + static uint64_t test_x86_mmio_uc_mem_rw_read_callback(uc_engine *uc, uint64_t offset, unsigned size, @@ -1849,6 +1869,7 @@ TEST_LIST = { {"test_x86_mmio", test_x86_mmio}, {"test_x86_missing_code", test_x86_missing_code}, {"test_x86_smc_xor", test_x86_smc_xor}, + {"test_x86_smc_add", test_x86_smc_add}, {"test_x86_mmio_uc_mem_rw", test_x86_mmio_uc_mem_rw}, {"test_x86_sysenter", test_x86_sysenter}, {"test_x86_hook_cpuid", test_x86_hook_cpuid},