[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v1 05/11] spapr: move memory hotplug size check into
From: |
David Hildenbrand |
Subject: |
[Qemu-devel] [PATCH v1 05/11] spapr: move memory hotplug size check into plug code |
Date: |
Mon, 11 Jun 2018 14:16:49 +0200 |
This might look like a step backwards, but it is not. get_memory_region()
should not be called on uninititalized devices. In general, only
properties should be access, but no "derived" satte like the memory
region.
1. We need duplicate error checks if memdev is actually already set.
realize() performs these checks, no need to duplicate.
2. This is bad practise as one can see when looking at the NVDIMM
implemetation. The call does not return sane data before the device
is realized. Although spapr does not use NVDIMM, conceptually it is
wrong.
So let's just move this call to the right place. We can then cleanup
get_memory_region().
Signed-off-by: David Hildenbrand <address@hidden>
---
hw/ppc/spapr.c | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f59999daac..a5f1bbd58a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3153,6 +3153,12 @@ static void spapr_memory_plug(HotplugHandler
*hotplug_dev, DeviceState *dev,
align = memory_region_get_alignment(mr);
size = memory_region_size(mr);
+ if (size % SPAPR_MEMORY_BLOCK_SIZE) {
+ error_setg(&local_err, "Hotplugged memory size must be a multiple of "
+ "%lld MB", SPAPR_MEMORY_BLOCK_SIZE / M_BYTE);
+ goto out;
+ }
+
pc_dimm_memory_plug(dev, MACHINE(ms), align, &local_err);
if (local_err) {
goto out;
@@ -3186,9 +3192,6 @@ static void spapr_memory_pre_plug(HotplugHandler
*hotplug_dev, DeviceState *dev,
{
const sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(hotplug_dev);
PCDIMMDevice *dimm = PC_DIMM(dev);
- PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
- MemoryRegion *mr;
- uint64_t size;
char *mem_dev;
if (!smc->dr_lmb_enabled) {
@@ -3196,18 +3199,6 @@ static void spapr_memory_pre_plug(HotplugHandler
*hotplug_dev, DeviceState *dev,
return;
}
- mr = ddc->get_memory_region(dimm, errp);
- if (!mr) {
- return;
- }
- size = memory_region_size(mr);
-
- if (size % SPAPR_MEMORY_BLOCK_SIZE) {
- error_setg(errp, "Hotplugged memory size must be a multiple of "
- "%lld MB", SPAPR_MEMORY_BLOCK_SIZE / M_BYTE);
- return;
- }
-
mem_dev = object_property_get_str(OBJECT(dimm), PC_DIMM_MEMDEV_PROP, NULL);
if (mem_dev && !kvmppc_is_mem_backend_page_size_ok(mem_dev)) {
error_setg(errp, "Memory backend has bad page size. "
--
2.17.1
[Qemu-devel] [PATCH v1 03/11] pc: factor out pc-dimm checks into pc_dimm_pre_plug(), David Hildenbrand, 2018/06/11
[Qemu-devel] [PATCH v1 06/11] pc-dimm: don't allow to access "size" before the device was realized, David Hildenbrand, 2018/06/11