From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH] pvh: Fix regression caused by assumption that HVM paths MUST use io-backend device. Date: Wed, 5 Feb 2014 10:26:49 -0500 Message-ID: <20140205152649.GA5167@phenom.dumpdata.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> <52F24C47.5070100@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" 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 1WB4OR-0001Yv-Bp for xen-devel@lists.xenproject.org; Wed, 05 Feb 2014 15:28:03 +0000 Content-Disposition: inline In-Reply-To: <52F24C47.5070100@eu.citrix.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: George Dunlap Cc: Jan Beulich , Konrad Rzeszutek Wilk , jun.nakajima@Intel.com, yang.z.zhang@Intel.com, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On Wed, Feb 05, 2014 at 02:35:51PM +0000, George Dunlap wrote: > 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..." Correct. > and the other does "!ioreq || ioreq->state...". The first one is > incorrect, AFAICT. Both of them fix the hypervisor blowing up with any PVH guest. > > -George > >