On Wed, 20 Jan 2021, Julien Grall wrote:
> Hi Stefano,
> 
> On 20/01/2021 00:50, Stefano Stabellini wrote:
> > On Tue, 19 Jan 2021, Oleksandr wrote:
> > > diff --git a/xen/arch/arm/ioreq.c b/xen/arch/arm/ioreq.c
> > > index 40b9e59..0508bd8 100644
> > > --- a/xen/arch/arm/ioreq.c
> > > +++ b/xen/arch/arm/ioreq.c
> > > @@ -101,12 +101,10 @@ enum io_state try_fwd_ioserv(struct cpu_user_regs
> > > *regs,
> > > 
> > >   bool arch_ioreq_complete_mmio(void)
> > >   {
> > > -    struct vcpu *v = current;
> > >       struct cpu_user_regs *regs = guest_cpu_user_regs();
> > >       const union hsr hsr = { .bits = regs->hsr };
> > > -    paddr_t addr = v->io.req.addr;
> > > 
> > > -    if ( try_handle_mmio(regs, hsr, addr) == IO_HANDLED )
> > > +    if ( handle_ioserv(regs, current) == IO_HANDLED )
> > >       {
> > >           advance_pc(regs, hsr);
> > >           return true;
> > 
> > Yes, but I think we want to keep the check
> > 
> >      vio->req.state == STATE_IORESP_READY
> > 
> > So maybe (uncompiled, untested):
> > 
> >      if ( v->io.req.state != STATE_IORESP_READY )
> >          return false;
> 
> Is it possible to reach this function with v->io.req.state !=
> STATE_IORESP_READY? If not, then I would suggest to add an
> ASSERT_UNREACHABLE() before the return.

If I am reading the state machine right it should *not* be possible to
get here with v->io.req.state != STATE_IORESP_READY, so yes,
ASSERT_UNREACHABLE() would work.