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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.04.15 at 08:32, <kai.huang@xxxxxxxxxxxxxxx> wrote:
> On 04/17/2015 03:37 PM, Jan Beulich wrote:
>>>>> On 17.04.15 at 09:23, <kai.huang@xxxxxxxxxxxxxxx> 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.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel