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 <address@hidden>
>>---
>>  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
>