Peter Maydell <peter.mayd...@linaro.org> writes:
> 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 ?
Aren't hwaddr's unique across the bus? Or is this because you would have
two address buses (secure and non-secure) with different address lines to
different chips?
But surely we have all the information we need because we've hooked the
two things that QEMU's softmmu code knows. The mmu_idx and the vaddr
with which the slow path can figure out what it needs.
>> 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?
Hmm maybe.. I guess it's a special case of device offset. Do you want to
drop this part for now?
>
> thanks
> -- PMM
--
Alex Bennée
