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 <gws...@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
>
