[PATCH v1 0/2] virito-mem: one fix and VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE

[PATCH v1 0/2] virito-mem: one fix and VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE

[David Hildenbrand]

One minor fix and introduction of / support for
VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE.

Looking into supporting file-based memory backends (shmem, hugetlbfs, ...)
for virtio-mem in QEMU cleanly, I realized that we have to indicate that
unplugged memory is completely inaccessible. Otherwise, Linux might in
corner cases read unplugged memory, which is harder to support (and harder
to protect from) in a hypervisor than with anonymous memory where we have
a shared zeropage.

To support VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE cleanly, we cannot (un)plug
memory in SBM (Sub Block Mode) and instead, can only support adding/
removing individual Linux memory blocks (e.g., 128MB on x86-64).

While we might still be able to allow for reading unplugged memory with
file-based memory backends in the future (and I have plans/prototypes for
it), at least in the near future we cannot support it.

David Hildenbrand (2):
  virtio-mem: don't read big block size in SBM
  virtio-mem: support VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE

 drivers/virtio/virtio_mem.c     | 27 ++++++++++++++++++++-------
 include/uapi/linux/virtio_mem.h | 10 +++++++---
 2 files changed, 27 insertions(+), 10 deletions(-)

-- 
2.29.2

[PATCH v1 1/2] virtio-mem: don't read big block size in SBM

[David Hildenbrand]

We are reading the a BBM (Big Block Mode) value while in SBM (Sub Block
Mode) while initializing. Fortunately, vm->bbm.bb_size maps to some counter
in the vm->sbm.mb_count array, which is 0 at that point in time.

No harm done; still, this was unintended and is not future-proof.

Fixes: 4ba50cd3355d ("virtio-mem: Big Block Mode (BBM) memory hotplug")
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Marek Kedzierski <mkedzier@redhat.com>
Cc: Hui Zhu <teawater@gmail.com>
Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
Cc: Wei Yang <richard.weiyang@linux.alibaba.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 drivers/virtio/virtio_mem.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index 9fc9ec4a25f5..6d4e01c4e2fa 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -2409,6 +2409,10 @@ static int virtio_mem_init(struct virtio_mem *vm)
 		dev_warn(&vm->vdev->dev,
 			 "Some memory is not addressable. This can make some memory unusable.\n");
 
+	/* Prepare the offline threshold - make sure we can add two blocks. */
+	vm->offline_threshold = max_t(uint64_t, 2 * memory_block_size_bytes(),
+				      VIRTIO_MEM_DEFAULT_OFFLINE_THRESHOLD);
+
 	/*
 	 * We want subblocks to span at least MAX_ORDER_NR_PAGES and
 	 * pageblock_nr_pages pages. This:
@@ -2453,14 +2457,11 @@ static int virtio_mem_init(struct virtio_mem *vm)
 		addr = vm->addr + vm->bbm.bb_size - 1;
 		vm->bbm.first_bb_id = virtio_mem_phys_to_bb_id(vm, addr);
 		vm->bbm.next_bb_id = vm->bbm.first_bb_id;
-	}
 
-	/* Prepare the offline threshold - make sure we can add two blocks. */
-	vm->offline_threshold = max_t(uint64_t, 2 * memory_block_size_bytes(),
-				      VIRTIO_MEM_DEFAULT_OFFLINE_THRESHOLD);
-	/* In BBM, we also want at least two big blocks. */
-	vm->offline_threshold = max_t(uint64_t, 2 * vm->bbm.bb_size,
-				      vm->offline_threshold);
+		/* Make sure we can add two big blocks. */
+		vm->offline_threshold = max_t(uint64_t, 2 * vm->bbm.bb_size,
+					      vm->offline_threshold);
+	}
 
 	dev_info(&vm->vdev->dev, "start address: 0x%llx", vm->addr);
 	dev_info(&vm->vdev->dev, "region size: 0x%llx", vm->region_size);
-- 
2.29.2

[PATCH v1 2/2] virtio-mem: support VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE

[David Hildenbrand]

The spec currently states that while unplugged memory should not be
read, the device has to allow for reading unplugged memory inside the
usable region. The primary motivation for this default handling was that
in some corner cases in Linux, unplugged memory inside added Linux memory
blocks (and exposed via /proc/iomem as "System RAM (...)") might still
be read. So to support SBM (Sub Block Mode) in Linux cleanly, the device
has to support reading unplugged memory.

One example is kdump(): when makedumpfile isn't used
or when an older version is used, PG_offline is ignored and unplugged
memory might still be read. Another corner-case example is /dev/mem: even
with STRICT_DEVMEM, some unplugged memory might be read by tools
automatically.

For example, QEMU won't support reading unplugged memory with
file-backed memory backends initially: there is no shared zero page,
thus, special handling will be required in the future to allow for
reading unplugged memory when using a file backing (tmpfs/shmem, hugetlbfs,
...).

The device will indicate VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE and fail
device initialization if the driver does not indicate support. The
result is that Linux won't be able to make use of any memory without
this change. With this change, Linux will at least be able to (un)plug
in Linux memory block granularity. Print an info so this handling can
be identified.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Marek Kedzierski <mkedzier@redhat.com>
Cc: Hui Zhu <teawater@gmail.com>
Cc: Sebastien Boeuf <sebastien.boeuf@intel.com>
Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
Cc: Wei Yang <richard.weiyang@linux.alibaba.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 drivers/virtio/virtio_mem.c     | 12 ++++++++++++
 include/uapi/linux/virtio_mem.h | 10 +++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index 6d4e01c4e2fa..58d8df528728 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -2425,6 +2425,17 @@ static int virtio_mem_init(struct virtio_mem *vm)
 			pageblock_nr_pages) * PAGE_SIZE;
 	sb_size = max_t(uint64_t, vm->device_block_size, sb_size);
 
+	/*
+	 * Unplugged memory might be read in corner cases, for example, via
+	 * kdump. Fallback to adding/removing individual Linux memory blocks.
+	 */
+	if (sb_size < memory_block_size_bytes() && !force_bbm &&
+	    virtio_has_feature(vm->vdev, VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE)) {
+		sb_size = memory_block_size_bytes();
+		dev_info(&vm->vdev->dev,
+			 "The device does not support reading unplugged memory: using big block mode\n");
+	}
+
 	if (sb_size < memory_block_size_bytes() && !force_bbm) {
 		/* SBM: At least two subblocks per Linux memory block. */
 		vm->in_sbm = true;
@@ -2711,6 +2722,7 @@ static unsigned int virtio_mem_features[] = {
 #if defined(CONFIG_NUMA) && defined(CONFIG_ACPI_NUMA)
 	VIRTIO_MEM_F_ACPI_PXM,
 #endif
+	VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE,
 };
 
 static const struct virtio_device_id virtio_mem_id_table[] = {
diff --git a/include/uapi/linux/virtio_mem.h b/include/uapi/linux/virtio_mem.h
index 70e01c687d5e..6ac77cfb8aca 100644
--- a/include/uapi/linux/virtio_mem.h
+++ b/include/uapi/linux/virtio_mem.h
@@ -68,9 +68,11 @@
  * explicitly triggered (VIRTIO_MEM_REQ_UNPLUG).
  *
  * There are no guarantees what will happen if unplugged memory is
- * read/written. Such memory should, in general, not be touched. E.g.,
- * even writing might succeed, but the values will simply be discarded at
- * random points in time.
+ * read/written. Often, unplugged memory inside the usable region can
+ * be read, to simplify creation of memory dumps; however, some devices
+ * don't even support that. Unplugged memory should, in general, not be
+ * touched. E.g., even writing might succeed, but the values will simply be
+ * discarded at random points in time.
  *
  * It can happen that the device cannot process a request, because it is
  * busy. The device driver has to retry later.
@@ -87,6 +89,8 @@
 
 /* node_id is an ACPI PXM and is valid */
 #define VIRTIO_MEM_F_ACPI_PXM		0
+/* any unplugged memory must never be accessed */
+#define VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE	1
 
 
 /* --- virtio-mem: guest -> host requests --- */
-- 
2.29.2