On Mon, Jul 13, 2020 at 21:04:10 +0100, 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. To
> prevent this we save the IOTLB data in case it is later needed by a
> plugin doing a lookup.
>
> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
(snip)
> +/*
> + * Save a potentially trashed IOTLB entry for later lookup by plugin.
> + *
> + * We also need to track the thread storage address because the RCU
> + * cleanup that runs when we leave the critical region (the current
> + * execution) is actually in a different thread.
As I mentioned in the previous iteration of this series, this comment is
outdated -- there is no thread storage nor RCU to worry about here.
> + * This almost never fails as the memory access being instrumented
> + * should have just filled the TLB. The one corner case is io_writex
> + * which can cause TLB flushes and potential resizing of the TLBs
> + * loosing the information we need. In those cases we need to recover
> + * data from a copy of the io_tlb entry.
> */
s/loosing/losing/
About the approach in this patch: it works as long as the caller is in
the same vCPU thread, otherwise we'd need a seqlock to avoid races
between readers and the writing vCPU. I see that qemu_plugin_get_hwaddr
does not even take a vCPU index, so this should be OK -- as long as this
is called only from a mem callback, it's in the same vCPU thread and it's
therefore safe.
With the above comments fixed,
Reviewed-by: Emilio G. Cota <c...@braap.org>
Thanks,
Emilio
