|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 1/2] xen/arm: Start to implement an ARM decoder instruction
On 08/08/2013 12:08 PM, Ian Campbell wrote:
> On Tue, 2013-08-06 at 19:31 +0100, Julien Grall wrote:
>> Some erratas on ARM processor requires to decode the instruction.
>
> Errata is already the plural of Erratum, I think.
Right, I will fix it.
>
>> 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>
>>
>> ---
>> Changes in v4:
>> - Add warning about the purpose of this function
>> - Add helper to update ISS (register, sign, size) field of DABT
>> - Improve decoding for THUMB 2 store instruction
>> - Only decode thumb instruction if it's a 32-bit guest
>> ---
>> xen/arch/arm/Makefile | 1 +
>> xen/arch/arm/decode.c | 168
>> +++++++++++++++++++++++++++++++++++++++++++++++++
>> xen/arch/arm/decode.h | 49 +++++++++++++++
>> 3 files changed, 218 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..0e7d8ba
>> --- /dev/null
>> +++ b/xen/arch/arm/decode.c
>> @@ -0,0 +1,168 @@
>> +/*
>> + * 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"
>> +
>> +static void update_dabt(struct hsr_dabt *dabt, int reg,
>> + uint8_t size, bool_t sign)
>> +{
>> + dabt->reg = reg;
>> + dabt->size = size;
>> + dabt->sign = sign;
>> +}
>> +
>> +/* TODO: Handle all THUMB2 instruction other than simple store */
>
> I think we decided to remove this (and the other one).
Oh right, I forgot to remove it.
>> +static int decode_thumb2(register_t pc, struct hsr_dabt *dabt, uint16_t hw1)
>> +{
>> + uint16_t hw2;
>> + int rc;
>> + uint16_t rt;
>> +
>> + rc = raw_copy_from_guest(&hw2, (void *__user)(pc + 2), sizeof (hw2));
>> + if ( rc )
>> + return rc;
>> +
>> + rt = (hw2 >> 12) & 0x7;
>> +
>> + switch ( (hw1 >> 9) & 0xf )
>> + {
>> + case 12:
>> + {
>> + bool_t sign = !!(hw1 & (1 << 8));
>> + bool_t load = !!(hw1 & (1 << 4));
>
> !! is unneeded with the bool type I think.
I was not sure, I will remove the '!!'.
>> +
>> + if ( (hw1 & 0x0110) == 0x0100 )
>> + /* NEON instruction */
>> + goto bad_thumb2;
>> +
>> + if ( (hw1 & 0x0070) == 0x0070 )
>> + /* Undefined opcodes */
>> + goto bad_thumb2;
>> +
>> + /* Store/Load single data item */
>> + if ( rt == 15 )
>> + /* XXX: Rt == 15 is only invalid for store instruction */
>
> So the check should be "rt == 15 && load" ?
No, when rt == 15 the instructions are memory hints (pld, pldw,..) and
the layout is different.
>> + goto bad_thumb2;
>> +
>> + if ( !load && sign )
>> + /* Store instruction doesn't support sign extension */
>> + goto bad_thumb2;
>> +
>> + update_dabt(dabt, rt, (hw1 >> 5) & 3, sign);
>> +
>> + break;
>> + }
>> + default:
>> + goto bad_thumb2;
>> + }
>> +
>> + return 0;
>> +
>> +bad_thumb2:
>> + printk("DOM%u: unhandled THUMB2 instruction 0x%x%x\n",
>> + current->domain->domain_id, hw1, hw2);
>
> Should this be gdprintk or gprintk?
I will use gdprintk.
>
>> +
>> + 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;
>> + int reg = instr & 7;
>> +
>> + switch ( opB & 0x3 )
>> + {
>> + case 0: /* Non-signed word */
>> + update_dabt(dabt, reg, 2, 0);
>> + break;
>> + case 1: /* Non-signed halfword */
>> + update_dabt(dabt, reg, 1, 0);
>> + break;
>> + case 2: /* Non-signed byte */
>> + update_dabt(dabt, reg, 0, 0);
>> + break;
>> + case 3: /* Signed byte */
>> + update_dabt(dabt, reg, 0, 1);
>> + break;
>> + }
>> +
>> + break;
>> + }
>> + case 6:
>> + /* Load/Store word immediate offset */
>> + update_dabt(dabt, instr & 7, 2, 0);
>> + break;
>> + case 7:
>> + /* Load/Store byte immediate offset */
>> + update_dabt(dabt, instr & 7, 0, 0);
>> + break;
>> + case 8:
>> + /* Load/Store halfword immediate offset */
>> + update_dabt(dabt, instr & 7, 1, 0);
>> + break;
>> + case 9:
>> + /* Load/Store word sp offset */
>> + update_dabt(dabt, (instr >> 8) & 7, 2, 0);
>> + 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);
>
> gdprint etc?
>
>> + return 1;
>> +}
>> +
>> +int decode_instruction(const struct cpu_user_regs *regs, struct hsr_dabt
>> *dabt)
>> +{
>> + if ( is_pv32_domain(current->domain) && regs->cpsr & PSR_THUMB )
>> + return decode_thumb(regs->pc, dabt);
>> +
>> + /* TODO: Handle ARM instruction */
>
> You log unhandled thumb instructions but not ARM one, I think you should
> log here too.
I'll add a warning.
>> +
>> + return 1;
>> +}
>> diff --git a/xen/arch/arm/decode.h b/xen/arch/arm/decode.h
>> new file mode 100644
>> index 0000000..4613763
>> --- /dev/null
>> +++ b/xen/arch/arm/decode.h
>> @@ -0,0 +1,49 @@
>> +/*
>> + * 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>
>> +
>> +/**
>> + * Decode an instruction from pc
>> + * /!\ This function is not intended to fully decode an instruction. It
>> + * considers that the instruction is valid.
>> + *
>> + * This function will get:
>> + * - The transfer register
>> + * - Sign bit
>> + * - Size
>> + */
>> +
>> +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:
>> + */
>
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |