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

[Blue Swirl]

On Tue, Apr 5, 2011 at 10:39 AM, Avi Kivity <address@hidden> wrote:
> On 04/04/2011 08:02 PM, 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.
>>
>> 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.
>
> The more complicated BARs cannot be described declaratively (at least
> without a lot of complicated infrastructure).  They can switch from RAM to
> MMIO mappings at runtime, and have different sub-regions.

Subregions should be possible, but I agree run time switch will not.
Those should be pretty rare, though.