On Fri, 5 Jun 2020, 21:43 Marek Marczykowski-Górecki, <
marma...@invisiblethingslab.com> wrote:
> On Fri, Jun 05, 2020 at 06:24:20PM +0100, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: 'Marek Marczykowski-Górecki' <marma...@invisiblethingslab.com>
> > > Sent: 05 June 2020 18:14
> > > To: p...@xen.org
> > > Cc: 'Jan Beulich' <jbeul...@suse.com>; 'Andrew Cooper' <
> andrew.coop...@citrix.com>; 'xen-devel' <xen-
> > > de...@lists.xenproject.org>
> > > 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 = 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
