From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54573) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTGmr-0003nZ-8X for qemu-devel@nongnu.org; Tue, 09 Feb 2016 17:29:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTGmq-0007ve-1I for qemu-devel@nongnu.org; Tue, 09 Feb 2016 17:29:33 -0500 Received: from mail-wm0-x230.google.com ([2a00:1450:400c:c09::230]:37334) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTGmp-0007vE-NK for qemu-devel@nongnu.org; Tue, 09 Feb 2016 17:29:31 -0500 Received: by mail-wm0-x230.google.com with SMTP id g62so3386338wme.0 for ; Tue, 09 Feb 2016 14:29:31 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <0e7c92d2ff8312229109245ba0c5b38ced753295.1454115217.git.alistair.francis@xilinx.com> <87bn7px409.fsf@linaro.org> Date: Tue, 9 Feb 2016 14:29:30 -0800 Message-ID: From: Peter Crosthwaite Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 02/16] register: Add Register API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alistair Francis Cc: Edgar Iglesias , Peter Maydell , "qemu-devel@nongnu.org Developers" , Edgar Iglesias , =?UTF-8?B?QWxleCBCZW5uw6ll?= , =?UTF-8?Q?Andreas_F=C3=A4rber?= , =?UTF-8?B?S09OUkFEIEZyw6lkw6lyaWM=?= On Tue, Feb 9, 2016 at 11:35 AM, Alistair Francis wrote: > On Tue, Feb 9, 2016 at 8:06 AM, Alex Benn=C3=A9e = wrote: >> >> Alistair Francis writes: >> >>> This API provides some encapsulation of registers and factors our some >>> common functionality to common code. Bits of device state (usually MMIO >>> registers), often have all sorts of access restrictions and semantics >>> associated with them. This API allow you to define what those >>> restrictions are on a bit-by-bit basis. >>> >>> Helper functions are then used to access the register which observe the >>> semantics defined by the RegisterAccessInfo struct. >>> >>> Some features: >>> Bits can be marked as read_only (ro field) >>> Bits can be marked as write-1-clear (w1c field) >>> Bits can be marked as reserved (rsvd field) >>> Reset values can be defined (reset) >>> Bits can throw guest errors when written certain values (ge0, ge1) >>> Bits can throw unimp errors when written certain values (ui0, ui1) >>> Bits can be marked clear on read (cor) >>> Pre and post action callbacks can be added to read and write ops >>> Verbose debugging info can be enabled/disabled >>> >>> Useful for defining device register spaces in a data driven way. Cuts >>> down on a lot of the verbosity and repetition in the switch-case blocks >>> in the standard foo_mmio_read/write functions. >>> >>> Also useful for automated generation of device models from hardware >>> design sources. >>> >>> Signed-off-by: Peter Crosthwaite >>> Signed-off-by: Alistair Francis >>> --- >>> V3: >>> - Address some comments from Fred >>> >>> hw/core/Makefile.objs | 1 + >>> hw/core/register.c | 189 ++++++++++++++++++++++++++++++++++++++++++= ++++++++ >>> include/hw/register.h | 132 +++++++++++++++++++++++++++++++++++ >>> 3 files changed, 322 insertions(+) >>> create mode 100644 hw/core/register.c >>> create mode 100644 include/hw/register.h >>> >>> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs >>> index abb3560..bf95db5 100644 >>> --- a/hw/core/Makefile.objs >>> +++ b/hw/core/Makefile.objs >>> @@ -14,4 +14,5 @@ common-obj-$(CONFIG_SOFTMMU) +=3D machine.o >>> common-obj-$(CONFIG_SOFTMMU) +=3D null-machine.o >>> common-obj-$(CONFIG_SOFTMMU) +=3D loader.o >>> common-obj-$(CONFIG_SOFTMMU) +=3D qdev-properties-system.o >>> +common-obj-$(CONFIG_SOFTMMU) +=3D register.o >>> common-obj-$(CONFIG_PLATFORM_BUS) +=3D platform-bus.o >>> diff --git a/hw/core/register.c b/hw/core/register.c >>> new file mode 100644 >>> index 0000000..f0fc39c >>> --- /dev/null >>> +++ b/hw/core/register.c >>> @@ -0,0 +1,189 @@ >>> +/* >>> + * Register Definition API >>> + * >>> + * Copyright (c) 2013 Xilinx Inc. >>> + * Copyright (c) 2013 Peter Crosthwaite >>> + * >>> + * This work is licensed under the terms of the GNU GPL, version 2. S= ee >>> + * the COPYING file in the top-level directory. >>> + */ >>> + >>> +#include "hw/register.h" >>> +#include "qemu/log.h" >>> + >>> +static inline void register_write_log(RegisterInfo *reg, int dir, uint= 64_t val, >>> + int mask, const char *msg, >>> + const char *reason) >>> +{ >>> + qemu_log_mask(mask, "%s:%s bits %#" PRIx64 " %s write of %d%s%s\n"= , >>> + reg->prefix, reg->access->name, val, msg, dir, >>> + reason ? ": " : "", reason ? reason : ""); >> >> I'm not sure mask needs to be passed down as every use of it seems to >> imply LOG_GUEST_USER. If you think this might change I would consider >> passing the mask as the first option to align with other logging functio= ns. > > Removing this function as it is no longer required. > >> >>> +} >>> + >>> +static inline void register_write_val(RegisterInfo *reg, uint64_t val) >>> +{ >>> + if (!reg->data) { >>> + return; >>> + } >> >> Can you have a defined register without a data field? If it is invalid I >> would use a g_assert() instead. > > Good point, I can't see any cases where there wouldn't be a data field. > >>From the register API point of view it should be valid. It just so happens when use the array based registration it never comes up. So I think assert is not quite right even though the path is dead as of this series. Regards, Peter