Re: [PATCH v1 1/3] i386: Remove the limitation of IP payloads for Intel PT

From: Eduardo Habkost <ehabkost@redhat.com>
To: Luwei Kang <luwei.kang@intel.com>
Cc: pbonzini@redhat.com, beeman.strong@intel.com,
	qemu-devel@nongnu.org, rth@twiddle.net
Subject: Re: [PATCH v1 1/3] i386: Remove the limitation of IP payloads for Intel PT
Date: Fri, 25 Sep 2020 12:15:39 -0400	[thread overview]
Message-ID: <20200925161539.GS3717385@habkost.net> (raw)
In-Reply-To: <1582580312-19864-2-git-send-email-luwei.kang@intel.com>

On Tue, Feb 25, 2020 at 05:38:30AM +0800, Luwei Kang wrote:
> The Intel PT packets which contain IP payloads will have LIP values, and it
> will include the CS base component if the CPUID.(EAX=14H,ECX=0H).ECX.[bit31]
> is set. But it will disabled the Intel PT in kvm guest because of the need
> of live migration safe(c078ca9 i386: Disable Intel PT if packets IP payloads
> have LIP values).
> 
> This patch will revert the previous limitation because the Intel new hardware
> will set this bit and LIP == RIP for most/all real code.

"works most of the time" might be good enough if it's a conscious
user choice, but not for something we might be enabling by
default.  Under which conditions this wouldn't work?  Can we
detect those conditions somehow?

To allow live migration between LIP=0 and LIP=1 hosts, we need
KVM to be able to properly emulate LIP=0 on LIP=1 hosts.  Does
the hardware make this possible?

If KVM can't emulate LIP=0 on a LIP=1 host, what you can do here
is to make the flag configurable and check if the configured
value matches the one in the host.  This way we can support both
types of hosts, just not allow live migration between them.

> 
> Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> ---
>  target/i386/cpu.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 69f518a..8c0d1e4 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -688,8 +688,6 @@ static CPUCacheInfo legacy_l3_cache = {
>   * bit[02]: Support Single-Range Output scheme;
>   */
>  #define INTEL_PT_MINIMAL_ECX     0x7
> -/* generated packets which contain IP payloads have LIP values */
> -#define INTEL_PT_IP_LIP          (1 << 31)
>  #define INTEL_PT_ADDR_RANGES_NUM 0x2 /* Number of configurable address ranges */
>  #define INTEL_PT_ADDR_RANGES_NUM_MASK 0x3
>  #define INTEL_PT_MTC_BITMAP      (0x0249 << 16) /* Support ART(0,3,6,9) */
> @@ -6281,8 +6279,7 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose)
>             ((eax_1 & INTEL_PT_ADDR_RANGES_NUM_MASK) <
>                                             INTEL_PT_ADDR_RANGES_NUM) ||
>             ((ebx_1 & (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) !=
> -                (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP)) ||
> -           (ecx_0 & INTEL_PT_IP_LIP)) {
> +                (INTEL_PT_PSB_BITMAP | INTEL_PT_CYCLE_BITMAP))) {
>              /*
>               * Processor Trace capabilities aren't configurable, so if the
>               * host can't emulate the capabilities we report on
> -- 
> 1.8.3.1
> 

-- 
Eduardo