[PATCH] examples/l3fwd-power: fix to configure the uncore env

[PATCH] examples/l3fwd-power: fix to configure the uncore env

[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.

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(-)

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index 5fbc16bb2a..9c0dcd343b 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -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;
diff --git a/lib/power/rte_power_uncore.c b/lib/power/rte_power_uncore.c
index 78a823d34c..d87328bedf 100644
--- a/lib/power/rte_power_uncore.c
+++ b/lib/power/rte_power_uncore.c
@@ -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;
diff --git a/lib/power/rte_power_uncore.h b/lib/power/rte_power_uncore.h
index 295017b7f4..ede50a5f87 100644
--- a/lib/power/rte_power_uncore.h
+++ b/lib/power/rte_power_uncore.h
@@ -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.
-- 
2.34.1

Re: [PATCH] examples/l3fwd-power: fix to configure the uncore env

[Kelly, Karen]

[-- Attachment #1: Type: text/plain, Size: 732 bytes --]


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>

[-- Attachment #2: Type: text/html, Size: 1380 bytes --]

Re: [PATCH] examples/l3fwd-power: fix to configure the uncore env

[Hunt, David]

[-- Attachment #1: Type: text/plain, Size: 867 bytes --]

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>

[-- Attachment #2: Type: text/html, Size: 2081 bytes --]

Re: [PATCH] examples/l3fwd-power: fix to configure the uncore env

[Thomas Monjalon]

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.

Re: [PATCH] examples/l3fwd-power: fix to configure the uncore env

[Ferruh Yigit]

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.

Re: [PATCH] examples/l3fwd-power: fix to configure the uncore env

[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.

Thanks,
/Bruce

[1] https://bugs.dpdk.org/show_bug.cgi?id=1304

Re: [PATCH] examples/l3fwd-power: fix to configure the uncore env

[Thomas Monjalon]

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.

Re: [PATCH] examples/l3fwd-power: fix to configure the uncore env

[Thomas Monjalon]

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