Hi,
On 07/02/2023 03:59, Penny Zheng wrote:
-----Original Message-----
From: Julien Grall <jul...@xen.org>
Sent: Monday, February 6, 2023 5:30 AM
To: Penny Zheng <penny.zh...@arm.com>; xen-devel@lists.xenproject.org
Cc: Wei Chen <wei.c...@arm.com>; Stefano Stabellini
<sstabell...@kernel.org>; Bertrand Marquis <bertrand.marq...@arm.com>;
Volodymyr Babchuk <volodymyr_babc...@epam.com>
Subject: Re: [PATCH v2 15/40] xen/arm: move MMU-specific memory
management code to mm_mmu.c/mm_mmu.h
Hi,
On 13/01/2023 05:28, Penny Zheng wrote:
From: Wei Chen <wei.c...@arm.com>
To make the code readable and maintainable, we move MMU-specific
memory management code from mm.c to mm_mmu.c and move MMU-
specific
definitions from mm.h to mm_mmu.h.
Later we will create mm_mpu.h and mm_mpu.c for MPU-specific memory
management code.
This sentence implies there is no mm_mpu.{c, h} yet and this is not touched
within this patch. However...
This will avoid lots of #ifdef in memory management code and header files.
Signed-off-by: Wei Chen <wei.c...@arm.com>
Signed-off-by: Penny Zheng <penny.zh...@arm.com>
---
xen/arch/arm/Makefile | 5 +
xen/arch/arm/include/asm/mm.h | 19 +-
xen/arch/arm/include/asm/mm_mmu.h | 35 +
xen/arch/arm/mm.c | 1352 +---------------------------
xen/arch/arm/mm_mmu.c | 1376
+++++++++++++++++++++++++++++
xen/arch/arm/mm_mpu.c | 67 ++
... It looks like they already exists and you are modifying them. That
said, it would be better if this patch only contains code movement (IOW
no MPU changes).
6 files changed, 1488 insertions(+), 1366 deletions(-)
create mode 100644 xen/arch/arm/include/asm/mm_mmu.h
create mode 100644 xen/arch/arm/mm_mmu.c
I don't particular like the naming. I think it would make more sense to
introduce two directories: "mmu" and "mpu" which includes code specific
to each flavor of Xen.
[...]
-
-/* Release all __init and __initdata ranges to be reused */
-void free_init_memory(void)
This function doesn't look specific to the MMU.
Functions like, early_fdt_map[1] / setup_frametable_mappings[2] /
free_init_memory [3] ...
I looked at setup_frametable_mappings() and didn't think it was possible
to share much code. But I agree for early_fdt_map and free_init_memory().
they both share quite the same logic as MMU does in MPU system, the difference
could only
be address translation regime. Still, in order to avoid putting too much #ifdef
here and there,
I implement different MMU and MPU version of them.
I am not sure why you would need to put #ifdef in the code. Looking at
it, there is usually only a chunk that is different for the mapping. So
you could provide an helper that will be implemented in the MMU/MPU code.
Or I keep them in generic file here, then in future commits when we implement MPU version
of them(I list related commits below), I transfer them to MMU file there.
I am not entirely sure. In one way it help to figure out the commonality
but on the other way we are mixing code movement and new code.
In this case, the code movement would probably small, so that might be
better for the review.
Cheers,
--
Julien Grall