Re: [GIT PULL] x86 setup BIOS workarounds

Re: [GIT PULL] x86 setup BIOS workarounds

[Len Brown]

> +		/* ACPI 3.0 added the extended flags support.  If bit 0
> +		   in the extended flags is zero, we're supposed to simply
> +		   ignore the entry -- a backwards incompatible change! */
> +		if (size > 20 && !(buf.ext_flags & 1))
> +			continue;


At the risk of rushing to the defense of the ACPI spec...

This does not look like a backwards incompatible change to me.

In ACPI 2.0, size of 20 is always returned, and it would
be a Linux bug if we examined the undefined values after byte 19.

In ACPI 3.0, byte 20 is now defined.  So if the BIOS returns
a size >= 21, we are permitted to examine byte 20.

So I agree with the test above, but I do not agree with the comment.

thanks,
Len Brown, Intel Open Source Technology Center

Re: [GIT PULL] x86 setup BIOS workarounds

[Len Brown]

On Wed, 1 Apr 2009, Linus Torvalds wrote:

> 
> 
> On Wed, 1 Apr 2009, H. Peter Anvin wrote:
> > 
> > implements handling for the backwards-incompatible(!) E820 handling in 
> > ACPI 3.
> 
> I am _extremely_ nervous about this one.
> 
> You do
> 
> 	size = sizeof buf;	/* ACPI-3 size */
> 	asm(.. "+c" (size)	/* size might change */
> 	..
> 	if (size > 20 && !(buf.ext_flags & 1))
> 		continue;
> 
> ie you are expecting that _all_ old pre-ACPI-3 BIOSES will always set size 
> to 20, or always write a low-bit-set value to that extended flag field 
> that doesn't even exist previously.

Yes, this expects old BIOS to always return 20.
No, it does not expect old BIOS to have any particular value
in buf.ext_flags -- since that is examined only for size > 20.

> I don't think that's likely true. Quite frankly, I'd expect a number of 
> BIOSen to entirely ignore %ecx, since it's irrelevant (it _has_ to be 
> bigger than 20 anyway on entry, and I doubt anybody really ever bothered 
> to test that it's 20 on exit).
> 
> So at a _minimum_, I'd suggest that we set bug.ext_flags to 1 before the 
> call - so that if some random BIOS just leaves %ecx unchanged, it won't 
> mean that the area just gets ignored as a ACPI-3 entry.

Good idea.

Len Brown, Intel Open Source Technology Center

Re: [GIT PULL] x86 setup BIOS workarounds

[Linus Torvalds]

On Thu, 2 Apr 2009, Len Brown wrote:
> 
> Yes, this expects old BIOS to always return 20.

Do you have any reason to expect that all BIOS'es are bug-free in this 
area? 

That would be a first.

We already check for other error cases where the BIOS didn't do the right 
thing in other ways in its e820 routine, or clobbered the wrogn registers 
or whatever. Why would you expect that the return value would always be 
ok?

> No, it does not expect old BIOS to have any particular value
> in buf.ext_flags -- since that is examined only for size > 20.

The point is, that expectation that the BIOS returns 20 seems very 
unreasonable. BIOS writers tend to have been on pain medication for so 
long that they can hardly remember their own name, much less actually make 
sure they follow all the documentation.

Now, if Windows has actually _depended_ on the right return value since 
Win95, that would be a good, strong argument.

Because that's the only case where we can pretty much depend on BIOS 
writers get things right - if Windows doesn't boot when they get it wrong. 
As far as I can tell, that has always been the only real quality assurance 
for most BIOS'es.

			Linus

Re: [GIT PULL] x86 setup BIOS workarounds

[H. Peter Anvin]

Len Brown wrote:
> 
> At the risk of rushing to the defense of the ACPI spec...
> 
> This does not look like a backwards incompatible change to me.
> 
> In ACPI 2.0, size of 20 is always returned, and it would
> be a Linux bug if we examined the undefined values after byte 19.
> 
> In ACPI 3.0, byte 20 is now defined.  So if the BIOS returns
> a size >= 21, we are permitted to examine byte 20.
> 
> So I agree with the test above, but I do not agree with the comment.
> 

If you look at the details of the ACPI spec, this flag is explicitly 
specified so that the BIOS can always return a fixed number of entries. 
  Now, a clever BIOS could of course skip these entries if the OS only 
requests 20 bytes -- but if it can do that, it could just equally well 
have *always* skipped those entries!  So the flag is useless.

However, the ACPI 3 spec is written so to actively lead people into 
implement things this way, and given BIOS developer track records, they 
would simply truncate the result to 20 bytes if the size passed in is 
20.  This, of course, means that it is now reporting an invalid entry, 
without retaining the information that it is invalid...

	-hpa