On Wed, 2013-03-27 at 10:35 +0800, Gavin Shan wrote: > On Tue, Mar 26, 2013 at 12:17:03PM -0600, Alex Williamson wrote: > >The config map previously used a byte per dword to map regions of > >config space to capabilities. Modulo a bug where we round the length > >of capabilities down instead of up, this theoretically works well and > >saves space so long as devices don't try to hide registers in the gaps > >between capabilities. Unfortunately they do exactly that so we need > >byte granularity on our config space map. Increase the allocation of > >the config map and split accesses at capability region boundaries. > > > > Alex, one question that isn't related to the patch: With current implementation, > (pdev->cfg_size) bytes are used for capability bits. That's really waste of > memory because the memory for specific capability will be reserved even though > that capability can't be supported by the device. I'm not sure the following scheme > is workable in order to save more memory for that? > > Organize the config space using RB-tree by following struct. The benefit would be > we won't reserve memory for those unsupported capabilities. > > struct pci_cap_map { > struct rb_node rb_node; > unsigned short start; /* Start position */ > unsigned short end; /* End position */ > unsigned short cap_id; /* Capability ID */ > }; I would definitely like to see some kind of data structure make this more efficient. The map obviously gives us direct indexing and if we just look at legacy config space, it's often tightly packed. 256 bytes vs (30 bytes * num_capability_regions) is nearly even on overall size. The extended config space is where space efficiency falls apart. We don't actually have that many elements to track either, maybe up to a dozen. A sorted list would make the structure size half that of the tree. We could even use two lists to split legacy vs extended. Interested in working on it? Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html