On Tue, Nov 12, 2019 at 03:26:35PM +0100, Daniel Vetter wrote:

On Tue, Nov 12, 2019 at 3:06 PM Christoph Hellwig [off-list ref] wrote:

quoted

On Tue, Nov 12, 2019 at 02:04:16PM +0100, Daniel Vetter wrote:

quoted

Wut ... Maybe I'm missing something, but from how we use mtrr in other
gpu drivers it's a) either you use MTRR because that's all you got or
b) you use pat. Mixing both sounds like a pretty bad idea, since if
you need MTRR for performance (because you dont have PAT) then you
can't fix the wc with the PAT-based ioremap_uc. And if you have PAT,
then you don't really need an MTRR to get wc.

So I'd revert this patch from Luis and ...

Sounds great to me..

quoted

... apply this one. Since the same reasoning should apply to anything
that's running on any cpu with PAT.

Can you take a look at "mfd: intel-lpss: Use devm_ioremap_uc for MMIO"
in linux-next, which also looks rather fishy to me?  Can't we use
the MTRR APIs to override the broken BIOS MTRR setup there as well?

Hm so that's way out of my knowledge, but I think mtrr_cleanup() was
supposed to fix up messy/broken MTRR setups by the bios. So maybe they
simply didn't enable that in their .config with CONFIG_MTRR_SANITIZER.

I had originally suggested to just make the driver build on x86, but an
atlternative was to provide the call for the missing architecture.

An explicit cleanup is currently not possible for drivers, since the
only interface exported to drivers is arch_phys_wc_add/del (which
short-circuits if pat works since you don't need mtrr in that case).

Right, the goal was to not call MTRR directly.

Adding everyone from that commit, plus Luis. Drivers really shouldn't
assume/work around the bios setting up superflous/wrong MTRR.

Such things are needed, otherwise some systems may not boot...

quoted

With that we could kill ioremap_uc entirely.

So yeah removing that seems definitely like the right thing.

I think this would be possible if we could flop ioremap_nocache() to UC
instead of UC- on x86. Otherwise, I can't see how we can remove this by
still not allowing direct MTRR calls.

  Luis