aboutsummaryrefslogtreecommitdiff
path: root/hw
diff options
context:
space:
mode:
authorDavid Hildenbrand <david@redhat.com>2024-01-17 14:55:54 +0100
committerDavid Hildenbrand <david@redhat.com>2024-02-06 08:14:59 +0100
commit540a1abbf0b243e4cfb4333c5d30a041f7080ba4 (patch)
treed9f2da1ab643c2c1b4cb04c8e8097ad93bf7e66e /hw
parentf77c5f38f49c71bc14cf1019ac92b0b95f572414 (diff)
downloadqemu-540a1abbf0b243e4cfb4333c5d30a041f7080ba4.zip
qemu-540a1abbf0b243e4cfb4333c5d30a041f7080ba4.tar.gz
qemu-540a1abbf0b243e4cfb4333c5d30a041f7080ba4.tar.bz2
memory-device: reintroduce memory region size check
We used to check that the memory region size is multiples of the overall requested address alignment for the device memory address. We removed that check, because there are cases (i.e., hv-balloon) where devices unconditionally request an address alignment that has a very large alignment (i.e., 32 GiB), but the actual memory device size might not be multiples of that alignment. However, this change: (a) allows for some practically impossible DIMM sizes, like "1GB+1 byte". (b) allows for DIMMs that partially cover hugetlb pages, previously reported in [1]. Both scenarios don't make any sense: we might even waste memory. So let's reintroduce that check, but only check that the memory region size is multiples of the memory region alignment (i.e., page size, huge page size), but not any additional memory device requirements communicated using md->get_min_alignment(). The following examples now fail again as expected: (a) 1M with 2M THP qemu-system-x86_64 -m 4g,maxmem=16g,slots=1 -S -nodefaults -nographic \ -object memory-backend-ram,id=mem1,size=1M \ -device pc-dimm,id=dimm1,memdev=mem1 -> backend memory size must be multiple of 0x200000 (b) 1G+1byte qemu-system-x86_64 -m 4g,maxmem=16g,slots=1 -S -nodefaults -nographic \ -object memory-backend-ram,id=mem1,size=1073741825B \ -device pc-dimm,id=dimm1,memdev=mem1 -> backend memory size must be multiple of 0x200000 (c) Unliagned hugetlb size (2M) qemu-system-x86_64 -m 4g,maxmem=16g,slots=1 -S -nodefaults -nographic \ -object memory-backend-file,id=mem1,mem-path=/dev/hugepages/tmp,size=511M \ -device pc-dimm,id=dimm1,memdev=mem1 backend memory size must be multiple of 0x200000 (d) Unliagned hugetlb size (1G) qemu-system-x86_64 -m 4g,maxmem=16g,slots=1 -S -nodefaults -nographic \ -object memory-backend-file,id=mem1,mem-path=/dev/hugepages1G/tmp,size=2047M \ -device pc-dimm,id=dimm1,memdev=mem1 -> backend memory size must be multiple of 0x40000000 Note that this fix depends on a hv-balloon change to communicate its additional alignment requirements using get_min_alignment() instead of through the memory region. [1] https://lkml.kernel.org/r/f77d641d500324525ac036fe1827b3070de75fc1.1701088320.git.mprivozn@redhat.com Message-ID: <20240117135554.787344-3-david@redhat.com> Reported-by: Zhenyu Zhang <zhenyzha@redhat.com> Reported-by: Michal Privoznik <mprivozn@redhat.com> Fixes: eb1b7c4bd413 ("memory-device: Drop size alignment check") Tested-by: Zhenyu Zhang <zhenyzha@redhat.com> Tested-by: Mario Casquero <mcasquer@redhat.com> Reviewed-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> Signed-off-by: David Hildenbrand <david@redhat.com>
Diffstat (limited to 'hw')
-rw-r--r--hw/mem/memory-device.c14
1 files changed, 14 insertions, 0 deletions
diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
index a1b1af2..e098585 100644
--- a/hw/mem/memory-device.c
+++ b/hw/mem/memory-device.c
@@ -374,6 +374,20 @@ void memory_device_pre_plug(MemoryDeviceState *md, MachineState *ms,
goto out;
}
+ /*
+ * We always want the memory region size to be multiples of the memory
+ * region alignment: for example, DIMMs with 1G+1byte size don't make
+ * any sense. Note that we don't check that the size is multiples
+ * of any additional alignment requirements the memory device might
+ * have when it comes to the address in physical address space.
+ */
+ if (!QEMU_IS_ALIGNED(memory_region_size(mr),
+ memory_region_get_alignment(mr))) {
+ error_setg(errp, "backend memory size must be multiple of 0x%"
+ PRIx64, memory_region_get_alignment(mr));
+ return;
+ }
+
if (legacy_align) {
align = *legacy_align;
} else {