On Tue, Apr 5, 2011 at 12:26 AM, Michael S. Tsirkin <m...@redhat.com> wrote: > On Tue, Apr 05, 2011 at 12:05:08AM +0300, Blue Swirl wrote: >> On Mon, Apr 4, 2011 at 10:44 PM, Michael S. Tsirkin <m...@redhat.com> wrote: >> > On Mon, Apr 04, 2011 at 08:02:23PM +0300, Blue Swirl wrote: >> >> On Mon, Apr 4, 2011 at 7:35 PM, Avi Kivity <a...@redhat.com> wrote: >> >> > On 04/04/2011 07:22 PM, Anthony Liguori wrote: >> >> >> >> >> >> On 04/04/2011 10:59 AM, Michael S. Tsirkin wrote: >> >> >>> >> >> >>> On Mon, Apr 04, 2011 at 06:27:57PM +0300, Avi Kivity wrote: >> >> >>>> >> >> >>>> Many PCI BARs that use the memory address space map a single MMIO >> >> >>>> region >> >> >>>> into >> >> >>>> the entire BAR range. Introduce an API pci_register_bar_simple() for >> >> >>>> that use >> >> >>>> case, and convert all users where this can be done trivially. >> >> >>>> >> >> >>>> This will reduce the work required to introduce a PCI memory API; >> >> >>>> it's >> >> >>>> also >> >> >>>> a nice code reduction in its own right. >> >> >>> >> >> >>> This will save some code, so >> >> >>> Acked-by: Michael S. Tsirkin<m...@redhat.com> >> >> >>> >> >> >>> I really hope the rest of devices will follow. >> >> >> >> >> >> How complete is this? >> >> > >> >> > I converted all devices which were easy to convert. There may be one >> >> > or two >> >> > more that can be converted with additional work (and perhaps with an >> >> > additional pic_bar_get_current_address() API, and a >> >> > pci_bar_set_coalescing() >> >> > API). The rest likely need to stick with the callback-based API. >> >> >> >> In my version which I sent earlier but didn't commit, also other BARs >> >> besides the first one and also tricky devices like VGA were handled. >> > >> > Yes, I liked that patchset too. What happened to it? >> >> Nothing, but I thought that there could be a "perfect" solution. >> >> I like in Avi's version that unnecessary API changes are avoided. > > Yes, it's nice that it's incremental. > >> >> But I didn't commit it because I felt it was not going to right >> >> direction. I think the BARs should be specified in PCIDeviceInfo >> >> instead of adding more function calls. The same applies to this patch >> >> set. >> > >> > Is that really that fundamental? What I do care about is >> > making pci.c track and register all device memory >> > so that we can finally implement pci bridge features >> > such as master abort handling and unmapped memory. >> >> The structure version can be done later. Right, pci.c should manage >> the device mappings. > > OK, so applying Avi's patchset and building on that is > your preferred approach too?
Avi's version is a bit too simple, at least multiple regions in a BAR should be handled (for example macio.c needs that). But also that can be added later (pci_register_bar_not_so_simple()?), so Avi's version could be used as the starting point.
