From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56824) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZSDXt-000841-Ao for qemu-devel@nongnu.org; Wed, 19 Aug 2015 20:17:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZSDXo-0005wR-HF for qemu-devel@nongnu.org; Wed, 19 Aug 2015 20:17:29 -0400 Received: from e23smtp09.au.ibm.com ([202.81.31.142]:48079) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZSDXn-0005vQ-VN for qemu-devel@nongnu.org; Wed, 19 Aug 2015 20:17:24 -0400 Received: from /spool/local by e23smtp09.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 20 Aug 2015 10:17:20 +1000 Date: Thu, 20 Aug 2015 10:16:16 +1000 From: Gavin Shan Message-ID: <20150820001616.GA13044@gwshan> References: <1439862430-14996-1-git-send-email-gwshan@linux.vnet.ibm.com> <1439862430-14996-4-git-send-email-gwshan@linux.vnet.ibm.com> <55D36C1D.8040608@redhat.com> <20150818235200.GB8064@gwshan> <20150819011535.GK3157@voom> <55D4AB9E.8040603@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55D4AB9E.8040603@redhat.com> Subject: Re: [Qemu-devel] [PATCH v5 3/4] 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: Thomas Huth Cc: peter.maydell@linaro.org, aik@ozlabs.ru, qemu-devel@nongnu.org, Gavin Shan , qemu-ppc@nongnu.org, David Gibson On Wed, Aug 19, 2015 at 09:15:26AM -0700, Thomas Huth wrote: >On 18/08/15 18:15, David Gibson wrote: >> On Wed, Aug 19, 2015 at 09:52:00AM +1000, Gavin Shan wrote: >>> On Tue, Aug 18, 2015 at 10:32:13AM -0700, Thomas Huth wrote: >>>> 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? >>>> >>> >>> Yes, it's what Alexey suggested. > >Then please update the commit message accordingly. > Yes, I'll update changelog accordingly. >>>>> 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? >>>> >>> >>> hehe~, nope. I'll drop those unnecessary paranthesitis :-) >> >> I'd prefer you didn't. Unlike Thomas, I also don't remember C order >> of ops that well and would prefer the clarity. > >You can always look it up if you're unsure, e.g.: > >http://en.cppreference.com/w/c/language/operator_precedence > >And once you've learnt it, the additional paranthesis just look >cumbersome. So please remove them! > Ok. I'll check the code and remove unnecessary paranthesis in next revision. Thanks, Gavin