From patchwork Mon Jun 11 12:16:49 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Hildenbrand X-Patchwork-Id: 927635 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=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 414BvC6Y0Rz9s01 for ; Mon, 11 Jun 2018 22:21:11 +1000 (AEST) Received: from localhost ([::1]:48414 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fSLor-0004mi-HV for incoming@patchwork.ozlabs.org; Mon, 11 Jun 2018 08:21:09 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48876) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fSLl1-00021u-BD for qemu-devel@nongnu.org; Mon, 11 Jun 2018 08:17:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fSLkz-0008Sc-O5 for qemu-devel@nongnu.org; Mon, 11 Jun 2018 08:17:11 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:51364 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fSLkz-0008SS-Ir; Mon, 11 Jun 2018 08:17:09 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2D10040711F3; Mon, 11 Jun 2018 12:17:09 +0000 (UTC) Received: from t460s.redhat.com (ovpn-117-21.ams2.redhat.com [10.36.117.21]) by smtp.corp.redhat.com (Postfix) with ESMTP id 49A462024CA2; Mon, 11 Jun 2018 12:17:07 +0000 (UTC) From: David Hildenbrand To: qemu-devel@nongnu.org Date: Mon, 11 Jun 2018 14:16:49 +0200 Message-Id: <20180611121655.19616-6-david@redhat.com> In-Reply-To: <20180611121655.19616-1-david@redhat.com> References: <20180611121655.19616-1-david@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Mon, 11 Jun 2018 12:17:09 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Mon, 11 Jun 2018 12:17:09 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'david@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v1 05/11] spapr: move memory hotplug size check into plug code 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: Eduardo Habkost , "Michael S . Tsirkin" , Xiao Guangrong , David Hildenbrand , Alexander Graf , qemu-ppc@nongnu.org, Paolo Bonzini , Igor Mammedov , David Gibson , Richard Henderson Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" 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 Acked-by: David Gibson --- 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. "