Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Xu, Yanfei" <yanfei.xu@intel.com>
To:     "Christopherson,, Sean" <seanjc@google.com>
CC:     "pbonzini@redhat.com" <pbonzini@redhat.com>,
        "vkuznets@redhat.com" <vkuznets@redhat.com>,
        "wanpengli@tencent.com" <wanpengli@tencent.com>,
        "jmattson@google.com" <jmattson@google.com>,
        "joro@8bytes.org" <joro@8bytes.org>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "bp@alien8.de" <bp@alien8.de>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "x86@kernel.org" <x86@kernel.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Wang, Wei W" <wei.w.wang@intel.com>,
        "Liang, Kan" <kan.liang@intel.com>
Subject: RE: [PATCH] KVM: x86: Fix the intel_pt PMI handling wrongly
 considered from guest
Thread-Topic: [PATCH] KVM: x86: Fix the intel_pt PMI handling wrongly
 considered from guest
Thread-Index: AQHYaH+s6PwCFUVqKUabQKcfsjJv5a0hiOoAgAX7yBA=
Date:   Fri, 20 May 2022 09:54:14 +0000
Message-ID: <DM6PR11MB41380A9DD32D6542CBC3A90BF0D39@DM6PR11MB4138.namprd11.prod.outlook.com>
References: <20220515171633.902901-1-yanfei.xu@intel.com>
 <YoJYah+Ct90aj1I5@google.com>
In-Reply-To: <YoJYah+Ct90aj1I5@google.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?9v/60GT0w3Sgiccaz1DY41/ik2XjVT5s9m8cjT4mJxv2hPQenfnSjiOSEbo/?=
 =?us-ascii?Q?8iVt7W98GM5N5i6HRXpQVtCq8DVIeGgQFe1KN1y/C+gZjiY/t0PLBBZ4/aU/?=
 =?us-ascii?Q?eSl8BRJkzoy1dU4ganeUKA6+vjkhyX5jv8C7gjG5HMfTJ77w2HXHHKboa5rk?=
 =?us-ascii?Q?Z/XyixL5PyrwvSisMv7vh/3A5F3kKh9/lZcU1UEjlnKMJCHKqD+twFJgH3J1?=
 =?us-ascii?Q?mu9B0LbXuYGtBFzlyhUVEKZDy4L/UbJIJibKZ/VQmf5A8DieJ+TmX8J5bU6R?=
 =?us-ascii?Q?/WopzhS/3e1KqA4FwpnHga8+M/p2P2HEdqjPtPFIcPDAwMIk8ytN1cDGbC2U?=
 =?us-ascii?Q?LFmaiKPxOzlKxVeNvGUmb/SHlOqkap8AwCSVLYbtSXx2Sv99s+WyzuChiNNq?=
 =?us-ascii?Q?Er7VHPQCYpix7SaZf+g0lP+zEPnG/L4lcm5M0sEiVavg/+pfB1Je37gAtlck?=
 =?us-ascii?Q?3IkNdI7bnHLBKlJiWL7puqyPzfctBf8tPDVWBpaaMdOpIRnTAObYt9Y/VVtl?=
 =?us-ascii?Q?sza8pwa19yxgTyl+2rdYjrFLzl+5GnAvACu3fMbND3d7L6EKneC1S4/Eb28I?=
 =?us-ascii?Q?UpkKCeDAoS/lOMKw65vccShhLqHu8XfhXVMC4Utkx6cHsCaUDINQmFcfT7BP?=
 =?us-ascii?Q?2w1MNjbY0D4Bhg5V6vwRmt5MZD/cVZeYGFounXOvYM4nE+Bv9iVcmx8rezcE?=
 =?us-ascii?Q?fHXcOzElmD27Y9Rki+RoKSYhJxuXpjfOi21lHDbtvNc7jjpte18/OR2yHjBX?=
 =?us-ascii?Q?L6yi5D9Q0iQngH43PadUbA5tvdEP2w3xWDr++eQXfVyFZsk8k6E7jeimsrNr?=
 =?us-ascii?Q?lgFmiOQ39wW4sBoEB7+Y+ng85tRLM0FRsgHvZn380rYLYHSP3Z0EFuNSOi0C?=
 =?us-ascii?Q?/lsJe5Pybxh70YRX4XvY54nltdHpuE1q2WjkSP9FgdNWcv//y4d4qxyTuha3?=
 =?us-ascii?Q?n34WuKRubRqTiSA1HZ/qoDWnt1e6QMt/Vzn37MoYzfehEqqsIxRV8othr8eq?=
 =?us-ascii?Q?vwsebw/++NszH47KGDIUISaubr1qBNXaT4JBN0JKDFCw57u12Ep4Ja6bxyKO?=
 =?us-ascii?Q?TMaBFLhRV1XG3LEFkx+d3HAnwy2GquNYZn2mnDkKpvVt0/QUiUnm49T4xXsq?=
 =?us-ascii?Q?HKzZUPIGdh55D/sy2mZGT5sdFV+0QmTBFb3Rr9R0Hmax+UdhUKrpQzscmSeN?=
 =?us-ascii?Q?ms9I31o4ftGb9y0MJcELkUTcjmz7z2kOw80D205zybkI+EqyWVZ2FZ07YWFv?=
 =?us-ascii?Q?YqDXtqSEV9fZ2TILicUig4jWuEd08jlHs885yCGWb8cnh8sxev3m8HUXezrD?=
 =?us-ascii?Q?B81jbLyS+hys3JPpEnL2Sq0aaCRMTdhyavoVRd9l+HiDiQ+EgnGOpiooL3Ar?=
 =?us-ascii?Q?65g6UBZ+NSFQKzo2paRlFQFclXsbeI/bwmmViBkPA3j8Ap5+05XprG8wGj2Q?=
 =?us-ascii?Q?jPRkyfLvt2AoIiMZCuHX1Q7AwV/ts3KDTga3jqAqz4jLvieTisyJlh+9BY3V?=
 =?us-ascii?Q?QX19DkIgV2D1h+AZfp3kOMcpkqYB2dF/5ADp0vyfnrONJwAP2iepHu2aZT7I?=
 =?us-ascii?Q?cLEKwMmBk2thdWqcopxqIW+VGUtnULQoV8PyG0+WpvfcU4ruBPK5Ci8FAAJJ?=
 =?us-ascii?Q?Hr02GFju1AEhR6MaBePwEyHWkxUsf/N+CYIG6Wo2eXyHzQHbCqjKdUw3f4P2?=
 =?us-ascii?Q?H/ZQ5ggLo2Epj0kTKDee6ShLB3iyeCWBoP+6UDKugcibHZetMW3uTq/iunng?=
 =?us-ascii?Q?h/ynfaVkPQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB4138.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a302ce4e-168f-4b6a-863d-08da3a46b2d9
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 May 2022 09:54:14.8599
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 8UIufCUawjStI0miAGLbOq5/KidUGgHk9ywLxalb2AUNLnhpQfftlTOMcqYKiOcvk6PGI2LsNLoBmU8DV1ZYww==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN7PR11MB2530
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Sean,
You are right, the change of kvm_arch_pmi_in_guest is incorrect, because it=
 should cover two cases of PMI.=20
For the PMI of intel pt, it certainly is the NMI PMI. So how about fixing i=
t like below?

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 610355b9ccce..378036c1cf94 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7856,7 +7856,7 @@ static unsigned int vmx_handle_intel_pt_intr(void)
        struct kvm_vcpu *vcpu =3D kvm_get_running_vcpu();

        /* '0' on failure so that the !PT case can use a RET0 static call. =
*/
-       if (!kvm_arch_pmi_in_guest(vcpu))
+       if (!kvm_handling_nmi_from_guest(vcpu))
                return 0;

        kvm_make_request(KVM_REQ_PMI, vcpu);

Thanks,
Yanfei

-----Original Message-----
From: Sean Christopherson <seanjc@google.com>=20
Sent: Monday, May 16, 2022 9:58 PM
To: Xu, Yanfei <yanfei.xu@intel.com>
Cc: pbonzini@redhat.com; vkuznets@redhat.com; wanpengli@tencent.com; jmatts=
on@google.com; joro@8bytes.org; tglx@linutronix.de; mingo@redhat.com; bp@al=
ien8.de; dave.hansen@linux.intel.com; x86@kernel.org; kvm@vger.kernel.org; =
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] KVM: x86: Fix the intel_pt PMI handling wrongly consid=
ered from guest

On Mon, May 16, 2022, Yanfei Xu wrote:
> When kernel handles the vm-exit caused by external interrupts and PMI,=20
> it always set a type of kvm_intr_type to handling_intr_from_guest to=20
> tell if it's dealing an IRQ or NMI.
> However, the further type judgment is missing in kvm_arch_pmi_in_guest().
> It could make the PMI of intel_pt wrongly considered it comes from a=20
> guest once the PMI breaks the handling of vm-exit of external interrupts.
>=20
> Fixes: db215756ae59 ("KVM: x86: More precisely identify NMI from guest=20
> when handling PMI")
> Signed-off-by: Yanfei Xu <yanfei.xu@intel.com>
> ---
>  arch/x86/include/asm/kvm_host.h | 8 +++++++-
>  arch/x86/kvm/x86.h              | 6 ------
>  2 files changed, 7 insertions(+), 7 deletions(-)
>=20
> diff --git a/arch/x86/include/asm/kvm_host.h=20
> b/arch/x86/include/asm/kvm_host.h index 4ff36610af6a..308cf19f123d=20
> 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1582,8 +1582,14 @@ static inline int kvm_arch_flush_remote_tlb(struct=
 kvm *kvm)
>  		return -ENOTSUPP;
>  }
> =20
> +enum kvm_intr_type {
> +	/* Values are arbitrary, but must be non-zero. */
> +	KVM_HANDLING_IRQ =3D 1,
> +	KVM_HANDLING_NMI,
> +};
> +
>  #define kvm_arch_pmi_in_guest(vcpu) \
> -	((vcpu) && (vcpu)->arch.handling_intr_from_guest)
> +	((vcpu) && (vcpu)->arch.handling_intr_from_guest =3D=3D=20
> +KVM_HANDLING_NMI)

My understanding is that this isn't correct as a general change, as perf ev=
ents can use regular IRQs in some cases.  See commit dd60d217062f4 ("KVM: x=
86: Fix perf timer mode IP reporting").

I assume there's got to be a way to know which mode perf is using, e.g. we =
should be able to make this look something like:

	((vcpu) && (vcpu)->arch.handling_intr_from_guest =3D=3D kvm_pmi_vector)


>  void kvm_mmu_x86_module_init(void);
>  int kvm_mmu_vendor_module_init(void); diff --git a/arch/x86/kvm/x86.h=20
> b/arch/x86/kvm/x86.h index 588792f00334..3bdf1bc76863 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -344,12 +344,6 @@ static inline bool kvm_cstate_in_guest(struct kvm *k=
vm)
>  	return kvm->arch.cstate_in_guest;
>  }
> =20
> -enum kvm_intr_type {
> -	/* Values are arbitrary, but must be non-zero. */
> -	KVM_HANDLING_IRQ =3D 1,
> -	KVM_HANDLING_NMI,
> -};
> -
>  static inline void kvm_before_interrupt(struct kvm_vcpu *vcpu,
>  					enum kvm_intr_type intr)
>  {
> --
> 2.32.0
>=20