Hi Henry,
On 09/10/2023 02:03, Henry Wang wrote:
From: Penny Zheng <[email protected]>
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(), and setup_virt_paging_one() for further MPU usage.
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.
Signed-off-by: Penny Zheng <[email protected]>
Signed-off-by: Wei Chen <[email protected]>
Signed-off-by: Henry Wang <[email protected]>
Some remarks about some of the code not moved:
* struct p2m_domain: The bulk of the fields seems to be MMU specific.
So depending on the number of common fields we either want to split or
move the structure to p2m_domain. I would be ok to wait until the MPU
code is present.
* p2m_type_t: It is not yet clear how this will apply to the MPU. I am
ok to wait before moving it.
* p2m_cache_flush_range(): I expect the code will need some change
because you may get large chunk of memory for the MPU.
* p2m_set_way_flush()/p2m_toggle_cache(): This was a giant hack to
support cache flush operations via set/way. To make it efficient, we
track the pages that have been touched and only flush them. For the MPU,
this would not work. Can we attempt to not emulate the instructions?
---
v7:
- No change.
v6:
- Also move relinquish_p2m_mapping() to mmu/p2m.c, make
__p2m_set_entry() static.
- Also move p2m_clear_root_pages() and p2m_flush_vm() to mmu/p2m.c.
- Don't add #ifdef CONFIG_MMU to the p2m_tlb_flush_sync() in
p2m_write_unlock(), this need further discussion.
- Correct typo in commit message.
v5:
- No change
v4:
- Rework the patch to drop the unnecessary changes.
- Rework the commit msg a bit.
v3:
- remove MPU stubs
- adapt to the introduction of new directories: mmu/
v2:
- new commit
---
xen/arch/arm/include/asm/mmu/p2m.h | 18 +
xen/arch/arm/include/asm/p2m.h | 26 +-
xen/arch/arm/mmu/Makefile | 1 +
xen/arch/arm/mmu/p2m.c | 1736 ++++++++++++++++++++++++++
xen/arch/arm/p2m.c | 1837 +---------------------------
5 files changed, 1832 insertions(+), 1786 deletions(-)
create mode 100644 xen/arch/arm/include/asm/mmu/p2m.h
create mode 100644 xen/arch/arm/mmu/p2m.c
diff --git a/xen/arch/arm/include/asm/mmu/p2m.h
b/xen/arch/arm/include/asm/mmu/p2m.h
new file mode 100644
index 0000000000..f829e325ce
--- /dev/null
+++ b/xen/arch/arm/include/asm/mmu/p2m.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef __ARM_MMU_P2M_H__
+#define __ARM_MMU_P2M_H__
+
+struct p2m_domain;
+void p2m_force_tlb_flush_sync(struct p2m_domain *p2m);
+void p2m_tlb_flush_sync(struct p2m_domain *p2m);
+
+#endif /* __ARM_MMU_P2M_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/include/asm/p2m.h b/xen/arch/arm/include/asm/p2m.h
index 940495d42b..a9622dac9a 100644
--- a/xen/arch/arm/include/asm/p2m.h
+++ b/xen/arch/arm/include/asm/p2m.h
@@ -19,6 +19,22 @@ extern unsigned int p2m_root_level;
#define P2M_ROOT_ORDER p2m_root_order
You seem to use P2M_ROOT_ORDER to allocate p2m->root in arm/p2m.c.
However, as I mentioned before, I don't think the defintion of p2m->root
is suitable for the MPU. I think the two functions using p2m->root
should be moved in mmu/p2m.c and P2M_ROOT_ORDER should be moved in
mmu/p2m.h.
#define P2M_ROOT_LEVEL p2m_root_level
P2M_ROOT_LEVEL doesn't seem to make sense for the MPU. The only use in
arch/arm/p2m.c seems to be in dump_p2m_lookup() which is calling an MMU
specific function. So I think this wants to be moved in the MMU code.
+#define MAX_VMID_8_BIT (1UL << 8)
+#define MAX_VMID_16_BIT (1UL << 16)
+
+#define INVALID_VMID 0 /* VMID 0 is reserved */
+
+#ifdef CONFIG_ARM_64
+extern unsigned int max_vmid;
+/* VMID is by default 8 bit width on AArch64 */
+#define MAX_VMID max_vmid
+#else
+/* VMID is always 8 bit width on AArch32 */
+#define MAX_VMID MAX_VMID_8_BIT
+#endif
+
+#define P2M_ROOT_PAGES (1<<P2M_ROOT_ORDER)
See above.
Also NIT, I would suggest to take the opportunity to use 1U and add
space before/after <<.
+
struct domain;
extern void memory_type_changed(struct domain *);
@@ -156,6 +172,10 @@ typedef enum {
#endif
#include <xen/p2m-common.h>
+#ifdef CONFIG_MMU
+#include <asm/mmu/p2m.h>
+#endif
+
static inline bool arch_acquire_resource_check(struct domain *d)
{
/*
@@ -180,7 +200,11 @@ void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
*/
void p2m_restrict_ipa_bits(unsigned int ipa_bits);
+void p2m_vmid_allocator_init(void);
+int p2m_alloc_vmid(struct domain *d);
+
/* Second stage paging setup, to be called on all CPUs */
+void setup_virt_paging_one(void *data);
I don't much like the idea to export setup_virt_paging_one(). Could we
instead move cpu_virt_paging_callback() & co to mmu/p2m.c?
Cheers,
--
Julien Grall
