diff mbox series

[Xen-devel,v3,14/14] xen/mm: Provide dummy M2P-related helpers when !CONFIG_HAVE_M2P

Message ID 20190603160350.29806-15-julien.grall@arm.com
State New
Headers show
Series xen/arm: Properly disable M2P on Arm | expand

Commit Message

Julien Grall June 3, 2019, 4:03 p.m. UTC 
At the moment, Arm is providing a dummy implementation for the M2P
helpers used in common code. However, they are quite isolated and could
be used by other architecture in the future. So move all the helpers in
xen/mm.h.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: George Dunlap <george.dunlap@citrix.com>

---
    Changes in v3:
        - Add Stefano's reviewed-by
        - Add George's acked-by

    Changes in v2:
        - Patch added
---
 xen/include/asm-arm/mm.h | 11 -----------
 xen/include/xen/mm.h     | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 11 deletions(-)

Comments

Jan Beulich June 5, 2019, 11:07 a.m. UTC | #1 
>>> On 03.06.19 at 18:03, <julien.grall@arm.com> wrote:
> At the moment, Arm is providing a dummy implementation for the M2P
> helpers used in common code. However, they are quite isolated and could
> be used by other architecture in the future. So move all the helpers in
> xen/mm.h.

And where's the problem then adding ...

> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -655,4 +655,18 @@ static inline void share_xen_page_with_privileged_guests(
>      share_xen_page_with_guest(page, dom_xen, flags);
>  }
>  
> +/*
> + * Dummy implementation of M2P-related helpers for common code when
> + * the architecture doesn't have an M2P.
> + */
> +#ifndef CONFIG_HAS_M2P
> +
> +#define INVALID_M2P_ENTRY        (~0UL)
> +#define SHARED_M2P_ENTRY         (~0UL - 1UL)
> +#define SHARED_M2P(_e)           ((_e) == SHARED_M2P_ENTRY)
> +
> +static inline void set_pfn_from_mfn(mfn_t mfn, unsigned long pfn) {}
> +
> +#endif

... mfn_to_gmfn() to this set? Perhaps a declaration without any
definition anywhere?

Also please take the opportunity and drop the unnecessary underscore
from _e. And actually, shouldn't this uniformly return false? At which
point you don't even need SHARED_M2P_ENTRY.

Jan
Julien Grall June 5, 2019, 11:17 a.m. UTC | #2 
Hi,

On 05/06/2019 12:07, Jan Beulich wrote:
>>>> On 03.06.19 at 18:03, <julien.grall@arm.com> wrote:
>> At the moment, Arm is providing a dummy implementation for the M2P
>> helpers used in common code. However, they are quite isolated and could
>> be used by other architecture in the future. So move all the helpers in
>> xen/mm.h.
> 
> And where's the problem then adding ...
> 
>> --- a/xen/include/xen/mm.h
>> +++ b/xen/include/xen/mm.h
>> @@ -655,4 +655,18 @@ static inline void share_xen_page_with_privileged_guests(
>>       share_xen_page_with_guest(page, dom_xen, flags);
>>   }
>>   
>> +/*
>> + * Dummy implementation of M2P-related helpers for common code when
>> + * the architecture doesn't have an M2P.
>> + */
>> +#ifndef CONFIG_HAS_M2P
>> +
>> +#define INVALID_M2P_ENTRY        (~0UL)
>> +#define SHARED_M2P_ENTRY         (~0UL - 1UL)
>> +#define SHARED_M2P(_e)           ((_e) == SHARED_M2P_ENTRY)
>> +
>> +static inline void set_pfn_from_mfn(mfn_t mfn, unsigned long pfn) {}
>> +
>> +#endif
> 
> ... mfn_to_gmfn() to this set? Perhaps a declaration without any
> definition anywhere?

The only purpose for this would be to make some code compile and expect the 
compiler to remove it. So I am not in favor of defining mfn_to_gmfn() in any 
form for Arm.

> 
> Also please take the opportunity and drop the unnecessary underscore
> from _e. And actually, shouldn't this uniformly return false? At which
> point you don't even need SHARED_M2P_ENTRY.

I thought about it. But I wasn't sure if there would be any issue with that 
implementation. I am happy to implement SHARED_M2P this way.

Cheers,
diff mbox series

Patch

diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 3c03be3bca..d68d1794e5 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -313,17 +313,6 @@  static inline void *page_to_virt(const struct page_info *pg)
 struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
                                     unsigned long flags);
 
-/*
- * Arm does not have an M2P, but common code expects a handful of
- * M2P-related defines and functions. Provide dummy versions of these.
- */
-#define INVALID_M2P_ENTRY        (~0UL)
-#define SHARED_M2P_ENTRY         (~0UL - 1UL)
-#define SHARED_M2P(_e)           ((_e) == SHARED_M2P_ENTRY)
-
-/* We don't have a M2P on Arm */
-static inline void set_pfn_from_mfn(mfn_t mfn, unsigned long pfn) {}
-
 /* 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 196ce9fcda..477d24d52f 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -655,4 +655,18 @@  static inline void share_xen_page_with_privileged_guests(
     share_xen_page_with_guest(page, dom_xen, flags);
 }
 
+/*
+ * Dummy implementation of M2P-related helpers for common code when
+ * the architecture doesn't have an M2P.
+ */
+#ifndef CONFIG_HAS_M2P
+
+#define INVALID_M2P_ENTRY        (~0UL)
+#define SHARED_M2P_ENTRY         (~0UL - 1UL)
+#define SHARED_M2P(_e)           ((_e) == SHARED_M2P_ENTRY)
+
+static inline void set_pfn_from_mfn(mfn_t mfn, unsigned long pfn) {}
+
+#endif
+
 #endif /* __XEN_MM_H__ */