Date:	Thu, 21 Jul 2011 13:14:43 +0800
From:	Huang Ying <ying.huang@...el.com>
To:	Bjorn Helgaas <bhelgaas@...gle.com>
CC:	Len Brown <lenb@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Andi Kleen <andi@...stfloor.org>,
	"Luck, Tony" <tony.luck@...el.com>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	Matthew Garrett <mjg@...hat.com>
Subject: Re: [BUGFIX] ACPI, APEI, EINJ Param support is disabled by default

On 07/21/2011 12:46 PM, Bjorn Helgaas wrote:
> On Wed, Jul 20, 2011 at 7:02 PM, Huang Ying <ying.huang@...el.com> wrote:
>> On 07/20/2011 11:22 PM, Bjorn Helgaas wrote:
>>> On Wed, Jul 20, 2011 at 2:09 AM, Huang Ying <ying.huang@...el.com> wrote:
>>>> EINJ parameter support is only usable for some specific BIOS.
>>>> Originally, it is expected to have no harm for BIOS does not support
>>>> it.  But now, we found it will cause issue (memory overwriting) for
>>>> some BIOS.  So param support is disabled by default and only enabled
>>>> when newly added module parameter named "param_extension" is
>>>> explicitly specified.
>>>
>>> Adding parameters always makes things harder for users.  Is there any
>>> way this could be done with a whitelist or other automatic mechanism?
>>
>> The only user of EINJ is RAS developer/tester.  So we adopt a simpler
>> solution.
>>
>>> Per 6e320ec1d98 (which added EINJ parameter support), parameters are
>>> an unpublished extension and are not part of the ACPI spec.  So if we
>>> pick up an MMIO address from a SET_ERROR_TYPE WRITE_REGISTER
>>> instruction and blindly fill in a structure (undefined by the spec)
>>> presumed to be at that address, it doesn't seem surprising that things
>>> will blow up on BIOSes that don't expect that behavior.  After all,
>>> the spec says to write to a generic address structure (not an
>>> einj_parameter structure) when we interpret the WRITE_REGISTER
>>> instruction (not at the magic time between SET_ERROR_TYPE and
>>> EXECUTE_OPERATION).
>>>
>>> If EINJ parameter support is ever added to the ACPI spec, I would
>>> expect a new EINJ flag bit or similar indication to be added at the
>>> same time, so the OS would know when to use it.
>>
>> EINJ parameter support has not been added to ACPI spec, it may be added
>> in the future, and should has some way to indicate whether parameter is
>> supported.
>>
>>> Can you add a whitelist of BIOSes that do support this extension?
>>> Maybe you could define a GUID that future platforms could supply so
>>> you wouldn't have to update the whitelist for every new platform that
>>> supports it?
>>
>> IMHO, the EINJ will not be used by the ordinary users.  If so, can we
>> use a simpler solution such as that in this patch?
> 
> It's not very clear to the code reader when this extension might be
> useful.  It's a bit of folklore that will have to be passed around via
> word-of-mouth.  If I want to modify the parameter code, I have no
> clues about what sort of machine I'd need to test it.
> 
> What happens if you load this module on a machine that supports the
> extension, but you don't supply the module parameter?  I would expect
> problems in that case, too, because the BIOS is expecting parameters
> and we won't fill them in.

On my testing machine with parameter extension, if no injecting
parameter is specified, some BIOS specified default parameter will be used.

> But maybe you're right that this is so specialized that we shouldn't
> over-engineer it.

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

Powered by blists - more mailing lists