Hi Julien, > On Oct 14, 2023, at 01:48, Julien Grall <[email protected]> wrote: > > On 13/10/2023 18:47, Julien Grall wrote: >> Hi Henry, >> On 09/10/2023 02:03, Henry Wang wrote: >>> From: Penny Zheng <[email protected]> >>> >>> init_secondary_pagetables() is a function in the common code path >>> of both MMU and future MPU support. Since "page table" is a MMU >>> specific concept, rename init_secondary_pagetables() to a generic >>> name prepare_secondary_mm() as the preparation for MPU support. >>> >>> Take the opportunity to fix the incorrect coding style of the in-code >>> comments. >>> >>> Signed-off-by: Penny Zheng <[email protected]> >>> Signed-off-by: Henry Wang <[email protected]> >>> --- >>> v7: >>> - No change. >>> v6: >>> - Only rename init_secondary_pagetables() to prepare_secondary_mm(). >>> --- >>> xen/arch/arm/arm32/head.S | 2 +- >>> xen/arch/arm/include/asm/mm.h | 8 +++++--- >>> xen/arch/arm/mmu/smpboot.c | 4 ++-- >>> xen/arch/arm/smpboot.c | 2 +- >>> 4 files changed, 9 insertions(+), 7 deletions(-) >>> >>> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S >>> index 39218cf15f..c7b2efb8f0 100644 >>> --- a/xen/arch/arm/arm32/head.S >>> +++ b/xen/arch/arm/arm32/head.S >>> @@ -257,7 +257,7 @@ GLOBAL(init_secondary) >>> secondary_switched: >>> /* >>> * Non-boot CPUs need to move on to the proper pagetables, which >>> were >>> - * setup in init_secondary_pagetables. >>> + * setup in prepare_secondary_mm. >>> * >>> * XXX: This is not compliant with the Arm Arm. >>> */ >>> diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h >>> index d23ebc7df6..db6d889826 100644 >>> --- a/xen/arch/arm/include/asm/mm.h >>> +++ b/xen/arch/arm/include/asm/mm.h >>> @@ -204,9 +204,11 @@ extern void setup_pagetables(unsigned long >>> boot_phys_offset); >>> extern void *early_fdt_map(paddr_t fdt_paddr); >>> /* Remove early mappings */ >>> extern void remove_early_mappings(void); >>> -/* Allocate and initialise pagetables for a secondary CPU. Sets init_ttbr >>> to the >>> - * new page table */ >>> -extern int init_secondary_pagetables(int cpu); >>> +/* >>> + * Allocate and initialise pagetables for a secondary CPU. >>> + * Sets init_ttbr to the new page table. >>> + */ >> AFAIU, with the renaming, you are trying to make the call MMU/MPU agnostic. >> But the comment is still very tailored to the MPU. I would consider to move >> the comment to mmu/smpboot.c and replace this one with a generic comment. >> Something like: >> /* Prepare the memory subystem to bring-up the given secondary CPU. */
Good suggestion! I will follow this in v8. > > I forgot to mention. With that: > > Reviewed-by: Julien Grall <[email protected]> Thanks! Kind regards, Henry > > Cheers, > > -- > Julien Grall
