Re: [v2 06/11] vmx: add help functions to support PML

[Kai Huang <kai.huang@linux.intel.com>]

On 04/24/2015 03:30 PM, Jan Beulich wrote:
>>>> On 24.04.15 at 08:32, <kai.huang@linux.intel.com> wrote:
>> On 04/17/2015 03:37 PM, Jan Beulich wrote:
>>>>>> On 17.04.15 at 09:23, <kai.huang@linux.intel.com> wrote:
>>>> I see. I will do as you suggested:
>>>>
>>>> ASSERT((v == current) || (!vcpu_runnable(v) && !v->is_running));
>>>>
>>>> And v != current should be the only case requires the vcpu to be paused.
>>> But if you require (or at least expect) the vCPU to be paused, this
>>> isn't what I suggested. Afaict
>>>
>>> ASSERT((v == current) || (!v->is_running && atomic_read(v->pause_count)));
>>>
>>> would then be the right check (and, while not be a full guarantee that
>>> things wouldn't change behind your back, would at least increase
>>> chances that the vCPU's runnable state won't change, as the vCPU
>>> could have been non-runnable for reasons other than having been
>>> paused).
>> Hi Jan,
>>
>> I tried the ASSERT with atomic_read(&v->pause_count), and it turns out
>> the ASSERT would fail and panic Xen. The reason is domain_pause only
>> increases d->pause_count, but it doesn't internally increase
>> v->pause_count for all vcpus.
>>
>> vmx_vcpu_flush_pml_buffer is only supposed to be called from PML buffer
>> full VMEXIT, and vmx_domain_flush_pml_buffer, before which domain_pause
>> should be called.
>>
>> Sorry that obviously I had some misunderstanding regarding to "require
>> (or at least expect) vCPU to be paused", and looks !vcpu_runnable(v) is
>> the right choice.
>>
>> What's your opinion?
> Then either go with the slightly weaker original (still quoted at the
> top) or (preferred by me) OR together both pause counts in the
> condition.
Thanks. Then I'd like to use original weaker one (below) instead of 
explicitly doing OR 'd->pause_count' and 'v->pause_count', as you are 
not objecting the former :)

ASSERT((v == current) || (!vcpu_runnable(v) && !v->is_running));

Thanks,
-Kai
>
> Jan
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel