From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59261) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aMcaT-0002JI-K7 for qemu-devel@nongnu.org; Fri, 22 Jan 2016 09:21:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aMcaO-0002GP-Vz for qemu-devel@nongnu.org; Fri, 22 Jan 2016 09:21:17 -0500 References: <1453340271-7417-1-git-send-email-david@gibson.dropbear.id.au> <20160122110225.2f9ae2d1@nial.brq.redhat.com> From: Paolo Bonzini Message-ID: <56A23AD1.20202@redhat.com> Date: Fri, 22 Jan 2016 15:21:05 +0100 MIME-Version: 1.0 In-Reply-To: <20160122110225.2f9ae2d1@nial.brq.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] dimm: Correct type of MemoryHotplugState->base List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov , David Gibson Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, ehabkost@redhat.com, bharata@linux.vnet.ibm.com On 22/01/2016 11:02, Igor Mammedov wrote: > On Thu, 21 Jan 2016 12:37:51 +1100 > David Gibson wrote: > >> The 'base' field of MemoryHotplugState is ram_addr_t, which indicates that >> it exists in the abstract address space of RAM regions. >> >> However, the actual usage of this field indicates that it is a concrete >> physical address (it's passed as an offset to memory_region_add_subgregion >> for example). >> >> So, correct its type to 'hwaddr'. >> >> Signed-off-by: David Gibson >> --- >> include/hw/mem/pc-dimm.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h >> index d83bf30..218dfb0 100644 >> --- a/include/hw/mem/pc-dimm.h >> +++ b/include/hw/mem/pc-dimm.h >> @@ -77,7 +77,7 @@ typedef struct PCDIMMDeviceClass { >> * @mr: hotplug memory address space container >> */ >> typedef struct MemoryHotplugState { >> - ram_addr_t base; >> + hwaddr base; >> MemoryRegion mr; >> } MemoryHotplugState; >> > > I agree with this fix but that's not the only place where > ram_addr_t needs to be replaced with hwaddr. > For example type of MachineState.[max]ram_size fields needs > to be changed as well. Because QEMU builds without CONFIG_XEN_BACKEND > on 32-bit hosts are broken since ram_addr_t is 32-bits there > while some targets assume and use it as 64-bit one. But on a 32-bit system without CONFIG_XEN_BACKEND you cannot allocate more than 4G anyway, so the choice of ram_addr_t is understandable in that case. On the other hand, on a 32-bit system without CONFIG_XEN_BACKEND you definitely can place 128M of hot plugged memory between say 4096MB and 4224MB. Paolo