Jan Beulich wrote:
>>>> "Jan Beulich" <jbeulich@xxxxxxxxxx> 17.09.08 09:43 >>>
>> This changeset reverts two previous corrections, for reasons that
>> escape me.
>>
>> First, the domain map is again being confined to NR_CPUS, which I had
>> submitted a patch to fix recently (yes, I realize the code has a
>> TODO in there, but those really get forgotten about far too often).
>>
>> Second, the platform hypercall was reverted back to require all
>> information to be passed to Xen in one chunk, whereas I recall that
>> even Intel folks (not sure if it was you) agreed that allowing
>> incremental information collection was more appropriate.
>>
>> Could you clarify why these changes were necessary and if/when you
>> plan to address the resulting issues?
>
> Also, were these changes tested on AMD CPUs? It would seem to me
> that the cpufreq_cpu_policy array would remain uninitialized here, and
> hence the first access in the powernow code would dereference a NULL
> pointer.
[Jinsong]:
No, we didn't test on AMD CPUs, we don't have AMD platform. AMD
powernow copy our cpufreq code, and share some data structure, this in
fact may result in bugs.
Are you review/update powernow code recently? Recently, we rebase
cpufreq logic greatly, and add support to IPF arch, this work will
complete in 1 week.
After cpufreq rebase and IPF support complete,
1. all arch-independent logic (like policy, governor algorithm, px
statistic, S3 suspend-resume, ppc dynamic handle, and most cpufreq init
logic, etc) will move to hypervisor common part (xen/drivers/cpufreq and
xen/drivers/acpi);
2. all arch-dependent part (only cpufreq_driver) reside in arch
dependent dir (like xen/arch/x86/cpufreq (for x86 cpu) and
xen/arch/ia64/cpufreq (for ia64 cpu));
So how about update AMD powernow cpufreq 1 week later? at that time,
all powernow current policy/governor/init logic can be cancelled, share
our common logic is OK, only need to leave powernow's cpufreq_driver,
which is AMD arch-dependent.
>
> Likewise the calls to cpufreq_{add,del}_cpu() from the CPU hot(un)plug
> paths seem to consider the Intel case only (as the functions
> themselves are Intel specific).
[Jinsong]:
Not quite sure about your question. I just check the code:
1. cpufreq_add/del_cpu() is arch-independent, since all arch-dependent
part has been handled by cpufreq_driver->init/exit().
2. cpu online/offline path is also arch-independent.
Would you please tell me more clear where is intel specific?
Thanks,
Jinsong
>
> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|