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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.07.2023 21:09, Shawn Anastasio wrote:
> 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.

Hmm, not really. If there was refactoring, the calculations using 1b
would also need adjusting. That's why it's relevant that all "sides"
agree on using one and the same label. (A similar "optimization"
exists in x86 as well, just fyi, so we have precedent of that.)

> I'll change it to 1f in v2.

Thanks.

Jan