Re: [PATCH 3/3] xen/arm64/mmu: head: Replace load_paddr with adr_l where appropriate

From: Julien Grall <julien@xen.org>
To: Michal Orzel <michal.orzel@amd.com>, xen-devel@lists.xenproject.org
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [PATCH 3/3] xen/arm64/mmu: head: Replace load_paddr with adr_l where appropriate
Date: Tue, 21 Nov 2023 16:30:29 +0000	[thread overview]
Message-ID: <d015e81d-16cd-4e93-80c9-ba6418a23b1d@xen.org> (raw)
In-Reply-To: <20231121094516.24714-4-michal.orzel@amd.com>

Hi Michal,

On 21/11/2023 09:45, Michal Orzel wrote:
> Macros load_paddr and adr_l are equivalent when used before the MMU is
> enabled, resulting in obtaining physical address of a symbol. The former
> requires to know the physical offset (PA - VA) and can be used both before
> and after the MMU is enabled. In the spirit of using something only when
> truly necessary, replace all instances of load_paddr with adr_l, except

I don't buy this argument. The advantage with using "load_paddr" is that 
it is pretty clear what you get from the call. With "adr_l" you will 
need to check whether the MMU is on or off.

> in create_table_entry macro. Even though there is currently no use of
> load_paddr after MMU is enabled, this macro used to be call in such a
> context and we can't rule out that it won't happen again.
> 
> This way, the logic behind using load_paddr/adr_l is consistent between
> arm32 and arm64, making it easier for developers to determine which one
> to use and when.

Not really. See above. But there is also no documentation stating that 
"load_paddr" should not be used before the MMU is on. And as I said 
above, I find it easier to work with compare to "adr_l".

Cheers,

-- 
Julien Grall