From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51947) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y3HAo-0008JO-TH for qemu-devel@nongnu.org; Mon, 22 Dec 2014 23:34:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y3HAm-0001w0-K7 for qemu-devel@nongnu.org; Mon, 22 Dec 2014 23:34:18 -0500 Date: Tue, 23 Dec 2014 15:22:06 +1100 From: David Gibson Message-ID: <20141223042206.GD4576@voom.redhat.com> References: <1418602508-30845-1-git-send-email-gwshan@linux.vnet.ibm.com> <1418602508-30845-3-git-send-email-gwshan@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="11Y7aswkeuHtSBEs" Content-Disposition: inline In-Reply-To: <1418602508-30845-3-git-send-email-gwshan@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v13 2/3] sPAPR: Implement EEH RTAS calls List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gavin Shan Cc: aik@ozlabs.ru, alex.williamson@redhat.com, qemu-ppc@nongnu.org, agraf@suse.de, qemu-devel@nongnu.org --11Y7aswkeuHtSBEs Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Dec 15, 2014 at 11:15:07AM +1100, Gavin Shan wrote: > The emulation for EEH RTAS requests from guest isn't covered > by QEMU yet and the patch implements them. >=20 > The patch defines constants used by EEH RTAS calls and adds > callback sPAPRPHBClass::eeh_handler, which is going to be used > this way: >=20 > * RTAS calls are received in spapr_pci.c, sanity check is done > there. > * RTAS handlers handle what they can. If there is something it > cannot handle and sPAPRPHBClass::eeh_handler callback is defined, > it is called. > * sPAPRPHBClass::eeh_handler is only implemented for VFIO now. It > does ioctl() to the IOMMU container fd to complete the call. Error > codes from that ioctl() are transferred back to the guest. >=20 > [aik: defined RTAS tokens for EEH RTAS calls] > Signed-off-by: Gavin Shan > --- > hw/ppc/spapr_pci.c | 246 ++++++++++++++++++++++++++++++++++++++= ++++++ > include/hw/pci-host/spapr.h | 7 ++ > include/hw/ppc/spapr.h | 43 +++++++- > 3 files changed, 294 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 3d70efe..3bb1971 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -406,6 +406,233 @@ static void rtas_ibm_query_interrupt_source_number(= PowerPCCPU *cpu, > rtas_st(rets, 2, 1);/* 0 =3D=3D level; 1 =3D=3D edge */ > } > =20 > +static int rtas_handle_eeh_request(sPAPREnvironment *spapr, > + uint64_t buid, uint32_t req, uint32_t= opt) > +{ > + sPAPRPHBState *sphb =3D spapr_pci_find_phb(spapr, buid); > + sPAPRPHBClass *info =3D SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + > + if (!sphb || !info->eeh_handler) { > + return -ENOENT; > + } > + > + return info->eeh_handler(sphb, req, opt); > +} > + > +static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu, > + sPAPREnvironment *spapr, > + uint32_t token, uint32_t nargs, > + target_ulong args, uint32_t nret, > + target_ulong rets) > +{ > + uint32_t addr, option; > + uint64_t buid =3D ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args,= 2); You're dereferencing RTAS parameters here before you've checked the number of parameters, which isn't safe. Similar problem in the other entry points as well. > + int ret; > + > + if ((nargs !=3D 4) || (nret !=3D 1)) { > + goto param_error_exit; > + } > + > + addr =3D rtas_ld(args, 0); > + option =3D rtas_ld(args, 3); > + switch (option) { > + case RTAS_EEH_ENABLE: > + if (!spapr_pci_find_dev(spapr, buid, addr)) { > + goto param_error_exit; > + } > + break; > + case RTAS_EEH_DISABLE: > + case RTAS_EEH_THAW_IO: > + case RTAS_EEH_THAW_DMA: > + break; > + default: > + goto param_error_exit; > + } > + > + ret =3D rtas_handle_eeh_request(spapr, buid, > + RTAS_EEH_REQ_SET_OPTION, option); > + if (ret >=3D 0) { > + rtas_st(rets, 0, RTAS_OUT_SUCCESS); > + return; > + } The fall through here means that any failure in rtas_handle_eeh_request will be reported as RTAS_OUT_PARAM_ERROR, which doesn't sound like it would always be the right error code. Similar in the other entry points. --=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 --11Y7aswkeuHtSBEs Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUmO3tAAoJEGw4ysog2bOSBrgQALzjdQR+iUWMWa+PnRJgsNEY 2vo8ELIwBiH9lvzWXuixyEUIjmDULyAED/SannozUAZl3aYSCG0cK2aaOT9y2jF3 mSNZ74tbDeAV+kTBXUw/9knCYxbtYKyCHlAnkOspWrVMPHCDsfHZZHIPDcA9n1lu LcH636Yjp45pmQcQFd+ybJAYslIuInWmpoNb9g8/VjF4fRPH5deSlBJq4cwc+WdU z/IGDaSVrwwL/lq/GOs/FLnoX3OCFuDolO9tE6eFES78lSueQF0t6d7AOlqknx4F OwKfXwhRMJS8osZ+oyze4I+k2MaRr2sTcG2kn2u/ccqdN0GD4zgziFA/Ube2noAR OhQgkfFmp8dBhkDZn8CDJrFgHy4+D3tpr3uMK4s2+7ff7Fr9hsULuPhtPq6VPj9p yNjjCKcYxaMtwH5wSnka3NryZAASZ8bDw4YIC6xV2G0yH9qgRNGdSIzryxYJFEkt h+6hQzy2bfPjScZG7MYYUwCEPZWdk2TOCfeADfcp27/ZgxCXEJb97ian50g7gdqE 2IzJQL8OrvNk6Qo77nQAAvfWVL6UouYO+pZHUgh+bJtQL+w3XJ/vPAQm71Two3oF UKB8dU1uJe/mmFaoowA/cOT25LFBUvX+VWZpKDf7maIaKoMVZ0I0U8sdlXpYjkjz 6BLVoXEGbgrzXcCZKbZH =0F5S -----END PGP SIGNATURE----- --11Y7aswkeuHtSBEs--