[v3,10/33] net/ena/hal: added a bus parameter to ena memcpy macro

Message ID 20240306122445.4350-11-shaibran@amazon.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series net/ena: v2.9.0 driver release |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Brandes, Shai March 6, 2024, 12:24 p.m. UTC
  From: Shai Brandes <shaibran@amazon.com>

ENA_MEMCPY_TO_DEVICE_64 macro needs pci bus id in order
to write to the device memory when using llq.

Signed-off-by: Shai Brandes <shaibran@amazon.com>
Reviewed-by: Amit Bernstein <amitbern@amazon.com>
---
 drivers/net/ena/hal/ena_eth_com.c   | 3 ++-
 drivers/net/ena/hal/ena_plat_dpdk.h | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)
  

Comments

Ferruh Yigit March 8, 2024, 5:25 p.m. UTC | #1
Addressed
On 3/6/2024 12:24 PM, shaibran@amazon.com wrote:
> From: Shai Brandes <shaibran@amazon.com>
> 
> ENA_MEMCPY_TO_DEVICE_64 macro needs pci bus id in order
> to write to the device memory when using llq.
> 

As far as I can see macro doesn't use 'bus' at all, "(void)(bus);",
how/why it is needed for LLQ? Can you please describe it more?
  
Brandes, Shai March 10, 2024, 3:08 p.m. UTC | #2
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Friday, March 8, 2024 7:25 PM
> To: Brandes, Shai <shaibran@amazon.com>
> Cc: dev@dpdk.org
> Subject: RE: [EXTERNAL] [PATCH v3 10/33] net/ena/hal: added a bus
> parameter to ena memcpy macro
> 
> CAUTION: This email originated from outside of the organization. Do not click
> links or open attachments unless you can confirm the sender and know the
> content is safe.
> 
> 
> 
> On 3/6/2024 12:24 PM, shaibran@amazon.com wrote:
> > From: Shai Brandes <shaibran@amazon.com>
> >
> > ENA_MEMCPY_TO_DEVICE_64 macro needs pci bus id in order to write to
> > the device memory when using llq.
> >
> 
> As far as I can see macro doesn't use 'bus' at all, "(void)(bus);", how/why it is
> needed for LLQ? Can you please describe it more?
[Brandes, Shai] I understand the confusion.
This is part of a hal change that concerns mac OS and required to modify the common ENA_MEMCPY_TO_DEVICE_64.
Since we expose only the DPDK-specific implementation, where this parameter is unused, it appears as void.
Avoiding this will create differences between the internal and upstream versions which will make it hard to maintain.
  
Ferruh Yigit March 13, 2024, 11:27 a.m. UTC | #3
On 3/10/2024 3:08 PM, Brandes, Shai wrote:
> 
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Friday, March 8, 2024 7:25 PM
>> To: Brandes, Shai <shaibran@amazon.com>
>> Cc: dev@dpdk.org
>> Subject: RE: [EXTERNAL] [PATCH v3 10/33] net/ena/hal: added a bus
>> parameter to ena memcpy macro
>>
>> CAUTION: This email originated from outside of the organization. Do not click
>> links or open attachments unless you can confirm the sender and know the
>> content is safe.
>>
>>
>>
>> On 3/6/2024 12:24 PM, shaibran@amazon.com wrote:
>>> From: Shai Brandes <shaibran@amazon.com>
>>>
>>> ENA_MEMCPY_TO_DEVICE_64 macro needs pci bus id in order to write to
>>> the device memory when using llq.
>>>
>>
>> As far as I can see macro doesn't use 'bus' at all, "(void)(bus);", how/why it is
>> needed for LLQ? Can you please describe it more?
> [Brandes, Shai] I understand the confusion.
> This is part of a hal change that concerns mac OS and required to modify the common ENA_MEMCPY_TO_DEVICE_64.
> Since we expose only the DPDK-specific implementation, where this parameter is unused, it appears as void.
> Avoiding this will create differences between the internal and upstream versions which will make it hard to maintain.
> 

It is OK to have it, as you said to reduce the diff, but please update
the commit log with above detail, and it will be OK.
  

Patch

diff --git a/drivers/net/ena/hal/ena_eth_com.c b/drivers/net/ena/hal/ena_eth_com.c
index 32090259cd..d6811c7b48 100644
--- a/drivers/net/ena/hal/ena_eth_com.c
+++ b/drivers/net/ena/hal/ena_eth_com.c
@@ -74,7 +74,8 @@  static int ena_com_write_bounce_buffer_to_dev(struct ena_com_io_sq *io_sq,
 	wmb();
 
 	/* The line is completed. Copy it to dev */
-	ENA_MEMCPY_TO_DEVICE_64(io_sq->desc_addr.pbuf_dev_addr + dst_offset,
+	ENA_MEMCPY_TO_DEVICE_64(io_sq->bus,
+				io_sq->desc_addr.pbuf_dev_addr + dst_offset,
 				bounce_buffer,
 				llq_info->desc_list_entry_size);
 
diff --git a/drivers/net/ena/hal/ena_plat_dpdk.h b/drivers/net/ena/hal/ena_plat_dpdk.h
index 14bf582a45..5f7cbd1ee7 100644
--- a/drivers/net/ena/hal/ena_plat_dpdk.h
+++ b/drivers/net/ena/hal/ena_plat_dpdk.h
@@ -301,11 +301,12 @@  ena_mem_alloc_coherent(struct rte_eth_dev_data *data, size_t size,
 #define ENA_WAIT_EVENTS_DESTROY(admin_queue) ((void)(admin_queue))
 
 /* The size must be 8 byte align */
-#define ENA_MEMCPY_TO_DEVICE_64(dst, src, size)				       \
+#define ENA_MEMCPY_TO_DEVICE_64(bus, dst, src, size)			       \
 	do {								       \
 		int count, i;						       \
 		uint64_t *to = (uint64_t *)(dst);			       \
 		const uint64_t *from = (const uint64_t *)(src);		       \
+		(void)(bus);						       \
 		count = (size) / 8;					       \
 		for (i = 0; i < count; i++, from++, to++)		       \
 			rte_write64_relaxed(*from, to);			       \