From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49797) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPgzw-0002ct-3p for qemu-devel@nongnu.org; Wed, 12 Aug 2015 21:08:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZPgzt-0007Gs-3I for qemu-devel@nongnu.org; Wed, 12 Aug 2015 21:08:00 -0400 Received: from e23smtp08.au.ibm.com ([202.81.31.141]:49927) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPgzs-0007CB-HW for qemu-devel@nongnu.org; Wed, 12 Aug 2015 21:07:57 -0400 Received: from /spool/local by e23smtp08.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 13 Aug 2015 11:07:52 +1000 Date: Thu, 13 Aug 2015 11:00:17 +1000 From: Gavin Shan Message-ID: <20150813010017.GA6343@gwshan> References: <1438918413-8247-1-git-send-email-gwshan@linux.vnet.ibm.com> <1438918413-8247-3-git-send-email-gwshan@linux.vnet.ibm.com> <20150810122456.GC18650@voom.redhat.com> <20150811005713.GA21087@gwshan> <55C955C0.4070709@ozlabs.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55C955C0.4070709@ozlabs.ru> Subject: Re: [Qemu-devel] [PATCH v3 2/3] sPAPR: Support RTAS call ibm, {open, close}-errinjct Reply-To: Gavin Shan List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Gavin Shan , David Gibson 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 >>>>--- >>>> 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