linux-kernel - Re: [PATCH linux dev-4.10] drivers/misc: (aspeed-lpc-snoop): Add ast2400 to compat

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAN+5HW3Ban209Yx-e_jSYBRNB27-jD_LMMYy3pjiqJWT9MqQ9A@mail.gmail.com>
Date:   Wed, 5 Jul 2017 10:52:49 -0700
From:   Rob Lippert <roblip@...il.com>
To:     Patrick Venture <venture@...gle.com>
Cc:     Joel Stanley <joel@....id.au>,
        Greg KH <gregkh@...uxfoundation.org>,
        Rick Altherr <raltherr@...gle.com>,
        Arnd Bergmann <arnd@...db.de>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH linux dev-4.10] drivers/misc: (aspeed-lpc-snoop): Add
 ast2400 to compat

On Wed, Jul 5, 2017 at 10:48 AM, Patrick Venture <venture@...gle.com> wrote:
> On Wed, Jul 5, 2017 at 10:33 AM, Rob Lippert <roblip@...il.com> wrote:
>> I checked the datasheets when I wrote this and ast2400 does not have
>> the (undocumented) HICRB register bits 14,15 that enables the BMC to
>> actually respond to the snoop'ed address.
>
> You're right, it is marked as "reserved" in the datasheet for the ast2400.
>
>>
>> Without setting that bit in the ast2500 the transactions to that I/O
>> port would timeout on the host side (although the BMC snoop logic
>> would still see it and log it).
>> Probably not an issue for x86 systems that don't have any LPC I/O
>> error handling anyways but LPC timeouts causes issues with POWER
>> systems since it sets a hardware FIR bit which can cause boot
>> failures.
>
> Interesting.  I've been running experiments on x86 and I haven't seen
> any errors, so that adds more credence to your point.  If a device
> doesn't respond within X time, three times in a row, you get a triple
> fault.  But, on x86, I don't think I've seen any mechanism with an
> expectation that a port IO write will have a guaranteed response.
>
> For the use-case I'm chasing, my goal being to snoop PoST codes from
> the host, there is in the datasheet a post-code control register set,
> but I haven't explored configuring them or whether someone has written
> the fifo driver for them.
>
>>
>> -Rob
>>
>> On Tue, Jul 4, 2017 at 8:45 AM, Patrick Venture <venture@...gle.com> wrote:
>>> This driver can be used on the aspeed ast2400.
>>>
>>> Tested: ast2400 on quanta-q71l
>>>
>>> Signed-off-by: Patrick Venture <venture@...gle.com>
>>> ---
>>>  drivers/misc/aspeed-lpc-snoop.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/misc/aspeed-lpc-snoop.c b/drivers/misc/aspeed-lpc-snoop.c
>>> index 593905565b74..0647cff6280a 100644
>>> --- a/drivers/misc/aspeed-lpc-snoop.c
>>> +++ b/drivers/misc/aspeed-lpc-snoop.c
>>> @@ -241,6 +241,7 @@ static int aspeed_lpc_snoop_remove(struct platform_device *pdev)
>>>
>>>  static const struct of_device_id aspeed_lpc_snoop_match[] = {
>>>         { .compatible = "aspeed,ast2500-lpc-snoop" },
>>> +       { .compatible = "aspeed,ast2400-lpc-snoop" },
>>>         { },
>
> An approach would be to ditch this change and instead refer to the
> ast2500-lpc-snoop in my device-tree to avoid anyone non-x86 from
> running this configuration and hitting issues.

This change is probably fine since the driver does still work but you
should also guard the setting of the HICRB bits with #ifdef
MACH_ASPEED_G5 or similar to avoid setting reserved bits on the G4
hardware.

-Rob