Re: [Xen-devel] [PATCH v5 04/10] Make MEM_ACCESS configurable

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 04/06/18 18:24, Stefano Stabellini wrote:
> Select MEM_ACCESS_ALWAYS_ON on x86 to mark that MEM_ACCESS is not
> configurable on x86. Avoid selecting it on ARM.
> Rename HAS_MEM_ACCESS to MEM_ACCESS everywhere. Add a prompt and a
> description to MEM_ACCESS in xen/common/Kconfig.
>
> The result is that the user-visible option is MEM_ACCESS, and it is
> configurable only on ARM (disabled by default).
It would be nice to mention in the commit message the shortcoming for 
Arm. Because you are just removing the guest interface, all the 
arch-specific infrastructure is still present.
> The purpose is to reduce code size. The option doesn't depend on EXPERT
> because it would be nice to ecurity-support configurations without
s/ecurity-support/security-support/

> MEM_ACCESS and a non-expert should be able to disable it.
>
> Suggested-by: Julien Grall <julien.grall@xxxxxxx>
> Signed-off-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> CC: andrew.cooper3@xxxxxxxxxx
> CC: George.Dunlap@xxxxxxxxxxxxx
> CC: ian.jackson@xxxxxxxxxxxxx
> CC: jbeulich@xxxxxxxx
> CC: julien.grall@xxxxxxx
> CC: konrad.wilk@xxxxxxxxxx
> CC: sstabellini@xxxxxxxxxx
> CC: tim@xxxxxxx
> CC: wei.liu2@xxxxxxxxxx
>
> ---
> Changes in v5:
> - change MEM_ACCESS_ALWAYS_ON to bool
> - change default for MEM_ACCESS, default y if MEM_ACCESS_ALWAYS_ON
>
> Changes in v4:
> - remove HAS_MEM_ACCESS
> - move MEM_ACCESS_ALWAYS_ON to common
> - combile default and bool to def_bool
>
> Changes in v3:
> - keep HAS_MEM_ACCESS to mark that an arch can do MEM_ACCESS
> - introduce MEM_ACCESS_ALWAYS_ON
> - the main MEM_ACCESS option is in xen/common/Kconfig
>
> Changes in v2:
> - patch added
> ---
>   tools/firmware/xen-dir/shim.config |  2 +-
>   xen/arch/arm/Kconfig               |  1 -
>   xen/arch/x86/Kconfig               |  2 +-
>   xen/common/Kconfig                 | 10 +++++++++-
>   xen/common/Makefile                |  2 +-
>   xen/common/domctl.c                |  2 +-
>   xen/include/xen/mem_access.h       |  4 ++--
>   xen/include/xsm/dummy.h            |  2 +-
>   xen/include/xsm/xsm.h              |  4 ++--
>   xen/xsm/dummy.c                    |  2 +-
>   xen/xsm/flask/hooks.c              |  4 ++--
You probably want an ack from Daniel here (CCed him).

>   11 files changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/tools/firmware/xen-dir/shim.config 
> b/tools/firmware/xen-dir/shim.config
> index 4d5630f..21d7075 100644
> --- a/tools/firmware/xen-dir/shim.config
> +++ b/tools/firmware/xen-dir/shim.config
> @@ -29,7 +29,7 @@ CONFIG_COMPAT=y
>   CONFIG_CORE_PARKING=y
>   CONFIG_HAS_ALTERNATIVE=y
>   CONFIG_HAS_EX_TABLE=y
> -CONFIG_HAS_MEM_ACCESS=y
> +CONFIG_MEM_ACCESS=y
>   CONFIG_HAS_MEM_PAGING=y
>   CONFIG_HAS_MEM_SHARING=y
>   CONFIG_HAS_PDX=y
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 66adce4..2b87111 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -17,7 +17,6 @@ config ARM
>         def_bool y
>         select HAS_ALTERNATIVE
>         select HAS_DEVICE_TREE
> -       select HAS_MEM_ACCESS
>         select HAS_PASSTHROUGH
>         select HAS_PDX
>   
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index f64fc56..9a85fe9 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -15,7 +15,7 @@ config X86
>         select HAS_GDBSX
>         select HAS_IOPORTS
>         select HAS_KEXEC
> -       select HAS_MEM_ACCESS
> +       select MEM_ACCESS_ALWAYS_ON
>         select HAS_MEM_PAGING
>         select HAS_MEM_SHARING
>         select HAS_NS16550
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index 9043dce..db6bb2d 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -20,9 +20,17 @@ config HAS_DEVICE_TREE
>   config HAS_EX_TABLE
>         bool
>   
> -config HAS_MEM_ACCESS
> +config MEM_ACCESS_ALWAYS_ON
>         bool
>   
> +config MEM_ACCESS
> +       def_bool MEM_ACCESS_ALWAYS_ON
> +       prompt "Memory Access and VM events" if !MEM_ACCESS_ALWAYS_ON
> +       ---help---
> +
> +         Framework to configure memory access types for guests and receive
> +         related events in userspace.
> +
>   config HAS_MEM_PAGING
>         bool
>   
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index 24d4752..6f2b3fc 100644
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -22,7 +22,7 @@ obj-y += lib.o
>   obj-$(CONFIG_NEEDS_LIST_SORT) += list_sort.o
>   obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o
>   obj-y += lzo.o
> -obj-$(CONFIG_HAS_MEM_ACCESS) += mem_access.o
> +obj-$(CONFIG_MEM_ACCESS) += mem_access.o
>   obj-y += memory.o
>   obj-y += monitor.o
>   obj-y += multicall.o
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index 9b7bc08..891ad58 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -1085,7 +1085,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> u_domctl)
>           copyback = 1;
>           break;
>   
> -#ifdef CONFIG_HAS_MEM_ACCESS
> +#ifdef CONFIG_MEM_ACCESS
>       case XEN_DOMCTL_set_access_required:
>           if ( unlikely(current->domain == d) ) /* no domain_pause() */
>               ret = -EPERM;
> diff --git a/xen/include/xen/mem_access.h b/xen/include/xen/mem_access.h
> index 5ab34c1..7e95eab 100644
> --- a/xen/include/xen/mem_access.h
> +++ b/xen/include/xen/mem_access.h
> @@ -78,7 +78,7 @@ long p2m_set_mem_access_multi(struct domain *d,
>    */
>   int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t *access);
>   
> -#ifdef CONFIG_HAS_MEM_ACCESS
> +#ifdef CONFIG_MEM_ACCESS
>   int mem_access_memop(unsigned long cmd,
>                        XEN_GUEST_HANDLE_PARAM(xen_mem_access_op_t) arg);
>   #else
> @@ -88,7 +88,7 @@ int mem_access_memop(unsigned long cmd,
>   {
>       return -ENOSYS;
>   }
> -#endif /* CONFIG_HAS_MEM_ACCESS */
> +#endif /* CONFIG_MEM_ACCESS */
>   
>   #endif /* _XEN_MEM_ACCESS_H */
>   
> diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
> index ff6b2db..b0ac1f6 100644
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -584,7 +584,7 @@ static XSM_INLINE int xsm_vm_event_control(XSM_DEFAULT_ARG 
> struct domain *d, int
>       return xsm_default_action(action, current->domain, d);
>   }
>   
> -#ifdef CONFIG_HAS_MEM_ACCESS
> +#ifdef CONFIG_MEM_ACCESS
>   static XSM_INLINE int xsm_mem_access(XSM_DEFAULT_ARG struct domain *d)
>   {
>       XSM_ASSERT_ACTION(XSM_DM_PRIV);
> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
> index f0c6fc7..7636bcb 100644
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -143,7 +143,7 @@ struct xsm_operations {
>   
>       int (*vm_event_control) (struct domain *d, int mode, int op);
>   
> -#ifdef CONFIG_HAS_MEM_ACCESS
> +#ifdef CONFIG_MEM_ACCESS
>       int (*mem_access) (struct domain *d);
>   #endif
>   
> @@ -582,7 +582,7 @@ static inline int xsm_vm_event_control (xsm_default_t def, struct domain *d, int
>       return xsm_ops->vm_event_control(d, mode, op);
>   }
>   
> -#ifdef CONFIG_HAS_MEM_ACCESS
> +#ifdef CONFIG_MEM_ACCESS
>   static inline int xsm_mem_access (xsm_default_t def, struct domain *d)
>   {
>       return xsm_ops->mem_access(d);
> diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
> index 6e75119..3290d04 100644
> --- a/xen/xsm/dummy.c
> +++ b/xen/xsm/dummy.c
> @@ -127,7 +127,7 @@ void __init xsm_fixup_ops (struct xsm_operations *ops)
>   
>       set_to_dummy_if_null(ops, vm_event_control);
>   
> -#ifdef CONFIG_HAS_MEM_ACCESS
> +#ifdef CONFIG_MEM_ACCESS
>       set_to_dummy_if_null(ops, mem_access);
>   #endif
>   
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index 78bc326..7a3ccfa 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1256,7 +1256,7 @@ static int flask_vm_event_control(struct domain *d, int 
> mode, int op)
>       return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__VM_EVENT);
>   }
>   
> -#ifdef CONFIG_HAS_MEM_ACCESS
> +#ifdef CONFIG_MEM_ACCESS
>   static int flask_mem_access(struct domain *d)
>   {
>       return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__MEM_ACCESS);
> @@ -1803,7 +1803,7 @@ static struct xsm_operations flask_ops = {
>   
>       .vm_event_control = flask_vm_event_control,
>   
> -#ifdef CONFIG_HAS_MEM_ACCESS
> +#ifdef CONFIG_MEM_ACCESS
>       .mem_access = flask_mem_access,
>   #endif
>   
--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel