Re: PCI BAR's prefetchable bit and pci_mapreg_map()

[Masanobu SAITOH <msaitoh%execsw.org@localhost>]

On 2017/03/24 8:13, Michael wrote:
> Hello,
>
> On Wed, 22 Mar 2017 16:35:21 +0900
> Masanobu SAITOH <msaitoh%execsw.org@localhost> wrote:
>
> >   A) modify pci_mapreg_map(). Stop setting BUS_SPACE_MAP_PREFETCHABLE
> >      by defalut when prefetchable bit is set. If a driver really know
> >      the whole area of the BAR is prefetchable, set BUS_SPACE_MAP_PREFETCHABLE
> >      in the 4th argument(busflags) of pci_mapreg_map(). pci_mapreg_map()
> >      check for both the 4th argument and the prefetchable bit, it sets
> >      BUS_SPACE_MAP_PREFETCHABLE only when both bits are set.
>
> On 2nd thought, I think we should ignore the prefetchable bit in the
> BAR here

 in pci_mapreg_map()?

> and just go with whatever the driver wants,

 You mean:

    For pci_mapreg_map():

	Don't check prefetchable bit and don't set BUS_SPACE_MAP_PREFETCHABLE
	in pci_mapreg_map() like OpenBSD.

    For each driver side:

	Don't use pci_mapreg_map() and use pci_mapreg_info() and
	bus_space_map() to use an area with BUS_SPACE_MAP_PREFETCHABLE
	(Plan D in my original mail)

Right? Let call this plan E.

The plan E is OK, but it makes code duplication and the come won't
be simple. To make code simple,

>  A) modify pci_mapreg_map(). Stop setting BUS_SPACE_MAP_PREFETCHABLE
>     by defalut when prefetchable bit is set. If a driver really know
>     the whole area of the BAR is prefetchable, set BUS_SPACE_MAP_PREFETCHABLE
>     in the 4th argument(busflags) of pci_mapreg_map(). pci_mapreg_map()
>     check for both the 4th argument and the prefetchable bit, it sets
>     BUS_SPACE_MAP_PREFETCHABLE only when both bits are set.
>
>  B) Don't modify pci_mapreg_ma(). Add new API like FreeBSD's pmap_change_attr()
>     and use it when a driver want.
>
>  C) make new function pci_mapreg_map_wc()
>
>  D) Keep. If a driver know a BAR has prefetchable bit and not the
>     whole area is prefetchable, do pci_mem_find() -> drop BUS_SPACE_MAP_PREFETCHABLE
>     and do bus_space_map().

   E) Don't check prefetchable bit and don't set BUS_SPACE_MAP_PREFETCHABLE
      in any case in pci_mapreg_map() like OpenBSD.
      Don't use pci_mapreg_map() and use pci_mapreg_info() and
      bus_space_map() if a driver want to use an area with
      BUS_SPACE_MAP_PREFETCHABLE.
 
   F) Any other solution(s).

Plan A, B and C are useful than D and E to make code simple.

> since you ran into
> devices which have the bit set and shouldn't, and I have seen graphics
> chips which don't set it for their video memory apertures.

Even if we choose A or E, we have to check all driver
which already use pci_mapreg_map() and modify some of
them.

    For A:
    - BAR has no prefetchable bit:
	not required to modify
    - BAR has prefetchable bit and BUS_SPACE_MAP_PREFETCHABLE can't be used:
	not required to modify
    - really prefetchable:
	set BUS_SPACE_MAP_PREFETCHABLE in the 4th argument

    For D:
    - BAR has no prefetchable bit:
	not required to modify
    - BAR has prefetchable bit and BUS_SPACE_MAP_PREFETCHABLE can't be used:
	not required to modify
    - really prefetchable:
	Extract it with pci_mapreg_info() and bus_space_map()


> have fun
> Michael


--
-----------------------------------------------
                SAITOH Masanobu (msaitoh%execsw.org@localhost
                                 msaitoh%netbsd.org@localhost)