On 13.01.2023 06:29, Penny Zheng wrote:
> VMAP in MMU system, is used to remap a range of normal memory
> or device memory to another virtual address with new attributes
> for specific purpose, like ALTERNATIVE feature. Since there is
> no virtual address translation support in MPU system, we can
> not support VMAP in MPU system.
>
> So in this patch, we disable VMAP for MPU systems, and some
> features depending on VMAP also need to be disabled at the same
> time, Like ALTERNATIVE, CPU ERRATA.
>
> Signed-off-by: Penny Zheng <[email protected]>
> Signed-off-by: Wei Chen <[email protected]>
> ---
> xen/arch/arm/Kconfig | 3 +-
> xen/arch/arm/Makefile | 2 +-
> xen/arch/arm/include/asm/alternative.h | 15 +++++
> xen/arch/arm/include/asm/cpuerrata.h | 12 ++++
> xen/arch/arm/setup.c | 7 +++
> xen/arch/x86/Kconfig | 1 +
> xen/common/Kconfig | 3 +
> xen/common/Makefile | 2 +-
> xen/include/xen/vmap.h | 81 ++++++++++++++++++++++++--
> 9 files changed, 119 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index c6b6b612d1..9230c8b885 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -11,12 +11,13 @@ config ARM_64
>
> config ARM
> def_bool y
> - select HAS_ALTERNATIVE
> + select HAS_ALTERNATIVE if !ARM_V8R
Judging from the connection you make in the description, I think this
wants to be "if HAS_VMAP".
> select HAS_DEVICE_TREE
> select HAS_PASSTHROUGH
> select HAS_PDX
> select HAS_PMAP
> select IOMMU_FORCE_PT_SHARE
> + select HAS_VMAP if !ARM_V8R
I think entries here are intended to be sorted alphabetically.
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -28,6 +28,7 @@ config X86
> select HAS_UBSAN
> select HAS_VPCI if HVM
> select NEEDS_LIBELF
> + select HAS_VMAP
Here they are certainly meant to be.
> --- a/xen/include/xen/vmap.h
> +++ b/xen/include/xen/vmap.h
> @@ -1,15 +1,17 @@
> -#if !defined(__XEN_VMAP_H__) && defined(VMAP_VIRT_START)
> +#if !defined(__XEN_VMAP_H__) && (defined(VMAP_VIRT_START) ||
> !defined(CONFIG_HAS_VMAP))
> #define __XEN_VMAP_H__
>
> -#include <xen/mm-frame.h>
> -#include <xen/page-size.h>
> -
> enum vmap_region {
> VMAP_DEFAULT,
> VMAP_XEN,
> VMAP_REGION_NR,
> };
>
> +#ifdef CONFIG_HAS_VMAP
> +
> +#include <xen/mm-frame.h>
> +#include <xen/page-size.h>
> +
> void vm_init_type(enum vmap_region type, void *start, void *end);
>
> void *__vmap(const mfn_t *mfn, unsigned int granularity, unsigned int nr,
> @@ -38,4 +40,75 @@ static inline void vm_init(void)
> vm_init_type(VMAP_DEFAULT, (void *)VMAP_VIRT_START,
> arch_vmap_virt_end());
> }
>
> +#else /* !CONFIG_HAS_VMAP */
> +
> +static inline void vm_init_type(enum vmap_region type, void *start, void
> *end)
> +{
> + ASSERT_UNREACHABLE();
> +}
Do you really need this and all other inline stubs? Imo the goal ought
to be to have as few of them as possible: The one above won't be
referenced if you further make LIVEPATCH depend on HAS_VMAP (which I
think you need to do anyway), and the only other call to the function
is visible in context above (i.e. won't be used either when !HAS_VMAP).
In other cases merely having a declaration (but no definition) may be
sufficient, as the compiler may be able to eliminate calls.
Jan
