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

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



On Tue, 5 Jun 2018, Julien Grall wrote:
> 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.

I'll do

> > 
> > 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).

I'll CC Daniel to this patch going forward


> >   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

 


Rackspace

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