> -----Original Message----- > From: Neil Horman [mailto:nhorman at tuxdriver.com] > Sent: Thursday, September 25, 2014 11:10 AM > To: Carew, Alan > Cc: dev at dpdk.org > Subject: Re: [PATCH v2 07/10] librte_power common interface for Guest and > Host > > On Wed, Sep 24, 2014 at 06:26:13PM +0100, Alan Carew wrote: > > Moved the current librte_power implementation to rte_power_acpi_cpufreq, > with > > renaming of functions only. > > Added rte_power_kvm_vm implmentation to support Power Management > from a VM. > > > > librte_power now hides the implementation based on the environment used. > > A new call rte_power_set_env() can explicidly set the environment, if not > > called then auto-detection takes place. > > > > rte_power_kvm_vm is subset of the librte_power APIs, the following is > supported: > > rte_power_init(unsigned lcore_id) > > rte_power_exit(unsigned lcore_id) > > rte_power_freq_up(unsigned lcore_id) > > rte_power_freq_down(unsigned lcore_id) > > rte_power_freq_min(unsigned lcore_id) > > rte_power_freq_max(unsigned lcore_id) > > > > The other unsupported APIs return -ENOTSUP > > > > Signed-off-by: Alan Carew <alan.carew at intel.com> > > --- > > lib/librte_power/rte_power.c | 540 > > ++++------------------------- > > lib/librte_power/rte_power.h | 120 +++++-- > > lib/librte_power/rte_power_acpi_cpufreq.c | 545 > ++++++++++++++++++++++++++++++ > > lib/librte_power/rte_power_acpi_cpufreq.h | 192 +++++++++++ > > lib/librte_power/rte_power_common.h | 39 +++ > > lib/librte_power/rte_power_kvm_vm.c | 160 +++++++++ > > lib/librte_power/rte_power_kvm_vm.h | 179 ++++++++++ > > 7 files changed, 1273 insertions(+), 502 deletions(-) > > create mode 100644 lib/librte_power/rte_power_acpi_cpufreq.c > > create mode 100644 lib/librte_power/rte_power_acpi_cpufreq.h > > create mode 100644 lib/librte_power/rte_power_common.h > > create mode 100644 lib/librte_power/rte_power_kvm_vm.c > > create mode 100644 lib/librte_power/rte_power_kvm_vm.h > > > > diff --git a/lib/librte_power/rte_power.c b/lib/librte_power/rte_power.c > > index 856da9a..998ed1c 100644 > > --- a/lib/librte_power/rte_power.c > > +++ b/lib/librte_power/rte_power.c > > @@ -31,515 +31,113 @@ > > * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > DAMAGE. > > */ > > > > -#include <stdio.h> > > -#include <sys/types.h> > > -#include <sys/stat.h> > > -#include <fcntl.h> > > -#include <stdlib.h> > > -#include <string.h> > > -#include <unistd.h> > > -#include <signal.h> > > -#include <limits.h> > > - > > -#include <rte_memcpy.h> > > #include <rte_atomic.h> > > > > #include "rte_power.h" > > +#include "rte_power_acpi_cpufreq.h" > > +#include "rte_power_kvm_vm.h" > > +#include "rte_power_common.h" > > > > -#ifdef RTE_LIBRTE_POWER_DEBUG > > -#define POWER_DEBUG_TRACE(fmt, args...) do { \ > > - RTE_LOG(ERR, POWER, "%s: " fmt, __func__, ## args); \ > > - } while (0) > > -#else > > -#define POWER_DEBUG_TRACE(fmt, args...) > > -#endif > > - > > -#define FOPEN_OR_ERR_RET(f, retval) do { \ > > - if ((f) == NULL) { \ > > - RTE_LOG(ERR, POWER, "File not openned\n"); \ > > - return (retval); \ > > - } \ > > -} while(0) > > - > > -#define FOPS_OR_NULL_GOTO(ret, label) do { \ > > - if ((ret) == NULL) { \ > > - RTE_LOG(ERR, POWER, "fgets returns nothing\n"); \ > > - goto label; \ > > - } \ > > -} while(0) > > - > > -#define FOPS_OR_ERR_GOTO(ret, label) do { \ > > - if ((ret) < 0) { \ > > - RTE_LOG(ERR, POWER, "File operations failed\n"); \ > > - goto label; \ > > - } \ > > -} while(0) > > - > > -#define STR_SIZE 1024 > > -#define POWER_CONVERT_TO_DECIMAL 10 > > +enum power_management_env global_default_env = PM_ENV_NOT_SET; > > > > -#define POWER_GOVERNOR_USERSPACE "userspace" > > -#define POWER_SYSFILE_GOVERNOR \ > > - "/sys/devices/system/cpu/cpu%u/cpufreq/scaling_governor" > > -#define POWER_SYSFILE_AVAIL_FREQ \ > > - > "/sys/devices/system/cpu/cpu%u/cpufreq/scaling_available_frequencie > s" > > -#define POWER_SYSFILE_SETSPEED \ > > - "/sys/devices/system/cpu/cpu%u/cpufreq/scaling_setspeed" > > +volatile uint32_t global_env_cfg_status = 0; > > > > -enum power_state { > > - POWER_IDLE = 0, > > - POWER_ONGOING, > > - POWER_USED, > > - POWER_UNKNOWN > > -}; > > +/* function pointers */ > > +rte_power_freqs_t rte_power_freqs = NULL; > > +rte_power_get_freq_t rte_power_get_freq = NULL; > > +rte_power_set_freq_t rte_power_set_freq = NULL; > > +rte_power_freq_change_t rte_power_freq_up = NULL; > > +rte_power_freq_change_t rte_power_freq_down = NULL; > > +rte_power_freq_change_t rte_power_freq_max = NULL; > > +rte_power_freq_change_t rte_power_freq_min = NULL; > > > > -/** > > - * Power info per lcore. > > - */ > > -struct rte_power_info { > > - unsigned lcore_id; /**< Logical core id */ > > - uint32_t freqs[RTE_MAX_LCORE_FREQS]; /**< Frequency array */ > > - uint32_t nb_freqs; /**< number of available freqs */ > > - FILE *f; /**< FD of scaling_setspeed */ > > - char governor_ori[32]; /**< Original governor name */ > > - uint32_t curr_idx; /**< Freq index in freqs array */ > > - volatile uint32_t state; /**< Power in use state */ > > -} __rte_cache_aligned; > > - > > -static struct rte_power_info lcore_power_info[RTE_MAX_LCORE]; > > - > > -/** > > - * It is to set specific freq for specific logical core, according to the > > index > > - * of supported frequencies. > > - */ > > -static int > > -set_freq_internal(struct rte_power_info *pi, uint32_t idx) > > +int > > +rte_power_set_env(enum power_management_env env) > > { > > - if (idx >= RTE_MAX_LCORE_FREQS || idx >= pi->nb_freqs) { > > - RTE_LOG(ERR, POWER, "Invalid frequency index %u, which " > > - "should be less than %u\n", idx, pi->nb_freqs); > > - return -1; > > - } > > - > > - /* Check if it is the same as current */ > > - if (idx == pi->curr_idx) > > + if (rte_atomic32_cmpset(&global_env_cfg_status, 0, 1) == 0) { > > return 0; > > - > 1 Nit here. If an invalid environment value is passed in on the first config > attempt here, you won't ever be able to set it. Maybe add some logic to > return > us to an initial state if a value env isn't selected? > > Neil
Hi Neil, I should have called it out in the commit, but there's also a rte_power_unset_env() function that resets the environment that allows for retrying a different environment. rte_power_unset_env() is also called when an invalid configuration is set. Thanks, Alan.
