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 ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference -- _______________________________________________ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
