Re: [Xen-devel] [PATCH v2] arm/mem_access: don't reinject stage 2 access exceptions

[Julien Grall <julien.grall@xxxxxxx>]

Hello Andrew,

On 27/09/2016 17:07, Andrew Cooper wrote:
> On 28/09/2016 01:01, Julien Grall wrote:
> > Hi Tamas,
> >
> > On 03/08/2016 11:13, Tamas K Lengyel wrote:
> > > The only way a guest may trip with stage 2 access violation is if
> > > mem_access is
> > > or was in-use, so reinjecting these exceptions to the guest is never
> > > required.
> > >
> > > Requested-by: Julien Grall <julien.grall@xxxxxxx>
> > > Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>
> > Reviewed-by: Julien Grall <julien.grall@xxxxxxx>
> >
> > Regards,
> >
> > > ---
> > > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > Cc: Julien Grall <julien.grall@xxxxxxx>
> > >
> > > v2: simplify the patch by never reinjecting the exception
> > > ---
> > >  xen/arch/arm/traps.c | 22 ++++++++++++----------
> > >  1 file changed, 12 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > > index f509a00..da951c0 100644
> > > --- a/xen/arch/arm/traps.c
> > > +++ b/xen/arch/arm/traps.c
> > > @@ -2415,11 +2415,12 @@ static void do_trap_instr_abort_guest(struct
> > > cpu_user_regs *regs,
> > >                  goto bad_insn_abort;
> > >          }
> > >
> > > -        rc = p2m_mem_access_check(gpa, gva, npfec);
> > > -
> > > -        /* Trap was triggered by mem_access, work here is done */
> > > -        if ( !rc )
> > > -            return;
> > > +        p2m_mem_access_check(gpa, gva, npfec);
> > > +        /*
> > > +         * The only way to get here right now is because of mem_access,
> > > +         * thus reinjecting the exception to the guest is never
> > > required.
> > > +         */
> > > +        return;
> Ignoring errors is always a bad option.  Surely the correct solution is
> therefore to ASSERT() that rc is 0.
p2m_mem_access_check returns a boolean to indicate whether a fault is 
coming from memaccess or not. You may receive a spurious permission 
fault if the page table have been modified between the time you receive 
the data abort and the time you try to handle the fault in memaccess.
So the ASSERT would be wrong here. The best solution is to return to the 
guest vCPU and retry the instruction.
Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel