From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by smtp.subspace.kernel.org (Postfix) with ESMTPS id 26F091CBA17;
	Thu, 24 Oct 2024 17:34:45 +0000 (UTC)
Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=148.163.158.5
ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;
	t=1729791288; cv=none; b=V1+Wz8cF0Akb0LymrFtZDg3LmqSbIIdgZTXilxhdfizRhI7M8Tl8+30w18UHF96TMDJU/tvJzCXCFxl/bqrVuhASVJ2ukxIDeAaNar1/uxOriQCqprwScsCRmx36N9Pmcp+wMEIl7TtihTK5S1zEm/2C2/gudIbF+/PSgs/HkTQ=
ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org;
	s=arc-20240116; t=1729791288; c=relaxed/simple;
	bh=rIcN/tJ77E24TgIzdyNdCRzv0Q63B57oiY16bt0oe9I=;
	h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=B0Qa0bCL/dcdTHIWaHpp6GG4Vg8wgQ6LvZ1+NCCbDvlQponlf7Ym1J3pHhMAJwjTUB9hpRsOI3bhqIX5/G4v/1Mmu251u6rhGQsUgV+G8FwTXZqeQCc42UajP237sRRN0DkpvcG7FlFM/VeXn8yumpKAf08N8fEl2nmZXI9qHJw=
ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.ibm.com; spf=pass smtp.mailfrom=linux.ibm.com; dkim=pass (2048-bit key) header.d=ibm.com header.i=@ibm.com header.b=l1UO1Zlj; arc=none smtp.client-ip=148.163.158.5
Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.ibm.com
Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.ibm.com
Authentication-Results: smtp.subspace.kernel.org;
	dkim=pass (2048-bit key) header.d=ibm.com header.i=@ibm.com header.b="l1UO1Zlj"
Received: from pps.filterd (m0353725.ppops.net [127.0.0.1])
	by mx0a-001b2d01.pphosted.com (8.18.1.2/8.18.1.2) with ESMTP id 49OEV4R4016737;
	Thu, 24 Oct 2024 17:34:32 GMT
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=cc
	:content-transfer-encoding:date:from:message-id:mime-version
	:subject:to; s=pp1; bh=bkrHZtFIVPKjyk0yZqzq4mDtLNuAwloD58YnxqpXY
	Bg=; b=l1UO1ZljGrIJvBklEaU5varjZBc+cs7eG4C9xpGcttI5R4XEpqJHELhX0
	11TeDnBrNLpcG0i1noPpnvhTfyp8miuThVEjxP+E9kJmxpwV/CwyKSvTOzUmOfSf
	J9i1xP9tDKN86U9ZXsVVEynqcpPSGPMFdCBDZiTE6Yw58dbpbbR3QhUzuatvAqcG
	fr8MoRmOYNdaarE7oxsJKyJAd7IMSfnv0+T/xlpdBpGShX6ROqvdHJxrZVaHQx/2
	2l2fyVC7UKE142pIooYFkbsUue8m/q2Sqa1nFuw9MCLjA5C7EKGKFItbgRMnoRSr
	IA3Wc3t9dB5G7RFG/UKbO96gHV4sg==
Received: from pps.reinject (localhost [127.0.0.1])
	by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 42emajsxu8-1
	(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT);
	Thu, 24 Oct 2024 17:34:32 +0000 (GMT)
Received: from m0353725.ppops.net (m0353725.ppops.net [127.0.0.1])
	by pps.reinject (8.18.0.8/8.18.0.8) with ESMTP id 49OHYV42015696;
	Thu, 24 Oct 2024 17:34:31 GMT
Received: from ppma13.dal12v.mail.ibm.com (dd.9e.1632.ip4.static.sl-reverse.com [50.22.158.221])
	by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 42emajsxu5-1
	(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT);
	Thu, 24 Oct 2024 17:34:31 +0000 (GMT)
Received: from pps.filterd (ppma13.dal12v.mail.ibm.com [127.0.0.1])
	by ppma13.dal12v.mail.ibm.com (8.18.1.2/8.18.1.2) with ESMTP id 49OEVhhm012603;
	Thu, 24 Oct 2024 17:34:31 GMT
Received: from smtprelay07.fra02v.mail.ibm.com ([9.218.2.229])
	by ppma13.dal12v.mail.ibm.com (PPS) with ESMTPS id 42emhfhmm9-1
	(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT);
	Thu, 24 Oct 2024 17:34:30 +0000
Received: from smtpav07.fra02v.mail.ibm.com (smtpav07.fra02v.mail.ibm.com [10.20.54.106])
	by smtprelay07.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 49OHYToL34996514
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK);
	Thu, 24 Oct 2024 17:34:29 GMT
Received: from smtpav07.fra02v.mail.ibm.com (unknown [127.0.0.1])
	by IMSVA (Postfix) with ESMTP id 42CAA20043;
	Thu, 24 Oct 2024 17:34:29 +0000 (GMT)
Received: from smtpav07.fra02v.mail.ibm.com (unknown [127.0.0.1])
	by IMSVA (Postfix) with ESMTP id D3A4420040;
	Thu, 24 Oct 2024 17:34:27 +0000 (GMT)
Received: from li-c6426e4c-27cf-11b2-a85c-95d65bc0de0e.ibm.com.com (unknown [9.124.210.16])
	by smtpav07.fra02v.mail.ibm.com (Postfix) with ESMTP;
	Thu, 24 Oct 2024 17:34:27 +0000 (GMT)
From: Gautam Menghani <gautam@linux.ibm.com>
To: mpe@ellerman.id.au, npiggin@gmail.com, christophe.leroy@csgroup.eu,
        naveen@kernel.org
Cc: Gautam Menghani <gautam@linux.ibm.com>, linuxppc-dev@lists.ozlabs.org,
        kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2] KVM: PPC: Book3S HV: Mask off LPCR_MER for a vCPU before running it to avoid spurious interrupts
Date: Thu, 24 Oct 2024 23:04:14 +0530
Message-ID: <20241024173417.95395-1-gautam@linux.ibm.com>
X-Mailer: git-send-email 2.47.0
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-TM-AS-GCONF: 00
X-Proofpoint-ORIG-GUID: rPSVTr5sSj3X-4dvbPrDDS4viP9J7qZZ
X-Proofpoint-GUID: Lnn7aZiXdWYvhNRbNr-4w1job6Z0k7rX
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1051,Hydra:6.0.680,FMLib:17.12.62.30
 definitions=2024-10-15_01,2024-10-11_01,2024-09-30_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 malwarescore=0
 mlxscore=0 phishscore=0 priorityscore=1501 suspectscore=0 impostorscore=0
 lowpriorityscore=0 spamscore=0 mlxlogscore=624 bulkscore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2409260000
 definitions=main-2410240143

Mask off the LPCR_MER bit before running a vCPU to ensure that it is not
set if there are no pending interrupts. Running a vCPU with LPCR_MER bit
set and no pending interrupts results in L2 vCPU getting an infinite flood
of spurious interrupts. The 'if check' in kvmhv_run_single_vcpu() sets
the LPCR_MER bit if there are pending interrupts.

The spurious flood problem can be observed in 2 cases:
1. Crashing the guest while interrupt heavy workload is running
  a. Start a L2 guest and run an interrupt heavy workload (eg: ipistorm)
  b. While the workload is running, crash the guest (make sure kdump
     is configured)
  c. Any one of the vCPUs of the guest will start getting an infinite
     flood of spurious interrupts.

2. Running LTP stress tests in multiple guests at the same time
   a. Start 4 L2 guests.
   b. Start running LTP stress tests on all 4 guests at same time.
   c. In some time, any one/more of the vCPUs of any of the guests will
      start getting an infinite flood of spurious interrupts.

The root cause of both the above issues is the same:
1. A NMI is sent to a running vCPU that has LPCR_MER bit set.
2. In the NMI path, all registers are refreshed, i.e, H_GUEST_GET_STATE
   is called for all the registers.
3. When H_GUEST_GET_STATE is called for lpcr, the vcpu->arch.vcore->lpcr
   of that vCPU at L1 level gets updated with LPCR_MER set to 1, and this
   new value is always used whenever that vCPU runs, regardless of whether
   there was a pending interrupt.
4. Since LPCR_MER is set, the vCPU in L2 always jumps to the external
   interrupt handler, and this cycle never ends.

Fix the spurious flood by making sure a vCPU's LPCR_MER is always masked
before running a vCPU.

Fixes: ec0f6639fa88 ("KVM: PPC: Book3S HV nestedv2: Ensure LPCR_MER bit is passed to the L0")
Cc: stable@vger.kernel.org # v6.8+
Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
---
V1 -> V2:
1. Mask off the LPCR_MER in vcpu->arch.vcore->lpcr instead of resetting
it so that we avoid grabbing vcpu->arch.vcore->lock. (Suggested by
Ritesh in an internal review)

 arch/powerpc/kvm/book3s_hv.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 8f7d7e37bc8c..b8701b5dde50 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -5089,9 +5089,19 @@ static int kvmppc_vcpu_run_hv(struct kvm_vcpu *vcpu)
 
 	do {
 		accumulate_time(vcpu, &vcpu->arch.guest_entry);
+		/*
+		 * L1's copy of L2's lpcr (vcpu->arch.vcore->lpcr) can get its MER bit
+		 * unexpectedly set - for e.g. during NMI handling when all register
+		 * states are synchronized from L0 to L1. L1 needs to inform L0 about
+		 * MER=1 only when there are pending external interrupts.
+		 * kvmhv_run_single_vcpu() anyway sets MER bit if there are pending
+		 * external interrupts. Hence, mask off MER bit when passing vcore->lpcr
+		 * here as otherwise it may generate spurious interrupts in L2 KVM
+		 * causing an endless loop, which results in L2 guest getting hung.
+		 */
 		if (cpu_has_feature(CPU_FTR_ARCH_300))
 			r = kvmhv_run_single_vcpu(vcpu, ~(u64)0,
-						  vcpu->arch.vcore->lpcr);
+						  vcpu->arch.vcore->lpcr & ~LPCR_MER);
 		else
 			r = kvmppc_run_vcpu(vcpu);
 
-- 
2.47.0


From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by smtp.subspace.kernel.org (Postfix) with ESMTPS id D8E1518C002;
	Fri, 25 Oct 2024 03:56:07 +0000 (UTC)
Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=150.107.74.76
ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;
	t=1729828572; cv=none; b=mdp37DZl1AdQmEJs2RC+oT9CmFFDQLYjODicK4kI8mDw3p27j/bu9Zmc3tYSCz5P9AM1EITgGloT9cPttKYBbXdKaQOHSQ98M3MpYnySkaOE8qafz8puN8na30/XeokaxPTq2Rn4LQZSE8K77pEEERe+A3Bg6AU83REWWldUU+s=
ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org;
	s=arc-20240116; t=1729828572; c=relaxed/simple;
	bh=nFfAqX1nuEjPAzz8vWo8La/0AzColIlWjX443+G9qwY=;
	h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID:
	 MIME-Version:Content-Type; b=KLbxf6mIkqEnvr2XXPSwnFU5AEiLZxa3aEi0cF7vVylSsJbwxgeW8xpX6Lk5v0ebXy80nIy+aI7w85EgmDMclI+xADWzz3O4q32s34stTWeon3XRAm6cFJo/i2Sxcjtvgd35ci8B+J9xyhPIJI7lMotFdayvC/CRWeTtNJ+BZ7E=
ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ellerman.id.au; spf=pass smtp.mailfrom=ellerman.id.au; dkim=pass (2048-bit key) header.d=ellerman.id.au header.i=@ellerman.id.au header.b=ghWgbpbQ; arc=none smtp.client-ip=150.107.74.76
Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ellerman.id.au
Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ellerman.id.au
Authentication-Results: smtp.subspace.kernel.org;
	dkim=pass (2048-bit key) header.d=ellerman.id.au header.i=@ellerman.id.au header.b="ghWgbpbQ"
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ellerman.id.au;
	s=201909; t=1729828564;
	bh=zhqxaFdelNULJR0EVgv2G8QYzzTg5epKTyLS+XXXoa0=;
	h=From:To:Cc:Subject:In-Reply-To:References:Date:From;
	b=ghWgbpbQ6Ssp0Ya8t/2aTa4zdB6SP0T8GbEZQzex5ddeyDYOJkLpD26nX42ZVWgMw
	 mbPeOXfF8MELMRShXm17Jk2H9nszVq1HUL4NZKaWkm9m2Ay3nfbEe4aiVFxHOOT1RA
	 yhP0nLJxqZ985HnQMFIjcF4JwrstDc4f1QKEPypYh997qpSim6EKtD4zne6KElIaar
	 0g4NitpCjZ0sJh+PalJO2mCjmt1b2uTyGHgmqF5BzdCs996XIxBuZS/Bt35vu9Ii4b
	 2M3tAe7dq0Lm9jOtVs9/x8DslW0z9/d0vqJ9BY0Ht8Ie4s88CA0EowQJR1XIU2HfPs
	 JcXvo8Jq6LeuQ==
Received: from authenticated.ozlabs.org (localhost [127.0.0.1])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256)
	(Client did not present a certificate)
	by mail.ozlabs.org (Postfix) with ESMTPSA id 4XZTTw2Pjsz4w2N;
	Fri, 25 Oct 2024 14:56:04 +1100 (AEDT)
From: Michael Ellerman <mpe@ellerman.id.au>
To: Gautam Menghani <gautam@linux.ibm.com>, npiggin@gmail.com,
 christophe.leroy@csgroup.eu, naveen@kernel.org
Cc: Gautam Menghani <gautam@linux.ibm.com>, linuxppc-dev@lists.ozlabs.org,
 kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] KVM: PPC: Book3S HV: Mask off LPCR_MER for a vCPU
 before running it to avoid spurious interrupts
In-Reply-To: <20241024173417.95395-1-gautam@linux.ibm.com>
References: <20241024173417.95395-1-gautam@linux.ibm.com>
Date: Fri, 25 Oct 2024 14:56:05 +1100
Message-ID: <877c9wkf8q.fsf@mail.lhotse>
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
Content-Type: text/plain

Hi Gautam,

A few comments below ...

Gautam Menghani <gautam@linux.ibm.com> writes:
> Mask off the LPCR_MER bit before running a vCPU to ensure that it is not
> set if there are no pending interrupts.

I would typically leave this until the end of the change log. ie.
describe the bug and how it happens first, then the fix at the end.

But it's not a hard rule, so up to you.

> Running a vCPU with LPCR_MER bit
            ^
        "an L2 vCPU"

In general if you can qualify L0 vs L1 vs L2 everywhere it would help
folks follow the description.

> set and no pending interrupts results in L2 vCPU getting an infinite flood
> of spurious interrupts. The 'if check' in kvmhv_run_single_vcpu() sets
> the LPCR_MER bit if there are pending interrupts.
>
> The spurious flood problem can be observed in 2 cases:
> 1. Crashing the guest while interrupt heavy workload is running
>   a. Start a L2 guest and run an interrupt heavy workload (eg: ipistorm)
>   b. While the workload is running, crash the guest (make sure kdump
>      is configured)
>   c. Any one of the vCPUs of the guest will start getting an infinite
>      flood of spurious interrupts.
>
> 2. Running LTP stress tests in multiple guests at the same time
>    a. Start 4 L2 guests.
>    b. Start running LTP stress tests on all 4 guests at same time.
>    c. In some time, any one/more of the vCPUs of any of the guests will
>       start getting an infinite flood of spurious interrupts.
>
> The root cause of both the above issues is the same:
> 1. A NMI is sent to a running vCPU that has LPCR_MER bit set.
> 2. In the NMI path, all registers are refreshed, i.e, H_GUEST_GET_STATE
>    is called for all the registers.
> 3. When H_GUEST_GET_STATE is called for lpcr, the vcpu->arch.vcore->lpcr
>    of that vCPU at L1 level gets updated with LPCR_MER set to 1, and this
>    new value is always used whenever that vCPU runs, regardless of whether
>    there was a pending interrupt.
> 4. Since LPCR_MER is set, the vCPU in L2 always jumps to the external
>    interrupt handler, and this cycle never ends.
>
> Fix the spurious flood by making sure a vCPU's LPCR_MER is always masked
> before running a vCPU.

I think your original sentence at the top of the change log is actually more
accurate. ie. it's not that LPCR_MER is always cleared, it's cleared
*unless there's a pending interrupt*.

> Fixes: ec0f6639fa88 ("KVM: PPC: Book3S HV nestedv2: Ensure LPCR_MER bit is passed to the L0")
> Cc: stable@vger.kernel.org # v6.8+
> Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> ---
> V1 -> V2:
> 1. Mask off the LPCR_MER in vcpu->arch.vcore->lpcr instead of resetting
> it so that we avoid grabbing vcpu->arch.vcore->lock. (Suggested by
> Ritesh in an internal review)

Did v1 take the vcore->lock? I don't remember it.

> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 8f7d7e37bc8c..b8701b5dde50 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -5089,9 +5089,19 @@ static int kvmppc_vcpu_run_hv(struct kvm_vcpu *vcpu)
>  
>  	do {
>  		accumulate_time(vcpu, &vcpu->arch.guest_entry);
> +		/*
> +		 * L1's copy of L2's lpcr (vcpu->arch.vcore->lpcr) can get its MER bit
                                     ^
                                     LPCR
> +		 * unexpectedly set - for e.g. during NMI handling when all register
> +		 * states are synchronized from L0 to L1. L1 needs to inform L0 about
> +		 * MER=1 only when there are pending external interrupts.
> +		 * kvmhv_run_single_vcpu() anyway sets MER bit if there are pending
> +		 * external interrupts. Hence, mask off MER bit when passing vcore->lpcr
> +		 * here as otherwise it may generate spurious interrupts in L2 KVM
> +		 * causing an endless loop, which results in L2 guest getting hung.
> +		 */
>  		if (cpu_has_feature(CPU_FTR_ARCH_300))
>  			r = kvmhv_run_single_vcpu(vcpu, ~(u64)0,
> -						  vcpu->arch.vcore->lpcr);
> +						  vcpu->arch.vcore->lpcr & ~LPCR_MER);
 
This is much better than v1 which hid the clearing of LPCR_MER in a macro.

But I still wonder if it would be better to clear it in
kvmhv_run_single_vcpu() itself.

The logic to set LPCR_MER is already in there, so why not ensure
LPCR_MER is cleared as part of that some block?

I realise there's another caller of kvmhv_run_single_vcpu() from the
nested code, but that's OK because there's already a nested check in
kvmhv_run_single_vcpu(), so you can still isolate this change to just
the non-nested case.

cheers

From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mail-pg1-f180.google.com (mail-pg1-f180.google.com [209.85.215.180])
	(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))
	(No client certificate requested)
	by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9923118C018;
	Fri, 25 Oct 2024 06:43:10 +0000 (UTC)
Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.180
ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;
	t=1729838592; cv=none; b=FUMqKuRoQIfLf6fogz3UhTIesnq0YPHosa2ZpThOTvIoOmFc8KhUKSZfOyRJz5B9DFMnPZXi+iy0T/OZn6aIqa8UzluxTDas0eH02XG3IwMDuuaNBp3aXORYVTCb6wv/JuWSxMKtF7UylK+/64W+GRkWuvQv8XjhpPcTgxFH6r0=
ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org;
	s=arc-20240116; t=1729838592; c=relaxed/simple;
	bh=tYWW8T550R8QEZd5gfGCnQaBxuLELI8mHkKVHTwpUMU=;
	h=From:To:Cc:Subject:In-Reply-To:Date:Message-ID:References; b=ZNZepCL1400CxijCjK8MzPMQrKjnQsgiU43NN1lrPhEblQLvtldkfeZb7dqFI1YO8E3D+/gG6Uny66nMYnqL2TsN1uCjcXeAV2sReizbowH3D4aXcKCu8vmgfk/DeEZ/vkivsnIhZAyL02uaH+lYNVIXfr4lHjG2+zSoAlHTl34=
ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=fLsLHJ6b; arc=none smtp.client-ip=209.85.215.180
Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com
Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com
Authentication-Results: smtp.subspace.kernel.org;
	dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="fLsLHJ6b"
Received: by mail-pg1-f180.google.com with SMTP id 41be03b00d2f7-7ea8d7341e6so1123888a12.3;
        Thu, 24 Oct 2024 23:43:10 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=gmail.com; s=20230601; t=1729838589; x=1730443389; darn=vger.kernel.org;
        h=references:message-id:date:in-reply-to:subject:cc:to:from:from:to
         :cc:subject:date:message-id:reply-to;
        bh=P47I0gdiUlkn3LmA4ci0Px6PUVaCXe2wtW4m1OPAR8s=;
        b=fLsLHJ6bW8WRrEt31EroqwA80Ok23bqef280bKa5k2tVKRvVQMUM1+BBvfa9dTFRRv
         jMJlZva4iNaBG5oPPQGQmYhPL92ZQLHCdaXh8aF4DgG2sflO+0mZO/7S20EZlnqtwsPM
         2gjjDhAjyQ2polYr9i8OSLx8cTz4k0amYz7KnANFMs9ooWcCY6Qdj1hcBpEpDwozUsoh
         xsej0RI3GSdQnUaaoTSF/HleFALrtWxDfp8wbmWtCs6m55SanQNjt3bZTyRWAuKdF7BH
         AEWPKaD4pBS77+XC2MCQRAII3VED8qfjoETG8+xOwH/6l0QNxJxiOatLv9Qt6yb8v1AJ
         mCEg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=1e100.net; s=20230601; t=1729838589; x=1730443389;
        h=references:message-id:date:in-reply-to:subject:cc:to:from
         :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;
        bh=P47I0gdiUlkn3LmA4ci0Px6PUVaCXe2wtW4m1OPAR8s=;
        b=PBDqNyLZnhwefTxh04/61B7OBWpDw8Yj6hH+v4y0MK7NxdAsXjmxS1CYkkiFPisxpA
         GUHTrUlFeau++Me15Yhu1p8PtDHsJrlmsZjT9kaqxV0DsK2IX4TxRZ1lg7lw8eRW3CS3
         mzHGtOjWQk5OUj5etNa8kZ2yd9RyH80KNLQWobu/NUs8f6Jda8kEYwAHLQwB1BMhQl1P
         c5tvLb9jKu8bzuHM6jhFLrLqp75nNqZbPTkCioOtCeT80TFQ5q0bqpRNartYkKlM+5kz
         0aajbbH0pXRzF/67WpYNmh3IEtM8MBl0wJMEVZEraqp9zrDPjKDCmVrLyz1Xvmjep4iK
         G8ow==
X-Forwarded-Encrypted: i=1; AJvYcCVt+WaAZQKv0BIhwGmEqR2tzJ9EFCUXofI+XRFi/vfxTMRSJjA6T2Am1Ml9wsX+dvEY50g=@vger.kernel.org, AJvYcCWxcDdD7nQAqb4s73DLpjNnvYEzlOH4SollbBrk9n0QI7tSionYyOrh/x30dLgtrGyK8ow0S1uM5IMzd9Lv@vger.kernel.org
X-Gm-Message-State: AOJu0YzY1Yp9P37858B0I3K0SBsBVM9xIy3vjU+2tPbaeDXcQve6g0Ti
	GCPiyegCbdCUtEbxmd6XWwWfrkaGVfV6g8hun8t7cxHwRiSJuRliZyMmMw==
X-Google-Smtp-Source: AGHT+IFxdZwIFKOQNiOIOKlXyUTiHQUYTmMXDxrzgTed1lrPb/WrWs4hS/ZL7SNSfx+CwmEz3d4d5g==
X-Received: by 2002:a05:6a21:3a82:b0:1d6:5f3d:4ab7 with SMTP id adf61e73a8af0-1d989b3c990mr5577835637.22.1729838588742;
        Thu, 24 Oct 2024 23:43:08 -0700 (PDT)
Received: from dw-tp ([171.76.85.20])
        by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72057a1f18esm446345b3a.146.2024.10.24.23.43.05
        (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
        Thu, 24 Oct 2024 23:43:07 -0700 (PDT)
From: "Ritesh Harjani (IBM)" <ritesh.list@gmail.com>
To: Gautam Menghani <gautam@linux.ibm.com>, mpe@ellerman.id.au, npiggin@gmail.com, christophe.leroy@csgroup.eu, naveen@kernel.org
Cc: Gautam Menghani <gautam@linux.ibm.com>, linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] KVM: PPC: Book3S HV: Mask off LPCR_MER for a vCPU before running it to avoid spurious interrupts
In-Reply-To: <20241024173417.95395-1-gautam@linux.ibm.com>
Date: Fri, 25 Oct 2024 11:37:52 +0530
Message-ID: <871q04oguf.fsf@gmail.com>
References: <20241024173417.95395-1-gautam@linux.ibm.com>
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>

Gautam Menghani <gautam@linux.ibm.com> writes:

> Mask off the LPCR_MER bit before running a vCPU to ensure that it is not
> set if there are no pending interrupts. Running a vCPU with LPCR_MER bit
> set and no pending interrupts results in L2 vCPU getting an infinite flood
> of spurious interrupts. The 'if check' in kvmhv_run_single_vcpu() sets
> the LPCR_MER bit if there are pending interrupts.
>
> The spurious flood problem can be observed in 2 cases:
> 1. Crashing the guest while interrupt heavy workload is running
>   a. Start a L2 guest and run an interrupt heavy workload (eg: ipistorm)
>   b. While the workload is running, crash the guest (make sure kdump
>      is configured)
>   c. Any one of the vCPUs of the guest will start getting an infinite
>      flood of spurious interrupts.
>
> 2. Running LTP stress tests in multiple guests at the same time
>    a. Start 4 L2 guests.
>    b. Start running LTP stress tests on all 4 guests at same time.
>    c. In some time, any one/more of the vCPUs of any of the guests will
>       start getting an infinite flood of spurious interrupts.
>
> The root cause of both the above issues is the same:
> 1. A NMI is sent to a running vCPU that has LPCR_MER bit set.
> 2. In the NMI path, all registers are refreshed, i.e, H_GUEST_GET_STATE
>    is called for all the registers.
> 3. When H_GUEST_GET_STATE is called for lpcr, the vcpu->arch.vcore->lpcr
>    of that vCPU at L1 level gets updated with LPCR_MER set to 1, and this
>    new value is always used whenever that vCPU runs, regardless of whether
>    there was a pending interrupt.
> 4. Since LPCR_MER is set, the vCPU in L2 always jumps to the external
>    interrupt handler, and this cycle never ends.
>
> Fix the spurious flood by making sure a vCPU's LPCR_MER is always masked
> before running a vCPU.
>
> Fixes: ec0f6639fa88 ("KVM: PPC: Book3S HV nestedv2: Ensure LPCR_MER bit is passed to the L0")
> Cc: stable@vger.kernel.org # v6.8+
> Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> ---
> V1 -> V2:
> 1. Mask off the LPCR_MER in vcpu->arch.vcore->lpcr instead of resetting
> it so that we avoid grabbing vcpu->arch.vcore->lock. (Suggested by
> Ritesh in an internal review)

Thanks Gautam for addressing the review comment. But let me improve the
changelog a little to make it more accurate for others too.

Removed the macro which was silently clearing LPCR_MER bit from vcore->lpcr
and instead just mask it off while sending it to kvmhv_run_single_vcpu().
Added an inline comment describing the reason to avoid anyone tipping
it over. - (suggested ...)


Yes, that would also mean that no need of taking any vcore lock since we
are not modifying any of the vcore state variables which came up in the
internal review discussion.
Having said that it will be good to document the usage of vcore->lock
above the struct kvmppc_vcore definition. Because it isn't obvious of
when all it should be taken and/or what all it protects?

>
>  arch/powerpc/kvm/book3s_hv.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 8f7d7e37bc8c..b8701b5dde50 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -5089,9 +5089,19 @@ static int kvmppc_vcpu_run_hv(struct kvm_vcpu *vcpu)
>  
>  	do {
>  		accumulate_time(vcpu, &vcpu->arch.guest_entry);
> +		/*
> +		 * L1's copy of L2's lpcr (vcpu->arch.vcore->lpcr) can get its MER bit
> +		 * unexpectedly set - for e.g. during NMI handling when all register
> +		 * states are synchronized from L0 to L1. L1 needs to inform L0 about
> +		 * MER=1 only when there are pending external interrupts.
> +		 * kvmhv_run_single_vcpu() anyway sets MER bit if there are pending
> +		 * external interrupts. Hence, mask off MER bit when passing vcore->lpcr
> +		 * here as otherwise it may generate spurious interrupts in L2 KVM
> +		 * causing an endless loop, which results in L2 guest getting hung.
> +		 */

Thanks for describing this inline.

>  		if (cpu_has_feature(CPU_FTR_ARCH_300))
>  			r = kvmhv_run_single_vcpu(vcpu, ~(u64)0,
> -						  vcpu->arch.vcore->lpcr);
> +						  vcpu->arch.vcore->lpcr & ~LPCR_MER);

While still at it - 
I too like mpe suggestion to clear the LPCR_MER bit at one place
itself within kvmhv_run_single_vcpu(). 

>  		else
>  			r = kvmppc_run_vcpu(vcpu);
>  
> -- 
> 2.47.0

-ritesh

From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3AE391F7547;
	Fri, 25 Oct 2024 14:22:50 +0000 (UTC)
Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=148.163.158.5
ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;
	t=1729866172; cv=none; b=VDKvMmCgsNw1vNzTBEvlzntzEh0CcMIQoW8kJwvsZ9E7aWztL4mFMAchGVQ+wcUthvTDPygQcIoW+Kl6HI9bMY84EICtCQoM8Fmhf5tZXG3mW1nOjDBknszpGnkTh3oTKQ4jIVu2qSiPgyCBBHhaKh/aR2MlI6QlHzOdzMQDZ1A=
ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org;
	s=arc-20240116; t=1729866172; c=relaxed/simple;
	bh=2dl/Xe419sk+LmUjBAuq5rggnFUXEKzvgD0WDXRXpRs=;
	h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:
	 Content-Type:Content-Disposition:In-Reply-To; b=mOhkWckiareh5O32Wga3TBi3iZd1fQzT7z2kQlaAyM+oXqPKqbX99U0dd0WGgmEoYfn5T1mT+SexZYh+WZor3/5Gg14vXeWHP15xMBYfDj1tMa7QH21B7uGEBHXch/oCzNG15yCHW3cFllYixjWjEhh5VSjVv4VDcNTNa3NkITs=
ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.ibm.com; spf=pass smtp.mailfrom=linux.ibm.com; dkim=pass (2048-bit key) header.d=ibm.com header.i=@ibm.com header.b=VFmXHLSi; arc=none smtp.client-ip=148.163.158.5
Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.ibm.com
Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.ibm.com
Authentication-Results: smtp.subspace.kernel.org;
	dkim=pass (2048-bit key) header.d=ibm.com header.i=@ibm.com header.b="VFmXHLSi"
Received: from pps.filterd (m0353725.ppops.net [127.0.0.1])
	by mx0a-001b2d01.pphosted.com (8.18.1.2/8.18.1.2) with ESMTP id 49P20eTu016724;
	Fri, 25 Oct 2024 14:22:31 GMT
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=cc
	:content-type:date:from:in-reply-to:message-id:mime-version
	:references:subject:to; s=pp1; bh=ZeVnIZsZ+W6LZTALEEW0rV0SpWEyEK
	WMjQtAarasrAs=; b=VFmXHLSi18rIA3bfaHj6q8I5FMWL1CMHjA7Za2xDW8tmdc
	NImQmyWh2aUPAn6C8Q9ouVGlm9p42bBGsq6jVMzFUe4wxl8QLJIbCMqZbboK4XJE
	NBE/1tidihamMyGO9raVN9kBAQsyGizUaIPdxfeMYPW7NOLpuICEc73/2Br671N+
	Q969V41NIiksVvpZ0YUI8leCc++eeMdz6Yg5jdeugLutJApyw3CrjKW3zWXM+Bq9
	Kl381Uv4kZhm3l25WcQu+Z1ktu0OJP6OFYP8jaWLZbvuxd5OtKQPgqPfZMQkrCY7
	nMw5fGkfL3E/LenIxuOrNwmKyQDgBkps1wZhB8kA==
Received: from pps.reinject (localhost [127.0.0.1])
	by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 42emajxgtp-1
	(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT);
	Fri, 25 Oct 2024 14:22:31 +0000 (GMT)
Received: from m0353725.ppops.net (m0353725.ppops.net [127.0.0.1])
	by pps.reinject (8.18.0.8/8.18.0.8) with ESMTP id 49PEMUpv029480;
	Fri, 25 Oct 2024 14:22:30 GMT
Received: from ppma11.dal12v.mail.ibm.com (db.9e.1632.ip4.static.sl-reverse.com [50.22.158.219])
	by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 42emajxgtk-1
	(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT);
	Fri, 25 Oct 2024 14:22:30 +0000 (GMT)
Received: from pps.filterd (ppma11.dal12v.mail.ibm.com [127.0.0.1])
	by ppma11.dal12v.mail.ibm.com (8.18.1.2/8.18.1.2) with ESMTP id 49PAbv44008796;
	Fri, 25 Oct 2024 14:22:30 GMT
Received: from smtprelay04.fra02v.mail.ibm.com ([9.218.2.228])
	by ppma11.dal12v.mail.ibm.com (PPS) with ESMTPS id 42emkax34j-1
	(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT);
	Fri, 25 Oct 2024 14:22:30 +0000
Received: from smtpav03.fra02v.mail.ibm.com (smtpav03.fra02v.mail.ibm.com [10.20.54.102])
	by smtprelay04.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 49PEMSYC32440840
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK);
	Fri, 25 Oct 2024 14:22:28 GMT
Received: from smtpav03.fra02v.mail.ibm.com (unknown [127.0.0.1])
	by IMSVA (Postfix) with ESMTP id 636EB2004B;
	Fri, 25 Oct 2024 14:22:28 +0000 (GMT)
Received: from smtpav03.fra02v.mail.ibm.com (unknown [127.0.0.1])
	by IMSVA (Postfix) with ESMTP id 790ED20043;
	Fri, 25 Oct 2024 14:22:26 +0000 (GMT)
Received: from li-c6426e4c-27cf-11b2-a85c-95d65bc0de0e.ibm.com (unknown [9.39.31.149])
	by smtpav03.fra02v.mail.ibm.com (Postfix) with ESMTPS;
	Fri, 25 Oct 2024 14:22:26 +0000 (GMT)
Date: Fri, 25 Oct 2024 19:52:23 +0530
From: Gautam Menghani <gautam@linux.ibm.com>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: npiggin@gmail.com, christophe.leroy@csgroup.eu, naveen@kernel.org,
        linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] KVM: PPC: Book3S HV: Mask off LPCR_MER for a vCPU
 before running it to avoid spurious interrupts
Message-ID: <x4sx3g3as4xzhby6gyonh73z54y6z7d5s37yferz3ybbyc6skd@fjwoxrgz6rds>
References: <20241024173417.95395-1-gautam@linux.ibm.com>
 <877c9wkf8q.fsf@mail.lhotse>
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <877c9wkf8q.fsf@mail.lhotse>
X-TM-AS-GCONF: 00
X-Proofpoint-ORIG-GUID: KOMBPlvXuVQ2ggS1xN5NnXnFwyFOY6Iw
X-Proofpoint-GUID: jdzenZ0xq_9A_mSgMqBUxBZxed7KPAIe
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1051,Hydra:6.0.680,FMLib:17.12.62.30
 definitions=2024-10-15_01,2024-10-11_01,2024-09-30_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 malwarescore=0
 mlxscore=0 phishscore=0 priorityscore=1501 suspectscore=0 impostorscore=0
 lowpriorityscore=0 spamscore=0 mlxlogscore=999 bulkscore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2409260000
 definitions=main-2410250110

On Fri, Oct 25, 2024 at 02:56:05PM +1100, Michael Ellerman wrote:
> Hi Gautam,
> 
> A few comments below ...
> 
> Gautam Menghani <gautam@linux.ibm.com> writes:
> > Mask off the LPCR_MER bit before running a vCPU to ensure that it is not
> > set if there are no pending interrupts.
> 
> I would typically leave this until the end of the change log. ie.
> describe the bug and how it happens first, then the fix at the end.
> 
> But it's not a hard rule, so up to you.

Yes agreed, that would make more sense.

> 
> > Running a vCPU with LPCR_MER bit
>             ^
>         "an L2 vCPU"
> 
> In general if you can qualify L0 vs L1 vs L2 everywhere it would help
> folks follow the description.

yes will add it in v3
> 
> > set and no pending interrupts results in L2 vCPU getting an infinite flood
> > of spurious interrupts. The 'if check' in kvmhv_run_single_vcpu() sets
> > the LPCR_MER bit if there are pending interrupts.
> >
> > The spurious flood problem can be observed in 2 cases:
> > 1. Crashing the guest while interrupt heavy workload is running
> >   a. Start a L2 guest and run an interrupt heavy workload (eg: ipistorm)
> >   b. While the workload is running, crash the guest (make sure kdump
> >      is configured)
> >   c. Any one of the vCPUs of the guest will start getting an infinite
> >      flood of spurious interrupts.
> >
> > 2. Running LTP stress tests in multiple guests at the same time
> >    a. Start 4 L2 guests.
> >    b. Start running LTP stress tests on all 4 guests at same time.
> >    c. In some time, any one/more of the vCPUs of any of the guests will
> >       start getting an infinite flood of spurious interrupts.
> >
> > The root cause of both the above issues is the same:
> > 1. A NMI is sent to a running vCPU that has LPCR_MER bit set.
> > 2. In the NMI path, all registers are refreshed, i.e, H_GUEST_GET_STATE
> >    is called for all the registers.
> > 3. When H_GUEST_GET_STATE is called for lpcr, the vcpu->arch.vcore->lpcr
> >    of that vCPU at L1 level gets updated with LPCR_MER set to 1, and this
> >    new value is always used whenever that vCPU runs, regardless of whether
> >    there was a pending interrupt.
> > 4. Since LPCR_MER is set, the vCPU in L2 always jumps to the external
> >    interrupt handler, and this cycle never ends.
> >
> > Fix the spurious flood by making sure a vCPU's LPCR_MER is always masked
> > before running a vCPU.
> 
> I think your original sentence at the top of the change log is actually more
> accurate. ie. it's not that LPCR_MER is always cleared, it's cleared
> *unless there's a pending interrupt*.

Yes agreed
> 
> > Fixes: ec0f6639fa88 ("KVM: PPC: Book3S HV nestedv2: Ensure LPCR_MER bit is passed to the L0")
> > Cc: stable@vger.kernel.org # v6.8+
> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> > ---
> > V1 -> V2:
> > 1. Mask off the LPCR_MER in vcpu->arch.vcore->lpcr instead of resetting
> > it so that we avoid grabbing vcpu->arch.vcore->lock. (Suggested by
> > Ritesh in an internal review)
> 
> Did v1 take the vcore->lock? I don't remember it.

No v1 did not take a lock, but ideally was supposed to take a lock. I
missed the locking part there.
> 
> > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> > index 8f7d7e37bc8c..b8701b5dde50 100644
> > --- a/arch/powerpc/kvm/book3s_hv.c
> > +++ b/arch/powerpc/kvm/book3s_hv.c
> > @@ -5089,9 +5089,19 @@ static int kvmppc_vcpu_run_hv(struct kvm_vcpu *vcpu)
> >  
> >  	do {
> >  		accumulate_time(vcpu, &vcpu->arch.guest_entry);
> > +		/*
> > +		 * L1's copy of L2's lpcr (vcpu->arch.vcore->lpcr) can get its MER bit
>                                      ^
>                                      LPCR

Ack. 
> > +		 * unexpectedly set - for e.g. during NMI handling when all register
> > +		 * states are synchronized from L0 to L1. L1 needs to inform L0 about
> > +		 * MER=1 only when there are pending external interrupts.
> > +		 * kvmhv_run_single_vcpu() anyway sets MER bit if there are pending
> > +		 * external interrupts. Hence, mask off MER bit when passing vcore->lpcr
> > +		 * here as otherwise it may generate spurious interrupts in L2 KVM
> > +		 * causing an endless loop, which results in L2 guest getting hung.
> > +		 */
> >  		if (cpu_has_feature(CPU_FTR_ARCH_300))
> >  			r = kvmhv_run_single_vcpu(vcpu, ~(u64)0,
> > -						  vcpu->arch.vcore->lpcr);
> > +						  vcpu->arch.vcore->lpcr & ~LPCR_MER);
>  
> This is much better than v1 which hid the clearing of LPCR_MER in a macro.
> 
> But I still wonder if it would be better to clear it in
> kvmhv_run_single_vcpu() itself.
> 
> The logic to set LPCR_MER is already in there, so why not ensure
> LPCR_MER is cleared as part of that some block?
> 
> I realise there's another caller of kvmhv_run_single_vcpu() from the
> nested code, but that's OK because there's already a nested check in
> kvmhv_run_single_vcpu(), so you can still isolate this change to just
> the non-nested case.
> 

Yes it would be better to mask off LPCR_MER inside
kvmhv_run_single_vcpu(), will make that change and send v3. 

> cheers

Thanks,
Gautam

From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by smtp.subspace.kernel.org (Postfix) with ESMTPS id E1ABF21219A;
	Fri, 25 Oct 2024 14:30:42 +0000 (UTC)
Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=148.163.158.5
ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;
	t=1729866645; cv=none; b=o4hlmuhdfJENqD36GsKeC00vveUqxyd0ebOcF1YytOs8TbaUmFiav7Q/y0KskQ1cwgRHgAqSbWbM5ItcAG4VJpD7GQyZ7bZGkb3KzAyyXMBxLInTHyZ5EkqmXTQZLxj4uaZQFiJ9v5Kn9moSiZe0Y7Yh5/+BycoccOSuCBUNqnA=
ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org;
	s=arc-20240116; t=1729866645; c=relaxed/simple;
	bh=pe8OQSnOrM8neSWZ7ZEC1q/8cQL38g4Im5mmnZmb1Lk=;
	h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:
	 Content-Type:Content-Disposition:In-Reply-To; b=DUcPT7gJD1j6ot9/mCaRxv3JN1S6LS7Imr5Bqz7wCy0ULULhhf2hHq6yOR7Fxig8vK8eDLcFFjQup7z/uHyPd854Ru9Xk225IjokEpYbtNJgi2lC6tBbAlmkCsS6yHB1NbpvF1MSYTzcUOIdWoXMGv/SyoNP/+vJYz/KtILuROo=
ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.ibm.com; spf=pass smtp.mailfrom=linux.ibm.com; dkim=pass (2048-bit key) header.d=ibm.com header.i=@ibm.com header.b=SZyqH4AD; arc=none smtp.client-ip=148.163.158.5
Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.ibm.com
Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.ibm.com
Authentication-Results: smtp.subspace.kernel.org;
	dkim=pass (2048-bit key) header.d=ibm.com header.i=@ibm.com header.b="SZyqH4AD"
Received: from pps.filterd (m0356516.ppops.net [127.0.0.1])
	by mx0a-001b2d01.pphosted.com (8.18.1.2/8.18.1.2) with ESMTP id 49PDiHZn026362;
	Fri, 25 Oct 2024 14:30:24 GMT
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=cc
	:content-type:date:from:in-reply-to:message-id:mime-version
	:references:subject:to; s=pp1; bh=4aSHpzunucJoG38Awg1OmiC6bUnrZj
	v8s+7/Ehjm9d8=; b=SZyqH4ADNt0w8ayshcFlgHgONelo/+jptXUf92oUsCZO71
	QAcaz9uOk+N4ksyKrqaMstnTx6HKBVzEteohZ/TyqCEC7VJCyZap6AtHbIVHaoCn
	TC/7VcsC9ZSEgwI3nRFb7UzuSnQWTZvPcPwyJq27r2ZoBslJP90a2nEHqhmRRD/n
	hlUqLsaSHrHMsC/ieKyEs73CKt7hlVK6zCZ72H4JCPOkULqcI4z1lqZMBxVCEzIF
	Bbsy1kHLc87Tl0UJBwg3vpT6q4ppqxKhj9ou/YH6qZgmATe50bZq6K39THJIqb9M
	c0glKC7Q5faVN6ntoQc7p8VsgRKfV5LPrcgHZMIQ==
Received: from pps.reinject (localhost [127.0.0.1])
	by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 42emaf6jke-1
	(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT);
	Fri, 25 Oct 2024 14:30:24 +0000 (GMT)
Received: from m0356516.ppops.net (m0356516.ppops.net [127.0.0.1])
	by pps.reinject (8.18.0.8/8.18.0.8) with ESMTP id 49PEUNnv001597;
	Fri, 25 Oct 2024 14:30:23 GMT
Received: from ppma12.dal12v.mail.ibm.com (dc.9e.1632.ip4.static.sl-reverse.com [50.22.158.220])
	by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 42emaf6jkb-1
	(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT);
	Fri, 25 Oct 2024 14:30:23 +0000 (GMT)
Received: from pps.filterd (ppma12.dal12v.mail.ibm.com [127.0.0.1])
	by ppma12.dal12v.mail.ibm.com (8.18.1.2/8.18.1.2) with ESMTP id 49PEQCL0001777;
	Fri, 25 Oct 2024 14:30:22 GMT
Received: from smtprelay01.fra02v.mail.ibm.com ([9.218.2.227])
	by ppma12.dal12v.mail.ibm.com (PPS) with ESMTPS id 42emk9p39j-1
	(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT);
	Fri, 25 Oct 2024 14:30:22 +0000
Received: from smtpav03.fra02v.mail.ibm.com (smtpav03.fra02v.mail.ibm.com [10.20.54.102])
	by smtprelay01.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 49PEUL3156557984
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK);
	Fri, 25 Oct 2024 14:30:21 GMT
Received: from smtpav03.fra02v.mail.ibm.com (unknown [127.0.0.1])
	by IMSVA (Postfix) with ESMTP id 1B29820040;
	Fri, 25 Oct 2024 14:30:21 +0000 (GMT)
Received: from smtpav03.fra02v.mail.ibm.com (unknown [127.0.0.1])
	by IMSVA (Postfix) with ESMTP id 0E83320043;
	Fri, 25 Oct 2024 14:30:19 +0000 (GMT)
Received: from li-c6426e4c-27cf-11b2-a85c-95d65bc0de0e.ibm.com (unknown [9.39.31.149])
	by smtpav03.fra02v.mail.ibm.com (Postfix) with ESMTPS;
	Fri, 25 Oct 2024 14:30:18 +0000 (GMT)
Date: Fri, 25 Oct 2024 20:00:16 +0530
From: Gautam Menghani <gautam@linux.ibm.com>
To: "Ritesh Harjani (IBM)" <ritesh.list@gmail.com>
Cc: mpe@ellerman.id.au, npiggin@gmail.com, christophe.leroy@csgroup.eu,
        naveen@kernel.org, linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] KVM: PPC: Book3S HV: Mask off LPCR_MER for a vCPU
 before running it to avoid spurious interrupts
Message-ID: <4enqw22rinnzklk7b3afty2r54pdlqhm7xry3r56d4levavgc5@znfakd4kw7wy>
References: <20241024173417.95395-1-gautam@linux.ibm.com>
 <871q04oguf.fsf@gmail.com>
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <871q04oguf.fsf@gmail.com>
X-TM-AS-GCONF: 00
X-Proofpoint-GUID: sh9bilT6NGGhLe9wFw9lFTIvmUIEdRug
X-Proofpoint-ORIG-GUID: Zx4ni3WpydW2FSAYVKzhcQLtXYUhIDM6
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1051,Hydra:6.0.680,FMLib:17.12.62.30
 definitions=2024-10-15_01,2024-10-11_01,2024-09-30_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0
 lowpriorityscore=0 phishscore=0 spamscore=0 mlxscore=0 mlxlogscore=963
 priorityscore=1501 suspectscore=0 clxscore=1011 impostorscore=0
 bulkscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.19.0-2409260000 definitions=main-2410250110

On Fri, Oct 25, 2024 at 11:37:52AM +0530, Ritesh Harjani (IBM) wrote:
> Gautam Menghani <gautam@linux.ibm.com> writes:
> 
> > Mask off the LPCR_MER bit before running a vCPU to ensure that it is not
> > set if there are no pending interrupts. Running a vCPU with LPCR_MER bit
> > set and no pending interrupts results in L2 vCPU getting an infinite flood
> > of spurious interrupts. The 'if check' in kvmhv_run_single_vcpu() sets
> > the LPCR_MER bit if there are pending interrupts.
> >
> > The spurious flood problem can be observed in 2 cases:
> > 1. Crashing the guest while interrupt heavy workload is running
> >   a. Start a L2 guest and run an interrupt heavy workload (eg: ipistorm)
> >   b. While the workload is running, crash the guest (make sure kdump
> >      is configured)
> >   c. Any one of the vCPUs of the guest will start getting an infinite
> >      flood of spurious interrupts.
> >
> > 2. Running LTP stress tests in multiple guests at the same time
> >    a. Start 4 L2 guests.
> >    b. Start running LTP stress tests on all 4 guests at same time.
> >    c. In some time, any one/more of the vCPUs of any of the guests will
> >       start getting an infinite flood of spurious interrupts.
> >
> > The root cause of both the above issues is the same:
> > 1. A NMI is sent to a running vCPU that has LPCR_MER bit set.
> > 2. In the NMI path, all registers are refreshed, i.e, H_GUEST_GET_STATE
> >    is called for all the registers.
> > 3. When H_GUEST_GET_STATE is called for lpcr, the vcpu->arch.vcore->lpcr
> >    of that vCPU at L1 level gets updated with LPCR_MER set to 1, and this
> >    new value is always used whenever that vCPU runs, regardless of whether
> >    there was a pending interrupt.
> > 4. Since LPCR_MER is set, the vCPU in L2 always jumps to the external
> >    interrupt handler, and this cycle never ends.
> >
> > Fix the spurious flood by making sure a vCPU's LPCR_MER is always masked
> > before running a vCPU.
> >
> > Fixes: ec0f6639fa88 ("KVM: PPC: Book3S HV nestedv2: Ensure LPCR_MER bit is passed to the L0")
> > Cc: stable@vger.kernel.org # v6.8+
> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> > ---
> > V1 -> V2:
> > 1. Mask off the LPCR_MER in vcpu->arch.vcore->lpcr instead of resetting
> > it so that we avoid grabbing vcpu->arch.vcore->lock. (Suggested by
> > Ritesh in an internal review)
> 
> Thanks Gautam for addressing the review comment. But let me improve the
> changelog a little to make it more accurate for others too.
> 
> Removed the macro which was silently clearing LPCR_MER bit from vcore->lpcr
> and instead just mask it off while sending it to kvmhv_run_single_vcpu().
> Added an inline comment describing the reason to avoid anyone tipping
> it over. - (suggested ...)

As suggested by mpe, it would be better to clear LPCR_MER inside
kvmhv_run_single_vcpu() where we check for pending interrupts. But I'll
add the rest of your changelog suggestion in v3.
> 
> 
> Yes, that would also mean that no need of taking any vcore lock since we
> are not modifying any of the vcore state variables which came up in the
> internal review discussion.
> Having said that it will be good to document the usage of vcore->lock
> above the struct kvmppc_vcore definition. Because it isn't obvious of
> when all it should be taken and/or what all it protects?

Yes agreed, I'll send a separate patch documenting this.
> 
> >
> >  arch/powerpc/kvm/book3s_hv.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> > index 8f7d7e37bc8c..b8701b5dde50 100644
> > --- a/arch/powerpc/kvm/book3s_hv.c
> > +++ b/arch/powerpc/kvm/book3s_hv.c
> > @@ -5089,9 +5089,19 @@ static int kvmppc_vcpu_run_hv(struct kvm_vcpu *vcpu)
> >  
> >  	do {
> >  		accumulate_time(vcpu, &vcpu->arch.guest_entry);
> > +		/*
> > +		 * L1's copy of L2's lpcr (vcpu->arch.vcore->lpcr) can get its MER bit
> > +		 * unexpectedly set - for e.g. during NMI handling when all register
> > +		 * states are synchronized from L0 to L1. L1 needs to inform L0 about
> > +		 * MER=1 only when there are pending external interrupts.
> > +		 * kvmhv_run_single_vcpu() anyway sets MER bit if there are pending
> > +		 * external interrupts. Hence, mask off MER bit when passing vcore->lpcr
> > +		 * here as otherwise it may generate spurious interrupts in L2 KVM
> > +		 * causing an endless loop, which results in L2 guest getting hung.
> > +		 */
> 
> Thanks for describing this inline.
> 
> >  		if (cpu_has_feature(CPU_FTR_ARCH_300))
> >  			r = kvmhv_run_single_vcpu(vcpu, ~(u64)0,
> > -						  vcpu->arch.vcore->lpcr);
> > +						  vcpu->arch.vcore->lpcr & ~LPCR_MER);
> 
> While still at it - 
> I too like mpe suggestion to clear the LPCR_MER bit at one place
> itself within kvmhv_run_single_vcpu(). 

Yes will send v3.
> 
> >  		else
> >  			r = kvmppc_run_vcpu(vcpu);
> >  
> > -- 
> > 2.47.0
> 
> -ritesh

Thanks,
Gautam

