Re: [Xen-devel] [RFC PATCH 7/7] xen/arm: cpufreq: add cpufreq driver

[Oleksandr Dmytryshyn <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx>]

On Tue, Oct 7, 2014 at 6:57 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> Hi Oleksandr,
>
> On 10/07/2014 03:20 PM, Oleksandr Dmytryshyn wrote:
>> Xen changes frequencies on CPUs using this driver.
>>
>> Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx>
>> ---
>>  drivers/xen/Kconfig         |  20 +
>>  drivers/xen/Makefile        |   1 +
>>  drivers/xen/xen-cpufreq.c   | 882 
>> ++++++++++++++++++++++++++++++++++++++++++++
>>  include/xen/interface/xen.h |   1 +
>>  4 files changed, 904 insertions(+)
>>  create mode 100644 drivers/xen/xen-cpufreq.c
>>
>> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
>> index 254a5cc..bb2d3d5 100644
>> --- a/drivers/xen/Kconfig
>> +++ b/drivers/xen/Kconfig
>> @@ -220,6 +220,26 @@ config XEN_ACPI_PROCESSOR
>>         called xen_acpi_processor  If you do not know what to choose, select
>>         M here. If the CPUFREQ drivers are built in, select Y here.
>>
>> +config XEN_CPUFREQ
>> +     bool "Xen Cpufreq driver"
>> +     depends on XEN_DOM0 && !CPU_FREQ
>
> Why xen-cpufreq is only available when CPU_FREQ is not present?
>
> As said early, a same kernel should be able to run as Xen domain (DOM0
> and other guests) and bare metal.
I can done it in this way.
There will be 2 cpufreq drivers in the Kernel: standart driver
(drivers/cpufreq/cpufreq.c)
and my driver: xen-cpufreq. I can create cpufreq driver ops and call it from
standart cpufreq functions (cpufreq_driver_target(), etc.) When kernel will boot
it will detect where it is running. If it is running under the Xen it will
register functions from xen-cpufreq driver and it will register functions
from drivers/cpufreq/cpufreq.c otherwise. In this case we can use only
one cpufreq driver at one time.

I have a question. Do we need that 2 cpufreq drivers will be used at the same
time (for example, Xen changes frequency of physical CPUs using
xen-cpufreq driver
and Kernel Dom0 chandes frequensy of virtual CPUs using
drivers/cpufreq/cpufreq.c
driver)?


>> +     default n
>> +     help
>> +       This driver uploads Power Management information to the Xen
>> +       hypervisor and changes CPUs frequency using CPU Frequency scaling
>> +       drivers.
>> +
>> +       To do that the driver uses CPU Frequency scaling drivers to parse
>> +       the Power Management data and uploads said information to the Xen
>> +       hypervisor. Then the Xen hypervisor can select the proper Pxx states.
>> +
>> +       Then the Xen hypervisor can change CPUs frequency by giving commands
>> +       via this driver to the CPU Frequency scaling driver.
>> +
>> +       To compile this driver as a module, choose M here: the module will be
>> +       called xen_acpi_processor  If you do not know what to choose, select
>
> Is guess you copied the config help from xen_acpi_processor?
Yes.


>> +       M here. If the CPUFREQ drivers are built in, select Y here.
>> +
>>  config XEN_MCE_LOG
>>       bool "Xen platform mcelog"
>>       depends on XEN_DOM0 && X86_64 && X86_MCE
>> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
>> index b7c835f..0345d65 100644
>> --- a/drivers/xen/Makefile
>> +++ b/drivers/xen/Makefile
>> @@ -33,6 +33,7 @@ obj-$(CONFIG_XEN_PRIVCMD)           += xen-privcmd.o
>>  obj-$(CONFIG_XEN_ACPI_HOTPLUG_MEMORY)        += xen-acpi-memhotplug.o
>>  obj-$(CONFIG_XEN_ACPI_HOTPLUG_CPU)   += xen-acpi-cpuhotplug.o
>>  obj-$(CONFIG_XEN_ACPI_PROCESSOR)     += xen-acpi-processor.o
>> +obj-$(CONFIG_XEN_CPUFREQ)            += xen-cpufreq.o
>
> I think this new cpufreq driver should live in drivers/cpufreq, not in
> drivers/xen.
>
>>  xen-evtchn-y                         := evtchn.o
>>  xen-gntdev-y                         := gntdev.o
>>  xen-gntalloc-y                               := gntalloc.o
>> diff --git a/drivers/xen/xen-cpufreq.c b/drivers/xen/xen-cpufreq.c
>> new file mode 100644
>> index 0000000..a0d9adc
>> --- /dev/null
>> +++ b/drivers/xen/xen-cpufreq.c
>
> [..]
>
>> +#ifdef CONFIG_CPUMASK_OFFSTACK
>> +#error CONFIG_CPUMASK_OFFSTACK config should not be used with this driver
>> +#endif
>
> hmmm. Why?
Please, see file include/linux/cpumask.h in Kernel:
#ifdef CONFIG_CPUMASK_OFFSTACK
/* Assuming NR_CPUS is huge, a runtime limit is more efficient.  Also,
 * not all bits may be allocated. */
#define nr_cpumask_bits nr_cpu_ids
#else
#define nr_cpumask_bits NR_CPUS
#endif

If this config is defined, nr_cpumask_bits will be equal to nr_cpu_ids.
nr_cpu_ids will be equal to the VCPUs number. In this case the function
cpumask_setall() will fill only nr_cpu_ids bits and this function is
used in cpufreq-cpu0 driver. When VCPUs number is < PCPUs number
this function will set less bits then needed. And xen-cpufreq driver
will work not correctly.


> Regards,
>
> --
> Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel