Re: Plugin Address Translations Inconsistent/Incorrect?

[Aaron Lindsay]

On Mar 02 16:06, Alex Bennée wrote:
> 
> Aaron Lindsay <aaron@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 <aaron@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.

-Aaron