On 20.01.21 21:47, Stefano Stabellini wrote:
Hi Julien, Stefano
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]
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;
--
Regards,
Oleksandr Tyshchenko