[v2,11/11] ioreq: provide support for long-running operations...

Commit Message

Roger Pau Monné Sept. 3, 2019, 4:14 p.m. UTC

...and switch vPCI to use this infrastructure for long running
physmap modification operations.

This allows to get rid of the vPCI specific modifications done to
handle_hvm_io_completion and allows generalizing the support for
long-running operations to other internal ioreq servers. Such support
is implemented as a specific handler that can be registers by internal
ioreq servers and that will be called to check for pending work.
Returning true from this handler will prevent the vcpu from running
until the handler returns false.

Signed-off-by: Roger Pau Monné <[e-mail]>
---
 xen/arch/x86/hvm/ioreq.c       | 55 +++++++++++++++++++++++++-----
 xen/drivers/vpci/header.c      | 61 ++++++++++++++++++----------------
 xen/drivers/vpci/vpci.c        |  8 ++++-
 xen/include/asm-x86/hvm/vcpu.h |  3 +-
 xen/include/xen/vpci.h         |  6 ----
 5 files changed, 89 insertions(+), 44 deletions(-)

Comments

Paul Durrant Sept. 10, 2019, 2:14 p.m. UTC | #1

> -----Original Message-----
> From: Roger Pau Monne <[e-mail]>
> Sent: 03 September 2019 17:14
> To: [e-mail]
> Cc: Roger Pau Monne <[e-mail]>; Paul Durrant <[e-mail]>; Jan Beulich
> <[e-mail]>; Andrew Cooper <[e-mail]>; Wei Liu <[e-mail]>; George Dunlap
> <[e-mail]>; Ian Jackson <[e-mail]>; Julien Grall <[e-mail]>;
> Konrad Rzeszutek Wilk <[e-mail]>; Stefano Stabellini <[e-mail]>; Tim
> (Xen.org) <[e-mail]>
> Subject: [PATCH v2 11/11] ioreq: provide support for long-running operations...
> 
> ...and switch vPCI to use this infrastructure for long running
> physmap modification operations.
> 
> This allows to get rid of the vPCI specific modifications done to
> handle_hvm_io_completion and allows generalizing the support for
> long-running operations to other internal ioreq servers. Such support
> is implemented as a specific handler that can be registers by internal
> ioreq servers and that will be called to check for pending work.
> Returning true from this handler will prevent the vcpu from running
> until the handler returns false.
> 
> Signed-off-by: Roger Pau Monné <[e-mail]>
> ---
>  xen/arch/x86/hvm/ioreq.c       | 55 +++++++++++++++++++++++++-----
>  xen/drivers/vpci/header.c      | 61 ++++++++++++++++++----------------
>  xen/drivers/vpci/vpci.c        |  8 ++++-
>  xen/include/asm-x86/hvm/vcpu.h |  3 +-
>  xen/include/xen/vpci.h         |  6 ----
>  5 files changed, 89 insertions(+), 44 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> index 33c56b880c..caa53dfa84 100644
> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -239,16 +239,48 @@ bool handle_hvm_io_completion(struct vcpu *v)
>      enum hvm_io_completion io_completion;
>      unsigned int id;
> 
> -    if ( has_vpci(d) && vpci_process_pending(v) )
> -    {
> -        raise_softirq(SCHEDULE_SOFTIRQ);
> -        return false;
> -    }
> -
> -    FOR_EACH_EXTERNAL_IOREQ_SERVER(d, id, s)
> +    FOR_EACH_IOREQ_SERVER(d, id, s)
>      {
>          struct hvm_ioreq_vcpu *sv;
> 
> +        if ( hvm_ioreq_is_internal(id) )
> +        {

I wonder whether it would be neater by saying:

           if ( !hvm_ioreq_is_internal(id) )
               continue;

here to avoid the indentation below.

> +            if ( vio->io_req.state == STATE_IOREQ_INPROCESS )
> +            {
> +                ioreq_t req = vio->io_req;
> +
> +                /*
> +                 * Check and convert the PIO/MMIO ioreq to a PCI config space
> +                 * access.
> +                 */
> +                convert_pci_ioreq(d, &req);
> +
> +                if ( s->handler(v, &req, s->data) == X86EMUL_RETRY )
> +                {
> +                    /*
> +                     * Need to raise a scheduler irq in order to prevent the
> +                     * guest vcpu from resuming execution.
> +                     *
> +                     * Note this is not required for external ioreq operations
> +                     * because in that case the vcpu is marked as blocked, but
> +                     * this cannot be done for long-running internal
> +                     * operations, since it would prevent the vcpu from being
> +                     * scheduled and thus the long running operation from
> +                     * finishing.
> +                     */
> +                    raise_softirq(SCHEDULE_SOFTIRQ);
> +                    return false;
> +                }
> +
> +                /* Finished processing the ioreq. */
> +                if ( hvm_ioreq_needs_completion(&vio->io_req) )
> +                    vio->io_req.state = STATE_IORESP_READY;
> +                else
> +                    vio->io_req.state = STATE_IOREQ_NONE;

IMO the above is also neater as:

    vio->io_req.state = hvm_ioreq_needs_completion(&vio->io_req) ?
                        STATE_IORESP_READY : STATE_IOREQ_NONE;

> +            }
> +            continue;
> +        }
> +
>          list_for_each_entry ( sv,
>                                &s->ioreq_vcpu_list,
>                                list_entry )
> @@ -1582,7 +1614,14 @@ int hvm_send_ioreq(ioservid_t id, ioreq_t *proto_p, bool buffered)
>          return hvm_send_buffered_ioreq(s, proto_p);
> 
>      if ( hvm_ioreq_is_internal(id) )
> -        return s->handler(curr, proto_p, s->data);
> +    {
> +        int rc = s->handler(curr, proto_p, s->data);
> +
> +        if ( rc == X86EMUL_RETRY )
> +            curr->arch.hvm.hvm_io.io_req.state = STATE_IOREQ_INPROCESS;
> +
> +        return rc;
> +    }
> 
>      if ( unlikely(!vcpu_start_shutdown_deferral(curr)) )
>          return X86EMUL_RETRY;
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index 3c794f486d..f1c1a69492 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -129,37 +129,42 @@ static void modify_decoding(const struct pci_dev *pdev, uint16_t cmd,
> 
>  bool vpci_process_pending(struct vcpu *v)
>  {
> -    if ( v->vpci.mem )
> +    struct map_data data = {
> +        .d = v->domain,
> +        .map = v->vpci.cmd & PCI_COMMAND_MEMORY,
> +    };
> +    int rc;
> +
> +    if ( !v->vpci.mem )
>      {
> -        struct map_data data = {
> -            .d = v->domain,
> -            .map = v->vpci.cmd & PCI_COMMAND_MEMORY,
> -        };
> -        int rc = rangeset_consume_ranges(v->vpci.mem, map_range, &data);
> -
> -        if ( rc == -ERESTART )
> -            return true;
> -
> -        spin_lock(&v->vpci.pdev->vpci->lock);
> -        /* Disable memory decoding unconditionally on failure. */
> -        modify_decoding(v->vpci.pdev,
> -                        rc ? v->vpci.cmd & ~PCI_COMMAND_MEMORY : v->vpci.cmd,
> -                        !rc && v->vpci.rom_only);
> -        spin_unlock(&v->vpci.pdev->vpci->lock);
> -
> -        rangeset_destroy(v->vpci.mem);
> -        v->vpci.mem = NULL;
> -        if ( rc )
> -            /*
> -             * FIXME: in case of failure remove the device from the domain.
> -             * Note that there might still be leftover mappings. While this is
> -             * safe for Dom0, for DomUs the domain will likely need to be
> -             * killed in order to avoid leaking stale p2m mappings on
> -             * failure.
> -             */
> -            vpci_remove_device(v->vpci.pdev);
> +        ASSERT_UNREACHABLE();
> +        return false;
>      }
> 
> +    rc = rangeset_consume_ranges(v->vpci.mem, map_range, &data);
> +

Extraneous blank line?

  Paul

> +    if ( rc == -ERESTART )
> +        return true;
> +
> +    spin_lock(&v->vpci.pdev->vpci->lock);
> +    /* Disable memory decoding unconditionally on failure. */
> +    modify_decoding(v->vpci.pdev,
> +                    rc ? v->vpci.cmd & ~PCI_COMMAND_MEMORY : v->vpci.cmd,
> +                    !rc && v->vpci.rom_only);
> +    spin_unlock(&v->vpci.pdev->vpci->lock);
> +
> +    rangeset_destroy(v->vpci.mem);
> +    v->vpci.mem = NULL;
> +    if ( rc )
> +        /*
> +         * FIXME: in case of failure remove the device from the domain.
> +         * Note that there might still be leftover mappings. While this is
> +         * safe for Dom0, for DomUs the domain will likely need to be
> +         * killed in order to avoid leaking stale p2m mappings on
> +         * failure.
> +         */
> +        vpci_remove_device(v->vpci.pdev);
> +
>      return false;
>  }
> 
> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
> index 5664020c2d..6069dff612 100644
> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -498,6 +498,12 @@ static int ioreq_handler(struct vcpu *v, ioreq_t *req, void *data)
>          return X86EMUL_UNHANDLEABLE;
>      }
> 
> +    if ( v->vpci.mem )
> +    {
> +        ASSERT(req->state == STATE_IOREQ_INPROCESS);
> +        return vpci_process_pending(v) ? X86EMUL_RETRY : X86EMUL_OKAY;
> +    }
> +
>      sbdf.sbdf = req->addr >> 32;
> 
>      if ( req->dir )
> @@ -505,7 +511,7 @@ static int ioreq_handler(struct vcpu *v, ioreq_t *req, void *data)
>      else
>          write(sbdf, req->addr, req->size, req->data);
> 
> -    return X86EMUL_OKAY;
> +    return v->vpci.mem ? X86EMUL_RETRY : X86EMUL_OKAY;
>  }
> 
>  int vpci_register_ioreq(struct domain *d)
> diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
> index 38f5c2bb9b..4563746466 100644
> --- a/xen/include/asm-x86/hvm/vcpu.h
> +++ b/xen/include/asm-x86/hvm/vcpu.h
> @@ -92,7 +92,8 @@ struct hvm_vcpu_io {
> 
>  static inline bool hvm_ioreq_needs_completion(const ioreq_t *ioreq)
>  {
> -    return ioreq->state == STATE_IOREQ_READY &&
> +    return (ioreq->state == STATE_IOREQ_READY ||
> +            ioreq->state == STATE_IOREQ_INPROCESS) &&
>             !ioreq->data_is_ptr &&
>             (ioreq->type != IOREQ_TYPE_PIO || ioreq->dir != IOREQ_WRITE);
>  }
> diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
> index 36f435ed5b..a65491e0c9 100644
> --- a/xen/include/xen/vpci.h
> +++ b/xen/include/xen/vpci.h
> @@ -225,12 +225,6 @@ static inline int vpci_register_ioreq(struct domain *d)
>  }
> 
>  static inline void vpci_dump_msi(void) { }
> -
> -static inline bool vpci_process_pending(struct vcpu *v)
> -{
> -    ASSERT_UNREACHABLE();
> -    return false;
> -}
>  #endif
> 
>  #endif
> --
> 2.22.0

Roger Pau Monné Sept. 10, 2019, 2:28 p.m. UTC | #2

On Tue, Sep 10, 2019 at 04:14:18PM +0200, Paul Durrant wrote:
> > -----Original Message-----
> > From: Roger Pau Monne <[e-mail]>
> > Sent: 03 September 2019 17:14
> > To: [e-mail]
> > Cc: Roger Pau Monne <[e-mail]>; Paul Durrant <[e-mail]>; Jan Beulich
> > <[e-mail]>; Andrew Cooper <[e-mail]>; Wei Liu <[e-mail]>; George Dunlap
> > <[e-mail]>; Ian Jackson <[e-mail]>; Julien Grall <[e-mail]>;
> > Konrad Rzeszutek Wilk <[e-mail]>; Stefano Stabellini <[e-mail]>; Tim
> > (Xen.org) <[e-mail]>
> > Subject: [PATCH v2 11/11] ioreq: provide support for long-running operations...
> > 
> > ...and switch vPCI to use this infrastructure for long running
> > physmap modification operations.
> > 
> > This allows to get rid of the vPCI specific modifications done to
> > handle_hvm_io_completion and allows generalizing the support for
> > long-running operations to other internal ioreq servers. Such support
> > is implemented as a specific handler that can be registers by internal
> > ioreq servers and that will be called to check for pending work.
> > Returning true from this handler will prevent the vcpu from running
> > until the handler returns false.
> > 
> > Signed-off-by: Roger Pau Monné <[e-mail]>
> > ---
> >  xen/arch/x86/hvm/ioreq.c       | 55 +++++++++++++++++++++++++-----
> >  xen/drivers/vpci/header.c      | 61 ++++++++++++++++++----------------
> >  xen/drivers/vpci/vpci.c        |  8 ++++-
> >  xen/include/asm-x86/hvm/vcpu.h |  3 +-
> >  xen/include/xen/vpci.h         |  6 ----
> >  5 files changed, 89 insertions(+), 44 deletions(-)
> > 
> > diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> > index 33c56b880c..caa53dfa84 100644
> > --- a/xen/arch/x86/hvm/ioreq.c
> > +++ b/xen/arch/x86/hvm/ioreq.c
> > @@ -239,16 +239,48 @@ bool handle_hvm_io_completion(struct vcpu *v)
> >      enum hvm_io_completion io_completion;
> >      unsigned int id;
> > 
> > -    if ( has_vpci(d) && vpci_process_pending(v) )
> > -    {
> > -        raise_softirq(SCHEDULE_SOFTIRQ);
> > -        return false;
> > -    }
> > -
> > -    FOR_EACH_EXTERNAL_IOREQ_SERVER(d, id, s)
> > +    FOR_EACH_IOREQ_SERVER(d, id, s)
> >      {
> >          struct hvm_ioreq_vcpu *sv;
> > 
> > +        if ( hvm_ioreq_is_internal(id) )
> > +        {
> 
> I wonder whether it would be neater by saying:
> 
>            if ( !hvm_ioreq_is_internal(id) )
>                continue;
> 
> here to avoid the indentation below.

I'm not sure I follow. This loop does work for both the internal and
the external ioreq servers, and hence skipping external servers would
prevent the iteration over ioreq_vcpu_list done below for external
servers.

> 
> > +            if ( vio->io_req.state == STATE_IOREQ_INPROCESS )
> > +            {
> > +                ioreq_t req = vio->io_req;
> > +
> > +                /*
> > +                 * Check and convert the PIO/MMIO ioreq to a PCI config space
> > +                 * access.
> > +                 */
> > +                convert_pci_ioreq(d, &req);
> > +
> > +                if ( s->handler(v, &req, s->data) == X86EMUL_RETRY )
> > +                {
> > +                    /*
> > +                     * Need to raise a scheduler irq in order to prevent the
> > +                     * guest vcpu from resuming execution.
> > +                     *
> > +                     * Note this is not required for external ioreq operations
> > +                     * because in that case the vcpu is marked as blocked, but
> > +                     * this cannot be done for long-running internal
> > +                     * operations, since it would prevent the vcpu from being
> > +                     * scheduled and thus the long running operation from
> > +                     * finishing.
> > +                     */
> > +                    raise_softirq(SCHEDULE_SOFTIRQ);
> > +                    return false;
> > +                }
> > +
> > +                /* Finished processing the ioreq. */
> > +                if ( hvm_ioreq_needs_completion(&vio->io_req) )
> > +                    vio->io_req.state = STATE_IORESP_READY;
> > +                else
> > +                    vio->io_req.state = STATE_IOREQ_NONE;
> 
> IMO the above is also neater as:
> 
>     vio->io_req.state = hvm_ioreq_needs_completion(&vio->io_req) ?
>                         STATE_IORESP_READY : STATE_IOREQ_NONE;
> 
> > +            }
> > +            continue;
> > +        }
> > +
> >          list_for_each_entry ( sv,
> >                                &s->ioreq_vcpu_list,
> >                                list_entry )

Thanks, Roger.

Paul Durrant Sept. 10, 2019, 2:40 p.m. UTC | #3

> -----Original Message-----
> From: Roger Pau Monne <[e-mail]>
> Sent: 10 September 2019 15:28
> To: Paul Durrant <[e-mail]>
> Cc: [e-mail]; Jan Beulich <[e-mail]>; Andrew Cooper
> <[e-mail]>; Wei Liu <[e-mail]>; George Dunlap <[e-mail]>; Ian
> Jackson <[e-mail]>; Julien Grall <[e-mail]>; Konrad Rzeszutek Wilk
> <[e-mail]>; Stefano Stabellini <[e-mail]>; Tim (Xen.org) <[e-mail]>
> Subject: Re: [PATCH v2 11/11] ioreq: provide support for long-running operations...
> 
> On Tue, Sep 10, 2019 at 04:14:18PM +0200, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Roger Pau Monne <[e-mail]>
> > > Sent: 03 September 2019 17:14
> > > To: [e-mail]
> > > Cc: Roger Pau Monne <[e-mail]>; Paul Durrant <[e-mail]>; Jan Beulich
> > > <[e-mail]>; Andrew Cooper <[e-mail]>; Wei Liu <[e-mail]>; George
> Dunlap
> > > <[e-mail]>; Ian Jackson <[e-mail]>; Julien Grall
> <[e-mail]>;
> > > Konrad Rzeszutek Wilk <[e-mail]>; Stefano Stabellini <[e-mail]>; Tim
> > > (Xen.org) <[e-mail]>
> > > Subject: [PATCH v2 11/11] ioreq: provide support for long-running operations...
> > >
> > > ...and switch vPCI to use this infrastructure for long running
> > > physmap modification operations.
> > >
> > > This allows to get rid of the vPCI specific modifications done to
> > > handle_hvm_io_completion and allows generalizing the support for
> > > long-running operations to other internal ioreq servers. Such support
> > > is implemented as a specific handler that can be registers by internal
> > > ioreq servers and that will be called to check for pending work.
> > > Returning true from this handler will prevent the vcpu from running
> > > until the handler returns false.
> > >
> > > Signed-off-by: Roger Pau Monné <[e-mail]>
> > > ---
> > >  xen/arch/x86/hvm/ioreq.c       | 55 +++++++++++++++++++++++++-----
> > >  xen/drivers/vpci/header.c      | 61 ++++++++++++++++++----------------
> > >  xen/drivers/vpci/vpci.c        |  8 ++++-
> > >  xen/include/asm-x86/hvm/vcpu.h |  3 +-
> > >  xen/include/xen/vpci.h         |  6 ----
> > >  5 files changed, 89 insertions(+), 44 deletions(-)
> > >
> > > diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> > > index 33c56b880c..caa53dfa84 100644
> > > --- a/xen/arch/x86/hvm/ioreq.c
> > > +++ b/xen/arch/x86/hvm/ioreq.c
> > > @@ -239,16 +239,48 @@ bool handle_hvm_io_completion(struct vcpu *v)
> > >      enum hvm_io_completion io_completion;
> > >      unsigned int id;
> > >
> > > -    if ( has_vpci(d) && vpci_process_pending(v) )
> > > -    {
> > > -        raise_softirq(SCHEDULE_SOFTIRQ);
> > > -        return false;
> > > -    }
> > > -
> > > -    FOR_EACH_EXTERNAL_IOREQ_SERVER(d, id, s)
> > > +    FOR_EACH_IOREQ_SERVER(d, id, s)
> > >      {
> > >          struct hvm_ioreq_vcpu *sv;
> > >
> > > +        if ( hvm_ioreq_is_internal(id) )
> > > +        {
> >
> > I wonder whether it would be neater by saying:
> >
> >            if ( !hvm_ioreq_is_internal(id) )
> >                continue;
> >
> > here to avoid the indentation below.
> 
> I'm not sure I follow. This loop does work for both the internal and
> the external ioreq servers, and hence skipping external servers would
> prevent the iteration over ioreq_vcpu_list done below for external
> servers.

Sorry, I got my nesting mixed up. What I actually mean is:

+    FOR_EACH_IOREQ_SERVER(d, id, s)
     {
         struct hvm_ioreq_vcpu *sv;

+        if ( hvm_ioreq_is_internal(id) )
+        {
+            ioreq_t req = vio->io_req;
+
+            if ( vio->io_req.state != STATE_IOREQ_INPROCESS )
+                continue;
+
+            /*
+             * Check and convert the PIO/MMIO ioreq to a PCI config space
+             * access.
+             */
+            convert_pci_ioreq(d, &req);
+
+            if ( s->handler(v, &req, s->data) == X86EMUL_RETRY )
+            {
+                /*
+                 * Need to raise a scheduler irq in order to prevent the
+                 * guest vcpu from resuming execution.
+                 *
+                 * Note this is not required for external ioreq operations
+                 * because in that case the vcpu is marked as blocked, but
+                 * this cannot be done for long-running internal
+                 * operations, since it would prevent the vcpu from being
+                 * scheduled and thus the long running operation from
+                 * finishing.
+                 */
+                raise_softirq(SCHEDULE_SOFTIRQ);
+                return false;
+            }
+
+            /* Finished processing the ioreq. */
+            vio->io_req.state = hvm_ioreq_needs_completion(&vio->io_req) ?
+                                STATE_IORESP_READY : STATE_IOREQ_NONE;
+            continue;
+        }
+

  Paul

> 
> >
> > > +            if ( vio->io_req.state == STATE_IOREQ_INPROCESS )
> > > +            {
> > > +                ioreq_t req = vio->io_req;
> > > +
> > > +                /*
> > > +                 * Check and convert the PIO/MMIO ioreq to a PCI config space
> > > +                 * access.
> > > +                 */
> > > +                convert_pci_ioreq(d, &req);
> > > +
> > > +                if ( s->handler(v, &req, s->data) == X86EMUL_RETRY )
> > > +                {
> > > +                    /*
> > > +                     * Need to raise a scheduler irq in order to prevent the
> > > +                     * guest vcpu from resuming execution.
> > > +                     *
> > > +                     * Note this is not required for external ioreq operations
> > > +                     * because in that case the vcpu is marked as blocked, but
> > > +                     * this cannot be done for long-running internal
> > > +                     * operations, since it would prevent the vcpu from being
> > > +                     * scheduled and thus the long running operation from
> > > +                     * finishing.
> > > +                     */
> > > +                    raise_softirq(SCHEDULE_SOFTIRQ);
> > > +                    return false;
> > > +                }
> > > +
> > > +                /* Finished processing the ioreq. */
> > > +                if ( hvm_ioreq_needs_completion(&vio->io_req) )
> > > +                    vio->io_req.state = STATE_IORESP_READY;
> > > +                else
> > > +                    vio->io_req.state = STATE_IOREQ_NONE;
> >
> > IMO the above is also neater as:
> >
> >     vio->io_req.state = hvm_ioreq_needs_completion(&vio->io_req) ?
> >                         STATE_IORESP_READY : STATE_IOREQ_NONE;
> >
> > > +            }
> > > +            continue;
> > > +        }
> > > +
> > >          list_for_each_entry ( sv,
> > >                                &s->ioreq_vcpu_list,
> > >                                list_entry )
> 
> Thanks, Roger.

Patch

diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index 33c56b880c..caa53dfa84 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -239,16 +239,48 @@  bool handle_hvm_io_completion(struct vcpu *v)
     enum hvm_io_completion io_completion;
     unsigned int id;
 
-    if ( has_vpci(d) && vpci_process_pending(v) )
-    {
-        raise_softirq(SCHEDULE_SOFTIRQ);
-        return false;
-    }
-
-    FOR_EACH_EXTERNAL_IOREQ_SERVER(d, id, s)
+    FOR_EACH_IOREQ_SERVER(d, id, s)
     {
         struct hvm_ioreq_vcpu *sv;
 
+        if ( hvm_ioreq_is_internal(id) )
+        {
+            if ( vio->io_req.state == STATE_IOREQ_INPROCESS )
+            {
+                ioreq_t req = vio->io_req;
+
+                /*
+                 * Check and convert the PIO/MMIO ioreq to a PCI config space
+                 * access.
+                 */
+                convert_pci_ioreq(d, &req);
+
+                if ( s->handler(v, &req, s->data) == X86EMUL_RETRY )
+                {
+                    /*
+                     * Need to raise a scheduler irq in order to prevent the
+                     * guest vcpu from resuming execution.
+                     *
+                     * Note this is not required for external ioreq operations
+                     * because in that case the vcpu is marked as blocked, but
+                     * this cannot be done for long-running internal
+                     * operations, since it would prevent the vcpu from being
+                     * scheduled and thus the long running operation from
+                     * finishing.
+                     */
+                    raise_softirq(SCHEDULE_SOFTIRQ);
+                    return false;
+                }
+
+                /* Finished processing the ioreq. */
+                if ( hvm_ioreq_needs_completion(&vio->io_req) )
+                    vio->io_req.state = STATE_IORESP_READY;
+                else
+                    vio->io_req.state = STATE_IOREQ_NONE;
+            }
+            continue;
+        }
+
         list_for_each_entry ( sv,
                               &s->ioreq_vcpu_list,
                               list_entry )
@@ -1582,7 +1614,14 @@  int hvm_send_ioreq(ioservid_t id, ioreq_t *proto_p, bool buffered)
         return hvm_send_buffered_ioreq(s, proto_p);
 
     if ( hvm_ioreq_is_internal(id) )
-        return s->handler(curr, proto_p, s->data);
+    {
+        int rc = s->handler(curr, proto_p, s->data);
+
+        if ( rc == X86EMUL_RETRY )
+            curr->arch.hvm.hvm_io.io_req.state = STATE_IOREQ_INPROCESS;
+
+        return rc;
+    }
 
     if ( unlikely(!vcpu_start_shutdown_deferral(curr)) )
         return X86EMUL_RETRY;
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 3c794f486d..f1c1a69492 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -129,37 +129,42 @@  static void modify_decoding(const struct pci_dev *pdev, uint16_t cmd,
 
 bool vpci_process_pending(struct vcpu *v)
 {
-    if ( v->vpci.mem )
+    struct map_data data = {
+        .d = v->domain,
+        .map = v->vpci.cmd & PCI_COMMAND_MEMORY,
+    };
+    int rc;
+
+    if ( !v->vpci.mem )
     {
-        struct map_data data = {
-            .d = v->domain,
-            .map = v->vpci.cmd & PCI_COMMAND_MEMORY,
-        };
-        int rc = rangeset_consume_ranges(v->vpci.mem, map_range, &data);
-
-        if ( rc == -ERESTART )
-            return true;
-
-        spin_lock(&v->vpci.pdev->vpci->lock);
-        /* Disable memory decoding unconditionally on failure. */
-        modify_decoding(v->vpci.pdev,
-                        rc ? v->vpci.cmd & ~PCI_COMMAND_MEMORY : v->vpci.cmd,
-                        !rc && v->vpci.rom_only);
-        spin_unlock(&v->vpci.pdev->vpci->lock);
-
-        rangeset_destroy(v->vpci.mem);
-        v->vpci.mem = NULL;
-        if ( rc )
-            /*
-             * FIXME: in case of failure remove the device from the domain.
-             * Note that there might still be leftover mappings. While this is
-             * safe for Dom0, for DomUs the domain will likely need to be
-             * killed in order to avoid leaking stale p2m mappings on
-             * failure.
-             */
-            vpci_remove_device(v->vpci.pdev);
+        ASSERT_UNREACHABLE();
+        return false;
     }
 
+    rc = rangeset_consume_ranges(v->vpci.mem, map_range, &data);
+
+    if ( rc == -ERESTART )
+        return true;
+
+    spin_lock(&v->vpci.pdev->vpci->lock);
+    /* Disable memory decoding unconditionally on failure. */
+    modify_decoding(v->vpci.pdev,
+                    rc ? v->vpci.cmd & ~PCI_COMMAND_MEMORY : v->vpci.cmd,
+                    !rc && v->vpci.rom_only);
+    spin_unlock(&v->vpci.pdev->vpci->lock);
+
+    rangeset_destroy(v->vpci.mem);
+    v->vpci.mem = NULL;
+    if ( rc )
+        /*
+         * FIXME: in case of failure remove the device from the domain.
+         * Note that there might still be leftover mappings. While this is
+         * safe for Dom0, for DomUs the domain will likely need to be
+         * killed in order to avoid leaking stale p2m mappings on
+         * failure.
+         */
+        vpci_remove_device(v->vpci.pdev);
+
     return false;
 }
 
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 5664020c2d..6069dff612 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -498,6 +498,12 @@  static int ioreq_handler(struct vcpu *v, ioreq_t *req, void *data)
         return X86EMUL_UNHANDLEABLE;
     }
 
+    if ( v->vpci.mem )
+    {
+        ASSERT(req->state == STATE_IOREQ_INPROCESS);
+        return vpci_process_pending(v) ? X86EMUL_RETRY : X86EMUL_OKAY;
+    }
+
     sbdf.sbdf = req->addr >> 32;
 
     if ( req->dir )
@@ -505,7 +511,7 @@  static int ioreq_handler(struct vcpu *v, ioreq_t *req, void *data)
     else
         write(sbdf, req->addr, req->size, req->data);
 
-    return X86EMUL_OKAY;
+    return v->vpci.mem ? X86EMUL_RETRY : X86EMUL_OKAY;
 }
 
 int vpci_register_ioreq(struct domain *d)
diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
index 38f5c2bb9b..4563746466 100644
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -92,7 +92,8 @@  struct hvm_vcpu_io {
 
 static inline bool hvm_ioreq_needs_completion(const ioreq_t *ioreq)
 {
-    return ioreq->state == STATE_IOREQ_READY &&
+    return (ioreq->state == STATE_IOREQ_READY ||
+            ioreq->state == STATE_IOREQ_INPROCESS) &&
            !ioreq->data_is_ptr &&
            (ioreq->type != IOREQ_TYPE_PIO || ioreq->dir != IOREQ_WRITE);
 }
diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index 36f435ed5b..a65491e0c9 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -225,12 +225,6 @@  static inline int vpci_register_ioreq(struct domain *d)
 }
 
 static inline void vpci_dump_msi(void) { }
-
-static inline bool vpci_process_pending(struct vcpu *v)
-{
-    ASSERT_UNREACHABLE();
-    return false;
-}
 #endif
 
 #endif