[v17,03/11] eal: change API of power intrinsics
Checks
Commit Message
Instead of passing around pointers and integers, collect everything
into struct. This makes API design around these intrinsics much easier.
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
Notes:
v16:
- Add error handling
drivers/event/dlb/dlb.c | 10 ++--
drivers/event/dlb2/dlb2.c | 10 ++--
lib/librte_eal/arm/rte_power_intrinsics.c | 20 +++-----
.../include/generic/rte_power_intrinsics.h | 50 ++++++++-----------
lib/librte_eal/ppc/rte_power_intrinsics.c | 20 +++-----
lib/librte_eal/x86/rte_power_intrinsics.c | 42 +++++++++-------
6 files changed, 70 insertions(+), 82 deletions(-)
Comments
14/01/2021 15:46, Anatoly Burakov:
> +struct rte_power_monitor_cond {
> + volatile void *addr; /**< Address to monitor for changes */
> + uint64_t val; /**< Before attempting the monitoring, the address
> + * may be read and compared against this value.
"may" be read and compared?
Is there a case where there is no read and compare?
> + **/
> + uint64_t mask; /**< 64-bit mask to extract current value from addr */
> + uint8_t data_sz; /**< Data size (in bytes) that will be used to compare
> + * expected value with the memory address. Can be 1,
> + * 2, 4, or 8. Supplying any other value will lead to
> + * undefined result. */
Other parameters are not prefixed with "data_",
so I think this field could be simply named "size".
> +};
I understand this struct is a direct translation of what existed
in 20.11 as function parameters and comments.
If you agree, these comments could be addressed in a separate patch.
On 18-Jan-21 10:26 PM, Thomas Monjalon wrote:
> 14/01/2021 15:46, Anatoly Burakov:
>> +struct rte_power_monitor_cond {
>> + volatile void *addr; /**< Address to monitor for changes */
>> + uint64_t val; /**< Before attempting the monitoring, the address
>> + * may be read and compared against this value.
>
> "may" be read and compared?
> Is there a case where there is no read and compare?
Yes, if the mask is not set.
>
>> + **/
>> + uint64_t mask; /**< 64-bit mask to extract current value from addr */
>> + uint8_t data_sz; /**< Data size (in bytes) that will be used to compare
>> + * expected value with the memory address. Can be 1,
>> + * 2, 4, or 8. Supplying any other value will lead to
>> + * undefined result. */
>
> Other parameters are not prefixed with "data_",
> so I think this field could be simply named "size".
OK.
>
>> +};
>
> I understand this struct is a direct translation of what existed
> in 20.11 as function parameters and comments.
> If you agree, these comments could be addressed in a separate patch.
>
I'll be respinning anyway, so might as well do some quick fixups.
19/01/2021 11:29, Burakov, Anatoly:
> On 18-Jan-21 10:26 PM, Thomas Monjalon wrote:
> > 14/01/2021 15:46, Anatoly Burakov:
> >> +struct rte_power_monitor_cond {
> >> + volatile void *addr; /**< Address to monitor for changes */
> >> + uint64_t val; /**< Before attempting the monitoring, the address
> >> + * may be read and compared against this value.
> >
> > "may" be read and compared?
> > Is there a case where there is no read and compare?
>
> Yes, if the mask is not set.
If the mask is not set, the address is "read" anyway
or it is only "watched" for any change?
Sorry the mechanism is really not clear to me.
On 19-Jan-21 10:42 AM, Thomas Monjalon wrote:
> 19/01/2021 11:29, Burakov, Anatoly:
>> On 18-Jan-21 10:26 PM, Thomas Monjalon wrote:
>>> 14/01/2021 15:46, Anatoly Burakov:
>>>> +struct rte_power_monitor_cond {
>>>> + volatile void *addr; /**< Address to monitor for changes */
>>>> + uint64_t val; /**< Before attempting the monitoring, the address
>>>> + * may be read and compared against this value.
>>>
>>> "may" be read and compared?
>>> Is there a case where there is no read and compare?
>>
>> Yes, if the mask is not set.
>
> If the mask is not set, the address is "read" anyway
> or it is only "watched" for any change?
>
> Sorry the mechanism is really not clear to me.
>
The "value" is only used to avoid the sleep, i.e. to check if the write
has already happened. We're waiting on *a write* rather than *a value*,
so it's not equivalent to "wait until equal" call. It's more of a "sleep
until something happens".
19/01/2021 12:23, Burakov, Anatoly:
> On 19-Jan-21 10:42 AM, Thomas Monjalon wrote:
> > 19/01/2021 11:29, Burakov, Anatoly:
> >> On 18-Jan-21 10:26 PM, Thomas Monjalon wrote:
> >>> 14/01/2021 15:46, Anatoly Burakov:
> >>>> +struct rte_power_monitor_cond {
> >>>> + volatile void *addr; /**< Address to monitor for changes */
> >>>> + uint64_t val; /**< Before attempting the monitoring, the address
> >>>> + * may be read and compared against this value.
> >>>
> >>> "may" be read and compared?
> >>> Is there a case where there is no read and compare?
> >>
> >> Yes, if the mask is not set.
> >
> > If the mask is not set, the address is "read" anyway
> > or it is only "watched" for any change?
> >
> > Sorry the mechanism is really not clear to me.
> >
>
> The "value" is only used to avoid the sleep, i.e. to check if the write
> has already happened. We're waiting on *a write* rather than *a value*,
> so it's not equivalent to "wait until equal" call. It's more of a "sleep
> until something happens".
Please make things explicit in doxygen.
The behaviour of each case should be explained crystal clear.
Thanks
On 19-Jan-21 2:17 PM, Thomas Monjalon wrote:
> 19/01/2021 12:23, Burakov, Anatoly:
>> On 19-Jan-21 10:42 AM, Thomas Monjalon wrote:
>>> 19/01/2021 11:29, Burakov, Anatoly:
>>>> On 18-Jan-21 10:26 PM, Thomas Monjalon wrote:
>>>>> 14/01/2021 15:46, Anatoly Burakov:
>>>>>> +struct rte_power_monitor_cond {
>>>>>> + volatile void *addr; /**< Address to monitor for changes */
>>>>>> + uint64_t val; /**< Before attempting the monitoring, the address
>>>>>> + * may be read and compared against this value.
>>>>>
>>>>> "may" be read and compared?
>>>>> Is there a case where there is no read and compare?
>>>>
>>>> Yes, if the mask is not set.
>>>
>>> If the mask is not set, the address is "read" anyway
>>> or it is only "watched" for any change?
>>>
>>> Sorry the mechanism is really not clear to me.
>>>
>>
>> The "value" is only used to avoid the sleep, i.e. to check if the write
>> has already happened. We're waiting on *a write* rather than *a value*,
>> so it's not equivalent to "wait until equal" call. It's more of a "sleep
>> until something happens".
>
> Please make things explicit in doxygen.
> The behaviour of each case should be explained crystal clear.
> Thanks
>
>
It is explained in the comments to `rte_power_monitor()` call. But OK,
i'll add more clarification for the struct too.
20/01/2021 11:32, Burakov, Anatoly:
> On 19-Jan-21 2:17 PM, Thomas Monjalon wrote:
> > 19/01/2021 12:23, Burakov, Anatoly:
> >> On 19-Jan-21 10:42 AM, Thomas Monjalon wrote:
> >>> 19/01/2021 11:29, Burakov, Anatoly:
> >>>> On 18-Jan-21 10:26 PM, Thomas Monjalon wrote:
> >>>>> 14/01/2021 15:46, Anatoly Burakov:
> >>>>>> +struct rte_power_monitor_cond {
> >>>>>> + volatile void *addr; /**< Address to monitor for changes */
> >>>>>> + uint64_t val; /**< Before attempting the monitoring, the address
> >>>>>> + * may be read and compared against this value.
> >>>>>
> >>>>> "may" be read and compared?
> >>>>> Is there a case where there is no read and compare?
> >>>>
> >>>> Yes, if the mask is not set.
> >>>
> >>> If the mask is not set, the address is "read" anyway
> >>> or it is only "watched" for any change?
> >>>
> >>> Sorry the mechanism is really not clear to me.
> >>>
> >>
> >> The "value" is only used to avoid the sleep, i.e. to check if the write
> >> has already happened. We're waiting on *a write* rather than *a value*,
> >> so it's not equivalent to "wait until equal" call. It's more of a "sleep
> >> until something happens".
> >
> > Please make things explicit in doxygen.
> > The behaviour of each case should be explained crystal clear.
> > Thanks
>
> It is explained in the comments to `rte_power_monitor()` call. But OK,
> i'll add more clarification for the struct too.
Please avoid the word "may" in API description.
This is what is explained in rte_power_monitor:
"
* Additionally, an `expected` 64-bit value and 64-bit mask are provided. If
* mask is non-zero, the current value pointed to by the `p` pointer will be
* checked against the expected value, and if they match, the entering of
* optimized power state may be aborted.
"
Can we replace "may" by "will"?
On 20-Jan-21 10:38 AM, Thomas Monjalon wrote:
> 20/01/2021 11:32, Burakov, Anatoly:
>> On 19-Jan-21 2:17 PM, Thomas Monjalon wrote:
>>> 19/01/2021 12:23, Burakov, Anatoly:
>>>> On 19-Jan-21 10:42 AM, Thomas Monjalon wrote:
>>>>> 19/01/2021 11:29, Burakov, Anatoly:
>>>>>> On 18-Jan-21 10:26 PM, Thomas Monjalon wrote:
>>>>>>> 14/01/2021 15:46, Anatoly Burakov:
>>>>>>>> +struct rte_power_monitor_cond {
>>>>>>>> + volatile void *addr; /**< Address to monitor for changes */
>>>>>>>> + uint64_t val; /**< Before attempting the monitoring, the address
>>>>>>>> + * may be read and compared against this value.
>>>>>>>
>>>>>>> "may" be read and compared?
>>>>>>> Is there a case where there is no read and compare?
>>>>>>
>>>>>> Yes, if the mask is not set.
>>>>>
>>>>> If the mask is not set, the address is "read" anyway
>>>>> or it is only "watched" for any change?
>>>>>
>>>>> Sorry the mechanism is really not clear to me.
>>>>>
>>>>
>>>> The "value" is only used to avoid the sleep, i.e. to check if the write
>>>> has already happened. We're waiting on *a write* rather than *a value*,
>>>> so it's not equivalent to "wait until equal" call. It's more of a "sleep
>>>> until something happens".
>>>
>>> Please make things explicit in doxygen.
>>> The behaviour of each case should be explained crystal clear.
>>> Thanks
>>
>> It is explained in the comments to `rte_power_monitor()` call. But OK,
>> i'll add more clarification for the struct too.
>
> Please avoid the word "may" in API description.
>
> This is what is explained in rte_power_monitor:
> "
> * Additionally, an `expected` 64-bit value and 64-bit mask are provided. If
> * mask is non-zero, the current value pointed to by the `p` pointer will be
> * checked against the expected value, and if they match, the entering of
> * optimized power state may be aborted.
> "
>
> Can we replace "may" by "will"?
>
Yep, we can. However, the "may" part was intended to leave some wiggle
room for a different implementation, should the need arise, and i find
"will" to be needlessly prescriptive. Frankly, i do not see the need for
such a detailed description of what the API does under the hood, as long
as it's clear what its effects are. The main purpose is waiting for a
write. The mask is only used to check whether the expected write has
already happened by the time we're calling the API. Whether the CPU then
does or does not go to sleep is not really relevant IMO.
20/01/2021 12:05, Burakov, Anatoly:
> On 20-Jan-21 10:38 AM, Thomas Monjalon wrote:
> > 20/01/2021 11:32, Burakov, Anatoly:
> >> On 19-Jan-21 2:17 PM, Thomas Monjalon wrote:
> >>> 19/01/2021 12:23, Burakov, Anatoly:
> >>>> On 19-Jan-21 10:42 AM, Thomas Monjalon wrote:
> >>>>> 19/01/2021 11:29, Burakov, Anatoly:
> >>>>>> On 18-Jan-21 10:26 PM, Thomas Monjalon wrote:
> >>>>>>> 14/01/2021 15:46, Anatoly Burakov:
> >>>>>>>> +struct rte_power_monitor_cond {
> >>>>>>>> + volatile void *addr; /**< Address to monitor for changes */
> >>>>>>>> + uint64_t val; /**< Before attempting the monitoring, the address
> >>>>>>>> + * may be read and compared against this value.
> >>>>>>>
> >>>>>>> "may" be read and compared?
> >>>>>>> Is there a case where there is no read and compare?
> >>>>>>
> >>>>>> Yes, if the mask is not set.
> >>>>>
> >>>>> If the mask is not set, the address is "read" anyway
> >>>>> or it is only "watched" for any change?
> >>>>>
> >>>>> Sorry the mechanism is really not clear to me.
> >>>>>
> >>>>
> >>>> The "value" is only used to avoid the sleep, i.e. to check if the write
> >>>> has already happened. We're waiting on *a write* rather than *a value*,
> >>>> so it's not equivalent to "wait until equal" call. It's more of a "sleep
> >>>> until something happens".
> >>>
> >>> Please make things explicit in doxygen.
> >>> The behaviour of each case should be explained crystal clear.
> >>> Thanks
> >>
> >> It is explained in the comments to `rte_power_monitor()` call. But OK,
> >> i'll add more clarification for the struct too.
> >
> > Please avoid the word "may" in API description.
> >
> > This is what is explained in rte_power_monitor:
> > "
> > * Additionally, an `expected` 64-bit value and 64-bit mask are provided. If
> > * mask is non-zero, the current value pointed to by the `p` pointer will be
> > * checked against the expected value, and if they match, the entering of
> > * optimized power state may be aborted.
> > "
> >
> > Can we replace "may" by "will"?
> >
>
> Yep, we can. However, the "may" part was intended to leave some wiggle
> room for a different implementation, should the need arise, and i find
> "will" to be needlessly prescriptive. Frankly, i do not see the need for
> such a detailed description of what the API does under the hood, as long
> as it's clear what its effects are. The main purpose is waiting for a
> write. The mask is only used to check whether the expected write has
> already happened by the time we're calling the API. Whether the CPU then
> does or does not go to sleep is not really relevant IMO.
I think it is relevant but I may be wrong.
Any other opinions?
On 20-Jan-21 11:11 AM, Thomas Monjalon wrote:
> 20/01/2021 12:05, Burakov, Anatoly:
>> On 20-Jan-21 10:38 AM, Thomas Monjalon wrote:
>>> 20/01/2021 11:32, Burakov, Anatoly:
>>>> On 19-Jan-21 2:17 PM, Thomas Monjalon wrote:
>>>>> 19/01/2021 12:23, Burakov, Anatoly:
>>>>>> On 19-Jan-21 10:42 AM, Thomas Monjalon wrote:
>>>>>>> 19/01/2021 11:29, Burakov, Anatoly:
>>>>>>>> On 18-Jan-21 10:26 PM, Thomas Monjalon wrote:
>>>>>>>>> 14/01/2021 15:46, Anatoly Burakov:
>>>>>>>>>> +struct rte_power_monitor_cond {
>>>>>>>>>> + volatile void *addr; /**< Address to monitor for changes */
>>>>>>>>>> + uint64_t val; /**< Before attempting the monitoring, the address
>>>>>>>>>> + * may be read and compared against this value.
>>>>>>>>>
>>>>>>>>> "may" be read and compared?
>>>>>>>>> Is there a case where there is no read and compare?
>>>>>>>>
>>>>>>>> Yes, if the mask is not set.
>>>>>>>
>>>>>>> If the mask is not set, the address is "read" anyway
>>>>>>> or it is only "watched" for any change?
>>>>>>>
>>>>>>> Sorry the mechanism is really not clear to me.
>>>>>>>
>>>>>>
>>>>>> The "value" is only used to avoid the sleep, i.e. to check if the write
>>>>>> has already happened. We're waiting on *a write* rather than *a value*,
>>>>>> so it's not equivalent to "wait until equal" call. It's more of a "sleep
>>>>>> until something happens".
>>>>>
>>>>> Please make things explicit in doxygen.
>>>>> The behaviour of each case should be explained crystal clear.
>>>>> Thanks
>>>>
>>>> It is explained in the comments to `rte_power_monitor()` call. But OK,
>>>> i'll add more clarification for the struct too.
>>>
>>> Please avoid the word "may" in API description.
>>>
>>> This is what is explained in rte_power_monitor:
>>> "
>>> * Additionally, an `expected` 64-bit value and 64-bit mask are provided. If
>>> * mask is non-zero, the current value pointed to by the `p` pointer will be
>>> * checked against the expected value, and if they match, the entering of
>>> * optimized power state may be aborted.
>>> "
>>>
>>> Can we replace "may" by "will"?
>>>
>>
>> Yep, we can. However, the "may" part was intended to leave some wiggle
>> room for a different implementation, should the need arise, and i find
>> "will" to be needlessly prescriptive. Frankly, i do not see the need for
>> such a detailed description of what the API does under the hood, as long
>> as it's clear what its effects are. The main purpose is waiting for a
>> write. The mask is only used to check whether the expected write has
>> already happened by the time we're calling the API. Whether the CPU then
>> does or does not go to sleep is not really relevant IMO.
>
> I think it is relevant but I may be wrong.
> Any other opinions?
>
I have no objection in documenting that further, so i'll go ahead and do
it :) It's just that i think it's not necessary to be *this* detailed
about how the API does things.
@@ -3161,6 +3161,7 @@ dlb_dequeue_wait(struct dlb_eventdev *dlb,
/* Interrupts not supported by PF PMD */
return 1;
} else if (dlb->umwait_allowed) {
+ struct rte_power_monitor_cond pmc;
volatile struct dlb_dequeue_qe *cq_base;
union {
uint64_t raw_qe[2];
@@ -3181,9 +3182,12 @@ dlb_dequeue_wait(struct dlb_eventdev *dlb,
else
expected_value = 0;
- rte_power_monitor(monitor_addr, expected_value,
- qe_mask.raw_qe[1], timeout + start_ticks,
- sizeof(uint64_t));
+ pmc.addr = monitor_addr;
+ pmc.val = expected_value;
+ pmc.mask = qe_mask.raw_qe[1];
+ pmc.data_sz = sizeof(uint64_t);
+
+ rte_power_monitor(&pmc, timeout + start_ticks);
DLB_INC_STAT(ev_port->stats.traffic.rx_umonitor_umwait, 1);
} else {
@@ -2870,6 +2870,7 @@ dlb2_dequeue_wait(struct dlb2_eventdev *dlb2,
if (elapsed_ticks >= timeout) {
return 1;
} else if (dlb2->umwait_allowed) {
+ struct rte_power_monitor_cond pmc;
volatile struct dlb2_dequeue_qe *cq_base;
union {
uint64_t raw_qe[2];
@@ -2890,9 +2891,12 @@ dlb2_dequeue_wait(struct dlb2_eventdev *dlb2,
else
expected_value = 0;
- rte_power_monitor(monitor_addr, expected_value,
- qe_mask.raw_qe[1], timeout + start_ticks,
- sizeof(uint64_t));
+ pmc.addr = monitor_addr;
+ pmc.val = expected_value;
+ pmc.mask = qe_mask.raw_qe[1];
+ pmc.data_sz = sizeof(uint64_t);
+
+ rte_power_monitor(&pmc, timeout + start_ticks);
DLB2_INC_STAT(ev_port->stats.traffic.rx_umonitor_umwait, 1);
} else {
@@ -8,15 +8,11 @@
* This function is not supported on ARM.
*/
int
-rte_power_monitor(const volatile void *p, const uint64_t expected_value,
- const uint64_t value_mask, const uint64_t tsc_timestamp,
- const uint8_t data_sz)
+rte_power_monitor(const struct rte_power_monitor_cond *pmc,
+ const uint64_t tsc_timestamp)
{
- RTE_SET_USED(p);
- RTE_SET_USED(expected_value);
- RTE_SET_USED(value_mask);
+ RTE_SET_USED(pmc);
RTE_SET_USED(tsc_timestamp);
- RTE_SET_USED(data_sz);
return -ENOTSUP;
}
@@ -25,16 +21,12 @@ rte_power_monitor(const volatile void *p, const uint64_t expected_value,
* This function is not supported on ARM.
*/
int
-rte_power_monitor_sync(const volatile void *p, const uint64_t expected_value,
- const uint64_t value_mask, const uint64_t tsc_timestamp,
- const uint8_t data_sz, rte_spinlock_t *lck)
+rte_power_monitor_sync(const struct rte_power_monitor_cond *pmc,
+ const uint64_t tsc_timestamp, rte_spinlock_t *lck)
{
- RTE_SET_USED(p);
- RTE_SET_USED(expected_value);
- RTE_SET_USED(value_mask);
+ RTE_SET_USED(pmc);
RTE_SET_USED(tsc_timestamp);
RTE_SET_USED(lck);
- RTE_SET_USED(data_sz);
return -ENOTSUP;
}
@@ -18,6 +18,18 @@
* which are architecture-dependent.
*/
+struct rte_power_monitor_cond {
+ volatile void *addr; /**< Address to monitor for changes */
+ uint64_t val; /**< Before attempting the monitoring, the address
+ * may be read and compared against this value.
+ **/
+ uint64_t mask; /**< 64-bit mask to extract current value from addr */
+ uint8_t data_sz; /**< Data size (in bytes) that will be used to compare
+ * expected value with the memory address. Can be 1,
+ * 2, 4, or 8. Supplying any other value will lead to
+ * undefined result. */
+};
+
/**
* @warning
* @b EXPERIMENTAL: this API may change without prior notice
@@ -35,20 +47,11 @@
* @warning It is responsibility of the user to check if this function is
* supported at runtime using `rte_cpu_get_intrinsics_support()` API call.
*
- * @param p
- * Address to monitor for changes.
- * @param expected_value
- * Before attempting the monitoring, the `p` address may be read and compared
- * against this value. If `value_mask` is zero, this step will be skipped.
- * @param value_mask
- * The 64-bit mask to use to extract current value from `p`.
+ * @param pmc
+ * The monitoring condition structure.
* @param tsc_timestamp
* Maximum TSC timestamp to wait for. Note that the wait behavior is
* architecture-dependent.
- * @param data_sz
- * Data size (in bytes) that will be used to compare expected value with the
- * memory address. Can be 1, 2, 4 or 8. Supplying any other value will lead
- * to undefined result.
*
* @return
* 0 on success
@@ -56,10 +59,8 @@
* -ENOTSUP if unsupported
*/
__rte_experimental
-int rte_power_monitor(const volatile void *p,
- const uint64_t expected_value, const uint64_t value_mask,
- const uint64_t tsc_timestamp, const uint8_t data_sz);
-
+int rte_power_monitor(const struct rte_power_monitor_cond *pmc,
+ const uint64_t tsc_timestamp);
/**
* @warning
* @b EXPERIMENTAL: this API may change without prior notice
@@ -80,20 +81,11 @@ int rte_power_monitor(const volatile void *p,
* @warning It is responsibility of the user to check if this function is
* supported at runtime using `rte_cpu_get_intrinsics_support()` API call.
*
- * @param p
- * Address to monitor for changes.
- * @param expected_value
- * Before attempting the monitoring, the `p` address may be read and compared
- * against this value. If `value_mask` is zero, this step will be skipped.
- * @param value_mask
- * The 64-bit mask to use to extract current value from `p`.
+ * @param pmc
+ * The monitoring condition structure.
* @param tsc_timestamp
* Maximum TSC timestamp to wait for. Note that the wait behavior is
* architecture-dependent.
- * @param data_sz
- * Data size (in bytes) that will be used to compare expected value with the
- * memory address. Can be 1, 2, 4 or 8. Supplying any other value will lead
- * to undefined result.
* @param lck
* A spinlock that must be locked before entering the function, will be
* unlocked while the CPU is sleeping, and will be locked again once the CPU
@@ -105,10 +97,8 @@ int rte_power_monitor(const volatile void *p,
* -ENOTSUP if unsupported
*/
__rte_experimental
-int rte_power_monitor_sync(const volatile void *p,
- const uint64_t expected_value, const uint64_t value_mask,
- const uint64_t tsc_timestamp, const uint8_t data_sz,
- rte_spinlock_t *lck);
+int rte_power_monitor_sync(const struct rte_power_monitor_cond *pmc,
+ const uint64_t tsc_timestamp, rte_spinlock_t *lck);
/**
* @warning
@@ -8,15 +8,11 @@
* This function is not supported on PPC64.
*/
int
-rte_power_monitor(const volatile void *p, const uint64_t expected_value,
- const uint64_t value_mask, const uint64_t tsc_timestamp,
- const uint8_t data_sz)
+rte_power_monitor(const struct rte_power_monitor_cond *pmc,
+ const uint64_t tsc_timestamp)
{
- RTE_SET_USED(p);
- RTE_SET_USED(expected_value);
- RTE_SET_USED(value_mask);
+ RTE_SET_USED(pmc);
RTE_SET_USED(tsc_timestamp);
- RTE_SET_USED(data_sz);
return -ENOTSUP;
}
@@ -25,16 +21,12 @@ rte_power_monitor(const volatile void *p, const uint64_t expected_value,
* This function is not supported on PPC64.
*/
int
-rte_power_monitor_sync(const volatile void *p, const uint64_t expected_value,
- const uint64_t value_mask, const uint64_t tsc_timestamp,
- const uint8_t data_sz, rte_spinlock_t *lck)
+rte_power_monitor_sync(const struct rte_power_monitor_cond *pmc,
+ const uint64_t tsc_timestamp, rte_spinlock_t *lck)
{
- RTE_SET_USED(p);
- RTE_SET_USED(expected_value);
- RTE_SET_USED(value_mask);
+ RTE_SET_USED(pmc);
RTE_SET_USED(tsc_timestamp);
RTE_SET_USED(lck);
- RTE_SET_USED(data_sz);
return -ENOTSUP;
}
@@ -46,9 +46,8 @@ __check_val_size(const uint8_t sz)
* Intel(R) 64 and IA-32 Architectures Software Developer's Manual.
*/
int
-rte_power_monitor(const volatile void *p, const uint64_t expected_value,
- const uint64_t value_mask, const uint64_t tsc_timestamp,
- const uint8_t data_sz)
+rte_power_monitor(const struct rte_power_monitor_cond *pmc,
+ const uint64_t tsc_timestamp)
{
const uint32_t tsc_l = (uint32_t)tsc_timestamp;
const uint32_t tsc_h = (uint32_t)(tsc_timestamp >> 32);
@@ -57,7 +56,10 @@ rte_power_monitor(const volatile void *p, const uint64_t expected_value,
if (!wait_supported)
return -ENOTSUP;
- if (__check_val_size(data_sz) < 0)
+ if (pmc == NULL)
+ return -EINVAL;
+
+ if (__check_val_size(pmc->data_sz) < 0)
return -EINVAL;
/*
@@ -68,14 +70,15 @@ rte_power_monitor(const volatile void *p, const uint64_t expected_value,
/* set address for UMONITOR */
asm volatile(".byte 0xf3, 0x0f, 0xae, 0xf7;"
:
- : "D"(p));
+ : "D"(pmc->addr));
- if (value_mask) {
- const uint64_t cur_value = __get_umwait_val(p, data_sz);
- const uint64_t masked = cur_value & value_mask;
+ if (pmc->mask) {
+ const uint64_t cur_value = __get_umwait_val(
+ pmc->addr, pmc->data_sz);
+ const uint64_t masked = cur_value & pmc->mask;
/* if the masked value is already matching, abort */
- if (masked == expected_value)
+ if (masked == pmc->val)
return 0;
}
/* execute UMWAIT */
@@ -93,9 +96,8 @@ rte_power_monitor(const volatile void *p, const uint64_t expected_value,
* Intel(R) 64 and IA-32 Architectures Software Developer's Manual.
*/
int
-rte_power_monitor_sync(const volatile void *p, const uint64_t expected_value,
- const uint64_t value_mask, const uint64_t tsc_timestamp,
- const uint8_t data_sz, rte_spinlock_t *lck)
+rte_power_monitor_sync(const struct rte_power_monitor_cond *pmc,
+ const uint64_t tsc_timestamp, rte_spinlock_t *lck)
{
const uint32_t tsc_l = (uint32_t)tsc_timestamp;
const uint32_t tsc_h = (uint32_t)(tsc_timestamp >> 32);
@@ -104,7 +106,10 @@ rte_power_monitor_sync(const volatile void *p, const uint64_t expected_value,
if (!wait_supported)
return -ENOTSUP;
- if (__check_val_size(data_sz) < 0)
+ if (pmc == NULL || lck == NULL)
+ return -EINVAL;
+
+ if (__check_val_size(pmc->data_sz) < 0)
return -EINVAL;
/*
@@ -115,14 +120,15 @@ rte_power_monitor_sync(const volatile void *p, const uint64_t expected_value,
/* set address for UMONITOR */
asm volatile(".byte 0xf3, 0x0f, 0xae, 0xf7;"
:
- : "D"(p));
+ : "D"(pmc->addr));
- if (value_mask) {
- const uint64_t cur_value = __get_umwait_val(p, data_sz);
- const uint64_t masked = cur_value & value_mask;
+ if (pmc->mask) {
+ const uint64_t cur_value = __get_umwait_val(
+ pmc->addr, pmc->data_sz);
+ const uint64_t masked = cur_value & pmc->mask;
/* if the masked value is already matching, abort */
- if (masked == expected_value)
+ if (masked == pmc->val)
return 0;
}
rte_spinlock_unlock(lck);