| Message ID | 20240808120936.3299937-2-ayan.kumar.halder@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: arm: Split MMU code in preparation for MPU work (part 2) | expand |
Hi Ayan, On 08/08/2024 14:09, 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". > > 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. > > xen/arch/arm/arm64/mmu/mm.c | 5 +++++ > xen/arch/arm/arm64/smpboot.c | 6 +++--- > xen/arch/arm/include/asm/mm.h | 2 ++ > 3 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/arm/arm64/mmu/mm.c b/xen/arch/arm/arm64/mmu/mm.c > index 293acb67e0..72e089a339 100644 > --- a/xen/arch/arm/arm64/mmu/mm.c > +++ b/xen/arch/arm/arm64/mmu/mm.c > @@ -125,6 +125,11 @@ void update_identity_mapping(bool enable) > BUG_ON(rc); > } > > +void update_boot_mapping(bool enable) > +{ > + update_identity_mapping(enable); With your approach, update_identity_mapping() would only be called from within this file, therefore it should be marked as static and the prototype removed. > +} > + > 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/mm.h b/xen/arch/arm/include/asm/mm.h > index f6ba611f01..e769d2d3b3 100644 > --- a/xen/arch/arm/include/asm/mm.h > +++ b/xen/arch/arm/include/asm/mm.h > @@ -426,6 +426,8 @@ static inline void page_set_xenheap_gfn(struct page_info *p, gfn_t gfn) > } while ( (y = cmpxchg(&p->u.inuse.type_info, x, nx)) != x ); > } > > +void update_boot_mapping(bool enable); The definition is only present for arm64. Shouldn't the prototype be moved to arm64 header? ~Michal
On 09/08/2024 13:01, Michal Orzel wrote: > Hi Ayan, Hi Michal, > > On 08/08/2024 14:09, 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". >> >> 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. >> >> xen/arch/arm/arm64/mmu/mm.c | 5 +++++ >> xen/arch/arm/arm64/smpboot.c | 6 +++--- >> xen/arch/arm/include/asm/mm.h | 2 ++ >> 3 files changed, 10 insertions(+), 3 deletions(-) >> >> diff --git a/xen/arch/arm/arm64/mmu/mm.c b/xen/arch/arm/arm64/mmu/mm.c >> index 293acb67e0..72e089a339 100644 >> --- a/xen/arch/arm/arm64/mmu/mm.c >> +++ b/xen/arch/arm/arm64/mmu/mm.c >> @@ -125,6 +125,11 @@ void update_identity_mapping(bool enable) >> BUG_ON(rc); >> } >> >> +void update_boot_mapping(bool enable) >> +{ >> + update_identity_mapping(enable); > With your approach, update_identity_mapping() would only be called from within this file, > therefore it should be marked as static and the prototype removed. yes, agreed. > >> +} >> + >> 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/mm.h b/xen/arch/arm/include/asm/mm.h >> index f6ba611f01..e769d2d3b3 100644 >> --- a/xen/arch/arm/include/asm/mm.h >> +++ b/xen/arch/arm/include/asm/mm.h >> @@ -426,6 +426,8 @@ static inline void page_set_xenheap_gfn(struct page_info *p, gfn_t gfn) >> } while ( (y = cmpxchg(&p->u.inuse.type_info, x, nx)) != x ); >> } >> >> +void update_boot_mapping(bool enable); > The definition is only present for arm64. Shouldn't the prototype be moved to arm64 header? Yes, I have replaced the prototype update_identity_mapping() with update_boot_mapping() in xen/arch/arm/include/asm/arm64/mm.h - Ayan
diff --git a/xen/arch/arm/arm64/mmu/mm.c b/xen/arch/arm/arm64/mmu/mm.c index 293acb67e0..72e089a339 100644 --- a/xen/arch/arm/arm64/mmu/mm.c +++ b/xen/arch/arm/arm64/mmu/mm.c @@ -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/mm.h b/xen/arch/arm/include/asm/mm.h index f6ba611f01..e769d2d3b3 100644 --- a/xen/arch/arm/include/asm/mm.h +++ b/xen/arch/arm/include/asm/mm.h @@ -426,6 +426,8 @@ static inline void page_set_xenheap_gfn(struct page_info *p, gfn_t gfn) } while ( (y = cmpxchg(&p->u.inuse.type_info, x, nx)) != x ); } +void update_boot_mapping(bool enable); + #endif /* __ARCH_ARM_MM__ */ /* * Local variables:
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". 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. xen/arch/arm/arm64/mmu/mm.c | 5 +++++ xen/arch/arm/arm64/smpboot.c | 6 +++--- xen/arch/arm/include/asm/mm.h | 2 ++ 3 files changed, 10 insertions(+), 3 deletions(-)