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

[Strong, Beeman]

LIP=0 will differ from LIP=1 behavior only when CSbase is non-zero, which 
requires 32-bit code.  In that case a LIP=0 implementation will provide only 
the EIP offset from CSbase in IP packets (like TIP or FUP), while LIP=1 
implementation will provide the full LIP (CSbase + EIP offset).

-----Original Message-----
From: Eduardo Habkost <ehabkost@redhat.com> 
Sent: Friday, September 25, 2020 9:16 AM
To: Kang, Luwei <luwei.kang@intel.com>
Cc: pbonzini@redhat.com; rth@twiddle.net; qemu-devel@nongnu.org; Strong, Beeman 
<beeman.strong@intel.com>
Subject: Re: [PATCH v1 1/3] i386: Remove the limitation of IP payloads for 
Intel PT

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