Re: [Qemu-devel] Re: [PATCH 00/10] pci: pci_register_bar_simple

[Blue Swirl]

On Mon, Apr 4, 2011 at 10:44 PM, Michael S. Tsirkin <address@hidden> 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 <address@hidden> 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<address@hidden>
>> >>>
>> >>> 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.

>> 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.