From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:38145) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R5eJh-0007yL-V3 for qemu-devel@nongnu.org; Mon, 19 Sep 2011 09:55:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R5eJg-0007ej-DM for qemu-devel@nongnu.org; Mon, 19 Sep 2011 09:55:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55748) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R5eJg-0007ee-4G for qemu-devel@nongnu.org; Mon, 19 Sep 2011 09:55:24 -0400 Message-ID: <4E7749C6.3090305@redhat.com> Date: Mon, 19 Sep 2011 15:55:18 +0200 From: Gerd Hoffmann MIME-Version: 1.0 References: <4E75E7A4.50702@web.de> <4E75E96E.90101@web.de> <4E761051.9000204@redhat.com> <4E76119F.5040004@web.de> <4E761220.3080707@redhat.com> <4E761C20.4040609@web.de> <4E76212A.20000@redhat.com> <4E764185.8020500@web.de> <4E773365.9010008@redhat.com> <4E773644.8020005@web.de> <4E7738C5.6050504@redhat.com> <4E773BD5.8020105@web.de> In-Reply-To: <4E773BD5.8020105@web.de> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/2] memory: Fix old portio word accesses List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Richard Henderson , Avi Kivity , qemu-devel Hi, >> The trick of having a way to register N callbacks with one shot is worth >> growing. Ideally each register in a BAR would have a callback and we'd >> do something like >> >> MemoryRegionOps mydev_ops = { >> .registers = { >> { MYDEV_REG_x, 4, 4, mydev_reg_x_read, mydev_reg_x_write, }, >> ... >> }, >> } >> >> with hints to the core like "this register sits at this offset, use it >> for reads instead of a callback", or, "this is a read-only register". > > This has pros and cons. If you have n registers to dispatch, you then > have to write n function prologues and maybe epilogues instead of just > one. Specifically if the register access is trivial, that could case > quite some LoC blowup on the device side. If the register access is trivial then you don't need to call into the driver at all ... You can have a look at hw/intel-hda.c which actually implements something like this, with some commonly needed features: * The "offset" field already mentioned by avi is there, so trivial reads/writes can be handled by the core. * A "wmask" field to specify which bits are guest writable. * A "wclear" field to specify which bits have write-one-to-clear semantics. * A "reset" field which specified the value this field has after device reset. Also serves as value for read-only registers. * read/write handlers of course. The write handler is called after the core applied sanity checks and calculated the new register value (using wmask+wclear). * A "name" field (for debug logging). It's pretty nice, alot more readable that a big switch, forces you to think which bits the guest can set (not specifying a wmask gives you a read-only register ;). Also no bloat. With this moving to memory core the all the handlers will gain a line with a container_of(), but that isn't too bad too IMHO. cheers, Gerd