>-----Original Message----- >From: Len Brown [mailto:lenb@xxxxxxxxxx] >Sent: Friday, July 31, 2009 12:29 AM >To: Yu, Ke >Cc: linux-acpi@xxxxxxxxxxxxxxx; Jeremy Fitzhardinge; Tian, Kevin >Subject: RE: [Xen-devel] [PATCH][pvops_dom0][2/4] Introduce the external >control operation interface for domain0 ACPI parser > >Unclear that the power management partitioning between xen hypervisor >and dom0 is fully baked. > >Uncear (to me) what xen is doing internally with these power management >objects, and how that differs from what Linux would do. > >While patches to the Linux kernel may be a good RFE, prototype, or base >for discussion, the unknowns above need to be addressed to before it >makes much sense to spent a large amount of time on the source. Oh yes, I would like to add the background. In xen architecture, only hypervisor can control the physical CPU, so all the physical CPU Cx/Px power management is done in hypervisor side. And the cpuidle/cpufreq driver in dom0 is disabled. the Xen PM algorithm is ported from linux side, so it is similar as linux. For cpuidle part, when idle vcpu is scheduled (similar as idle process in linux), it will invoke cpuidle routine. cpuidle routine firstly decide which C state to go by calling cpuidle governor (e.g. menu governor), then cpuidle routine will enter C state using the ACPI method (e.g. I/O port, mwait described in _CST). For cpufreq part, cpufreq driver will register the governor (e.g. ondemand) and cpu driver (cpufreq-acpi driver). once the cpufreq start on certain CPU, the governor will change the frequency by calling the corresponding cpu driver. http://wiki.xensource.com/xenwiki/xenpm has one picture on the cpufreq architecture. All the necessary ACPI information in the above process is parsed by dom0 and pass to hypervisor. > >some things did jump out of the patch, however... > >I do not recommend believing _PSD. >Our experience is that 50% of the time it is crap. That is true, we also meet crap _PSD in especially the new platform. We will add more check on the _PSD data. > >Why does xen_processor_px exists when it is the same as acpi_processor_px? >ditto for acpi_processor_cx and xen_processor_cx xen_processor_px is part of the hypercall interface, and mainly used in hypervisor. since hypervisor has no acpi_processor_px as kernel has, so we have to add the same data structure for information passing purpose. similar for xen_processor_cx. > >Lose the ifdefs Can you elaborate which part lose the ifdef? for the drivers/xen/processor_extcntl.c, since it has "obj-$(CONFIG_ACPI_PROCESSOR_XEN) += processor_extcntl.o" in Makefile, it implies #ifdef CONFIG_ACPI_PROCESSOR_XEN. >Lose the tests for xen on the inline code when >they can be inside the called routines. Can you elaborate which part? > >This doesn't look like an abstraction layer, >it looks more like a simple conduit. >The thing at the other end (xen) will need to know >just as much about these data structures as the >thing that sent them (linux) The reason is that xen and linux use the similar power management algorithm, so the data structure xen need is also similar as Linux side. This actually make life easier, in that we don't need to invent an abstraction layer here to handle the difference, since there is actually no difference. > >the patch doesn't apply to the upstream kernel -- >even the ACPI specific parts. That is strange. I can successfully apply the ACPI specific parts to the upstream (linus 2.6 tree, parent commit b592972). For the xen part, it is based on Jeremy's git tree (git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git) rebase/master branch, so it cannot be cleanly applied to upstream. > >checkpatch.pl: >total: 168 errors, 42 warnings, 643 lines checked > >Don't send me another patch that can't pass checkpatch.pl, >even if just an RFC. Sorry for that. I have fix the code style issue and the attached updated version can pass the checkpatch.pl BTW, thanks for your comments. Best Regards Ke
Attachment:
03.patch
Description: 03.patch
