From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43885) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zext6-0000VA-Md for qemu-devel@nongnu.org; Thu, 24 Sep 2015 00:12:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zext5-0003F6-Mr for qemu-devel@nongnu.org; Thu, 24 Sep 2015 00:12:04 -0400 Date: Thu, 24 Sep 2015 14:12:05 +1000 From: David Gibson Message-ID: <20150924041205.GP15944@voom.fritz.box> References: <1442893189-5680-1-git-send-email-bharata@linux.vnet.ibm.com> <1442893189-5680-2-git-send-email-bharata@linux.vnet.ibm.com> <20150923032853.GU20331@voom.fritz.box> <20150924034152.GE16381@in.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="zn4k3Q+N5puqXur4" Content-Disposition: inline In-Reply-To: <20150924034152.GE16381@in.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v0 1/2] spapr: Allocate HTAB from machine init List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: nfont@linux.vnet.ibm.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com --zn4k3Q+N5puqXur4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Sep 24, 2015 at 09:11:52AM +0530, Bharata B Rao wrote: > On Wed, Sep 23, 2015 at 01:28:53PM +1000, David Gibson wrote: > > On Tue, Sep 22, 2015 at 09:09:48AM +0530, Bharata B Rao wrote: > > > Allocate HTAB from ppc_spapr_init() so that we can abort the guest > > > if requested HTAB size is't allocated by the host. However retain the > > > htab reset call in spapr_reset_htab() so that HTAB gets reset (and > > > not allocated) during machine reset. > >=20 > > I was briefly worried about this, because I recall there as a reason > > htab allocation got moved to the reset handler in the first place. > > Looking at the git history, however, I've convinced myself this is > > basically ok (because you preserve the call during reset to wipe clean > > the htab). > >=20 > > > Signed-off-by: Bharata B Rao > > > --- > > > hw/ppc/spapr.c | 13 ++++++++++++- > > > 1 file changed, 12 insertions(+), 1 deletion(-) > > >=20 > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > index 7f4f196..4692122 100644 > > > --- a/hw/ppc/spapr.c > > > +++ b/hw/ppc/spapr.c > > > @@ -979,7 +979,7 @@ static void emulate_spapr_hypercall(PowerPCCPU *c= pu) > > > #define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &=3D tswap64(~HPT= E64_V_HPTE_DIRTY)) > > > #define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |=3D tswap64(HPTE= 64_V_HPTE_DIRTY)) > > > =20 > > > -static void spapr_reset_htab(sPAPRMachineState *spapr) > > > +static void spapr_alloc_htab(sPAPRMachineState *spapr) > > > { > > > long shift; > > > int index; > > > @@ -1012,6 +1012,16 @@ static void spapr_reset_htab(sPAPRMachineState= *spapr) > > > DIRTY_HPTE(HPTE(spapr->htab, index)); > > > } > > > } > > > +} > > > + > > > +static void spapr_reset_htab(sPAPRMachineState *spapr) > > > +{ > > > + /* > > > + * We have already allocated the hash page table, this call will > > > + * not again allocate but only result in clearing of hash page > > > + * table entries. > > > + */ > > > + kvmppc_reset_htab(spapr->htab_shift); > >=20 > > It's unlikely the kernel will give us less htab than we already have, > > but we really should at least check for that. Probably not much we > > can do except abort() but at least we can give a useful error message. >=20 > With the change I am doing here, this is no longer an allocation path. > Host kernel will just clear the HTAB and return the same htab_shift > that we passed here. So do you think it still makes sense to check > return value ? That's the current kernel behaviour, but the interface doesn't guarantee that. So, yes, I still think you have to check the return value. --=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 --zn4k3Q+N5puqXur4 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWA3gVAAoJEGw4ysog2bOSavwP/0QpWgTB2ZX4T1inbPga2RFx iKtXSYxTotAumwUBbQf5WMK09D7VuL2bCLdOvMu5FlAd92pau7WBUc+yxFrLzsZv FD5h8qiC/cDZ7+go1zuPiaRd+gc+DDKwBAECVZjOx1Nm8GLGuOlXcYSHheDfkB4a PGEEaEwhMfhGsGdwkIcrhnMsIG98ruEA8WyK0BPDV9vDKLYp8H0TOdy7o3R09WTR PtCEthwIDgZaVKSro8LdaHItek+YhgirLODfUc+atE811AyajPXzssL2HASE04Q4 GgsWCgvglqDXTA7WwoXcF3MwjcNv6BtT+l+s/z1+f/S3TseRqanr3NmSAYEmng6w WBRPDGoGno4v0Igq89VK6vW/d8BKLGiY1KCqAU8D9DLWgkv63zzHiE9V5y8NvrRr Hh9GP4TaKEVYqI9eYErHkDfO/9sRDwQrmkFqRXd1HmC41fGyU9PnXyh+WHJmkJ13 zaN5vqdbUXALNoB7tXJpEN33ioykoe/U5TdYXQWPP7TchJc6seJ5hEWeKYt2FceW lAuKkDoH04CiTOsCxKsBLTbb3/xJDPq/yWGdxaPcDiT7RtTeWiF+w4YKF5ucRYRS ejLhLkbW11THUCPx/eVe175olLnHgHhULDI2JqWP2eYyz7gNl8ADYc9IvyZG6m1k YfJ8s9wBV6Cw054v4ZR9 =U0qo -----END PGP SIGNATURE----- --zn4k3Q+N5puqXur4--