From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60163) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afmA1-0005q2-8S for qemu-devel@nongnu.org; Tue, 15 Mar 2016 06:25:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1afmA0-0003lJ-1y for qemu-devel@nongnu.org; Tue, 15 Mar 2016 06:25:09 -0400 Date: Tue, 15 Mar 2016 20:59:52 +1100 From: David Gibson Message-ID: <20160315095952.GF9032@voom> References: <1458021080-2145-1-git-send-email-aik@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="rWhLK7VZz0iBluhq" Content-Disposition: inline In-Reply-To: <1458021080-2145-1-git-send-email-aik@ozlabs.ru> Subject: Re: [Qemu-devel] [PATCH qemu] spapr/target-ppc/kvm: Only add hcall-instructions if KVM supports it List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: Paul Mackerras , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Alexander Graf --rWhLK7VZz0iBluhq Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Mar 15, 2016 at 04:51:20PM +1100, Alexey Kardashevskiy wrote: > ePAPR defines "hcall-instructions" device-tree property which contains > code to call hypercalls in ePAPR paravirtualized guests. However this > property is also present for pseries guests where it does not make sense, > even though it contains dummy code which simply fails. >=20 > Instead of maintaining the property (which used to be BE only; then was > fixed to be endian-agnostic) and confusing the guest (which might think > there is ePAPR host while there is none), this simply does not > the property to the device tree if the host kernel does not implement it. >=20 > In order to tell the machine code if the host kernel supports > KVM_CAP_PPC_GET_PVINFO, this changes kvmppc_get_hypercall() to return 1 > if the host kernel does not implement it (which is HV KVM case). >=20 > Signed-off-by: Alexey Kardashevskiy So the idea of only adding the property when the host kernel supplies a suitable value seems good, but I'm a bit nervous about applying this, because I'm not sure what case the original fallback hypercall code was supposed to handle. agraf, if you could enlighten us with some history that could be good. > Alexander, >=20 > We just got a bug report that LE guests would not boot under quite old QE= MU > and we (powerkvm) wonder if it makes sense to backport endian-agnostic > hypercall code to older QEMU or it is simpler/more correct > not to have epapr-hypercall property in the tree. >=20 >=20 > --- > hw/ppc/spapr.c | 9 +++++---- > target-ppc/kvm.c | 2 +- > 2 files changed, 6 insertions(+), 5 deletions(-) >=20 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 43708a2..8130eb4 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -497,10 +497,11 @@ static void *spapr_create_fdt_skel(hwaddr initrd_ba= se, > * Older KVM versions with older guest kernels were broken w= ith the > * magic page, don't allow the guest to map it. > */ > - kvmppc_get_hypercall(first_cpu->env_ptr, hypercall, > - sizeof(hypercall)); > - _FDT((fdt_property(fdt, "hcall-instructions", hypercall, > - sizeof(hypercall)))); > + if (!kvmppc_get_hypercall(first_cpu->env_ptr, hypercall, > + sizeof(hypercall))) { > + _FDT((fdt_property(fdt, "hcall-instructions", hypercall, > + sizeof(hypercall)))); > + } > } > _FDT((fdt_end_node(fdt))); > } > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > index 776336b..e5183db 100644 > --- a/target-ppc/kvm.c > +++ b/target-ppc/kvm.c > @@ -2001,7 +2001,7 @@ int kvmppc_get_hypercall(CPUPPCState *env, uint8_t = *buf, int buf_len) > hc[2] =3D cpu_to_be32(0x48000008); > hc[3] =3D cpu_to_be32(bswap32(0x3860ffff)); Since you're now returning a value which means the caller is supposed to ignore the hc code, there's not much point actually populating it above. > - return 0; > + return 1; > } > =20 > static inline int kvmppc_enable_hcall(KVMState *s, target_ulong hcall) --=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 --rWhLK7VZz0iBluhq Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJW590YAAoJEGw4ysog2bOSc18QAKyp2f0xZlvt6A7IEbHmaGg2 hezsuDcqKUDxzDP/Qv4SCjzJjJhqt+8SK0uzxons0xNnrMpZF3UIyMhd3Mr97nQi K5WKfWkhiymGDYLCsZMlHoCSbjK0bTrjeg5nzhjs2DZ+7Caf2fw0zjSXpmFJ9rmW 0FpG0Rer3f8eID7EWvSXUEcHTNrZviIl/dVJDFf4wMW8Actyxo9DqynjY8mo8CQ9 y7dvtfMbndWOKgMEwtKvOjMS4oWsViZwbkJqUJSfiG2YETVlWeC+pUhivlFVsNlJ xOmkBJrMpofCg+jcoQj6uULekn+Ky4N1wZQFkcSz+freu9kmm7IKTu2OlbHsDUj1 wjllndt4I4OxMH34XHbEr4R8eUWb9UGr+3heBUtw+pryya3mXaDOXF/QpRu6jLkT DCv/b+QqV181CuLll6NGn2iI++tF3BK66qArG29on4RzM2zTDwPj9eFF8R2JlIwI onChDCXYMEDC2fpUuazrdQrtpbsuKBMVXv35QMFhE7P0ESaWM1JAPAOCJIwG3vjf VXUWI1RnImJhXAzQxxBXwS8GgWTFfUDV+uW66Y+YKwsvLXEZ5uUF/wEc9yktVckh tGnv9DaoiB/GKjtZv0gP6C/G3/E6qehNkArFgmiNW4bzx88/AL+iKz4nfC/Jl1Es A+YP8TzgKTx20/qBYuwW =URC+ -----END PGP SIGNATURE----- --rWhLK7VZz0iBluhq--