On 27.06.2022 11:52, Julien Grall wrote:
>
>
> On 27/06/2022 07:31, Michal Orzel wrote:
>> Hi Julien,
>
> Hi Michal,
>
>> On 24.06.2022 11:11, Julien Grall wrote:
>>> From: Julien Grall <[email protected]>
>>>
>>> A lot of places in the ARM32 assembly requires to load the physical address
>>> of a symbol. Rather than open-coding the translation, introduce a new macro
>>> that will load the phyiscal address of a symbol.
>>>
>>> Lastly, use the new macro to replace all the current open-coded version.
>>>
>>> Note that most of the comments associated to the code changed have been
>>> removed because the code is now self-explanatory.
>>>
>>> Signed-off-by: Julien Grall <[email protected]>
>>> ---
>>> xen/arch/arm/arm32/head.S | 23 +++++++++++------------
>>> 1 file changed, 11 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
>>> index c837d3054cf9..77f0a619ca51 100644
>>> --- a/xen/arch/arm/arm32/head.S
>>> +++ b/xen/arch/arm/arm32/head.S
>>> @@ -65,6 +65,11 @@
>>> .endif
>>> .endm
>>> +.macro load_paddr rb, sym
>>> + ldr \rb, =\sym
>>> + add \rb, \rb, r10
>>> +.endm
>>> +
>> All the macros in this file have a comment so it'd be useful to follow this
>> convention.
> This is not really a convention. Most of the macros are non-trivial (e.g.
> they may clobber registers).
>
> The comment I have in mind here would be:
>
> "Load the physical address of \sym in \rb"
>
> I am fairly confident that anyone can understand from the ".macro" line... So
> I don't feel the comment is necessary.
>
Fair enough although you did put a comment when introducing load_paddr for
arm64 head.S.
Anyway, I'm ok not to add it.
> Cheers,
>
