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: <d4ba49bf-60fa-4c5e-a1a8-52b1668b3d30@broadcom.com>
Date: Wed, 11 Sep 2024 15:19:10 -0700
From: Florian Fainelli <florian.fainelli@...adcom.com>
To: Jim Quinlan <james.quinlan@...adcom.com>
Cc: linux-serial@...r.kernel.org, Kevin Cernekee <cernekee@...il.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Jiri Slaby <jirislaby@...nel.org>, John Ogness <john.ogness@...utronix.de>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 Thomas Gleixner <tglx@...utronix.de>,
 "open list:TTY LAYER AND SERIAL DRIVERS" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tty: rp2: Fix reset with non forgiving PCIe host bridges

On 9/11/24 15:16, Jim Quinlan wrote:
> On Wed, Sep 11, 2024 at 6:01 PM Florian Fainelli
> <florian.fainelli@...adcom.com> wrote:
>>
>> On 9/11/24 14:47, Jim Quinlan wrote:
>>> On Fri, Sep 6, 2024 at 6:54 PM Florian Fainelli
>>> <florian.fainelli@...adcom.com> wrote:
>>>>
>>>> The write to RP2_GLOBAL_CMD followed by an immediate read of
>>>> RP2_GLOBAL_CMD in rp2_reset_asic() is intented to flush out the write,
>>>> however by then the device is already in reset and cannot respond to a
>>>> memory cycle access.
>>>>
>>>> On platforms such as the Raspberry Pi 4 and others using the
>>>> pcie-brcmstb.c driver, any memory access to a device that cannot respond
>>>> is met with a fatal system error, rather than being substituted with all
>>>> 1s as is usually the case on PC platforms.
>>>>
>>>> Swapping the delay and the read ensures that the device has finished
>>>> resetting before we attempt to read from it.
>>>>
>>>> Fixes: 7d9f49afa451 ("serial: rp2: New driver for Comtrol RocketPort 2 cards")
>>>> Suggested-by: Jim Quinlan <james.quinlan@...adcom.com>
>>>> Signed-off-by: Florian Fainelli <florian.fainelli@...adcom.com>
>>>> ---
>>>>    drivers/tty/serial/rp2.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/tty/serial/rp2.c b/drivers/tty/serial/rp2.c
>>>> index 4132fcff7d4e..8bab2aedc499 100644
>>>> --- a/drivers/tty/serial/rp2.c
>>>> +++ b/drivers/tty/serial/rp2.c
>>>> @@ -577,8 +577,8 @@ static void rp2_reset_asic(struct rp2_card *card, unsigned int asic_id)
>>>>           u32 clk_cfg;
>>>>
>>>>           writew(1, base + RP2_GLOBAL_CMD);
>>>> -       readw(base + RP2_GLOBAL_CMD);
>>>>           msleep(100);
>>>> +       readw(base + RP2_GLOBAL_CMD);
>>>
>>> Since the assumed purpose of the readw() was to flush the writew(),
>>> would it make sense to add a barrier after the writew()?
>>
>> AFAICT there is one which is implied within the name, as it is not a
>> _relaxed() variant. Did you mean a different sort of barrier to be used?
> 
> Not sure.  The __raw_writew() is followed by __io_aw(), which is a
> no-op on arm64.  I don't know arm64 well enough to know if a follow-up
> barrier is needed.

By definition all of the {read,write}{b,w,l,q} do include an adequate 
barrier and perform the adequate endian swapping since they originated 
from PCI drivers on x86. If you do not want any barrier you would have 
to use the _relaxed variants, or if you want native ordering, you would 
use the __raw_* variant.
-- 
Florian

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ