'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' 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.
Common IOMMU devices don't have this issue since they are only in the
path of DMA access. Currently, the bug only occurs when ARM MPC device
(hw/misc/tz-mpc.c) returns 'blocked_io_as' to emulate blocked access
handling. Upcoming RISC-V wgChecker [1] and IOPMP [2] devices are also
affected by this bug.
[1] RISC-V WG:
https://patchew.org/QEMU/20251021155548.584543-1-jim.shu@sifive.com/
[2] RISC-V IOPMP:
https://patchew.org/QEMU/20250312093735.1517740-1-ethan84@andestech.com/
Signed-off-by: Jim Shu <jim.shu@sifive.com>
---
accel/tcg/cputlb.c | 32 +++++++++++++++-----------------
include/accel/tcg/iommu.h | 15 ---------------
include/exec/cputlb.h | 2 +-
include/hw/core/cpu.h | 12 +++++++-----
system/physmem.c | 25 -------------------------
5 files changed, 23 insertions(+), 63 deletions(-)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 6900a126827..c61339d10a3 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1090,7 +1090,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,
@@ -1141,10 +1141,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
@@ -1154,7 +1153,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 = iotlb - addr_page;
+ full->section = section;
full->phys_addr = paddr_page;
/* Now calculate the new entry */
@@ -1270,14 +1270,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 + addr;
cpu->mem_io_pc = retaddr;
if (!cpu->neg.can_do_io) {
cpu_io_recompile(cpu, retaddr);
@@ -1336,7 +1336,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;
trace_memory_notdirty_write_access(mem_vaddr, ram_addr, size);
@@ -1593,9 +1593,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 {
@@ -1981,7 +1979,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();
@@ -2002,7 +2000,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();
@@ -2499,7 +2497,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();
@@ -2519,7 +2517,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 90cfd6c0ed1..547f8ea0ef0 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 0d1d46429c9..e599a0f7627 100644
--- a/include/exec/cputlb.h
+++ b/include/exec/cputlb.h
@@ -44,7 +44,7 @@ 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
+ * @full must be filled, except for xlat, and constitute
* the complete description of the translated page.
*
* This is generally called by the target tlb_fill function after
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 61da2ea4331..7de576ab602 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -219,15 +219,17 @@ 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:
+ * @xlat contains:
+ * - a 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;
+
+ /* @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 b0311f45312..d17596a77fb 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -747,31 +747,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.0Hi Jim,
On 28/1/26 07:39, Jim Shu wrote:
> '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' 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.
>
> Common IOMMU devices don't have this issue since they are only in the
> path of DMA access. Currently, the bug only occurs when ARM MPC device
> (hw/misc/tz-mpc.c) returns 'blocked_io_as' to emulate blocked access
> handling. Upcoming RISC-V wgChecker [1] and IOPMP [2] devices are also
> affected by this bug.
>
> [1] RISC-V WG:
> https://patchew.org/QEMU/20251021155548.584543-1-jim.shu@sifive.com/
> [2] RISC-V IOPMP:
> https://patchew.org/QEMU/20250312093735.1517740-1-ethan84@andestech.com/
>
> Signed-off-by: Jim Shu <jim.shu@sifive.com>
> ---
> accel/tcg/cputlb.c | 32 +++++++++++++++-----------------
> include/accel/tcg/iommu.h | 15 ---------------
> include/exec/cputlb.h | 2 +-
> include/hw/core/cpu.h | 12 +++++++-----
> system/physmem.c | 25 -------------------------
> 5 files changed, 23 insertions(+), 63 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 6900a126827..c61339d10a3 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1090,7 +1090,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,
> @@ -1141,10 +1141,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
> @@ -1154,7 +1153,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 = iotlb - addr_page;
> + full->section = section;
> full->phys_addr = paddr_page;
>
> /* Now calculate the new entry */
> @@ -1270,14 +1270,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)
Can you move the io_prepare() argument change in a preliminary patch
to make this one simpler to review?
> {
> 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 + addr;
> cpu->mem_io_pc = retaddr;
> if (!cpu->neg.can_do_io) {
> cpu_io_recompile(cpu, retaddr);
> @@ -1336,7 +1336,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;
>
> trace_memory_notdirty_write_access(mem_vaddr, ram_addr, size);
>
> @@ -1593,9 +1593,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 {
> @@ -1981,7 +1979,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();
> @@ -2002,7 +2000,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();
> @@ -2499,7 +2497,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();
> @@ -2519,7 +2517,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 90cfd6c0ed1..547f8ea0ef0 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 0d1d46429c9..e599a0f7627 100644
> --- a/include/exec/cputlb.h
> +++ b/include/exec/cputlb.h
> @@ -44,7 +44,7 @@ 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
> + * @full must be filled, except for xlat, and constitute
> * the complete description of the translated page.
> *
> * This is generally called by the target tlb_fill function after
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 61da2ea4331..7de576ab602 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -219,15 +219,17 @@ 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:
> + * @xlat contains:
> + * - a TARGET_PAGE_BITS aligned offset which must be added to
Drop "contains":
* @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;
I'd rename as 'xlat_offset' instead for clarity.
> +
> + /* @section contains physical section. */
> + MemoryRegionSection *section;
Good. Safe because 'the CPUIOTLBEntry layout is not as critical as that
of the CPUTLBEntry.' (commit e469b22ffda).
>
> /*
> * @phys_addr contains the physical address in the address space
> diff --git a/system/physmem.c b/system/physmem.c
> index b0311f45312..d17596a77fb 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -747,31 +747,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 */
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Hi Philippe,
Thanks for reviewing it!
On Wed, Jan 28, 2026 at 4:08 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Hi Jim,
>
> On 28/1/26 07:39, Jim Shu wrote:
> > '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' 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.
> >
> > Common IOMMU devices don't have this issue since they are only in the
> > path of DMA access. Currently, the bug only occurs when ARM MPC device
> > (hw/misc/tz-mpc.c) returns 'blocked_io_as' to emulate blocked access
> > handling. Upcoming RISC-V wgChecker [1] and IOPMP [2] devices are also
> > affected by this bug.
> >
> > [1] RISC-V WG:
> > https://patchew.org/QEMU/20251021155548.584543-1-jim.shu@sifive.com/
> > [2] RISC-V IOPMP:
> > https://patchew.org/QEMU/20250312093735.1517740-1-ethan84@andestech.com/
> >
> > Signed-off-by: Jim Shu <jim.shu@sifive.com>
> > ---
> > accel/tcg/cputlb.c | 32 +++++++++++++++-----------------
> > include/accel/tcg/iommu.h | 15 ---------------
> > include/exec/cputlb.h | 2 +-
> > include/hw/core/cpu.h | 12 +++++++-----
> > system/physmem.c | 25 -------------------------
> > 5 files changed, 23 insertions(+), 63 deletions(-)
> >
> > diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> > index 6900a126827..c61339d10a3 100644
> > --- a/accel/tcg/cputlb.c
> > +++ b/accel/tcg/cputlb.c
> > @@ -1090,7 +1090,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,
> > @@ -1141,10 +1141,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
> > @@ -1154,7 +1153,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 = iotlb - addr_page;
> > + full->section = section;
> > full->phys_addr = paddr_page;
> >
> > /* Now calculate the new entry */
> > @@ -1270,14 +1270,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)
>
> Can you move the io_prepare() argument change in a preliminary patch
> to make this one simpler to review?
I will fix it in the v2 patch.
>
> > {
> > 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 + addr;
> > cpu->mem_io_pc = retaddr;
> > if (!cpu->neg.can_do_io) {
> > cpu_io_recompile(cpu, retaddr);
> > @@ -1336,7 +1336,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;
> >
> > trace_memory_notdirty_write_access(mem_vaddr, ram_addr, size);
> >
> > @@ -1593,9 +1593,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 {
> > @@ -1981,7 +1979,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();
> > @@ -2002,7 +2000,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();
> > @@ -2499,7 +2497,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();
> > @@ -2519,7 +2517,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 90cfd6c0ed1..547f8ea0ef0 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 0d1d46429c9..e599a0f7627 100644
> > --- a/include/exec/cputlb.h
> > +++ b/include/exec/cputlb.h
> > @@ -44,7 +44,7 @@ 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
> > + * @full must be filled, except for xlat, and constitute
> > * the complete description of the translated page.
> > *
> > * This is generally called by the target tlb_fill function after
> > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > index 61da2ea4331..7de576ab602 100644
> > --- a/include/hw/core/cpu.h
> > +++ b/include/hw/core/cpu.h
> > @@ -219,15 +219,17 @@ 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:
> > + * @xlat contains:
> > + * - a TARGET_PAGE_BITS aligned offset which must be added to
>
> Drop "contains":
I will fix it in the v2 patch.
>
> * @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;
>
> I'd rename as 'xlat_offset' instead for clarity.
I will rename it in the v2 patch.
>
> > +
> > + /* @section contains physical section. */
> > + MemoryRegionSection *section;
>
> Good. Safe because 'the CPUIOTLBEntry layout is not as critical as that
> of the CPUTLBEntry.' (commit e469b22ffda).
>
> >
> > /*
> > * @phys_addr contains the physical address in the address space
> > diff --git a/system/physmem.c b/system/physmem.c
> > index b0311f45312..d17596a77fb 100644
> > --- a/system/physmem.c
> > +++ b/system/physmem.c
> > @@ -747,31 +747,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 */
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
Glad to see I’m not alone seeing this bug :-)
And THANKS for the fix Jim,
My idea had been to carry the address space in the fitly full entry, but I like your plan even more, it removes even more complexity.
I’ve tested in it my setup, and it works,
Cheers
Mark.
> On 28 Jan 2026, at 09:08, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
>
> Hi Jim,
>
> On 28/1/26 07:39, Jim Shu wrote:
>> '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' 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.
>>
>> Common IOMMU devices don't have this issue since they are only in the
>> path of DMA access. Currently, the bug only occurs when ARM MPC device
>> (hw/misc/tz-mpc.c) returns 'blocked_io_as' to emulate blocked access
>> handling. Upcoming RISC-V wgChecker [1] and IOPMP [2] devices are also
>> affected by this bug.
>>
>> [1] RISC-V WG:
>> https://patchew.org/QEMU/20251021155548.584543-1-jim.shu@sifive.com/
>> [2] RISC-V IOPMP:
>> https://patchew.org/QEMU/20250312093735.1517740-1-ethan84@andestech.com/
>>
>> Signed-off-by: Jim Shu <jim.shu@sifive.com>
>> ---
>> accel/tcg/cputlb.c | 32 +++++++++++++++-----------------
>> include/accel/tcg/iommu.h | 15 ---------------
>> include/exec/cputlb.h | 2 +-
>> include/hw/core/cpu.h | 12 +++++++-----
>> system/physmem.c | 25 -------------------------
>> 5 files changed, 23 insertions(+), 63 deletions(-)
>>
>> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
>> index 6900a126827..c61339d10a3 100644
>> --- a/accel/tcg/cputlb.c
>> +++ b/accel/tcg/cputlb.c
>> @@ -1090,7 +1090,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,
>> @@ -1141,10 +1141,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
>> @@ -1154,7 +1153,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 = iotlb - addr_page;
>> + full->section = section;
>> full->phys_addr = paddr_page;
>>
>> /* Now calculate the new entry */
>> @@ -1270,14 +1270,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)
>
> Can you move the io_prepare() argument change in a preliminary patch
> to make this one simpler to review?
>
>> {
>> 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 + addr;
>> cpu->mem_io_pc = retaddr;
>> if (!cpu->neg.can_do_io) {
>> cpu_io_recompile(cpu, retaddr);
>> @@ -1336,7 +1336,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;
>>
>> trace_memory_notdirty_write_access(mem_vaddr, ram_addr, size);
>>
>> @@ -1593,9 +1593,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 {
>> @@ -1981,7 +1979,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();
>> @@ -2002,7 +2000,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();
>> @@ -2499,7 +2497,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();
>> @@ -2519,7 +2517,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 90cfd6c0ed1..547f8ea0ef0 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 0d1d46429c9..e599a0f7627 100644
>> --- a/include/exec/cputlb.h
>> +++ b/include/exec/cputlb.h
>> @@ -44,7 +44,7 @@ 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
>> + * @full must be filled, except for xlat, and constitute
>> * the complete description of the translated page.
>> *
>> * This is generally called by the target tlb_fill function after
>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>> index 61da2ea4331..7de576ab602 100644
>> --- a/include/hw/core/cpu.h
>> +++ b/include/hw/core/cpu.h
>> @@ -219,15 +219,17 @@ 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:
>> + * @xlat contains:
>> + * - a TARGET_PAGE_BITS aligned offset which must be added to
>
> Drop "contains":
>
> * @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;
>
> I'd rename as 'xlat_offset' instead for clarity.
>
>> +
>> + /* @section contains physical section. */
>> + MemoryRegionSection *section;
>
> Good. Safe because 'the CPUIOTLBEntry layout is not as critical as that
> of the CPUTLBEntry.' (commit e469b22ffda).
>
>>
>> /*
>> * @phys_addr contains the physical address in the address space
>> diff --git a/system/physmem.c b/system/physmem.c
>> index b0311f45312..d17596a77fb 100644
>> --- a/system/physmem.c
>> +++ b/system/physmem.c
>> @@ -747,31 +747,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 */
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
© 2016 - 2026 Red Hat, Inc.