On Mon, 14 Oct 2019 at 12:25, Alex Bennée <alex.ben...@linaro.org> wrote:
>
> We need to keep a local per-cpu copy of the data as other threads may
> be running. We use a automatically growing array and re-use the space
> for subsequent queries.
>
> +#ifdef CONFIG_SOFTMMU
> +static __thread struct qemu_plugin_hwaddr hwaddr_info;
> +
> +struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
> + uint64_t vaddr)
> +{
> + CPUState *cpu = current_cpu;
> + unsigned int mmu_idx = info >> TRACE_MEM_MMU_SHIFT;
> +
> + if (!tlb_plugin_lookup(cpu, vaddr, mmu_idx,
> + info & TRACE_MEM_ST, &hwaddr_info)) {
> + error_report("invalid use of qemu_plugin_get_hwaddr");
> + return NULL;
> + }
> +
> + return &hwaddr_info;
> +}
Apologies for dropping into the middle of this patchset, but
this API looks a bit odd. A hwaddr alone isn't a complete
definition of an access -- you need an (AddressSpace, hwaddr)
tuple for that. So this API looks like it doesn't really cope
with things like TrustZone ?
> uint64_t qemu_plugin_hwaddr_to_raddr(const struct qemu_plugin_hwaddr *haddr)
> {
> +#ifdef CONFIG_SOFTMMU
> + ram_addr_t ram_addr = 0;
> +
> + if (haddr && !haddr->is_io) {
> + ram_addr = qemu_ram_addr_from_host((void *) haddr->hostaddr);
> + if (ram_addr == RAM_ADDR_INVALID) {
> + error_report("Bad ram pointer %"PRIx64"", haddr->hostaddr);
> + abort();
> + }
> + }
> + return ram_addr;
> +#else
> return 0;
> +#endif
> }
This looks odd to see in the plugin API -- ramaddrs should
be a QEMU internal concept, shouldn't they?
thanks
-- PMM
