From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46077) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YND4P-0003gP-HP for qemu-devel@nongnu.org; Sun, 15 Feb 2015 23:14:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YND4N-0000Yd-KA for qemu-devel@nongnu.org; Sun, 15 Feb 2015 23:14:05 -0500 Date: Mon, 16 Feb 2015 12:52:48 +1100 From: David Gibson Message-ID: <20150216015248.GG26645@voom.fritz.box> References: <1424042162-12249-1-git-send-email-gwshan@linux.vnet.ibm.com> <1424042162-12249-2-git-send-email-gwshan@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="PyMzGVE0NRonI6bs" Content-Disposition: inline In-Reply-To: <1424042162-12249-2-git-send-email-gwshan@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v18 1/2] 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, qemu-devel@nongnu.org, agraf@suse.de --PyMzGVE0NRonI6bs Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Feb 16, 2015 at 10:16:01AM +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 > callbacks sPAPRPHBClass::{eeh_set_option, eeh_get_state, eeh_reset, > eeh_configure}, which are going to be used as follows: >=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 the corresponding sPAPRPHBClass callback is > defined, it is called. > * Those callbacks are only implemented for VFIO now. They do ioctl() > to the IOMMU container fd to complete the calls. 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 | 281 ++++++++++++++++++++++++++++++++++++++= ++++++ > include/hw/pci-host/spapr.h | 4 + > include/hw/ppc/spapr.h | 43 ++++++- > 3 files changed, 326 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index cebdeb3..29b071d 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -406,6 +406,268 @@ 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 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) > +{ > + sPAPRPHBState *sphb; > + sPAPRPHBClass *spc; > + uint32_t addr, option; > + uint64_t buid; > + int ret; > + > + if ((nargs !=3D 4) || (nret !=3D 1)) { > + goto param_error_exit; > + } > + > + buid =3D ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); > + addr =3D rtas_ld(args, 0); > + option =3D rtas_ld(args, 3); > + > + sphb =3D find_phb(spapr, buid); > + if (!sphb) { > + goto param_error_exit; > + } > + > + spc =3D SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + if (!spc->eeh_set_option) { > + goto param_error_exit; > + } > + > + /* > + * The EEH functionality is enabled on basis of PCI device, > + * instead of PE. We need check the validity of the PCI > + * device address. > + */ > + if (option =3D=3D RTAS_EEH_ENABLE && > + !find_dev(spapr, buid, addr)) { > + goto param_error_exit; > + } You're still breaking your layering by doing checks dependent on the specific option both here and in the callback. What I meant by my comments on the previous version was that this find_dev() test should also move into the eeh_set_option callback. Obviously that means adding addr into the parameters - but surely if the addr has any meaning whatsoever, it must be at least potentially needed by the callback anyway. Apart from that, Reviewed-by: David Gibson --=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 --PyMzGVE0NRonI6bs Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJU4U1wAAoJEGw4ysog2bOSEEYQAI9nHyQS32D6AI7HjTU1CX8+ EdjGJoKTXLNzxFnX6bxB4grLaJSJ3Ibp5YuGq7I1v0RAkV+It5FY1SzklTci6mWF wjAkWLq2p4TfMC81j2BnoLzvx3o6PSUtVKXJZzCatmMgQ2jP3gAzmuMINr9JmhM/ e5wss3IeMaTgwoAvlatAaEuzsovny2YVT58uOzDQh7GktJmRYeAlgU6MVt8xwauZ n6mGskM6yArDxOAaRlYe/4qjOeeQwuIIdmLUwSDb+PWmmjDl7nty8z4qma3YaBfu U/Tbg/TovDT3BjsBt0QOI3amu6EBxXhTMYuflOQ/VA0oC14lvjpQ9RPZPzLOmx11 nRb5z8OvHNi9FPvb5xRFlTT7rtVL0Zpmfz0IDCHg+tI99X6H4vY17rR8uHTXnnHZ qGyTxihQ9SoXVKylOfZlOnWrAP2lGQpRNa06EN0v1ZfGZB+yD+cW0CKHrPMvzrxB 2OhNkIDoguQ7744ltpbkHR+UWoF+oaJlWzU4JdgsNIK4Nwg8RxUzRF0NlNo5uN3R Rqo3i2kEFQbYvjax9AiuVTKsaihVuOAt0Il88PLT8MKnr66IesONlxIpV4BzGuiD ivPfScuU3laM/OJj8TL5xl5DscEl1vJxiid7D2h83KIeAA5ZPck7qSGDyrVo9lYs xULuFTEn/jKC/XhbdVh1 =quVc -----END PGP SIGNATURE----- --PyMzGVE0NRonI6bs--