From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60833) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJD4m-0006WZ-LP for qemu-devel@nongnu.org; Wed, 04 Feb 2015 22:25:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YJD4k-0006LC-JD for qemu-devel@nongnu.org; Wed, 04 Feb 2015 22:25:56 -0500 Date: Thu, 5 Feb 2015 14:19:49 +1100 From: David Gibson Message-ID: <20150205031949.GI25675@voom.fritz.box> References: <1423016856-11256-1-git-send-email-gwshan@linux.vnet.ibm.com> <1423016856-11256-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="zgY/UHCnsaNnNXRx" Content-Disposition: inline In-Reply-To: <1423016856-11256-2-git-send-email-gwshan@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v16 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 --zgY/UHCnsaNnNXRx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Feb 04, 2015 at 01:27:35PM +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 | 310 ++++++++++++++++++++++++++++++++++++++= ++++++ > include/hw/pci-host/spapr.h | 7 + > include/hw/ppc/spapr.h | 43 +++++- > 3 files changed, 358 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 6deeb19..3fac5a9 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -406,6 +406,297 @@ 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_handler) { Should this case be RTAS_OUT_PARAM_ERROR or RTAS_OUT_NOT_SUPPORTED? > + goto param_error_exit; > + } > + > + switch (option) { > + case RTAS_EEH_ENABLE: > + if (!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 spc->eeh_handler(sphb, RTAS_EEH_REQ_SET_OPTION, option); > + if (ret < 0) { > + rtas_st(rets, 0, RTAS_OUT_HW_ERROR); It seems like it would make sense for the eeh_handler callback to return rtas error codes, so that if it makes sense it can signal errors other than RTAS_OUT_HW_ERROR. > + return; > + } > + > + rtas_st(rets, 0, RTAS_OUT_SUCCESS); > + return; > + > +param_error_exit: > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > +} > + > +static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu, > + sPAPREnvironment *spapr, > + uint32_t token, uint32_t narg= s, > + target_ulong args, uint32_t n= ret, > + target_ulong rets) > +{ > + sPAPRPHBState *sphb; > + sPAPRPHBClass *spc; > + PCIDevice *pdev; > + uint32_t addr, option; > + uint64_t buid; > + > + if ((nargs !=3D 4) || (nret !=3D 2)) { > + goto param_error_exit; > + } > + > + buid =3D ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); > + sphb =3D find_phb(spapr, buid); > + if (!sphb) { > + goto param_error_exit; > + } > + > + spc =3D SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + if (!spc->eeh_handler) { > + goto param_error_exit; > + } > + > + addr =3D rtas_ld(args, 0); > + option =3D rtas_ld(args, 3); > + if (option !=3D RTAS_GET_PE_ADDR && option !=3D RTAS_GET_PE_MODE) { > + goto param_error_exit; > + } > + > + pdev =3D find_dev(spapr, buid, addr); > + if (!pdev) { > + goto param_error_exit; > + } > + > + /* > + * For now, we always have bus level PE whose address > + * has format "00BBSS00". The guest OS might regard > + * PE address 0 as invalid. We avoid that simply by > + * extending it with one. This comment is a bit confusing. Why not just say we have PE addresses of form "00BBSS01". Also you should say what the BB and SS mean in your template. > + */ > + if (option =3D=3D RTAS_GET_PE_ADDR) { I think this would be clearer as a switch statement, which folds in the other if statement above. > + rtas_st(rets, 1, (pci_bus_num(pdev->bus) << 16) + 1); > + } else { > + rtas_st(rets, 1, RTAS_PE_MODE_SHARED); > + } > + > + rtas_st(rets, 0, RTAS_OUT_SUCCESS); > + return; > + > +param_error_exit: > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > +} > + > +static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu, > + sPAPREnvironment *spapr, > + uint32_t token, uint32_t nar= gs, > + target_ulong args, uint32_t = nret, > + target_ulong rets) > +{ > + sPAPRPHBState *sphb; > + sPAPRPHBClass *spc; > + uint64_t buid; > + int ret; > + > + if ((nargs !=3D 3) || (nret !=3D 4 && nret !=3D 5)) { > + goto param_error_exit; > + } > + > + buid =3D ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); > + sphb =3D find_phb(spapr, buid); > + if (!sphb) { > + goto param_error_exit; > + } > + > + spc =3D SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + if (!spc->eeh_handler) { > + goto param_error_exit; > + } > + > + ret =3D spc->eeh_handler(sphb, RTAS_EEH_REQ_GET_STATE, 0); > + if (ret < 0) { > + goto param_error_exit; So here, all errors from eeh_handler are turned into RTAS_OUT_PARAM_ERROR, whereas above they were all turned into RTAS_OUT_HW_ERROR. > + } > + > + rtas_st(rets, 1, ret); > + rtas_st(rets, 2, RTAS_EEH_SUPPORT); > + rtas_st(rets, 3, RTAS_EEH_PE_UNAVAIL_INFO); > + if (nret >=3D 5) { > + rtas_st(rets, 4, RTAS_EEH_PE_RECOVER_INFO); > + } > + > + rtas_st(rets, 0, RTAS_OUT_SUCCESS); > + return; > + > +param_error_exit: > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > +} > + > +static void rtas_ibm_set_slot_reset(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 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); > + 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_handler) { > + goto param_error_exit; > + } > + > + switch (option) { > + case RTAS_SLOT_RESET_DEACTIVATE: > + case RTAS_SLOT_RESET_HOT: > + case RTAS_SLOT_RESET_FUNDAMENTAL: > + break; > + default: > + goto param_error_exit; > + } > + > + ret =3D spc->eeh_handler(sphb, RTAS_EEH_REQ_RESET, option); > + if (ret < 0) { > + rtas_st(rets, 0, RTAS_OUT_HW_ERROR); > + return; > + } > + > + rtas_st(rets, 0, RTAS_OUT_SUCCESS); > + return; > + > +param_error_exit: > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > +} > + > +static void rtas_ibm_configure_pe(PowerPCCPU *cpu, > + sPAPREnvironment *spapr, > + uint32_t token, uint32_t nargs, > + target_ulong args, uint32_t nret, > + target_ulong rets) > +{ > + sPAPRPHBState *sphb; > + sPAPRPHBClass *spc; > + uint64_t buid; > + int ret; > + > + if ((nargs !=3D 3) || (nret !=3D 1)) { > + goto param_error_exit; > + } > + > + buid =3D ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); > + sphb =3D find_phb(spapr, buid); > + if (!sphb) { > + goto param_error_exit; > + } > + > + spc =3D SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + if (!spc->eeh_handler) { > + goto param_error_exit; > + } > + > + ret =3D spc->eeh_handler(sphb, RTAS_EEH_REQ_CONFIGURE, 0); > + if (ret < 0) { > + goto param_error_exit; > + } > + > + rtas_st(rets, 0, RTAS_OUT_SUCCESS); > + return; > + > +param_error_exit: > + rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > +} > + > +/* To support it later */ > +static void rtas_ibm_slot_error_detail(PowerPCCPU *cpu, > + sPAPREnvironment *spapr, > + uint32_t token, uint32_t nargs, > + target_ulong args, uint32_t nret, > + target_ulong rets) > +{ > + sPAPRPHBState *sphb; > + sPAPRPHBClass *spc; > + int option; > + uint64_t buid; > + > + if ((nargs !=3D 8) || (nret !=3D 1)) { > + goto no_error_log; Surely this should be RTAS_OUT_PARAM_ERROR, not RTAS_OUT_NO_ERRORS_FOUND. > + } > + > + buid =3D ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); > + sphb =3D find_phb(spapr, buid); > + if (!sphb) { > + goto no_error_log; =2E. and this one.. > + } > + > + spc =3D SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + if (!spc->eeh_handler) { > + goto no_error_log; =2E. possibly not this one .. > + } > + > + option =3D rtas_ld(args, 7); > + switch (option) { > + case RTAS_SLOT_TEMP_ERR_LOG: > + case RTAS_SLOT_PERM_ERR_LOG: > + break; > + default: > + goto no_error_log; =2E. but again this one .. > + } > + > + /* We don't get error log yet */ > + rtas_st(rets, 0, RTAS_OUT_NO_ERRORS_FOUND); > + return; > + > +no_error_log: > + rtas_st(rets, 0, RTAS_OUT_NO_ERRORS_FOUND); > +} > + > static int pci_spapr_swizzle(int slot, int pin) > { > return (slot + pin) % PCI_NUM_PINS; > @@ -964,6 +1255,25 @@ void spapr_pci_rtas_init(void) > spapr_rtas_register(RTAS_IBM_CHANGE_MSI, "ibm,change-msi", > rtas_ibm_change_msi); > } > + > + spapr_rtas_register(RTAS_IBM_SET_EEH_OPTION, > + "ibm,set-eeh-option", > + rtas_ibm_set_eeh_option); > + spapr_rtas_register(RTAS_IBM_GET_CONFIG_ADDR_INFO2, > + "ibm,get-config-addr-info2", > + rtas_ibm_get_config_addr_info2); > + spapr_rtas_register(RTAS_IBM_READ_SLOT_RESET_STATE2, > + "ibm,read-slot-reset-state2", > + rtas_ibm_read_slot_reset_state2); > + spapr_rtas_register(RTAS_IBM_SET_SLOT_RESET, > + "ibm,set-slot-reset", > + rtas_ibm_set_slot_reset); > + spapr_rtas_register(RTAS_IBM_CONFIGURE_PE, > + "ibm,configure-pe", > + rtas_ibm_configure_pe); > + spapr_rtas_register(RTAS_IBM_SLOT_ERROR_DETAIL, > + "ibm,slot-error-detail", > + rtas_ibm_slot_error_detail); > } > =20 > static void spapr_pci_register_types(void) > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h > index 876ecf0..bfde083 100644 > --- a/include/hw/pci-host/spapr.h > +++ b/include/hw/pci-host/spapr.h > @@ -49,6 +49,7 @@ struct sPAPRPHBClass { > PCIHostBridgeClass parent_class; > =20 > void (*finish_realize)(sPAPRPHBState *sphb, Error **errp); > + int (*eeh_handler)(sPAPRPHBState *sphb, int req, int opt); > }; > =20 > typedef struct spapr_pci_msi { > @@ -109,6 +110,12 @@ struct sPAPRPHBVFIOState { > =20 > #define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL > =20 > +/* EEH related requests */ > +#define RTAS_EEH_REQ_SET_OPTION 0 > +#define RTAS_EEH_REQ_GET_STATE 1 > +#define RTAS_EEH_REQ_RESET 2 > +#define RTAS_EEH_REQ_CONFIGURE 3 AFAICT these are internal constants only, not defined by PAPR, used to multiplex the eeh_handler callback. We have to cope with the multiplexed calls defined by PAPR, but we should avoid adding multiplexers of our own. I think these functions should each be separate callbacks in the PHB class. > static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int= pin) > { > return xics_get_qirq(spapr->icp, phb->lsi_table[pin].irq); > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 642cdc3..1cf5e89 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -338,6 +338,39 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target= _ulong opcode, > int spapr_allocate_irq(int hint, bool lsi); > int spapr_allocate_irq_block(int num, bool lsi, bool msi); > =20 > +/* ibm,set-eeh-option */ > +#define RTAS_EEH_DISABLE 0 > +#define RTAS_EEH_ENABLE 1 > +#define RTAS_EEH_THAW_IO 2 > +#define RTAS_EEH_THAW_DMA 3 > + > +/* ibm,get-config-addr-info2 */ > +#define RTAS_GET_PE_ADDR 0 > +#define RTAS_GET_PE_MODE 1 > +#define RTAS_PE_MODE_NONE 0 > +#define RTAS_PE_MODE_NOT_SHARED 1 > +#define RTAS_PE_MODE_SHARED 2 > + > +/* ibm,read-slot-reset-state2 */ > +#define RTAS_EEH_PE_STATE_NORMAL 0 > +#define RTAS_EEH_PE_STATE_RESET 1 > +#define RTAS_EEH_PE_STATE_STOPPED_IO_DMA 2 > +#define RTAS_EEH_PE_STATE_STOPPED_DMA 4 > +#define RTAS_EEH_PE_STATE_UNAVAIL 5 > +#define RTAS_EEH_NOT_SUPPORT 0 > +#define RTAS_EEH_SUPPORT 1 > +#define RTAS_EEH_PE_UNAVAIL_INFO 1000 > +#define RTAS_EEH_PE_RECOVER_INFO 0 > + > +/* ibm,set-slot-reset */ > +#define RTAS_SLOT_RESET_DEACTIVATE 0 > +#define RTAS_SLOT_RESET_HOT 1 > +#define RTAS_SLOT_RESET_FUNDAMENTAL 3 > + > +/* ibm,slot-error-detail */ > +#define RTAS_SLOT_TEMP_ERR_LOG 1 > +#define RTAS_SLOT_PERM_ERR_LOG 2 > + > /* RTAS return codes */ > #define RTAS_OUT_SUCCESS 0 > #define RTAS_OUT_NO_ERRORS_FOUND 1 > @@ -382,8 +415,14 @@ int spapr_allocate_irq_block(int num, bool lsi, bool= msi); > #define RTAS_GET_SENSOR_STATE (RTAS_TOKEN_BASE + 0x1D) > #define RTAS_IBM_CONFIGURE_CONNECTOR (RTAS_TOKEN_BASE + 0x1E) > #define RTAS_IBM_OS_TERM (RTAS_TOKEN_BASE + 0x1F) > - > -#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x20) > +#define RTAS_IBM_SET_EEH_OPTION (RTAS_TOKEN_BASE + 0x20) > +#define RTAS_IBM_GET_CONFIG_ADDR_INFO2 (RTAS_TOKEN_BASE + 0x21) > +#define RTAS_IBM_READ_SLOT_RESET_STATE2 (RTAS_TOKEN_BASE + 0x22) > +#define RTAS_IBM_SET_SLOT_RESET (RTAS_TOKEN_BASE + 0x23) > +#define RTAS_IBM_CONFIGURE_PE (RTAS_TOKEN_BASE + 0x24) > +#define RTAS_IBM_SLOT_ERROR_DETAIL (RTAS_TOKEN_BASE + 0x25) > + > +#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x26) > =20 > /* RTAS ibm,get-system-parameter token values */ > #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20 --=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 --zgY/UHCnsaNnNXRx Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJU0uFVAAoJEGw4ysog2bOS8FIP/iWtpirhjAYoGzWdvTYODqbi AgVpvwgLOIZWzku8oz0u5qe27ZNxwfFaN/UQnHoHzaBgFfJ4es72omXnZP5iMuXZ La4PVTyVSfqaxYavtVOl6aYuCbX4KoO/7WF8p6KCb67yoBBs9so9GmuzBtVvjt7n xNnpXi6D8rcSHWTE7+9hlPbCN4bsW3c9e94VTONsJQcfPOyymppAHPQjHrjGlASx J02T5yY9hRRaZV2Up5MB8rlHqzTliiEqspJVfqJ3kGAJxvI0IVv9YtQRGfs/Izkp IVhTTCay9VCneHtkPl4cBDdUuT2uNFTQPEkgkVFDrA62Fp2IKb8dKckSgmGUdoiA sZFQ2K1Yphh9Bj2lsQyxCwK69NWPBG2DIe7ZVZsmsCiBC4TbGCJ5miEd6n6w9hDB tY0PCii3iPQQN8tw27jgVnfWnJy5/rCnShT5B3+9ncV3pJ2ziVHXfE5q8ZbSjFi/ ldisqBuBcoUv8fVBGnnBHu2rq0Npvf4sbxXJ991yoK2vhG7QNOIhFCQU6NV7VkDi u/mklA4KTkw3lXiseK9/7SYgYn/5M7UhHk7wFcJfK6JZOzi+IzW6wlDLmb4Cyp8h AFlBdbaNQQo2rMCqjh/IevDdblija037ZaVgK17smUIn+cVgGC+n9ZZevmOZrdf3 gnb1FpB08MGHvDOLLxPv =K9st -----END PGP SIGNATURE----- --zgY/UHCnsaNnNXRx--