Re: [PATCH v12] xen/arm64: io: Handle data abort due to cache maintenance instructions

[Julien Grall <julien@xxxxxxx>]

Hi,

On 18/04/2022 22:02, Stefano Stabellini wrote:
> On Fri, 1 Apr 2022, Julien Grall wrote:
> > On 24/03/2022 13:37, Ayan Kumar Halder wrote:
> > >        /*
> > >         * At this point, we know that the instruction is either valid or has
> > > been
> > >         * decoded successfully. Thus, Xen should be allowed to execute the
> > > diff --git a/xen/arch/arm/ioreq.c b/xen/arch/arm/ioreq.c
> > > index 54167aebcb..87a6240f2a 100644
> > > --- a/xen/arch/arm/ioreq.c
> > > +++ b/xen/arch/arm/ioreq.c
> > > @@ -47,7 +47,7 @@ enum io_state try_fwd_ioserv(struct cpu_user_regs *regs,
> > >                                 struct vcpu *v, mmio_info_t *info)
> > >    {
> > >        struct vcpu_io *vio = &v->io;
> > > -    struct instr_details instr = info->dabt_instr;
> > > +    const struct instr_details instr = info->dabt_instr;
> > >        struct hsr_dabt dabt = info->dabt;
> > >        ioreq_t p = {
> > >            .type = IOREQ_TYPE_COPY,
> > > @@ -62,7 +62,6 @@ enum io_state try_fwd_ioserv(struct cpu_user_regs *regs,
> > >             * memory access. So for now, we can safely always set to 0.
> > >             */
> > >            .df = 0,
> > > -        .data = get_user_reg(regs, info->dabt.reg),
> > >            .state = STATE_IOREQ_READY,
> > >        };
> > >        struct ioreq_server *s = NULL;
> > > @@ -74,12 +73,23 @@ enum io_state try_fwd_ioserv(struct cpu_user_regs *regs,
> > >            return IO_ABORT;
> > >        }
> > >    +    if ( instr.state == INSTR_CACHE )
> > > +        p.size = dcache_line_bytes;
> > I think it would be best to only set the p.size when instr.state !=
> > INSTR_CACHE in the else here.
> >
> > I can do that on commit. I will also give a chance to Stefano to reply.
> The patch looks OK to me, please go ahead and make the change on
> commit.
I have commited with my reviewed-by tag after folding the following diff:

For the record, I folded this diff:

diff --git a/xen/arch/arm/ioreq.c b/xen/arch/arm/ioreq.c
index 87a6240f2aa9..bdd536e873e5 100644
--- a/xen/arch/arm/ioreq.c
+++ b/xen/arch/arm/ioreq.c
@@ -52,7 +52,6 @@ enum io_state try_fwd_ioserv(struct cpu_user_regs *regs,
     ioreq_t p = {
         .type = IOREQ_TYPE_COPY,
         .addr = info->gpa,
-        .size = 1 << info->dabt.size,
         .count = 1,
         .dir = !info->dabt.write,
         /*
@@ -75,6 +74,8 @@ enum io_state try_fwd_ioserv(struct cpu_user_regs *regs,

     if ( instr.state == INSTR_CACHE )
         p.size = dcache_line_bytes;
+    else
+        p.size = 1U << info->dabt.size;

     s = ioreq_server_select(v->domain, &p);
     if ( !s )

Cheers,

--
Julien Grall