Re: [Qemu-devel] [PATCH v3 2/3] sPAPR: Support RTAS call ibm, {open, close}-errinjct

[Gavin Shan <gwshan@linux.vnet.ibm.com>]

On Tue, Aug 11, 2015 at 11:54:08AM +1000, Alexey Kardashevskiy wrote:
>On 08/11/2015 10:57 AM, Gavin Shan wrote:
>>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 it is enough to send a token which is "not opened" when it is
>zero. The chunk below demonstrates versions use.
>

Agree. we don't have to track the token is opened or not. It's fine to
have "not opened" state in the target VM. I'll integrate this piece of
code into next revision. Thanks!

>diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>index e046265..e568d41 100644
>--- a/hw/ppc/spapr.c
>+++ b/hw/ppc/spapr.c
>@@ -1213,17 +1213,21 @@ 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[]) {
>         /* used to be @next_irq */
>         VMSTATE_UNUSED_BUFFER(version_before_3, 0, 4),
>
>+
>         /* RTC offset */
>         VMSTATE_UINT64_TEST(rtc_offset, sPAPRMachineState, version_before_3),
>
>         VMSTATE_PPC_TIMEBASE_V(tb, sPAPRMachineState, 2),
>+
>+        /* Error injection token */
>+        VMSTATE_UINT32_V(errinjct_next_token, sPAPRMachineState, 4),
>         VMSTATE_END_OF_LIST()
>     },
> };
>
>
>
>>>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