Thread (69 messages) 69 messages, 6 authors, 2021-03-02

Re: [dpdk-dev] [PATCH 0/6] power: fix make build for power apps

From: David Hunt <hidden>
Date: 2021-01-13 13:25:36

On 13/1/2021 11:18 AM, Burakov, Anatoly wrote:
On 13-Jan-21 11:14 AM, David Hunt wrote:
quoted
Hi Anatoly,

On 13/1/2021 11:08 AM, Burakov, Anatoly wrote:
quoted
On 08-Jan-21 2:30 PM, David Hunt wrote:
quoted
The power example applications that uses the virtio-serial method of
communication cannot currently be built with make, and can only be 
built
using meson/ninja.

The guest channel message definitions and functions in guest_channel.h
are needed by applications and need to be made public.

This worked pre-20.11, but now with all the meson/ninja changes, 
making
these apps externally no longer works. To fix, we need to move the 
header
file with the API definitions for the channel commands public, and 
rename
the functions accordingly.

The main change is to rename channel_commands.h to
rte_power_guest_channel.h so that it gets picked up by the 
installer and
copied to /usr/local/include. Other changes include renaming 
#defines to
have RTE_ at the beginning instead of CPU_. Finally we refactor the 
code
to work with those changes.

---
v2 changes
   - re-worked from monolithic patch to a 6 patch patchset for 
easier review

[PATCH v2 1/6] power: create guest channel public header file
[PATCH v2 2/6] power: make channel msg functions public
[PATCH v2 3/6] power: rename public structs
[PATCH v2 4/6] power: rename defines
[PATCH v2 5/6] power: add new header file to export list
[PATCH v2 6/6] power: clean up includes
Just a general question: wouldn't it be better to move this stuff 
entirely into sample app and not bother with keeping it in the 
library? There is precedent - ethtool app has a "library" and an 
"application" part, i think you should be able to move it out of the 
library and into the sample app entirely without too much trouble, 
as code looks to be fairly self-contained.
Agreed, that's a great idea. I could have a common lib under 
examples/vm_power_manager, then two apps, vm_power_manager and 
guest_cli. That would keep everything nicely local, and not expose 
the channel API publicly. The only reason we were making it public 
was to allow "make" to work, so that's not a good enought reason, 
tbh. I'll throw a prototype together today.
Yep, IIRC Make works perfectly fine with ethtool, so i don't see why 
it wouldn't work for your sample app as well. Thanks!

Hi Anatoly,

OK, so I was investigating this, and have come across a blocker on this 
method.

There are three methods for managing frequency, acpi, pstate and vm. 
It's the third method that's causing the problem with moving the channel 
functionality out into a sample library alongside vm_power_manger. VM's 
can call channel functions in the power library to affect frequency for 
their cores, and these functions use api function calls and several 
structures and #defines in their code, which is currently part of the 
power management library. These function declarations, structs and 
#defines ere needed in both the examples lib/apps and the power library 
itself, and the prototype changes I made ended up looking very much like 
the patch set that's already on the mailing list.

So, while I would have liked to have a solution along the lines of what 
you've proposed, it's not possible based on the dependencies on common 
structues and #defines.

Thanks for the suggestion, though.

Rgds,
Dave.




Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help