From mboxrd@z Thu Jan  1 00:00:00 1970
From: Anthony Liguori <anthony@codemonkey.ws>
Subject: Re: [patch 4/4] KVM-trace port to tracepoints
Date: Thu, 17 Jul 2008 11:52:43 -0500
Message-ID: <487F78DB.2070303@codemonkey.ws>
References: <20080717155724.897537670@polymtl.ca> <20080717160003.359557938@polymtl.ca>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: akpm@linux-foundation.org, Ingo Molnar <mingo@elte.hu>,
	linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Avi Kivity <avi@qumranet.com>, kvm@vger.kernel.org,
	"Feng(Eric) Liu" <eric.e.liu@intel.com>
To: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Return-path: <kvm-owner@vger.kernel.org>
Received: from an-out-0708.google.com ([209.85.132.243]:34711 "EHLO
	an-out-0708.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1756809AbYGQQxO (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 17 Jul 2008 12:53:14 -0400
Received: by an-out-0708.google.com with SMTP id d40so173694and.103
        for <kvm@vger.kernel.org>; Thu, 17 Jul 2008 09:53:13 -0700 (PDT)
In-Reply-To: <20080717160003.359557938@polymtl.ca>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Hi Mathieu,

It's difficult to review your patches because they aren't inlined.

At any rate, this patches are unusable with SVM.  They try to execute VT 
instructions unconditionally.  For instance, you changed:
>
> -    KVMTRACE_1D(INTR, vcpu, vmcs_read32(VM_EXIT_INTR_INFO), handler);
> +    trace_kvm_intr(vcpu);

Which lived in VT-specific code (vmx.c)

To:

> +static void probe_kvm_intr(struct kvm_vcpu *vcpu)
> +{
> +    kvm_add_trace(KVM_TRC_INTR, vcpu, 1,
> +        (u32 []){ vmcs_read32(VM_EXIT_INTR_INFO) });
> +}
> +

Which lives in common code (kvm_trace.c).  But vmcs_read32() is 
VT-specific and should not be used in common code so this motion is 
wrong.  Why not just pass more arguments to probe_kvm_intr()?  Then your 
first two patches can be dropped completely.

Regards,

Anthony Liguori

Mathieu Desnoyers wrote: