| 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
| ||
|
Message-ID: <20150327233137.GP5622@wotan.suse.de>
Date: Sat, 28 Mar 2015 00:31:37 +0100
From: "Luis R. Rodriguez" <mcgrof@...e.com>
To: Andy Lutomirski <luto@...capital.net>
Cc: "Luis R. Rodriguez" <mcgrof@...not-panic.com>,
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>,
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 27, 2015 at 02:21:34PM -0700, Andy Lutomirski wrote:
> On Fri, Mar 27, 2015 at 1:12 PM, Luis R. Rodriguez <mcgrof@...e.com> wrote:
> > On Fri, Mar 20, 2015 at 04:52:18PM -0700, Andy Lutomirski wrote:
> >> On Fri, Mar 20, 2015 at 4:17 PM, Luis R. Rodriguez
> >> <mcgrof@...not-panic.com> wrote:
> >> > 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.
> >
> > Precicely, in this case the WC hole was obtained by using MTRR WC. This
> > patch removes that WC hole trick and now we can be explciit about
> > only wanting ioremap_nocache() on the registers, that is WC is not
> > desired here and is not used. The patch does not highlight the fact
> > that there was left in place another ioremap() call for the framebuffer:
> >
> > info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len);
> >
> > That is the one that later after this patch we use ioremap_wc() for.
> > This patch just removes the hole solution. That's all.
> >
>
> I don't understand.
>
> If I read it right, there's a 2^n byte BAR. You're requesting WC for
> the whole think using arch_phys_wc_add.
I believe there is a misunderstanding of order of changes.
Let's split when we use mtrr_add() Vs arch_phys_wc_add() to avoid
confusion as in this patch we don't yet use arch_phys_wc_add(). That
is done in the next patch.
The commit log describes best the state of affairs prior to this
patch:
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.
Then this patch drops the MTRR_TYPE_UNCACHABLE rewrite thing
and instead corrects the ioremap() call for the framebuffer
to only be called for the framebuffer alone. For the MMIO
area we adjust to use then ioremap_nocache(). The MTRR left
is now only *for the framebuffer* and it should not be touching
the MMIO area. So the MMIO area has its own ioremap_nocache()
area alone, the framebuffer is left with an ioremap() followed
by an mtrr_add() call.
The next patch replaces the mtrr_add() with arch_phys_wc_add()
and then also uses ioremap_wc().
> On a PAT system that has no effect and all is well.
Yeah we're not doing arch_phys_wc_add() on the entire PCI BAR.
That was dumb, this fixes that, and on this patch mtrr_add()
is still used.
> On a non-PAT system, it adds an MTRR. That
> means that you need to override the MTRR somehow for the mmio regs,
> and UC- won't do the trick.
We don't need to solve that problem here as the MTRR should only
be for the framebuffer.
Luis
--
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