On 09/28/2007 10:27 AM, Linus Torvalds wrote:
>
> On Fri, 28 Sep 2007, H. Peter Anvin wrote:
>> [x86 setup] Correct the SMAP check for INT 0x15, AX=0xe820
>>
>> The e820 probe code was checking %edx, not %eax, for the SMAP
>> signature on return. This worked on *almost* all systems, since %edx
>> still contained SMAP from the call on entry, but on a handful of
>> systems it failed -- plus, we would have missed real mismatches.
>>
>> Signed-off-by: H. Peter Anvin <[EMAIL PROTECTED]>
>>
>> diff --git a/arch/i386/boot/memory.c b/arch/i386/boot/memory.c
>> index bccaa1c..2f37568 100644
>> --- a/arch/i386/boot/memory.c
>> +++ b/arch/i386/boot/memory.c
>> @@ -28,11 +28,10 @@ static int detect_memory_e820(void)
>>
>> do {
>> size = sizeof(struct e820entry);
>> - id = SMAP;
>> asm("int $0x15; setc %0"
>> - : "=am" (err), "+b" (next), "+d" (id), "+c" (size),
>> + : "=dm" (err), "+b" (next), "=a" (id), "+c" (size),
>> "=m" (*desc)
>> - : "D" (desc), "a" (0xe820));
>> + : "D" (desc), "d" (SMAP), "a" (0xe820));
>
> Hmm. If I read this correctly, I don't think this can be right.
>
> Why? You don't mark %edx as possibly corrupted by the asm any more.
>
> The "=dm" means that quite often (probably effectively always), gcc will
> allocate %edx to be the output register for %0, but at least in theory, it
> could easily decide that it's going to put %0 in memory, and in that case,
> it may well decide that %edx is not modified by the asm statement. Which
> may or may not be true - I'd bet that there are BIOSes out there that *do*
> modify it.
>
> So what happens then? If gcc decides that %edx isn't modified by the asm,
> it will assume that it still contains the value it had on entry, which is
> the "SMAP" value, and then it might decide to do the
>
> if (id != SMAP) {
>
> check as a
>
> cmpl %edx,%eax
>
> since the "id" return is in %eax, and the compiler decides that it may be
> cheaper to re-use the register that already contains the constant, than to
> use a (longer) compare instruction with an explicit constant.
>
> IOW, I think you need to either (a) _force_ gcc to use %edx for the "err"
> return, avoiding this issue, or (b) mark edx clobbered (which in turn
> means that you need to remove it from the output constraint for "err"). I
> suspect (a) is simpler/more straightforward.
>
Patch with option (a) applied [output 0 changed to: "=d" (err)] tested and
works on the Dell XPS M1330 that was broken by the previous e820 change.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
