examples/l3fwd-power: fix to configure the uncore env
Checks
Commit Message
Updated the l3fwd-power app to configure the uncore env before invoking
any uncore APIs. With auto-detection in 'rte_power_uncore_init()' it is
too late because other APIs already called.
Bugzilla ID: 1304
Fixes: ac1edcb6621a ("power: refactor uncore power management API")
Cc: karen.kelly@intel.com
Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
---
examples/l3fwd-power/main.c | 6 ++++++
lib/power/rte_power_uncore.c | 7 +++++++
lib/power/rte_power_uncore.h | 9 +++++----
3 files changed, 18 insertions(+), 4 deletions(-)
Comments
On 26/10/2023 16:19, Sivaprasad Tummala wrote:
> Updated the l3fwd-power app to configure the uncore env before invoking
> any uncore APIs. With auto-detection in 'rte_power_uncore_init()' it is
> too late because other APIs already called.
>
> Bugzilla ID: 1304
> Fixes: ac1edcb6621a ("power: refactor uncore power management API")
> Cc:karen.kelly@intel.com
>
> Signed-off-by: Sivaprasad Tummala<sivaprasad.tummala@amd.com>
> ---
> examples/l3fwd-power/main.c | 6 ++++++
> lib/power/rte_power_uncore.c | 7 +++++++
> lib/power/rte_power_uncore.h | 9 +++++----
> 3 files changed, 18 insertions(+), 4 deletions(-)
Ran on my system, bug is resolved after applying this patch.
Tested-by: Karen Kelly <karen.kelly@intel.com>
Hi Sivaprasad, Karen,
On 27/10/2023 13:36, Kelly, Karen wrote:
>
>
> On 26/10/2023 16:19, Sivaprasad Tummala wrote:
>> Updated the l3fwd-power app to configure the uncore env before invoking
>> any uncore APIs. With auto-detection in 'rte_power_uncore_init()' it is
>> too late because other APIs already called.
>>
>> Bugzilla ID: 1304
>> Fixes: ac1edcb6621a ("power: refactor uncore power management API")
>> Cc:karen.kelly@intel.com
>>
>> Signed-off-by: Sivaprasad Tummala<sivaprasad.tummala@amd.com>
>> ---
>> examples/l3fwd-power/main.c | 6 ++++++
>> lib/power/rte_power_uncore.c | 7 +++++++
>> lib/power/rte_power_uncore.h | 9 +++++----
>> 3 files changed, 18 insertions(+), 4 deletions(-)
> Ran on my system, bug is resolved after applying this patch.
>
> Tested-by: Karen Kelly <karen.kelly@intel.com>
>
Acked-by: David Hunt <david.hunt@intel.com>
26/10/2023 17:19, Sivaprasad Tummala:
> Updated the l3fwd-power app to configure the uncore env before invoking
> any uncore APIs. With auto-detection in 'rte_power_uncore_init()' it is
> too late because other APIs already called.
You are also updating the uncore API.
> + if (env == RTE_UNCORE_PM_ENV_AUTO_DETECT)
> + /* Currently only intel_uncore is supported. This will be
> + * extended with auto-detection support for multiple uncore
> + * implementations.
> + */
> + env = RTE_UNCORE_PM_ENV_INTEL_UNCORE;
It looks like this patch does not make sense without AMD support.
On 11/23/2023 1:58 AM, Thomas Monjalon wrote:
> 26/10/2023 17:19, Sivaprasad Tummala:
>> Updated the l3fwd-power app to configure the uncore env before invoking
>> any uncore APIs. With auto-detection in 'rte_power_uncore_init()' it is
>> too late because other APIs already called.
>
> You are also updating the uncore API.
>
>> + if (env == RTE_UNCORE_PM_ENV_AUTO_DETECT)
>> + /* Currently only intel_uncore is supported. This will be
>> + * extended with auto-detection support for multiple uncore
>> + * implementations.
>> + */
>> + env = RTE_UNCORE_PM_ENV_INTEL_UNCORE;
>
> It looks like this patch does not make sense without AMD support.
>
Yes, right now auto detect directly fallback to Intel, but there is an
intention to add AMD support too, that is why instead directly using
Intel preferred the auto detection abstraction.
On Thu, Nov 23, 2023 at 02:58:58AM +0100, Thomas Monjalon wrote:
> 26/10/2023 17:19, Sivaprasad Tummala:
> > Updated the l3fwd-power app to configure the uncore env before invoking
> > any uncore APIs. With auto-detection in 'rte_power_uncore_init()' it is
> > too late because other APIs already called.
>
> You are also updating the uncore API.
>
> > + if (env == RTE_UNCORE_PM_ENV_AUTO_DETECT)
> > + /* Currently only intel_uncore is supported. This will be
> > + * extended with auto-detection support for multiple uncore
> > + * implementations.
> > + */
> > + env = RTE_UNCORE_PM_ENV_INTEL_UNCORE;
>
> It looks like this patch does not make sense without AMD support.
>
This patch is fixing a regression introduced by an earlier patch in this
area (referenced in the fixes line). See bugzilla for more details on it[1]
This should go into 23.11 as, without it, what was working in earlier
releases no longer does so.
Thanks,
/Bruce
[1] https://bugs.dpdk.org/show_bug.cgi?id=1304
27/11/2023 17:54, Bruce Richardson:
> On Thu, Nov 23, 2023 at 02:58:58AM +0100, Thomas Monjalon wrote:
> > 26/10/2023 17:19, Sivaprasad Tummala:
> > > Updated the l3fwd-power app to configure the uncore env before invoking
> > > any uncore APIs. With auto-detection in 'rte_power_uncore_init()' it is
> > > too late because other APIs already called.
> >
> > You are also updating the uncore API.
> >
> > > + if (env == RTE_UNCORE_PM_ENV_AUTO_DETECT)
> > > + /* Currently only intel_uncore is supported. This will be
> > > + * extended with auto-detection support for multiple uncore
> > > + * implementations.
> > > + */
> > > + env = RTE_UNCORE_PM_ENV_INTEL_UNCORE;
> >
> > It looks like this patch does not make sense without AMD support.
> >
> This patch is fixing a regression introduced by an earlier patch in this
> area (referenced in the fixes line). See bugzilla for more details on it[1]
>
> This should go into 23.11 as, without it, what was working in earlier
> releases no longer does so.
Thank you for the heads-up, I've looked at it too much quickly.
27/10/2023 14:37, Hunt, David:
> On 27/10/2023 13:36, Kelly, Karen wrote:
> > On 26/10/2023 16:19, Sivaprasad Tummala wrote:
> >> Updated the l3fwd-power app to configure the uncore env before invoking
> >> any uncore APIs. With auto-detection in 'rte_power_uncore_init()' it is
> >> too late because other APIs already called.
> >>
> >> Bugzilla ID: 1304
> >> Fixes: ac1edcb6621a ("power: refactor uncore power management API")
> >> Cc:karen.kelly@intel.com
> >>
> >> Signed-off-by: Sivaprasad Tummala<sivaprasad.tummala@amd.com>
> >> ---
> >> examples/l3fwd-power/main.c | 6 ++++++
> >> lib/power/rte_power_uncore.c | 7 +++++++
> >> lib/power/rte_power_uncore.h | 9 +++++----
> >> 3 files changed, 18 insertions(+), 4 deletions(-)
> > Ran on my system, bug is resolved after applying this patch.
> >
> > Tested-by: Karen Kelly <karen.kelly@intel.com>
> >
>
> Acked-by: David Hunt <david.hunt@intel.com>
So I merge it to fix a bug in the library, despite the patch title.
I am fixing a typo, and the commit description.
Applied
@@ -1561,6 +1561,12 @@ parse_uncore_options(enum uncore_choice choice, const char *argument)
{
unsigned int die, pkg, max_pkg, max_die;
int ret = 0;
+ ret = rte_power_set_uncore_env(RTE_UNCORE_PM_ENV_AUTO_DETECT);
+ if (ret < 0) {
+ RTE_LOG(INFO, L3FWD_POWER, "Failed to set uncore env\n");
+ return ret;
+ }
+
max_pkg = rte_power_uncore_get_num_pkgs();
if (max_pkg == 0)
return -1;
@@ -105,6 +105,13 @@ rte_power_set_uncore_env(enum rte_uncore_power_mgmt_env env)
return -1;
}
+ if (env == RTE_UNCORE_PM_ENV_AUTO_DETECT)
+ /* Currently only intel_uncore is supported. This will be
+ * extended with auto-detection support for multiple uncore
+ * implementations.
+ */
+ env = RTE_UNCORE_PM_ENV_INTEL_UNCORE;
+
ret = 0;
if (env == RTE_UNCORE_PM_ENV_INTEL_UNCORE) {
rte_power_get_uncore_freq = power_get_intel_uncore_freq;
@@ -21,15 +21,16 @@ extern "C" {
/* Uncore Power Management Environment */
enum rte_uncore_power_mgmt_env {
RTE_UNCORE_PM_ENV_NOT_SET,
+ RTE_UNCORE_PM_ENV_AUTO_DETECT,
RTE_UNCORE_PM_ENV_INTEL_UNCORE,
RTE_UNCORE_PM_ENV_AMD_HSMP
};
/**
- * Set the default uncore power management implementation. If this is not called prior
- * to rte_power_uncore_init(), then auto-detect of the environment will take place.
- * It is thread safe. New env can be set only in uninitialized state
- * (thus rte_power_unset_uncore_env must be called if different env was already set).
+ * Set the default uncore power management implementation. This has to be called
+ * prior to calling any other rte_power_uncore_*() API.
+ * It is thread safe. New env can be set only in uninitialized state.
+ * rte_power_unset_uncore_env must be called if different env was already set).
*
* @param env
* env. The environment in which to initialise Uncore Power Management for.