From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60368) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YbSyE-0007n0-H9 for qemu-devel@nongnu.org; Fri, 27 Mar 2015 08:02:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YbSyB-0008Dp-6G for qemu-devel@nongnu.org; Fri, 27 Mar 2015 08:02:38 -0400 Received: from mail-pd0-x22a.google.com ([2607:f8b0:400e:c02::22a]:33702) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YbSyA-0008D9-V3 for qemu-devel@nongnu.org; Fri, 27 Mar 2015 08:02:35 -0400 Received: by pdnc3 with SMTP id c3so94822096pdn.0 for ; Fri, 27 Mar 2015 05:02:33 -0700 (PDT) Date: Fri, 27 Mar 2015 22:02:28 +1000 From: "Edgar E. Iglesias" Message-ID: <20150327120228.GA19483@toto> References: <1426526422-28338-1-git-send-email-peter.maydell@linaro.org> <1426526422-28338-2-git-send-email-peter.maydell@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [RFC 1/5] memory: Define API for MemoryRegionOps to take attrs and return status List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Peter Crosthwaite , Patch Tracking , QEMU Developers , Greg Bellows , Paolo Bonzini , Alex =?iso-8859-1?Q?Benn=E9e?= , Richard Henderson On Fri, Mar 27, 2015 at 10:58:01AM +0000, Peter Maydell wrote: > On 16 March 2015 at 17:20, Peter Maydell wrote: > > Define an API so that devices can register MemoryRegionOps whose read > > and write callback functions are passed an arbitrary pointer to some > > transaction attributes and can return a success-or-failure status code. > > This will allow us to model devices which: > > * behave differently for ARM Secure/NonSecure memory accesses > > * behave differently for privileged/unprivileged accesses > > * may return a transaction failure (causing a guest exception) > > for erroneous accesses > > > The success/failure response indication is currently ignored; it is > > provided so we can implement it later without having to change the > > callback function API yet again in future. > > > +/* New-style MMIO accessors can indicate that the transaction failed. > > + * This is currently ignored, but provided in the API to allow us to add > > + * support later without changing the MemoryRegionOps functions yet again. > > + */ > > +typedef enum { > > + MemTx_OK = 0, > > + MemTx_DecodeError = 1, /* "nothing at that address" */ > > + MemTx_SlaveError = 2, /* "device unhappy with request" (eg misalignment) */ > > +} MemTxResult; > > So I was looking at how this would actually get plumbed through > the memory subsystem code, and there are some awkwardnesses > with this simple enum approach. In particular, functions like > address_space_rw want to combine the error returns from > several io_mem_read/write calls into a single response to > return to the caller. With an enum we'd need some pretty > ugly code to prioritise particular failure types, or to > do something arbitrary like "return first failure code". > Alternatively we could: > (a) make MemTxResult a uint32_t, where all-bits zero indicates > "OK" and any bit set indicates some kind of error, eg > bit 0 set for "device returned an error", and bit 1 for > "decode error", and higher bits available for other kinds > of extra info about errors in future. Then address_space_rw > just ORs together all the bits in all the return codes it > receives. > (b) give up and say "just use a bool" > > Opinions? Hi Peter, Is this related to masters relying on the memory frameworks magic handling of unaliged accesses? I guess that masters that really care about accurate the error handling would need to issue transactions without relying on the intermediate "magic" that splits unaligned accesses... Anyway, I think your option a sounds the most flexible... Cheers, Edgar