| Message ID | 20210312172821.31647-12-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | plugins/next (phys addr, syscalls, lots of docs) | expand |
A few clarifications inline: On Mar 12 17:28, Alex Bennée wrote: > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > include/qemu/qemu-plugin.h | 52 ++++++++++++++++++++++++++++++++++++-- > 1 file changed, 50 insertions(+), 2 deletions(-) > > diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h > index dc05fc1932..d4adce730a 100644 > --- a/include/qemu/qemu-plugin.h > +++ b/include/qemu/qemu-plugin.h > @@ -327,21 +327,69 @@ void qemu_plugin_register_vcpu_insn_exec_inline(struct qemu_plugin_insn *insn, > enum qemu_plugin_op op, > void *ptr, uint64_t imm); > > -/* > - * Helpers to query information about the instructions in a block > +/** > + * qemu_plugin_tb_n_insns() - query helper for number of insns in TB > + * @tb: opaque handle to TB passed to callback > + * > + * Returns: number of instructions in this block > */ > size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb); > > +/** > + * qemu_plugin_tb_vaddr() - query helper for vaddr of TB start > + * @tb: opaque handle to TB passed to callback > + * > + * Returns: virtual address of block start > + */ > uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb); > > +/** > + * qemu_plugin_tb_get_insn() - retrieve handle for instruction > + * @tb: opaque handle to TB passed to callback > + * @idx: instruction number, 0 indexed > + * > + * The returned handle can be used in follow up helper queries as well > + * as when instrumenting an instruction. It is only valid for the > + * lifetime of the callback. > + * > + * Returns: opaque handle to instruction > + */ > struct qemu_plugin_insn * > qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx); > > +/** > + * qemu_plugin_insn_data() - return ptr to instruction data > + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn() > + * > + * Note: data is only valid for duration of callback. See > + * qemu_plugin_insn_size() to calculate size of stream. > + * > + * Returns: pointer to a stream of bytes Maybe this could be a little more explicit, something like "Returns: pointer to a stream of bytes containing the value of this instruction's opcode"? > + */ > const void *qemu_plugin_insn_data(const struct qemu_plugin_insn *insn); > > +/** > + * qemu_plugin_insn_size() - return size of instruction > + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn() > + * > + * Returns: size of instruction size in bytes? > + */ > size_t qemu_plugin_insn_size(const struct qemu_plugin_insn *insn); > > +/** > + * qemu_plugin_insn_vaddr() - return vaddr of instruction > + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn() > + * > + * Returns: virtual address of instruction > + */ > uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn); > + > +/** > + * qemu_plugin_insn_haddr() - return vaddr of instruction Copypasta: s/vaddr/haddr/ ? > + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn() > + * > + * Returns: hardware (physical) address of instruction > + */ > void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn); Is this the physical address of the instruction on the host or target? -Aaron
Aaron Lindsay <aaron@os.amperecomputing.com> writes: > A few clarifications inline: > > On Mar 12 17:28, Alex Bennée wrote: >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> include/qemu/qemu-plugin.h | 52 ++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 50 insertions(+), 2 deletions(-) >> >> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h >> index dc05fc1932..d4adce730a 100644 >> --- a/include/qemu/qemu-plugin.h >> +++ b/include/qemu/qemu-plugin.h >> @@ -327,21 +327,69 @@ void qemu_plugin_register_vcpu_insn_exec_inline(struct qemu_plugin_insn *insn, >> enum qemu_plugin_op op, >> void *ptr, uint64_t imm); >> >> -/* >> - * Helpers to query information about the instructions in a block >> +/** >> + * qemu_plugin_tb_n_insns() - query helper for number of insns in TB >> + * @tb: opaque handle to TB passed to callback >> + * >> + * Returns: number of instructions in this block >> */ >> size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb); >> >> +/** >> + * qemu_plugin_tb_vaddr() - query helper for vaddr of TB start >> + * @tb: opaque handle to TB passed to callback >> + * >> + * Returns: virtual address of block start >> + */ >> uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb); >> >> +/** >> + * qemu_plugin_tb_get_insn() - retrieve handle for instruction >> + * @tb: opaque handle to TB passed to callback >> + * @idx: instruction number, 0 indexed >> + * >> + * The returned handle can be used in follow up helper queries as well >> + * as when instrumenting an instruction. It is only valid for the >> + * lifetime of the callback. >> + * >> + * Returns: opaque handle to instruction >> + */ >> struct qemu_plugin_insn * >> qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx); >> >> +/** >> + * qemu_plugin_insn_data() - return ptr to instruction data >> + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn() >> + * >> + * Note: data is only valid for duration of callback. See >> + * qemu_plugin_insn_size() to calculate size of stream. >> + * >> + * Returns: pointer to a stream of bytes > > Maybe this could be a little more explicit, something like "Returns: > pointer to a stream of bytes containing the value of this instruction's > opcode"? > >> + */ >> const void *qemu_plugin_insn_data(const struct qemu_plugin_insn *insn); >> >> +/** >> + * qemu_plugin_insn_size() - return size of instruction >> + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn() >> + * >> + * Returns: size of instruction > > size in bytes? > >> + */ >> size_t qemu_plugin_insn_size(const struct qemu_plugin_insn *insn); >> >> +/** >> + * qemu_plugin_insn_vaddr() - return vaddr of instruction >> + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn() >> + * >> + * Returns: virtual address of instruction >> + */ >> uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn); >> + >> +/** >> + * qemu_plugin_insn_haddr() - return vaddr of instruction > > Copypasta: s/vaddr/haddr/ ? > >> + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn() >> + * >> + * Returns: hardware (physical) address of instruction >> + */ >> void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn); > > Is this the physical address of the instruction on the host or target? target. > > -Aaron -- Alex Bennée
On Mar 16 13:48, Alex Bennée wrote: > Aaron Lindsay <aaron@os.amperecomputing.com> writes: > > On Mar 12 17:28, Alex Bennée wrote: > >> + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn() > >> + * > >> + * Returns: hardware (physical) address of instruction > >> + */ > >> void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn); > > > > Is this the physical address of the instruction on the host or target? > > target. An observation: We're exposing a target physical address here as a void * and for memory accesses (qemu_plugin_hwaddr_phys_addr) as a uint64_t. -Aaron
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h index dc05fc1932..d4adce730a 100644 --- a/include/qemu/qemu-plugin.h +++ b/include/qemu/qemu-plugin.h @@ -327,21 +327,69 @@ void qemu_plugin_register_vcpu_insn_exec_inline(struct qemu_plugin_insn *insn, enum qemu_plugin_op op, void *ptr, uint64_t imm); -/* - * Helpers to query information about the instructions in a block +/** + * qemu_plugin_tb_n_insns() - query helper for number of insns in TB + * @tb: opaque handle to TB passed to callback + * + * Returns: number of instructions in this block */ size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb); +/** + * qemu_plugin_tb_vaddr() - query helper for vaddr of TB start + * @tb: opaque handle to TB passed to callback + * + * Returns: virtual address of block start + */ uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb); +/** + * qemu_plugin_tb_get_insn() - retrieve handle for instruction + * @tb: opaque handle to TB passed to callback + * @idx: instruction number, 0 indexed + * + * The returned handle can be used in follow up helper queries as well + * as when instrumenting an instruction. It is only valid for the + * lifetime of the callback. + * + * Returns: opaque handle to instruction + */ struct qemu_plugin_insn * qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx); +/** + * qemu_plugin_insn_data() - return ptr to instruction data + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn() + * + * Note: data is only valid for duration of callback. See + * qemu_plugin_insn_size() to calculate size of stream. + * + * Returns: pointer to a stream of bytes + */ const void *qemu_plugin_insn_data(const struct qemu_plugin_insn *insn); +/** + * qemu_plugin_insn_size() - return size of instruction + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn() + * + * Returns: size of instruction + */ size_t qemu_plugin_insn_size(const struct qemu_plugin_insn *insn); +/** + * qemu_plugin_insn_vaddr() - return vaddr of instruction + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn() + * + * Returns: virtual address of instruction + */ uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn); + +/** + * qemu_plugin_insn_haddr() - return vaddr of instruction + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn() + * + * Returns: hardware (physical) address of instruction + */ void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn); /*
Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- include/qemu/qemu-plugin.h | 52 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) -- 2.20.1