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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

Doing so will catch the case where this comment is no longer accurate,
due to either a bug or change in semantics.

~Andrew

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