Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Message-ID: <6d3417f7-062e-9934-01ab-20e3a46656a7@oracle.com>
Date:   Tue, 12 Dec 2023 23:28:13 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH v2] KVM: x86/intr: Explicitly check NMI from guest to
 eliminate false positives
To:     Like Xu <like.xu.linux@gmail.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>,
        Andi Kleen <ak@linux.intel.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Sean Christopherson <seanjc@google.com>
References: <20231206032054.55070-1-likexu@tencent.com>
Content-Language: en-US
From:   Dongli Zhang <dongli.zhang@oracle.com>
In-Reply-To: <20231206032054.55070-1-likexu@tencent.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bllEc252eU16RXhCUlRpckVodW84MXErU2RHVmRYS1FVeTBNQjV1eHFhVXY0?=
 =?utf-8?B?ZW12QnZKZ0ZIdkRDdyszdHorN3JhdUI3ZU5XU3NaSTBGeUxMNU9iMXdHd1BW?=
 =?utf-8?B?dDU5MzJxRXArVnp0SjZNTngrNlhkT3Z0TlZabFJwTzFmQTN0VXZhSVFKbk5m?=
 =?utf-8?B?ZWRIK1ErNE1CUVNEN2NJcEloRG1TQXIycUNxUmFZenpxYlFDRXhURzczVFZI?=
 =?utf-8?B?N0xoVTZJWU0xQmViNlFWYnhISUJqY29xRzdnVDBCZFVhMVpJTER5RGVFVDdG?=
 =?utf-8?B?akpWUmRLYmpTV2hraXhKSnF6alI5dkNZQU54NE1iaUNRN2w2RzdISFZGTWpK?=
 =?utf-8?B?WkNLb0h2c2puT0hpd094UFJxTXpWUncyZDY2OUhxcnJZR3ROUmZvRE50MVRi?=
 =?utf-8?B?SU9MaHhWWFlDbVh3cTNGQmpnQXFSOFdET3Vmd2VpWDhNR0lLV2ExUHc0Vlpk?=
 =?utf-8?B?WFlQZGFuRGZ2SmNnTEdjTjJBRDNlamF1SGF1YUFFTVdSajBjQ2Evd1h0SWl0?=
 =?utf-8?B?NSs4alZKejlaNXp3aXBQQ2syUk5XV0FyNnJXcmgwaVdjejdhZlVkdnhZSjNv?=
 =?utf-8?B?SGt1Sk5nWjA1V3pEazI2cEI2Q1JEWnpwNlpOZk5DdUJia2szcVQ2NUcvQnBj?=
 =?utf-8?B?alNwUzZTWmpDcEpkc3FoSVh3UXdHYUV0U3BXRW9KeEJBNzZ3c25ZcXl1eEVh?=
 =?utf-8?B?R1BUR2NWSGtzMlBlWmVFTjhQVGZwajkybXFucEd3TU8zZmVDZkc5dTdEV3RZ?=
 =?utf-8?B?TmMxSHg5VFBTTXNjQVFCem5GL1Q2aUpUZzl2MXV3bTJDSjZBZmJWaWMxS1or?=
 =?utf-8?B?RUxDZHFGcS90eDVneE1USDNJNml4M2pHOE1yeXdtSitieThLa2dBZGYybTZR?=
 =?utf-8?B?MWtwUEcrZ0xCZXFmL2h5L3lyWUU0MTR6R25oeGhKMUpMOUE0UmdrVHQ5ZzF2?=
 =?utf-8?B?bGJiVGZndXpieUxmWEE5ZUxXdmcwUUswOVR4b083RS9uNkNjelV4NFp0WGVX?=
 =?utf-8?B?YVFqWG9aQ1ZicU9VSVhhUmd5ZnU0aDhUMkpKQllWS2o0S0lSTk9LaEdnUVlO?=
 =?utf-8?B?dHE5cWx6cm1QNWo4TUpKYU1lOG8xNHZSZ21LaUxtaS90VzJQOWNONm9Qang3?=
 =?utf-8?B?Wm1pb0c4MFd0SWdoekZDWXZTZkc4RFU1a0NDUmY2TlN4a0Myb0FMZ2k1aVJH?=
 =?utf-8?B?Yy9PaDZvanRGaWZKWFBBdGs4VXMvQW5ydzFNeE0zMHNxUUV2VU82dnpRbS9I?=
 =?utf-8?B?SUQvZGgyWjMvZzhFT2NSankrd1dBcG9PNWdTMm9YSEpKVkYra0Y3TGZqZmpi?=
 =?utf-8?B?eS9SclR6NG9GZ1BFekJEQXAwWVBpMmFxS3FyQ2thUU4wSzJSWEpZd3FZNHBX?=
 =?utf-8?B?WklpZ2lMNXFZRGhzZlA5NWhwd3lzQysvdVJURTgrY3REc3dsZUdPdzArMHdL?=
 =?utf-8?B?QzhPbzI3K1lxNkR6UmRrdzJFUk9SeWEwcUJtNGFLcTloTlZxS1pPWVZ4eWFx?=
 =?utf-8?B?ZW0vSUdBekhtNDdBU3lya2ZlTzdsaWorL01tSGhsaUV2emJ2eVlUVkZWdnB1?=
 =?utf-8?B?QkRKMGJHQnBTR0Z5M3ptaTIwS2ZldTBNSWsxS1dZMGFqWDdOeHpzN3hPemRa?=
 =?utf-8?B?QUxCZGUrWkdWSWErWERGeGQyVlQybEQ1dnlLcVgwd3JOaklUQVdjYXJTaUk1?=
 =?utf-8?B?cGZjRCtEU3ZJYlV3TFZQT0x4Vk50dHBvelZpSEpRZERzVkFWcFBsVEhSTldy?=
 =?utf-8?B?dTAvalZSVUg2cE9MRWJvb2FZR3RjMlM1ZElHZ0NRMlEwN3k3RUo2SjhUblNP?=
 =?utf-8?B?TkgvU2x3SEc1dzloTG1LVkpkdHo3bUxLQy80MitvcVBzSkNTYmJKZFdtRlFZ?=
 =?utf-8?B?WDAwckl1NlN4cVBJQmpLVVZsd2l2NGF5cGZFSzgrSEI0czgrUkJMaUdKWkZG?=
 =?utf-8?B?a0xRbk9Yanl6RTRHOFFwUEZsVDlHY2k4Z1ZkSE15M3VGWXUwMU1vdnRmVzlx?=
 =?utf-8?B?QUNERVNsUzBncTFDekRya2ZtRjhMYTlWenFZRjJUZy9RYWFJMWFwaGRpcStu?=
 =?utf-8?B?bGhvclVlbmZjNTAxK1pvMGpSandmWDAxc1poWGtEdXpra0thWUkwWVQxZmow?=
 =?utf-8?Q?o2akB0Cx1dWHGOqW3BclN31CE?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: E5FjyI2td2Uw1kz+gcYcp6TN8/iysiDQz+zXlW5Yncl+hfa79mDMYkxasl+3DXQjza4JIB0S+utLvmloGsU6JNmO2EndwY0vyk3PS8LNgl8Ods/iP94B1uVgcZRdS2axSaawf3I2cWxJmVsp8TYWDLeXSBRyF4Po5Jo2cDnCSNcS+E6Wm4PS5MwS1IzAdNpnwWd1lp5Lkkppsgx6l2MNShvIjZYq3utvcK8q1Qv9mVLe/tCO04EgCh1ntq2nRZy+90N3QoL7wH2besxunrFJ/wWOSt5UfC1zKMJb6JUFFGYWj5Yc4ozX0gLAxkW/RmQ10rtYX6nlFlWpF36cKoSUzgNfiCPyoj5FT7/9MNXkhtUVMO8DC12OVagx7yYOzkLkN6oeg4X1dTGhr50nKLQkSc/vqSgB+OCmAxJEicWubDbyr+0ry3ab2YAtmPmqwwaQ/+ZGfnUtUnXeI+brFXgBLCG10q83iMd1vVCqFfm4KaFdF5+cXBDOITAhjCPRy7dxO/8sI5WZY/Z4PFeZduww7qIe/HeqNB2SuhJPjf2H5YB6tjt3CDoJAtjOGvTv2hIp13oq24LfNLVQzLpBmBferukdjM0DfBJkdFqlCAXmWEf0tME2ZLkCksSH9rQaJaFasb4fnK2hOpxkNz+7JUOIpQnE2Z5Ksw6ZmohKAZXqDYgjBy11uV+LTQ2VXwzuRufoS0Z/bGfqBmcbhScZ8mJnEnKWlbLTQLA/gowR2GB/V+ZEuak+K7TnI63DDSgK2vjnwyC6xC5S0eilMu1irfnTq5vENgR04ZxHztUSB8rnRYxCf6uN9lFszz76R3xzakKLXxJMmuOIioNqUTpA/sybcC1U9NYMzgC2o/eyi1kZyNcVfaKFONzoo8abib7pqU05yJpAWfWe5yBTGNy3JZOZnw==
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f42fb6c6-3753-4530-598f-08dbfbad1210
X-MS-Exchange-CrossTenant-AuthSource: BYAPR10MB2663.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2023 07:28:15.5888
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Oa3iDH1E/1x1sj9cTXjF3t9W3fZqkFEDWJpnivw1gOBBjAmd5dLlUwDpzsAcxeTbBnzcHpwm4XWLqa6alWm3Mg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR10MB6659
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-12-12_14,2023-12-12_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 bulkscore=0 spamscore=0
 mlxscore=0 adultscore=0 phishscore=0 malwarescore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2312130053
X-Proofpoint-GUID: qQQwBZLMfvpZbP5A-40cgio3LmMY_j9S
X-Proofpoint-ORIG-GUID: qQQwBZLMfvpZbP5A-40cgio3LmMY_j9S
X-Spam-Status: No, score=-3.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Tue, 12 Dec 2023 23:29:02 -0800 (PST)

Hi Like,

On 12/5/23 19:20, Like Xu wrote:
> From: Like Xu <likexu@tencent.com>
> 
> Explicitly checking the source of external interrupt is indeed NMI and not
> other types in the kvm_arch_pmi_in_guest(), which prevents perf-kvm false
> positive samples generated in perf/core NMI mode after vm-exit but before
> kvm_before_interrupt() from being incorrectly labelled as guest samples:

About the before kvm_before_interrupt() ...

> 
> # test: perf-record + cpu-cycles:HP (which collects host-only precise samples)
> # Symbol                                   Overhead       sys       usr  guest sys  guest usr
> # .......................................  ........  ........  ........  .........  .........
> #
> # Before:
>   [g] entry_SYSCALL_64                       24.63%     0.00%     0.00%     24.63%      0.00%
>   [g] syscall_return_via_sysret              23.23%     0.00%     0.00%     23.23%      0.00%
>   [g] files_lookup_fd_raw                     6.35%     0.00%     0.00%      6.35%      0.00%
> # After:
>   [k] perf_adjust_freq_unthr_context         57.23%    57.23%     0.00%      0.00%      0.00%
>   [k] __vmx_vcpu_run                          4.09%     4.09%     0.00%      0.00%      0.00%
>   [k] vmx_update_host_rsp                     3.17%     3.17%     0.00%      0.00%      0.00%
> 
> In the above case, perf records the samples labelled '[g]', the RIPs behind
> the weird samples are actually being queried by perf_instruction_pointer()
> after determining whether it's in GUEST state or not, and here's the issue:
> 
> If vm-exit is caused by a non-NMI interrupt (such as hrtimer_interrupt) and
> at least one PMU counter is enabled on host, the kvm_arch_pmi_in_guest()
> will remain true (KVM_HANDLING_IRQ is set) until kvm_before_interrupt().

... and here.

Would you mind helping why kvm_arch_pmi_in_guest() remains true before
*kvm_before_interrupt()*.

According to the source code, the vcpu->arch.handling_intr_from_guest
is set to non-zero only at kvm_before_interrupt(), and cleared at
kvm_after_interrupt().

Or would you mean kvm_after_interrupt()?

Thank you very much!

Dongli Zhang

> 
> During this window, if a PMI occurs on host (since the KVM instructions on
> host are being executed), the control flow, with the help of the host NMI
> context, will be transferred to perf/core to generate performance samples,
> thus perf_instruction_pointer() and perf_guest_get_ip() is called.
> 
> Since kvm_arch_pmi_in_guest() only checks if there is an interrupt, it may
> cause perf/core to mistakenly assume that the source RIP of the host NMI
> belongs to the guest world and use perf_guest_get_ip() to get the RIP of
> a vCPU that has already exited by a non-NMI interrupt.
> 
> Error samples are recorded and presented to the end-user via perf-report.
> Such false positive samples could be eliminated by explicitly determining
> if the exit reason is KVM_HANDLING_NMI.
> 
> Note that when vm-exit is indeed triggered by PMI and before HANDLING_NMI
> is cleared, it's also still possible that another PMI is generated on host.
> Also for perf/core timer mode, the false positives are still possible since
> that non-NMI sources of interrupts are not always being used by perf/core.
> In both cases above, perf/core should correctly distinguish between real
> RIP sources or even need to generate two samples, belonging to host and
> guest separately, but that's perf/core's story for interested warriors.
> 
> Fixes: dd60d217062f ("KVM: x86: Fix perf timer mode IP reporting")
> Signed-off-by: Like Xu <likexu@tencent.com>
> ---
> V1 -> V2 Changelog:
> - Refine commit message to cover both perf/core timer and NMI modes;
> - Use in_nmi() to distinguish whether it's NMI mode or not; (Sean)
> V1: https://urldefense.com/v3/__https://lore.kernel.org/kvm/20231204074535.9567-1-likexu@tencent.com/__;!!ACWV5N9M2RV99hQ!MQ8FetD27SVKN34CS_P-K3qrhspFnpf_Mqb0McFN9y5vSUeScc5b0TlZ3ZMDvt4Cn4b3g0h9ci6EO9k3PBEQXpePrg$ 
>  arch/x86/include/asm/kvm_host.h | 10 +++++++++-
>  arch/x86/kvm/x86.h              |  6 ------
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index c8c7e2475a18..167d592e08d0 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1868,8 +1868,16 @@ static inline int kvm_arch_flush_remote_tlbs_range(struct kvm *kvm, gfn_t gfn,
>  }
>  #endif /* CONFIG_HYPERV */
>  
> +enum kvm_intr_type {
> +	/* Values are arbitrary, but must be non-zero. */
> +	KVM_HANDLING_IRQ = 1,
> +	KVM_HANDLING_NMI,
> +};
> +
> +/* Enable perf NMI and timer modes to work, and minimise false positives. */
>  #define kvm_arch_pmi_in_guest(vcpu) \
> -	((vcpu) && (vcpu)->arch.handling_intr_from_guest)
> +	((vcpu) && (vcpu)->arch.handling_intr_from_guest && \
> +	 (in_nmi() == ((vcpu)->arch.handling_intr_from_guest == KVM_HANDLING_NMI)))
>  
>  void __init kvm_mmu_x86_module_init(void);
>  int kvm_mmu_vendor_module_init(void);
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index 2f7e19166658..4dc38092d599 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -431,12 +431,6 @@ static inline bool kvm_notify_vmexit_enabled(struct kvm *kvm)
>  	return kvm->arch.notify_vmexit_flags & KVM_X86_NOTIFY_VMEXIT_ENABLED;
>  }
>  
> -enum kvm_intr_type {
> -	/* Values are arbitrary, but must be non-zero. */
> -	KVM_HANDLING_IRQ = 1,
> -	KVM_HANDLING_NMI,
> -};
> -
>  static __always_inline void kvm_before_interrupt(struct kvm_vcpu *vcpu,
>  						 enum kvm_intr_type intr)
>  {
> 
> base-commit: 1ab097653e4dd8d23272d028a61352c23486fd4a