On 22/12/2023 15:13, Oleksii Kurochko wrote:
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
---
Changes in V3:
- add SPDX
- drop unneeded for now p2m types.
- return false in all functions implemented with BUG() inside.
- update the commit message
---
Changes in V2:
- Nothing changed. Only rebase.
---
xen/arch/ppc/include/asm/p2m.h | 3 +-
xen/arch/riscv/include/asm/p2m.h | 102 +++++++++++++++++++++++++++++++
2 files changed, 103 insertions(+), 2 deletions(-)
create mode 100644 xen/arch/riscv/include/asm/p2m.h
diff --git a/xen/arch/ppc/include/asm/p2m.h b/xen/arch/ppc/include/asm/p2m.h
index 25ba054668..3bc05b7c05 100644
--- a/xen/arch/ppc/include/asm/p2m.h
+++ b/xen/arch/ppc/include/asm/p2m.h
@@ -50,8 +50,7 @@ static inline void memory_type_changed(struct domain *d)
static inline int guest_physmap_mark_populate_on_demand(struct domain *d,
unsigned long gfn,
unsigned int order)
{
- BUG_ON("unimplemented");
- return 1;
+ return -EOPNOTSUPP;
}
static inline int guest_physmap_add_entry(struct domain *d,
diff --git a/xen/arch/riscv/include/asm/p2m.h b/xen/arch/riscv/include/asm/p2m.h
new file mode 100644
index 0000000000..d270ef6635
--- /dev/null
+++ b/xen/arch/riscv/include/asm/p2m.h
@@ -0,0 +1,102 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_RISCV_P2M_H__
+#define __ASM_RISCV_P2M_H__
+
+#include <asm/page-bits.h>
+
+#define paddr_bits PADDR_BITS
+
+/*
+ * List of possible type for each page in the p2m entry.
+ * The number of available bit per page in the pte for this purpose is 4 bits.
+ * So it's possible to only have 16 fields. If we run out of value in the
+ * future, it's possible to use higher value for pseudo-type and don't store
+ * them in the p2m entry.
+ */
This looks like a verbatim copy from Arm. Did you actually check RISC-V
has 4 bits available in the PTE to store this value?
+typedef enum {
+ p2m_invalid = 0, /* Nothing mapped here */
+ p2m_ram_rw, /* Normal read/write guest RAM */
s/guest/domain/ as this also applies for dom0.
+} p2m_type_t;
+
+#include <xen/p2m-common.h>
+
+static inline int get_page_and_type(struct page_info *page,
+ struct domain *domain,
+ unsigned long type)
+{
+ BUG();
I understand your goal with the BUG() but I find it risky. This is not a
problem right now, it is more when we will decide to have RISC-V
supported. You will have to go through all the BUG() to figure out which
one are warrant or not.
To reduce the load, I would recommend to switch to ASSERT_UNREACHABLE()
(or maybe introduced a different macro) that would lead to a crash on
debug build but propagate the error normally on production build.
