Re: [PATCH v10 0/6] power: refactor power management library

[David Marchand <david.marchand@redhat.com>]

Hello Siva, Thomas,

On Sun, Nov 10, 2024 at 7:36 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 28/10/2024 20:55, Sivaprasad Tummala:
> > This patchset refactors the power management library, addressing both
> > core and uncore power management. The primary changes involve the
> > creation of dedicated directories for each driver within
> > 'drivers/power/core/*' and 'drivers/power/uncore/*'.
> >
> > This refactor significantly improves code organization, enhances
> > clarity, and boosts maintainability. It lays the foundation for more
> > focused development on individual drivers and facilitates seamless
> > integration of future enhancements, particularly the AMD uncore driver.
> >
> > Furthermore, this effort aims to streamline code maintenance by
> > consolidating common functions for cpufreq and cppc across various
> > core drivers, thus reducing code duplication.
> >
> > Sivaprasad Tummala (6):
> >   power: refactor core power management library
> >   power: refactor uncore power management library
> >   test/power: removed function pointer validations
> >   drivers/power: uncore support for AMD EPYC processors
> >   maintainers: update for drivers/power
> >   power: rename library sources for cpu frequency management
>
> I'm a bit sad there is not more reviews.
>
> I've moved the pointers check removal first,
> renamed intel_pstate files (not the functions),
> fixed few things like __cplusplus, include guards,
> sorting and maintainers file.
>
> Applied

This series breaks compilation of the vm_power_manager example as the
"guest channel" API symbols are not provided by the power library
(itself) anymore.

ninja: Entering directory `/home/dmarchan/builds/main/build-gcc-shared'
[3355/3373] Linking target examples/dpdk-guest_cli
FAILED: examples/dpdk-guest_cli
gcc  -o examples/dpdk-guest_cli
examples/dpdk-guest_cli.p/vm_power_manager_guest_cli_main.c.o
examples/dpdk-guest_cli.p/vm_power_manager_guest_cli_parse.c.o
examples/dpdk-guest_cli.p/vm_power_manager_guest_cli_vm_power_cli_guest.c.o
-Wl,--as-needed -Wl,--no-undefined -Wl,--no-as-needed
-Wl,--undefined-version -pthread -Wl,--start-group -lm -ldl -lnuma
-lfdt '-Wl,-rpath,$ORIGIN/../lib'
-Wl,-rpath-link,/home/dmarchan/builds/main/build-gcc-shared/lib
lib/librte_eal.so.25.0 lib/librte_kvargs.so.25.0
lib/librte_log.so.25.0 lib/librte_telemetry.so.25.0
lib/librte_mempool.so.25.0 lib/librte_ring.so.25.0
lib/librte_net.so.25.0 lib/librte_mbuf.so.25.0
lib/librte_ethdev.so.25.0 lib/librte_meter.so.25.0
lib/librte_cmdline.so.25.0 lib/librte_power.so.25.0
lib/librte_timer.so.25.0 -lpcap -lvirt /usr/lib64/libbsd.so
/usr/lib64/libarchive.so -Wl,--end-group
/usr/bin/ld: examples/dpdk-guest_cli.p/vm_power_manager_guest_cli_vm_power_cli_guest.c.o:
in function `check_response_cmd':
/home/dmarchan/builds/main/build-gcc-shared/../../../git/pub/dpdk.org/main/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c:382:
undefined reference to `rte_power_guest_channel_receive_msg'
/usr/bin/ld: examples/dpdk-guest_cli.p/vm_power_manager_guest_cli_vm_power_cli_guest.c.o:
in function `query_data':
/home/dmarchan/builds/main/build-gcc-shared/../../../git/pub/dpdk.org/main/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c:147:
undefined reference to `rte_power_guest_channel_send_msg'
/usr/bin/ld: examples/dpdk-guest_cli.p/vm_power_manager_guest_cli_vm_power_cli_guest.c.o:
in function `receive_capabilities':
/home/dmarchan/builds/main/build-gcc-shared/../../../git/pub/dpdk.org/main/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c:271:
undefined reference to `rte_power_guest_channel_receive_msg'
/usr/bin/ld: examples/dpdk-guest_cli.p/vm_power_manager_guest_cli_vm_power_cli_guest.c.o:
in function `send_policy':
/home/dmarchan/builds/main/build-gcc-shared/../../../git/pub/dpdk.org/main/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c:476:
undefined reference to `rte_power_guest_channel_send_msg'
/usr/bin/ld: examples/dpdk-guest_cli.p/vm_power_manager_guest_cli_vm_power_cli_guest.c.o:
in function `query_data':
/home/dmarchan/builds/main/build-gcc-shared/../../../git/pub/dpdk.org/main/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c:147:
undefined reference to `rte_power_guest_channel_send_msg'
/usr/bin/ld: examples/dpdk-guest_cli.p/vm_power_manager_guest_cli_vm_power_cli_guest.c.o:
in function `receive_freq_list':
/home/dmarchan/builds/main/build-gcc-shared/../../../git/pub/dpdk.org/main/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c:161:
undefined reference to `rte_power_guest_channel_receive_msg'
collect2: error: ld returned 1 exit status
[3357/3373] Generating drivers/rte_common_cnxk.sym_chk with a custom
command (wrapped by meson to capture output)
ninja: build stopped: subcommand failed.

Siva, please have a look quickly.

Here is a quick fix written before first coffee of the day:
$ git diff --cached
diff --git a/drivers/power/kvm_vm/meson.build b/drivers/power/kvm_vm/meson.build
index fe11179ab3..e921c012e9 100644
--- a/drivers/power/kvm_vm/meson.build
+++ b/drivers/power/kvm_vm/meson.build
@@ -10,5 +10,6 @@ sources = files(
         'guest_channel.c',
         'kvm_vm.c',
 )
+headers = files('rte_power_guest_channel.h')

 deps += ['power']
diff --git a/lib/power/rte_power_guest_channel.h
b/drivers/power/kvm_vm/rte_power_guest_channel.h
similarity index 100%
rename from lib/power/rte_power_guest_channel.h
rename to drivers/power/kvm_vm/rte_power_guest_channel.h
diff --git a/drivers/power/kvm_vm/version.map b/drivers/power/kvm_vm/version.map
new file mode 100644
index 0000000000..ffa676624b
--- /dev/null
+++ b/drivers/power/kvm_vm/version.map
@@ -0,0 +1,8 @@
+DPDK_25 {
+       global:
+
+       rte_power_guest_channel_receive_msg;
+       rte_power_guest_channel_send_msg;
+
+       local: *;
+};
diff --git a/examples/vm_power_manager/guest_cli/meson.build
b/examples/vm_power_manager/guest_cli/meson.build
index a69f809e3b..bc3916a170 100644
--- a/examples/vm_power_manager/guest_cli/meson.build
+++ b/examples/vm_power_manager/guest_cli/meson.build
@@ -6,7 +6,7 @@
 # To build this example as a standalone application with an already-installed
 # DPDK instance, use 'make'

-deps += ['power']
+deps += ['power', 'power/kvm_vm']

 sources = files(
         'main.c',
diff --git a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
index 803b6d1f82..14d1f3dd95 100644
--- a/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
+++ b/examples/vm_power_manager/guest_cli/vm_power_cli_guest.c
@@ -19,6 +19,7 @@
 #include <rte_ethdev.h>

 #include <rte_power_cpufreq.h>
+#include <rte_power_guest_channel.h>

 #include "vm_power_cli_guest.h"

diff --git a/examples/vm_power_manager/meson.build
b/examples/vm_power_manager/meson.build
index b866d8fd54..1903b68ed9 100644
--- a/examples/vm_power_manager/meson.build
+++ b/examples/vm_power_manager/meson.build
@@ -6,7 +6,7 @@
 # To build this example as a standalone application with an already-installed
 # DPDK instance, use 'make'

-deps += ['power']
+deps += ['power', 'power/kvm_vm']

 if dpdk_conf.has('RTE_NET_BNXT')
     deps += ['net_bnxt']
diff --git a/lib/power/meson.build b/lib/power/meson.build
index cd7c83b6e9..b3a7bc7b2e 100644
--- a/lib/power/meson.build
+++ b/lib/power/meson.build
@@ -22,7 +22,6 @@ headers = files(
         'power_cpufreq.h',
         'power_uncore_ops.h',
         'rte_power_cpufreq.h',
-        'rte_power_guest_channel.h',
         'rte_power_pmd_mgmt.h',
         'rte_power_qos.h',
         'rte_power_uncore.h',
diff --git a/lib/power/rte_power_cpufreq.h b/lib/power/rte_power_cpufreq.h
index 73f9820bdf..82d274214b 100644
--- a/lib/power/rte_power_cpufreq.h
+++ b/lib/power/rte_power_cpufreq.h
@@ -13,7 +13,6 @@

 #include <rte_common.h>
 #include <rte_log.h>
-#include <rte_power_guest_channel.h>

 #include "power_cpufreq.h"

diff --git a/lib/power/version.map b/lib/power/version.map
index 920c8e79b3..9a36046a64 100644
--- a/lib/power/version.map
+++ b/lib/power/version.map
@@ -16,8 +16,6 @@ DPDK_25 {
        rte_power_get_env;
        rte_power_get_freq;
        rte_power_get_uncore_freq;
-       rte_power_guest_channel_receive_msg;
-       rte_power_guest_channel_send_msg;
        rte_power_init;
        rte_power_pmd_mgmt_get_emptypoll_max;
        rte_power_pmd_mgmt_get_pause_duration;



-- 
David Marchand