Subject: Re: [PATCH] x86: efi/e820 table merge fix
From: Huang Ying <ying.huang@intel.com>
To: "H. Peter Anvin" <hpa@zytor.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>
In-Reply-To: <4A386B14.8030702@zytor.com>
References: <E1MGgRQ-0002pu-6Z@eag09.americas.sgi.com>
	 <1245201005.11965.4.camel@yhuang-dev.sh.intel.com>
	 <4A384914.1090900@zytor.com>
	 <1245203099.11965.7.camel@yhuang-dev.sh.intel.com>
	 <4A386B14.8030702@zytor.com>
Content-Type: text/plain
Date: Wed, 17 Jun 2009 13:08:22 +0800
Message-Id: <1245215302.11965.19.camel@yhuang-dev.sh.intel.com>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2156
Lines: 52

On Wed, 2009-06-17 at 12:03 +0800, H. Peter Anvin wrote:
> 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.

Yes. You are right. Thank you for your patient.

> 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.

Because it appears that checking EFI_MEMORY_WB is not necessary, maybe
it is necessary to add some comments about why it is checked to prevent
it to be deleted later.

Best Regards,
Huang Ying


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/