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

[Richard Henderson <rth@twiddle.net>]

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.

> --- 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.

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.

Also, doing this action with macros, here, seems truly backward.  Why not
simply modify the real tcg_gen_qemu_ld_i32 in tcg.c?


r~