From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Mwi7Z-0001n9-BT for qemu-devel@nongnu.org; Sat, 10 Oct 2009 16:00:53 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mwi7U-0001md-Mm for qemu-devel@nongnu.org; Sat, 10 Oct 2009 16:00:53 -0400 Received: from [199.232.76.173] (port=50443 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mwi7U-0001ma-JX for qemu-devel@nongnu.org; Sat, 10 Oct 2009 16:00:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37966) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Mwi7T-0003CH-S8 for qemu-devel@nongnu.org; Sat, 10 Oct 2009 16:00:48 -0400 Date: Sat, 10 Oct 2009 21:58:39 +0200 From: "Michael S. Tsirkin" Message-ID: <20091010195839.GH14275@redhat.com> References: <1255069742-15724-1-git-send-email-yamahata@valinux.co.jp> <1255069742-15724-26-git-send-email-yamahata@valinux.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1255069742-15724-26-git-send-email-yamahata@valinux.co.jp> Subject: [Qemu-devel] Re: [PATCH V5 25/29] pci: add helper functions for pci config write function. List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Isaku Yamahata Cc: qemu-devel@nongnu.org On Fri, Oct 09, 2009 at 03:28:58PM +0900, Isaku Yamahata wrote: > add helper functions for pci config write function to check > if its configuration space is changed. > To detect the change in configuration space, memcpy the original > value and memcmp with updated value. > The original value is allocated in PCIDevice because its length > might be 4K for pci express which is a bit too large for stack. > > Those functions will be used later and generic enough for specific > pci device to use. > > Signed-off-by: Isaku Yamahata > --- > hw/pci.c | 2 ++ > hw/pci.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 52 insertions(+), 0 deletions(-) > > diff --git a/hw/pci.c b/hw/pci.c > index 8e396b6..ece429f 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -446,6 +446,7 @@ static void pci_config_alloc(PCIDevice *pci_dev) > pci_dev->cmask = qemu_mallocz(config_size); > pci_dev->wmask = qemu_mallocz(config_size); > pci_dev->used = qemu_mallocz(config_size); > + pci_dev->orig = qemu_mallocz(config_size); > } > > static void pci_config_free(PCIDevice *pci_dev) > @@ -454,6 +455,7 @@ static void pci_config_free(PCIDevice *pci_dev) > qemu_free(pci_dev->cmask); > qemu_free(pci_dev->wmask); > qemu_free(pci_dev->used); > + qemu_free(pci_dev->orig); > } > > /* -1 for devfn means auto assign */ > diff --git a/hw/pci.h b/hw/pci.h > index bfa29c8..2896b0e 100644 > --- a/hw/pci.h > +++ b/hw/pci.h > @@ -192,6 +192,9 @@ struct PCIDevice { > /* Used to allocate config space for capabilities. */ > uint8_t *used; > > + /* Used to implement configuration space change */ > + uint8_t *orig; > + > /* the following fields are read only */ > PCIBus *bus; > uint32_t devfn; > @@ -388,6 +391,53 @@ static inline uint32_t pci_config_size(PCIDevice *d) > return pci_is_express(d) ? PCIE_CONFIG_SPACE_SIZE : PCI_CONFIG_SPACE_SIZE; > } > > +struct pci_config_update { > + PCIDevice *d; > + uint32_t addr; > + uint32_t val; > + int len; > +}; > + > +static inline void pci_write_config_init(struct pci_config_update *update, > + PCIDevice *d, > + uint32_t addr, uint32_t val, int len) > +{ > + update->d = d; > + update->addr = addr; > + update->val = val; > + update->len = len; > + memcpy(d->orig, d->config, pci_config_size(d)); > +} > + I think this is a bad API, because it is not re-entrant: If someone calls pci_write_config_init and then default_write_config which calls pci_write_config_init internally, everything breaks. What happens if default write onfig will just update regions on each write into PCI header? That would simplify code very much. > +static inline void pci_write_config_update(struct pci_config_update *update) > +{ > + PCIDevice *d = update->d; > + uint32_t addr = update->addr; > + uint32_t val = update->val; > + uint32_t config_size = pci_config_size(d); > + int i; > + > + for(i = 0; i < update->len && addr < config_size; val >>= 8, ++i, ++addr) { > + uint8_t wmask = d->wmask[addr]; > + d->config[addr] = (d->config[addr] & ~wmask) | (val & wmask); > + } > +} > + > +/* check if [base, end) in configuration space is changed */ > +static inline int pci_config_changed(const struct pci_config_update *update, > + uint32_t base, uint32_t end) > +{ > + return memcmp(update->d->orig + base, update->d->config + base, > + end - base); > +} > + > +/* for convinience not to type symbol constant twice */ > +static inline int pci_config_changed_with_size( > + const struct pci_config_update *update, uint32_t base, uint32_t size) > +{ > + return pci_config_changed(update, base, base + size); > +} > + > /* lsi53c895a.c */ > #define LSI_MAX_DEVS 7 > > -- > 1.6.0.2