[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC PATCH v5 11/12] cpufreq: add hwdom-cpufreq driver

>>> On 11.11.14 at 14:17, <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx> wrote:
> This driver uses hwdom to change frequencies on physical
> CPUs.
> Workflow:
>  * cpufreq governor driver in Xen wants to change the
>    frequency of the physical CPU
>  * hwdom-cpufreq driver sets parameters in the shared
>    memory
>  * hwdom-cpufreq driver sends an event via event channel
>    to notify the hardware domain
>  * cpufreq driver in the hardware domain reads parameters
>    from the shared memory, changes frequency and copies
>    the result of the operation to the shared memory
>  * cpufreq driver in the hwdom sends an event via event
>    channel to notify the hwdom-cpufreq driver

Without proof that the architecturally more sound model of having
the hypervisor do the hardware manipulation is indeed unusable
I continue to not agree to this going in. I don't think I've seen any
numbers on the claimed slowness when using hypercalls for the
I2C accesses you need multiplexed for this, and the need to
multiplex them in the first place is a sign of a virtualization
unfriendly system design, which I'm not sure we really want/need
to accept a large piece of code to work around.

> --- a/xen/drivers/cpufreq/Makefile
> +++ b/xen/drivers/cpufreq/Makefile
> @@ -2,3 +2,4 @@ obj-y += cpufreq.o
>  obj-y += cpufreq_ondemand.o
>  obj-y += cpufreq_misc_governors.o
>  obj-y += utility.o
> +obj-$(HAS_HWDOM_CPUFREQ) += hwdom-cpufreq.o

Unless you strictly need this at the end, this should go ahead of
utility.o. It's also questionable whether under cpufreq/ you need
to name a file *-cpufreq.*.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.