public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: "H. Peter Anvin" <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@kernel.org>, Ingo Molnar <mingo@elte.hu>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [Intel-gfx] Ugly patches for stolen reservation
Date: Fri, 26 Jul 2013 19:52:52 +0200	[thread overview]
Message-ID: <20130726175252.GG6084@phenom.ffwll.local> (raw)
In-Reply-To: <51F1C76A.8020407@zytor.com>

On Thu, Jul 25, 2013 at 05:48:42PM -0700, H. Peter Anvin wrote:
> On 07/25/2013 05:31 PM, Linus Torvalds wrote:
> > On Thu, Jul 25, 2013 at 3:42 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> >> So the bootloader is just as likely to step on things... what happens when/if it does?
> > 
> > This isn't a new problem. We've had this "firmware tables don't show
> > all devices" issue before.
> > 
> 
> Yes, I just want to know what happens.
> 
> > The only odd thing about this one is how the quirk in question uses
> > "e820_add_region()" instead of just adding things to the MMIO list.
> > And I think that's actually likely a mistake.
> > 
> > So Jesse, why don't you do what the other quirks do, and claim an
> > actual MMIO resource? If you make it a real resource, you'll get to
> > use fancy things like REAL NAMES, and actually document it. With
> > human-readable strings.
> > 
> > See quirk_io_region() in drivers/pci/quirks.c for example. The same
> > code except for IORESOURCE_MEM should do a lovely job..
> > 
> > And even *if* it's already marked reserved in the e820 table, it just
> > looks nice in /proc/iomem.
> 
> We should do both -- mark it reserved in early boot, and add it as an
> MMIO region later during boot.

We started to reserve this range in drm/i915 (and still do), that's why
we've noticed that something is amiss.
> 
> The problem here, if I'm reading this right, is that this memory region
> is marked as normal RAM in e820, which is much worse than just not
> marking it as reserved; we need to intercept this memory before we
> genuinely turn it into normal RAM.

I haven't seen a case where it's marked as ram, but many systems don't
mark it as reserved. Then the pci code can go around and put a bar into
the middle of that range. The other issue (benign but ugly) is that the
RAM buffer (used to protect stolen memory without knowing where exactly it
is because it's not properly reserved in the e820 map) sometimes ends up
in the graphics stolen range. With the new iomeme reserve code in drm/i915
we've stopped using stolen in such cases to avoid surprises, but that
kills  a few neat features.

Thus far we haven't tracked down any bugs to such overlaps yet, but otoh
we don't use stolen all that much yet. But we have plans to put it to real
use (and use the full range) and I'd like to get the reservation conflicts
sorted out before we do so.

> At the same time, we have no protection against the bootloader using
> this as memory or even placing the kernel there.  The BIOS needs to be
> fixed regardless of what workarounds we do in the kernel.

Like I've said we haven't seen it earmarked as ram anywhere yet, so I
don't think this is something we need to concern ousrselves with.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

  reply	other threads:[~2013-07-26 17:52 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-25 16:37 Ugly patches for stolen reservation Jesse Barnes
2013-07-25 16:37 ` [PATCH 1/2] drm/i915: split PCI IDs out into i915_drm.h v3 Jesse Barnes
2013-07-25 23:06   ` Chris Wilson
2013-07-25 16:37 ` [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3 Jesse Barnes
2013-07-25 22:42   ` Chris Wilson
2013-07-25 22:59     ` Chris Wilson
2013-07-25 23:18       ` Jesse Barnes
2013-07-26  8:10   ` [Intel-gfx] " Daniel Vetter
2013-07-26 16:58   ` H. Peter Anvin
2013-07-26 17:12     ` Jesse Barnes
2013-07-25 20:05 ` Ugly patches for stolen reservation Ingo Molnar
2013-07-25 20:16   ` Jesse Barnes
2013-07-25 22:10     ` [Intel-gfx] " Daniel Vetter
2013-07-25 22:42   ` H. Peter Anvin
2013-07-25 23:17     ` Jesse Barnes
2013-07-26  0:49       ` H. Peter Anvin
2013-07-26  0:31     ` Linus Torvalds
2013-07-26  0:48       ` H. Peter Anvin
2013-07-26 17:52         ` Daniel Vetter [this message]
2013-07-26 15:51       ` Jesse Barnes
     [not found] <rddfy5e894xuv6d47gu7hbuh.1374804892449@email.android.com>
2013-07-26  3:31 ` H. Peter Anvin
2013-07-26 15:33   ` Jesse Barnes
2013-07-26 20:24     ` Ingo Molnar
2013-07-26 20:28       ` H. Peter Anvin
2013-07-26 20:45         ` [Intel-gfx] " Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130726175252.GG6084@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=hpa@zytor.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mingo@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox