From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50109) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zhvfg-0007NB-DS for qemu-devel@nongnu.org; Fri, 02 Oct 2015 04:26:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zhvfc-0006ka-Ak for qemu-devel@nongnu.org; Fri, 02 Oct 2015 04:26:28 -0400 Received: from mail-pa0-f48.google.com ([209.85.220.48]:35081) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zhvfc-0006jb-6I for qemu-devel@nongnu.org; Fri, 02 Oct 2015 04:26:24 -0400 Received: by pacfv12 with SMTP id fv12so103026067pac.2 for ; Fri, 02 Oct 2015 01:26:23 -0700 (PDT) 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> From: Alexey Kardashevskiy Message-ID: <560E3FA8.4090300@ozlabs.ru> Date: Fri, 2 Oct 2015 18:26:16 +1000 MIME-Version: 1.0 In-Reply-To: <55D4AB9E.8040603@redhat.com> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 3/4] sPAPR: Support RTAS call ibm, {open, close}-errinjct List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth , David Gibson , Gavin Shan Cc: peter.maydell@linaro.org, qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 08/20/2015 02:15 AM, 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. > >>>>> 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! I still prefer the clarity and therefore more braces which make no harm whatsoever. -- Alexey