Richard Henderson writes:
> On 01/31/2014 08:10 AM, Lluís Vilanova wrote:
>> +#define ldub(p) ({ trace_guest_vmem(p, 1, 0); ldub_raw(p); })
> Are you sure you want to log these here? Uses of these macros are
> not restricted to the guest. Therefore you could wind up with e.g.
> PCI device accesses being attributed to the target cpu.
These defines are only enabled in user-level mode.
But I wrote them really long ago, and I just realized they are not
up-to-date. The changes should also cover the 'cpu_*_data' and 'cpu_*_kernel'
variants. These macros are mostly used in helpers (e.g., helper_boundl).
>> --- a/include/exec/softmmu_header.h
>> +++ b/include/exec/softmmu_header.h
>> @@ -25,6 +25,11 @@
>> * You should have received a copy of the GNU Lesser General Public
>> * License along with this library; if not, see
>> <http://www.gnu.org/licenses/>.
>> */
>> +
>> +#if !defined(TRACE_TCG_CODE_ACCESSOR)
>> +#include "trace.h"
>> +#endif
>> +
>> #if DATA_SIZE == 8
>> #define SUFFIX q
>> #define USUFFIX q
>> @@ -88,6 +93,10 @@ glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env,
>> target_ulong ptr)
>> target_ulong addr;
>> int mmu_idx;
>>
>> +#if !defined(TRACE_TCG_CODE_ACCESSOR)
>> + trace_guest_vmem(ptr, DATA_SIZE, 0);
>> +#endif
> These are going to result in double-logging the same access with
>> +#define tcg_gen_qemu_ld_i32(val, addr, idx, memop) \
>> + do { \
>> + uint8_t _memop_size = _tcg_memop_size(memop); \
>> + trace_guest_vmem_tcg(addr, _memop_size, 0); \
>> + tcg_gen_qemu_ld_i32(val, addr, idx, memop); \
>> + } while (0)
> ... these.
I don't see how 'tcg_gen_qemu_ld_i32' gets to call 'cpu_ldl_data' (for
example). I also did this long ago, so maybe it changed, but a quick look at the
code shows that in softmmu-mode, a TLB miss performs a slow path access with the
functions in "softmmu_template.h", not "softmmu_exec.h" (which includes
"softmmu_header.h").
Maybe you're referring to some case I've missed.
> Of course, those softmmu functions are also used by the system emulators in
> the
> same way the ldub macro above is used for userland emulation. So again you
> have non-target accesses being attributed to the target.
What do you mean by non-target accesses? Accesses not directly "encoded" in the
semantics of a guest instruction? If so, I did this in purpose (like the helper
example on the top).
> Also, doing this action with macros, here, seems truly backward. Why not
> simply modify the real tcg_gen_qemu_ld_i32 in tcg.c?
The 'trace_guest_vmem_tcg' function just calls a helper generator function in
"helper.h", which cannot be included in "tcg.c". Another possibility is to just
forget about using "helper.h", and instead "manually" generate the call to the
helper; but using macros seems to me it's easier to maintain.
Thanks,
Lluis
--
"And it's much the same thing with knowledge, for whenever you learn
something new, the whole world becomes that much richer."
-- The Princess of Pure Reason, as told by Norton Juster in The Phantom
Tollbooth
