Re: [Xen-devel] [RFC 09/16] xen/arm: arm64: Add helpers to decode and encode branch instructions

[Julien Grall <julien.grall@xxxxxxx>]

Hi Konrad,

On 13/05/2016 21:35, Konrad Rzeszutek Wilk wrote:
> > diff --git a/xen/include/asm-arm/arm64/insn.h b/xen/include/asm-arm/arm64/insn.h
> > new file mode 100644
> > index 0000000..cfcdbe9
> > --- /dev/null
> > +++ b/xen/include/asm-arm/arm64/insn.h
> > @@ -0,0 +1,72 @@
> > +/*
> > + * Copyright (C) 2013 Huawei Ltd.
> > + * Author: Jiang Liu <liuj97@xxxxxxxxx>
> > + *
> > + * Copyright (C) 2014 Zi Shen Lim <zlim.lnx@xxxxxxxxx>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * 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.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +#ifndef __ARCH_ARM_ARM64_INSN
> > +#define __ARCH_ARM_ARM64_INSN
> > +
> > +#include <xen/config.h>
> > +#include <xen/types.h>
> > +#include <xen/stdbool.h>
> > +
> > +enum aarch64_insn_imm_type {
> > +       AARCH64_INSN_IMM_ADR,
> > +       AARCH64_INSN_IMM_26,
> > +       AARCH64_INSN_IMM_19,
> > +       AARCH64_INSN_IMM_16,
> > +       AARCH64_INSN_IMM_14,
> > +       AARCH64_INSN_IMM_12,
> > +       AARCH64_INSN_IMM_9,
> > +       AARCH64_INSN_IMM_7,
> > +       AARCH64_INSN_IMM_6,
> > +       AARCH64_INSN_IMM_S,
> > +       AARCH64_INSN_IMM_R,
> > +       AARCH64_INSN_IMM_MAX
> > +};
> > +
> > +#define        __AARCH64_INSN_FUNCS(abbr, mask, val)   \
> > +static always_inline bool_t aarch64_insn_is_##abbr(u32 code) \
> > +{ return (code & (mask)) == (val); } \
> > +static always_inline u32 aarch64_insn_get_##abbr##_value(void) \
> > +{ return (val); }
> > +
> > +__AARCH64_INSN_FUNCS(b,                0xFC000000, 0x14000000)
> This looks odd here but in the file the aligment is OK.
> > +__AARCH64_INSN_FUNCS(bl,       0xFC000000, 0x94000000)
> > +__AARCH64_INSN_FUNCS(cbz,      0x7F000000, 0x34000000)
> > +__AARCH64_INSN_FUNCS(cbnz,     0x7F000000, 0x35000000)
> > +__AARCH64_INSN_FUNCS(tbz,      0x7F000000, 0x36000000)
> > +__AARCH64_INSN_FUNCS(tbnz,     0x7F000000, 0x37000000)
> > +__AARCH64_INSN_FUNCS(bcond,    0xFF000010, 0x54000000)
> > +
> > +bool aarch64_insn_is_branch_imm(u32 insn);
> > +
> > +u64 aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn);
> > +u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
> > +                                 u32 insn, u64 imm);
> While this is off. I think you have tabs here instead of spaces?
The code is a verbatim copy from Linux, so I would prefer to not modify 
the code for future port/bug fixing.
Regards,

--
Julien Grall

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