Re: [for-4.19] xen/arm64: head: only use the macro load_paddr() in the MMU code

From: Julien Grall <julien@xen.org>
To: Michal Orzel <michal.orzel@amd.com>, xen-devel@lists.xenproject.org
Cc: ayan.kumar.halder@amd.com,
	Stefano Stabellini <sstabellini@kernel.org>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [for-4.19] xen/arm64: head: only use the macro load_paddr() in the MMU code
Date: Tue, 17 Oct 2023 15:39:49 +0100	[thread overview]
Message-ID: <2b10267a-514c-4c9b-b715-f65c26d7f757@xen.org> (raw)
In-Reply-To: <9487bcbc-fedd-4d0e-86a4-023480bc9c54@amd.com>

Hi,

On 17/10/2023 15:07, Michal Orzel wrote:
> On 17/10/2023 14:52, Julien Grall wrote:
>>
>>
>> The macro load_paddr() requires to know the offset between the
>> physical location of Xen and the virtual location.
>>
>> When using the MPU, x20 will always be 0. Rather than wasting
>> a register for a compile-time constant value, it would be best if
>> we can avoid using load_paddr() altogher in the common head.S code.
> s/altogher/altogether/

I will fix it.

> 
>>
>> The current use of load_paddr() are equivalent to adr_l() because
>> the MMU is off.
>>
>> All the use of load_paddr() in arm64/head.S are now replaced with
>> adr_l(). With that, load_paddr() can now be moved in arm64/mmu/head.S.
>>
>> For now, x20 is still unconditionally set. But this could change
>> in the future if needed.
>>
>> Signed-off-by: Julien Grall <julien@xen.org>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
> 
> Side note:
> Looking at all the uses of load_paddr(), none of them takes place after enabling MMU
> which would indicate that we actually don't need this macro at all. Do you agree?

I agree they are none today called after enabling the MMU. But, 
create_table_entry() used to be called after and I can't rule out this 
will not happen again.

So I don't think we should replace the use in create_table_entry() with 
adr_l. We could consider to open-code code though.

Cheers,

-- 
Julien Grall