From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57242) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNENq-0002pO-8w for qemu-devel@nongnu.org; Mon, 16 Feb 2015 00:38:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YNENm-0007OL-44 for qemu-devel@nongnu.org; Mon, 16 Feb 2015 00:38:14 -0500 Received: from ozlabs.org ([103.22.144.67]:44049) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YNENl-0007O3-J2 for qemu-devel@nongnu.org; Mon, 16 Feb 2015 00:38:10 -0500 Date: Mon, 16 Feb 2015 15:56:24 +1100 From: David Gibson Message-ID: <20150216045624.GI26645@voom.fritz.box> References: <1420697420-16053-1-git-send-email-bharata@linux.vnet.ibm.com> <1420697420-16053-12-git-send-email-bharata@linux.vnet.ibm.com> <20150212051936.GB20593@voom.redhat.com> <20150212053914.GB3307@in.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="7IgncvKP0CVPV/ZZ" Content-Disposition: inline In-Reply-To: <20150212053914.GB3307@in.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v1 11/13] spapr: Initialize hotplug memory address space List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: imammedo@redhat.com, agraf@suse.de, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com --7IgncvKP0CVPV/ZZ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Feb 12, 2015 at 11:09:14AM +0530, Bharata B Rao wrote: > On Thu, Feb 12, 2015 at 04:19:36PM +1100, David Gibson wrote: > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > index 44405b2..9ff08ff 100644 > > > --- a/hw/ppc/spapr.c > > > +++ b/hw/ppc/spapr.c > > > @@ -120,6 +120,8 @@ struct sPAPRMachineState { > > > =20 > > > /*< public >*/ > > > char *kvm_type; > > > + ram_addr_t hotplug_memory_base; > > > + MemoryRegion hotplug_memory; > >=20 > > We should really unify sPAPRMachineState with sPAPREnvironment at some > > point (I realise that doesn't reasonably fit within the scope of this > > series). >=20 > ok. >=20 > >=20 > > > }; > > > =20 > > > sPAPREnvironment *spapr; > > > @@ -1403,6 +1405,7 @@ static void ppc_spapr_init(MachineState *machin= e) > > > bool kernel_le =3D false; > > > char *filename; > > > int smt =3D kvmppc_smt_threads(); > > > + sPAPRMachineState *ms =3D SPAPR_MACHINE(machine); > > > =20 > > > msi_supported =3D true; > > > =20 > > > @@ -1492,6 +1495,29 @@ static void ppc_spapr_init(MachineState *machi= ne) > > > memory_region_add_subregion(sysmem, 0, rma_region); > > > } > > > =20 > > > + if (machine->ram_size < machine->maxram_size) { > > > + ram_addr_t hotplug_mem_size =3D machine->maxram_size - machi= ne->ram_size; > > > + > > > + if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) { > > > + error_report("unsupported amount of memory slots: %"PRIu= 64, > > > + machine->ram_slots); > > > + exit(EXIT_FAILURE); > > > + } > > > + > > > + ms->hotplug_memory_base =3D ROUND_UP(machine->ram_size, 1ULL= << 30); > >=20 > > Is there a particular significance to the 1GiB alignment? Is it just > > a conveniently large alignment, or is that value specified in PAPR > > somewhere? Using a named constant would probably help to clarify that. >=20 > I am basing this on x86 memory hotplug and that's how 1GB is coming. It > is not PAPR specified. Ok, it should definitely be a #define. > > > + if ((ms->hotplug_memory_base + hotplug_mem_size) < hotplug_m= em_size) { > > > + error_report("unsupported amount of maximum memory: " RA= M_ADDR_FMT, > > > + machine->maxram_size); > > > + exit(EXIT_FAILURE); > > > + } > > > + > > > + memory_region_init(&ms->hotplug_memory, OBJECT(ms), > > > + "hotplug-memory", hotplug_mem_size); > > > + memory_region_add_subregion(sysmem, ms->hotplug_memory_base, > > > + &ms->hotplug_memory); > > > + } > > > + > > > filename =3D qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin= "); > > > spapr->rtas_size =3D get_image_size(filename); > > > spapr->rtas_blob =3D g_malloc(spapr->rtas_size); > > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > > > index ae8b4e1..64681c4 100644 > > > --- a/include/hw/ppc/spapr.h > > > +++ b/include/hw/ppc/spapr.h > > > @@ -482,6 +482,9 @@ struct sPAPRTCETable { > > > #define TIMEBASE_FREQ 512000000ULL > > > #define SPAPR_MIN_MEMORY_BLOCK_SIZE (1 << 28) /* 256MB */ > > > =20 > > > +/* Support a min of 1TB hotplug memory assuming 256MB per slot */ > > > +#define SPAPR_MAX_RAM_SLOTS (1ULL << 12) > >=20 > > Is this constraint arbitrary, or does it come from something in PAPR+? >=20 > Arbitrary max, not defined by PAPR. Ok. Why do we need a fixed maximum value? I don't see this used to size any arrays.. --=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 --7IgncvKP0CVPV/ZZ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJU4Xh4AAoJEGw4ysog2bOS3B0QALT+MZwdHe3kaLNjUmO2xdY7 e8U3HA/8Jg6MX1da27lOnAAgqib4XurYsa+xgo3VxHwOxwFum384svSNWZowRsXF 5BYyX2yKLvYYJTv58fXdxJP2dJv++JRu+1R0Nikse5igiu52vYDaA5nqHlWT3WEG +mIJFVV6pJT/XPY1kRVoamVgRs52ch9ttpwbC7BWMD5HrN2ChsyEpnpjXGFN9RBG i91ta/KDqRH9vAh6LvOBNKDolk0hIwCETREajEW8ZY0ZxCZoDJ7kZceHw1pwLN/U 3Qr3NZRfwa+AhfArDvNM0/BJ29x9JW+k0/v2qhpj9zWZuo5dP55kPLnfYrEauuOC DFE84Lp0Hi/1O7KkPkdcFus5v5lnvZZ5p9kSOsiwyDBZjvVMMm7THhJ7ousSVRdr rWLsutf4Kk58vsMzcfJu3+iwhoUrfQab89j9LLrnGQPmtiAutK93TK7/u4d1wiB8 LAs1fuPrkGyUo/o4tvHbe/FawFULAGLtrIY9pCpLlZuK3oQoMU44kjBMcZSXMyWf 7X98NnmRIYWU0CVQQIrplsOgrfBg4rNTCEO/CVSyo6R6GqOWEqfWJmSxDSGKkyKC 6/aEw02F9PQYIPTlI82oxGHdRPZaQm03/7kW1B1im8ElLVCqtd9MYbytZxfKhmRL z50LrawxNdVb2qlOXLfy =QISE -----END PGP SIGNATURE----- --7IgncvKP0CVPV/ZZ--