From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43847) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZRlGK-0005rS-Tm for qemu-devel@nongnu.org; Tue, 18 Aug 2015 14:05:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZRlGH-0003Zj-IL for qemu-devel@nongnu.org; Tue, 18 Aug 2015 14:05:28 -0400 Message-ID: <55D373CB.7040905@redhat.com> Date: Tue, 18 Aug 2015 11:04:59 -0700 From: Thomas Huth MIME-Version: 1.0 References: <1439862430-14996-1-git-send-email-gwshan@linux.vnet.ibm.com> <1439862430-14996-5-git-send-email-gwshan@linux.vnet.ibm.com> In-Reply-To: <1439862430-14996-5-git-send-email-gwshan@linux.vnet.ibm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v5 4/4] sPAPR: Support RTAS call ibm, errinjct List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gavin Shan , qemu-devel@nongnu.org Cc: aik@ozlabs.ru, peter.maydell@linaro.org, qemu-ppc@nongnu.org, david@gibson.dropbear.id.au On 17/08/15 18:47, Gavin Shan wrote: > The patch supports RTAS call "ibm,errinjct" to allow injecting > EEH errors to VFIO PCI devices. The implementation is similiar > to EEH support for VFIO PCI devices: The RTAS request is captured > by QEMU and routed to sPAPRPHBClass::eeh_inject_error() where the > request is translated to VFIO container IOCTL command to be handled > by the host. >=20 > Signed-off-by: Gavin Shan > --- > hw/ppc/spapr_pci.c | 36 +++++++++++++++++++++ > hw/ppc/spapr_pci_vfio.c | 56 +++++++++++++++++++++++++++++++++ > hw/ppc/spapr_rtas.c | 77 +++++++++++++++++++++++++++++++++++++= ++++++++ > include/hw/pci-host/spapr.h | 2 ++ > include/hw/ppc/spapr.h | 9 +++++- > 5 files changed, 179 insertions(+), 1 deletion(-) >=20 > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 9d41060..f6223ce 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -682,6 +682,42 @@ param_error_exit: > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > } > =20 > +int spapr_rtas_errinjct_ioa(sPAPRMachineState *spapr, > + target_ulong param_buf, > + bool is_64bits) > +{ > + sPAPRPHBState *sphb; > + sPAPRPHBClass *spc; > + uint64_t buid, addr, mask; > + uint32_t func; > + > + if (is_64bits) { > + addr =3D ((uint64_t)rtas_ld(param_buf, 0) << 32) | rtas_ld(par= am_buf, 1); > + mask =3D ((uint64_t)rtas_ld(param_buf, 2) << 32) | rtas_ld(par= am_buf, 3); > + buid =3D ((uint64_t)rtas_ld(param_buf, 5) << 32) | rtas_ld(par= am_buf, 6); You might want to consider to introduce a helper function (e.g "ras_ld64"?) that loads the two 32 bit values and combines them. > + func =3D rtas_ld(param_buf, 7); > + } else { > + addr =3D rtas_ld(param_buf, 0); > + mask =3D rtas_ld(param_buf, 1); > + buid =3D ((uint64_t)rtas_ld(param_buf, 3) << 32) | rtas_ld(par= am_buf, 4); > + func =3D rtas_ld(param_buf, 5); > + } > + > + /* Find PHB */ > + sphb =3D spapr_pci_find_phb(spapr, buid); > + if (!sphb) { > + return RTAS_OUT_PARAM_ERROR; > + } > + > + spc =3D SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > + if (!spc->eeh_inject_error) { > + return RTAS_OUT_PARAM_ERROR; > + } > + > + /* Handle the request */ > + return spc->eeh_inject_error(sphb, func, addr, mask, is_64bits); > +} > + > static int pci_spapr_swizzle(int slot, int pin) > { > return (slot + pin) % PCI_NUM_PINS; > diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c > index cca45ed..a3674ee 100644 > --- a/hw/ppc/spapr_pci_vfio.c > +++ b/hw/ppc/spapr_pci_vfio.c > @@ -17,6 +17,8 @@ > * along with this program; if not, see . > */ > =20 > +#include This does not work when building on non-powerpc systems. I think you have to use something like this instead: #include "asm-powerpc/eeh.h" > #include "hw/ppc/spapr.h" > #include "hw/pci-host/spapr.h" > #include "hw/pci/msix.h" > @@ -250,6 +252,59 @@ static int spapr_phb_vfio_eeh_configure(sPAPRPHBSt= ate *sphb) > return RTAS_OUT_SUCCESS; > } > =20 > +static int spapr_phb_vfio_eeh_inject_error(sPAPRPHBState *sphb, > + uint32_t func, uint64_t add= r, > + uint64_t mask, bool is_64bi= ts) > +{ > + sPAPRPHBVFIOState *svphb =3D SPAPR_PCI_VFIO_HOST_BRIDGE(sphb); > + struct vfio_eeh_pe_op op =3D { > + .op =3D VFIO_EEH_PE_INJECT_ERR, > + .argsz =3D sizeof(op) > + }; > + int ret =3D RTAS_OUT_SUCCESS; > + > + op.err.type =3D is_64bits ? EEH_ERR_TYPE_64 : EEH_ERR_TYPE_32; > + op.err.addr =3D addr; > + op.err.mask =3D mask; > + > + switch (func) { > + case EEH_ERR_FUNC_LD_MEM_ADDR: > + case EEH_ERR_FUNC_LD_MEM_DATA: > + case EEH_ERR_FUNC_LD_IO_ADDR: > + case EEH_ERR_FUNC_LD_IO_DATA: > + case EEH_ERR_FUNC_LD_CFG_ADDR: > + case EEH_ERR_FUNC_LD_CFG_DATA: > + case EEH_ERR_FUNC_ST_MEM_ADDR: > + case EEH_ERR_FUNC_ST_MEM_DATA: > + case EEH_ERR_FUNC_ST_IO_ADDR: > + case EEH_ERR_FUNC_ST_IO_DATA: > + case EEH_ERR_FUNC_ST_CFG_ADDR: > + case EEH_ERR_FUNC_ST_CFG_DATA: > + case EEH_ERR_FUNC_DMA_RD_ADDR: > + case EEH_ERR_FUNC_DMA_RD_DATA: > + case EEH_ERR_FUNC_DMA_RD_MASTER: > + case EEH_ERR_FUNC_DMA_RD_TARGET: > + case EEH_ERR_FUNC_DMA_WR_ADDR: > + case EEH_ERR_FUNC_DMA_WR_DATA: > + case EEH_ERR_FUNC_DMA_WR_MASTER: EEH_ERR_FUNC_DMA_WR_TARGET is missing in the list ... I assume this is on purpose? > + op.err.func =3D func; > + break; > + default: > + ret =3D RTAS_OUT_PARAM_ERROR; > + goto out; > + } > + > + if (vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid= , > + VFIO_EEH_PE_OP, &op) < 0) { > + ret =3D RTAS_OUT_HW_ERROR; > + goto out; > + } > + > + ret =3D RTAS_OUT_SUCCESS; > +out: > + return ret; > +} > + > static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc =3D DEVICE_CLASS(klass); > @@ -262,6 +317,7 @@ static void spapr_phb_vfio_class_init(ObjectClass *= klass, void *data) > spc->eeh_get_state =3D spapr_phb_vfio_eeh_get_state; > spc->eeh_reset =3D spapr_phb_vfio_eeh_reset; > spc->eeh_configure =3D spapr_phb_vfio_eeh_configure; > + spc->eeh_inject_error =3D spapr_phb_vfio_eeh_inject_error; > } > =20 > static const TypeInfo spapr_phb_vfio_info =3D { > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c > index 8405056..5645f43 100644 > --- a/hw/ppc/spapr_rtas.c > +++ b/hw/ppc/spapr_rtas.c > @@ -632,6 +632,54 @@ out: > rtas_st(rets, 1, ret); > } > =20 > +static void rtas_ibm_errinjct(PowerPCCPU *cpu, > + sPAPRMachineState *spapr, > + uint32_t token, uint32_t nargs, > + target_ulong args, uint32_t nret, > + target_ulong rets) > +{ > + target_ulong param_buf; > + uint32_t type, open_token; > + int32_t ret; > + > + /* Sanity check on number of arguments */ > + if ((nargs !=3D 3) || (nret !=3D 1)) { Less paranthesis, please? > + ret =3D RTAS_OUT_PARAM_ERROR; > + goto out; > + } > + > + /* Check if we have opened token */ > + open_token =3D rtas_ld(args, 1); > + if (!spapr->is_errinjct_opened || > + spapr->errinjct_token !=3D open_token) { > + ret =3D RTAS_OUT_CLOSE_ERROR; > + goto out; > + } > + > + /* The parameter buffer should be 1KB aligned */ > + param_buf =3D rtas_ld(args, 2); > + if (param_buf & 0x3ff) { > + ret =3D RTAS_OUT_PARAM_ERROR; > + goto out; > + } > + > + /* Check the error type */ > + type =3D rtas_ld(args, 0); > + switch (type) { > + case RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR: > + ret =3D spapr_rtas_errinjct_ioa(spapr, param_buf, false); > + break; > + case RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR64: > + ret =3D spapr_rtas_errinjct_ioa(spapr, param_buf, true); > + break; > + default: > + ret =3D RTAS_OUT_PARAM_ERROR; > + } > + > +out: > + rtas_st(rets, 0, ret); > +} > + > static void rtas_ibm_close_errinjct(PowerPCCPU *cpu, > sPAPRMachineState *spapr, > uint32_t token, uint32_t nargs, > @@ -723,6 +771,33 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr= rtas_addr, > int i; > uint32_t lrdr_capacity[5]; > MachineState *machine =3D MACHINE(qdev_get_machine()); > + char errinjct_tokens[1024]; > + int fdt_offset, offset; > + const int tokens[] =3D { > + RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR, > + RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR64 > + }; > + const char *token_strings[] =3D { > + "ioa-bus-error", > + "ioa-bus-error-64" > + }; > + > + /* ibm,errinjct-tokens */ > + offset =3D 0; > + for (i =3D 0; i < ARRAY_SIZE(tokens); i++) { > + offset +=3D sprintf(errinjct_tokens + offset, "%s", token_stri= ngs[i]); > + errinjct_tokens[offset++] =3D '\0'; > + *(int *)(&errinjct_tokens[offset]) =3D tokens[i]; You can also get rid of some paranthesis here. Also I am not sure, but I think you have to take care of the endianess here? =3D=3D> Use stl_be_p i= nstead? > + offset +=3D sizeof(int); > + } > + > + fdt_offset =3D fdt_path_offset(fdt, "/rtas"); > + ret =3D fdt_setprop(fdt, fdt_offset, "ibm,errinjct-tokens", > + errinjct_tokens, offset); > + if (ret < 0) { > + fprintf(stderr, "Couldn't add ibm,errinjct-tokens\n"); > + return ret; > + } Thomas