Re: handle_pio looping during domain shutdown, with qemu 4.2.0 in stubdom

[Paul Durrant <xadimgnik@xxxxxxxxx>]

On Fri, 5 Jun 2020, 21:43 Marek Marczykowski-Górecki, <marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:

On Fri, Jun 05, 2020 at 06:24:20PM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: 'Marek Marczykowski-Górecki' <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> > Sent: 05 June 2020 18:14
> > To: paul@xxxxxxx
> > Cc: 'Jan Beulich' <jbeulich@xxxxxxxx>; 'Andrew Cooper' <andrew.cooper3@xxxxxxxxxx>; 'xen-devel' <xen-
> > devel@xxxxxxxxxxxxxxxxxxxx>
> > Subject: Re: handle_pio looping during domain shutdown, with qemu 4.2.0 in stubdom
> >
> > On Fri, Jun 05, 2020 at 04:48:39PM +0100, Paul Durrant wrote:
> > > This (untested) patch might help:
> >
> > It is different now. I don't get domain_crash because of
> > X86EMUL_UNHANDLEABLE anymore, but I still see handle_pio looping for
> > some time. But it eventually ends, not really sure why.
>
> That'll be the shutdown deferral, which I realised later that I forgot about...
>
> >
> > I've tried the patch with a modification to make it build:
> >
> > > diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> > > index c55c4bc4bc..8aa8779ba2 100644
> > > --- a/xen/arch/x86/hvm/ioreq.c
> > > +++ b/xen/arch/x86/hvm/ioreq.c
> > > @@ -109,12 +109,7 @@ static void hvm_io_assist(struct hvm_ioreq_vcpu *sv, uint64_t data)
> > >      ioreq_t *ioreq = &v->arch.hvm.hvm_io.io_req;
> > >
> > >      if ( hvm_ioreq_needs_completion(ioreq) )
> > > -    {
> > > -        ioreq->state = STATE_IORESP_READY;
> > >          ioreq->data = ""> > > > -    }
> > > -    else
> > > -        ioreq->state = STATE_IOREQ_NONE;
> > >
> > >      msix_write_completion(v);
> > >      vcpu_end_shutdown_deferral(v);
>
> In fact, move both of these lines...
>
> > > @@ -209,6 +204,9 @@ bool handle_hvm_io_completion(struct vcpu *v)
> > >          }
> > >      }
> > >
> > > +    ioreq->state = hvm_ioreq_needs_completion(&vio->ioreq) ?
> >        vio->io_req->state ... &vio->io_req
> >
> > > +        STATE_IORESP_READY : STATE_IOREQ_NONE;
> > > +
>
> ... to here too.
>
> > >      io_completion = vio->io_completion;
> > >      vio->io_completion = HVMIO_no_completion;
> > >
> >
> > The full patch (together with my debug prints):
> > https://gist.github.com/marmarek/da37da3722179057a6e7add4fb361e06

The current patch:
https://gist.github.com/marmarek/5ae795129c1be2dae13bfc517547c0f1

> I guess it is the destroy being held off by the shutdown deferral? Hopefully the above tweaks should sort that out.

Yes, now it works (tried 5 times in a row, previously it crashed on
the first or the second one). Thanks!

--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Cool. I will send a proper patch tomorrow.

  Paul