Philippe Mathieu-Daudé <f4...@amsat.org> writes:
> Refactor debug code as tlb_assert_iotlb_entry_for_ptr_present() helper.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> ---
> What this code does is out of my league, but refactoring it allow
> keeping tlb_addr_write() local to accel/tcg/cputlb.c in the next
> patch.
The assertion that the table entry is current is just a simple
housekeeping one. The details of how the MTE implementation uses
(abuses?) the iotlb entries requires a closer reading of the code.
> ---
> include/exec/exec-all.h | 9 +++++++++
> accel/tcg/cputlb.c | 14 ++++++++++++++
> target/arm/mte_helper.c | 11 ++---------
> target/arm/sve_helper.c | 10 ++--------
> 4 files changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index f933c74c446..c5e8e355b7f 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -296,6 +296,15 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong
> vaddr,
> void tlb_set_page(CPUState *cpu, target_ulong vaddr,
> hwaddr paddr, int prot,
> int mmu_idx, target_ulong size);
> +
> +/*
> + * Find the iotlbentry for ptr. This *must* be present in the TLB
> + * because we just found the mapping.
> + */
> +void tlb_assert_iotlb_entry_for_ptr_present(CPUArchState *env, int
> ptr_mmu_idx,
> + uint64_t ptr,
> + MMUAccessType ptr_access,
> + uintptr_t index);
Probably worth making this an empty inline for the non CONFIG_DEBUG_TCG
case so we can eliminate the call to an empty function.
> #else
> static inline void tlb_init(CPUState *cpu)
> {
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 8a7b779270a..a6247da34a0 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -429,6 +429,20 @@ void tlb_flush_all_cpus_synced(CPUState *src_cpu)
> tlb_flush_by_mmuidx_all_cpus_synced(src_cpu, ALL_MMUIDX_BITS);
> }
>
> +void tlb_assert_iotlb_entry_for_ptr_present(CPUArchState *env, int
> ptr_mmu_idx,
> + uint64_t ptr,
> + MMUAccessType ptr_access,
> + uintptr_t index)
> +{
> +#ifdef CONFIG_DEBUG_TCG
> + CPUTLBEntry *entry = tlb_entry(env, ptr_mmu_idx, ptr);
> + target_ulong comparator = (ptr_access == MMU_DATA_LOAD
> + ? entry->addr_read
> + : tlb_addr_write(entry));
> + g_assert(tlb_hit(comparator, ptr));
> +#endif
> +}
> +
> static bool tlb_hit_page_mask_anyprot(CPUTLBEntry *tlb_entry,
> target_ulong page, target_ulong mask)
> {
> diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
> index 6cea9d1b506..f47d3b4570e 100644
> --- a/target/arm/mte_helper.c
> +++ b/target/arm/mte_helper.c
> @@ -111,15 +111,8 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int
> ptr_mmu_idx,
> * matching tlb entry + iotlb entry.
> */
> index = tlb_index(env, ptr_mmu_idx, ptr);
> -# ifdef CONFIG_DEBUG_TCG
> - {
> - CPUTLBEntry *entry = tlb_entry(env, ptr_mmu_idx, ptr);
> - target_ulong comparator = (ptr_access == MMU_DATA_LOAD
> - ? entry->addr_read
> - : tlb_addr_write(entry));
> - g_assert(tlb_hit(comparator, ptr));
> - }
> -# endif
> + tlb_assert_iotlb_entry_for_ptr_present(env, ptr_mmu_idx, ptr,
> + ptr_access, index);
> iotlbentry = &env_tlb(env)->d[ptr_mmu_idx].iotlb[index];
>
> /* If the virtual page MemAttr != Tagged, access unchecked. */
> diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
> index c8cdf7618eb..a5708da0f2f 100644
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -4089,14 +4089,8 @@ static bool sve_probe_page(SVEHostPage *info, bool
> nofault,
> {
> uintptr_t index = tlb_index(env, mmu_idx, addr);
>
> -# ifdef CONFIG_DEBUG_TCG
> - CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
> - target_ulong comparator = (access_type == MMU_DATA_LOAD
> - ? entry->addr_read
> - : tlb_addr_write(entry));
> - g_assert(tlb_hit(comparator, addr));
> -# endif
> -
> + tlb_assert_iotlb_entry_for_ptr_present(env, mmu_idx, addr,
> + access_type, index);
> CPUIOTLBEntry *iotlbentry = &env_tlb(env)->d[mmu_idx].iotlb[index];
> info->attrs = iotlbentry->attrs;
> }
with the inline fix:
Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
--
Alex Bennée
