| Message ID | 20250108075719.84967-1-michal.orzel@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [for-4.20] xen/arm64: Drop relocate_and_switch_ttbr() stub | expand |
> 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>
> 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> > > >
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)
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 >
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)
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(-)