From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50144) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQpf2-0004RO-Uz for qemu-devel@nongnu.org; Wed, 03 Feb 2016 00:07:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aQpez-0000Ln-PT for qemu-devel@nongnu.org; Wed, 03 Feb 2016 00:07:24 -0500 Date: Wed, 3 Feb 2016 16:08:17 +1100 From: David Gibson Message-ID: <20160203050817.GK15080@voom.fritz.box> References: <1453340271-7417-1-git-send-email-david@gibson.dropbear.id.au> <20160122110225.2f9ae2d1@nial.brq.redhat.com> <56A23AD1.20202@redhat.com> <20160122153252.0d47355e@nial.brq.redhat.com> <20160201023309.GD23043@voom.redhat.com> <20160202173712.GB26314@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="vk/v8fjDPiDepTtA" Content-Disposition: inline In-Reply-To: <20160202173712.GB26314@thinpad.lan.raisama.net> Subject: Re: [Qemu-devel] [PATCH] dimm: Correct type of MemoryHotplugState->base List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: mst@redhat.com, qemu-devel@nongnu.org, Paolo Bonzini , qemu-ppc@nongnu.org, bharata@linux.vnet.ibm.com, Igor Mammedov --vk/v8fjDPiDepTtA Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Feb 02, 2016 at 03:37:12PM -0200, Eduardo Habkost wrote: > On Mon, Feb 01, 2016 at 01:33:09PM +1100, David Gibson wrote: > > On Fri, Jan 22, 2016 at 03:32:52PM +0100, Igor Mammedov wrote: > > > On Fri, 22 Jan 2016 15:21:05 +0100 > > > Paolo Bonzini wrote: > > >=20 > > > > On 22/01/2016 11:02, Igor Mammedov wrote: > > > > > On Thu, 21 Jan 2016 12:37:51 +1100 > > > > > David Gibson wrote: > > > > > =20 > > > > >> The 'base' field of MemoryHotplugState is ram_addr_t, which indi= cates that > > > > >> it exists in the abstract address space of RAM regions. > > > > >> > > > > >> However, the actual usage of this field indicates that it is a c= oncrete > > > > >> 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; > > > > >> =20 > > > > >=20 > > > > > 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_BAC= KEND > > > > > 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. =20 > > > >=20 > > > > But on a 32-bit system without CONFIG_XEN_BACKEND you cannot alloca= te > > > > more than 4G anyway, so the choice of ram_addr_t is understandable = in > > > > that case. > > > QEMU build will probably fail with above config but if it succeeds > > > then maxmem will be silently truncated. > > >=20 > > > >=20 > > > > 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. > > > True. > > >=20 > > > Anyway for this patch > > > Reviewed-by: Igor Mammedov > >=20 > > Who needs to take this patch? I'm not sure if I need to do anything > > further to push it forwards. >=20 > Most pc-dimm changes were merged either by me or Michael. I can > merge it if I get an Acked-by from Michael. >=20 > Otherwise, Michael has my Acked-by if he wants to merge it > through his tree. >=20 > Acked-by: Eduardo Habkost Ah, I didn't realise Michael worked in this area. Adding him to CC.. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --vk/v8fjDPiDepTtA Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWsYtBAAoJEGw4ysog2bOSYYUP/3hLN9CiyAICHMrE/kaw6UG5 rDNxkj/tfhUSIxiJ04r5LfWs4Uquid+Q5g2RdTcx+COxu62nvcopoqjCnCX55Jkb QI4AKgIp2C3dgC6EMjPuQO0BrAuvkrguw4gN6XVqMvyrlB1jpm1HmzV+w7cayaoJ C4FRxR0GBkKQDICVGSgSWxIz7GVOsYuMOy3Lhe30YnlLAvgc7etSAQUF4pJ/BRmh wyuHMVW01P6/vfWkM849UHbwUHnP4gWVaEGp3pTzOd9SmGNlbsRXvEL6GKXKCPW3 Vg3PfKdXNYH/xmKHjyyabVkbcAhpNy3/8jZYokvDC6mi+7RYDFkgBcx9AP7Z67a6 6/d2k8scec3rwauUopZGKUlavSRf79w+ULwlzQw3FT2WXva3fE1i/WQd0dRCa5nb 6DIX+NiXFs1fa82/lloz3a94g9GWS0MJT12ZX/7/WTD6efE4UJIQL+NfupqeAXf0 HS+U+zFVGjW5ZERXG+U/RlpkjghmOM5Xf+5VGdMYZsyZUneONmJ1cwZo7qpQrW2A 6E4onT19m6f9JAmWNWvWFKu2wrMKr38rbqpqGr2NA0oX8Dr61CyiF7F8tSkOnkZH 1qs+Tcf7BRzleaAGCrHHFxOsKpvKepm5CGwFfUghfUahTaIoyhfQHOGZ65obvzqi kRbGEmXxpbv1S7XbgL+r =xA/N -----END PGP SIGNATURE----- --vk/v8fjDPiDepTtA--