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.
>
> Signed-off-by: Gavin Shan <gws...@linux.vnet.ibm.com>
> ---
> 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(-)
>
> 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);
> }
>
> +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 = ((uint64_t)rtas_ld(param_buf, 0) << 32) | rtas_ld(param_buf,
> 1);
> + mask = ((uint64_t)rtas_ld(param_buf, 2) << 32) | rtas_ld(param_buf,
> 3);
> + buid = ((uint64_t)rtas_ld(param_buf, 5) << 32) | rtas_ld(param_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 = rtas_ld(param_buf, 7);
> + } else {
> + addr = rtas_ld(param_buf, 0);
> + mask = rtas_ld(param_buf, 1);
> + buid = ((uint64_t)rtas_ld(param_buf, 3) << 32) | rtas_ld(param_buf,
> 4);
> + func = rtas_ld(param_buf, 5);
> + }
> +
> + /* Find PHB */
> + sphb = spapr_pci_find_phb(spapr, buid);
> + if (!sphb) {
> + return RTAS_OUT_PARAM_ERROR;
> + }
> +
> + spc = 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 <http://www.gnu.org/licenses/>.
> */
>
> +#include <asm/eeh.h>
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(sPAPRPHBState
> *sphb)
> return RTAS_OUT_SUCCESS;
> }
>
> +static int spapr_phb_vfio_eeh_inject_error(sPAPRPHBState *sphb,
> + uint32_t func, uint64_t addr,
> + uint64_t mask, bool is_64bits)
> +{
> + sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> + struct vfio_eeh_pe_op op = {
> + .op = VFIO_EEH_PE_INJECT_ERR,
> + .argsz = sizeof(op)
> + };
> + int ret = RTAS_OUT_SUCCESS;
> +
> + op.err.type = is_64bits ? EEH_ERR_TYPE_64 : EEH_ERR_TYPE_32;
> + op.err.addr = addr;
> + op.err.mask = 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 = func;
> + break;
> + default:
> + ret = RTAS_OUT_PARAM_ERROR;
> + goto out;
> + }
> +
> + if (vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
> + VFIO_EEH_PE_OP, &op) < 0) {
> + ret = RTAS_OUT_HW_ERROR;
> + goto out;
> + }
> +
> + ret = RTAS_OUT_SUCCESS;
> +out:
> + return ret;
> +}
> +
> static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -262,6 +317,7 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass,
> void *data)
> spc->eeh_get_state = spapr_phb_vfio_eeh_get_state;
> spc->eeh_reset = spapr_phb_vfio_eeh_reset;
> spc->eeh_configure = spapr_phb_vfio_eeh_configure;
> + spc->eeh_inject_error = spapr_phb_vfio_eeh_inject_error;
> }
>
> static const TypeInfo spapr_phb_vfio_info = {
> 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);
> }
>
> +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 != 3) || (nret != 1)) {
Less paranthesis, please?
> + ret = RTAS_OUT_PARAM_ERROR;
> + goto out;
> + }
> +
> + /* Check if we have opened token */
> + open_token = rtas_ld(args, 1);
> + if (!spapr->is_errinjct_opened ||
> + spapr->errinjct_token != open_token) {
> + ret = RTAS_OUT_CLOSE_ERROR;
> + goto out;
> + }
> +
> + /* The parameter buffer should be 1KB aligned */
> + param_buf = rtas_ld(args, 2);
> + if (param_buf & 0x3ff) {
> + ret = RTAS_OUT_PARAM_ERROR;
> + goto out;
> + }
> +
> + /* Check the error type */
> + type = rtas_ld(args, 0);
> + switch (type) {
> + case RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR:
> + ret = spapr_rtas_errinjct_ioa(spapr, param_buf, false);
> + break;
> + case RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR64:
> + ret = spapr_rtas_errinjct_ioa(spapr, param_buf, true);
> + break;
> + default:
> + ret = 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 = MACHINE(qdev_get_machine());
> + char errinjct_tokens[1024];
> + int fdt_offset, offset;
> + const int tokens[] = {
> + RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR,
> + RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR64
> + };
> + const char *token_strings[] = {
> + "ioa-bus-error",
> + "ioa-bus-error-64"
> + };
> +
> + /* ibm,errinjct-tokens */
> + offset = 0;
> + for (i = 0; i < ARRAY_SIZE(tokens); i++) {
> + offset += sprintf(errinjct_tokens + offset, "%s", token_strings[i]);
> + errinjct_tokens[offset++] = '\0';
> + *(int *)(&errinjct_tokens[offset]) = 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? ==> Use stl_be_p instead?
> + offset += sizeof(int);
> + }
> +
> + fdt_offset = fdt_path_offset(fdt, "/rtas");
> + ret = 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
