Re: [Qemu-ppc] [PATCH v6 7/8] sPAPR: Support RTAS call ibm, {open, close}-errinjct

[Thomas Huth]

On 24/08/15 14:03, Gavin Shan wrote:
> This 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 sequential number for that.
> It's notable that the least bit of the token is reserved to indicate
> if the token has been opened, meaning the valid token should be always
> odd.
> 
> Signed-off-by: Gavin Shan <address@hidden>
> ---
>  hw/ppc/spapr.c         |  6 ++++-
>  hw/ppc/spapr_rtas.c    | 60 
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h |  9 +++++++-
>  3 files changed, 73 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 06d000d..591a1a7 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1191,7 +1191,7 @@ static bool version_before_3(void *opaque, int 
> version_id)
>  
>  static const VMStateDescription vmstate_spapr = {
>      .name = "spapr",
> -    .version_id = 3,
> +    .version_id = 4,
>      .minimum_version_id = 1,
>      .post_load = spapr_post_load,
>      .fields = (VMStateField[]) {
> @@ -1202,6 +1202,10 @@ 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_UINT32_V(errinjct_token, sPAPRMachineState, 4),
> +
>          VMSTATE_END_OF_LIST()
>      },
>  };
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index e99e25f..64924c6 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -604,6 +604,62 @@ 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->errinjct_token & 1) {
> +        ret = RTAS_OUT_TOKEN_OPENED;
> +        goto out;
> +    }
> +
> +    /* Grab the token */
> +    rtas_st(rets, 0, ++spapr->errinjct_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;
> +    }
> +
> +    /* Match with the passed token */
> +    open_token = rtas_ld(args, 0);
> +    if (!(spapr->errinjct_token & 1) ||
> +        spapr->errinjct_token != open_token) {
> +        ret = RTAS_OUT_CLOSE_ERROR;
> +        goto out;
> +    }
> +
> +    spapr->errinjct_token++;
> +    ret = RTAS_OUT_SUCCESS;
> +out:
> +    rtas_st(rets, 0, ret);
> +}

This basically now looks fine to me! I am just wondering what happens
when the guest opens a token, but then resets the system before it
closes it again?
I.e. shouldn't the errinjct_token be set to 0 back again during the
reset handler?

 Thomas