From patchwork Tue Mar 5 05:11:33 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Gibson X-Patchwork-Id: 1051618 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.b="PdLm19NS"; dkim-atps=neutral Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44D4lR1vDHz9s3q for ; Tue, 5 Mar 2019 16:12:35 +1100 (AEDT) Received: from localhost ([127.0.0.1]:37573 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h12NV-0001Ru-5S for incoming@patchwork.ozlabs.org; Tue, 05 Mar 2019 00:12:33 -0500 Received: from eggs.gnu.org ([209.51.188.92]:54533) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h12Mi-0001Cy-ED for qemu-devel@nongnu.org; Tue, 05 Mar 2019 00:11:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h12Mh-0005qj-Jc for qemu-devel@nongnu.org; Tue, 05 Mar 2019 00:11:44 -0500 Received: from ozlabs.org ([2401:3900:2:1::2]:56483) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1h12Mg-0005mz-Rs; Tue, 05 Mar 2019 00:11:43 -0500 Received: by ozlabs.org (Postfix, from userid 1007) id 44D4kL1l9vz9s47; Tue, 5 Mar 2019 16:11:38 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gibson.dropbear.id.au; s=201602; t=1551762698; bh=cswdvFfIgyqvGN/o6bK8yh8/+qNC6VyOyrGbbfmYugc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=PdLm19NSqfjaKR9NVd0TMusrJhj5lbQtmE9mIHY8RhM1P1sKdImK2YzcWPW5Qc2py xR85au0CYbRxOuN4oDo6ZxG9NltvA+IIwYIGPM7wa3WmWvqbXMZv8pVJiOz8WLTIt5 X7bLK4Z2sJtev7+eDOzp9N78CsM04MDsCB6pyxPQ= From: David Gibson To: mst@redhat.com, david@redhat.com Date: Tue, 5 Mar 2019 16:11:33 +1100 Message-Id: <20190305051134.27930-2-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190305051134.27930-1-david@gibson.dropbear.id.au> References: <20190305051134.27930-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2401:3900:2:1::2 Subject: [Qemu-devel] [RFC 1/2] virtio-balloon: Fix possible guest memory corruption with inflates & deflates X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, David Gibson Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" This fixes a balloon bug with a nasty consequence - potentially corrupting guest memory - but which is extremely unlikely to be triggered in practice. The balloon always works in 4kiB units, but the host could have a larger page size on certain platforms. Since ed48c59 "virtio-balloon: Safely handle BALLOON_PAGE_SIZE < host page size" we've handled this by accumulating requests to balloon 4kiB subpages until they formed a full host page. Since f6deb6d "virtio-balloon: Remove unnecessary MADV_WILLNEED on deflate" we essentially ignore deflate requests. Suppose we have a host with 8kiB pages, and one host page has subpages A & B. If we get this sequence of events - inflate A deflate A inflate B - the current logic will discard the whole host page. That's incorrect because the guest has deflated subpage A, and could have written important data to it. This patch fixes the problem by adjusting our state information about partially ballooned host pages when deflate requests are received. Fixes: ed48c59 "virtio-balloon: Safely handle BALLOON_PAGE_SIZE < host page size" Signed-off-by: David Gibson --- hw/virtio/virtio-balloon.c | 48 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index d3f2913a85..e5e82b556d 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -111,6 +111,43 @@ static void balloon_inflate_page(VirtIOBalloon *balloon, } } +static void balloon_deflate_page(VirtIOBalloon *balloon, + MemoryRegion *mr, hwaddr offset) +{ + void *addr = memory_region_get_ram_ptr(mr) + offset; + RAMBlock *rb; + size_t rb_page_size; + ram_addr_t ram_offset, host_page_base; + + /* XXX is there a better way to get to the RAMBlock than via a + * host address? */ + rb = qemu_ram_block_from_host(addr, false, &ram_offset); + rb_page_size = qemu_ram_pagesize(rb); + host_page_base = ram_offset & ~(rb_page_size - 1); + + if (balloon->pbp + && rb == balloon->pbp->rb + && host_page_base == balloon->pbp->base) { + int subpages = rb_page_size / BALLOON_PAGE_SIZE; + + /* + * This means the guest has asked to discard some of the 4kiB + * subpages of a host page, but then changed its mind and + * asked to keep them after all. It's exceedingly unlikely + * for a guest to do this in practice, but handle it anyway, + * since getting it wrong could mean discarding memory the + * guest is still using. */ + bitmap_clear(balloon->pbp->bitmap, + (ram_offset - balloon->pbp->base) / BALLOON_PAGE_SIZE, + subpages); + + if (bitmap_empty(balloon->pbp->bitmap, subpages)) { + free(balloon->pbp); + balloon->pbp = NULL; + } + } +} + static const char *balloon_stat_names[] = { [VIRTIO_BALLOON_S_SWAP_IN] = "stat-swap-in", [VIRTIO_BALLOON_S_SWAP_OUT] = "stat-swap-out", @@ -314,8 +351,15 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq) trace_virtio_balloon_handle_output(memory_region_name(section.mr), pa); - if (!qemu_balloon_is_inhibited() && vq != s->dvq) { - balloon_inflate_page(s, section.mr, section.offset_within_region); + if (!qemu_balloon_is_inhibited()) { + if (vq == s->ivq) { + balloon_inflate_page(s, section.mr, + section.offset_within_region); + } else if (vq == s->dvq) { + balloon_deflate_page(s, section.mr, section.offset_within_region); + } else { + g_assert_not_reached(); + } } memory_region_unref(section.mr); } From patchwork Tue Mar 5 05:11:34 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Gibson X-Patchwork-Id: 1051619 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.b="a4HXPOPX"; dkim-atps=neutral Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44D4nW5C8lz9s3q for ; Tue, 5 Mar 2019 16:14:22 +1100 (AEDT) Received: from localhost ([127.0.0.1]:37584 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h12PE-0002bX-3W for incoming@patchwork.ozlabs.org; Tue, 05 Mar 2019 00:14:20 -0500 Received: from eggs.gnu.org ([209.51.188.92]:54512) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h12Mh-0001Ch-Kj for qemu-devel@nongnu.org; Tue, 05 Mar 2019 00:11:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h12Mg-0005pe-S1 for qemu-devel@nongnu.org; Tue, 05 Mar 2019 00:11:43 -0500 Received: from ozlabs.org ([2401:3900:2:1::2]:46527) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1h12Mg-0005n4-AC; Tue, 05 Mar 2019 00:11:42 -0500 Received: by ozlabs.org (Postfix, from userid 1007) id 44D4kL36N2z9s7T; Tue, 5 Mar 2019 16:11:38 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gibson.dropbear.id.au; s=201602; t=1551762698; bh=I+ybT0xcEk1EEZor34IilzQ9tq530tHRymLSQoAIItI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=a4HXPOPXGLyMVsw038sU3tB1BqccoShMaXRfhlLC1scXh0fGamwElqpY6CGTpqGpP blumdT5KiRsTng/zNhBhg2V/82JwS/bPctrT3MqueZdFBfjx9JPPa3ul42BQ1EXDkr WSsrdqwGGhtpzHuaSqdIGOBzb/3WZGDGPs+RuXVU= From: David Gibson To: mst@redhat.com, david@redhat.com Date: Tue, 5 Mar 2019 16:11:34 +1100 Message-Id: <20190305051134.27930-3-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190305051134.27930-1-david@gibson.dropbear.id.au> References: <20190305051134.27930-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2401:3900:2:1::2 Subject: [Qemu-devel] [RFC 2/2] virtio-balloon: Restore MADV_WILLNEED hint on balloon deflate X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, David Gibson Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Prior to f6deb6d9 "virtio-balloon: Remove unnecessary MADV_WILLNEED on deflate", the balloon device issued an madvise() MADV_WILLNEED on pages removed from the balloon. That would hint to the host kernel that the pages were likely to be needed by the guest in the near future. It's unclear if this is actually valuable or not, and so f6deb6d9 removed this, essentially ignoring balloon deflate requests. However, concerns have been raised that this might cause a performance regression by causing extra latency for the guest in certain configurations. So, until we can get actual benchmark data to see if that's the case, this restores (by default) the old behaviour, issuing a MADV_WILLNEED when a page is removed from the balloon. A new property on the balloon device "hint-on-deflate" can be set to false to remove this behaviour for testing. Signed-off-by: David Gibson --- hw/virtio/virtio-balloon.c | 15 +++++++++++++++ include/hw/virtio/virtio-balloon.h | 1 + 2 files changed, 16 insertions(+) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index e5e82b556d..69968502d9 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -146,6 +146,20 @@ static void balloon_deflate_page(VirtIOBalloon *balloon, balloon->pbp = NULL; } } + + if (balloon->hint_on_deflate) { + void *host_addr = (void *)((uintptr_t)addr & ~(rb_page_size - 1)); + int ret; + + /* When a page is deflated, we hint the whole host page it + * lives on, since we can't do anything smaller */ + ret = qemu_madvise(host_addr, rb_page_size, QEMU_MADV_WILLNEED); + if (ret != 0) { + warn_report("Couldn't MADV_WILLNEED on balloon deflate: %s", + strerror(errno)); + /* Otherwise ignore, failing to page hint shouldn't be fatal */ + } + } } static const char *balloon_stat_names[] = { @@ -622,6 +636,7 @@ static const VMStateDescription vmstate_virtio_balloon = { static Property virtio_balloon_properties[] = { DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features, VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false), + DEFINE_PROP_BOOL("hint-on-deflate", VirtIOBalloon, hint_on_deflate, true), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h index 99dcd6d105..69732cedaa 100644 --- a/include/hw/virtio/virtio-balloon.h +++ b/include/hw/virtio/virtio-balloon.h @@ -45,6 +45,7 @@ typedef struct VirtIOBalloon { int64_t stats_poll_interval; uint32_t host_features; PartiallyBalloonedPage *pbp; + bool hint_on_deflate; } VirtIOBalloon; #endif