[PATCH 0/6] xentrace/xenalyze support on ARM

[PATCH 0/6] xentrace/xenalyze support on ARM

[Benjamin Sanda]

From: bensanda <ben.sanda@dornerworks.com>

This patch sets adds support for xentrace and xenalyze to the ARM platform.
The Xen heap memory mapping, timestamping, and P2M translation needed by
xentrace is corrected for operation on the ARM platform using the x86 
platform as reference. Trace buffer initialization is added to setup.c, 
and Flask permissions are modified to allow write mapping of the Xen heap. 
Finally the xenaylze makefile is configured to build the tool for ARM.

bensanda (6):
  Flask: Support for ARM xentrace
  xenalyze: Support for ARM platform
  xentrace: P2M lookup suport for ARM platform
  xentrace: ARM platform DOMID_XEN mapping support
  xentrace: Trace buffer support for ARM platform
  xentrace: ARM platform timestamp support

 tools/flask/policy/policy/modules/xen/xen.te |  2 +
 tools/xentrace/Makefile                      |  3 +-
 xen/arch/arm/mm.c                            | 56 +++++++++++++++++++++++++++-
 xen/arch/arm/p2m.c                           | 19 ++++++++--
 xen/arch/arm/setup.c                         |  3 ++
 xen/include/asm-arm/time.h                   | 13 ++++---
 6 files changed, 83 insertions(+), 13 deletions(-)

-- 
2.7.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 1/6] Flask: Support for ARM xentrace

[Benjamin Sanda]

From: bensanda <ben.sanda@dornerworks.com>

Modified to provide support for xentrace on the ARM platform. Added flask credential to allow dom0 dom_xen mapping and write access for trace buffers.

Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
---
 tools/flask/policy/policy/modules/xen/xen.te | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/flask/policy/policy/modules/xen/xen.te b/tools/flask/policy/policy/modules/xen/xen.te
index d35ae22..41d276a 100644
--- a/tools/flask/policy/policy/modules/xen/xen.te
+++ b/tools/flask/policy/policy/modules/xen/xen.te
@@ -90,6 +90,8 @@ allow dom0_t dom0_t:domain2 {
 };
 allow dom0_t dom0_t:resource { add remove };
 
+allow dom0_t domxen_t:mmu { memorymap map_write };
+
 # These permissions allow using the FLASK security server to compute access
 # checks locally, which could be used by a domain or service (such as xenstore)
 # that does not have its own security server to make access decisions based on
-- 
2.7.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 1/6] Flask: Support for ARM xentrace

[Julien Grall]

Hello Benjamin,

Thank you for the patch.

On 16/03/16 20:51, Benjamin Sanda wrote:
> From: bensanda <ben.sanda@dornerworks.com>
>
> Modified to provide support for xentrace on the ARM platform. Added flask credential to allow dom0 dom_xen mapping and write access for trace buffers.

A line in the commit message should not be longer than 72 characters.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 1/6] Flask: Support for ARM xentrace

[Julien Grall]

(+ Daniel De Graaf maintainer of the XSM/Flask code)

On 16/03/16 20:51, Benjamin Sanda wrote:
> From: bensanda <ben.sanda@dornerworks.com>
>
> Modified to provide support for xentrace on the ARM platform. Added flask credential to allow dom0 dom_xen mapping and write access for trace buffers.
>
> Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
> ---
>   tools/flask/policy/policy/modules/xen/xen.te | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/tools/flask/policy/policy/modules/xen/xen.te b/tools/flask/policy/policy/modules/xen/xen.te
> index d35ae22..41d276a 100644
> --- a/tools/flask/policy/policy/modules/xen/xen.te
> +++ b/tools/flask/policy/policy/modules/xen/xen.te
> @@ -90,6 +90,8 @@ allow dom0_t dom0_t:domain2 {
>   };
>   allow dom0_t dom0_t:resource { add remove };
>
> +allow dom0_t domxen_t:mmu { memorymap map_write };
> +
>   # These permissions allow using the FLASK security server to compute access
>   # checks locally, which could be used by a domain or service (such as xenstore)
>   # that does not have its own security server to make access decisions based on
>

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 1/6] Flask: Support for ARM xentrace

[Konrad Rzeszutek Wilk]

On Wed, Mar 16, 2016 at 01:51:34PM -0700, Benjamin Sanda wrote:
> From: bensanda <ben.sanda@dornerworks.com>
> 
> Modified to provide support for xentrace on the ARM platform. Added flask credential to allow dom0 dom_xen mapping and write access for trace buffers.

So .. what does that mean?

Is that something xentrace requests? Why is this ARM specific?
Looking at xsm_sysctl and how the trace is setup it checks for
XEN__TBUFCONTROL?

But this is more specific?
> 
> Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
> ---
>  tools/flask/policy/policy/modules/xen/xen.te | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/flask/policy/policy/modules/xen/xen.te b/tools/flask/policy/policy/modules/xen/xen.te
> index d35ae22..41d276a 100644
> --- a/tools/flask/policy/policy/modules/xen/xen.te
> +++ b/tools/flask/policy/policy/modules/xen/xen.te
> @@ -90,6 +90,8 @@ allow dom0_t dom0_t:domain2 {
>  };
>  allow dom0_t dom0_t:resource { add remove };
>  
> +allow dom0_t domxen_t:mmu { memorymap map_write };
> +
>  # These permissions allow using the FLASK security server to compute access
>  # checks locally, which could be used by a domain or service (such as xenstore)
>  # that does not have its own security server to make access decisions based on
> -- 
> 2.7.2
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 1/6] Flask: Support for ARM xentrace

[Ben Sanda]

Konrad,

> Modified to provide support for xentrace on the ARM platform. Added
> flask credential to allow dom0 dom_xen mapping and write access for
> trace buffers.
>>
>> So .. what does that mean?
>>
>> Is that something xentrace requests? Why is this ARM specific?
>> Looking at xsm_sysctl and how the trace is setup it checks for
>> XEN__TBUFCONTROL?

I need to bring in Paul for this conversation. The Flask modifications
were originally made by him so I'm not sure on their exact purpose
or derivation.

Paul: Can you provide any insight as to the changes to the Flask code?

Thanks,
Ben

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 2/6] xenalyze: Support for ARM platform

[Benjamin Sanda]

From: bensanda <ben.sanda@dornerworks.com>

Modified to provide building of the xenalyze binary for ARM platforms. This was done in conjunction with patches to xentrace allowing its use on ARM. The xenalyze binary is now built as part of the SBIN list.

Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
---
 tools/xentrace/Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/xentrace/Makefile b/tools/xentrace/Makefile
index 0157be2..180add4 100644
--- a/tools/xentrace/Makefile
+++ b/tools/xentrace/Makefile
@@ -9,9 +9,8 @@ LDLIBS += $(LDLIBS_libxenevtchn)
 LDLIBS += $(LDLIBS_libxenctrl)
 LDLIBS += $(ARGP_LDFLAGS)
 
-BIN-$(CONFIG_X86) = xenalyze
 BIN      = $(BIN-y)
-SBIN     = xentrace xentrace_setsize
+SBIN     = xenalyze xentrace xentrace_setsize 
 LIBBIN   = xenctx
 SCRIPTS  = xentrace_format
 
-- 
2.7.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 2/6] xenalyze: Support for ARM platform

[Andrew Cooper]

On 16/03/2016 20:51, Benjamin Sanda wrote:
> From: bensanda <ben.sanda@dornerworks.com>
>
> Modified to provide building of the xenalyze binary for ARM platforms. This was done in conjunction with patches to xentrace allowing its use on ARM. The xenalyze binary is now built as part of the SBIN list.

It should be part of the unconditional BIN list, rather than SBIN.

xenalyse does not need special permissions to run, so shouldn't be
restricted to supervisor-only.

~Andrew

>
> Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
> ---
>  tools/xentrace/Makefile | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/tools/xentrace/Makefile b/tools/xentrace/Makefile
> index 0157be2..180add4 100644
> --- a/tools/xentrace/Makefile
> +++ b/tools/xentrace/Makefile
> @@ -9,9 +9,8 @@ LDLIBS += $(LDLIBS_libxenevtchn)
>  LDLIBS += $(LDLIBS_libxenctrl)
>  LDLIBS += $(ARGP_LDFLAGS)
>  
> -BIN-$(CONFIG_X86) = xenalyze
>  BIN      = $(BIN-y)
> -SBIN     = xentrace xentrace_setsize
> +SBIN     = xenalyze xentrace xentrace_setsize 
>  LIBBIN   = xenctx
>  SCRIPTS  = xentrace_format
>  


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 2/6] xenalyze: Support for ARM platform

[George Dunlap]

On Wed, Mar 16, 2016 at 8:59 PM, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> On 16/03/2016 20:51, Benjamin Sanda wrote:
>> From: bensanda <ben.sanda@dornerworks.com>
>>
>> Modified to provide building of the xenalyze binary for ARM platforms. This was done in conjunction with patches to xentrace allowing its use on ARM. The xenalyze binary is now built as part of the SBIN list.
>
> It should be part of the unconditional BIN list, rather than SBIN.
>
> xenalyse does not need special permissions to run, so shouldn't be
> restricted to supervisor-only.

Andrew is correct -- please replace both BIN lines with

BIN     = xenalyze

Thanks,
 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 3/6] xentrace: P2M lookup suport for ARM platform

[Benjamin Sanda]

From: bensanda <ben.sanda@dornerworks.com>

Modified p2m_lookup() to provide support for xentrace on the ARM platform. Added check for DOMID_XEN which skips PFN to MFN translation. xentrace sends a MFN dirrectly when requesting DOMID_XEN, so no translation is needed. Also sets page memory type, p2m_type_t, to p2m_ram_rw to provide correct access.

Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
---
 xen/arch/arm/p2m.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index a2a9c4b..2e7da43 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -228,10 +228,21 @@ paddr_t p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
     paddr_t ret;
     struct p2m_domain *p2m = &d->arch.p2m;
 
-    spin_lock(&p2m->lock);
-    ret = __p2m_lookup(d, paddr, t);
-    spin_unlock(&p2m->lock);
-
+    /* Check for DOMID_XEN: If we are called with DOMID_XEN (from xentrace)
+    then paddr is already a MFN and no translation is needed. We only set the 
+    page type as p2m_raw_rw and return the MFN directly */
+    if(DOMID_XEN != d->domain_id)
+    {
+        spin_lock(&p2m->lock);
+        ret = __p2m_lookup(d, paddr, t);
+        spin_unlock(&p2m->lock);
+    }
+    else
+    {
+        *t = p2m_ram_rw;
+        ret = paddr;
+    }
+    
     return ret;
 }
 
-- 
2.7.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 3/6] xentrace: P2M lookup suport for ARM platform

[Julien Grall]

Hello Benjamin,

Thank you for the patch.

On 16/03/16 20:51, Benjamin Sanda wrote:
> From: bensanda <ben.sanda@dornerworks.com>
>
> Modified p2m_lookup() to provide support for xentrace on the ARM platform. Added check for DOMID_XEN which skips PFN to MFN translation. xentrace sends a MFN dirrectly when requesting DOMID_XEN, so no translation is needed. Also sets page memory type, p2m_type_t, to p2m_ram_rw to provide correct access.

A line in the commit message should not be longer than 72 characters.

>
> Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
> ---
>   xen/arch/arm/p2m.c | 19 +++++++++++++++----
>   1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index a2a9c4b..2e7da43 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -228,10 +228,21 @@ paddr_t p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
>       paddr_t ret;
>       struct p2m_domain *p2m = &d->arch.p2m;
>
> -    spin_lock(&p2m->lock);
> -    ret = __p2m_lookup(d, paddr, t);
> -    spin_unlock(&p2m->lock);
> -
> +    /* Check for DOMID_XEN: If we are called with DOMID_XEN (from xentrace)

Multi-lines comment in Xen should be:

/*
  * Foo
  * Bar
  */

You can find the coding style in CODING_STYLE.

> +    then paddr is already a MFN and no translation is needed. We only set the
> +    page type as p2m_raw_rw and return the MFN directly */

DOMID_XEN is not specific to xentrace. It's a mechanism to share xenheap 
page to any guest.

xentrace is using directly an MFN because DOMID_XEN is considered as a 
PV guest on x86 (i.e MFN == GFN). And we don't have a such concept on ARM.

> +    if(DOMID_XEN != d->domain_id)


if ( d->domain_id != DOMID_XEN )

> +    {
> +        spin_lock(&p2m->lock);
> +        ret = __p2m_lookup(d, paddr, t);
> +        spin_unlock(&p2m->lock);
> +    }
> +    else
> +    {
> +        *t = p2m_ram_rw;

A DOMID_XEN page could be read only too. For instance, the meta-data of 
the trace buffer is read-only (see t_info), we don't want a domain to be 
able to overwrite them.

However, all the foreign page are mapped read-write. You will need to 
rework the code to map a foreign domain (see XENMAPSPACE_gmfn_foreign) 
to allow read-only foreign page (maybe by adding a new p2m_type_t?).

> +        ret = paddr;
> +    }
> +
>       return ret;
>   }
>
>

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 3/6] xentrace: P2M lookup suport for ARM platform

[Ben Sanda]

Julien and George,

Thank you for the comments. I had one question I wanted to ask.

>A DOMID_XEN page could be read only too. For instance, the meta-data
>of the trace buffer is read-only (see t_info), we don't want a domain
>to be able to overwrite them.

>However, all the foreign page are mapped read-write. You will need to
>rework the code to map a foreign domain (see
>XENMAPSPACE_gmfn_foreign) to allow read-only foreign page (maybe by
>adding a new p2m_type_t?).

I understand what you are saying in general, but I'm not familiar
enough with the Xen memory mapping system to know how to actually
implement this. The p2m_type_t p2m_ram_ro exists, which I could assign
to read-only pages, but I'm unsure as to how to detect whether a
request is to a read only mapping or a read-write. The normal (non
DOMID_XEN) p2m_lookup function normally does this by reading the root-
level page tables and somehow extracting the mapping type from the
lpae_t structure. Given that we are not looking up the page tables for
non-translated addresses, I'm not sure where/how to find the correct
mapping type. Can I still lookup the page table entries for the MFN
address and extract the p2m_type_t the same way?

Thank you for any insight or assistance,
Ben
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 3/6] xentrace: P2M lookup suport for ARM platform

[Julien Grall]

On 28/03/16 19:55, Ben Sanda wrote:
> Julien and George,

Hi Ben,

Sorry for the late answer.

> Thank you for the comments. I had one question I wanted to ask.
>
>> A DOMID_XEN page could be read only too. For instance, the meta-data
>> of the trace buffer is read-only (see t_info), we don't want a domain
>> to be able to overwrite them.
>
>> However, all the foreign page are mapped read-write. You will need to
>> rework the code to map a foreign domain (see
>> XENMAPSPACE_gmfn_foreign) to allow read-only foreign page (maybe by
>> adding a new p2m_type_t?).
>
> I understand what you are saying in general, but I'm not familiar
> enough with the Xen memory mapping system to know how to actually
> implement this. The p2m_type_t p2m_ram_ro exists, which I could assign
> to read-only pages, but I'm unsure as to how to detect whether a
> request is to a read only mapping or a read-write. The normal (non
> DOMID_XEN) p2m_lookup function normally does this by reading the root-
> level page tables and somehow extracting the mapping type from the
> lpae_t structure. Given that we are not looking up the page tables for
> non-translated addresses, I'm not sure where/how to find the correct
> mapping type. Can I still lookup the page table entries for the MFN
> address and extract the p2m_type_t the same way?

You can know if the page is writable by looking to the page field 
u.inuse.type_info (PGT_writable_page means the page is writable).

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 4/6] xentrace: ARM platform DOMID_XEN mapping support

[Benjamin Sanda]

From: bensanda <ben.sanda@dornerworks.com>

Modified xenmem_add_to_physmap_one() to provide support for xentrace on the ARM platform. Checks for DOMID_XEN added via new function, get_pg_owner, ported from x86 code base. This provides correct calls to rcu_lock_domain() when DOMID_XEN is requested. DOMID_XEN checks also adde to skip page to MFN translation (xentrace sends a MFN dirrectly and so does not need to be translated).

Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
---
 xen/arch/arm/mm.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 54 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 81f9e2e..b1d834f 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -41,6 +41,7 @@
 #include <xen/pfn.h>
 
 struct domain *dom_xen, *dom_io, *dom_cow;
+static struct domain *get_pg_owner(domid_t domid);
 
 /* Static start-of-day pagetables that we use before the allocators
  * are up. These are used by all CPUs during bringup before switching
@@ -1099,7 +1100,8 @@ int xenmem_add_to_physmap_one(
     {
         struct domain *od;
         p2m_type_t p2mt;
-        od = rcu_lock_domain_by_any_id(foreign_domid);
+        od = get_pg_owner(foreign_domid);
+
         if ( od == NULL )
             return -ESRCH;
 
@@ -1132,7 +1134,17 @@ int xenmem_add_to_physmap_one(
             return -EINVAL;
         }
 
-        mfn = page_to_mfn(page);
+        /* If DOMID_XEN then no page to MFN translation is 
+        needed as we already have the MFN directly */
+        if(DOMID_XEN !=od->domain_id)
+        {
+            mfn = page_to_mfn(page);
+        }
+        else
+        {
+            mfn = idx;
+        }
+
         t = p2m_map_foreign;
 
         rcu_unlock_domain(od);
@@ -1312,6 +1324,46 @@ void clear_and_clean_page(struct page_info *page)
     unmap_domain_page(p);
 }
 
+/* Ported from x86 architecture: checks for special domain requests for
+DOMID_XEN or DOMID_IO which must be handled differently then guest domain 
+requests */
+static struct domain *get_pg_owner(domid_t domid)
+{
+    struct domain *pg_owner = NULL, *curr = current->domain;
+
+    if ( likely(domid == DOMID_SELF) )
+    {
+        pg_owner = rcu_lock_current_domain();
+        goto out;
+    }
+
+    if ( unlikely(domid == curr->domain_id) )
+    {
+        goto out;
+    }
+
+    /* check for special domain cases of DOMID_IO or DOMID_XEN which
+    must use rcu_lock_domain() and dom_xen/dom_io as the domid_t */
+    switch ( domid )
+    {
+    case DOMID_IO:
+        pg_owner = rcu_lock_domain(dom_io);
+        break;
+    case DOMID_XEN:
+        pg_owner = rcu_lock_domain(dom_xen);
+        break;
+    default:
+        if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
+        {
+            break;
+        }
+        break;
+    }
+
+ out:
+    return pg_owner;
+}
+
 /*
  * Local variables:
  * mode: C
-- 
2.7.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 4/6] xentrace: ARM platform DOMID_XEN mapping support

[Julien Grall]

Hello Benjamin,

Thank you for the patch.

On 16/03/16 20:51, Benjamin Sanda wrote:
> From: bensanda <ben.sanda@dornerworks.com>
>
> Modified xenmem_add_to_physmap_one() to provide support for xentrace on the ARM platform. Checks for DOMID_XEN added via new function, get_pg_owner, ported from x86 code base. This provides correct calls to rcu_lock_domain() when DOMID_XEN is requested. DOMID_XEN checks also adde to skip page to MFN translation (xentrace sends a MFN dirrectly and so does not need to be translated).

A line in the commit message should not be longer than 72 characters.

>
> Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
> ---
>   xen/arch/arm/mm.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 54 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 81f9e2e..b1d834f 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -41,6 +41,7 @@
>   #include <xen/pfn.h>
>
>   struct domain *dom_xen, *dom_io, *dom_cow;
> +static struct domain *get_pg_owner(domid_t domid);

I would rather avoid to forward declare a function if there is no strict 
dependency on other functions. Instead, I would add the function before 
the caller.

>
>   /* Static start-of-day pagetables that we use before the allocators
>    * are up. These are used by all CPUs during bringup before switching
> @@ -1099,7 +1100,8 @@ int xenmem_add_to_physmap_one(
>       {
>           struct domain *od;
>           p2m_type_t p2mt;
> -        od = rcu_lock_domain_by_any_id(foreign_domid);
> +        od = get_pg_owner(foreign_domid);
> +
>           if ( od == NULL )
>               return -ESRCH;
>
> @@ -1132,7 +1134,17 @@ int xenmem_add_to_physmap_one(
>               return -EINVAL;
>           }
>
> -        mfn = page_to_mfn(page);
> +        /* If DOMID_XEN then no page to MFN translation is
> +        needed as we already have the MFN directly */
> +        if(DOMID_XEN !=od->domain_id)
> +        {
> +            mfn = page_to_mfn(page);
> +        }
> +        else
> +        {
> +            mfn = idx;
> +        }
> +

Please avoid to spread the DOMID_ID specific case everywhere. The cost 
to calculate the MFN of a page is very limited.

>           t = p2m_map_foreign;
>
>           rcu_unlock_domain(od);
> @@ -1312,6 +1324,46 @@ void clear_and_clean_page(struct page_info *page)
>       unmap_domain_page(p);
>   }
>
> +/* Ported from x86 architecture: checks for special domain requests for
> +DOMID_XEN or DOMID_IO which must be handled differently then guest domain
> +requests */
> +static struct domain *get_pg_owner(domid_t domid)

This function is very similar to the x86 one. I think it would benefit 
to implement get_pg_owner in the common code and add arch specific 
helper when it's necessary.

Also, please introduce the helper put_pg_owner to stay consistent.

> +{
> +    struct domain *pg_owner = NULL, *curr = current->domain;
> +
> +    if ( likely(domid == DOMID_SELF) )
> +    {
> +        pg_owner = rcu_lock_current_domain();
> +        goto out;
> +    }
> +
> +    if ( unlikely(domid == curr->domain_id) )
> +    {
> +        goto out;
> +    }
> +
> +    /* check for special domain cases of DOMID_IO or DOMID_XEN which
> +    must use rcu_lock_domain() and dom_xen/dom_io as the domid_t */
> +    switch ( domid )
> +    {
> +    case DOMID_IO:
> +        pg_owner = rcu_lock_domain(dom_io);
> +        break;
> +    case DOMID_XEN:
> +        pg_owner = rcu_lock_domain(dom_xen);
> +        break;
> +    default:
> +        if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
> +        {
> +            break;
> +        }
> +        break;
> +    }
> +
> + out:
> +    return pg_owner;
> +}
> +
>   /*
>    * Local variables:
>    * mode: C
>

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 5/6] xentrace: Trace buffer support for ARM platform

[Benjamin Sanda]

From: bensanda <ben.sanda@dornerworks.com>

Modified to provide support for xentrace on the ARM platform. Added call to init_trace_bufs() to initialize the trace buffers for use by xentrace.

Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
---
 xen/arch/arm/setup.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 6d205a9..87e70c9 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -35,6 +35,7 @@
 #include <xen/cpu.h>
 #include <xen/pfn.h>
 #include <xen/vmap.h>
+#include <xen/trace.h>
 #include <xen/libfdt/libfdt.h>
 #include <xen/acpi.h>
 #include <asm/page.h>
@@ -851,6 +852,8 @@ void __init start_xen(unsigned long boot_phys_offset,
     /* Scrub RAM that is still free and so may go to an unprivileged domain. */
     scrub_heap_pages();
 
+    init_trace_bufs();
+
     init_constructors();
 
     console_endboot();
-- 
2.7.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 6/6] xentrace: ARM platform timestamp support

[Benjamin Sanda]

From: bensanda <ben.sanda@dornerworks.com>

Modified to provide support for xentrace on the ARM platform. Changed get_cycles() to return the core timestamp tick count for use by the trace buffer timestamping routines in xentrace.

Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
---
 xen/include/asm-arm/time.h | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h
index 5b9a31d..f3a22d5 100644
--- a/xen/include/asm-arm/time.h
+++ b/xen/include/asm-arm/time.h
@@ -1,15 +1,21 @@
 #ifndef __ARM_TIME_H__
 #define __ARM_TIME_H__
 
+#include <asm/regs.h>
+
 #define DT_MATCH_TIMER                      \
     DT_MATCH_COMPATIBLE("arm,armv7-timer"), \
     DT_MATCH_COMPATIBLE("arm,armv8-timer")
 
-typedef unsigned long cycles_t;
+/* Counter value at boot time */
+extern uint64_t boot_count;
+
+typedef uint64_t cycles_t;
 
 static inline cycles_t get_cycles (void)
 {
-        return 0;
+        /* return raw tick count of main timer */
+        return READ_SYSREG64(CNTPCT_EL0) - boot_count;
 }
 
 /* List of timer's IRQ */
@@ -34,9 +40,6 @@ unsigned int timer_get_irq(enum timer_ppi ppi);
 /* Set up the timer interrupt on this CPU */
 extern void init_timer_interrupt(void);
 
-/* Counter value at boot time */
-extern uint64_t boot_count;
-
 extern s_time_t ticks_to_ns(uint64_t ticks);
 extern uint64_t ns_to_ticks(s_time_t ns);
 
-- 
2.7.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 6/6] xentrace: ARM platform timestamp support

[Konrad Rzeszutek Wilk]

On Wed, Mar 16, 2016 at 01:51:39PM -0700, Benjamin Sanda wrote:
> From: bensanda <ben.sanda@dornerworks.com>
> 
> Modified to provide support for xentrace on the ARM platform. Changed get_cycles() to return the core timestamp tick count for use by the trace buffer timestamping routines in xentrace.
> 
> Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>

That is missing the CC to Stefano or Julien. CC-ing them.
> ---
>  xen/include/asm-arm/time.h | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h
> index 5b9a31d..f3a22d5 100644
> --- a/xen/include/asm-arm/time.h
> +++ b/xen/include/asm-arm/time.h
> @@ -1,15 +1,21 @@
>  #ifndef __ARM_TIME_H__
>  #define __ARM_TIME_H__
>  
> +#include <asm/regs.h>
> +
>  #define DT_MATCH_TIMER                      \
>      DT_MATCH_COMPATIBLE("arm,armv7-timer"), \
>      DT_MATCH_COMPATIBLE("arm,armv8-timer")
>  
> -typedef unsigned long cycles_t;
> +/* Counter value at boot time */
> +extern uint64_t boot_count;
> +
> +typedef uint64_t cycles_t;
>  
>  static inline cycles_t get_cycles (void)
>  {
> -        return 0;
> +        /* return raw tick count of main timer */
> +        return READ_SYSREG64(CNTPCT_EL0) - boot_count;
>  }
>  
>  /* List of timer's IRQ */
> @@ -34,9 +40,6 @@ unsigned int timer_get_irq(enum timer_ppi ppi);
>  /* Set up the timer interrupt on this CPU */
>  extern void init_timer_interrupt(void);
>  
> -/* Counter value at boot time */
> -extern uint64_t boot_count;
> -
>  extern s_time_t ticks_to_ns(uint64_t ticks);
>  extern uint64_t ns_to_ticks(s_time_t ns);
>  
> -- 
> 2.7.2
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 6/6] xentrace: ARM platform timestamp support

[Stefano Stabellini]

On Fri, 25 Mar 2016, Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 16, 2016 at 01:51:39PM -0700, Benjamin Sanda wrote:
> > From: bensanda <ben.sanda@dornerworks.com>
> > 
> > Modified to provide support for xentrace on the ARM platform. Changed get_cycles() to return the core timestamp tick count for use by the trace buffer timestamping routines in xentrace.
> > 
> > Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
> 
> That is missing the CC to Stefano or Julien. CC-ing them.

Thanks


> >  xen/include/asm-arm/time.h | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h
> > index 5b9a31d..f3a22d5 100644
> > --- a/xen/include/asm-arm/time.h
> > +++ b/xen/include/asm-arm/time.h
> > @@ -1,15 +1,21 @@
> >  #ifndef __ARM_TIME_H__
> >  #define __ARM_TIME_H__
> >  
> > +#include <asm/regs.h>
> > +
> >  #define DT_MATCH_TIMER                      \
> >      DT_MATCH_COMPATIBLE("arm,armv7-timer"), \
> >      DT_MATCH_COMPATIBLE("arm,armv8-timer")
> >  
> > -typedef unsigned long cycles_t;
> > +/* Counter value at boot time */
> > +extern uint64_t boot_count;

Changing cycles_t to uint64_t sounds good, but why did you move
boot_count here from below?


> > +typedef uint64_t cycles_t;
> >  
> >  static inline cycles_t get_cycles (void)
> >  {
> > -        return 0;
> > +        /* return raw tick count of main timer */
> > +        return READ_SYSREG64(CNTPCT_EL0) - boot_count;
> >  }
> >  
> >  /* List of timer's IRQ */
> > @@ -34,9 +40,6 @@ unsigned int timer_get_irq(enum timer_ppi ppi);
> >  /* Set up the timer interrupt on this CPU */
> >  extern void init_timer_interrupt(void);
> >  
> > -/* Counter value at boot time */
> > -extern uint64_t boot_count;
> > -
> >  extern s_time_t ticks_to_ns(uint64_t ticks);
> >  extern uint64_t ns_to_ticks(s_time_t ns);
> >  
> > -- 
> > 2.7.2
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 6/6] xentrace: ARM platform timestamp support

[Ben Sanda]

Stefano,

Thank you for the comments. In response:

> Changing cycles_t to uint64_t sounds good, but why did you move
> boot_count here from below?

I had to move it up so it would be defined for use in the updated
get_cycles() call: 
return READ_SYSREG64(CNTPCT_EL0) - boot_count;

Should get_cyclces not use boot_count? I included it as the normal
system time call uses it so I assumed this should be consistent.

Thanks,
Ben Sanda

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 6/6] xentrace: ARM platform timestamp support

[Stefano Stabellini]

On Thu, 31 Mar 2016, Ben Sanda wrote:
> Stefano,
> 
> Thank you for the comments. In response:
> 
> > Changing cycles_t to uint64_t sounds good, but why did you move
> > boot_count here from below?
> 
> I had to move it up so it would be defined for use in the updated
> get_cycles() call: 
> return READ_SYSREG64(CNTPCT_EL0) - boot_count;
> 
> Should get_cyclces not use boot_count? I included it as the normal
> system time call uses it so I assumed this should be consistent.

Sorry I misread your patch. Your implementation of get_cycles is
correct but I would prefer to avoid including asm/regs.h in time.h.

I would prefer if the implementation was in xen/arch/arm/time.c (even if
that means no static inline). Then you could base get_s_time on get_cycles.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 0/6] xentrace/xenalyze support on ARM

[Julien Grall]

Hello Benjamin,

On 16/03/16 20:51, Benjamin Sanda wrote:
> From: bensanda <ben.sanda@dornerworks.com>
>
> This patch sets adds support for xentrace and xenalyze to the ARM platform.
> The Xen heap memory mapping, timestamping, and P2M translation needed by
> xentrace is corrected for operation on the ARM platform using the x86
> platform as reference. Trace buffer initialization is added to setup.c,
> and Flask permissions are modified to allow write mapping of the Xen heap.
> Finally the xenaylze makefile is configured to build the tool for ARM.

Can you please CC the relevant maintainers in the next version of this 
series? You can use scripts/get_maintainers.pl for this purpose.

Regards,

> bensanda (6):
>    Flask: Support for ARM xentrace
>    xenalyze: Support for ARM platform
>    xentrace: P2M lookup suport for ARM platform
>    xentrace: ARM platform DOMID_XEN mapping support
>    xentrace: Trace buffer support for ARM platform
>    xentrace: ARM platform timestamp support
>
>   tools/flask/policy/policy/modules/xen/xen.te |  2 +
>   tools/xentrace/Makefile                      |  3 +-
>   xen/arch/arm/mm.c                            | 56 +++++++++++++++++++++++++++-
>   xen/arch/arm/p2m.c                           | 19 ++++++++--
>   xen/arch/arm/setup.c                         |  3 ++
>   xen/include/asm-arm/time.h                   | 13 ++++---
>   6 files changed, 83 insertions(+), 13 deletions(-)
>

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 0/6] xentrace/xenalyze support on ARM

[Ben Sanda]

Julien,

> Can you please CC the relevant maintainers in the next version of this series?
> You can use scripts/get_maintainers.pl for this purpose.

I did. The list output to me was:
Ian Jackson <ian.jackson@eu.citrix.com>
Jan Beulich <jbeulich@suse.com>
Keir Fraser <keir@xen.org>
Tim Deegan <tim@xen.org>

I also added a few more that I had already
been in contact with regarding this effort but
no other names were output. I can be sure 
to include you in future updates.

Ben
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 0/6] xentrace/xenalyze support on ARM

[Julien Grall]

Hello Benjamin,

On 17/03/16 16:50, Ben Sanda wrote:
>> Can you please CC the relevant maintainers in the next version of this series?
>> You can use scripts/get_maintainers.pl for this purpose.
> 
> I did. The list output to me was:
> Ian Jackson <ian.jackson@eu.citrix.com>
> Jan Beulich <jbeulich@suse.com>
> Keir Fraser <keir@xen.org>
> Tim Deegan <tim@xen.org>

How did you invoke get_maintainers.pl?

I tried with the patch #4 and got the following ouput:

42sh$ scripts/get_maintainers.pl < xentrace-ARM-platform-DOMID_XEN-mapping-support.patch

Stefano Stabellini <stefano.stabellini@citrix.com>
Julien Grall <julien.grall@arm.com>
xen-devel@lists.xen.org

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 0/6] xentrace/xenalyze support on ARM

[Ben Sanda]

Julien,

>How did you invoke get_maintainers.pl?
>
>I tried with the patch #4 and got the following ouput:
>
>42sh$ scripts/get_maintainers.pl < xentrace-ARM-platform-DOMID_XEN-mapping-support.patch
>
>Stefano Stabellini <stefano.stabellini@citrix.com> Julien Grall <julien.grall@arm.com> xen-devel@lists.xen.org

Since I had a multifile patch I invoked it on the folder generated by
git send-email --format-patch

./scripts/get_maintainer.pl -f new_patch

Reading the help usage it indicated you could run it on folders or
directories. It would seem you have to run it on a per file basic
though? I will be sure to correct this moving forward.

Thanks,
Ben

-----Original Message-----
From: Julien Grall [mailto:julien.grall@arm.com] 
Sent: 17 March, 2016 13:01
To: Ben Sanda <Ben.Sanda@dornerworks.com>; xen-devel@lists.xenproject.org
Subject: Re: [Xen-devel] [PATCH 0/6] xentrace/xenalyze support on ARM

Hello Benjamin,

On 17/03/16 16:50, Ben Sanda wrote:
>> Can you please CC the relevant maintainers in the next version of this series?
>> You can use scripts/get_maintainers.pl for this purpose.
> 
> I did. The list output to me was:
> Ian Jackson <ian.jackson@eu.citrix.com> Jan Beulich 
> <jbeulich@suse.com> Keir Fraser <keir@xen.org> Tim Deegan 
> <tim@xen.org>

How did you invoke get_maintainers.pl?

I tried with the patch #4 and got the following ouput:

42sh$ scripts/get_maintainers.pl < xentrace-ARM-platform-DOMID_XEN-mapping-support.patch

Stefano Stabellini <stefano.stabellini@citrix.com> Julien Grall <julien.grall@arm.com> xen-devel@lists.xen.org

Regards,

--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 0/6] xentrace/xenalyze support on ARM

[Wei Liu]

On Thu, Mar 17, 2016 at 05:04:50PM +0000, Ben Sanda wrote:
> Julien,
> 
> >How did you invoke get_maintainers.pl?
> >
> >I tried with the patch #4 and got the following ouput:
> >
> >42sh$ scripts/get_maintainers.pl < xentrace-ARM-platform-DOMID_XEN-mapping-support.patch
> >
> >Stefano Stabellini <stefano.stabellini@citrix.com> Julien Grall <julien.grall@arm.com> xen-devel@lists.xen.org
> 
> Since I had a multifile patch I invoked it on the folder generated by
> git send-email --format-patch
> 
> ./scripts/get_maintainer.pl -f new_patch
> 
> Reading the help usage it indicated you could run it on folders or
> directories. It would seem you have to run it on a per file basic
> though? I will be sure to correct this moving forward.
> 

I think you did it wrong -- when using get_maintainer.pl on actual
patches, you don't need -f. It's only that when you use it on
files that are not patches that you need -f.

In you case, the invocation should be
  ./scripts/get_maintainer.pl new_patch

Wei.

> Thanks,
> Ben
> 
> -----Original Message-----
> From: Julien Grall [mailto:julien.grall@arm.com] 
> Sent: 17 March, 2016 13:01
> To: Ben Sanda <Ben.Sanda@dornerworks.com>; xen-devel@lists.xenproject.org
> Subject: Re: [Xen-devel] [PATCH 0/6] xentrace/xenalyze support on ARM
> 
> Hello Benjamin,
> 
> On 17/03/16 16:50, Ben Sanda wrote:
> >> Can you please CC the relevant maintainers in the next version of this series?
> >> You can use scripts/get_maintainers.pl for this purpose.
> > 
> > I did. The list output to me was:
> > Ian Jackson <ian.jackson@eu.citrix.com> Jan Beulich 
> > <jbeulich@suse.com> Keir Fraser <keir@xen.org> Tim Deegan 
> > <tim@xen.org>
> 
> How did you invoke get_maintainers.pl?
> 
> I tried with the patch #4 and got the following ouput:
> 
> 42sh$ scripts/get_maintainers.pl < xentrace-ARM-platform-DOMID_XEN-mapping-support.patch
> 
> Stefano Stabellini <stefano.stabellini@citrix.com> Julien Grall <julien.grall@arm.com> xen-devel@lists.xen.org
> 
> Regards,
> 
> --
> Julien Grall
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 0/6] xentrace/xenalyze support on ARM

[George Dunlap]

On 16/03/16 20:51, Benjamin Sanda wrote:
> From: bensanda <ben.sanda@dornerworks.com>
> 
> This patch sets adds support for xentrace and xenalyze to the ARM platform.
> The Xen heap memory mapping, timestamping, and P2M translation needed by
> xentrace is corrected for operation on the ARM platform using the x86 
> platform as reference. Trace buffer initialization is added to setup.c, 
> and Flask permissions are modified to allow write mapping of the Xen heap. 
> Finally the xenaylze makefile is configured to build the tool for ARM.

Hey Ben,

Thanks for this work.  Modulo the foreign mapping stuff Julien pointed
out, and the placement of xenalyze in BIN rather than SBIN, the code
looks good to me.

 -George


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel