Re: [Qemu-devel] [PATCH v3 2/3] tcg/aarch64: Use ADRP+ADD to compute target address

[Pranith Kumar]

On Fri, Jun 30, 2017 at 12:47 AM, Richard Henderson <address@hidden> wrote:
> On 06/29/2017 05:40 PM, Pranith Kumar wrote:
>>
>>   void aarch64_tb_set_jmp_target(uintptr_t jmp_addr, uintptr_t addr)
>>   {
>>       tcg_insn_unit *code_ptr = (tcg_insn_unit *)jmp_addr;
>> -    tcg_insn_unit *target = (tcg_insn_unit *)addr;
>> +    tcg_insn_unit i1, i2;
>> +    uint64_t pair;
>>   +    ptrdiff_t offset = addr - jmp_addr;
>> +
>> +    if (offset == sextract64(offset, 0, 26)) {
>> +        i1 = NOP;
>> +        i2 = I3206_B | ((offset >> 2) & 0x3ffffff);
>
>
> Branch first, since that's the offset you calculated.
> Also, the nop need not be executed.

This is exactly how I form the instruction pair below (B+NOP, not
NOP+B). But I get your point. It is confusing to use i1 for the second
instruction. I'll change it.

>
>> +    } else {
>> +        offset = (addr >> 12) - (jmp_addr >> 12);
>> +
>> +        /* patch ADRP */
>> +        i2 = deposit32(*code_ptr++, 29, 2, offset & 0x3);
>> +        i2 = deposit32(i2, 5, 19, offset >> 2);
>> +        /* patch ADDI */
>> +        i1 = deposit32(*code_ptr, 10, 12, addr & 0xfff);
>
>
> You can't just patch these insns, because they aren't necessarily ADRP+ADD.
> Indeed, they will very likely be B and NOP.  The first address we patch in
> is tb_jmp_reset_offset, which is the following opcode, which is definitely
> in range of the branch above.

Whoops, I totally missed that we patch these out the first time out. I
will explicitly generate the ADRP+ADD pair from here.

Thanks,
-- 
Pranith