Re: [PATCH v1 5/9] cputlb: ensure we re-fill the TLB if it has reset

[Richard Henderson]

On 6/2/20 8:46 AM, Alex Bennée wrote:
> Any write to a device might cause a re-arrangement of memory
> triggering a TLB flush and potential re-size of the TLB invalidating
> previous entries. This would cause users of qemu_plugin_get_hwaddr()
> to see the warning:
> 
>   invalid use of qemu_plugin_get_hwaddr
> 
> because of the failed tlb_lookup which should always succeed. We catch
> this case by checking to see if the list of entries has been cleared
> and if so triggering a re-fill.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  accel/tcg/cputlb.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index eb2cf9de5e6..b7d329f7155 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1091,6 +1091,20 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry 
> *iotlbentry,
>                                 MMU_DATA_STORE, mmu_idx, iotlbentry->attrs, r,
>                                 retaddr);
>      }
> +
> +    /*
> +     * The memory_region_dispatch may have triggered a flush/resize
> +     * so for plugins we need to ensure we have reset the tlb_entry
> +     * so any later lookup is correct.
> +     */
> +#ifdef CONFIG_PLUGIN
> +    if (env_tlb(env)->d[mmu_idx].n_used_entries == 0) {
> +        int size = op & MO_SIZE;
> +        tlb_fill(env_cpu(env), addr, size, MMU_DATA_STORE,
> +                 mmu_idx, retaddr);

Ouch.  What if the target has a soft tlb fill, so this requires a call into the
OS, so this fill actually raises another exception?  This will not be happy fun
making.

I recall I had objections to recording this translation, saying that "we can
always get it back again".  Clearly I was wrong, and we should just preserve
the required CPUTLBEntry details before they're lost by a device.


r~