Re: [RFC Patch] use MTRR for write combining if PAT is not available

From: Thomas Schlichter
Date: Tue Oct 13 2009 - 17:12:55 EST


Eric Anholt wrote:
> On Mon, 2009-10-12 at 21:45 +0200, Thomas Schlichter wrote:
> > Yes, maybe this patch tries to change current behavior too less. Indeed,
> > if setting up MTRR entries it simply behaves as today, and userspace does
> > not see that write combining is not correctly enabled.
>
> I'm uncomfortable with this patch because it doesn't appear to cover any
> callers of these functions inside of the kernel. Have you audited them
> to make sure they can handle NULL being returned?

No, I haven't. And to be honest, I think the earlier patch that adds MTRR
entries should be more safe, as it modifies behavior only slightly.

> Seems like we should install an MTRR instead. Requiring userland to set
> up the MTRR on the kernel's behalf is backwards.

Exactly.

> With modern drivers we're installing any required MTRRs at module load
> in the kernel, and that's where things should be moving.

I think in general this is not possible, because not for all graphics chips
there are kernel drivers (required). E.g. for my VIA VX800 based notebook,
there is no kernel module that should be responsible to set-up the MTRR
entries. Here it's up to the (userspace) X.org driver. It opens the
/sys/bus/pci/devices/.../resource0_wc device and mmaps the framebuffer memory.
With PAT this will set up a write combining memory area, and with my first
patch this should also happen without PAT with MTRR.

> As long as
> this doesn't interfere with them, I'm OK. And if some kernel driver is
> failing to install its MTRR, well, let's fix it.

Well, I think the mtrr_add inside mmap should not do any harm...

Kind regards,
Thomas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/