From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kai Huang Subject: Re: [v2 06/11] vmx: add help functions to support PML Date: Fri, 24 Apr 2015 15:41:06 +0800 Message-ID: <5539F392.1020002@linux.intel.com> References: <1429081433-9600-1-git-send-email-kai.huang@linux.intel.com> <1429081433-9600-7-git-send-email-kai.huang@linux.intel.com> <552FF47B0200007800072E23@mail.emea.novell.com> <5530798C.2060108@linux.intel.com> <5530C30D02000078000730D4@mail.emea.novell.com> <5530AD7D.4020101@linux.intel.com> <5530CB4A0200007800073125@mail.emea.novell.com> <5530B4E6.1020907@linux.intel.com> <5530D471020000780007319D@mail.emea.novell.com> <5539E36A.9080203@linux.intel.com> <553A0D370200007800075768@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <553A0D370200007800075768@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: andrew.cooper3@citrix.com, kevin.tian@intel.com, tim@xen.org, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 04/24/2015 03:30 PM, Jan Beulich wrote: >>>> On 24.04.15 at 08:32, wrote: >> On 04/17/2015 03:37 PM, Jan Beulich wrote: >>>>>> On 17.04.15 at 09:23, 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