[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 1/4] xen/arm: traps: Merge try_handle_mmio() and handle_mmio()

On 02/02/18 14:34, Andre Przywara wrote:


On 02/02/18 10:14, Julien Grall wrote:
At the moment, try_handle_mmio() will do check on the HSR and bail out
if one check fail. This means that another method will be tried to
handle the fault even for bad access on emulated region. While this
should not be an issue, this is not future proof.

Move the checks of try_handle_mmio() in handle_mmio() after we identified
the fault to target an emulated MMIO. While this does not fix the potential
fall-through, a follow-up patch will do by distinguish the potential error.

Note that the handle_mmio() was renamed to try_handle_mmio() and the
prototype adapted.

Why is that? I think the prefix "try_" only makes sense if you have a
non-try version as well. To some degree most functions "try" something,
when they check for and return errors.
I think the return type makes it clear what the semantics are.
So personally I would prefer just "handle_mmio" as the function name.
Because we have another function called try_map_mmio() just below, so I wanted to keep similar.

Also, I think "try_" makes sense here because if you don't succeed, you will fallback to another method. Most of the times, this is not the case of other functions.

But this only a nit, definitely not worth a respin on its own.

While merging the 2 functions, remove the check whether the fault is
stage-2 abort on stage-1 translation walk because the instruction
syndrome will always be invalid (see B3-1433 in DDI 0406C.c and
D10-2460 in DDI 0487C.a).

Yes, that looks correct to me.

Signed-off-by: Julien Grall <julien.grall@xxxxxxx>

Reviewed-by: Andre Przywara <andre.przywara@xxxxxxx>



Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.