| 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
| ||
|
Message-ID: <00f601d8dc71$f0ded460$d29c7d20$@trustnetic.com>
Date: Mon, 10 Oct 2022 14:31:33 +0800
From: Jiawen Wu <jiawenwu@...stnetic.com>
To: "'Andrew Lunn'" <andrew@...n.ch>
Cc: <netdev@...r.kernel.org>, <mengyuanlou@...-swift.com>
Subject: RE: [PATCH net-next v3 2/3] net: txgbe: Reset hardware
On Friday, September 30, 2022 12:46 AM, Andrew Lunn wrote:
> On Thu, Sep 29, 2022 at 05:34:23PM +0800, Jiawen Wu wrote:
> > Reset and initialize the hardware by configuring the MAC layer.
> >
> > Signed-off-by: Jiawen Wu <jiawenwu@...stnetic.com>
> > ---
> > drivers/net/ethernet/wangxun/libwx/wx_hw.c | 160 ++++++++++++++++++
> > drivers/net/ethernet/wangxun/libwx/wx_hw.h | 2 +
> > drivers/net/ethernet/wangxun/libwx/wx_type.h | 144 ++++++++++++++++
> > drivers/net/ethernet/wangxun/txgbe/Makefile | 3 +-
> > drivers/net/ethernet/wangxun/txgbe/txgbe.h | 5 +-
> > drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c | 86 ++++++++++
> > drivers/net/ethernet/wangxun/txgbe/txgbe_hw.h | 9 +
> > .../net/ethernet/wangxun/txgbe/txgbe_main.c | 21 +++
> > .../net/ethernet/wangxun/txgbe/txgbe_type.h | 11 +-
> > 9 files changed, 432 insertions(+), 9 deletions(-) create mode
> > 100644 drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c
> > create mode 100644 drivers/net/ethernet/wangxun/txgbe/txgbe_hw.h
> >
> > diff --git a/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> > b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> > index fed51c2f3071..76f88cfb2476 100644
> > --- a/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> > +++ b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> > @@ -7,6 +7,21 @@
> > #include "wx_type.h"
> > #include "wx_hw.h"
> >
> > +static void wx_intr_disable(struct wx_hw *wxhw, u64 qmask) {
> > + u32 mask;
> > +
> > + mask = (qmask & 0xFFFFFFFF);
> > + if (mask)
> > + wr32(wxhw, WX_PX_IMS(0), mask);
> > +
> > + if (wxhw->mac.type == wx_mac_sp) {
> > + mask = (qmask >> 32);
> > + if (mask)
> > + wr32(wxhw, WX_PX_IMS(1), mask);
> > + }
> > +}
> > +
> > /* cmd_addr is used for some special command:
> > * 1. to be sector address, when implemented erase sector command
> > * 2. to be flash address when implemented read, write flash address
> > @@ -56,6 +71,151 @@ int wx_check_flash_load(struct wx_hw *hw, u32
> > check_bit) } EXPORT_SYMBOL(wx_check_flash_load);
> >
> > +static void wx_disable_rx(struct wx_hw *wxhw) {
> > + u32 pfdtxgswc;
> > + u32 rxctrl;
> > +
> > + rxctrl = rd32(wxhw, WX_RDB_PB_CTL);
> > + if (rxctrl & WX_RDB_PB_CTL_RXEN) {
> > + pfdtxgswc = rd32(wxhw, WX_PSR_CTL);
> > + if (pfdtxgswc & WX_PSR_CTL_SW_EN) {
> > + pfdtxgswc &= ~WX_PSR_CTL_SW_EN;
> > + wr32(wxhw, WX_PSR_CTL, pfdtxgswc);
> > + wxhw->mac.set_lben = true;
> > + } else {
> > + wxhw->mac.set_lben = false;
> > + }
> > + rxctrl &= ~WX_RDB_PB_CTL_RXEN;
> > + wr32(wxhw, WX_RDB_PB_CTL, rxctrl);
> > +
> > + if (!(((wxhw->subsystem_device_id & WX_NCSI_MASK) == WX_NCSI_SUP) ||
> > + ((wxhw->subsystem_device_id & WX_WOL_MASK) == WX_WOL_SUP))) {
> > + /* disable mac receiver */
> > + wr32m(wxhw, WX_MAC_RX_CFG,
> > + WX_MAC_RX_CFG_RE, 0);
> > + }
> > + }
> > +}
> > +
> > +/**
> > + * wx_disable_pcie_master - Disable PCI-express master access
> > + * @wxhw: pointer to hardware structure
> > + *
> > + * Disables PCI-Express master access and verifies there are no
> > +pending
> > + * requests.
> > + **/
> > +static int wx_disable_pcie_master(struct wx_hw *wxhw) {
> > + int status = 0;
> > + u32 val;
> > +
> > + /* Always set this bit to ensure any future transactions are blocked */
> > + pci_clear_master(wxhw->pdev);
> > +
> > + /* Exit if master requests are blocked */
> > + if (!(rd32(wxhw, WX_PX_TRANSACTION_PENDING)))
> > + return 0;
> > +
> > + /* Poll for master request bit to clear */
> > + status = read_poll_timeout(rd32, val, !val, 100, WX_PCI_MASTER_DISABLE_TIMEOUT,
> > + false, wxhw, WX_PX_TRANSACTION_PENDING);
> > + if (status < 0)
> > + wx_err(wxhw, "PCIe transaction pending bit did not clear.\n");
> > +
> > + return status;
> > +}
> > +
> > +/**
> > + * wx_stop_adapter - Generic stop Tx/Rx units
> > + * @hw: pointer to hardware structure
> > + *
> > + * Sets the adapter_stopped flag within wx_hw struct. Clears
> > +interrupts,
> > + * disables transmit and receive units. The adapter_stopped flag is
> > +used by
> > + * the shared code and drivers to determine if the adapter is in a
> > +stopped
> > + * state and should not touch the hardware.
> > + **/
> > +int wx_stop_adapter(struct wx_hw *wxhw) {
> > + u16 i;
> > +
> > + /* Set the adapter_stopped flag so other driver functions stop touching
> > + * the hardware
> > + */
> > + wxhw->adapter_stopped = true;
> > +
> > + /* Disable the receive unit */
> > + wx_disable_rx(wxhw);
> > +
> > + /* Set interrupt mask to stop interrupts from being generated */
> > + wx_intr_disable(wxhw, WX_INTR_ALL);
> > +
> > + /* Clear any pending interrupts, flush previous writes */
> > + wr32(wxhw, WX_PX_MISC_IC, 0xffffffff);
> > + wr32(wxhw, WX_BME_CTL, 0x3);
> > +
> > + /* Disable the transmit unit. Each queue must be disabled. */
> > + for (i = 0; i < wxhw->mac.max_tx_queues; i++) {
> > + wr32m(wxhw, WX_PX_TR_CFG(i),
> > + WX_PX_TR_CFG_SWFLSH | WX_PX_TR_CFG_ENABLE,
> > + WX_PX_TR_CFG_SWFLSH);
> > + }
> > +
> > + /* Disable the receive unit by stopping each queue */
> > + for (i = 0; i < wxhw->mac.max_rx_queues; i++) {
> > + wr32m(wxhw, WX_PX_RR_CFG(i),
> > + WX_PX_RR_CFG_RR_EN, 0);
> > + }
> > +
> > + /* flush all queues disables */
> > + WX_WRITE_FLUSH(wxhw);
>
> Please don't hide memory barriers like this. Memory barriers are hard, so you want them in plain
view, so
> you can understand them.
>
> > +/* flush PCI read and write */
> > +#define WX_WRITE_FLUSH(H) rd32(H, WX_MIS_PWR)
>
> I don't think you actually need to do anything here. rd32 is using
> readl():
>
> static inline u32 readl(const volatile void __iomem *addr) {
> u32 val;
>
> __io_br();
> val = __le32_to_cpu((__le32 __force)__raw_readl(addr));
> __io_ar(val);
> return val;
> }
>
> So you have an IO barrier before and a read barrier afterwards. So all i think you need is a
mb(), not a
> full rd32().
>
> Andrew
>
I think we need a readl(), because there are problems that sometimes IO is not synchronized with
flushing memory on some domestic cpu platforms.
It can become a serious problem, causing register error configurations.
Powered by blists - more mailing lists