[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH V4 14/24] arm/ioreq: Introduce arch specific bits for IOREQ/DM features
On Thu, 21 Jan 2021, Oleksandr wrote: > On 20.01.21 21:47, Stefano Stabellini wrote: > > 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. > Agree here. If the assumption is not correct (unlikely), I think I will catch > this during testing. > In addition, we can probably drop case STATE_IORESP_READY in try_fwd_ioserv(). > > > [not tested] Yes, looks OK > diff --git a/xen/arch/arm/ioreq.c b/xen/arch/arm/ioreq.c > index 40b9e59..c7ee1a7 100644 > --- a/xen/arch/arm/ioreq.c > +++ b/xen/arch/arm/ioreq.c > @@ -71,9 +71,6 @@ enum io_state try_fwd_ioserv(struct cpu_user_regs *regs, > case STATE_IOREQ_NONE: > break; > > - case STATE_IORESP_READY: > - return IO_HANDLED; > - > default: > gdprintk(XENLOG_ERR, "wrong state %u\n", vio->req.state); > return IO_ABORT; > @@ -104,9 +101,14 @@ 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 ( v->io.req.state != STATE_IORESP_READY ) > + { > + ASSERT_UNREACHABLE(); > + return false; > + } > + > + if ( handle_ioserv(regs, v) == IO_HANDLED ) > { > advance_pc(regs, hsr); > return true;
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |