efifb: add a tunable to select the framebuffer cache attribute

[kevans]

Summary

Mapping the framebuffer with WC (Write Combined) memory type can, in
practice, cause some memory transactions to be rate-limited at a
fraction of the fb write rate. WC allows one core to queue up many
globally visible write transactions, and in the process some unrelated
transactions may end up having to wait for all of the queued up PCI
writes to be flushed.

Add an hw.efifb.cache_attr tunable to allow mapping the framebuffer as
uncacheable instead. We should likely be taking a more careful approach
of checking the memory map to determine which cacheability attributes
are feasible, but the knob lets us use our historically functional
behavior while offering a convenient way to switch on a stock kernel.

The only valid values for hw.efifb.cache_attr at this time are "uc" and
"wc".

Original patch by Marc De La Gueronniere <[e-mail]>

MFC after: 1 week
Sponsored by: Verisign, Inc.
Sponsored by: Klara, Inc.

[jhb]

I think what we want to do is have a loader tunable to control this. We probably want to default to WC still, but a loader tunable makes it easy to toggle this for cases that need UC instead.

[imp]

I agree with John here. It's likely OK, but there's been little to no testing, nor any presentation of benchmark results to show this is a good fix, nor comparisons to what others, eg Linux, do here.
I'd like to see this be at least a loader tunable.

[kevans]

In D17884#869536, @imp wrote:

I agree with John here. It's likely OK, but there's been little to no testing, nor any presentation of benchmark results to show this is a good fix, nor comparisons to what others, eg Linux, do here.
I'd like to see this be at least a loader tunable.

I actually think both this patch and the patched code are technically wrong. Linux searches for a matching segment in the memory map and checks the cacheability attributes on it before determining that it can do either WC/UC in the first place, which... actually makes a lot of sense.

We can probably shelve that for now and add a tunable, though I note that "little to no testing" isn't really accurate -- Verisign's had this deployed for a number of years at this point with no problem, though I don't really have a good idea of how diverse their hardware set is.

[imp]

In D17884#869935, @kevans wrote:

In D17884#869536, @imp wrote:

I agree with John here. It's likely OK, but there's been little to no testing, nor any presentation of benchmark results to show this is a good fix, nor comparisons to what others, eg Linux, do here.
I'd like to see this be at least a loader tunable.

I actually think both this patch and the patched code are technically wrong. Linux searches for a matching segment in the memory map and checks the cacheability attributes on it before determining that it can do either WC/UC in the first place, which... actually makes a lot of sense.

Indeed, that makes a lot more sense...

We can probably shelve that for now and add a tunable, though I note that "little to no testing" isn't really accurate -- Verisign's had this deployed for a number of years at this point with no problem, though I don't really have a good idea of how diverse their hardware set is.

Fair points, both... And re-reading it, it sounds crankier than I'd wanted it to sound... I just meant to say no characterization of what hardware that this was needed for, or what it worked with was offered.

[kevans]

Switch to a hw.efifb.cache_attr tunable to select between UNCACHEABLE and WRITE_COMBINING, note that we're probably subtly wrong and have room for future improvement.

[imp]

As you say, this is imperfect, but it does move the state of the hard forward.