[Intel-wired-lan] [PATCH net 2/2] net: ixgbe: Use new IXGBE_FLAG2_ROOT_NO_RELAXED_ORDERING flag

Alexander Duyck alexander.duyck at gmail.com
Wed Aug 16 18:17:29 UTC 2017


On Wed, Aug 16, 2017 at 2:41 AM, Ding Tianhong <dingtianhong at huawei.com> wrote:
> The ixgbe driver use the compile check to determine if it can
> send TLPs to Root Port with the Relaxed Ordering Attribute set,
> this is too inconvenient, now the new flag PCI_DEV_FLAGS_NO_RELAXED_ORDERING
> has been added to the kernel and we could check the bit4 in the PCIe
> Davice Control register to determine whether we should use the Relaxed
> Ordering Attributes or not, so we add a new flag which called
> IXGBE_FLAG2_ROOT_NO_RELAXED_ORDERING to the ixgbe driver, it will
> be set if the Root Port couldn't deal the upstream TLPs with Relaxed
> Ordering Attribute, then the driver could know what to do next.
>
> Signed-off-by: Ding Tianhong <dingtianhong at huawei.com>

I would say this is a good start but you probably don't even need the
IXGBE_FLAG2 bit. The hardware will honor the PCIe configuration space.
So I would just say you should probably drop the checks all together
and just strip the #ifdef CONFIG_SPARC bits. It should make the
resultant patch quite small and have the exact same effect.

> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe.h        |  1 +
>  drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c  | 37 ++++++++++++-------------
>  drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 32 +++++++++++----------
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c   | 17 ++++++++++++
>  4 files changed, 53 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> index dd55787..50e0553 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> @@ -621,6 +621,7 @@ struct ixgbe_adapter {
>  #define IXGBE_FLAG2_EEE_CAPABLE                        BIT(14)
>  #define IXGBE_FLAG2_EEE_ENABLED                        BIT(15)
>  #define IXGBE_FLAG2_RX_LEGACY                  BIT(16)
> +#define IXGBE_FLAG2_ROOT_NO_RELAXED_ORDERING   BIT(17)
>
>         /* Tx fast path data */
>         int num_tx_queues;
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
> index 523f9d0..0727a30 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
> @@ -175,31 +175,30 @@ static s32 ixgbe_init_phy_ops_82598(struct ixgbe_hw *hw)
>   **/
>  static s32 ixgbe_start_hw_82598(struct ixgbe_hw *hw)
>  {
> -#ifndef CONFIG_SPARC
> -       u32 regval;
> -       u32 i;
> -#endif
> +       u32 regval, i;
>         s32 ret_val;
> +       struct ixgbe_adapter *adapter = hw->back;
>
>         ret_val = ixgbe_start_hw_generic(hw);
>
> -#ifndef CONFIG_SPARC
> -       /* Disable relaxed ordering */
> -       for (i = 0; ((i < hw->mac.max_tx_queues) &&
> -            (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
> -               regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL(i));
> -               regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
> -               IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL(i), regval);
> -       }
> +       if (adapter->flags2 & IXGBE_FLAG2_ROOT_NO_RELAXED_ORDERING) {
> +               /* Disable relaxed ordering */
> +               for (i = 0; ((i < hw->mac.max_tx_queues) &&
> +                    (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
> +                       regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL(i));
> +                       regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
> +                       IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL(i), regval);
> +               }
>
> -       for (i = 0; ((i < hw->mac.max_rx_queues) &&
> -            (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
> -               regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
> -               regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
> -                           IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
> -               IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
> +               for (i = 0; ((i < hw->mac.max_rx_queues) &&
> +                    (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
> +                       regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
> +                       regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
> +                                   IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
> +                       IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
> +               }
>         }
> -#endif
> +
>         if (ret_val)
>                 return ret_val;
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> index d4933d2..2473c0b 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> @@ -342,6 +342,7 @@ s32 ixgbe_start_hw_generic(struct ixgbe_hw *hw)
>  s32 ixgbe_start_hw_gen2(struct ixgbe_hw *hw)
>  {
>         u32 i;
> +       struct ixgbe_adapter *adapter = hw->back;
>
>         /* Clear the rate limiters */
>         for (i = 0; i < hw->mac.max_tx_queues; i++) {
> @@ -350,25 +351,26 @@ s32 ixgbe_start_hw_gen2(struct ixgbe_hw *hw)
>         }
>         IXGBE_WRITE_FLUSH(hw);
>
> -#ifndef CONFIG_SPARC
> -       /* Disable relaxed ordering */
> -       for (i = 0; i < hw->mac.max_tx_queues; i++) {
> -               u32 regval;
> +       if (adapter->flags2 & IXGBE_FLAG2_ROOT_NO_RELAXED_ORDERING) {
> +               /* Disable relaxed ordering */
> +               for (i = 0; i < hw->mac.max_tx_queues; i++) {
> +                       u32 regval;
>
> -               regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL_82599(i));
> -               regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
> -               IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL_82599(i), regval);
> -       }
> +                       regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL_82599(i));
> +                       regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
> +                       IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL_82599(i), regval);
> +               }
>
> -       for (i = 0; i < hw->mac.max_rx_queues; i++) {
> -               u32 regval;
> +               for (i = 0; i < hw->mac.max_rx_queues; i++) {
> +                       u32 regval;
>
> -               regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
> -               regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
> -                           IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
> -               IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
> +                       regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
> +                       regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
> +                                   IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
> +                       IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
> +               }
>         }
> -#endif
> +
>         return 0;
>  }
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index f1dbdf2..f576be7 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -10081,6 +10081,23 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>                 goto err_ioremap;
>         }
>
> +       /* If possible, we use PCIe Relaxed Ordering Attribute to deliver
> +        * Ingress Packet Data to Free List Buffers in order to allow for
> +        * chipset performance optimizations between the Root Complex and
> +        * Memory Controllers.  (Messages to the associated Ingress Queue
> +        * notifying new Packet Placement in the Free Lists Buffers will be
> +        * send without the Relaxed Ordering Attribute thus guaranteeing that
> +        * all preceding PCIe Transaction Layer Packets will be processed
> +        * first.)  But some Root Complexes have various issues with Upstream
> +        * Transaction Layer Packets with the Relaxed Ordering Attribute set.
> +        * The PCIe devices which under the Root Complexes will be cleared the
> +        * Relaxed Ordering bit in the configuration space, So we check our
> +        * PCIe configuration space to see if it's flagged with advice against
> +        * using Relaxed Ordering.
> +        */
> +       if (!pcie_relaxed_ordering_enabled(pdev))
> +               adapter->flags2 |= IXGBE_FLAG2_ROOT_NO_RELAXED_ORDERING;
> +
>         netdev->netdev_ops = &ixgbe_netdev_ops;
>         ixgbe_set_ethtool_ops(netdev);
>         netdev->watchdog_timeo = 5 * HZ;
> --
> 1.8.3.1
>
>


More information about the Intel-wired-lan mailing list