[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] xen/mm: Introduce CONFIG_M2P and provide common fallback logic



Hi Andrew,

On Mon, 24 Aug 2020, 19:15 Andrew Cooper, <andrew.cooper3@xxxxxxxxxx> wrote:
Architectures which don't implement an M2P shouldn't be forced to implement
stubs.  Implement them in common code.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Julien Grall <julien@xxxxxxx>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>

I'm pretty sure the mfn_to_gmfn() stub is bogus before and after this change.
The two uses in common code are getdomaininfo and in memory_exchange(), which
result in junk.

I presume ARM toolstacks don't ever try to map info->shared_info_frame,
because I can't see how it would work.

It is broken. I had a series that tried to remove the M2P (see [1]) but there was some disagreement on how to implement it.

---
 xen/arch/x86/Kconfig     |  1 +
 xen/common/Kconfig       |  3 +++
 xen/include/asm-arm/mm.h |  5 -----
 xen/include/xen/mm.h     | 10 ++++++++++
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index a636a4bb1e..9bc97a1cf5 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -24,6 +24,7 @@ config X86
        select HAS_SCHED_GRANULARITY
        select HAS_UBSAN
        select HAS_VPCI if !PV_SHIM_EXCLUSIVE && HVM
+       select M2P
        select NEEDS_LIBELF
        select NUMA

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 15e3b79ff5..0bc186d67b 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -63,6 +63,9 @@ config HAS_IOPORTS
 config HAS_SCHED_GRANULARITY
        bool

+config M2P
+       bool
+
 config NEEDS_LIBELF
        bool

diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index f8ba49b118..f4e1864703 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -326,11 +326,6 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
 #define SHARED_M2P_ENTRY         (~0UL - 1UL)
 #define SHARED_M2P(_e)           ((_e) == SHARED_M2P_ENTRY)

-/* Xen always owns P2M on ARM */
-#define set_gpfn_from_mfn(mfn, pfn) do { (void) (mfn), (void)(pfn); } while (0)
-#define mfn_to_gmfn(_d, mfn)  (mfn)
-
-
 /* Arch-specific portion of memory_op hypercall. */
 long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg);

diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 1061765bcd..8f6858f954 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -685,4 +685,14 @@ static inline void put_page_alloc_ref(struct page_info *page)
     }
 }

+/*
+ * For architectures which don't maintain their own M2P, provide a stub
+ * implementation for common code to use.
+ */
+#ifndef CONFIG_M2P
+static inline void set_gpfn_from_mfn(unsigned long mfn, unsigned long pfn) {}
+static inline unsigned long mfn_to_gmfn(
+    const struct domain *d, unsigned long mfn) { return mfn; }
+#endif

Please don't add this hack in common code. This is a broken implementation and we are lucky it didn't result to a disaster yet.

The correct way to implement it is to remove mfn_to_gmfn from common code.

I would be happy to try to revive my series.

Cheers,



+
 #endif /* __XEN_MM_H__ */
--
2.11.0


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.