| Message ID | 20240813171356.46760-2-ayan.kumar.halder@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xen: arm: Split MMU code in preparation for MPU work (part 2) | expand |
Hi Ayan, On 13/08/2024 19:13, Ayan Kumar Halder wrote: > update_boot_mapping() invokes update_identity_mapping() for the MMU specific > code. > Later when the MPU code is added, update_boot_mapping() would invoke the > equivalent. > > The common code now invokes update_boot_mapping() instead of > update_identity_mapping(). So, that there is clear abstraction between the > common and MMU/MPU specific logic. > > This is in continuation to commit > f661a20aa880: "Extract MMU-specific MM code". > > update_identity_mapping() is now marked as static as it is called within > xen/arch/arm/arm64/mmu/mm.c only. Also, updated the prototype to > update_boot_mapping() which is now invoked from other files. > > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> > --- > Changes from :- > > v1 - 1. Introduced update_boot_mapping() which invokes > update_identity_mapping() in MMU specific code. > > v2 - 1. Make update_identity_mapping() static and update the prototype. > > xen/arch/arm/arm64/mmu/mm.c | 7 ++++++- > xen/arch/arm/arm64/smpboot.c | 6 +++--- > xen/arch/arm/include/asm/arm64/mm.h | 2 +- > 3 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/arm/arm64/mmu/mm.c b/xen/arch/arm/arm64/mmu/mm.c > index 293acb67e0..1afbbeda5a 100644 > --- a/xen/arch/arm/arm64/mmu/mm.c > +++ b/xen/arch/arm/arm64/mmu/mm.c > @@ -111,7 +111,7 @@ void __init arch_setup_page_tables(void) > prepare_runtime_identity_mapping(); > } > > -void update_identity_mapping(bool enable) > +static void update_identity_mapping(bool enable) > { > paddr_t id_addr = virt_to_maddr(_start); > int rc; > @@ -125,6 +125,11 @@ void update_identity_mapping(bool enable) > BUG_ON(rc); > } > > +void update_boot_mapping(bool enable) > +{ > + update_identity_mapping(enable); > +} > + > extern void switch_ttbr_id(uint64_t ttbr); > > typedef void (switch_ttbr_fn)(uint64_t ttbr); > diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c > index a225fae64d..789f352ab6 100644 > --- a/xen/arch/arm/arm64/smpboot.c > +++ b/xen/arch/arm/arm64/smpboot.c > @@ -112,18 +112,18 @@ int arch_cpu_up(int cpu) > if ( !smp_enable_ops[cpu].prepare_cpu ) > return -ENODEV; > > - update_identity_mapping(true); > + update_boot_mapping(true); > > rc = smp_enable_ops[cpu].prepare_cpu(cpu); > if ( rc ) > - update_identity_mapping(false); > + update_boot_mapping(false); > > return rc; > } > > void arch_cpu_up_finish(void) > { > - update_identity_mapping(false); > + update_boot_mapping(false); > } > > /* > diff --git a/xen/arch/arm/include/asm/arm64/mm.h b/xen/arch/arm/include/asm/arm64/mm.h > index e0bd23a6ed..ac8d1f5c78 100644 > --- a/xen/arch/arm/include/asm/arm64/mm.h > +++ b/xen/arch/arm/include/asm/arm64/mm.h > @@ -21,7 +21,7 @@ void arch_setup_page_tables(void); > * Note that nested call (e.g. enable=true, enable=true) is not > * supported. > */ > -void update_identity_mapping(bool enable); > +void update_boot_mapping(bool enable); The whole point of adding a wrapper was to avoid ambiguity \wrt identity mapping and MPU. You changed the name of a function but left the comment which is pretty MMU specific. I think it should be moved to update_identity_mapping. Apart from that: Reviewed-by: Michal Orzel <michal.orzel@amd.com> ~Michal
On 14/08/2024 13:50, Michal Orzel wrote: > Hi Ayan, Hi Michal, > > On 13/08/2024 19:13, Ayan Kumar Halder wrote: >> update_boot_mapping() invokes update_identity_mapping() for the MMU specific >> code. >> Later when the MPU code is added, update_boot_mapping() would invoke the >> equivalent. >> >> The common code now invokes update_boot_mapping() instead of >> update_identity_mapping(). So, that there is clear abstraction between the >> common and MMU/MPU specific logic. >> >> This is in continuation to commit >> f661a20aa880: "Extract MMU-specific MM code". >> >> update_identity_mapping() is now marked as static as it is called within >> xen/arch/arm/arm64/mmu/mm.c only. Also, updated the prototype to >> update_boot_mapping() which is now invoked from other files. >> >> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> >> --- >> Changes from :- >> >> v1 - 1. Introduced update_boot_mapping() which invokes >> update_identity_mapping() in MMU specific code. >> >> v2 - 1. Make update_identity_mapping() static and update the prototype. >> >> xen/arch/arm/arm64/mmu/mm.c | 7 ++++++- >> xen/arch/arm/arm64/smpboot.c | 6 +++--- >> xen/arch/arm/include/asm/arm64/mm.h | 2 +- >> 3 files changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/xen/arch/arm/arm64/mmu/mm.c b/xen/arch/arm/arm64/mmu/mm.c >> index 293acb67e0..1afbbeda5a 100644 >> --- a/xen/arch/arm/arm64/mmu/mm.c >> +++ b/xen/arch/arm/arm64/mmu/mm.c >> @@ -111,7 +111,7 @@ void __init arch_setup_page_tables(void) >> prepare_runtime_identity_mapping(); >> } >> >> -void update_identity_mapping(bool enable) >> +static void update_identity_mapping(bool enable) >> { >> paddr_t id_addr = virt_to_maddr(_start); >> int rc; >> @@ -125,6 +125,11 @@ void update_identity_mapping(bool enable) >> BUG_ON(rc); >> } >> >> +void update_boot_mapping(bool enable) >> +{ >> + update_identity_mapping(enable); >> +} >> + >> extern void switch_ttbr_id(uint64_t ttbr); >> >> typedef void (switch_ttbr_fn)(uint64_t ttbr); >> diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c >> index a225fae64d..789f352ab6 100644 >> --- a/xen/arch/arm/arm64/smpboot.c >> +++ b/xen/arch/arm/arm64/smpboot.c >> @@ -112,18 +112,18 @@ int arch_cpu_up(int cpu) >> if ( !smp_enable_ops[cpu].prepare_cpu ) >> return -ENODEV; >> >> - update_identity_mapping(true); >> + update_boot_mapping(true); >> >> rc = smp_enable_ops[cpu].prepare_cpu(cpu); >> if ( rc ) >> - update_identity_mapping(false); >> + update_boot_mapping(false); >> >> return rc; >> } >> >> void arch_cpu_up_finish(void) >> { >> - update_identity_mapping(false); >> + update_boot_mapping(false); >> } >> >> /* >> diff --git a/xen/arch/arm/include/asm/arm64/mm.h b/xen/arch/arm/include/asm/arm64/mm.h >> index e0bd23a6ed..ac8d1f5c78 100644 >> --- a/xen/arch/arm/include/asm/arm64/mm.h >> +++ b/xen/arch/arm/include/asm/arm64/mm.h >> @@ -21,7 +21,7 @@ void arch_setup_page_tables(void); >> * Note that nested call (e.g. enable=true, enable=true) is not >> * supported. >> */ >> -void update_identity_mapping(bool enable); >> +void update_boot_mapping(bool enable); > The whole point of adding a wrapper was to avoid ambiguity \wrt identity mapping and MPU. > You changed the name of a function but left the comment which is pretty MMU specific. I think > it should be moved to update_identity_mapping. Yes, you are correct. I will move the comment before static void update_identity_mapping(bool enable) {...} > > Apart from that: > Reviewed-by: Michal Orzel <michal.orzel@amd.com> I will keep your R-b - Ayan
diff --git a/xen/arch/arm/arm64/mmu/mm.c b/xen/arch/arm/arm64/mmu/mm.c index 293acb67e0..1afbbeda5a 100644 --- a/xen/arch/arm/arm64/mmu/mm.c +++ b/xen/arch/arm/arm64/mmu/mm.c @@ -111,7 +111,7 @@ void __init arch_setup_page_tables(void) prepare_runtime_identity_mapping(); } -void update_identity_mapping(bool enable) +static void update_identity_mapping(bool enable) { paddr_t id_addr = virt_to_maddr(_start); int rc; @@ -125,6 +125,11 @@ void update_identity_mapping(bool enable) BUG_ON(rc); } +void update_boot_mapping(bool enable) +{ + update_identity_mapping(enable); +} + extern void switch_ttbr_id(uint64_t ttbr); typedef void (switch_ttbr_fn)(uint64_t ttbr); diff --git a/xen/arch/arm/arm64/smpboot.c b/xen/arch/arm/arm64/smpboot.c index a225fae64d..789f352ab6 100644 --- a/xen/arch/arm/arm64/smpboot.c +++ b/xen/arch/arm/arm64/smpboot.c @@ -112,18 +112,18 @@ int arch_cpu_up(int cpu) if ( !smp_enable_ops[cpu].prepare_cpu ) return -ENODEV; - update_identity_mapping(true); + update_boot_mapping(true); rc = smp_enable_ops[cpu].prepare_cpu(cpu); if ( rc ) - update_identity_mapping(false); + update_boot_mapping(false); return rc; } void arch_cpu_up_finish(void) { - update_identity_mapping(false); + update_boot_mapping(false); } /* diff --git a/xen/arch/arm/include/asm/arm64/mm.h b/xen/arch/arm/include/asm/arm64/mm.h index e0bd23a6ed..ac8d1f5c78 100644 --- a/xen/arch/arm/include/asm/arm64/mm.h +++ b/xen/arch/arm/include/asm/arm64/mm.h @@ -21,7 +21,7 @@ void arch_setup_page_tables(void); * Note that nested call (e.g. enable=true, enable=true) is not * supported. */ -void update_identity_mapping(bool enable); +void update_boot_mapping(bool enable); #endif /* __ARM_ARM64_MM_H__ */
update_boot_mapping() invokes update_identity_mapping() for the MMU specific code. Later when the MPU code is added, update_boot_mapping() would invoke the equivalent. The common code now invokes update_boot_mapping() instead of update_identity_mapping(). So, that there is clear abstraction between the common and MMU/MPU specific logic. This is in continuation to commit f661a20aa880: "Extract MMU-specific MM code". update_identity_mapping() is now marked as static as it is called within xen/arch/arm/arm64/mmu/mm.c only. Also, updated the prototype to update_boot_mapping() which is now invoked from other files. Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> --- Changes from :- v1 - 1. Introduced update_boot_mapping() which invokes update_identity_mapping() in MMU specific code. v2 - 1. Make update_identity_mapping() static and update the prototype. xen/arch/arm/arm64/mmu/mm.c | 7 ++++++- xen/arch/arm/arm64/smpboot.c | 6 +++--- xen/arch/arm/include/asm/arm64/mm.h | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-)