Re: [mm patch] i915: fix invalid opcode exception on cpus without clflush
On Thu, Jan 17, 2008 at 09:03:17PM -0500, H. Peter Anvin wrote: > The #ifdef is bogus. If it's required, it should go into > asm-x86/required_features.h and then cpu_has_clflush is static; otherwise > it's just plain wrong. > I have no objection to making cpu_has_clflush constant on x86_64. The point was to get rid of a needless test & branch on 64-bit, and making it constant will have the same effect. cheers, Kyle -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mm patch] i915: fix invalid opcode exception on cpus without clflush
On Thu, 2008-01-17 at 21:03 -0500, H. Peter Anvin wrote: > Kyle McMartin wrote: > > i915_flush_ttm was unconditionally executing a clflush instruction > > to (obviously) flush the cache. Instead, check if the cpu supports > > clflush, and if not, fall back to calling wbinvd to flush the entire > > cache. > > > > Signed-off-by: Kyle McMartin <[EMAIL PROTECTED]> > > > > --- a/drivers/char/drm/i915_buffer.c > > +++ b/drivers/char/drm/i915_buffer.c > > @@ -286,7 +286,18 @@ void i915_flush_ttm(struct drm_ttm *ttm) > > return; > > > > DRM_MEMORYBARRIER(); > > + > > +#ifdef CONFIG_X86_32 > > + /* Hopefully nobody has built an x86-64 processor without clflush */ > > + if (!cpu_has_clflush) { > > + wbinvd(); > > + DRM_MEMORYBARRIER(); > > + return; > > + } > > +#endif > > + > > for (i = ttm->num_pages - 1; i >= 0; i--) > > drm_cache_flush_page(drm_ttm_get_page(ttm, i)); > > + > > DRM_MEMORYBARRIER(); > > } > > The #ifdef is bogus. If it's required, it should go into > asm-x86/required_features.h and then cpu_has_clflush is static; > otherwise it's just plain wrong. I think Andi's CPA patches have some changes regarding wbinvd and clflush handling, perhaps you can leverage some of his work? Harvey -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mm patch] i915: fix invalid opcode exception on cpus without clflush
Kyle McMartin wrote: i915_flush_ttm was unconditionally executing a clflush instruction to (obviously) flush the cache. Instead, check if the cpu supports clflush, and if not, fall back to calling wbinvd to flush the entire cache. Signed-off-by: Kyle McMartin <[EMAIL PROTECTED]> --- a/drivers/char/drm/i915_buffer.c +++ b/drivers/char/drm/i915_buffer.c @@ -286,7 +286,18 @@ void i915_flush_ttm(struct drm_ttm *ttm) return; DRM_MEMORYBARRIER(); + +#ifdef CONFIG_X86_32 + /* Hopefully nobody has built an x86-64 processor without clflush */ + if (!cpu_has_clflush) { + wbinvd(); + DRM_MEMORYBARRIER(); + return; + } +#endif + for (i = ttm->num_pages - 1; i >= 0; i--) drm_cache_flush_page(drm_ttm_get_page(ttm, i)); + DRM_MEMORYBARRIER(); } The #ifdef is bogus. If it's required, it should go into asm-x86/required_features.h and then cpu_has_clflush is static; otherwise it's just plain wrong. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[mm patch] i915: fix invalid opcode exception on cpus without clflush
i915_flush_ttm was unconditionally executing a clflush instruction to (obviously) flush the cache. Instead, check if the cpu supports clflush, and if not, fall back to calling wbinvd to flush the entire cache. Signed-off-by: Kyle McMartin <[EMAIL PROTECTED]> --- a/drivers/char/drm/i915_buffer.c +++ b/drivers/char/drm/i915_buffer.c @@ -286,7 +286,18 @@ void i915_flush_ttm(struct drm_ttm *ttm) return; DRM_MEMORYBARRIER(); + +#ifdef CONFIG_X86_32 + /* Hopefully nobody has built an x86-64 processor without clflush */ + if (!cpu_has_clflush) { + wbinvd(); + DRM_MEMORYBARRIER(); + return; + } +#endif + for (i = ttm->num_pages - 1; i >= 0; i--) drm_cache_flush_page(drm_ttm_get_page(ttm, i)); + DRM_MEMORYBARRIER(); } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/