Re: [PATCH v8 8/8] xen/arm: mmu: move MMU specific P2M code to mmu/p2m.{c,h}

[Julien Grall <julien@xxxxxxx>]

On 02/11/2023 00:35, Henry Wang wrote:
> Hi Julien,
Hi,

> > On Nov 2, 2023, at 02:34, Julien Grall <julien@xxxxxxx> wrote:
> >
> > Hi Henry,
> >
> > On 23/10/2023 03:13, Henry Wang wrote:
> > > From: Penny Zheng <penny.zheng@xxxxxxx>
> > > Current P2M implementation is designed for MMU system only.
> > > We move the MMU-specific codes into mmu/p2m.c, and only keep generic
> > > codes in p2m.c, like VMID allocator, etc. We also move MMU-specific
> > > definitions and declarations to mmu/p2m.h, such as p2m_tlb_flush_sync().
> > > Also expose previously static functions p2m_vmid_allocator_init(),
> > > p2m_alloc_vmid() for further MPU usage. Since with the code movement
> > > p2m_free_vmid() is now used in two files, also expose p2m_free_vmid().
> > > With the code movement, global variable max_vmid is used in multiple
> > > files instead of a single file (and will be used in MPU P2M
> > > implementation), declare it in the header and remove the "static" of
> > > this variable.
> > > Also, since p2m_invalidate_root() should be MMU only and after the
> > > code movement the only caller of p2m_invalidate_root() outside of
> > > mmu/p2m.c is arch_domain_creation_finished(), creating a new function
> > > named p2m_domain_creation_finished() in mmu/p2m.c for the original
> > > code in arch_domain_creation_finished(), and marking
> > > p2m_invalidate_root() as static.
> > > Take the opportunity to fix the incorrect coding style when possible.
> > > When there is bit shift in macros, take the opportunity to add the
> > > missing 'U' as a compliance of MISRA.
> > > Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> > > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> > > Signed-off-by: Henry Wang <Henry.Wang@xxxxxxx>
> > Acked-by: Julien Grall <jgrall@xxxxxxxxxx>
> Thanks!
>
> > I think the series is now fully acked. But I will wait for 4.18 to be released 
> > before merging this series.
> I think the third patch "xen/arm: Fold mmu_init_secondary_cpu() to head.S” will 
> need the
> double check from your side :)
Oh yes :).

> Here is what I have locally, to save time I will just show the content here for 
> you to check,
> and I will push it in the next few days:
>
> commit ba72d6dc17fd7ce9a863b9e00b06b33c069c7641
> Author: Henry Wang <Henry.Wang@xxxxxxx>
> Date:   Wed Aug 23 17:59:50 2023 +0800
>
>      xen/arm: Fold mmu_init_secondary_cpu() to head.S
>
>      Currently mmu_init_secondary_cpu() only enforces the page table
>      should not contain mapping that are both Writable and eXecutables
>      after boot. To ease the arch/arm/mm.c split work, fold this function
>      to head.S.
>
>      For arm32, the WXN bit cannot be set early because at the point when
>      the MMU is enabled, the page-tables may still contain mapping which
>      are writable and executable. Therefore, introduce an assembly macro
>      pt_enforce_wxn. The macro is called before secondary CPUs jumping
>      into the C world.
>
>      For arm64, set the SCTLR_Axx_ELx_WXN flag right when the MMU is
>      enabled. This would avoid the extra TLB flush and SCTLR dance.
>
>      Signed-off-by: Henry Wang <Henry.Wang@xxxxxxx>
>      Co-authored-by: Julien Grall <jgrall@xxxxxxxxxx>
>      Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
>      Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
The commit message is clearer. Thanks! Feel free to add my tag for the 
next version:
Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall