Re: [PATCH v6 07/13] xen/arm: Split MMU system SMP MM bringup code to mmu/smpboot.c

[Henry Wang <Henry.Wang@xxxxxxx>]

Hi Julien,

> On Sep 26, 2023, at 03:13, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi Henry,
> 
> On 28/08/2023 02:32, Henry Wang wrote:
>> Move the code related to secondary page table initialization, clear
>> boot page tables and the global variable definitions of these boot
>> page tables from arch/arm/mm.c to arch/arm/mmu/smpboot.c
>> Drop the "static" attribute of arm32 global variable cpu0_pgtable,
>> and declare it in the arm32 mm.h header, because this variable
>> will be used by both arch/arm/mm.c and arch/arm/mmu/smpboot.c.
> 
> I don't think you need to export cpu0_pgtable. You can instead replace use 
> with per_cpu(xen_pgtable, 0).

You mean using
```
memcpy(first, per_cpu(xen_pgtable, 0), PAGE_SIZE);
```
in the arm32 implementation of prepare_secondary_mm() in mmu/smpboot.c?

That sounds good to me and I will fix in v7.

> 
>> + */
>> +DEFINE_BOOT_PAGE_TABLE(boot_pgtable);
>> +#ifdef CONFIG_ARM_64
>> +DEFINE_BOOT_PAGE_TABLE(boot_first);
>> +DEFINE_BOOT_PAGE_TABLE(boot_first_id);
>> +#endif
>> +DEFINE_BOOT_PAGE_TABLE(boot_second_id);
>> +DEFINE_BOOT_PAGE_TABLE(boot_third_id);
>> +DEFINE_BOOT_PAGE_TABLE(boot_second);
>> +DEFINE_BOOT_PAGE_TABLES(boot_third, XEN_NR_ENTRIES(2));
> 
> Strictly speaking, boot_* are also used for CPU0 but not in C code. That 
> said, I am ok if there are moved here.
> 
> In the longer term, I would actually like to remove boot_*. This should 
> already be possible for arm64 (in fact they are not used for secondary CPU 
> bring-up). For arm32, it requires a bit more work.
> 
> Anyway, that's not a request for your series. :)

Let’s see if we can do that in 4.19 dev cycle then :)

> 
>> +    /* Set init_ttbr for this CPU coming up */
>> +    init_ttbr = __pa(first);
> 
> init_ttbr will only be used by smpboot.c and assembly code. So I think the 
> declaration needs to also be moved and the prototype moved to mmu/mm.h.

Ok, will do in v7.

Kind regards,
Henry

> 
> Cheers,
> 
> -- 
> Julien Grall