Re: [PATCH 2/5] xen/ppc: Switch to medium PIC code model

[Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>]

On 7/31/23 10:58 AM, Jan Beulich wrote:
> On 28.07.2023 23:35, Shawn Anastasio wrote:
>> --- a/xen/arch/ppc/ppc64/head.S
>> +++ b/xen/arch/ppc/ppc64/head.S
>> @@ -1,9 +1,11 @@
>>  /* SPDX-License-Identifier: GPL-2.0-or-later */
>>  
>>  #include <asm/asm-defns.h>
>> +#include <asm/asm-offsets.h>
>>  
>>      .section .text.header, "ax", %progbits
>>  
>> +
>>  ENTRY(start)
> 
> Nit: Stray change?
> 
>> @@ -11,16 +13,19 @@ ENTRY(start)
>>      FIXUP_ENDIAN
>>  
>>      /* set up the TOC pointer */
>> -    LOAD_IMM32(%r2, .TOC.)
>> +    bcl         20, 31, .+4
> 
> Could you use a label name instead of .+4? Aiui you really mean
> 
>> +1:  mflr    %r12
> 
> ... "1f" there?

Yes, good point. I'll point out that this form of the `bcl` instruction
is specifically defined in the ISA specification as the recommended
way to obtain the address of the next instruction, and hardware
implementations presumably optimize it. Using a label instead of +4
would of course be fine as long as the label immediately follows the
bcl, but if the label was elsewhere then the optimization that the ISA
allows for this specific instruction might not be hit. Just something
that should be kept in mind in case this code is ever refactored.

I'll change it to 1f in v2.

> 
> Jan

Thanks,
Shawn