Message ID | 20091008092911.GC5269@redhat.com |
---|---|
State | New |
Headers | show |
On Thu, Oct 08, 2009 at 11:29:11AM +0200, Michael S. Tsirkin wrote: > On Thu, Oct 08, 2009 at 09:08:07AM +0900, Isaku Yamahata wrote: > > On Wed, Oct 07, 2009 at 02:33:49PM +0200, Michael S. Tsirkin wrote: > > > There's no need to save all of config space before each config cycle: > > > just the 64 byte header is enough for our purposes. This will become > > > more important as we add pci express support, which has 4K config space. > > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > Cc: Isaku Yamahata <yamahata@valinux.co.jp> > > > --- > > > > > > Isaku Yamahata, I think with this change, you can > > > increase the size of config space to 4K without > > > need for helper functions. Makes sense? > > > > It is good that the patch makes the function header size > > independent(256 or 4K). > > However, your patch just makes the code special. > > I think helper functions should be introduced eventually. > > So maybe we should get away from both memcpy and range checks. How > about the following? Note: it's untested, and we should see what this > does to boot times. Quite likely nothing. I'd like to see some number on > how many times does PCI header get written to during boot. I might check > it myself but not anytime soon. Looks good. I'll go for this approach. thanks, > Not-Quite-Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > diff --git a/hw/pci.c b/hw/pci.c > index 1debdab..19b8cc6 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -583,18 +583,14 @@ uint32_t pci_default_read_config(PCIDevice *d, > > void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l) > { > - uint8_t orig[PCI_CONFIG_SPACE_SIZE]; > int i; > > /* not efficient, but simple */ > - memcpy(orig, d->config, PCI_CONFIG_SPACE_SIZE); > for(i = 0; i < l && addr < PCI_CONFIG_SPACE_SIZE; val >>= 8, ++i, ++addr) { > uint8_t wmask = d->wmask[addr]; > d->config[addr] = (d->config[addr] & ~wmask) | (val & wmask); > } > - if (memcmp(orig + PCI_BASE_ADDRESS_0, d->config + PCI_BASE_ADDRESS_0, 24) > - || ((orig[PCI_COMMAND] ^ d->config[PCI_COMMAND]) > - & (PCI_COMMAND_MEMORY | PCI_COMMAND_IO))) > + if (ranges_overlap(addr, l, PCI_COMMAND, PCI_ROM_ADDRESS + 3)) > pci_update_mappings(d); > } > > diff --git a/hw/pci.h b/hw/pci.h > index 93f93fb..b9374d1 100644 > --- a/hw/pci.h > +++ b/hw/pci.h > @@ -366,4 +366,27 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base, > PCIBus *sh_pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq, > void *pic, int devfn_min, int nirq); > > +/* These are not pci specific. Should move into a separate header. */ > + > +/* Check whether a given range covers a given byte. */ > +static inline int range_covers_byte(uint64_t first, uint64_t last, > + uint64_t byte) > +{ > + return first <= byte && last >= byte; > +} > + > +/* Check whether 2 given ranges overlap. Undefined if last < first. */ > +static inline int ranges_overlap(uint64_t first1, uint64_t last1, > + uint64_t first2, uint64_t last2) > +{ > + return first1 >= last2 && first2 >= last1; > +} > + > +/* Get last byte of a range from offset + length. > + * Undefined for ranges that wrap around 0. */ > +static inline uint64_t range_get_last(uint64_t offset, uint64_t len) > +{ > + return offset + len - 1; > +} > + > #endif >
diff --git a/hw/pci.c b/hw/pci.c index 1debdab..19b8cc6 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -583,18 +583,14 @@ uint32_t pci_default_read_config(PCIDevice *d, void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l) { - uint8_t orig[PCI_CONFIG_SPACE_SIZE]; int i; /* not efficient, but simple */ - memcpy(orig, d->config, PCI_CONFIG_SPACE_SIZE); for(i = 0; i < l && addr < PCI_CONFIG_SPACE_SIZE; val >>= 8, ++i, ++addr) { uint8_t wmask = d->wmask[addr]; d->config[addr] = (d->config[addr] & ~wmask) | (val & wmask); } - if (memcmp(orig + PCI_BASE_ADDRESS_0, d->config + PCI_BASE_ADDRESS_0, 24) - || ((orig[PCI_COMMAND] ^ d->config[PCI_COMMAND]) - & (PCI_COMMAND_MEMORY | PCI_COMMAND_IO))) + if (ranges_overlap(addr, l, PCI_COMMAND, PCI_ROM_ADDRESS + 3)) pci_update_mappings(d); } diff --git a/hw/pci.h b/hw/pci.h index 93f93fb..b9374d1 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -366,4 +366,27 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base, PCIBus *sh_pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq, void *pic, int devfn_min, int nirq); +/* These are not pci specific. Should move into a separate header. */ + +/* Check whether a given range covers a given byte. */ +static inline int range_covers_byte(uint64_t first, uint64_t last, + uint64_t byte) +{ + return first <= byte && last >= byte; +} + +/* Check whether 2 given ranges overlap. Undefined if last < first. */ +static inline int ranges_overlap(uint64_t first1, uint64_t last1, + uint64_t first2, uint64_t last2) +{ + return first1 >= last2 && first2 >= last1; +} + +/* Get last byte of a range from offset + length. + * Undefined for ranges that wrap around 0. */ +static inline uint64_t range_get_last(uint64_t offset, uint64_t len) +{ + return offset + len - 1; +} + #endif