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: <b4e44b37-8a0a-491d-a248-0b50f6668e17@amd.com>
Date: Mon, 29 Jan 2024 17:16:28 -0800
From: "Nelson, Shannon" <shannon.nelson@....com>
To: darinzon@...zon.com, David Miller <davem@...emloft.net>,
 Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org
Cc: "Woodhouse, David" <dwmw@...zon.com>, "Machulsky, Zorik"
 <zorik@...zon.com>, "Matushevsky, Alexander" <matua@...zon.com>,
 Saeed Bshara <saeedb@...zon.com>, "Wilson, Matt" <msw@...zon.com>,
 "Liguori, Anthony" <aliguori@...zon.com>, "Bshara, Nafea"
 <nafea@...zon.com>, "Belgazal, Netanel" <netanel@...zon.com>,
 "Saidi, Ali" <alisaidi@...zon.com>, "Herrenschmidt, Benjamin"
 <benh@...zon.com>, "Kiyanovski, Arthur" <akiyano@...zon.com>,
 "Dagan, Noam" <ndagan@...zon.com>, "Agroskin, Shay" <shayagr@...zon.com>,
 "Itzko, Shahar" <itzko@...zon.com>, "Abboud, Osama" <osamaabb@...zon.com>,
 "Ostrovsky, Evgeny" <evostrov@...zon.com>, "Tabachnik, Ofir"
 <ofirt@...zon.com>, "Koler, Nati" <nkolder@...zon.com>
Subject: Re: [PATCH v1 net-next 05/11] net: ena: Remove CQ tail pointer update

On 1/29/2024 12:55 AM, darinzon@...zon.com wrote:
> 
> From: David Arinzon <darinzon@...zon.com>
> 
> The functionality was added to allow the drivers to create an
> SQ and CQ of different sizes.
> When the RX/TX SQ and CQ have the same size, such update isn't
> necessary as the device can safely assume it doesn't override
> unprocessed completions. However, if the SQ is larger than the CQ,
> the device might "have" more completions it wants to update about
> than there's room in the CQ.
> 
> There's no support for different SQ and CQ sizes, therefore,
> removing the API and its usage.
> 
> '____cacheline_aligned' compiler attribute was added to
> 'struct ena_com_io_cq' to ensure that the removal of the
> 'cq_head_db_reg' field doesn't change the cache-line layout
> of this struct.
> 
> Signed-off-by: Shay Agroskin <shayagr@...zon.com>
> Signed-off-by: David Arinzon <darinzon@...zon.com>
> ---
>   drivers/net/ethernet/amazon/ena/ena_com.c     |  5 ----
>   drivers/net/ethernet/amazon/ena/ena_com.h     |  5 +---
>   drivers/net/ethernet/amazon/ena/ena_eth_com.h | 24 -------------------
>   drivers/net/ethernet/amazon/ena/ena_netdev.c  |  5 +---
>   drivers/net/ethernet/amazon/ena/ena_xdp.c     |  1 -
>   5 files changed, 2 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
> index 9a8a43b..675ee72 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_com.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_com.c
> @@ -1427,11 +1427,6 @@ int ena_com_create_io_cq(struct ena_com_dev *ena_dev,
>          io_cq->unmask_reg = (u32 __iomem *)((uintptr_t)ena_dev->reg_bar +
>                  cmd_completion.cq_interrupt_unmask_register_offset);
> 
> -       if (cmd_completion.cq_head_db_register_offset)
> -               io_cq->cq_head_db_reg =
> -                       (u32 __iomem *)((uintptr_t)ena_dev->reg_bar +
> -                       cmd_completion.cq_head_db_register_offset);
> -
>          if (cmd_completion.numa_node_register_offset)
>                  io_cq->numa_node_cfg_reg =
>                          (u32 __iomem *)((uintptr_t)ena_dev->reg_bar +
> diff --git a/drivers/net/ethernet/amazon/ena/ena_com.h b/drivers/net/ethernet/amazon/ena/ena_com.h
> index f3176fc..8f90c3c 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_com.h
> +++ b/drivers/net/ethernet/amazon/ena/ena_com.h
> @@ -109,8 +109,6 @@ struct ena_com_io_cq {
>          /* Interrupt unmask register */
>          u32 __iomem *unmask_reg;
> 
> -       /* The completion queue head doorbell register */
> -       u32 __iomem *cq_head_db_reg;

You'll want to remove one of the surrounding blank lines as well so as 
to not end up with multiple blanks in a row.

sln

> 
>          /* numa configuration register (for TPH) */
>          u32 __iomem *numa_node_cfg_reg;
> @@ -118,7 +116,7 @@ struct ena_com_io_cq {
>          /* The value to write to the above register to unmask
>           * the interrupt of this queue
>           */
> -       u32 msix_vector;
> +       u32 msix_vector ____cacheline_aligned;
> 
>          enum queue_direction direction;
> 
> @@ -134,7 +132,6 @@ struct ena_com_io_cq {
>          /* Device queue index */
>          u16 idx;
>          u16 head;
> -       u16 last_head_update;
>          u8 phase;
>          u8 cdesc_entry_size_in_bytes;
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_eth_com.h b/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> index 372b259..4d65d82 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> +++ b/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> @@ -8,8 +8,6 @@
> 
>   #include "ena_com.h"
> 
> -/* head update threshold in units of (queue size / ENA_COMP_HEAD_THRESH) */
> -#define ENA_COMP_HEAD_THRESH 4
>   /* we allow 2 DMA descriptors per LLQ entry */
>   #define ENA_LLQ_ENTRY_DESC_CHUNK_SIZE  (2 * sizeof(struct ena_eth_io_tx_desc))
>   #define ENA_LLQ_HEADER         (128UL - ENA_LLQ_ENTRY_DESC_CHUNK_SIZE)
> @@ -172,28 +170,6 @@ static inline int ena_com_write_sq_doorbell(struct ena_com_io_sq *io_sq)
>          return 0;
>   }
> 
> -static inline int ena_com_update_dev_comp_head(struct ena_com_io_cq *io_cq)
> -{
> -       u16 unreported_comp, head;
> -       bool need_update;
> -
> -       if (unlikely(io_cq->cq_head_db_reg)) {
> -               head = io_cq->head;
> -               unreported_comp = head - io_cq->last_head_update;
> -               need_update = unreported_comp > (io_cq->q_depth / ENA_COMP_HEAD_THRESH);
> -
> -               if (unlikely(need_update)) {
> -                       netdev_dbg(ena_com_io_cq_to_ena_dev(io_cq)->net_device,
> -                                  "Write completion queue doorbell for queue %d: head: %d\n",
> -                                  io_cq->qid, head);
> -                       writel(head, io_cq->cq_head_db_reg);
> -                       io_cq->last_head_update = head;
> -               }
> -       }
> -
> -       return 0;
> -}
> -
>   static inline void ena_com_update_numa_node(struct ena_com_io_cq *io_cq,
>                                              u8 numa_node)
>   {
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index 0b7f94f..cd75e5a 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -856,7 +856,6 @@ static int ena_clean_tx_irq(struct ena_ring *tx_ring, u32 budget)
> 
>          tx_ring->next_to_clean = next_to_clean;
>          ena_com_comp_ack(tx_ring->ena_com_io_sq, total_done);
> -       ena_com_update_dev_comp_head(tx_ring->ena_com_io_cq);
> 
>          netdev_tx_completed_queue(txq, tx_pkts, tx_bytes);
> 
> @@ -1303,10 +1302,8 @@ static int ena_clean_rx_irq(struct ena_ring *rx_ring, struct napi_struct *napi,
>                        ENA_RX_REFILL_THRESH_PACKET);
> 
>          /* Optimization, try to batch new rx buffers */
> -       if (refill_required > refill_threshold) {
> -               ena_com_update_dev_comp_head(rx_ring->ena_com_io_cq);
> +       if (refill_required > refill_threshold)
>                  ena_refill_rx_bufs(rx_ring, refill_required);
> -       }
> 
>          if (xdp_flags & ENA_XDP_REDIRECT)
>                  xdp_do_flush();
> diff --git a/drivers/net/ethernet/amazon/ena/ena_xdp.c b/drivers/net/ethernet/amazon/ena/ena_xdp.c
> index fc1c4ef..337c435 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_xdp.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_xdp.c
> @@ -412,7 +412,6 @@ static int ena_clean_xdp_irq(struct ena_ring *tx_ring, u32 budget)
> 
>          tx_ring->next_to_clean = next_to_clean;
>          ena_com_comp_ack(tx_ring->ena_com_io_sq, total_done);
> -       ena_com_update_dev_comp_head(tx_ring->ena_com_io_cq);
> 
>          netif_dbg(tx_ring->adapter, tx_done, tx_ring->netdev,
>                    "tx_poll: q %d done. total pkts: %d\n",
> --
> 2.40.1
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ