|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 1/2] xen/arm: Start to implement an ARM decoder instruction
On Wed, 2013-07-31 at 15:49 +0100, Julien Grall wrote:
> Some erratas on ARM processor requires to decode the instruction.
> The decoder will, obviously, decode and fill the ISS fields of the hsr_dabt.
>
> For the moment, the decoder only supports:
> - THUMB2 store instruction
> - THUMB single load/store instruction
>
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> ---
> xen/arch/arm/Makefile | 1 +
> xen/arch/arm/decode.c | 143
> +++++++++++++++++++++++++++++++++++++++++++++++++
> xen/arch/arm/decode.h | 38 +++++++++++++
> 3 files changed, 182 insertions(+)
> create mode 100644 xen/arch/arm/decode.c
> create mode 100644 xen/arch/arm/decode.h
>
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 5ae5831..5c13a65 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -30,6 +30,7 @@ obj-y += vtimer.o
> obj-y += vpl011.o
> obj-y += hvm.o
> obj-y += device.o
> +obj-y += decode.o
>
> #obj-bin-y += ....o
>
> diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
> new file mode 100644
> index 0000000..e432d2a
> --- /dev/null
> +++ b/xen/arch/arm/decode.c
> @@ -0,0 +1,143 @@
> +/*
> + * xen/arch/arm/decode.c
> + *
> + * Instruction decoder
> + *
> + * Julien Grall <julien.grall@xxxxxxxxxx>
> + * Copyright (C) 2013 Linaro Limited.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <xen/types.h>
> +#include <xen/sched.h>
> +#include <asm/current.h>
> +#include <asm/guest_access.h>
> +#include <xen/lib.h>
> +
> +#include "decode.h"
> +
> +/* TODO: Handle all THUMB2 instruction other than simple store */
I think we can take it as a given that things will be added as and when
they are required, no need to mention it IMHO.
> +static int decode_thumb2(register_t pc, struct hsr_dabt *dabt, uint16_t hw1)
> +{
> + uint16_t hw2;
> + int rc;
> + uint16_t op1, op2, op;
> +
> + rc = raw_copy_from_guest(&hw2, (void *__user)(pc + 2), sizeof (hw2));
> + if ( rc )
> + return rc;
> +
> + /* See A6.3 of DDI 0406C.b */
> + op1 = (hw1 >> 11) & 0x3;
> + op2 = (hw1 >> 4) & 0x7f;
> + op = hw2 >> 15;
> +
> + if ( op1 == 3 && ((op2 & 0x71) == 0x00) )
Please can we do the op1 decode with a switch. I know there is only one
case now but it will avoid the temptation to add a chain of elses.
`
op2 decoding is a pain. oh well.
> + {
> + /* Store single data item */
> + dabt->reg = (hw2 >> 12) & 0x7;
> + /* TODO: Handle access size */
Is this difficult to do? Perhaps you need to just decode op2 a bit
further?
Having done that it may turn out to be easier to decode op1 and op2
together as a single switch statement.
> + return 0;
> + }
> +
> + printk("DOM%u: unhandled THUMB2 instruction 0x%x%x\n",
> + current->domain->domain_id, hw1, hw2);
> +
> + return 1;
> +}
> +
> +/* TODO: Handle all THUMB instructions other than store */
> +static int decode_thumb(register_t pc, struct hsr_dabt *dabt)
> +{
> + uint16_t instr;
> + int rc;
> +
> + rc = raw_copy_from_guest(&instr, (void * __user)pc, sizeof (instr));
> + if ( rc )
> + return rc;
> +
> + switch ( instr >> 12 )
> + {
> + case 5:
> + {
> + /* Load/Store register */
> + uint16_t opB = (instr >> 9) & 0x7;
> +
> + switch ( opB & 0x3 )
> + {
> + case 0:
> + dabt->size = 2;
> + break;
> + case 1:
> + dabt->size = 1;
->sign is uninitialised for these two cases?
Actually, for many of them I think?
> + break;
> + case 3:
> + dabt->sign = 1;
> + /* Fall-through */
> + case 2:
> + dabt->size = 0;
> + break;
> + }
> +
> + dabt->reg = instr & 7;
> +
> + break;
> + }
> + case 6:
> + /* Load/Store word immediate offset */
> + dabt->size = 2;
> + dabt->reg = instr & 7;
> + break;
> + case 7:
> + /* Load/Store byte immediate offset */
> + dabt->size = 0;
> + dabt->reg = instr & 7;
> + break;
> + case 8:
> + /* Load/Store halfword immediate offset */
> + dabt->size = 1;
> + dabt->reg = instr & 7;
> + break;
> + case 9:
> + /* Load/Store word sp offset */
> + dabt->size = 2;
> + dabt->reg = (instr >> 8) & 7;
> + break;
> + case 14:
> + if ( instr & (1 << 11) )
> + return decode_thumb2(pc, dabt, instr);
> + goto bad_thumb;
> + case 15:
> + return decode_thumb2(pc, dabt, instr);
> + default:
> + goto bad_thumb;
> + }
> +
> + return 0;
> +
> +bad_thumb:
> + printk("DOM%u: unhandled THUMB instruction 0x%x\n",
> + current->domain->domain_id, instr);
> + return 1;
> +}
> +
> +int decode_instruction(const struct cpu_user_regs *regs, struct hsr_dabt
> *dabt)
> +{
> + /* XXX: zeroed ISS when decode will be fully implemented */
> +
> + if ( regs->cpsr & PSR_THUMB )
> + return decode_thumb(regs->pc, dabt);
Needs an is_pv32_domain too.
> +
> + /* TODO: Handle ARM instruction */
> +
> + return 1;
> +}
> diff --git a/xen/arch/arm/decode.h b/xen/arch/arm/decode.h
> new file mode 100644
> index 0000000..dc130a3
> --- /dev/null
> +++ b/xen/arch/arm/decode.h
> @@ -0,0 +1,38 @@
> +/*
> + * xen/arch/arm/decode.h
> + *
> + * Instruction decoder
> + *
> + * Julien Grall <julien.grall@xxxxxxxxxx>
> + * Copyright (C) 2013 Linaro Limited.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __ARCH_ARM_DECODE_H_
> +#define __ARCH_ARM_DECODE_H_
> +
> +#include <asm/regs.h>
> +#include <asm/processor.h>
> +
> +int decode_instruction(const struct cpu_user_regs *regs,
> + struct hsr_dabt *dabt);
> +
> +#endif /* __ARCH_ARM_DECODE_H_ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |