[Qemu-devel] [PATCH v1 0/3] Support PCI Error Injection

[Qemu-devel] [PATCH v1 0/3] Support PCI Error Injection

[Gavin Shan]

There is one guest userland utility "errinjct" used to inject various types
of errors for testing purpose. The utility works with 3 RTAS calls, which
are defined in PAPR spec as follows:

- "ibm,open-errinjct": Apply for token to do error injection
- "ibm,close-errinjct": Free the token assigned previously
- "ibm,errinjct": Do error injection

The series of patches support above RTAS calls in order to support error
injection. For now, we only support PCI related error injection (32-bits
and 64-bits PCI error types) and have to figure out the error injection
for other types in future.

It requires corresponding kernel changes as follows. Please comments, thanks!

http://patchwork.ozlabs.org/patch/362637/
http://patchwork.ozlabs.org/patch/362638/
http://patchwork.ozlabs.org/patch/362639/

Gavin Shan (3):
  sPAPR: Implement PCI error injection RTAS calls
  sPAPR: Implement sPAPRPHBClass::format_errinjct_cmd
  sPAPR: Export RTAS property <ibm,errinjct-tokens>

 hw/ppc/spapr.c              |  19 +++++
 hw/ppc/spapr_pci_vfio.c     |  19 +++++
 hw/ppc/spapr_rtas.c         | 198 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/pci-host/spapr.h |  11 +++
 include/hw/ppc/spapr.h      |  35 ++++++++
 5 files changed, 282 insertions(+)

-- 
1.8.3.2

[Qemu-devel] [PATCH v1 1/3] sPAPR: Implement PCI error injection RTAS calls

[Gavin Shan]

The patch implements PCI error injection RTAS calls for sPAPR
platform, which are defined PAPR spec as follows. Those RTAS
calls are expected to be invoked in strict sequence of
"ibm,open-errinjct", "ibm,errinjct", "ibm,close-errinjct".

- "ibm,open-errinjct": Apply for token to do error injection
- "ibm,close-errinjct": Free the token assigned previously
- "ibm,errinjct": Do error injection

The patch defines constants used by error injection RTAS calls
and adds callback sPAPRPHBClass::format_errinjct_cmd, which is
going to be used like this way:

- Error injection RTAS calls are received in spapr_rtas.c. If that's
  "ibm,open-errinjct" or "ibm,close-errinjct", we handle it there
  with the help of static counter "sPAPREnvironment::errinjct_token_cnt".
  If the  RTAS call is "ibm,errinjct", sanity check is done there and
  just returns failure if the error type isn't PCI related. Otherwise,
  the input parameters are figured out and route the request to
  sPAPRPHBClass::format_errinjct_cmd.
- sPAPRPHBClass::format_errinjct_cmd translates the address (BUID, PE
  number) to IOMMU group ID and then return the formated string back to
  spapr_rtas.c where the string is writen to firmware sysfs file
  "/sys/firmware/opal/errinjct" to do error injection.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 hw/ppc/spapr_rtas.c         | 198 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/pci-host/spapr.h |  11 +++
 include/hw/ppc/spapr.h      |  35 ++++++++
 3 files changed, 244 insertions(+)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index a7233e4..94fb866 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -32,6 +32,7 @@
 
 #include "hw/ppc/spapr.h"
 #include "hw/ppc/spapr_vio.h"
+#include "hw/pci-host/spapr.h"
 
 #include <libfdt.h>
 
@@ -268,6 +269,196 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
     rtas_st(rets, 0, ret);
 }
 
+static sPAPRErrInjct *rtas_find_errinjct_token(sPAPREnvironment *spapr,
+                                               uint32_t token_num)
+{
+    sPAPRErrInjct *open_token;
+
+    QLIST_FOREACH(open_token, &spapr->errinjct_open_tokens, list) {
+        if (open_token->token == token_num) {
+            return open_token;
+        }
+    }
+
+    return NULL;
+}
+
+static void rtas_ibm_open_errinjct(PowerPCCPU *cpu,
+                                   sPAPREnvironment *spapr,
+                                   uint32_t token, uint32_t nargs,
+                                   target_ulong args, uint32_t nret,
+                                   target_ulong rets)
+{
+    sPAPRErrInjct *open_token;
+    int32_t ret = RTAS_OUT_HW_ERROR;
+
+    /* Sanity check on number of arguments */
+    if ((nargs != 0) || (nret != 2)) {
+        goto out;
+    }
+
+    /* Allocate token */
+    open_token = g_malloc(sizeof(*open_token));
+    if (!open_token) {
+        goto out;
+    }
+    open_token->token = spapr->errinjct_open_token_cnt++;
+    QLIST_INSERT_HEAD(&spapr->errinjct_open_tokens, open_token, list);
+
+    /* Success */
+    rtas_st(rets, 0, open_token->token);
+    ret = RTAS_OUT_SUCCESS;
+out:
+    rtas_st(rets, 1, ret);
+}
+
+static char *rtas_do_errinjct_ioa(sPAPREnvironment *spapr,
+                                sPAPRErrInjctIOA *ioa)
+{
+    sPAPRPHBState *sphb;
+    sPAPRPHBClass *info;
+
+    /* Sanity check on argument */
+    if (!spapr || !ioa) {
+        return NULL;
+    }
+
+    /* Invalid option ? */
+    if (ioa->func > RTAS_ERRINJCT_IOA_DMA_WR_MEM_TARGET) {
+        return NULL;
+    }
+
+    /* Find PHB */
+    sphb = spapr_find_phb(spapr, ioa->buid);
+    if (!sphb) {
+        return NULL;
+    }
+
+    /* PHB doesn't support error injection ? */
+    info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
+    if (!info->format_errinjct_cmd) {
+        return NULL;
+    }
+
+    return info->format_errinjct_cmd(sphb, ioa);
+}
+
+static void rtas_ibm_errinjct(PowerPCCPU *cpu,
+                              sPAPREnvironment *spapr,
+                              uint32_t token, uint32_t nargs,
+                              target_ulong args, uint32_t nret,
+                              target_ulong rets)
+{
+    sPAPRErrInjctIOA ioa;
+    sPAPRErrInjct *open_token;
+    uint32_t token_num, ei_token;
+    target_ulong param_buf;
+    char *pbuf;
+    int fd;
+    int32_t ret = RTAS_OUT_PARAM_ERROR;
+
+    /* Sanity check on number of arguments */
+    if ((nargs != 3) || (nret != 1)) {
+        goto out;
+    }
+
+    /* Check if we have opened token */
+    token_num = rtas_ld(args, 1);
+    open_token = rtas_find_errinjct_token(spapr, token_num);
+    if (!open_token) {
+        goto out;
+    }
+
+    /* Argument buffer should be aligned with 1KB */
+    param_buf = rtas_ld(args, 2);
+    if (param_buf & 0x3ff) {
+        goto out;
+    }
+
+    /* We only support IOA error for now */
+    ei_token = rtas_ld(args, 0);
+    switch (ei_token) {
+    case RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR:
+        ioa.ei_token = RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR;
+        ioa.addr = rtas_ld(param_buf, 0);
+        ioa.mask = rtas_ld(param_buf, 1);
+        ioa.cfg_addr = rtas_ld(param_buf, 2);
+        ioa.buid = ((uint64_t)rtas_ld(param_buf, 3) << 32) | rtas_ld(param_buf, 4);
+        ioa.func = rtas_ld(param_buf, 5);
+
+        break;
+    case RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR64:
+        ioa.ei_token = RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR64;
+        ioa.addr = ((uint64_t)rtas_ld(param_buf, 0) << 32) | rtas_ld(param_buf, 1);
+        ioa.mask = ((uint64_t)rtas_ld(param_buf, 2) << 32) | rtas_ld(param_buf, 3);
+        ioa.cfg_addr = rtas_ld(param_buf, 4);
+        ioa.buid = ((uint64_t)rtas_ld(param_buf, 5) << 32) | rtas_ld(param_buf, 6);
+        ioa.func = rtas_ld(param_buf, 7);
+
+        break;
+    default:
+        goto out;
+    }
+
+    /* We have nothing written to sysfs */
+    pbuf = rtas_do_errinjct_ioa(spapr, &ioa);
+    if (!pbuf) {
+        goto out;
+    }
+
+    /* Open firmware sysfs file */
+    fd = qemu_open("/sys/firmware/opal/errinjct", O_WRONLY);
+    if (fd < 0) {
+        goto free_buf_out;
+    }
+
+    /* Inject error */
+    if (pwrite(fd, pbuf, strlen(pbuf), 0) != strlen(pbuf)) {
+        goto close_fd_out;
+    }
+
+    /* Success */
+    ret = RTAS_OUT_SUCCESS;
+
+close_fd_out:
+    qemu_close(fd);
+free_buf_out:
+    g_free(pbuf);
+out:
+    rtas_st(rets, 0, ret);
+}
+
+static void rtas_ibm_close_errinjct(PowerPCCPU *cpu,
+                                    sPAPREnvironment *spapr,
+                                    uint32_t token, uint32_t nargs,
+                                    target_ulong args, uint32_t nret,
+                                    target_ulong rets)
+{
+    sPAPRErrInjct *open_token;
+    uint32_t token_num;
+    int32_t ret = RTAS_OUT_HW_ERROR;
+
+    /* Sanity check on number of arguments */
+    if ((nargs != 1) || (nret != 1)) {
+        goto out;
+    }
+
+    /* Do we have opened token ? */
+    token_num = rtas_ld(args, 0);
+    open_token = rtas_find_errinjct_token(spapr, token_num);
+    if (!open_token) {
+        goto out;
+    }
+
+    QLIST_REMOVE(open_token, list);
+    g_free(open_token);
+
+    /* Success */
+    ret = RTAS_OUT_SUCCESS;
+out:
+    rtas_st(rets, 0, ret);
+}
+
 static struct rtas_call {
     const char *name;
     spapr_rtas_fn fn;
@@ -393,6 +584,13 @@ static void core_rtas_register_types(void)
                         rtas_ibm_get_system_parameter);
     spapr_rtas_register("ibm,set-system-parameter",
                         rtas_ibm_set_system_parameter);
+
+    spapr_rtas_register("ibm,open-errinjct",
+                        rtas_ibm_open_errinjct);
+    spapr_rtas_register("ibm,errinjct",
+                        rtas_ibm_errinjct);
+    spapr_rtas_register("ibm,close-errinjct",
+                        rtas_ibm_close_errinjct);
 }
 
 type_init(core_rtas_register_types)
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 4b1f6aa..5fd64d5 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -41,15 +41,26 @@
 #define SPAPR_PCI_HOST_BRIDGE_GET_CLASS(obj) \
      OBJECT_GET_CLASS(sPAPRPHBClass, (obj), TYPE_SPAPR_PCI_HOST_BRIDGE)
 
+typedef struct sPAPRErrInjctIOA sPAPRErrInjctIOA;
 typedef struct sPAPRPHBClass sPAPRPHBClass;
 typedef struct sPAPRPHBState sPAPRPHBState;
 typedef struct sPAPRPHBVFIOState sPAPRPHBVFIOState;
 
+struct sPAPRErrInjctIOA {
+    uint32_t ei_token;
+    uint64_t addr;
+    uint64_t mask;
+    uint64_t buid;
+    uint32_t cfg_addr;
+    uint32_t func;
+};
+
 struct sPAPRPHBClass {
     PCIHostBridgeClass parent_class;
 
     void (*finish_realize)(sPAPRPHBState *sphb, Error **errp);
     int (*eeh_handler)(sPAPRPHBState *sphb, int req, int opt);
+    char *(*format_errinjct_cmd)(sPAPRPHBState *sphb, sPAPRErrInjctIOA *ioa);
 };
 
 struct sPAPRPHBState {
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 7167ab2..79f7eb1 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -9,6 +9,13 @@ struct VIOsPAPRBus;
 struct sPAPRPHBState;
 struct sPAPRNVRAM;
 
+typedef struct sPAPRErrInjct sPAPRErrInjct;
+
+struct sPAPRErrInjct {
+	uint32_t token;
+	QLIST_ENTRY(sPAPRErrInjct) list;
+};
+
 #define HPTE64_V_HPTE_DIRTY     0x0000000000000040ULL
 
 typedef struct sPAPREnvironment {
@@ -41,6 +48,10 @@ typedef struct sPAPREnvironment {
     bool htab_first_pass;
     int htab_fd;
 
+    /* Error injection tokens */
+    uint32_t errinjct_open_token_cnt;
+    QLIST_HEAD(, sPAPRErrInjct) errinjct_open_tokens;
+
     /* platform state - sensors and indicators */
     uint32_t state;
 } sPAPREnvironment;
@@ -439,6 +450,30 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi);
 #define RTAS_SLOT_TEMP_ERR_LOG			1
 #define RTAS_SLOT_PERM_ERR_LOG			2
 
+/* ibm,errinjct */
+#define RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR	7
+#define RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR64	8
+#define  RTAS_ERRINJCT_IOA_LD_MEM_ADDR		0
+#define  RTAS_ERRINJCT_IOA_LD_MEM_DATA		1
+#define  RTAS_ERRINJCT_IOA_LD_IO_ADDR		2
+#define  RTAS_ERRINJCT_IOA_LD_IO_DATA		3
+#define  RTAS_ERRINJCT_IOA_LD_CONFIG_ADDR	4
+#define  RTAS_ERRINJCT_IOA_LD_CONFIG_DATA	5
+#define  RTAS_ERRINJCT_IOA_ST_MEM_ADDR		6
+#define  RTAS_ERRINJCT_IOA_ST_MEM_DATA		7
+#define  RTAS_ERRINJCT_IOA_ST_IO_ADDR		8
+#define  RTAS_ERRINJCT_IOA_ST_IO_DATA		9
+#define  RTAS_ERRINJCT_IOA_ST_CONFIG_ADDR	10
+#define  RTAS_ERRINJCT_IOA_ST_CONFIG_DATA	11
+#define  RTAS_ERRINJCT_IOA_DMA_RD_MEM_ADDR	12
+#define  RTAS_ERRINJCT_IOA_DMA_RD_MEM_DATA	13
+#define  RTAS_ERRINJCT_IOA_DMA_RD_MEM_MASTER	14
+#define  RTAS_ERRINJCT_IOA_DMA_RD_MEM_TARGET	15
+#define  RTAS_ERRINJCT_IOA_DMA_WR_MEM_ADDR	16
+#define  RTAS_ERRINJCT_IOA_DMA_WR_MEM_DATA	17
+#define  RTAS_ERRINJCT_IOA_DMA_WR_MEM_MASTER	18
+#define  RTAS_ERRINJCT_IOA_DMA_WR_MEM_TARGET	19
+
 /* RTAS return codes */
 #define RTAS_OUT_SUCCESS            0
 #define RTAS_OUT_NO_ERRORS_FOUND    1
-- 
1.8.3.2

[Qemu-devel] [PATCH v1 2/3] sPAPR: Implement sPAPRPHBClass::format_errinjct_cmd

[Gavin Shan]

The patch implements sPAPRPHBClass::format_errinjct_cmd to do the
address translation (BUID+PE number to IOMMU group ID) and then
come up with the formatted string for PCI error injection.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 hw/ppc/spapr_pci_vfio.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index 0a1e902..1f9f0bf 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -227,6 +227,24 @@ static int spapr_phb_vfio_eeh_handler(sPAPRPHBState *sphb, int req, int opt)
                                 VFIO_EEH_PE_OP, &op);
 }
 
+static char *format_errinjct_cmd(sPAPRPHBState *sphb, sPAPRErrInjctIOA *ioa)
+{
+    sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
+    char *pstr = NULL;
+
+    /* Invalid IOMMU group ID ? */
+    if (svphb->iommugroupid == -1) {
+        goto out;
+    }
+
+    /* Formatted string */
+    pstr = g_strdup_printf("%x:%lx:%lx:%x:%x",
+                           ioa->ei_token, ioa->addr, ioa->mask,
+                           svphb->iommugroupid, ioa->func);
+out:
+    return pstr;
+}
+
 static void spapr_phb_vfio_reset(DeviceState *qdev)
 {
     /* Do nothing */
@@ -241,6 +259,7 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data)
     dc->reset = spapr_phb_vfio_reset;
     spc->finish_realize = spapr_phb_vfio_finish_realize;
     spc->eeh_handler = spapr_phb_vfio_eeh_handler;
+    spc->format_errinjct_cmd = format_errinjct_cmd;
 }
 
 static const TypeInfo spapr_phb_vfio_info = {
-- 
1.8.3.2

[Qemu-devel] [PATCH v1 3/3] sPAPR: Export RTAS property <ibm, errinjct-tokens>

[Gavin Shan]

The patch exports RTAS property "ibm,errinjct-tokens", which is
defined in PAPR spec and used to indicate various error types
we can inject.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a61af85..1d52229 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -443,6 +443,23 @@ static void spapr_create_drc_dt_entries(void *fdt)
     } while (0)
 
 
+static void add_errinjct_token(GString *s, uint32_t token, const gchar *desc)
+{
+    g_string_append_len(s, desc, strlen(desc) + 1);
+    g_string_append_len(s, (gchar *)&token, sizeof(token));
+}
+
+static void add_errinjct_token_prop(void *fdt)
+{
+    GString *s = g_string_sized_new(256);
+
+    add_errinjct_token(s, RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR, "ioa-bus-error");
+    add_errinjct_token(s, RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR64, "ioa-bus-error-64");
+
+    _FDT((fdt_property(fdt, "ibm,errinjct-tokens", s->str, s->len)));
+    g_string_free(s, true);
+}
+
 static void *spapr_create_fdt_skel(hwaddr initrd_base,
                                    hwaddr initrd_size,
                                    hwaddr kernel_size,
@@ -664,6 +681,8 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
     _FDT((fdt_property(fdt, "qemu,hypertas-functions", qemu_hypertas_prop,
                        sizeof(qemu_hypertas_prop))));
 
+    add_errinjct_token_prop(fdt);
+
     _FDT((fdt_property(fdt, "ibm,associativity-reference-points",
         refpoints, sizeof(refpoints))));
 
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH v1 1/3] sPAPR: Implement PCI error injection RTAS calls

[Alexander Graf]

On 23.06.14 04:22, Gavin Shan wrote:
> The patch implements PCI error injection RTAS calls for sPAPR
> platform, which are defined PAPR spec as follows. Those RTAS
> calls are expected to be invoked in strict sequence of
> "ibm,open-errinjct", "ibm,errinjct", "ibm,close-errinjct".
>
> - "ibm,open-errinjct": Apply for token to do error injection
> - "ibm,close-errinjct": Free the token assigned previously
> - "ibm,errinjct": Do error injection
>
> The patch defines constants used by error injection RTAS calls
> and adds callback sPAPRPHBClass::format_errinjct_cmd, which is
> going to be used like this way:
>
> - Error injection RTAS calls are received in spapr_rtas.c. If that's
>    "ibm,open-errinjct" or "ibm,close-errinjct", we handle it there
>    with the help of static counter "sPAPREnvironment::errinjct_token_cnt".
>    If the  RTAS call is "ibm,errinjct", sanity check is done there and
>    just returns failure if the error type isn't PCI related. Otherwise,
>    the input parameters are figured out and route the request to
>    sPAPRPHBClass::format_errinjct_cmd.
> - sPAPRPHBClass::format_errinjct_cmd translates the address (BUID, PE
>    number) to IOMMU group ID and then return the formated string back to
>    spapr_rtas.c where the string is writen to firmware sysfs file
>    "/sys/firmware/opal/errinjct" to do error injection.
>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
>   hw/ppc/spapr_rtas.c         | 198 ++++++++++++++++++++++++++++++++++++++++++++
>   include/hw/pci-host/spapr.h |  11 +++
>   include/hw/ppc/spapr.h      |  35 ++++++++
>   3 files changed, 244 insertions(+)
>
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index a7233e4..94fb866 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -32,6 +32,7 @@
>   
>   #include "hw/ppc/spapr.h"
>   #include "hw/ppc/spapr_vio.h"
> +#include "hw/pci-host/spapr.h"
>   
>   #include <libfdt.h>
>   
> @@ -268,6 +269,196 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
>       rtas_st(rets, 0, ret);
>   }
>   
> +static sPAPRErrInjct *rtas_find_errinjct_token(sPAPREnvironment *spapr,
> +                                               uint32_t token_num)
> +{
> +    sPAPRErrInjct *open_token;
> +
> +    QLIST_FOREACH(open_token, &spapr->errinjct_open_tokens, list) {
> +        if (open_token->token == token_num) {
> +            return open_token;
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +static void rtas_ibm_open_errinjct(PowerPCCPU *cpu,
> +                                   sPAPREnvironment *spapr,
> +                                   uint32_t token, uint32_t nargs,
> +                                   target_ulong args, uint32_t nret,
> +                                   target_ulong rets)
> +{
> +    sPAPRErrInjct *open_token;
> +    int32_t ret = RTAS_OUT_HW_ERROR;
> +
> +    /* Sanity check on number of arguments */
> +    if ((nargs != 0) || (nret != 2)) {
> +        goto out;
> +    }
> +
> +    /* Allocate token */
> +    open_token = g_malloc(sizeof(*open_token));

With this hypercall the guest can OOM us by simply allocating useless 
tokens.

> +    if (!open_token) {
> +        goto out;
> +    }
> +    open_token->token = spapr->errinjct_open_token_cnt++;
> +    QLIST_INSERT_HEAD(&spapr->errinjct_open_tokens, open_token, list);
> +
> +    /* Success */
> +    rtas_st(rets, 0, open_token->token);
> +    ret = RTAS_OUT_SUCCESS;
> +out:
> +    rtas_st(rets, 1, ret);
> +}
> +
> +static char *rtas_do_errinjct_ioa(sPAPREnvironment *spapr,
> +                                sPAPRErrInjctIOA *ioa)
> +{
> +    sPAPRPHBState *sphb;
> +    sPAPRPHBClass *info;
> +
> +    /* Sanity check on argument */
> +    if (!spapr || !ioa) {
> +        return NULL;
> +    }
> +
> +    /* Invalid option ? */
> +    if (ioa->func > RTAS_ERRINJCT_IOA_DMA_WR_MEM_TARGET) {
> +        return NULL;
> +    }
> +
> +    /* Find PHB */
> +    sphb = spapr_find_phb(spapr, ioa->buid);
> +    if (!sphb) {
> +        return NULL;
> +    }
> +
> +    /* PHB doesn't support error injection ? */
> +    info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> +    if (!info->format_errinjct_cmd) {
> +        return NULL;
> +    }
> +
> +    return info->format_errinjct_cmd(sphb, ioa);
> +}
> +
> +static void rtas_ibm_errinjct(PowerPCCPU *cpu,
> +                              sPAPREnvironment *spapr,
> +                              uint32_t token, uint32_t nargs,
> +                              target_ulong args, uint32_t nret,
> +                              target_ulong rets)
> +{
> +    sPAPRErrInjctIOA ioa;
> +    sPAPRErrInjct *open_token;
> +    uint32_t token_num, ei_token;
> +    target_ulong param_buf;
> +    char *pbuf;
> +    int fd;
> +    int32_t ret = RTAS_OUT_PARAM_ERROR;
> +
> +    /* Sanity check on number of arguments */
> +    if ((nargs != 3) || (nret != 1)) {
> +        goto out;
> +    }
> +
> +    /* Check if we have opened token */
> +    token_num = rtas_ld(args, 1);
> +    open_token = rtas_find_errinjct_token(spapr, token_num);
> +    if (!open_token) {
> +        goto out;
> +    }
> +
> +    /* Argument buffer should be aligned with 1KB */
> +    param_buf = rtas_ld(args, 2);
> +    if (param_buf & 0x3ff) {
> +        goto out;
> +    }
> +
> +    /* We only support IOA error for now */
> +    ei_token = rtas_ld(args, 0);
> +    switch (ei_token) {
> +    case RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR:
> +        ioa.ei_token = RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR;
> +        ioa.addr = rtas_ld(param_buf, 0);
> +        ioa.mask = rtas_ld(param_buf, 1);
> +        ioa.cfg_addr = rtas_ld(param_buf, 2);
> +        ioa.buid = ((uint64_t)rtas_ld(param_buf, 3) << 32) | rtas_ld(param_buf, 4);
> +        ioa.func = rtas_ld(param_buf, 5);
> +
> +        break;
> +    case RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR64:
> +        ioa.ei_token = RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR64;
> +        ioa.addr = ((uint64_t)rtas_ld(param_buf, 0) << 32) | rtas_ld(param_buf, 1);
> +        ioa.mask = ((uint64_t)rtas_ld(param_buf, 2) << 32) | rtas_ld(param_buf, 3);
> +        ioa.cfg_addr = rtas_ld(param_buf, 4);
> +        ioa.buid = ((uint64_t)rtas_ld(param_buf, 5) << 32) | rtas_ld(param_buf, 6);
> +        ioa.func = rtas_ld(param_buf, 7);
> +
> +        break;
> +    default:
> +        goto out;
> +    }
> +
> +    /* We have nothing written to sysfs */
> +    pbuf = rtas_do_errinjct_ioa(spapr, &ioa);
> +    if (!pbuf) {
> +        goto out;
> +    }
> +
> +    /* Open firmware sysfs file */
> +    fd = qemu_open("/sys/firmware/opal/errinjct", O_WRONLY);

Device emulation code shouldn't even remotely have an idea what host 
it's running on. Also semantically there are a few issues with this approach

   1) QEMU is usually running with user privileges, so it doesn't have 
access to the file above
   2) QEMU's channel to hardware devices is via normal kernel API. For 
physical devices that's VFIO. No side channels please.
   3) Ownership of the question whether a PE is in error mode is 
responsibility of the PHB. In the emulated case, the PHB would have to 
set itself into a mode where it behaves as if it's blocked.


Alex

Re: [Qemu-devel] [PATCH v1 1/3] sPAPR: Implement PCI error injection RTAS calls

[Benjamin Herrenschmidt]

On Mon, 2014-06-23 at 18:18 +0200, Alexander Graf wrote:
> Device emulation code shouldn't even remotely have an idea what host 
> it's running on. Also semantically there are a few issues with this approach
> 
>    1) QEMU is usually running with user privileges, so it doesn't have 
> access to the file above

Right, this needs to go via VFIO like the rest of the EEH stuff

>    2) QEMU's channel to hardware devices is via normal kernel API. For 
> physical devices that's VFIO. No side channels please.

Indeed. If the user gets access to that file, suddenly qemu can
"manufacture" a bad string and error inject in other devices it doesn't
own which isn't great.

Gavin, this needs to go via the same path as normal EEH and be limited
to injecting errors that are completely bounded to the PE.

I don't think this is very high priority. We should first write a good
host side error injection tool and sort out the reporting of the EEH log
from host to guest.

>    3) Ownership of the question whether a PE is in error mode is 
> responsibility of the PHB. In the emulated case, the PHB would have to 
> set itself into a mode where it behaves as if it's blocked.

We don't have to support error injection for emulated since we don't
support (yet) the rest oF EEH for them. We could one day but it's
really not urgent.

Cheers,
Ben.

Re: [Qemu-devel] [PATCH v1 1/3] sPAPR: Implement PCI error injection RTAS calls

[Alexander Graf]

> Am 23.06.2014 um 23:03 schrieb Benjamin Herrenschmidt <benh@kernel.crashing.org>:
> 
>> On Mon, 2014-06-23 at 18:18 +0200, Alexander Graf wrote:
>> Device emulation code shouldn't even remotely have an idea what host 
>> it's running on. Also semantically there are a few issues with this approach
>> 
>>   1) QEMU is usually running with user privileges, so it doesn't have 
>> access to the file above
> 
> Right, this needs to go via VFIO like the rest of the EEH stuff
> 
>>   2) QEMU's channel to hardware devices is via normal kernel API. For 
>> physical devices that's VFIO. No side channels please.
> 
> Indeed. If the user gets access to that file, suddenly qemu can
> "manufacture" a bad string and error inject in other devices it doesn't
> own which isn't great.
> 
> Gavin, this needs to go via the same path as normal EEH and be limited
> to injecting errors that are completely bounded to the PE.
> 
> I don't think this is very high priority. We should first write a good
> host side error injection tool and sort out the reporting of the EEH log
> from host to guest.
> 
>>   3) Ownership of the question whether a PE is in error mode is 
>> responsibility of the PHB. In the emulated case, the PHB would have to 
>> set itself into a mode where it behaves as if it's blocked.
> 
> We don't have to support error injection for emulated since we don't
> support (yet) the rest oF EEH for them. We could one day but it's
> really not urgent.

I agree, but the layers are the same ;)

Alex

> 
> Cheers,
> Ben.
> 
> 

Re: [Qemu-devel] [PATCH v1 1/3] sPAPR: Implement PCI error injection RTAS calls

[Gavin Shan]

On Mon, Jun 23, 2014 at 11:13:45PM +0200, Alexander Graf wrote:
>
>> Am 23.06.2014 um 23:03 schrieb Benjamin Herrenschmidt <benh@kernel.crashing.org>:
>> 
>>> On Mon, 2014-06-23 at 18:18 +0200, Alexander Graf wrote:
>>> Device emulation code shouldn't even remotely have an idea what host 
>>> it's running on. Also semantically there are a few issues with this approach
>>> 
>>>   1) QEMU is usually running with user privileges, so it doesn't have 
>>> access to the file above
>> 
>> Right, this needs to go via VFIO like the rest of the EEH stuff
>> 
>>>   2) QEMU's channel to hardware devices is via normal kernel API. For 
>>> physical devices that's VFIO. No side channels please.
>> 
>> Indeed. If the user gets access to that file, suddenly qemu can
>> "manufacture" a bad string and error inject in other devices it doesn't
>> own which isn't great.
>> 
>> Gavin, this needs to go via the same path as normal EEH and be limited
>> to injecting errors that are completely bounded to the PE.
>> 
>> I don't think this is very high priority. We should first write a good
>> host side error injection tool and sort out the reporting of the EEH log
>> from host to guest.
>> 
>>>   3) Ownership of the question whether a PE is in error mode is 
>>> responsibility of the PHB. In the emulated case, the PHB would have to 
>>> set itself into a mode where it behaves as if it's blocked.
>> 
>> We don't have to support error injection for emulated since we don't
>> support (yet) the rest oF EEH for them. We could one day but it's
>> really not urgent.
>
>I agree, but the layers are the same ;)
>

Thanks, Ben and Alex. Yes, it's fair enough for VFIO (ioctl cmd) to
routing the PCI error injection.

Thanks,
Gavin

>Alex
>
>> 
>> Cheers,
>> Ben.
>> 

Re: [Qemu-devel] [PATCH v1 1/3] sPAPR: Implement PCI error injection RTAS calls

[Gavin Shan]

On Mon, Jun 23, 2014 at 06:18:37PM +0200, Alexander Graf wrote:
>
>On 23.06.14 04:22, Gavin Shan wrote:
>>The patch implements PCI error injection RTAS calls for sPAPR
>>platform, which are defined PAPR spec as follows. Those RTAS
>>calls are expected to be invoked in strict sequence of
>>"ibm,open-errinjct", "ibm,errinjct", "ibm,close-errinjct".
>>
>>- "ibm,open-errinjct": Apply for token to do error injection
>>- "ibm,close-errinjct": Free the token assigned previously
>>- "ibm,errinjct": Do error injection
>>
>>The patch defines constants used by error injection RTAS calls
>>and adds callback sPAPRPHBClass::format_errinjct_cmd, which is
>>going to be used like this way:
>>
>>- Error injection RTAS calls are received in spapr_rtas.c. If that's
>>   "ibm,open-errinjct" or "ibm,close-errinjct", we handle it there
>>   with the help of static counter "sPAPREnvironment::errinjct_token_cnt".
>>   If the  RTAS call is "ibm,errinjct", sanity check is done there and
>>   just returns failure if the error type isn't PCI related. Otherwise,
>>   the input parameters are figured out and route the request to
>>   sPAPRPHBClass::format_errinjct_cmd.
>>- sPAPRPHBClass::format_errinjct_cmd translates the address (BUID, PE
>>   number) to IOMMU group ID and then return the formated string back to
>>   spapr_rtas.c where the string is writen to firmware sysfs file
>>   "/sys/firmware/opal/errinjct" to do error injection.
>>
>>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>---
>>  hw/ppc/spapr_rtas.c         | 198 ++++++++++++++++++++++++++++++++++++++++++++
>>  include/hw/pci-host/spapr.h |  11 +++
>>  include/hw/ppc/spapr.h      |  35 ++++++++
>>  3 files changed, 244 insertions(+)
>>
>>diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>>index a7233e4..94fb866 100644
>>--- a/hw/ppc/spapr_rtas.c
>>+++ b/hw/ppc/spapr_rtas.c
>>@@ -32,6 +32,7 @@
>>  #include "hw/ppc/spapr.h"
>>  #include "hw/ppc/spapr_vio.h"
>>+#include "hw/pci-host/spapr.h"
>>  #include <libfdt.h>
>>@@ -268,6 +269,196 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
>>      rtas_st(rets, 0, ret);
>>  }
>>+static sPAPRErrInjct *rtas_find_errinjct_token(sPAPREnvironment *spapr,
>>+                                               uint32_t token_num)
>>+{
>>+    sPAPRErrInjct *open_token;
>>+
>>+    QLIST_FOREACH(open_token, &spapr->errinjct_open_tokens, list) {
>>+        if (open_token->token == token_num) {
>>+            return open_token;
>>+        }
>>+    }
>>+
>>+    return NULL;
>>+}
>>+
>>+static void rtas_ibm_open_errinjct(PowerPCCPU *cpu,
>>+                                   sPAPREnvironment *spapr,
>>+                                   uint32_t token, uint32_t nargs,
>>+                                   target_ulong args, uint32_t nret,
>>+                                   target_ulong rets)
>>+{
>>+    sPAPRErrInjct *open_token;
>>+    int32_t ret = RTAS_OUT_HW_ERROR;
>>+
>>+    /* Sanity check on number of arguments */
>>+    if ((nargs != 0) || (nret != 2)) {
>>+        goto out;
>>+    }
>>+
>>+    /* Allocate token */
>>+    open_token = g_malloc(sizeof(*open_token));
>
>With this hypercall the guest can OOM us by simply allocating useless
>tokens.
>

Yeah, that's possible. I'll have the limitation that each QEMU process
only can have one token. Then we don't need allocate memory blocks to
maintain the tokens.

Thanks,
Gavin

>>+    if (!open_token) {
>>+        goto out;
>>+    }
>>+    open_token->token = spapr->errinjct_open_token_cnt++;
>>+    QLIST_INSERT_HEAD(&spapr->errinjct_open_tokens, open_token, list);
>>+
>>+    /* Success */
>>+    rtas_st(rets, 0, open_token->token);
>>+    ret = RTAS_OUT_SUCCESS;
>>+out:
>>+    rtas_st(rets, 1, ret);
>>+}
>>+
>>+static char *rtas_do_errinjct_ioa(sPAPREnvironment *spapr,
>>+                                sPAPRErrInjctIOA *ioa)
>>+{
>>+    sPAPRPHBState *sphb;
>>+    sPAPRPHBClass *info;
>>+
>>+    /* Sanity check on argument */
>>+    if (!spapr || !ioa) {
>>+        return NULL;
>>+    }
>>+
>>+    /* Invalid option ? */
>>+    if (ioa->func > RTAS_ERRINJCT_IOA_DMA_WR_MEM_TARGET) {
>>+        return NULL;
>>+    }
>>+
>>+    /* Find PHB */
>>+    sphb = spapr_find_phb(spapr, ioa->buid);
>>+    if (!sphb) {
>>+        return NULL;
>>+    }
>>+
>>+    /* PHB doesn't support error injection ? */
>>+    info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
>>+    if (!info->format_errinjct_cmd) {
>>+        return NULL;
>>+    }
>>+
>>+    return info->format_errinjct_cmd(sphb, ioa);
>>+}
>>+
>>+static void rtas_ibm_errinjct(PowerPCCPU *cpu,
>>+                              sPAPREnvironment *spapr,
>>+                              uint32_t token, uint32_t nargs,
>>+                              target_ulong args, uint32_t nret,
>>+                              target_ulong rets)
>>+{
>>+    sPAPRErrInjctIOA ioa;
>>+    sPAPRErrInjct *open_token;
>>+    uint32_t token_num, ei_token;
>>+    target_ulong param_buf;
>>+    char *pbuf;
>>+    int fd;
>>+    int32_t ret = RTAS_OUT_PARAM_ERROR;
>>+
>>+    /* Sanity check on number of arguments */
>>+    if ((nargs != 3) || (nret != 1)) {
>>+        goto out;
>>+    }
>>+
>>+    /* Check if we have opened token */
>>+    token_num = rtas_ld(args, 1);
>>+    open_token = rtas_find_errinjct_token(spapr, token_num);
>>+    if (!open_token) {
>>+        goto out;
>>+    }
>>+
>>+    /* Argument buffer should be aligned with 1KB */
>>+    param_buf = rtas_ld(args, 2);
>>+    if (param_buf & 0x3ff) {
>>+        goto out;
>>+    }
>>+
>>+    /* We only support IOA error for now */
>>+    ei_token = rtas_ld(args, 0);
>>+    switch (ei_token) {
>>+    case RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR:
>>+        ioa.ei_token = RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR;
>>+        ioa.addr = rtas_ld(param_buf, 0);
>>+        ioa.mask = rtas_ld(param_buf, 1);
>>+        ioa.cfg_addr = rtas_ld(param_buf, 2);
>>+        ioa.buid = ((uint64_t)rtas_ld(param_buf, 3) << 32) | rtas_ld(param_buf, 4);
>>+        ioa.func = rtas_ld(param_buf, 5);
>>+
>>+        break;
>>+    case RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR64:
>>+        ioa.ei_token = RTAS_ERRINJCT_TYPE_IOA_BUS_ERROR64;
>>+        ioa.addr = ((uint64_t)rtas_ld(param_buf, 0) << 32) | rtas_ld(param_buf, 1);
>>+        ioa.mask = ((uint64_t)rtas_ld(param_buf, 2) << 32) | rtas_ld(param_buf, 3);
>>+        ioa.cfg_addr = rtas_ld(param_buf, 4);
>>+        ioa.buid = ((uint64_t)rtas_ld(param_buf, 5) << 32) | rtas_ld(param_buf, 6);
>>+        ioa.func = rtas_ld(param_buf, 7);
>>+
>>+        break;
>>+    default:
>>+        goto out;
>>+    }
>>+
>>+    /* We have nothing written to sysfs */
>>+    pbuf = rtas_do_errinjct_ioa(spapr, &ioa);
>>+    if (!pbuf) {
>>+        goto out;
>>+    }
>>+
>>+    /* Open firmware sysfs file */
>>+    fd = qemu_open("/sys/firmware/opal/errinjct", O_WRONLY);
>
>Device emulation code shouldn't even remotely have an idea what host
>it's running on. Also semantically there are a few issues with this
>approach
>
>  1) QEMU is usually running with user privileges, so it doesn't have
>access to the file above
>  2) QEMU's channel to hardware devices is via normal kernel API. For
>physical devices that's VFIO. No side channels please.
>  3) Ownership of the question whether a PE is in error mode is
>responsibility of the PHB. In the emulated case, the PHB would have
>to set itself into a mode where it behaves as if it's blocked.
>
>
>Alex
>