Re: High CPU temp, suspend problem - xen 4.1.5-pre, linux 3.7.x

[konrad wilk <konrad.wilk@oracle.com>]

On 4/15/2013 8:46 PM, Ben Guthro wrote:
> On Tue, Apr 16, 2013 at 1:19 AM, Ben Guthro <ben@guthro.net> wrote:
>> On Tue, Apr 16, 2013 at 12:51 AM, konrad wilk <konrad.wilk@oracle.com> wrote:
>>>>> 3. ACPI C-states are only present for CPU0 (after resume of course),
>>>>> fixed by
>>>>> "xen: Re-upload processor PM data to hypervisor after S3" patch by Ben,
>>>>> but it
>>>>> isn't in upstream linux (nor Konrad's acpi-s3 branches).
>>>> I don't recall seeing any ACK / NAK from Konrad on this.
>>>>
>>>> Original post:
>>>> https://patchwork.kernel.org/patch/2033981/
>>>>
>>>> Konrad - do you have any thoughts about incorporating this into a
>>>> future merge window?
>>>
>>> Hey Ben,
>>> I seem to have missed it.
>>> I think the patch is missing a change to pr_backup->acpi_id = i, otherwise
>>> it would resend
>>> the C-states with the same APIC ID. Also the upstream version does
>>> kfree(pr_backup) at some point.
>> Hmm. I'll look into this, and re-submit.
> At the risk of seeming a bit dim, could you elaborate a bit here?
Part of what xen-acpi-processor has to deal with is the 
'dom0_max_vcpus=' case. Which means that when
'acpi_processor_get_performance_info' is called to parse ACPI C-states 
it will limit itself to only the 'online'
CPUs it sees. Meaning that all the other ones (which might be physically 
present) which Linux does not see are skipped.

As such there is this:

545                 if (!pr_backup) {
546                         pr_backup = kzalloc(sizeof(struct 
acpi_processor), GFP_KERNEL);
547                         if (pr_backup)
548                                 memcpy(pr_backup, _pr, sizeof(struct 
acpi_processor));
549                 }

And then later

552         rc = check_acpi_ids(pr_backup);

which walks the ACPI namespace checking whether it has uploaded the 
ACPI-IDs for all the CPUs. If there
are some that are missing (b/c dom0_max_vcpus=X) was used, then it 
uploads the pr_backup with the ACPI ID altered.

What I think you ought to try is just to call check_acpi_ids after the 
for_cpu_online() loop with the pr_backup.

Hm, you could actually make this even easier. Just move this code:

539         for_each_possible_cpu(i) {
540                 struct acpi_processor *_pr;
541                 _pr = per_cpu(processors, i /* APIC ID */);
542                 if (!_pr)
543                         continue;
544
545                 if (!pr_backup) {
546                         pr_backup = kzalloc(sizeof(struct 
acpi_processor), GFP_KERNEL);
547                         if (pr_backup)
548                                 memcpy(pr_backup, _pr, sizeof(struct 
acpi_processor));
549                 }
550                 (void)upload_pm_data(_pr);
551         }
552         rc = check_acpi_ids(pr_backup);

in its own function. Then make both the module loading _and_ the syscore 
resume call said function.
Viola!

Naturally the kfree(pr_backup) and pr_backup = NULL have to be 
eliminated from the module_init function.. and the module_exit needs the 
pr_backup moved past the syscore_unregister.