xen/arm64: Drop relocate_and_switch_ttbr() stub

[for-4.20][PATCH] xen/arm64: Drop relocate_and_switch_ttbr() stub

Posted by Michal Orzel 9 months, 4 weeks ago

In the original patch e7a80636f16e ("xen/arm: add cache coloring support
for Xen image"), the stub was added under wrong assumption that DCE
won't remove the function call if it's not static. This assumption is
incorrect as we already rely on DCE for cases like this one. Therefore
drop the stub, that otherwise would be a place potentially prone to
errors in the future.

Suggested-by: Julien Grall <julien@xen.org>
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
As suggested by Julien, we should have it for 4.20. Leaving a stub like that
without something like BUG_ON inside can potentially lead to problems in
the future provided the function misuse slipped through the review process.
---
 xen/arch/arm/arm64/mmu/mm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/xen/arch/arm/arm64/mmu/mm.c b/xen/arch/arm/arm64/mmu/mm.c
index 26361c4fe4c0..c1efa1348aee 100644
--- a/xen/arch/arm/arm64/mmu/mm.c
+++ b/xen/arch/arm/arm64/mmu/mm.c
@@ -171,8 +171,6 @@ void __init relocate_and_switch_ttbr(uint64_t ttbr)
      */
     update_identity_mapping(false);
 }
-#else
-void __init relocate_and_switch_ttbr(uint64_t ttbr) {}
 #endif
 
 void __init switch_ttbr(uint64_t ttbr)
-- 
2.25.1

Re: [for-4.20][PATCH] xen/arm64: Drop relocate_and_switch_ttbr() stub

Posted by Bertrand Marquis 9 months, 4 weeks ago

Hi Michal

> On 8 Jan 2025, at 08:57, Michal Orzel <michal.orzel@amd.com> wrote:
> 
> In the original patch e7a80636f16e ("xen/arm: add cache coloring support
> for Xen image"), the stub was added under wrong assumption that DCE
> won't remove the function call if it's not static. This assumption is
> incorrect as we already rely on DCE for cases like this one. Therefore
> drop the stub, that otherwise would be a place potentially prone to
> errors in the future.
> 
> Suggested-by: Julien Grall <julien@xen.org>
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> ---
> As suggested by Julien, we should have it for 4.20. Leaving a stub like that
> without something like BUG_ON inside can potentially lead to problems in
> the future provided the function misuse slipped through the review process.
> ---
> xen/arch/arm/arm64/mmu/mm.c | 2 --
> 1 file changed, 2 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/mmu/mm.c b/xen/arch/arm/arm64/mmu/mm.c
> index 26361c4fe4c0..c1efa1348aee 100644
> --- a/xen/arch/arm/arm64/mmu/mm.c
> +++ b/xen/arch/arm/arm64/mmu/mm.c
> @@ -171,8 +171,6 @@ void __init relocate_and_switch_ttbr(uint64_t ttbr)
>      */
>     update_identity_mapping(false);
> }
> -#else
> -void __init relocate_and_switch_ttbr(uint64_t ttbr) {}
> #endif
> 
> void __init switch_ttbr(uint64_t ttbr)
> -- 
> 2.25.1
>

Re: [for-4.20][PATCH] xen/arm64: Drop relocate_and_switch_ttbr() stub

Posted by Oleksii Kurochko 9 months, 4 weeks ago

On 1/8/25 8:57 AM, Michal Orzel wrote:
> In the original patch e7a80636f16e ("xen/arm: add cache coloring support
> for Xen image"), the stub was added under wrong assumption that DCE
> won't remove the function call if it's not static. This assumption is
> incorrect as we already rely on DCE for cases like this one. Therefore
> drop the stub, that otherwise would be a place potentially prone to
> errors in the future.
>
> Suggested-by: Julien Grall<julien@xen.org>
> Signed-off-by: Michal Orzel<michal.orzel@amd.com>
> ---
> As suggested by Julien, we should have it for 4.20. Leaving a stub like that
> without something like BUG_ON inside can potentially lead to problems in
> the future provided the function misuse slipped through the review process.
> ---

Release-Acked-By: Oleksii Kurochko <oleksii.kurochko@gmail.com>

~ Oleksii

>   xen/arch/arm/arm64/mmu/mm.c | 2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/mmu/mm.c b/xen/arch/arm/arm64/mmu/mm.c
> index 26361c4fe4c0..c1efa1348aee 100644
> --- a/xen/arch/arm/arm64/mmu/mm.c
> +++ b/xen/arch/arm/arm64/mmu/mm.c
> @@ -171,8 +171,6 @@ void __init relocate_and_switch_ttbr(uint64_t ttbr)
>        */
>       update_identity_mapping(false);
>   }
> -#else
> -void __init relocate_and_switch_ttbr(uint64_t ttbr) {}
>   #endif
>   
>   void __init switch_ttbr(uint64_t ttbr)

Re: [for-4.20][PATCH] xen/arm64: Drop relocate_and_switch_ttbr() stub

Posted by Luca Fancellu 9 months, 4 weeks ago


> On 8 Jan 2025, at 07:57, Michal Orzel <michal.orzel@amd.com> wrote:
> 
> In the original patch e7a80636f16e ("xen/arm: add cache coloring support
> for Xen image"), the stub was added under wrong assumption that DCE
> won't remove the function call if it's not static. This assumption is
> incorrect as we already rely on DCE for cases like this one. Therefore
> drop the stub, that otherwise would be a place potentially prone to
> errors in the future.
> 
> Suggested-by: Julien Grall <julien@xen.org>
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> ---

Reviewed-by: Luca Fancellu <luca.fancellu@gmail.com>

Re: [for-4.20][PATCH] xen/arm64: Drop relocate_and_switch_ttbr() stub

Posted by Luca Fancellu 9 months, 4 weeks ago


> On 8 Jan 2025, at 09:01, Luca Fancellu <Luca.Fancellu@arm.com> wrote:
> 
> 
> 
>> On 8 Jan 2025, at 07:57, Michal Orzel <michal.orzel@amd.com> wrote:
>> 
>> In the original patch e7a80636f16e ("xen/arm: add cache coloring support
>> for Xen image"), the stub was added under wrong assumption that DCE
>> won't remove the function call if it's not static. This assumption is
>> incorrect as we already rely on DCE for cases like this one. Therefore
>> drop the stub, that otherwise would be a place potentially prone to
>> errors in the future.
>> 
>> Suggested-by: Julien Grall <julien@xen.org>
>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>> ---
> 
> Reviewed-by: Luca Fancellu <luca.fancellu@gmail.com>

Apologies, I meant:

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

> 
> 
>