Messages in this thread | | | Subject | Re: [PATCH] x86: efi/e820 table merge fix | From | Huang Ying <> | Date | Wed, 17 Jun 2009 13:08:22 +0800 |
| |
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
| |