[Qemu-devel] Re: [PATCH] qemu/pci: optimize pci config handling

["Michael S. Tsirkin" <mst@redhat.com>]

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.

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