On 20.11.2021 02:41, Stefano Stabellini wrote:
> On Fri, 19 Nov 2021, Ayan Kumar Halder wrote:
>> +static int decode_64bit_loadstore_postindexing(register_t pc, struct
>> hsr_dabt *dabt)
>> +{
>> + uint32_t instr;
>> + int size;
>> + int v;
>> + int opc;
>> + int rt;
>> + int imm9;
>> +
>> + /* For details on decoding, refer to Armv8 Architecture reference manual
>> + * Section - "Load/store register (immediate post-indexed)", Pg 318
>> + */
>> + if ( raw_copy_from_guest(&instr, (void * __user)pc, sizeof (instr)) )
>> + return -EFAULT;
>> +
>> + /* First, let's check for the fixed values */
>> +
>> + /* As per the "Encoding table for the Loads and Stores group", Pg 299
>> + * op4 = 1 - Load/store register (immediate post-indexed)
>> + */
>> + if ( extract32(instr, 10, 2) != 1 )
>> + goto bad_64bit_loadstore;
>> +
>> + /* For the following, refer to "Load/store register (immediate
>> post-indexed)"
>> + * to get the fixed values at various bit positions.
>> + */
>> + if ( extract32(instr, 21, 1) != 0 )
>> + goto bad_64bit_loadstore;
>> +
>> + if ( extract32(instr, 24, 2) != 0 )
>> + goto bad_64bit_loadstore;
>> +
>> + if ( extract32(instr, 27, 3) != 7 )
>> + goto bad_64bit_loadstore;
>> +
>> + size = extract32(instr, 30, 2);
>> + v = extract32(instr, 26, 1);
>> + opc = extract32(instr, 22, 1);
>> +
>> + /* At the moment, we support STR(immediate) - 32 bit variant and
>> + * LDR(immediate) - 32 bit variant only.
>> + */
>> + if (!((size==2) && (v==0) && ((opc==0) || (opc==1))))
>> + goto bad_64bit_loadstore;
>
> The opc field is actually 2 bits, not 1. I think we should get both
> bits for opc even if some of the configurations are not interesting.
Even more so that checking the value extracted from a 1-bit field
against both 0 and 1 is pointless.
Jan
