Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 23 Feb 2016 14:19:57 -0600
From:      Justin Hibbits <jrh29@alumni.cwru.edu>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org>
Subject:   Re: RF_CACHEABLE flag
Message-ID:  <CAHSQbTDZVpNU0WsXSHM8yuDqn_5vmy9Ox0fnLZLb2NJfoC7Exg@mail.gmail.com>
In-Reply-To: <20160222121836.GH91220@kib.kiev.ua>
References:  <CAHSQbTA5A3uSDT143e3yWmfzWZyOCDJ4GSo6JO2NiLc_VAKoYg@mail.gmail.com> <20160222121836.GH91220@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Feb 22, 2016 at 6:18 AM, Konstantin Belousov
<kostikbel@gmail.com> wrote:
> On Sun, Feb 21, 2016 at 07:42:49PM -0600, Justin Hibbits wrote:
>> The Freescale/NXP Datapath Acceleration Architecture uses both
>> cache-inhibited and cache-enabled memory regions for buffer portals.
>> This doesn't quite fit right into the existing framework, so I've
>> added to my personal repo (on github) a RF_CACHEABLE flag to be used
>> by this.  Now that I'm ready to commit the driver to head, I want to
>> float this on -arch to get opinions.
>>
>> I did consider another route, using bus_space_map()/bus_space_unmap(),
>> and stashing sizes around, but adding a simple flag to rman would take
>> care of all the details, and rman already knows all the other details
>> for the region anyway.
>>
>> I put the diff on phabricator, at https://reviews.freebsd.org/D5384 .
>>
>> Thoughts on this?
>
> Not making any opinion about RF_CACHEABLE, only pointing out some facts
> that might be interesting to you.  On x86, some BARs need specific memory
> access modes at least for performance.
>
> E.g., for Intel GPUs, there is a combined BAR where the first 512KB or
> 2M maps the registers and must be uncacheable, and the rest of the BAR
> maps GTT. To get a reasonable performance from the graphics hardware,
> GTT should be mapped as write-combining (i.e. not cacheable but writes
> may sit in the combining buffers and flushed on serialization points).
>
> Look at dev/agp/agp_i810.c:agp_gen4_install_gatt() to see direct
> pmap_change_attr(WC) call to set it up.
>
> No flag could accomodate this mode.  OTOH, why couldn't you explicitely
> add pmap_change_attr() to the driver of your device ?

Already mentioned this on IRC yesterday, but best for me to follow up here.

This really isn't much different from bus_space_map() conceptually.
The work involved is pretty much the same, and if this route is deemed
the Wrong Way(TM), I'll go that route.

Grepping through sys/, only x86 currently implements
pmap_change_attr(), and arm has the declaration but no definition that
I could see.  Writing it wouldn't be difficult of course, there's just
not much compelling case for it right now.  But, yes, either of these
alternatives are acceptable, and I had explored it.  Seeing John
Baldwin's comment on the phab review, it looks like he wants to go a
different (parallel) direction.

- Justin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAHSQbTDZVpNU0WsXSHM8yuDqn_5vmy9Ox0fnLZLb2NJfoC7Exg>