From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58668) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCTIe-0007yP-U6 for qemu-devel@nongnu.org; Sun, 09 Feb 2014 07:15:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WCTIa-0002Rv-76 for qemu-devel@nongnu.org; Sun, 09 Feb 2014 07:15:52 -0500 Received: from mail-lb0-f173.google.com ([209.85.217.173]:36333) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCTIZ-0002Rr-W9 for qemu-devel@nongnu.org; Sun, 09 Feb 2014 07:15:48 -0500 Received: by mail-lb0-f173.google.com with SMTP id s7so2494196lbd.32 for ; Sun, 09 Feb 2014 04:15:46 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <1391183143-30724-1-git-send-email-peter.maydell@linaro.org> <1391183143-30724-16-git-send-email-peter.maydell@linaro.org> From: Peter Maydell Date: Sun, 9 Feb 2014 12:15:25 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v2 15/35] target-arm: Drop success/fail return from cpreg read and write functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Rob Herring , Laurent Desnogues , Patch Tracking , Michael Matz , Alexander Graf , "qemu-devel@nongnu.org Developers" , Claudio Fontana , Dirk Mueller , Will Newton , =?UTF-8?B?QWxleCBCZW5uw6ll?= , "kvmarm@lists.cs.columbia.edu" , Christoffer Dall , Richard Henderson On 9 February 2014 03:27, Peter Crosthwaite wrote: > On Sat, Feb 1, 2014 at 1:45 AM, Peter Maydell wrote: >> -/* Access functions for coprocessor registers. These should always succeed. */ >> -typedef int CPReadFn(CPUARMState *env, const ARMCPRegInfo *opaque, >> - uint64_t *value); >> -typedef int CPWriteFn(CPUARMState *env, const ARMCPRegInfo *opaque, >> - uint64_t value); >> +/* Access functions for coprocessor registers. These cannot fail and >> + * may not raise exceptions. > > Is this based on an assumption that the only possible reason for an > exception is access violation? This series quite validly obsoletes the > need for exception-via-return-path, but my understanding is that CP > accessor functions are able to implement any form of side affects. If > an exception is thus generated from the side effects of a CP access > then thats fair game - it just happens via the already existing > mechanisms for generating an exception from helper context. Long story > short, I think you can just drop second sentence from this comment. It's based on the fact that the translate.c code no longer supports calling raise_exception() from within a read/write function -- if you try it the PC will be wrong because we haven't synchronised it before calling the helper function. You're right that in theory a coprocessor access could raise an arbitrary exception, but if anybody needs that they'll need to add the support (probably by adding an extra ARM_CP_ flag for "can raise exceptions" so we don't take the hit of synchronising PC except in the odd case where it's necessary.) In practice I think it is unlikely that there will be any such situation which couldn't be handled by a suitable accessfn. >> uint32_t HELPER(get_cp_reg)(CPUARMState *env, void *rip) >> { >> const ARMCPRegInfo *ri = rip; >> - uint64_t value; >> - int excp = ri->readfn(env, ri, &value); >> - if (excp) { >> - raise_exception(env, excp); >> - } >> - return value; > > Should ideally be a blank line here, but .. > >> + return ri->readfn(env, ri); > > ... you could just drop the single use variable completely with: > > return ri->readfn(env, (const ARMCPRegInfo *)rip); I dislike casts like that. I'll put in the blank line if you like. thanks -- PMM