public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "H. Peter Anvin" <hpa@zytor.com>
To: Huang Ying <ying.huang@intel.com>
Cc: Cliff Wickman <cpw@sgi.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"mingo@elte.hu" <mingo@elte.hu>,
	"yinghai@kernel.org" <yinghai@kernel.org>
Subject: Re: [PATCH] x86: efi/e820 table merge fix
Date: Tue, 16 Jun 2009 21:03:32 -0700	[thread overview]
Message-ID: <4A386B14.8030702@zytor.com> (raw)
In-Reply-To: <1245203099.11965.7.camel@yhuang-dev.sh.intel.com>

Huang Ying wrote:
>>> Why does BIOS mark memory region without EFI_MEMORY_WB as these types?
>>> Any example?
>>>
>> Probably not, but if it does, it's broken, and the memory should be
>> ignored. The original code had the EFI_MEMORY_WB check already, so it
>> seems prudent to keep it.
> 
> Maybe we need a real life example for that "fix". And attribute that to
> the vendor in comments.
> 
> Best Regards,
> Huang Ying

I think you're reading the patch backwards.

Before the patch, the EFI code didn't look at the type *AT ALL*, it only
looked at the EFI_MEMORY_WB attribute.  This broke for SGI when they
were -- correctly -- reserving real memory (and hence still
EFI_MEMORY_WB) with the type set to EFI_RESERVED_TYPE.  This is correct
behavior, but the old code saw that it was EFI_MEMORY_WB and therefore
considered it usable RAM.  This is obviously broken.

Now why, you're asking, do we still look at md->attribute at all?
That's where caution dictates that it is prudent to diverge from the
previous behavior, but it is not *this* patch that should be the source
of that question, but from the author of the existing code, which
appears to be Paul Jackson of SGI.  Unfortunately, his email now bounces
and noone has that information.

If you think about it, though, we don't want to consider it as usable
RAM if it isn't EFI_MEMORY_WB, and it would in fact be a bug (or
workaround for a broken system) to ignore it.  In fact, we go through
great pains elsewhere in the kernel to remove memory which isn't WB from
the usable pool.
	
	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


  reply	other threads:[~2009-06-17  4:04 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-16 21:43 [PATCH] x86: efi/e820 table merge fix Cliff Wickman
2009-06-17  1:10 ` Huang Ying
2009-06-17  1:38   ` H. Peter Anvin
2009-06-17  1:44     ` Huang Ying
2009-06-17  4:03       ` H. Peter Anvin [this message]
2009-06-17  5:08         ` Huang Ying
2009-06-17 14:58           ` Cliff Wickman
2009-06-17 18:28             ` H. Peter Anvin
2009-06-17 18:30             ` H. Peter Anvin

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=4A386B14.8030702@zytor.com \
    --to=hpa@zytor.com \
    --cc=cpw@sgi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=ying.huang@intel.com \
    --cc=yinghai@kernel.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