|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/3] xen/arm: io: Distinguish unhandled IO from aborted one
On Tue, 30 Jan 2018, Julien Grall wrote:
> Currently, Xen is considering that an IO could either be handled or
> unhandled. When unhandled, the stage-2 abort function will try another
> way to resolve the abort.
>
> However, the MMIO emulation may return unhandled when the address
> belongs to an emulated range but was not correct. In that case, Xen
> should avodi to try another way and directly inject a guest data abort.
^ avoid
> Introduce a tri-state return to distinguish the following state:
> * IO_ABORT: The IO was handled but resulted to an abort
^ in an abort
> * IO_HANDLED: The IO was handled
> * IO_UNHANDLED: The IO was unhandled
>
> For now, it is considered that an IO belonging to an emulated range
> could either be handled or inject an abort. This could be revisit in the
> future if overlapped region exist (or we want to try another way to
> resolve the abort).
>
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> ---
> xen/arch/arm/io.c | 24 ++++++++++++++----------
> xen/arch/arm/traps.c | 34 +++++++++++++++++++++++-----------
> xen/include/asm-arm/mmio.h | 9 ++++++++-
> 3 files changed, 45 insertions(+), 22 deletions(-)
>
> diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
> index c748d8f5bf..a74ec21b86 100644
> --- a/xen/arch/arm/io.c
> +++ b/xen/arch/arm/io.c
> @@ -23,8 +23,9 @@
> #include <asm/current.h>
> #include <asm/mmio.h>
>
> -static int handle_read(const struct mmio_handler *handler, struct vcpu *v,
> - mmio_info_t *info)
> +static enum io_state handle_read(const struct mmio_handler *handler,
> + struct vcpu *v,
> + mmio_info_t *info)
> {
> const struct hsr_dabt dabt = info->dabt;
> struct cpu_user_regs *regs = guest_cpu_user_regs();
> @@ -37,7 +38,7 @@ static int handle_read(const struct mmio_handler *handler,
> struct vcpu *v,
> uint8_t size = (1 << dabt.size) * 8;
>
> if ( !handler->ops->read(v, info, &r, handler->priv) )
> - return 0;
> + return IO_ABORT;
>
> /*
> * Sign extend if required.
> @@ -57,17 +58,20 @@ static int handle_read(const struct mmio_handler
> *handler, struct vcpu *v,
>
> set_user_reg(regs, dabt.reg, r);
>
> - return 1;
> + return IO_HANDLED;
> }
>
> -static int handle_write(const struct mmio_handler *handler, struct vcpu *v,
> - mmio_info_t *info)
> +static enum io_state handle_write(const struct mmio_handler *handler,
> + struct vcpu *v,
> + mmio_info_t *info)
> {
> const struct hsr_dabt dabt = info->dabt;
> struct cpu_user_regs *regs = guest_cpu_user_regs();
> + int ret;
>
> - return handler->ops->write(v, info, get_user_reg(regs, dabt.reg),
> - handler->priv);
> + ret = handler->ops->write(v, info, get_user_reg(regs, dabt.reg),
> + handler->priv);
> + return ( ret ) ? IO_HANDLED : IO_ABORT;
> }
>
> /* This function assumes that mmio regions are not overlapped */
> @@ -100,14 +104,14 @@ static const struct mmio_handler
> *find_mmio_handler(struct domain *d,
> return handler;
> }
>
> -int handle_mmio(mmio_info_t *info)
> +enum io_state handle_mmio(mmio_info_t *info)
> {
> struct vcpu *v = current;
> const struct mmio_handler *handler = NULL;
>
> handler = find_mmio_handler(v->domain, info->gpa);
> if ( !handler )
> - return 0;
> + return IO_UNHANDLED;
>
> if ( info->dabt.write )
> return handle_write(handler, v, info);
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index c8534d6cff..843adf4959 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1864,10 +1864,10 @@ static inline bool hpfar_is_valid(bool s1ptw, uint8_t
> fsc)
> return s1ptw || (fsc == FSC_FLT_TRANS && !check_workaround_834220());
> }
>
> -static bool try_handle_mmio(struct cpu_user_regs *regs,
> - const union hsr hsr,
> - paddr_t gpa)
> -{
> +static enum io_state try_handle_mmio(struct cpu_user_regs *regs,
> + const union hsr hsr,
> + paddr_t gpa)
> + {
> const struct hsr_dabt dabt = hsr.dabt;
> mmio_info_t info = {
> .gpa = gpa,
> @@ -1879,11 +1879,11 @@ static bool try_handle_mmio(struct cpu_user_regs
> *regs,
>
> /* stage-1 page table should never live in an emulated MMIO region */
> if ( dabt.s1ptw )
> - return false;
> + return IO_UNHANDLED;
>
> /* All the instructions used on emulated MMIO region should be valid */
> if ( !dabt.valid )
> - return false;
> + return IO_UNHANDLED;
>
> /*
> * Erratum 766422: Thumb store translation fault to Hypervisor may
> @@ -1896,11 +1896,11 @@ static bool try_handle_mmio(struct cpu_user_regs
> *regs,
> if ( rc )
> {
> gprintk(XENLOG_DEBUG, "Unable to decode instruction\n");
> - return false;
> + return IO_ABORT;
> }
> }
Why do the first two error checks result in IO_UNHANDLED, while the
third result in IO_ABORT? Specifically in relation to pagetable walk
failures due to someone else changing stage-2 entry simultaneously (see
comment toward the end of do_trap_stage2_abort_guest)?
> - return !!handle_mmio(&info);
> + return handle_mmio(&info);
> }
>
> /*
> @@ -2005,10 +2005,21 @@ static void do_trap_stage2_abort_guest(struct
> cpu_user_regs *regs,
> *
> * Note that emulated region cannot be executed
> */
> - if ( is_data && try_handle_mmio(regs, hsr, gpa) )
> + if ( is_data )
> {
> - advance_pc(regs, hsr);
> - return;
> + enum io_state state = try_handle_mmio(regs, hsr, gpa);
> +
> + switch ( state )
> + {
> + case IO_ABORT:
> + goto inject_abt;
> + case IO_HANDLED:
> + advance_pc(regs, hsr);
> + return;
> + case IO_UNHANDLED:
> + /* Nothing to do */
> + break;
> + }
> }
>
> /*
> @@ -2029,6 +2040,7 @@ static void do_trap_stage2_abort_guest(struct
> cpu_user_regs *regs,
> hsr.bits, xabt.fsc);
> }
>
> +inject_abt:
> gdprintk(XENLOG_DEBUG, "HSR=0x%x pc=%#"PRIregister" gva=%#"PRIvaddr
> " gpa=%#"PRIpaddr"\n", hsr.bits, regs->pc, gva, gpa);
> if ( is_data )
> diff --git a/xen/include/asm-arm/mmio.h b/xen/include/asm-arm/mmio.h
> index 37e2b7a707..baaecf6931 100644
> --- a/xen/include/asm-arm/mmio.h
> +++ b/xen/include/asm-arm/mmio.h
> @@ -32,6 +32,13 @@ typedef struct
> paddr_t gpa;
> } mmio_info_t;
>
> +enum io_state
> +{
> + IO_ABORT, /* The IO was handled by the helper and lead to an
> abort. */
^ led
> + IO_HANDLED, /* The IO was successfully handled by the helper. */
> + IO_UNHANDLED, /* The IO was not handled by the helper. */
> +};
> +
> typedef int (*mmio_read_t)(struct vcpu *v, mmio_info_t *info,
> register_t *r, void *priv);
> typedef int (*mmio_write_t)(struct vcpu *v, mmio_info_t *info,
> @@ -56,7 +63,7 @@ struct vmmio {
> struct mmio_handler *handlers;
> };
>
> -extern int handle_mmio(mmio_info_t *info);
> +extern enum io_state handle_mmio(mmio_info_t *info);
> void register_mmio_handler(struct domain *d,
> const struct mmio_handler_ops *ops,
> paddr_t addr, paddr_t size, void *priv);
> --
> 2.11.0
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |