[dpdk-dev] [PATCH v5 06/10] net/ixgbe: implement power management API

[Burakov, Anatoly]

On 12-Oct-20 9:09 AM, Wang, Haiyue wrote:
> Hi Liang,
> 
>> -----Original Message-----
>> From: Burakov, Anatoly <anatoly.burakov at intel.com>
>> Sent: Saturday, October 10, 2020 00:02
>> To: dev at dpdk.org
>> Cc: Ma, Liang J <liang.j.ma at intel.com>; Guo, Jia <jia.guo at intel.com>; Wang, Haiyue
>> <haiyue.wang at intel.com>; Hunt, David <david.hunt at intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev at intel.com>; jerinjacobk at gmail.com; Richardson, Bruce <bruce.richardson at intel.com>;
>> thomas at monjalon.net; McDaniel, Timothy <timothy.mcdaniel at intel.com>; Eads, Gage <gage.eads at intel.com>;
>> Macnamara, Chris <chris.macnamara at intel.com>
>> Subject: [PATCH v5 06/10] net/ixgbe: implement power management API
>>
>> From: Liang Ma <liang.j.ma at intel.com>
>>
>> Implement support for the power management API by implementing a
>> `get_wake_addr` function that will return an address of an RX ring's
>> status bit.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov at intel.com>
>> Signed-off-by: Liang Ma <liang.j.ma at intel.com>
>> ---
>>   drivers/net/ixgbe/ixgbe_ethdev.c |  1 +
>>   drivers/net/ixgbe/ixgbe_rxtx.c   | 22 ++++++++++++++++++++++
>>   drivers/net/ixgbe/ixgbe_rxtx.h   |  2 ++
>>   3 files changed, 25 insertions(+)
>>
> 
> 
>>
>> +int ixgbe_get_wake_addr(void *rx_queue, volatile void **tail_desc_addr,
>> +uint64_t *expected, uint64_t *mask)
>> +{
>> +volatile union ixgbe_adv_rx_desc *rxdp;
>> +struct ixgbe_rx_queue *rxq = rx_queue;
>> +uint16_t desc;
>> +
>> +desc = rxq->rx_tail;
>> +rxdp = &rxq->rx_ring[desc];
>> +/* watch for changes in status bit */
>> +*tail_desc_addr = &rxdp->wb.upper.status_error;
>> +
>> +/*
>> + * we expect the DD bit to be set to 1 if this descriptor was already
>> + * written to.
>> + */
>> +*expected = rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD);
>> +*mask = rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD);
>> +
> 
> Seems have one issue about the byte endian:
> Like for BIG endian:
>           *expected = rte_bswap32(IXGBE_RXDADV_STAT_DD)
>               !=
>           *expected = rte_bswap64(IXGBE_RXDADV_STAT_DD)
> 
> And in API 'rte_power_monitor', use uint64_t type to access the wake up
> data:
> 
> static inline void rte_power_monitor(const volatile void *p,
> const uint64_t expected_value, const uint64_t value_mask,
> const uint64_t tsc_timestamp)
> {
> if (value_mask) {
> const uint64_t cur_value = *(const volatile uint64_t *)p;
> const uint64_t masked = cur_value & value_mask;
> /* if the masked value is already matching, abort */
> if (masked == expected_value)
> return;
> }
> 
> 
> So that we need the wake up address type like 16/32/64b ?

Endian differences strike again! You're right of course.

I suspect casting everything to CPU endinanness would fix it, would it not?

> 
>> --
>> 2.17.1


-- 
Thanks,
Anatoly