lkml.org 
[lkml]   [2019]   [Oct]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2] ACPI / processor_idle: use ndelay instead of io port access for wait
From
Date
On 10/18/2019 6:12 PM, Rafael J. Wysocki wrote:
> On Wednesday, October 16, 2019 7:56:17 AM CEST Yin, Fengwei wrote:
>> Hi David,
>>
>> On 10/15/2019 7:48 PM, David Laight wrote:
>>> From: Yin Fengwei
>>>> Sent: 15 October 2019 09:04
>>>> In function acpi_idle_do_entry(), an ioport access is used for dummy
>>>> wait to guarantee hardware behavior. But it could trigger unnecessary
>>>> vmexit in virtualization environment.
>>>>
>>>> If we run linux as guest and export all available native C state to
>>>> guest, we did see many PM timer access triggered VMexit when guest
>>>> enter deeper C state in our environment (We used ACRN hypervisor
>>>> instead of kvm or xen which has PM timer emulated and exports all
>>>> native C state to guest).
>>>>
>>>> According to the original comments of this part of code, io port
>>>> access is only for dummy wait. We could use busy wait instead of io
>>>> port access to guarantee hardware behavior and avoid unnecessary
>>>> VMexit.
>>>
>>> You need some hard synchronisation instruction(s) after the inb()
>>> and before any kind of delay to ensure your delay code is executed
>>> after the inb() completes.
>>>
>>> I'm pretty sure that inb() is only synchronised with memory reads.
>> Thanks a lot for the comments.
>>
>> I didn't find the common serializing instructions API in kernel (only
>> memory barrier which is used to make sure of memory access). For Intel
>> x86, cpuid could be used as serializing instruction. But it's not
>> suitable for common code here. Do you have any suggestion?
>
> In the virt guest case you don't need to worry at all AFAICS, because the inb()
> itself will trap to the HV.
This is not always valid. If the physical cpu is totally owned by guest
(not shared with other guest), it's possible we passthru the C state
port to guest. In that case, inb() which trigger C state transaction
doesn't trap to the HV.

>
>>>
>>> ...
>>>> + /* profiling the time used for dummy wait op */
>>>> + ktime_get_real_ts64(&ts0);
>>>> + inl(acpi_gbl_FADT.xpm_timer_block.address);
>>>> + ktime_get_real_ts64(&ts1);
>
> You may as well use ktime_get() for this, as it's almost the same code as
> ktime_get_real_ts64() AFAICS, only simpler.
>
> Plus, static vars need not be initialized to 0.
Thanks for pointing this out. Will update the patch accordingly.

>
>>>
>>> That could be dominated by the cost of ktime_get_real_ts64().
>>> It also need synchronising instructions.
>> I did some testing. ktime_get_real_ts64() takes much less time than io
>> port access.
>>
>> The test code is like:
>> 1.
>> local_irq_save(flag);
>> ktime_get_real_ts64(&ts0);
>> inl(acpi_gbl_FADT.xpm_timer_block.address);
>> ktime_get_real_ts64(&ts1);
>> local_irq_restore(flag);
>>
>> 2.
>> local_irq_save(flag);
>> ktime_get_real_ts64(&ts0);
>> ktime_get_real_ts64(&ts1);
>> local_irq_restore(flag);
>>
>> The delta in 1 is about 500000ns. And delta in 2 is about
>> 2000ns. The date is gotten on Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz.
>> So I suppose the impact of ktime_get_real_ts64 is small.
>
> You may not be hitting the worst case for ktime_get_real_ts64(), though.
>
> I wonder if special casing the virt guest would be a better approach.
>
> Then, you could leave the code as is for non-virt and I'm not sure if the
> delay is needed in the virt guest case at all.
>
> So maybe do something like "if not in a virt guest, do the dummy inl()"
> and that would be it?
Yes. This is better. Which we could control the impact to non-virt env.

Regards
Yin, Fengwei

>
>
>

\
 
 \ /
  Last update: 2019-10-18 12:39    [W:0.550 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site