| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 20 Mar 2015 16:52:18 -0700
From: Andy Lutomirski <luto@...capital.net>
To: "Luis R. Rodriguez" <mcgrof@...not-panic.com>
Cc: Ingo Molnar <mingo@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
"H. Peter Anvin" <hpa@...or.com>, Juergen Gross <jgross@...e.com>,
Jan Beulich <JBeulich@...e.com>, Borislav Petkov <bp@...e.de>,
Suresh Siddha <suresh.b.siddha@...el.com>,
venkatesh.pallipadi@...el.com, Dave Airlie <airlied@...hat.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Linux Fbdev development list <linux-fbdev@...r.kernel.org>,
X86 ML <x86@...nel.org>,
"xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
"Luis R. Rodriguez" <mcgrof@...e.com>, Ingo Molnar <mingo@...e.hu>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Daniel Vetter <daniel.vetter@...ll.ch>,
Antonino Daplas <adaplas@...il.com>,
Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>,
Tomi Valkeinen <tomi.valkeinen@...com>
Subject: Re: [PATCH v1 09/47] vidoe: fbdev: atyfb: remove and fix MTRR MMIO
"hole" work around
On Fri, Mar 20, 2015 at 4:17 PM, Luis R. Rodriguez
<mcgrof@...not-panic.com> wrote:
> From: "Luis R. Rodriguez" <mcgrof@...e.com>
>
> The atyfb driver uses an MTRR work around since some
> cards use the same PCI BAR for the framebuffer and MMIO.
> In such cards the last page is used for MMIO, the rest for
> the framebuffer, so on those cards we ioremap() the MMIO
> page alone, then again ioremap() the full framebuffer
> including the MMIO space *and* ___then___ use an MTRR with
> MTRR_TYPE_WRCOMB on the full PCI BAR... and finally "hole"
> in an MTRR_TYPE_UNCACHABLE MTRR only for MMIO.
>
> This is a terrible fucking work around, and should by no means
> be necessary however evidence through a large series of conversion
> of drivers to ioremap_wc() for the framebuffer shows that around
> the time MTRR started becoming popular devices did not have things
> lined up for easily separating the framebuffer and MMIO register
> access. In some cases a driver requires significant intrusive
> changes in order to make the split for an ioremap() for MMIO registers
> and another ioremap_wc() for the framebuffer, at other times a
> bit of careful study of the driver suffices. This example driver
> falls into the later category.
>
> We can replace the MTRR MTRR_TYPE_UNCACHABLE
> work around by using ioremap_nocache(), the length of the
> MMIO space should already be correct. The other part we
> need to correct is ensuring we ioremap() for the framebuffer
> only the required size. Since the ioremap() happens early
> on probe for PCI devices before aty_init() where we typically
> adjust the length and know how to do it, we can fix this by
> pegging the bus type as PCI on PCI probe, and finally fudging
> and framebuffer length just as we do on aty_init().
>
> The last thing we do must do to remain sane is ensure we
> use the info->fix.smem_start and info->fix.smem_len for
> the framebuffer MTRR as we know that is always well adjusted.
> The *one* concern here would be if the MTRR is not in units
> of 4K __but__ we already know that in the PCI case this cannot
> happen, in the shared space setting the MTRR would be up to
> 0x7ff000 and assuming a 4K page:
>
> ; 0x7ff000 / 0x1000
> 2047
>
> Also, internally when MTRR is used mtrr_add() will use mtrr_check()
> and that should splat a warning when the MTRR base and size are
> not compatible with what is expected for MTRR usage.
>
> This fix lets us nuke the MTRR_TYPE_UNCACHABLE MTRR "hole".
>
> Cc: Suresh Siddha <suresh.b.siddha@...el.com>
> Cc: Venkatesh Pallipadi <venkatesh.pallipadi@...el.com>
> Cc: Ingo Molnar <mingo@...e.hu>
> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Juergen Gross <jgross@...e.com>
> Cc: Daniel Vetter <daniel.vetter@...ll.ch>
> Cc: Andy Lutomirski <luto@...capital.net>
> Cc: Dave Airlie <airlied@...hat.com>
> Cc: Antonino Daplas <adaplas@...il.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@...com>
> Cc: linux-fbdev@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
> Signed-off-by: Luis R. Rodriguez <mcgrof@...e.com>
> ---
> drivers/video/fbdev/aty/atyfb.h | 1 -
> drivers/video/fbdev/aty/atyfb_base.c | 28 ++++++----------------------
> 2 files changed, 6 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/video/fbdev/aty/atyfb.h b/drivers/video/fbdev/aty/atyfb.h
> index 1f39a62..89ec439 100644
> --- a/drivers/video/fbdev/aty/atyfb.h
> +++ b/drivers/video/fbdev/aty/atyfb.h
> @@ -184,7 +184,6 @@ struct atyfb_par {
> spinlock_t int_lock;
> #ifdef CONFIG_MTRR
> int mtrr_aper;
> - int mtrr_reg;
> #endif
> u32 mem_cntl;
> struct crtc saved_crtc;
> diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
> index 8025624..8875e56 100644
> --- a/drivers/video/fbdev/aty/atyfb_base.c
> +++ b/drivers/video/fbdev/aty/atyfb_base.c
> @@ -2630,21 +2630,10 @@ static int aty_init(struct fb_info *info)
>
> #ifdef CONFIG_MTRR
> par->mtrr_aper = -1;
> - par->mtrr_reg = -1;
> if (!nomtrr) {
> - /* Cover the whole resource. */
> - par->mtrr_aper = mtrr_add(par->res_start, par->res_size,
> + par->mtrr_aper = mtrr_add(info->fix.smem_start,
> + info->fix.smem_len,
> MTRR_TYPE_WRCOMB, 1);
> - if (par->mtrr_aper >= 0 && !par->aux_start) {
> - /* Make a hole for mmio. */
> - par->mtrr_reg = mtrr_add(par->res_start + 0x800000 -
> - GUI_RESERVE, GUI_RESERVE,
> - MTRR_TYPE_UNCACHABLE, 1);
> - if (par->mtrr_reg < 0) {
> - mtrr_del(par->mtrr_aper, 0, 0);
> - par->mtrr_aper = -1;
> - }
> - }
> }
> #endif
>
> @@ -2776,10 +2765,6 @@ aty_init_exit:
> par->pll_ops->set_pll(info, &par->saved_pll);
>
> #ifdef CONFIG_MTRR
> - if (par->mtrr_reg >= 0) {
> - mtrr_del(par->mtrr_reg, 0, 0);
> - par->mtrr_reg = -1;
> - }
> if (par->mtrr_aper >= 0) {
> mtrr_del(par->mtrr_aper, 0, 0);
> par->mtrr_aper = -1;
> @@ -3466,7 +3451,7 @@ static int atyfb_setup_generic(struct pci_dev *pdev, struct fb_info *info,
> }
>
> info->fix.mmio_start = raddr;
> - par->ati_regbase = ioremap(info->fix.mmio_start, 0x1000);
> + par->ati_regbase = ioremap_nocache(info->fix.mmio_start, 0x1000);
Double-check me, but I think that ioremap_nocache + WC MTRR = WC. I
think we might need ioremap_nocache_me_harder (or maybe ioremap_x86_uc
if you prefer that bikeshed color) for this.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists