From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH] pvh: Fix regression caused by assumption that HVM paths MUST use io-backend device. Date: Wed, 5 Feb 2014 14:35:51 +0000 Message-ID: <52F24C47.5070100@eu.citrix.com> References: <1391447001-19100-1-git-send-email-konrad.wilk@oracle.com> <1391447001-19100-2-git-send-email-konrad.wilk@oracle.com> <52F0B8F30200007800118E81@nat28.tlf.novell.com> <20140204144833.GE3853@phenom.dumpdata.com> <52F10F2402000078001190B7@nat28.tlf.novell.com> <20140204153258.GA6847@phenom.dumpdata.com> <52F119780200007800119172@nat28.tlf.novell.com> <20140204164258.GB7443@phenom.dumpdata.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WB3aH-00048Y-Kp for xen-devel@lists.xenproject.org; Wed, 05 Feb 2014 14:36:13 +0000 In-Reply-To: <20140204164258.GB7443@phenom.dumpdata.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: Konrad Rzeszutek Wilk , Jan Beulich Cc: yang.z.zhang@Intel.com, Konrad Rzeszutek Wilk , jun.nakajima@Intel.com, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On 02/04/2014 04:42 PM, Konrad Rzeszutek Wilk wrote: > On Tue, Feb 04, 2014 at 03:46:48PM +0000, Jan Beulich wrote: >>>>> On 04.02.14 at 16:32, Konrad Rzeszutek Wilk wrote: >>> On Tue, Feb 04, 2014 at 03:02:44PM +0000, Jan Beulich wrote: >>>> Wasn't it that Mukesh's patch simply was yours with the two >>>> get_ioreq()s folded by using a local variable? >>> Yes. As so >> Thanks. Except that ... >> >>> --- a/xen/arch/x86/hvm/vmx/vvmx.c >>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c >>> @@ -1394,13 +1394,13 @@ void nvmx_switch_guest(void) >>> struct vcpu *v = current; >>> struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v); >>> struct cpu_user_regs *regs = guest_cpu_user_regs(); >>> - >>> + ioreq_t *p = get_ioreq(v); >> ... you don't want to drop the blank line, and naming the new >> variable "ioreq" would seem preferable. >> >>> /* >>> * a pending IO emualtion may still no finished. In this case, >>> * no virtual vmswith is allowed. Or else, the following IO >>> * emulation will handled in a wrong VCPU context. >>> */ >>> - if ( get_ioreq(v)->state != STATE_IOREQ_NONE ) >>> + if ( p && p->state != STATE_IOREQ_NONE ) >> And, as said before, I'd think "!p ||" instead of "p &&" would be >> the right thing here. Yang, Jun? > I have two patches - one the simpler one that is pretty straightfoward > and the one you suggested. Either one fixes PVH guests. I also did > bootup tests with HVM guests to make sure they worked. > > Attached and inline. But they do different things -- one does "ioreq && ioreq->state..." and the other does "!ioreq || ioreq->state...". The first one is incorrect, AFAICT. -George