[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH v5 3/4] sPAPR: Support RTAS call ibm,
From: |
Thomas Huth |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH v5 3/4] sPAPR: Support RTAS call ibm, {open, close}-errinjct |
Date: |
Tue, 18 Aug 2015 10:32:13 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 |
On 17/08/15 18:47, 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.
Looking at the code, you're using a sequence number now instead of a
random number?
> Signed-off-by: Gavin Shan <address@hidden>
> ---
> hw/ppc/spapr.c | 6 ++++-
> hw/ppc/spapr_rtas.c | 66
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/ppc/spapr.h | 10 +++++++-
> 3 files changed, 80 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),
Ok, so you're only saving the errinjct_token here, but not
is_errinjct_opened?
> VMSTATE_END_OF_LIST()
> },
> };
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index e99e25f..8405056 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)) {
Uh, did Alexey infect you with paranthesitis?
> + 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_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;
> + }
... and here you check that another status variable "is_errinjct_opened"
which is not saved in the VMStateDescription and thus this information
will get lost during migration, I think. That looks like a bug to me.
Can you do your code completely without "is_errinjct_opened"? I.e. by
using errinjct_token == 0 for signalling that no injection progress is
currently taking place?
> + /* Match with the passed token */
> + open_token = rtas_ld(args, 0);
> + if (spapr->errinjct_token != open_token) {
> + ret = RTAS_OUT_CLOSE_ERROR;
> + goto out;
> + }
> +
> + spapr->is_errinjct_opened = false;
> + ret = RTAS_OUT_SUCCESS;
> +out:
> + rtas_st(rets, 0, ret);
> +}
Thomas
- [Qemu-ppc] [PATCH v5 0/4] sPAPR: Support EEH Error Injection, Gavin Shan, 2015/08/17
- [Qemu-ppc] [PATCH v5 3/4] sPAPR: Support RTAS call ibm, {open, close}-errinjct, Gavin Shan, 2015/08/17
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v5 3/4] sPAPR: Support RTAS call ibm, {open, close}-errinjct,
Thomas Huth <=
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v5 3/4] sPAPR: Support RTAS call ibm, {open, close}-errinjct, Gavin Shan, 2015/08/18
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v5 3/4] sPAPR: Support RTAS call ibm, {open, close}-errinjct, David Gibson, 2015/08/18
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v5 3/4] sPAPR: Support RTAS call ibm, {open, close}-errinjct, Thomas Huth, 2015/08/19
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v5 3/4] sPAPR: Support RTAS call ibm, {open, close}-errinjct, Gavin Shan, 2015/08/19
[Qemu-ppc] [PATCH v5 1/4] scripts: Include arch/powerpc/include/uapi/asm/eeh.h, Gavin Shan, 2015/08/17
[Qemu-ppc] [PATCH v5 2/4] linux-headers: Add eeh.h, Gavin Shan, 2015/08/17