linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kurz <groug@kaod.org>
To: Paul Mackerras <paulus@ozlabs.org>
Cc: linux-kernel@vger.kernel.org, kvm-ppc@vger.kernel.org,
	"Satheesh Rajendran" <sathnaga@linux.vnet.ibm.com>,
	"Cédric Le Goater" <clg@kaod.org>,
	"Lijun Pan" <ljp@linux.ibm.com>,
	stable@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	"David Gibson" <david@gibson.dropbear.id.au>
Subject: [PATCH] KVM: PPC: Book3S HV: XIVE: Free previous EQ page when setting up a new one
Date: Mon, 11 Nov 2019 10:49:26 +0100	[thread overview]
Message-ID: <157346576671.818016.10401178701091199969.stgit@bahia.lan> (raw)

The EQ page is allocated by the guest and then passed to the hypervisor
with the H_INT_SET_QUEUE_CONFIG hcall. A reference is taken on the page
before handing it over to the HW. This reference is dropped either when
the guest issues the H_INT_RESET hcall or when the KVM device is released.
But, the guest can legitimately call H_INT_SET_QUEUE_CONFIG several times
to reset the EQ (vCPU hot unplug) or set a new EQ (guest reboot). In both
cases the EQ page reference is leaked. This is especially visible when
the guest memory is backed with huge pages: start a VM up to the guest
userspace, either reboot it or unplug a vCPU, quit QEMU. The leak is
observed by comparing the value of HugePages_Free in /proc/meminfo before
and after the VM is run.

Note that the EQ reset path seems to be calling put_page() but this is
done after xive_native_configure_queue() which clears the qpage field
in the XIVE queue structure, ie. the put_page() block is a nop and the
previous page pointer was just overwritten anyway. In the other case of
configuring a new EQ page, nothing seems to be done to release the old
one.

Fix both cases by always calling put_page() on the existing EQ page in
kvmppc_xive_native_set_queue_config(). This is a seemless change for the
EQ reset case. However this causes xive_native_configure_queue() to be
called twice for the new EQ page case: one time to reset the EQ and another
time to configure the new page. This is needed because we cannot release
the EQ page before calling xive_native_configure_queue() since it may still
be used by the HW. We cannot modify xive_native_configure_queue() to drop
the reference either because this function is also used by the XICS-on-XIVE
device which requires free_pages() instead of put_page(). This isn't a big
deal anyway since H_INT_SET_QUEUE_CONFIG isn't a hot path.

Reported-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Cc: stable@vger.kernel.org # v5.2
Fixes: 13ce3297c576 ("KVM: PPC: Book3S HV: XIVE: Add controls for the EQ configuration")
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 arch/powerpc/kvm/book3s_xive_native.c |   21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
index 34bd123fa024..8ab908d23dc2 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -570,10 +570,12 @@ static int kvmppc_xive_native_set_queue_config(struct kvmppc_xive *xive,
 		 __func__, server, priority, kvm_eq.flags,
 		 kvm_eq.qshift, kvm_eq.qaddr, kvm_eq.qtoggle, kvm_eq.qindex);
 
-	/* reset queue and disable queueing */
-	if (!kvm_eq.qshift) {
-		q->guest_qaddr  = 0;
-		q->guest_qshift = 0;
+	/*
+	 * Reset queue and disable queueing. It will be re-enabled
+	 * later on if the guest is configuring a new EQ page.
+	 */
+	if (q->guest_qshift) {
+		page = virt_to_page(q->qpage);
 
 		rc = xive_native_configure_queue(xc->vp_id, q, priority,
 						 NULL, 0, true);
@@ -583,12 +585,13 @@ static int kvmppc_xive_native_set_queue_config(struct kvmppc_xive *xive,
 			return rc;
 		}
 
-		if (q->qpage) {
-			put_page(virt_to_page(q->qpage));
-			q->qpage = NULL;
-		}
+		put_page(page);
 
-		return 0;
+		if (!kvm_eq.qshift) {
+			q->guest_qaddr  = 0;
+			q->guest_qshift = 0;
+			return 0;
+		}
 	}
 
 	/*


             reply	other threads:[~2019-11-11  9:51 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-11  9:49 Greg Kurz [this message]
2019-11-11 11:26 ` [PATCH] KVM: PPC: Book3S HV: XIVE: Free previous EQ page when setting up a new one Cédric Le Goater
2019-11-12 12:01   ` Greg Kurz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=157346576671.818016.10401178701091199969.stgit@bahia.lan \
    --to=groug@kaod.org \
    --cc=clg@kaod.org \
    --cc=david@gibson.dropbear.id.au \
    --cc=kvm-ppc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=ljp@linux.ibm.com \
    --cc=paulus@ozlabs.org \
    --cc=sathnaga@linux.vnet.ibm.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).