Aaron Lindsay <aa...@os.amperecomputing.com> writes:
> On Mar 02 16:06, Alex Bennée wrote:
>>
>> Aaron Lindsay <aa...@os.amperecomputing.com> writes:
>>
>> > On Feb 23 15:53, Aaron Lindsay wrote:
>> >> On Feb 22 15:48, Aaron Lindsay wrote:
>> >> > On Feb 22 19:30, Alex Bennée wrote:
>> >> > > Aaron Lindsay <aa...@os.amperecomputing.com> writes:
>> >> > > That said I think we could add an additional helper to translate a
>> >> > > hwaddr to a global address space address. I'm open to suggestions of
>> >> > > the
>> >> > > best way to structure this.
>> >> >
>> >> > Haven't put a ton of thought into it, but what about something like this
>> >> > (untested):
>> >> >
>> >> > uint64_t qemu_plugin_hwaddr_phys_addr(const struct qemu_plugin_hwaddr
>> >> > *haddr)
>> >> > {
>> >> > #ifdef CONFIG_SOFTMMU
>> >> > if (haddr) {
>> >> > if (!haddr->is_io) {
>> >> > RAMBlock *block;
>> >> > ram_addr_t offset;
>> >> >
>> >> > block = qemu_ram_block_from_host((void *)
>> >> > haddr->v.ram.hostaddr, false, &offset);
>> >> > if (!block) {
>> >> > error_report("Bad ram pointer %"PRIx64"",
>> >> > haddr->v.ram.hostaddr);
>> >> > abort();
>> >> > }
>> >> >
>> >> > return block->offset + offset + block->mr->addr;
>> >> > } else {
>> >> > MemoryRegionSection *mrs = haddr->v.io.section;
>> >> > return haddr->v.io.offset + mrs->mr->addr;
>> >> > }
>> >> > }
>> >> > #endif
>> >> > return 0;
>> >> > }
>> >>
>> >> This appears to successfully return correct physical addresses for RAM
>> >> at least, though I've not tested it thoroughly for MMIO yet.
>> >>
>> >> If it ends up being desirable based on the discussion elsewhere on this
>> >> thread I am willing to perform more complete testing, turn this into a
>> >> patch, and submit it.
>> >
>> > Ping - Is this something worth me pursuing?
>>
>> Yes please.
>
> Okay, I'll work on it. Is your thinking that this would this be a
> separate call as shown above, or a replacement of the existing
> qemu_plugin_hwaddr_device_offset function? And, if a replacement, should
> we keep the name similar to retain compatibility, or make a clean break?
>
> It seemed like Peter may have been saying the device offset shouldn't be
> exposed at all (leading me to consider full replacement), but I also
> don't see a definitive resolution of that conversation.
I think a full replacement and an increment of the minimum API version.
That way people will at least query why the plugin failed to load and
hopefully will read the release notes ;-)
>
> -Aaron
--
Alex Bennée
