Re: [PATCH] pvh: Fix regression caused by assumption that HVM paths MUST use io-backend device.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

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 <konrad.wilk@oracle.com> 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
> 
>