When HAS_VMAP is disabled, the xv{malloc,zalloc,...} functions
should fall back to the simple x{malloc,zalloc,...} variant,
implement that because MPU systems won't have virtual memory.
Additionally remove VMAP_VIRT_START from vmap.h guards since
MPU systems won't have it defined.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
Changes from v2:
- Don't protect declarations.
Changes from v1:
- put back static inline iounmap
- changed commit message
- hide not used declaration for system with !HAS_VMAP
- correct function declared in xvmalloc.h to be static inline
- prefer '#ifdef' instead of '#if defined' where possible
---
---
xen/include/xen/vmap.h | 2 +-
xen/include/xen/xvmalloc.h | 36 +++++++++++++++++++++++++++++++-----
2 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/xen/include/xen/vmap.h b/xen/include/xen/vmap.h
index c1dd7ac22f30..26c831757a11 100644
--- a/xen/include/xen/vmap.h
+++ b/xen/include/xen/vmap.h
@@ -5,7 +5,7 @@
* purpose area (VMAP_DEFAULT) and a livepatch-specific area (VMAP_XEN). The
* latter is used when loading livepatches and the former for everything else.
*/
-#if !defined(__XEN_VMAP_H__) && defined(VMAP_VIRT_START)
+#ifndef __XEN_VMAP_H__
#define __XEN_VMAP_H__
#include <xen/mm-frame.h>
diff --git a/xen/include/xen/xvmalloc.h b/xen/include/xen/xvmalloc.h
index 440d85a284bb..e97a30f61e96 100644
--- a/xen/include/xen/xvmalloc.h
+++ b/xen/include/xen/xvmalloc.h
@@ -40,20 +40,46 @@
((typeof(ptr))_xvrealloc(ptr, offsetof(typeof(*(ptr)), field[nr]), \
__alignof__(typeof(*(ptr)))))
+#ifdef CONFIG_HAS_VMAP
+
/* Free any of the above. */
void xvfree(void *va);
+/* Underlying functions */
+void *_xvmalloc(size_t size, unsigned int align);
+void *_xvzalloc(size_t size, unsigned int align);
+void *_xvrealloc(void *va, size_t size, unsigned int align);
+
+#else /* !CONFIG_HAS_VMAP */
+
+static inline void xvfree(void *va)
+{
+ xfree(va);
+}
+
+static inline void *_xvmalloc(size_t size, unsigned int align)
+{
+ return _xmalloc(size, align);
+}
+
+static inline void *_xvzalloc(size_t size, unsigned int align)
+{
+ return _xzalloc(size, align);
+}
+
+static inline void *_xvrealloc(void *va, size_t size, unsigned int align)
+{
+ return _xrealloc(va, size, align);
+}
+
+#endif /* CONFIG_HAS_VMAP */
+
/* Free an allocation, and zero the pointer to it. */
#define XVFREE(p) do { \
xvfree(p); \
(p) = NULL; \
} while ( false )
-/* Underlying functions */
-void *_xvmalloc(size_t size, unsigned int align);
-void *_xvzalloc(size_t size, unsigned int align);
-void *_xvrealloc(void *va, size_t size, unsigned int align);
-
static inline void *_xvmalloc_array(
size_t size, unsigned int align, unsigned long num)
{
--
2.34.1On 29.11.2024 10:12, Luca Fancellu wrote:
> --- a/xen/include/xen/xvmalloc.h
> +++ b/xen/include/xen/xvmalloc.h
> @@ -40,20 +40,46 @@
> ((typeof(ptr))_xvrealloc(ptr, offsetof(typeof(*(ptr)), field[nr]), \
> __alignof__(typeof(*(ptr)))))
>
> +#ifdef CONFIG_HAS_VMAP
> +
> /* Free any of the above. */
> void xvfree(void *va);
>
> +/* Underlying functions */
> +void *_xvmalloc(size_t size, unsigned int align);
> +void *_xvzalloc(size_t size, unsigned int align);
> +void *_xvrealloc(void *va, size_t size, unsigned int align);
> +
> +#else /* !CONFIG_HAS_VMAP */
> +
> +static inline void xvfree(void *va)
> +{
> + xfree(va);
> +}
> +
> +static inline void *_xvmalloc(size_t size, unsigned int align)
> +{
> + return _xmalloc(size, align);
> +}
> +
> +static inline void *_xvzalloc(size_t size, unsigned int align)
> +{
> + return _xzalloc(size, align);
> +}
> +
> +static inline void *_xvrealloc(void *va, size_t size, unsigned int align)
> +{
> + return _xrealloc(va, size, align);
> +}
Just to double check: Was it at least considered to use simple #define-s
to effect the aliasing? Wrapper functions like the above ones have the
downside of needing touching (easy to miss) when the wrapped function
types change in whichever minor way. (And yes, I do understand that we
generally aim at using inline functions in preference to macros.)
Jan
Hi Jan,
> On 29 Nov 2024, at 11:06, Jan Beulich <jbeulich@suse.com> wrote:
>
> On 29.11.2024 10:12, Luca Fancellu wrote:
>> --- a/xen/include/xen/xvmalloc.h
>> +++ b/xen/include/xen/xvmalloc.h
>> @@ -40,20 +40,46 @@
>> ((typeof(ptr))_xvrealloc(ptr, offsetof(typeof(*(ptr)), field[nr]), \
>> __alignof__(typeof(*(ptr)))))
>>
>> +#ifdef CONFIG_HAS_VMAP
>> +
>> /* Free any of the above. */
>> void xvfree(void *va);
>>
>> +/* Underlying functions */
>> +void *_xvmalloc(size_t size, unsigned int align);
>> +void *_xvzalloc(size_t size, unsigned int align);
>> +void *_xvrealloc(void *va, size_t size, unsigned int align);
>> +
>> +#else /* !CONFIG_HAS_VMAP */
>> +
>> +static inline void xvfree(void *va)
>> +{
>> + xfree(va);
>> +}
>> +
>> +static inline void *_xvmalloc(size_t size, unsigned int align)
>> +{
>> + return _xmalloc(size, align);
>> +}
>> +
>> +static inline void *_xvzalloc(size_t size, unsigned int align)
>> +{
>> + return _xzalloc(size, align);
>> +}
>> +
>> +static inline void *_xvrealloc(void *va, size_t size, unsigned int align)
>> +{
>> + return _xrealloc(va, size, align);
>> +}
>
> Just to double check: Was it at least considered to use simple #define-s
> to effect the aliasing? Wrapper functions like the above ones have the
> downside of needing touching (easy to miss) when the wrapped function
> types change in whichever minor way. (And yes, I do understand that we
> generally aim at using inline functions in preference to macros.)
Yes, I think I tried and I didn’t have issues using #define-s, I asked here
https://patchwork.kernel.org/project/xen-devel/patch/20241115105036.218418-2-luca.fancellu@arm.com/#26123448
about a preferred approach, but I didn’t have any reply, so I went for what
I believed was preferred as you said, static inline-s instead of macros.
Cheers,
Luca
On 29.11.2024 12:14, Luca Fancellu wrote:
> Hi Jan,
>
>> On 29 Nov 2024, at 11:06, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 29.11.2024 10:12, Luca Fancellu wrote:
>>> --- a/xen/include/xen/xvmalloc.h
>>> +++ b/xen/include/xen/xvmalloc.h
>>> @@ -40,20 +40,46 @@
>>> ((typeof(ptr))_xvrealloc(ptr, offsetof(typeof(*(ptr)), field[nr]), \
>>> __alignof__(typeof(*(ptr)))))
>>>
>>> +#ifdef CONFIG_HAS_VMAP
>>> +
>>> /* Free any of the above. */
>>> void xvfree(void *va);
>>>
>>> +/* Underlying functions */
>>> +void *_xvmalloc(size_t size, unsigned int align);
>>> +void *_xvzalloc(size_t size, unsigned int align);
>>> +void *_xvrealloc(void *va, size_t size, unsigned int align);
>>> +
>>> +#else /* !CONFIG_HAS_VMAP */
>>> +
>>> +static inline void xvfree(void *va)
>>> +{
>>> + xfree(va);
>>> +}
>>> +
>>> +static inline void *_xvmalloc(size_t size, unsigned int align)
>>> +{
>>> + return _xmalloc(size, align);
>>> +}
>>> +
>>> +static inline void *_xvzalloc(size_t size, unsigned int align)
>>> +{
>>> + return _xzalloc(size, align);
>>> +}
>>> +
>>> +static inline void *_xvrealloc(void *va, size_t size, unsigned int align)
>>> +{
>>> + return _xrealloc(va, size, align);
>>> +}
>>
>> Just to double check: Was it at least considered to use simple #define-s
>> to effect the aliasing? Wrapper functions like the above ones have the
>> downside of needing touching (easy to miss) when the wrapped function
>> types change in whichever minor way. (And yes, I do understand that we
>> generally aim at using inline functions in preference to macros.)
>
> Yes, I think I tried and I didn’t have issues using #define-s, I asked here
> https://patchwork.kernel.org/project/xen-devel/patch/20241115105036.218418-2-luca.fancellu@arm.com/#26123448
> about a preferred approach, but I didn’t have any reply, so I went for what
> I believed was preferred as you said, static inline-s instead of macros.
As Andrew's idea didn't work out, personally I think the simple #define
approach you suggested would be preferable in this case. There is in
particular no type-safety concern here, as the wrapped functions will
all have the intended type checking applied.
Jan
>>> >>> Just to double check: Was it at least considered to use simple #define-s >>> to effect the aliasing? Wrapper functions like the above ones have the >>> downside of needing touching (easy to miss) when the wrapped function >>> types change in whichever minor way. (And yes, I do understand that we >>> generally aim at using inline functions in preference to macros.) >> >> Yes, I think I tried and I didn’t have issues using #define-s, I asked here >> https://patchwork.kernel.org/project/xen-devel/patch/20241115105036.218418-2-luca.fancellu@arm.com/#26123448 >> about a preferred approach, but I didn’t have any reply, so I went for what >> I believed was preferred as you said, static inline-s instead of macros. > > As Andrew's idea didn't work out, personally I think the simple #define > approach you suggested would be preferable in this case. There is in > particular no type-safety concern here, as the wrapped functions will > all have the intended type checking applied. ok, I’ll re-spin this one with #defines. > > Jan
© 2016 - 2024 Red Hat, Inc.