From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=58231 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OvrXC-0008AA-Nz for qemu-devel@nongnu.org; Wed, 15 Sep 2010 08:56:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OvrX6-0007Of-2M for qemu-devel@nongnu.org; Wed, 15 Sep 2010 08:56:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:25315) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OvrX5-0007OV-Qt for qemu-devel@nongnu.org; Wed, 15 Sep 2010 08:56:16 -0400 Date: Wed, 15 Sep 2010 14:49:50 +0200 From: "Michael S. Tsirkin" Message-ID: <20100915124950.GB31520@redhat.com> References: <4b8294628910d08f11544451861008d1c73362f6.1284528424.git.yamahata@valinux.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4b8294628910d08f11544451861008d1c73362f6.1284528424.git.yamahata@valinux.co.jp> Subject: [Qemu-devel] Re: [PATCH v3 03/13] pci: introduce helper function pci_shift_word/long which returns shifted value. List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Isaku Yamahata Cc: skandasa@cisco.com, etmartin@cisco.com, qemu-devel@nongnu.org, wexu2@cisco.com On Wed, Sep 15, 2010 at 02:38:16PM +0900, Isaku Yamahata wrote: > introduce helper function pci_shift_{word, long}() which returns > returns shifted word/long of given position and range. > They will be used later. > > Signed-off-by: Isaku Yamahata So I think the reason you *think* you need these is because you set the wmask wrong: you make all capability readonly and then write a ton of custom code to tweak it. Instead, Make writeable registers writeable, even if read always returns 0. Then in your handler do if (dev->config[offset] & mask) { handle bit write dev->config[offset] &= ~mask; } no range checks necessary. If you need to do something on register change, just keep the old state in your structure, then dev->exp.a != dev->config[offset] tells you there was a change. BTW if you need to do this to words or longs, not just bytes, maybe pci_set_word/pci_clear_word would be helpful? > --- > hw/pci.h | 19 +++++++++++++++++++ > 1 files changed, 19 insertions(+), 0 deletions(-) > > diff --git a/hw/pci.h b/hw/pci.h > index f4ea97a..630631b 100644 > --- a/hw/pci.h > +++ b/hw/pci.h > @@ -327,6 +327,25 @@ pci_config_set_interrupt_pin(uint8_t *pci_config, uint8_t val) > pci_set_byte(&pci_config[PCI_INTERRUPT_PIN], val); > } > > +static inline uint32_t > +pci_shift_long(uint32_t addr, uint32_t val, uint32_t pos) > +{ > + if (addr >= pos) { > + assert(addr - pos <= 32 / 8); > + val <<= (addr - pos) * 8; > + } else { > + assert(pos - addr <= 32 / 8); > + val >>= (pos - addr) * 8; > + } > + return val; > +} > + > +static inline uint16_t > +pci_shift_word(uint32_t addr, uint32_t val, uint32_t pos) > +{ > + return pci_shift_long(addr, val, pos); > +} > + > typedef int (*pci_qdev_initfn)(PCIDevice *dev); > typedef struct { > DeviceInfo qdev; > -- > 1.7.1.1