Re: [Xen-devel] [PATCH v2 2/2] p2m: split mem_access into separate files

[Stefano Stabellini]

On Fri, 9 Dec 2016, Tamas K Lengyel wrote:
> This patch relocates mem_access components that are currently mixed with p2m
> code into separate files. This better aligns the code with similar subsystems,
> such as mem_sharing and mem_paging, which are already in separate files. There
> are no code-changes introduced, the patch is mechanical code movement.
> 
> On ARM we also relocate the static inline gfn_next_boundary function to p2m.h
> as it is a function the mem_access code needs access to.
> 
> Signed-off-by: Tamas K Lengyel 
> Acked-by: Razvan Cojocaru 

Reviewed-by: Stefano Stabellini 

I'll commit both patches shortly.

> ---
> Cc: Stefano Stabellini 
> Cc: Julien Grall 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: George Dunlap 
> 
> v2: Don't move ARM radix tree functions
> Include asm/mem_accesss.h in xen/mem_access.h
> ---
>  MAINTAINERS  |   2 +
>  xen/arch/arm/Makefile|   1 +
>  xen/arch/arm/mem_access.c| 431 
>  xen/arch/arm/p2m.c   | 414 +--
>  xen/arch/arm/traps.c |   1 +
>  xen/arch/x86/mm/Makefile |   1 +
>  xen/arch/x86/mm/mem_access.c | 462 
> +++
>  xen/arch/x86/mm/p2m.c| 421 ---
>  xen/arch/x86/vm_event.c  |   3 +-
>  xen/common/mem_access.c  |   2 +-
>  xen/include/asm-arm/mem_access.h |  53 +
>  xen/include/asm-arm/p2m.h|  31 ++-
>  xen/include/asm-x86/mem_access.h |  61 ++
>  xen/include/asm-x86/p2m.h|  24 +-
>  xen/include/xen/mem_access.h |  67 +-
>  xen/include/xen/p2m-common.h |  52 -
>  16 files changed, 1089 insertions(+), 937 deletions(-)
>  create mode 100644 xen/arch/arm/mem_access.c
>  create mode 100644 xen/arch/x86/mm/mem_access.c
>  create mode 100644 xen/include/asm-arm/mem_access.h
>  create mode 100644 xen/include/asm-x86/mem_access.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f0d0202..fb26be3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -402,6 +402,8 @@ S:Supported
>  F:   tools/tests/xen-access
>  F:   xen/arch/*/monitor.c
>  F:   xen/arch/*/vm_event.c
> +F:   xen/arch/arm/mem_access.c
> +F:   xen/arch/x86/mm/mem_access.c
>  F:   xen/arch/x86/hvm/monitor.c
>  F:   xen/common/mem_access.c
>  F:   xen/common/monitor.c
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index da39d39..b095e8a 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -24,6 +24,7 @@ obj-y += io.o
>  obj-y += irq.o
>  obj-y += kernel.o
>  obj-$(CONFIG_LIVEPATCH) += livepatch.o
> +obj-y += mem_access.o
>  obj-y += mm.o
>  obj-y += monitor.o
>  obj-y += p2m.o
> diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
> new file mode 100644
> index 000..a6e5bcd
> --- /dev/null
> +++ b/xen/arch/arm/mem_access.c
> @@ -0,0 +1,431 @@
> +/*
> + * arch/arm/mem_access.c
> + *
> + * Architecture-specific mem_access handling routines
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public
> + * License v2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; If not, see 
> .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static int __p2m_get_mem_access(struct domain *d, gfn_t gfn,
> +xenmem_access_t *access)
> +{
> +struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +void *i;
> +unsigned int index;
> +
> +static const xenmem_access_t memaccess[] = {
> +#define ACCESS(ac) [p2m_access_##ac] = XENMEM_access_##ac
> +ACCESS(n),
> +ACCESS(r),
> +ACCESS(w),
> +ACCESS(rw),
> +ACCESS(x),
> +ACCESS(rx),
> +ACCESS(wx),
> +ACCESS(rwx),
> +ACCESS(rx2rw),
> +ACCESS(n2rwx),
> +#undef ACCESS
> +};
> +
> +ASSERT(p2m_is_locked(p2m));
> +
> +/* If no setting was ever set, just return rwx. */
> +if ( !p2m->mem_access_enabled )
> +{
> +*access = XENMEM_access_rwx;
> +return 0;
> +}
> +
> +/* If request to get default access. */
> +if ( gfn_eq(gfn, INVALID_GFN) )
> +{
> +*access = memaccess[p2m->default_access];
> +return 0;
> +

Re: [Xen-devel] [PATCH v2 2/2] p2m: split mem_access into separate files

[Konrad Rzeszutek Wilk]

On Tue, Jan 03, 2017 at 08:31:25AM -0700, Tamas K Lengyel wrote:
> On Fri, Dec 9, 2016 at 12:59 PM, Tamas K Lengyel
>  wrote:
> > This patch relocates mem_access components that are currently mixed with p2m
> > code into separate files. This better aligns the code with similar 
> > subsystems,
> > such as mem_sharing and mem_paging, which are already in separate files. 
> > There
> > are no code-changes introduced, the patch is mechanical code movement.
> >
> > On ARM we also relocate the static inline gfn_next_boundary function to 
> > p2m.h
> > as it is a function the mem_access code needs access to.
> >
> > Signed-off-by: Tamas K Lengyel 
> > Acked-by: Razvan Cojocaru 
> 
> Acked-by: George Dunlap 

Acked-by: Konrad Rzeszutek Wilk 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 2/2] p2m: split mem_access into separate files

[Tamas K Lengyel]

On Fri, Dec 9, 2016 at 12:59 PM, Tamas K Lengyel
 wrote:
> This patch relocates mem_access components that are currently mixed with p2m
> code into separate files. This better aligns the code with similar subsystems,
> such as mem_sharing and mem_paging, which are already in separate files. There
> are no code-changes introduced, the patch is mechanical code movement.
>
> On ARM we also relocate the static inline gfn_next_boundary function to p2m.h
> as it is a function the mem_access code needs access to.
>
> Signed-off-by: Tamas K Lengyel 
> Acked-by: Razvan Cojocaru 

Acked-by: George Dunlap 

> ---
> Cc: Stefano Stabellini 
> Cc: Julien Grall 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: George Dunlap 
>
> v2: Don't move ARM radix tree functions
> Include asm/mem_accesss.h in xen/mem_access.h

Patch ping. I think this only needs an ARM-side ack.

> ---
>  MAINTAINERS  |   2 +
>  xen/arch/arm/Makefile|   1 +
>  xen/arch/arm/mem_access.c| 431 
>  xen/arch/arm/p2m.c   | 414 +--
>  xen/arch/arm/traps.c |   1 +
>  xen/arch/x86/mm/Makefile |   1 +
>  xen/arch/x86/mm/mem_access.c | 462 
> +++
>  xen/arch/x86/mm/p2m.c| 421 ---
>  xen/arch/x86/vm_event.c  |   3 +-
>  xen/common/mem_access.c  |   2 +-
>  xen/include/asm-arm/mem_access.h |  53 +
>  xen/include/asm-arm/p2m.h|  31 ++-
>  xen/include/asm-x86/mem_access.h |  61 ++
>  xen/include/asm-x86/p2m.h|  24 +-
>  xen/include/xen/mem_access.h |  67 +-
>  xen/include/xen/p2m-common.h |  52 -
>  16 files changed, 1089 insertions(+), 937 deletions(-)
>  create mode 100644 xen/arch/arm/mem_access.c
>  create mode 100644 xen/arch/x86/mm/mem_access.c
>  create mode 100644 xen/include/asm-arm/mem_access.h
>  create mode 100644 xen/include/asm-x86/mem_access.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f0d0202..fb26be3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -402,6 +402,8 @@ S:  Supported
>  F: tools/tests/xen-access
>  F: xen/arch/*/monitor.c
>  F: xen/arch/*/vm_event.c
> +F: xen/arch/arm/mem_access.c
> +F: xen/arch/x86/mm/mem_access.c
>  F: xen/arch/x86/hvm/monitor.c
>  F: xen/common/mem_access.c
>  F: xen/common/monitor.c
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index da39d39..b095e8a 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -24,6 +24,7 @@ obj-y += io.o
>  obj-y += irq.o
>  obj-y += kernel.o
>  obj-$(CONFIG_LIVEPATCH) += livepatch.o
> +obj-y += mem_access.o
>  obj-y += mm.o
>  obj-y += monitor.o
>  obj-y += p2m.o
> diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
> new file mode 100644
> index 000..a6e5bcd
> --- /dev/null
> +++ b/xen/arch/arm/mem_access.c
> @@ -0,0 +1,431 @@
> +/*
> + * arch/arm/mem_access.c
> + *
> + * Architecture-specific mem_access handling routines
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public
> + * License v2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; If not, see 
> .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static int __p2m_get_mem_access(struct domain *d, gfn_t gfn,
> +xenmem_access_t *access)
> +{
> +struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +void *i;
> +unsigned int index;
> +
> +static const xenmem_access_t memaccess[] = {
> +#define ACCESS(ac) [p2m_access_##ac] = XENMEM_access_##ac
> +ACCESS(n),
> +ACCESS(r),
> +ACCESS(w),
> +ACCESS(rw),
> +ACCESS(x),
> +ACCESS(rx),
> +ACCESS(wx),
> +ACCESS(rwx),
> +ACCESS(rx2rw),
> +ACCESS(n2rwx),
> +#undef ACCESS
> +};
> +
> +ASSERT(p2m_is_locked(p2m));
> +
> +/* If no setting was ever set, just return rwx. */
> +if ( !p2m->mem_access_enabled )
> +{
> +*access = XENMEM_access_rwx;
> +return 0;
> +}
> +
> +/* If request to get default access. */
> +if ( gfn_eq(gfn, INVALID_GFN) )
> +{
> +*access 

[Xen-devel] [PATCH v2 2/2] p2m: split mem_access into separate files

[Tamas K Lengyel]

This patch relocates mem_access components that are currently mixed with p2m
code into separate files. This better aligns the code with similar subsystems,
such as mem_sharing and mem_paging, which are already in separate files. There
are no code-changes introduced, the patch is mechanical code movement.

On ARM we also relocate the static inline gfn_next_boundary function to p2m.h
as it is a function the mem_access code needs access to.

Signed-off-by: Tamas K Lengyel 
Acked-by: Razvan Cojocaru 
---
Cc: Stefano Stabellini 
Cc: Julien Grall 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: George Dunlap 

v2: Don't move ARM radix tree functions
Include asm/mem_accesss.h in xen/mem_access.h
---
 MAINTAINERS  |   2 +
 xen/arch/arm/Makefile|   1 +
 xen/arch/arm/mem_access.c| 431 
 xen/arch/arm/p2m.c   | 414 +--
 xen/arch/arm/traps.c |   1 +
 xen/arch/x86/mm/Makefile |   1 +
 xen/arch/x86/mm/mem_access.c | 462 +++
 xen/arch/x86/mm/p2m.c| 421 ---
 xen/arch/x86/vm_event.c  |   3 +-
 xen/common/mem_access.c  |   2 +-
 xen/include/asm-arm/mem_access.h |  53 +
 xen/include/asm-arm/p2m.h|  31 ++-
 xen/include/asm-x86/mem_access.h |  61 ++
 xen/include/asm-x86/p2m.h|  24 +-
 xen/include/xen/mem_access.h |  67 +-
 xen/include/xen/p2m-common.h |  52 -
 16 files changed, 1089 insertions(+), 937 deletions(-)
 create mode 100644 xen/arch/arm/mem_access.c
 create mode 100644 xen/arch/x86/mm/mem_access.c
 create mode 100644 xen/include/asm-arm/mem_access.h
 create mode 100644 xen/include/asm-x86/mem_access.h

diff --git a/MAINTAINERS b/MAINTAINERS
index f0d0202..fb26be3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -402,6 +402,8 @@ S:  Supported
 F: tools/tests/xen-access
 F: xen/arch/*/monitor.c
 F: xen/arch/*/vm_event.c
+F: xen/arch/arm/mem_access.c
+F: xen/arch/x86/mm/mem_access.c
 F: xen/arch/x86/hvm/monitor.c
 F: xen/common/mem_access.c
 F: xen/common/monitor.c
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index da39d39..b095e8a 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -24,6 +24,7 @@ obj-y += io.o
 obj-y += irq.o
 obj-y += kernel.o
 obj-$(CONFIG_LIVEPATCH) += livepatch.o
+obj-y += mem_access.o
 obj-y += mm.o
 obj-y += monitor.o
 obj-y += p2m.o
diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
new file mode 100644
index 000..a6e5bcd
--- /dev/null
+++ b/xen/arch/arm/mem_access.c
@@ -0,0 +1,431 @@
+/*
+ * arch/arm/mem_access.c
+ *
+ * Architecture-specific mem_access handling routines
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static int __p2m_get_mem_access(struct domain *d, gfn_t gfn,
+xenmem_access_t *access)
+{
+struct p2m_domain *p2m = p2m_get_hostp2m(d);
+void *i;
+unsigned int index;
+
+static const xenmem_access_t memaccess[] = {
+#define ACCESS(ac) [p2m_access_##ac] = XENMEM_access_##ac
+ACCESS(n),
+ACCESS(r),
+ACCESS(w),
+ACCESS(rw),
+ACCESS(x),
+ACCESS(rx),
+ACCESS(wx),
+ACCESS(rwx),
+ACCESS(rx2rw),
+ACCESS(n2rwx),
+#undef ACCESS
+};
+
+ASSERT(p2m_is_locked(p2m));
+
+/* If no setting was ever set, just return rwx. */
+if ( !p2m->mem_access_enabled )
+{
+*access = XENMEM_access_rwx;
+return 0;
+}
+
+/* If request to get default access. */
+if ( gfn_eq(gfn, INVALID_GFN) )
+{
+*access = memaccess[p2m->default_access];
+return 0;
+}
+
+i = radix_tree_lookup(>mem_access_settings, gfn_x(gfn));
+
+if ( !i )
+{
+/*
+ * No setting was found in the Radix tree. Check if the
+ * entry exists in the page-tables.
+ */
+mfn_t mfn = p2m_get_entry(p2m, gfn, NULL, NULL, NULL);
+
+if ( mfn_eq(mfn, INVALID_MFN) )
+return -ESRCH;
+
+/* If entry exists then its rwx.