Hi Henry,
I haven't checked that the code movement is just a code movement. For
now, I am mainly looking at the split.
On 28/08/2023 02:32, Henry Wang wrote:
The extraction of MMU related code is the basis of MPU support.
This commit starts this work by firstly splitting the page table
related code to mmu/pt.c, so that we will not end up with again
massive mm.c files.
Introduce a mmu specific directory and setup the Makefiles for it.
Move the page table related functions and macros from arch/arm/mm.c
to arch/arm/mmu/pt.c. Expose the previously static global variable
"phys_offset".
I don't much like the idea to expose phys_offset everywhere. Looking at
the code there are two users:
* pte_of_xenaddr(): I realize you suggested to add it in pt.c and I
agreed. However, looking at the split, this function is exclusively used
for boot (and you confirmed below). So I think it would be preferable to
move it in mmu/setup.c.
* prepare_secondary_mm(): I think we could switch to virt_to_mfn().
I can understand if you don't want to make the second change. So I would
at least request to ...
While moving, mark pte_of_xenaddr() as __init to make clear that
this helper is only intended to be used during early boot.
Take the opportunity to fix the in-code comment coding styles when
possible, and drop the unnecessary #include headers in the original
arch/arm/mm.c.
Signed-off-by: Henry Wang <henry.w...@arm.com>
Signed-off-by: Penny Zheng <penny.zh...@arm.com>
---
v6:
- Rework the original patch "[v5,07/13] xen/arm: Extract MMU-specific
code", only split the page table related code out in this patch.
---
xen/arch/arm/Makefile | 1 +
xen/arch/arm/include/asm/mm.h | 2 +
... declare phys_offset in asm/mmu/mm.h because this variable doesn't
make a lot of sense when the MPU is used (it will always be equal to 0).
The rest of the split looks good to me.
[...]
-lpae_t pte_of_xenaddr(vaddr_t va)
-{
- paddr_t ma = va + phys_offset;
-
- return mfn_to_xen_entry(maddr_to_mfn(ma), MT_NORMAL);
-}
-
See above. I think this should stay here for now and the be moved to
setup.c in a follow-up patch.
Cheers,
--
Julien Grall
