qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 12/12] trace: [all] Add "guest_vmem" event


From: Lluís Vilanova
Subject: Re: [Qemu-devel] [PATCH 12/12] trace: [all] Add "guest_vmem" event
Date: Tue, 04 Feb 2014 21:01:44 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]