From 858e6bb252e075e09cca6e78299151d3af0bf5fb Mon Sep 17 00:00:00 2001 From: Quan Sun Date: Tue, 28 Apr 2026 14:56:36 -0400 Subject: [PATCH] accel/tcg: Fix iotlb_to_section() for different AddressSpace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'CPUTLBEntryFull.xlat_section' stores section_index in last 12 bits to find the correct section when CPU access the IO region over the IOTLB. However, section_index is only unique inside single AddressSpace. If address space translation is over IOMMUMemoryRegion, it could return section from other AddressSpace. 'iotlb_to_section()' API only finds the sections from CPU's AddressSpace so that it couldn't find section in other AddressSpace. Thus, using 'iotlb_to_section()' API will find the wrong section and QEMU will have wrong load/store access. To fix this bug of iotlb_to_section(), store complete MemoryRegionSection pointer in CPUTLBEntryFull to replace the section_index in xlat_section. Rename 'xlat_section' to 'xlat_offset' as we remove last 12 bits section_index inside. Also, since we directly use section pointer in the CPUTLBEntryFull (full->section), we can remove the unused functions: iotlb_to_section(), memory_region_section_get_iotlb(). This bug occurs only when (1) IOMMUMemoryRegion is in the path of CPU access. (2) IOMMUMemoryRegion returns different target_as and the section is in the IO region. This patch incorporates prerequisite changes from upstream commit 94c6e9cf0440 ("accel/tcg: Send the CPUTLBEntryFull struct into io_prepare()") needed for the fix to apply cleanly. Upstream-Status: Backport [https://gitlab.com/qemu-project/qemu/-/commit/854cd16e318eed12de2995014b28d9f374c64bf7] Signed-off-by: Jim Shu Reviewed-by: Philippe Mathieu-Daudé Tested-by: Mark Burton Reviewed-by: Pierrick Bouvier Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Quan Sun --- accel/tcg/cputlb.c | 32 +++++++++++++++----------------- include/accel/tcg/iommu.h | 15 --------------- include/exec/cputlb.h | 4 ++-- include/hw/core/cpu.h | 17 +++++++++-------- system/physmem.c | 25 ------------------------- 5 files changed, 26 insertions(+), 67 deletions(-) diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index fd1606c85..fa0f4d8b3 100644 --- a/accel/tcg/cputlb.c +++ b/accel/tcg/cputlb.c @@ -1089,7 +1089,7 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx, } } else { /* I/O or ROMD */ - iotlb = memory_region_section_get_iotlb(cpu, section) + xlat; + iotlb = xlat; /* * Writes to romd devices must go through MMIO to enable write. * Reads to romd devices go through the ram_ptr found above, @@ -1140,10 +1140,9 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx, /* * When memory region is ram, iotlb contains a TARGET_PAGE_BITS * aligned ram_addr_t of the page base of the target RAM. - * Otherwise, iotlb contains - * - a physical section number in the lower TARGET_PAGE_BITS - * - the offset within section->mr of the page base (I/O, ROMD) with the - * TARGET_PAGE_BITS masked off. + * Otherwise, iotlb contains a TARGET_PAGE_BITS aligned + * offset within section->mr of the page base (I/O, ROMD) + * * We subtract addr_page (which is page aligned and thus won't * disturb the low bits) to give an offset which can be added to the * (non-page-aligned) vaddr of the eventual memory access to get @@ -1153,7 +1152,8 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx, */ desc->fulltlb[index] = *full; full = &desc->fulltlb[index]; - full->xlat_section = iotlb - addr_page; + full->xlat_offset = iotlb - addr_page; + full->section = section; full->phys_addr = paddr_page; /* Now calculate the new entry */ @@ -1269,14 +1269,14 @@ static inline void cpu_unaligned_access(CPUState *cpu, vaddr addr, } static MemoryRegionSection * -io_prepare(hwaddr *out_offset, CPUState *cpu, hwaddr xlat, +io_prepare(hwaddr *out_offset, CPUState *cpu, CPUTLBEntryFull *full, MemTxAttrs attrs, vaddr addr, uintptr_t retaddr) { MemoryRegionSection *section; hwaddr mr_offset; - section = iotlb_to_section(cpu, xlat, attrs); - mr_offset = (xlat & TARGET_PAGE_MASK) + addr; + section = full->section; + mr_offset = full->xlat_offset + addr; cpu->mem_io_pc = retaddr; if (!cpu->neg.can_do_io) { cpu_io_recompile(cpu, retaddr); @@ -1335,7 +1335,7 @@ static bool victim_tlb_hit(CPUState *cpu, size_t mmu_idx, size_t index, static void notdirty_write(CPUState *cpu, vaddr mem_vaddr, unsigned size, CPUTLBEntryFull *full, uintptr_t retaddr) { - ram_addr_t ram_addr = mem_vaddr + full->xlat_section; + ram_addr_t ram_addr = mem_vaddr + full->xlat_offset; trace_memory_notdirty_write_access(mem_vaddr, ram_addr, size); @@ -1592,9 +1592,7 @@ bool tlb_plugin_lookup(CPUState *cpu, vaddr addr, int mmu_idx, /* We must have an iotlb entry for MMIO */ if (tlb_addr & TLB_MMIO) { - MemoryRegionSection *section = - iotlb_to_section(cpu, full->xlat_section & ~TARGET_PAGE_MASK, - full->attrs); + MemoryRegionSection *section = full->section; data->is_io = true; data->mr = section->mr; } else { @@ -1980,7 +1978,7 @@ static uint64_t do_ld_mmio_beN(CPUState *cpu, CPUTLBEntryFull *full, tcg_debug_assert(size > 0 && size <= 8); attrs = full->attrs; - section = io_prepare(&mr_offset, cpu, full->xlat_section, attrs, addr, ra); + section = io_prepare(&mr_offset, cpu, full, attrs, addr, ra); mr = section->mr; BQL_LOCK_GUARD(); @@ -2001,7 +1999,7 @@ static Int128 do_ld16_mmio_beN(CPUState *cpu, CPUTLBEntryFull *full, tcg_debug_assert(size > 8 && size <= 16); attrs = full->attrs; - section = io_prepare(&mr_offset, cpu, full->xlat_section, attrs, addr, ra); + section = io_prepare(&mr_offset, cpu, full, attrs, addr, ra); mr = section->mr; BQL_LOCK_GUARD(); @@ -2521,7 +2519,7 @@ static uint64_t do_st_mmio_leN(CPUState *cpu, CPUTLBEntryFull *full, tcg_debug_assert(size > 0 && size <= 8); attrs = full->attrs; - section = io_prepare(&mr_offset, cpu, full->xlat_section, attrs, addr, ra); + section = io_prepare(&mr_offset, cpu, full, attrs, addr, ra); mr = section->mr; BQL_LOCK_GUARD(); @@ -2541,7 +2539,7 @@ static uint64_t do_st16_mmio_leN(CPUState *cpu, CPUTLBEntryFull *full, tcg_debug_assert(size > 8 && size <= 16); attrs = full->attrs; - section = io_prepare(&mr_offset, cpu, full->xlat_section, attrs, addr, ra); + section = io_prepare(&mr_offset, cpu, full, attrs, addr, ra); mr = section->mr; BQL_LOCK_GUARD(); diff --git a/include/accel/tcg/iommu.h b/include/accel/tcg/iommu.h index 90cfd6c0e..547f8ea0e 100644 --- a/include/accel/tcg/iommu.h +++ b/include/accel/tcg/iommu.h @@ -14,18 +14,6 @@ #include "exec/hwaddr.h" #include "exec/memattrs.h" -/** - * iotlb_to_section: - * @cpu: CPU performing the access - * @index: TCG CPU IOTLB entry - * - * Given a TCG CPU IOTLB entry, return the MemoryRegionSection that - * it refers to. @index will have been initially created and returned - * by memory_region_section_get_iotlb(). - */ -MemoryRegionSection *iotlb_to_section(CPUState *cpu, - hwaddr index, MemTxAttrs attrs); - MemoryRegionSection *address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr addr, @@ -34,8 +22,5 @@ MemoryRegionSection *address_space_translate_for_iotlb(CPUState *cpu, MemTxAttrs attrs, int *prot); -hwaddr memory_region_section_get_iotlb(CPUState *cpu, - MemoryRegionSection *section); - #endif diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h index 9bec0e789..16f866990 100644 --- a/include/exec/cputlb.h +++ b/include/exec/cputlb.h @@ -43,8 +43,8 @@ void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length); * @full: the details of the tlb entry * * Add an entry to @cpu tlb index @mmu_idx. All of the fields of - * @full must be filled, except for xlat_section, and constitute - * the complete description of the translated page. + * @full must be filled, except for xlat_offset & section, and + * constitute the complete description of the translated page. * * This is generally called by the target tlb_fill function after * having performed a successful page table walk to find the physical diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 961505177..a3db3f66f 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -214,15 +214,16 @@ typedef uint32_t MMUIdxMap; */ struct CPUTLBEntryFull { /* - * @xlat_section contains: - * - in the lower TARGET_PAGE_BITS, a physical section number - * - with the lower TARGET_PAGE_BITS masked off, an offset which - * must be added to the virtual address to obtain: - * + the ram_addr_t of the target RAM (if the physical section - * number is PHYS_SECTION_NOTDIRTY or PHYS_SECTION_ROM) - * + the offset within the target MemoryRegion (otherwise) + * @xlat_offset: TARGET_PAGE_BITS aligned offset which must be added to + * the virtual address to obtain: + * + the ram_addr_t of the target RAM (if the physical section + * number is PHYS_SECTION_NOTDIRTY or PHYS_SECTION_ROM) + * + the offset within the target MemoryRegion (otherwise) */ - hwaddr xlat_section; + hwaddr xlat_offset; + + /* @section contains physical section. */ + MemoryRegionSection *section; /* * @phys_addr contains the physical address in the address space diff --git a/system/physmem.c b/system/physmem.c index c9869e404..a21e7ca64 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -748,31 +748,6 @@ translate_fail: return &d->map.sections[PHYS_SECTION_UNASSIGNED]; } -MemoryRegionSection *iotlb_to_section(CPUState *cpu, - hwaddr index, MemTxAttrs attrs) -{ - int asidx = cpu_asidx_from_attrs(cpu, attrs); - CPUAddressSpace *cpuas = &cpu->cpu_ases[asidx]; - AddressSpaceDispatch *d = address_space_to_dispatch(cpuas->as); - int section_index = index & ~TARGET_PAGE_MASK; - MemoryRegionSection *ret; - - assert(section_index < d->map.sections_nb); - ret = d->map.sections + section_index; - assert(ret->mr); - assert(ret->mr->ops); - - return ret; -} - -/* Called from RCU critical section */ -hwaddr memory_region_section_get_iotlb(CPUState *cpu, - MemoryRegionSection *section) -{ - AddressSpaceDispatch *d = flatview_to_dispatch(section->fv); - return section - d->map.sections; -} - #endif /* CONFIG_TCG */ void cpu_address_space_init(CPUState *cpu, int asidx, -- 2.43.0