| Message ID | 1438918413-8247-3-git-send-email-gwshan@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 08/07/2015 01:33 PM, Gavin Shan wrote: > The patch supports RTAS calls "ibm,{open,close}-errinjct" to > manupliate the token, which is passed to RTAS call "ibm,errinjct" > to indicate the valid context for error injection. Each VM is > permitted to have only one token at once and we simply have one > random number for that. > > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> > --- > hw/ppc/spapr.c | 5 ++++ > hw/ppc/spapr_rtas.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++ > include/hw/ppc/spapr.h | 10 +++++++- > 3 files changed, 80 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index dfd808f..1ebd0b2 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1225,6 +1225,11 @@ static const VMStateDescription vmstate_spapr = { > VMSTATE_UINT64_TEST(rtc_offset, sPAPRMachineState, version_before_3), > > VMSTATE_PPC_TIMEBASE_V(tb, sPAPRMachineState, 2), > + > + /* Error injection token */ > + VMSTATE_BOOL(is_errinjct_opened, sPAPRMachineState), > + VMSTATE_UINT32(errinjct_next_token, sPAPRMachineState), > + > VMSTATE_END_OF_LIST() > }, > }; > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c > index e99e25f..f1b92ca 100644 > --- a/hw/ppc/spapr_rtas.c > +++ b/hw/ppc/spapr_rtas.c > @@ -604,6 +604,68 @@ out: > rtas_st(rets, 0, rc); > } > > +static void rtas_ibm_open_errinjct(PowerPCCPU *cpu, > + sPAPRMachineState *spapr, > + uint32_t token, uint32_t nargs, > + target_ulong args, uint32_t nret, > + target_ulong rets) > +{ > + int32_t ret; > + > + /* Sanity check on number of arguments */ > + if ((nargs != 0) || (nret != 2)) { > + ret = RTAS_OUT_PARAM_ERROR; > + goto out; > + } > + > + /* Check if we already had token */ > + if (spapr->is_errinjct_opened) { > + ret = RTAS_OUT_TOKEN_OPENED; > + goto out; > + } > + > + /* Grab the token */ > + spapr->is_errinjct_opened = true; > + rtas_st(rets, 0, spapr->errinjct_next_token++); If you did "++spapr->errinjct_next_token" (or just moved to a separate line before rtas_st())... > + ret = RTAS_OUT_SUCCESS; > +out: > + rtas_st(rets, 1, ret); > +} > + > +static void rtas_ibm_close_errinjct(PowerPCCPU *cpu, > + sPAPRMachineState *spapr, > + uint32_t token, uint32_t nargs, > + target_ulong args, uint32_t nret, > + target_ulong rets) > +{ > + uint32_t open_token; > + int32_t ret; > + > + /* Sanity check on number of arguments */ > + if ((nargs != 1) || (nret != 1)) { > + ret = RTAS_OUT_PARAM_ERROR; > + goto out; > + } > + > + /* Check if we had opened token */ > + if (!spapr->is_errinjct_opened) { > + ret = RTAS_OUT_CLOSE_ERROR; > + goto out; > + } > + > + /* Match with the passed token */ > + open_token = rtas_ld(args, 0); > + if (spapr->errinjct_next_token != (open_token + 1)) { ... you would not need "+ 1" here. > + ret = RTAS_OUT_CLOSE_ERROR; > + goto out; > + } > + > + spapr->is_errinjct_opened = false; > + ret = RTAS_OUT_SUCCESS; > +out: > + rtas_st(rets, 0, ret); > +} > + > static struct rtas_call { > const char *name; > spapr_rtas_fn fn; > @@ -754,6 +816,10 @@ static void core_rtas_register_types(void) > rtas_get_sensor_state); > spapr_rtas_register(RTAS_IBM_CONFIGURE_CONNECTOR, "ibm,configure-connector", > rtas_ibm_configure_connector); > + spapr_rtas_register(RTAS_IBM_OPEN_ERRINJCT, "ibm,open-errinjct", > + rtas_ibm_open_errinjct); > + spapr_rtas_register(RTAS_IBM_CLOSE_ERRINJCT, "ibm,close-errinjct", > + rtas_ibm_close_errinjct); > } > > type_init(core_rtas_register_types) > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index b6cb0d0..8ecf127 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -73,6 +73,10 @@ struct sPAPRMachineState { > int htab_fd; > bool htab_fd_stale; > > + /* Error injection token */ > + bool is_errinjct_opened; > + uint32_t errinjct_next_token; > + > /* RTAS state */ > QTAILQ_HEAD(, sPAPRConfigureConnectorState) ccs_list; > > @@ -412,6 +416,8 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi); > #define RTAS_OUT_BUSY -2 > #define RTAS_OUT_PARAM_ERROR -3 > #define RTAS_OUT_NOT_SUPPORTED -3 > +#define RTAS_OUT_TOKEN_OPENED -4 > +#define RTAS_OUT_CLOSE_ERROR -4 > #define RTAS_OUT_NOT_AUTHORIZED -9002 > > /* RTAS tokens */ > @@ -455,8 +461,10 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi); > #define RTAS_IBM_SET_SLOT_RESET (RTAS_TOKEN_BASE + 0x23) > #define RTAS_IBM_CONFIGURE_PE (RTAS_TOKEN_BASE + 0x24) > #define RTAS_IBM_SLOT_ERROR_DETAIL (RTAS_TOKEN_BASE + 0x25) > +#define RTAS_IBM_OPEN_ERRINJCT (RTAS_TOKEN_BASE + 0x26) > +#define RTAS_IBM_CLOSE_ERRINJCT (RTAS_TOKEN_BASE + 0x27) > > -#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x26) > +#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x28) > > /* RTAS ibm,get-system-parameter token values */ > #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20 >
On Sat, Aug 08, 2015 at 02:04:17AM +1000, Alexey Kardashevskiy wrote: >On 08/07/2015 01:33 PM, Gavin Shan wrote: >>The patch supports RTAS calls "ibm,{open,close}-errinjct" to >>manupliate the token, which is passed to RTAS call "ibm,errinjct" >>to indicate the valid context for error injection. Each VM is >>permitted to have only one token at once and we simply have one >>random number for that. >> >>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >>--- >> hw/ppc/spapr.c | 5 ++++ >> hw/ppc/spapr_rtas.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> include/hw/ppc/spapr.h | 10 +++++++- >> 3 files changed, 80 insertions(+), 1 deletion(-) >> >>diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>index dfd808f..1ebd0b2 100644 >>--- a/hw/ppc/spapr.c >>+++ b/hw/ppc/spapr.c >>@@ -1225,6 +1225,11 @@ static const VMStateDescription vmstate_spapr = { >> VMSTATE_UINT64_TEST(rtc_offset, sPAPRMachineState, version_before_3), >> >> VMSTATE_PPC_TIMEBASE_V(tb, sPAPRMachineState, 2), >>+ >>+ /* Error injection token */ >>+ VMSTATE_BOOL(is_errinjct_opened, sPAPRMachineState), >>+ VMSTATE_UINT32(errinjct_next_token, sPAPRMachineState), >>+ >> VMSTATE_END_OF_LIST() >> }, >> }; >>diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c >>index e99e25f..f1b92ca 100644 >>--- a/hw/ppc/spapr_rtas.c >>+++ b/hw/ppc/spapr_rtas.c >>@@ -604,6 +604,68 @@ out: >> rtas_st(rets, 0, rc); >> } >> >>+static void rtas_ibm_open_errinjct(PowerPCCPU *cpu, >>+ sPAPRMachineState *spapr, >>+ uint32_t token, uint32_t nargs, >>+ target_ulong args, uint32_t nret, >>+ target_ulong rets) >>+{ >>+ int32_t ret; >>+ >>+ /* Sanity check on number of arguments */ >>+ if ((nargs != 0) || (nret != 2)) { >>+ ret = RTAS_OUT_PARAM_ERROR; >>+ goto out; >>+ } >>+ >>+ /* Check if we already had token */ >>+ if (spapr->is_errinjct_opened) { >>+ ret = RTAS_OUT_TOKEN_OPENED; >>+ goto out; >>+ } >>+ >>+ /* Grab the token */ >>+ spapr->is_errinjct_opened = true; >>+ rtas_st(rets, 0, spapr->errinjct_next_token++); > > >If you did "++spapr->errinjct_next_token" (or just moved to a separate line >before rtas_st())... > > > >>+ ret = RTAS_OUT_SUCCESS; >>+out: >>+ rtas_st(rets, 1, ret); >>+} >>+ >>+static void rtas_ibm_close_errinjct(PowerPCCPU *cpu, >>+ sPAPRMachineState *spapr, >>+ uint32_t token, uint32_t nargs, >>+ target_ulong args, uint32_t nret, >>+ target_ulong rets) >>+{ >>+ uint32_t open_token; >>+ int32_t ret; >>+ >>+ /* Sanity check on number of arguments */ >>+ if ((nargs != 1) || (nret != 1)) { >>+ ret = RTAS_OUT_PARAM_ERROR; >>+ goto out; >>+ } >>+ >>+ /* Check if we had opened token */ >>+ if (!spapr->is_errinjct_opened) { >>+ ret = RTAS_OUT_CLOSE_ERROR; >>+ goto out; >>+ } >>+ >>+ /* Match with the passed token */ >>+ open_token = rtas_ld(args, 0); >>+ if (spapr->errinjct_next_token != (open_token + 1)) { > > >... you would not need "+ 1" here. > Ok. Then it should be renamed to "errinject_current_token", or just "errinjct_token". I prefer later one. In that case, the token will start from 1, not 0. Thanks, Gavin > >>+ ret = RTAS_OUT_CLOSE_ERROR; >>+ goto out; >>+ } >>+ >>+ spapr->is_errinjct_opened = false; >>+ ret = RTAS_OUT_SUCCESS; >>+out: >>+ rtas_st(rets, 0, ret); >>+} >>+ >> static struct rtas_call { >> const char *name; >> spapr_rtas_fn fn; >>@@ -754,6 +816,10 @@ static void core_rtas_register_types(void) >> rtas_get_sensor_state); >> spapr_rtas_register(RTAS_IBM_CONFIGURE_CONNECTOR, "ibm,configure-connector", >> rtas_ibm_configure_connector); >>+ spapr_rtas_register(RTAS_IBM_OPEN_ERRINJCT, "ibm,open-errinjct", >>+ rtas_ibm_open_errinjct); >>+ spapr_rtas_register(RTAS_IBM_CLOSE_ERRINJCT, "ibm,close-errinjct", >>+ rtas_ibm_close_errinjct); >> } >> >> type_init(core_rtas_register_types) >>diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h >>index b6cb0d0..8ecf127 100644 >>--- a/include/hw/ppc/spapr.h >>+++ b/include/hw/ppc/spapr.h >>@@ -73,6 +73,10 @@ struct sPAPRMachineState { >> int htab_fd; >> bool htab_fd_stale; >> >>+ /* Error injection token */ >>+ bool is_errinjct_opened; >>+ uint32_t errinjct_next_token; >>+ >> /* RTAS state */ >> QTAILQ_HEAD(, sPAPRConfigureConnectorState) ccs_list; >> >>@@ -412,6 +416,8 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi); >> #define RTAS_OUT_BUSY -2 >> #define RTAS_OUT_PARAM_ERROR -3 >> #define RTAS_OUT_NOT_SUPPORTED -3 >>+#define RTAS_OUT_TOKEN_OPENED -4 >>+#define RTAS_OUT_CLOSE_ERROR -4 >> #define RTAS_OUT_NOT_AUTHORIZED -9002 >> >> /* RTAS tokens */ >>@@ -455,8 +461,10 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi); >> #define RTAS_IBM_SET_SLOT_RESET (RTAS_TOKEN_BASE + 0x23) >> #define RTAS_IBM_CONFIGURE_PE (RTAS_TOKEN_BASE + 0x24) >> #define RTAS_IBM_SLOT_ERROR_DETAIL (RTAS_TOKEN_BASE + 0x25) >>+#define RTAS_IBM_OPEN_ERRINJCT (RTAS_TOKEN_BASE + 0x26) >>+#define RTAS_IBM_CLOSE_ERRINJCT (RTAS_TOKEN_BASE + 0x27) >> >>-#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x26) >>+#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x28) >> >> /* RTAS ibm,get-system-parameter token values */ >> #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20 >> > > >-- >Alexey >
On Fri, Aug 07, 2015 at 01:33:32PM +1000, Gavin Shan wrote: > The patch supports RTAS calls "ibm,{open,close}-errinjct" to > manupliate the token, which is passed to RTAS call "ibm,errinjct" > to indicate the valid context for error injection. Each VM is > permitted to have only one token at once and we simply have one > random number for that. > > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> > --- > hw/ppc/spapr.c | 5 ++++ > hw/ppc/spapr_rtas.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++ > include/hw/ppc/spapr.h | 10 +++++++- > 3 files changed, 80 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index dfd808f..1ebd0b2 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1225,6 +1225,11 @@ static const VMStateDescription vmstate_spapr = { > VMSTATE_UINT64_TEST(rtc_offset, sPAPRMachineState, version_before_3), > > VMSTATE_PPC_TIMEBASE_V(tb, sPAPRMachineState, 2), > + > + /* Error injection token */ > + VMSTATE_BOOL(is_errinjct_opened, sPAPRMachineState), > + VMSTATE_UINT32(errinjct_next_token, sPAPRMachineState), Because you're adding fields to the vmstate you'll need to define a new version number and make these fields only considered in the new version. Technically you only need to transfer whether the token is open, and *if* it is open the current token value. Not sure if it's worth restricting to that though.
On Mon, Aug 10, 2015 at 10:24:56PM +1000, David Gibson wrote: >On Fri, Aug 07, 2015 at 01:33:32PM +1000, Gavin Shan wrote: >> The patch supports RTAS calls "ibm,{open,close}-errinjct" to >> manupliate the token, which is passed to RTAS call "ibm,errinjct" >> to indicate the valid context for error injection. Each VM is >> permitted to have only one token at once and we simply have one >> random number for that. >> >> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >> --- >> hw/ppc/spapr.c | 5 ++++ >> hw/ppc/spapr_rtas.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> include/hw/ppc/spapr.h | 10 +++++++- >> 3 files changed, 80 insertions(+), 1 deletion(-) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index dfd808f..1ebd0b2 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -1225,6 +1225,11 @@ static const VMStateDescription vmstate_spapr = { >> VMSTATE_UINT64_TEST(rtc_offset, sPAPRMachineState, version_before_3), >> >> VMSTATE_PPC_TIMEBASE_V(tb, sPAPRMachineState, 2), >> + >> + /* Error injection token */ >> + VMSTATE_BOOL(is_errinjct_opened, sPAPRMachineState), >> + VMSTATE_UINT32(errinjct_next_token, sPAPRMachineState), > >Because you're adding fields to the vmstate you'll need to define a >new version number and make these fields only considered in the new >version. > Agree, do we have an example for me to refer to? >Technically you only need to transfer whether the token is open, and >*if* it is open the current token value. Not sure if it's worth >restricting to that though. > It would be fine to transfer the token even it's closed, I think. Thanks, Gavin
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index dfd808f..1ebd0b2 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1225,6 +1225,11 @@ static const VMStateDescription vmstate_spapr = { VMSTATE_UINT64_TEST(rtc_offset, sPAPRMachineState, version_before_3), VMSTATE_PPC_TIMEBASE_V(tb, sPAPRMachineState, 2), + + /* Error injection token */ + VMSTATE_BOOL(is_errinjct_opened, sPAPRMachineState), + VMSTATE_UINT32(errinjct_next_token, sPAPRMachineState), + VMSTATE_END_OF_LIST() }, }; diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c index e99e25f..f1b92ca 100644 --- a/hw/ppc/spapr_rtas.c +++ b/hw/ppc/spapr_rtas.c @@ -604,6 +604,68 @@ out: rtas_st(rets, 0, rc); } +static void rtas_ibm_open_errinjct(PowerPCCPU *cpu, + sPAPRMachineState *spapr, + uint32_t token, uint32_t nargs, + target_ulong args, uint32_t nret, + target_ulong rets) +{ + int32_t ret; + + /* Sanity check on number of arguments */ + if ((nargs != 0) || (nret != 2)) { + ret = RTAS_OUT_PARAM_ERROR; + goto out; + } + + /* Check if we already had token */ + if (spapr->is_errinjct_opened) { + ret = RTAS_OUT_TOKEN_OPENED; + goto out; + } + + /* Grab the token */ + spapr->is_errinjct_opened = true; + rtas_st(rets, 0, spapr->errinjct_next_token++); + ret = RTAS_OUT_SUCCESS; +out: + rtas_st(rets, 1, ret); +} + +static void rtas_ibm_close_errinjct(PowerPCCPU *cpu, + sPAPRMachineState *spapr, + uint32_t token, uint32_t nargs, + target_ulong args, uint32_t nret, + target_ulong rets) +{ + uint32_t open_token; + int32_t ret; + + /* Sanity check on number of arguments */ + if ((nargs != 1) || (nret != 1)) { + ret = RTAS_OUT_PARAM_ERROR; + goto out; + } + + /* Check if we had opened token */ + if (!spapr->is_errinjct_opened) { + ret = RTAS_OUT_CLOSE_ERROR; + goto out; + } + + /* Match with the passed token */ + open_token = rtas_ld(args, 0); + if (spapr->errinjct_next_token != (open_token + 1)) { + ret = RTAS_OUT_CLOSE_ERROR; + goto out; + } + + spapr->is_errinjct_opened = false; + ret = RTAS_OUT_SUCCESS; +out: + rtas_st(rets, 0, ret); +} + static struct rtas_call { const char *name; spapr_rtas_fn fn; @@ -754,6 +816,10 @@ static void core_rtas_register_types(void) rtas_get_sensor_state); spapr_rtas_register(RTAS_IBM_CONFIGURE_CONNECTOR, "ibm,configure-connector", rtas_ibm_configure_connector); + spapr_rtas_register(RTAS_IBM_OPEN_ERRINJCT, "ibm,open-errinjct", + rtas_ibm_open_errinjct); + spapr_rtas_register(RTAS_IBM_CLOSE_ERRINJCT, "ibm,close-errinjct", + rtas_ibm_close_errinjct); } type_init(core_rtas_register_types) diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index b6cb0d0..8ecf127 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -73,6 +73,10 @@ struct sPAPRMachineState { int htab_fd; bool htab_fd_stale; + /* Error injection token */ + bool is_errinjct_opened; + uint32_t errinjct_next_token; + /* RTAS state */ QTAILQ_HEAD(, sPAPRConfigureConnectorState) ccs_list; @@ -412,6 +416,8 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi); #define RTAS_OUT_BUSY -2 #define RTAS_OUT_PARAM_ERROR -3 #define RTAS_OUT_NOT_SUPPORTED -3 +#define RTAS_OUT_TOKEN_OPENED -4 +#define RTAS_OUT_CLOSE_ERROR -4 #define RTAS_OUT_NOT_AUTHORIZED -9002 /* RTAS tokens */ @@ -455,8 +461,10 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi); #define RTAS_IBM_SET_SLOT_RESET (RTAS_TOKEN_BASE + 0x23) #define RTAS_IBM_CONFIGURE_PE (RTAS_TOKEN_BASE + 0x24) #define RTAS_IBM_SLOT_ERROR_DETAIL (RTAS_TOKEN_BASE + 0x25) +#define RTAS_IBM_OPEN_ERRINJCT (RTAS_TOKEN_BASE + 0x26) +#define RTAS_IBM_CLOSE_ERRINJCT (RTAS_TOKEN_BASE + 0x27) -#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x26) +#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x28) /* RTAS ibm,get-system-parameter token values */ #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20
The patch supports RTAS calls "ibm,{open,close}-errinjct" to manupliate the token, which is passed to RTAS call "ibm,errinjct" to indicate the valid context for error injection. Each VM is permitted to have only one token at once and we simply have one random number for that. Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com> --- hw/ppc/spapr.c | 5 ++++ hw/ppc/spapr_rtas.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 10 +++++++- 3 files changed, 80 insertions(+), 1 deletion(-)