FAILED: patch "[PATCH] KVM: PPC: Book3S HV: XIVE: Free escalation interrupts before" failed to apply to 4.19-stable tree

FAILED: patch "[PATCH] KVM: PPC: Book3S HV: XIVE: Free escalation interrupts before" failed to apply to 4.19-stable tree

[gregkh]

The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 237aed48c642328ff0ab19b63423634340224a06 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= <clg@kaod.org>
Date: Tue, 6 Aug 2019 19:25:38 +0200
Subject: [PATCH] KVM: PPC: Book3S HV: XIVE: Free escalation interrupts before
 disabling the VP
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When a vCPU is brought done, the XIVE VP (Virtual Processor) is first
disabled and then the event notification queues are freed. When freeing
the queues, we check for possible escalation interrupts and free them
also.

But when a XIVE VP is disabled, the underlying XIVE ENDs also are
disabled in OPAL. When an END (Event Notification Descriptor) is
disabled, its ESB pages (ESn and ESe) are disabled and loads return all
1s. Which means that any access on the ESB page of the escalation
interrupt will return invalid values.

When an interrupt is freed, the shutdown handler computes a 'saved_p'
field from the value returned by a load in xive_do_source_set_mask().
This value is incorrect for escalation interrupts for the reason
described above.

This has no impact on Linux/KVM today because we don't make use of it
but we will introduce in future changes a xive_get_irqchip_state()
handler. This handler will use the 'saved_p' field to return the state
of an interrupt and 'saved_p' being incorrect, softlockup will occur.

Fix the vCPU cleanup sequence by first freeing the escalation interrupts
if any, then disable the XIVE VP and last free the queues.

Fixes: 90c73795afa2 ("KVM: PPC: Book3S HV: Add a new KVM device for the XIVE native exploitation mode")
Fixes: 5af50993850a ("KVM: PPC: Book3S HV: Native usage of the XIVE interrupt controller")
Cc: stable@vger.kernel.org # v4.12+
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20190806172538.5087-1-clg@kaod.org

diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index e3ba67095895..09f838aa3138 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -1134,20 +1134,22 @@ void kvmppc_xive_cleanup_vcpu(struct kvm_vcpu *vcpu)
 	/* Mask the VP IPI */
 	xive_vm_esb_load(&xc->vp_ipi_data, XIVE_ESB_SET_PQ_01);
 
-	/* Disable the VP */
-	xive_native_disable_vp(xc->vp_id);
-
-	/* Free the queues & associated interrupts */
+	/* Free escalations */
 	for (i = 0; i < KVMPPC_XIVE_Q_COUNT; i++) {
-		struct xive_q *q = &xc->queues[i];
-
-		/* Free the escalation irq */
 		if (xc->esc_virq[i]) {
 			free_irq(xc->esc_virq[i], vcpu);
 			irq_dispose_mapping(xc->esc_virq[i]);
 			kfree(xc->esc_virq_names[i]);
 		}
-		/* Free the queue */
+	}
+
+	/* Disable the VP */
+	xive_native_disable_vp(xc->vp_id);
+
+	/* Free the queues */
+	for (i = 0; i < KVMPPC_XIVE_Q_COUNT; i++) {
+		struct xive_q *q = &xc->queues[i];
+
 		xive_native_disable_queue(xc->vp_id, q, i);
 		if (q->qpage) {
 			free_pages((unsigned long)q->qpage,
diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
index a998823f68a3..368427fcad20 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -67,10 +67,7 @@ void kvmppc_xive_native_cleanup_vcpu(struct kvm_vcpu *vcpu)
 	xc->valid = false;
 	kvmppc_xive_disable_vcpu_interrupts(vcpu);
 
-	/* Disable the VP */
-	xive_native_disable_vp(xc->vp_id);
-
-	/* Free the queues & associated interrupts */
+	/* Free escalations */
 	for (i = 0; i < KVMPPC_XIVE_Q_COUNT; i++) {
 		/* Free the escalation irq */
 		if (xc->esc_virq[i]) {
@@ -79,8 +76,13 @@ void kvmppc_xive_native_cleanup_vcpu(struct kvm_vcpu *vcpu)
 			kfree(xc->esc_virq_names[i]);
 			xc->esc_virq[i] = 0;
 		}
+	}
 
-		/* Free the queue */
+	/* Disable the VP */
+	xive_native_disable_vp(xc->vp_id);
+
+	/* Free the queues */
+	for (i = 0; i < KVMPPC_XIVE_Q_COUNT; i++) {
 		kvmppc_xive_native_cleanup_queue(vcpu, i);
 	}
 

Re: FAILED: patch "[PATCH] KVM: PPC: Book3S HV: XIVE: Free escalation interrupts before" failed to apply to 4.19-stable tree

[Sasha Levin]

On Tue, Oct 08, 2019 at 09:20:08AM +0200, gregkh@linuxfoundation.org wrote:
>
>The patch below does not apply to the 4.19-stable tree.
>If someone wants it applied there, or to any other stable or longterm
>tree, then please email the backport, including the original git commit
>id to <stable@vger.kernel.org>.
>
>thanks,
>
>greg k-h
>
>------------------ original commit in Linus's tree ------------------
>
>From 237aed48c642328ff0ab19b63423634340224a06 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= <clg@kaod.org>
>Date: Tue, 6 Aug 2019 19:25:38 +0200
>Subject: [PATCH] KVM: PPC: Book3S HV: XIVE: Free escalation interrupts before
> disabling the VP
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>When a vCPU is brought done, the XIVE VP (Virtual Processor) is first
>disabled and then the event notification queues are freed. When freeing
>the queues, we check for possible escalation interrupts and free them
>also.
>
>But when a XIVE VP is disabled, the underlying XIVE ENDs also are
>disabled in OPAL. When an END (Event Notification Descriptor) is
>disabled, its ESB pages (ESn and ESe) are disabled and loads return all
>1s. Which means that any access on the ESB page of the escalation
>interrupt will return invalid values.
>
>When an interrupt is freed, the shutdown handler computes a 'saved_p'
>field from the value returned by a load in xive_do_source_set_mask().
>This value is incorrect for escalation interrupts for the reason
>described above.
>
>This has no impact on Linux/KVM today because we don't make use of it
>but we will introduce in future changes a xive_get_irqchip_state()
>handler. This handler will use the 'saved_p' field to return the state
>of an interrupt and 'saved_p' being incorrect, softlockup will occur.
>
>Fix the vCPU cleanup sequence by first freeing the escalation interrupts
>if any, then disable the XIVE VP and last free the queues.
>
>Fixes: 90c73795afa2 ("KVM: PPC: Book3S HV: Add a new KVM device for the XIVE native exploitation mode")
>Fixes: 5af50993850a ("KVM: PPC: Book3S HV: Native usage of the XIVE interrupt controller")
>Cc: stable@vger.kernel.org # v4.12+
>Signed-off-by: Cédric Le Goater <clg@kaod.org>
>Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>Link: https://lore.kernel.org/r/20190806172538.5087-1-clg@kaod.org

I've dropped the xive native part on 4.19 and 4.14 because 90c73795afa24
("KVM: PPC: Book3S HV: Add a new KVM device for the XIVE native
exploitation mode") isn't there.

-- 
Thanks,
Sasha

Re: FAILED: patch "[PATCH] KVM: PPC: Book3S HV: XIVE: Free escalation interrupts before" failed to apply to 4.19-stable tree

[Cédric Le Goater]

On 08/10/2019 23:45, Sasha Levin wrote:
> On Tue, Oct 08, 2019 at 09:20:08AM +0200, gregkh@linuxfoundation.org wrote:
>>
>> The patch below does not apply to the 4.19-stable tree.
>> If someone wants it applied there, or to any other stable or longterm
>> tree, then please email the backport, including the original git commit
>> id to <stable@vger.kernel.org>.
>>
>> thanks,
>>
>> greg k-h
>>
>> ------------------ original commit in Linus's tree ------------------
>>
>> From 237aed48c642328ff0ab19b63423634340224a06 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= <clg@kaod.org>
>> Date: Tue, 6 Aug 2019 19:25:38 +0200
>> Subject: [PATCH] KVM: PPC: Book3S HV: XIVE: Free escalation interrupts before
>> disabling the VP
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
>> When a vCPU is brought done, the XIVE VP (Virtual Processor) is first
>> disabled and then the event notification queues are freed. When freeing
>> the queues, we check for possible escalation interrupts and free them
>> also.
>>
>> But when a XIVE VP is disabled, the underlying XIVE ENDs also are
>> disabled in OPAL. When an END (Event Notification Descriptor) is
>> disabled, its ESB pages (ESn and ESe) are disabled and loads return all
>> 1s. Which means that any access on the ESB page of the escalation
>> interrupt will return invalid values.
>>
>> When an interrupt is freed, the shutdown handler computes a 'saved_p'
>> field from the value returned by a load in xive_do_source_set_mask().
>> This value is incorrect for escalation interrupts for the reason
>> described above.
>>
>> This has no impact on Linux/KVM today because we don't make use of it
>> but we will introduce in future changes a xive_get_irqchip_state()
>> handler. This handler will use the 'saved_p' field to return the state
>> of an interrupt and 'saved_p' being incorrect, softlockup will occur.
>>
>> Fix the vCPU cleanup sequence by first freeing the escalation interrupts
>> if any, then disable the XIVE VP and last free the queues.
>>
>> Fixes: 90c73795afa2 ("KVM: PPC: Book3S HV: Add a new KVM device for the XIVE native exploitation mode")
>> Fixes: 5af50993850a ("KVM: PPC: Book3S HV: Native usage of the XIVE interrupt controller")
>> Cc: stable@vger.kernel.org # v4.12+
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> Link: https://lore.kernel.org/r/20190806172538.5087-1-clg@kaod.org
> 
> I've dropped the xive native part on 4.19 and 4.14 because 90c73795afa24
> ("KVM: PPC: Book3S HV: Add a new KVM device for the XIVE native
> exploitation mode") isn't there.

yes. It was introduced in 5.2. 

The fixes on the XICS-on-XIVE KVM device and the XIVE native KVM device 
are often the same patch because they have a lot in common. 
We should try to separate the patches in the future to ease backport
on the stable trees. Thanks for doing so,

C.