[PATCH v4 00/39] arm/altp2m: Introducing altp2m to ARM

[PATCH v4 00/39] arm/altp2m: Introducing altp2m to ARM

[Sergej Proskurin]

Hi all,

The following patch series can be found on Github[0] and is part of my
contribution to last year's Google Summer of Code (GSoC)[1]. My project is
managed by the organization The Honeynet Project. As part of GSoC, I was being
supervised by the Xen maintainer Tamas K. Lengyel <tamas@tklengyel.com>, George
D. Webster, and Steven Maresca.

In this patch series, we provide an implementation of the altp2m subsystem for
ARM. Our implementation is based on the altp2m subsystem for x86, providing
additional --alternate-- views on the guest's physical memory by means of the
ARM 2nd stage translation mechanism. The patches introduce new HVMOPs and
extend the p2m subsystem. Also, we extend libxl to support altp2m on ARM and
modify xen-access to test the suggested functionality.

To be more precise, altp2m allows to create and switch to additional p2m views
(i.e. gfn to mfn mappings). These views can be manipulated and activated as
will through the provided HVMOPs. In this way, the active guest instance in
question can seamlessly proceed execution without noticing that anything has
changed. The prime scope of application of altp2m is Virtual Machine
Introspection, where guest systems are analyzed from the outside of the VM.

Altp2m can be activated by means of the guest control parameter "altp2m" on x86
and ARM architectures. For use-cases requiring purely external access to
altp2m, this patch allows to specify if the altp2m interface should be external
only.

This version is a revised version of v3 that has been submitted in 2016. It
incorporates the comments of the previous patch series. Although the previous
version has been submitted last year, I have kept the comments of the
individual patches. Both the purpose and changes from v3 to v4 are stated
inside the individual commits.

Best regards,
~Sergej

[0] https://github.com/sergej-proskurin/xen (branch arm-altp2m-v4)
[1] https://summerofcode.withgoogle.com/projects/#4970052843470848

Sergej Proskurin (38):
  arm/p2m: Introduce p2m_(switch|restore)_vttbr_and_(g|s)et_flags
  arm/p2m: Add first altp2m HVMOP stubs
  arm/p2m: Add hvm_allow_(set|get)_param
  arm/p2m: Add HVMOP_altp2m_get_domain_state
  arm/p2m: Introduce p2m_is_(hostp2m|altp2m)
  arm/p2m: Cosmetic fix - substitute _gfn(ULONG_MAX) for INVALID_GFN
  arm/p2m: Move hostp2m init/teardown to individual functions
  arm/p2m: Cosmetic fix - function prototype of p2m_alloc_table
  arm/p2m: Rename parameter in p2m_alloc_vmid
  arm/p2m: Change func prototype and impl of p2m_(alloc|free)_vmid
  altp2m: Move (MAX|INVALID)_ALTP2M to xen/p2m-common.h
  arm/p2m: Add altp2m init/teardown routines
  arm/p2m: Add altp2m table flushing routine
  arm/p2m: Add HVMOP_altp2m_set_domain_state
  arm/p2m: Add HVMOP_altp2m_create_p2m
  arm/p2m: Add HVMOP_altp2m_destroy_p2m
  arm/p2m: Add HVMOP_altp2m_switch_p2m
  arm/p2m: Add p2m_get_active_p2m macro
  arm/p2m: Make p2m_restore_state ready for altp2m
  arm/p2m: Make get_page_from_gva ready for altp2m
  arm/p2m: Cosmetic fix - __p2m_get_mem_access
  arm/p2m: Make p2m_mem_access_check ready for altp2m
  arm/p2m: Cosmetic fix - function prototypes
  arm/p2m: Make p2m_put_l3_page ready for altp2m
  arm/p2m: Modify reference count only if hostp2m active
  arm/p2m: Add HVMOP_altp2m_set_mem_access
  arm/p2m: Add altp2m_propagate_change
  altp2m: Rename p2m_altp2m_check to altp2m_check
  x86/altp2m: Move altp2m_check to altp2m.c
  arm/altp2m: Move altp2m_check to altp2m.h
  arm/altp2m: Introduce altp2m_switch_vcpu_altp2m_by_id
  arm/altp2m: Make altp2m_vcpu_idx ready for altp2m
  arm/p2m: Add altp2m paging mechanism
  arm/p2m: Add HVMOP_altp2m_change_gfn
  arm/p2m: Adjust debug information to altp2m
  altp2m: Allow activating altp2m on ARM domains
  arm/xen-access: Extend xen-access for altp2m on ARM
  arm/xen-access: Add test of xc_altp2m_change_gfn

Tamas K Lengyel (1):
  altp2m: Document external-only use on ARM

 docs/man/xl.cfg.pod.5.in            |   8 +-
 tools/libxl/libxl.h                 |  10 +-
 tools/libxl/libxl_dom.c             |  16 +-
 tools/libxl/libxl_types.idl         |   2 +-
 tools/tests/xen-access/Makefile     |   2 +-
 tools/tests/xen-access/xen-access.c | 213 ++++++++++++-
 xen/arch/arm/Makefile               |   1 +
 xen/arch/arm/altp2m.c               | 601 ++++++++++++++++++++++++++++++++++++
 xen/arch/arm/hvm.c                  | 202 +++++++++++-
 xen/arch/arm/mem_access.c           | 112 +++++--
 xen/arch/arm/p2m.c                  | 219 +++++++++----
 xen/arch/arm/traps.c                |  17 +
 xen/arch/x86/mm/altp2m.c            |   6 +
 xen/arch/x86/mm/p2m.c               |   6 -
 xen/common/vm_event.c               |   3 +-
 xen/include/asm-arm/altp2m.h        |  73 ++++-
 xen/include/asm-arm/domain.h        |  15 +
 xen/include/asm-arm/p2m.h           |  62 +++-
 xen/include/asm-x86/altp2m.h        |   3 +
 xen/include/asm-x86/domain.h        |   3 +-
 xen/include/asm-x86/p2m.h           |   3 -
 xen/include/xen/altp2m-common.h     |   8 +
 22 files changed, 1444 insertions(+), 141 deletions(-)
 create mode 100644 xen/arch/arm/altp2m.c
 create mode 100644 xen/include/xen/altp2m-common.h

-- 
2.13.3


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

[PATCH v4 01/39] arm/p2m: Introduce p2m_(switch|restore)_vttbr_and_(g|s)et_flags

[Sergej Proskurin]

This commit introduces macros for switching and restoring the vttbr
considering the currently set irq flags. We define these macros, as the
following commits will use the associated functionality multiple times
throughout different files.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v4: Save the content of VTTBR_EL2 inside of the introduced macro
    "p2m_switch_vttbr_and_get_flags".

    Move the introduced macros into ./xen/include/asm-arm/p2m.h, as they will
    be used by different files in the future commits.
---
 xen/arch/arm/p2m.c        | 15 ++-------------
 xen/include/asm-arm/p2m.h | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index c484469e6c..4334e3bc81 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -147,22 +147,11 @@ static void p2m_flush_tlb(struct p2m_domain *p2m)
      * ARM only provides an instruction to flush TLBs for the current
      * VMID. So switch to the VTTBR of a given P2M if different.
      */
-    ovttbr = READ_SYSREG64(VTTBR_EL2);
-    if ( ovttbr != p2m->vttbr )
-    {
-        local_irq_save(flags);
-        WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
-        isb();
-    }
+    p2m_switch_vttbr_and_get_flags(ovttbr, p2m->vttbr, flags);
 
     flush_tlb();
 
-    if ( ovttbr != READ_SYSREG64(VTTBR_EL2) )
-    {
-        WRITE_SYSREG64(ovttbr, VTTBR_EL2);
-        isb();
-        local_irq_restore(flags);
-    }
+    p2m_restore_vttbr_and_set_flags(ovttbr, flags);
 }
 
 /*
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index aa0d60ae3a..500dc88fbc 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -12,6 +12,27 @@
 
 #define paddr_bits PADDR_BITS
 
+#define p2m_switch_vttbr_and_get_flags(ovttbr, nvttbr, flags)       \
+({                                                                  \
+     ovttbr = READ_SYSREG64(VTTBR_EL2);                             \
+     if ( ovttbr != nvttbr )                                        \
+     {                                                              \
+        local_irq_save(flags);                                      \
+        WRITE_SYSREG64(nvttbr, VTTBR_EL2);                          \
+        isb();                                                      \
+     }                                                              \
+})
+
+#define p2m_restore_vttbr_and_set_flags(ovttbr, flags)              \
+({                                                                  \
+     if ( ovttbr != READ_SYSREG64(VTTBR_EL2) )                      \
+     {                                                              \
+        WRITE_SYSREG64(ovttbr, VTTBR_EL2);                          \
+        isb();                                                      \
+        local_irq_restore(flags);                                   \
+     }                                                              \
+})
+
 /* Holds the bit size of IPAs in p2m tables.  */
 extern unsigned int p2m_ipa_bits;
 
-- 
2.13.3


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

Re: [PATCH v4 01/39] arm/p2m: Introduce p2m_(switch|restore)_vttbr_and_(g|s)et_flags

[Julien Grall]

Hi Sergej,

On 30/08/17 19:32, Sergej Proskurin wrote:
> This commit introduces macros for switching and restoring the vttbr
> considering the currently set irq flags. We define these macros, as the
> following commits will use the associated functionality multiple times
> throughout different files.
> 
> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> ---
> v4: Save the content of VTTBR_EL2 inside of the introduced macro
>      "p2m_switch_vttbr_and_get_flags".
> 
>      Move the introduced macros into ./xen/include/asm-arm/p2m.h, as they will
>      be used by different files in the future commits.

I don't like the idea of moving such macros in p2m.h because it expose 
the underlying implementation of P2M.

A better solution would be introduce a helper to translate from a guest 
VA to guest PA in p2m.c. This helper will switch to the host P2M if 
necessary.

Cheers,

-- 
Julien Grall

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

Re: [PATCH v4 01/39] arm/p2m: Introduce p2m_(switch|restore)_vttbr_and_(g|s)et_flags

[Sergej Proskurin]

Hi Julien,

please excuse me for the long delay.

On 10/09/2017 06:25 PM, Julien Grall wrote:
> Hi Sergej,
>
> On 30/08/17 19:32, Sergej Proskurin wrote:
>> This commit introduces macros for switching and restoring the vttbr
>> considering the currently set irq flags. We define these macros, as the
>> following commits will use the associated functionality multiple times
>> throughout different files.
>>
>> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
>> ---
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Cc: Julien Grall <julien.grall@arm.com>
>> ---
>> v4: Save the content of VTTBR_EL2 inside of the introduced macro
>>      "p2m_switch_vttbr_and_get_flags".
>>
>>      Move the introduced macros into ./xen/include/asm-arm/p2m.h, as
>> they will
>>      be used by different files in the future commits.
>
> I don't like the idea of moving such macros in p2m.h because it expose
> the underlying implementation of P2M.
>
> A better solution would be introduce a helper to translate from a
> guest VA to guest PA in p2m.c. This helper will switch to the host P2M
> if necessary.

I agree, thank you. This would eliminate the unnecessary macro
invocations in mem_access.c. I will include such wrapper in p2m.c in the
next version of the altp2m patch series.

Do you think it would make sense to submit the upper macros for saving
and restoring the VTTBR and flags as a standalone patch in form of a
cosmetic fix? This would simplify the function p2m_force_tlb_flush_sync
in p2m.c.

Thanks,
~Sergej


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

[PATCH v4 02/39] arm/p2m: Add first altp2m HVMOP stubs

[Sergej Proskurin]

This commit copies and extends the altp2m-related code from x86 to ARM.
Functions that are no yet supported notify the caller or print a BUG
message stating their absence.

Currently, we prohibit concurrent access of the altp2m interface by
locking the entire domain. As stated in the provided TODO statement,
future implementation should determine, which HVMOPs can be executed
concurrently.

Also, the struct arch_domain is extended with the altp2m_active
attribute, representing the current altp2m activity configuration of the
domain.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v2: Removed altp2m command-line option: Guard through HVM_PARAM_ALTP2M.
    Removed not used altp2m helper stubs in altp2m.h.

v3: Cosmetic fixes.

    Added domain lock in "do_altp2m_op" to avoid concurrent execution of
    altp2m-related HVMOPs.

    Added check making sure that HVM_PARAM_ALTP2M is set before
    execution of altp2m-related HVMOPs.

v4: Cosmetic fixes.

    Added a TODO proposing to determine, which HVMOPs can be executed
    concurrently instead of locking the entire domain and hence
    prohibiting concurrent access of the altp2m interface.

    Adjust to the current code base by explicitly checking whether
    altp2m is disabled.

    Change the type bool_t to bool of the field altp2m_active in struct
    arch_domain.
---
 xen/arch/arm/hvm.c           | 97 ++++++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/altp2m.h |  4 +-
 xen/include/asm-arm/domain.h |  3 ++
 3 files changed, 102 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index a56b3fe3fb..042bdda979 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -31,6 +31,99 @@
 
 #include <asm/hypercall.h>
 
+#include <asm/altp2m.h>
+
+static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
+{
+    struct xen_hvm_altp2m_op a;
+    struct domain *d = NULL;
+    uint64_t mode;
+    int rc = 0;
+
+    if ( copy_from_guest(&a, arg, 1) )
+        return -EFAULT;
+
+    if ( a.pad1 || a.pad2 ||
+         (a.version != HVMOP_ALTP2M_INTERFACE_VERSION) ||
+         (a.cmd < HVMOP_altp2m_get_domain_state) ||
+         (a.cmd > HVMOP_altp2m_change_gfn) )
+        return -EINVAL;
+
+    d = (a.cmd != HVMOP_altp2m_vcpu_enable_notify) ?
+        rcu_lock_domain_by_any_id(a.domain) : rcu_lock_current_domain();
+
+    if ( d == NULL )
+        return -ESRCH;
+
+    /*
+     * TODO: We prohibit concurrent access of the altp2m interface by locking
+     * the entire domain. Determine which HVMOPs can be executed concurrently.
+     */
+
+    /* Prevent concurrent execution of the following HVMOPs. */
+    domain_lock(d);
+
+    if ( (a.cmd != HVMOP_altp2m_get_domain_state) &&
+         (a.cmd != HVMOP_altp2m_set_domain_state) &&
+         !altp2m_active(d) )
+    {
+        rc = -EOPNOTSUPP;
+        goto out;
+    }
+
+    mode = d->arch.hvm_domain.params[HVM_PARAM_ALTP2M];
+
+    if ( XEN_ALTP2M_disabled == mode )
+    {
+        rc = -EINVAL;
+        goto out;
+    }
+
+    if ( (rc = xsm_hvm_altp2mhvm_op(XSM_OTHER, d, mode, a.cmd)) )
+        goto out;
+
+    switch ( a.cmd )
+    {
+    case HVMOP_altp2m_get_domain_state:
+        rc = -EOPNOTSUPP;
+        break;
+
+    case HVMOP_altp2m_set_domain_state:
+        rc = -EOPNOTSUPP;
+        break;
+
+    case HVMOP_altp2m_vcpu_enable_notify:
+        rc = -EOPNOTSUPP;
+        break;
+
+    case HVMOP_altp2m_create_p2m:
+        rc = -EOPNOTSUPP;
+        break;
+
+    case HVMOP_altp2m_destroy_p2m:
+        rc = -EOPNOTSUPP;
+        break;
+
+    case HVMOP_altp2m_switch_p2m:
+        rc = -EOPNOTSUPP;
+        break;
+
+    case HVMOP_altp2m_set_mem_access:
+        rc = -EOPNOTSUPP;
+        break;
+
+    case HVMOP_altp2m_change_gfn:
+        rc = -EOPNOTSUPP;
+        break;
+    }
+
+out:
+    domain_unlock(d);
+    rcu_unlock_domain(d);
+
+    return rc;
+}
+
 long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     long rc = 0;
@@ -79,6 +172,10 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
             rc = -EINVAL;
         break;
 
+    case HVMOP_altp2m:
+        rc = do_altp2m_op(arg);
+        break;
+
     default:
     {
         gdprintk(XENLOG_DEBUG, "HVMOP op=%lu: not implemented\n", op);
diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index a87747a291..0711796123 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -2,6 +2,7 @@
  * Alternate p2m
  *
  * Copyright (c) 2014, Intel Corporation.
+ * Copyright (c) 2016, Sergej Proskurin <proskurin@sec.in.tum.de>.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -24,8 +25,7 @@
 /* Alternate p2m on/off per domain */
 static inline bool_t altp2m_active(const struct domain *d)
 {
-    /* Not implemented on ARM. */
-    return 0;
+    return d->arch.altp2m_active;
 }
 
 /* Alternate p2m VCPU */
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 8dfc1d1ec2..0991a0a79d 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -145,6 +145,9 @@ struct arch_domain
     struct {
         uint8_t privileged_call_enabled : 1;
     } monitor;
+
+    /* altp2m: allow multiple copies of the host's p2m */
+    bool altp2m_active;
 }  __cacheline_aligned;
 
 struct arch_vcpu
-- 
2.13.3


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

Re: [PATCH v4 02/39] arm/p2m: Add first altp2m HVMOP stubs

[Julien Grall]

Hi Sergej,

On 30/08/17 19:32, Sergej Proskurin wrote:
> This commit copies and extends the altp2m-related code from x86 to ARM.
> Functions that are no yet supported notify the caller or print a BUG
> message stating their absence.

I am still concerned on the locking differing between x86 and Arm 
(likely the former is wrong) and for maintain POV in the future.

Last year you said you were working on getting do_altp2m_op common 
between x86 and Arm. What's the status?

> diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
> index a56b3fe3fb..042bdda979 100644
> --- a/xen/arch/arm/hvm.c
> +++ b/xen/arch/arm/hvm.c
> @@ -31,6 +31,99 @@
>   
>   #include <asm/hypercall.h>
>   
> +#include <asm/altp2m.h>
> +
> +static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
> +{
> +    struct xen_hvm_altp2m_op a;
> +    struct domain *d = NULL;
> +    uint64_t mode;
> +    int rc = 0;
> +
> +    if ( copy_from_guest(&a, arg, 1) )
> +        return -EFAULT;
> +
> +    if ( a.pad1 || a.pad2 ||
> +         (a.version != HVMOP_ALTP2M_INTERFACE_VERSION) ||
> +         (a.cmd < HVMOP_altp2m_get_domain_state) ||
> +         (a.cmd > HVMOP_altp2m_change_gfn) )

That exactly support my view above. You resent a series after a year and 
don't even look at what changed in x86.

I don't expect any better improvement as people will add more features 
in each side.

[...]

> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index 8dfc1d1ec2..0991a0a79d 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -145,6 +145,9 @@ struct arch_domain
>       struct {
>           uint8_t privileged_call_enabled : 1;
>       } monitor;
> +
> +    /* altp2m: allow multiple copies of the host's p2m */
> +    bool altp2m_active;

Does it have to be in arch_domain? Can't this be done in common code?

>   }  __cacheline_aligned;
>   
>   struct arch_vcpu
> 

Cheers,

-- 
Julien Grall

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

Re: [PATCH v4 02/39] arm/p2m: Add first altp2m HVMOP stubs

[Sergej Proskurin]

Hi Julien,


On 10/09/2017 06:43 PM, Julien Grall wrote:
> Hi Sergej,
>
> On 30/08/17 19:32, Sergej Proskurin wrote:
>> This commit copies and extends the altp2m-related code from x86 to ARM.
>> Functions that are no yet supported notify the caller or print a BUG
>> message stating their absence.
>
> I am still concerned on the locking differing between x86 and Arm
> (likely the former is wrong) and for maintain POV in the future.
>
> Last year you said you were working on getting do_altp2m_op common
> between x86 and Arm. What's the status?

I remember us having the discussion about pulling out common code of
both architectures. I still believe that this is necessary. Yet, as I
told you the last time, I really would like to first get the
implementation for ARM into mainline before blowing up this patch series
even more.

>
>> diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
>> index a56b3fe3fb..042bdda979 100644
>> --- a/xen/arch/arm/hvm.c
>> +++ b/xen/arch/arm/hvm.c
>> @@ -31,6 +31,99 @@
>>     #include <asm/hypercall.h>
>>   +#include <asm/altp2m.h>
>> +
>> +static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
>> +{
>> +    struct xen_hvm_altp2m_op a;
>> +    struct domain *d = NULL;
>> +    uint64_t mode;
>> +    int rc = 0;
>> +
>> +    if ( copy_from_guest(&a, arg, 1) )
>> +        return -EFAULT;
>> +
>> +    if ( a.pad1 || a.pad2 ||
>> +         (a.version != HVMOP_ALTP2M_INTERFACE_VERSION) ||
>> +         (a.cmd < HVMOP_altp2m_get_domain_state) ||
>> +         (a.cmd > HVMOP_altp2m_change_gfn) )
>
> That exactly support my view above. You resent a series after a year
> and don't even look at what changed in x86.
>
> I don't expect any better improvement as people will add more features
> in each side.
>
> [...]
>
>> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
>> index 8dfc1d1ec2..0991a0a79d 100644
>> --- a/xen/include/asm-arm/domain.h
>> +++ b/xen/include/asm-arm/domain.h
>> @@ -145,6 +145,9 @@ struct arch_domain
>>       struct {
>>           uint8_t privileged_call_enabled : 1;
>>       } monitor;
>> +
>> +    /* altp2m: allow multiple copies of the host's p2m */
>> +    bool altp2m_active;
>
> Does it have to be in arch_domain? Can't this be done in common code?

I will investigate how to best combine this flag between both
architectures and provide a suggestion in v5.

Thanks
~Sergej


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

Re: [PATCH v4 02/39] arm/p2m: Add first altp2m HVMOP stubs

[Julien Grall]

On 10/01/18 17:16, Sergej Proskurin wrote:
> Hi Julien,

Hi,

> 
> On 10/09/2017 06:43 PM, Julien Grall wrote:
>> Hi Sergej,
>>
>> On 30/08/17 19:32, Sergej Proskurin wrote:
>>> This commit copies and extends the altp2m-related code from x86 to ARM.
>>> Functions that are no yet supported notify the caller or print a BUG
>>> message stating their absence.
>>
>> I am still concerned on the locking differing between x86 and Arm
>> (likely the former is wrong) and for maintain POV in the future.
>>
>> Last year you said you were working on getting do_altp2m_op common
>> between x86 and Arm. What's the status?
> 
> I remember us having the discussion about pulling out common code of
> both architectures. I still believe that this is necessary. Yet, as I
> told you the last time, I really would like to first get the
> implementation for ARM into mainline before blowing up this patch series
> even more.

That's a no-go from my side from 2 reasons:
	1) You add burden in review. I have to make sure that every new code 
you add and make sure you don't get the locking wrong. Common code makes 
easier for that
	2) Most of features added to Arm that already coming from x86 are 
usually consolidated.

So to make clear, the current state:

NAcked-by: Julien Grall <julien.grall@linaro.org>

Cheers,

-- 
Julien Grall

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

[PATCH v4 03/39] arm/p2m: Add hvm_allow_(set|get)_param

[Sergej Proskurin]

This commit introduces the functions hvm_allow_(set|get)_param. These
can be used as a filter controlling access to HVM params. This
functionality has been inspired by the x86 implementation.

The introduced filter ensures that the HVM param HVM_PARAM_ALTP2M is set
once and not altered by guest domains.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/hvm.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 56 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index 042bdda979..6f5f9b41ac 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -124,6 +124,48 @@ out:
     return rc;
 }
 
+static int hvm_allow_set_param(struct domain *d, const struct xen_hvm_param *a)
+{
+    uint64_t value = d->arch.hvm_domain.params[a->index];
+    int rc;
+
+    rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_set_param);
+    if ( rc )
+        return rc;
+
+    switch ( a->index )
+    {
+    /* The following parameters should only be changed once. */
+    case HVM_PARAM_ALTP2M:
+        if ( value != 0 && a->value != value )
+            rc = -EEXIST;
+        break;
+    default:
+        break;
+    }
+
+    return rc;
+}
+
+static int hvm_allow_get_param(struct domain *d, const struct xen_hvm_param *a)
+{
+    int rc;
+
+    rc = xsm_hvm_param(XSM_TARGET, d, HVMOP_get_param);
+    if ( rc )
+        return rc;
+
+    switch ( a->index )
+    {
+        /* This switch statement can be used to control/limit guest access to
+         * certain HVM params. */
+    default:
+        break;
+    }
+
+    return rc;
+}
+
 long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     long rc = 0;
@@ -146,21 +188,26 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( d == NULL )
             return -ESRCH;
 
-        rc = xsm_hvm_param(XSM_TARGET, d, op);
-        if ( rc )
-            goto param_fail;
-
-        if ( op == HVMOP_set_param )
+        switch ( op )
         {
+        case HVMOP_set_param:
+            rc = hvm_allow_set_param(d, &a);
+            if ( rc )
+                break;
+
             d->arch.hvm_domain.params[a.index] = a.value;
-        }
-        else
-        {
+            break;
+
+        case HVMOP_get_param:
+            rc = hvm_allow_get_param(d, &a);
+            if ( rc )
+                break;
+
             a.value = d->arch.hvm_domain.params[a.index];
             rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
+            break;
         }
 
-    param_fail:
         rcu_unlock_domain(d);
         break;
     }
-- 
2.13.3


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

[PATCH v4 04/39] arm/p2m: Add HVMOP_altp2m_get_domain_state

[Sergej Proskurin]

This commit adopts the x86 HVMOP_altp2m_get_domain_state implementation.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v3: Removed the "altp2m_enabled" check in HVMOP_altp2m_get_domain_state
    case as it has been moved in front of the switch statement in
    "do_altp2m_op".

    Removed the macro "altp2m_enabled". Instead, check directly for the
    HVM_PARAM_ALTP2M param in d->arch.hvm_domain.
---
 xen/arch/arm/hvm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index 6f5f9b41ac..43b8352cb7 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -85,7 +85,8 @@ static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
     switch ( a.cmd )
     {
     case HVMOP_altp2m_get_domain_state:
-        rc = -EOPNOTSUPP;
+        a.u.domain_state.state = altp2m_active(d);
+        rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
         break;
 
     case HVMOP_altp2m_set_domain_state:
-- 
2.13.3


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

[PATCH v4 05/39] arm/p2m: Introduce p2m_is_(hostp2m|altp2m)

[Sergej Proskurin]

This commit adds a p2m class to the struct p2m_domain to distinguish
between the host's original p2m and alternate p2m's. The need for this
functionality will be shown in the following commits.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v4: Change return type of p2m_is_(hostp2m|altp2m) from bool_t to bool.
---
 xen/include/asm-arm/p2m.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 500dc88fbc..332d74f11c 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -40,6 +40,11 @@ struct domain;
 
 extern void memory_type_changed(struct domain *);
 
+typedef enum {
+    p2m_host,
+    p2m_alternate,
+} p2m_class_t;
+
 /* Per-p2m-table state */
 struct p2m_domain {
     /*
@@ -126,6 +131,9 @@ struct p2m_domain {
 
     /* Keeping track on which CPU this p2m was used and for which vCPU */
     uint8_t last_vcpu_ran[NR_CPUS];
+
+    /* Choose between: host/alternate. */
+    p2m_class_t p2m_class;
 };
 
 /*
@@ -359,6 +367,16 @@ static inline int get_page_and_type(struct page_info *page,
 /* get host p2m table */
 #define p2m_get_hostp2m(d) (&(d)->arch.p2m)
 
+static inline bool p2m_is_hostp2m(const struct p2m_domain *p2m)
+{
+    return p2m->p2m_class == p2m_host;
+}
+
+static inline bool p2m_is_altp2m(const struct p2m_domain *p2m)
+{
+    return p2m->p2m_class == p2m_alternate;
+}
+
 static inline bool_t p2m_vm_event_sanity_check(struct domain *d)
 {
     return 1;
-- 
2.13.3


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

[PATCH v4 06/39] arm/p2m: Cosmetic fix - substitute _gfn(ULONG_MAX) for INVALID_GFN

[Sergej Proskurin]

In ./xen/arch/arm/p2m.c, we compare the gfn's with INVALID_GFN
throughout the code. Thus it makes sense to use the macro INVALID_GFN
instead of a hard coded value to initialize "p2m->lowest_mapped_gfn".

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/p2m.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 4334e3bc81..5e86368010 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1238,7 +1238,7 @@ int p2m_init(struct domain *d)
 
     p2m->domain = d;
     p2m->max_mapped_gfn = _gfn(0);
-    p2m->lowest_mapped_gfn = _gfn(ULONG_MAX);
+    p2m->lowest_mapped_gfn = INVALID_GFN;
 
     p2m->default_access = p2m_access_rwx;
     p2m->mem_access_enabled = false;
-- 
2.13.3


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

[PATCH v4 07/39] arm/p2m: Move hostp2m init/teardown to individual functions

[Sergej Proskurin]

This commit pulls out generic init/teardown functionality out of
"p2m_init" and "p2m_teardown" into "p2m_init_one", "p2m_teardown_one",
and "p2m_flush_table" functions.  This allows our future implementation
to reuse existing code for the initialization/teardown of altp2m views.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v2: Added the function p2m_flush_table to the previous version.

v3: Removed struct vttbr.

    Moved define INVALID_VTTBR to p2m.h.

    Exported function prototypes of "p2m_flush_table", "p2m_init_one",
    and "p2m_teardown_one" in p2m.h.

    Extended the function "p2m_flush_table" by additionally resetting
    the fields lowest_mapped_gfn and max_mapped_gfn.

    Added a "p2m_flush_tlb" call in "p2m_flush_table". On altp2m reset
    in function "altp2m_reset", it is important to flush the TLBs after
    clearing the root table pages and before clearing the intermediate
    altp2m page tables to prevent illegal access to stalled TLB entries
    on currently active VCPUs.

    Added a check checking whether p2m->root is NULL in p2m_flush_table.

    Renamed the function "p2m_free_one" to "p2m_teardown_one".

    Removed resetting p2m->vttbr in "p2m_teardown_one", as it the p2m
    will be destroyed afterwards.

    Moved call to "p2m_alloc_table" back to "p2m_init_one".

    Moved the introduction of the type p2m_class_t out of this patch.

    Moved the backpointer to the struct domain out of the struct
    p2m_domain.

v4: Replaced the former use of clear_and_clean_page in p2m_flush_table
    by a routine that invalidates every p2m entry atomically. This
    avoids inconsistencies on CPUs that continue to use the views that
    are to be flushed (e.g., see altp2m_reset).

    Removed unnecessary initializations in the functions "p2m_init_one"
    and "p2m_teardown_one".

    Removed the define INVALID_VTTBR as it is not used any more.

    Cosmetic fixes.
---
 xen/arch/arm/p2m.c        | 74 +++++++++++++++++++++++++++++++++++++++++++----
 xen/include/asm-arm/p2m.h |  9 ++++++
 2 files changed, 78 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 5e86368010..3a1a38e7af 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1203,27 +1203,65 @@ static void p2m_free_vmid(struct domain *d)
     spin_unlock(&vmid_alloc_lock);
 }
 
-void p2m_teardown(struct domain *d)
+/* Reset this p2m table to be empty. */
+void p2m_flush_table(struct p2m_domain *p2m)
 {
-    struct p2m_domain *p2m = p2m_get_hostp2m(d);
     struct page_info *pg;
+    unsigned int i, j;
+    lpae_t *table;
+
+    if ( p2m->root )
+    {
+        /* Clear all concatenated root level pages. */
+        for ( i = 0; i < P2M_ROOT_PAGES; i++ )
+        {
+            table = __map_domain_page(p2m->root + i);
+
+            for ( j = 0; j < LPAE_ENTRIES; j++ )
+            {
+                lpae_t *entry = table + j;
+
+                /*
+                 * Individual altp2m views can be flushed, whilst altp2m is
+                 * active. To avoid inconsistencies on CPUs that continue to
+                 * use the views to be flushed (e.g., see altp2m_reset), we
+                 * must remove every p2m entry atomically.
+                 */
+                p2m_remove_pte(entry, p2m->clean_pte);
+            }
+        }
+    }
+
+    /*
+     * Flush TLBs before releasing remaining intermediate p2m page tables to
+     * prevent illegal access to stalled TLB entries.
+     */
+    p2m_flush_tlb(p2m);
 
+    /* Free the rest of the trie pages back to the paging pool. */
     while ( (pg = page_list_remove_head(&p2m->pages)) )
         free_domheap_page(pg);
 
+    p2m->lowest_mapped_gfn = INVALID_GFN;
+    p2m->max_mapped_gfn = _gfn(0);
+}
+
+void p2m_teardown_one(struct p2m_domain *p2m)
+{
+    p2m_flush_table(p2m);
+
     if ( p2m->root )
         free_domheap_pages(p2m->root, P2M_ROOT_ORDER);
 
     p2m->root = NULL;
 
-    p2m_free_vmid(d);
+    p2m_free_vmid(p2m->domain);
 
     radix_tree_destroy(&p2m->mem_access_settings, NULL);
 }
 
-int p2m_init(struct domain *d)
+int p2m_init_one(struct domain *d, struct p2m_domain *p2m)
 {
-    struct p2m_domain *p2m = p2m_get_hostp2m(d);
     int rc = 0;
     unsigned int cpu;
 
@@ -1268,6 +1306,32 @@ int p2m_init(struct domain *d)
     return rc;
 }
 
+static void p2m_teardown_hostp2m(struct domain *d)
+{
+    struct p2m_domain *p2m = p2m_get_hostp2m(d);
+
+    p2m_teardown_one(p2m);
+}
+
+void p2m_teardown(struct domain *d)
+{
+    p2m_teardown_hostp2m(d);
+}
+
+static int p2m_init_hostp2m(struct domain *d)
+{
+    struct p2m_domain *p2m = p2m_get_hostp2m(d);
+
+    p2m->p2m_class = p2m_host;
+
+    return p2m_init_one(d, p2m);
+}
+
+int p2m_init(struct domain *d)
+{
+    return p2m_init_hostp2m(d);
+}
+
 /*
  * The function will go through the p2m and remove page reference when it
  * is required. The mapping will be removed from the p2m.
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 332d74f11c..9bb38e689a 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -295,6 +295,15 @@ static inline int guest_physmap_add_page(struct domain *d,
 
 mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn);
 
+/* Flushes the page table held by the p2m. */
+void p2m_flush_table(struct p2m_domain *p2m);
+
+/* Initialize the p2m structure. */
+int p2m_init_one(struct domain *d, struct p2m_domain *p2m);
+
+/* Release resources held by the p2m structure. */
+void p2m_teardown_one(struct p2m_domain *p2m);
+
 /*
  * Populate-on-demand
  */
-- 
2.13.3


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

Re: [PATCH v4 07/39] arm/p2m: Move hostp2m init/teardown to individual functions

[Julien Grall]

Hi Sergej,

On 30/08/17 19:32, Sergej Proskurin wrote:
> This commit pulls out generic init/teardown functionality out of
> "p2m_init" and "p2m_teardown" into "p2m_init_one", "p2m_teardown_one",
> and "p2m_flush_table" functions.  This allows our future implementation
> to reuse existing code for the initialization/teardown of altp2m views.

You likely want to expand the commit message here to explain:
	1) Why you export the functions
	2) How come you split p2m_teardown is now also flushing the tables...

Very likely 2) should be a separate patch as it is an addition of the 
code and not a split.

> 
> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> ---
> v2: Added the function p2m_flush_table to the previous version.
> 
> v3: Removed struct vttbr.
> 
>      Moved define INVALID_VTTBR to p2m.h.
> 
>      Exported function prototypes of "p2m_flush_table", "p2m_init_one",
>      and "p2m_teardown_one" in p2m.h.
> 
>      Extended the function "p2m_flush_table" by additionally resetting
>      the fields lowest_mapped_gfn and max_mapped_gfn.
> 
>      Added a "p2m_flush_tlb" call in "p2m_flush_table". On altp2m reset
>      in function "altp2m_reset", it is important to flush the TLBs after
>      clearing the root table pages and before clearing the intermediate
>      altp2m page tables to prevent illegal access to stalled TLB entries
>      on currently active VCPUs.
> 
>      Added a check checking whether p2m->root is NULL in p2m_flush_table.
> 
>      Renamed the function "p2m_free_one" to "p2m_teardown_one".
> 
>      Removed resetting p2m->vttbr in "p2m_teardown_one", as it the p2m
>      will be destroyed afterwards.
> 
>      Moved call to "p2m_alloc_table" back to "p2m_init_one".
> 
>      Moved the introduction of the type p2m_class_t out of this patch.
> 
>      Moved the backpointer to the struct domain out of the struct
>      p2m_domain.
> 
> v4: Replaced the former use of clear_and_clean_page in p2m_flush_table
>      by a routine that invalidates every p2m entry atomically. This
>      avoids inconsistencies on CPUs that continue to use the views that
>      are to be flushed (e.g., see altp2m_reset).
> 
>      Removed unnecessary initializations in the functions "p2m_init_one"
>      and "p2m_teardown_one".
> 
>      Removed the define INVALID_VTTBR as it is not used any more.
> 
>      Cosmetic fixes.
> ---
>   xen/arch/arm/p2m.c        | 74 +++++++++++++++++++++++++++++++++++++++++++----
>   xen/include/asm-arm/p2m.h |  9 ++++++
>   2 files changed, 78 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 5e86368010..3a1a38e7af 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1203,27 +1203,65 @@ static void p2m_free_vmid(struct domain *d)
>       spin_unlock(&vmid_alloc_lock);
>   }
>   
> -void p2m_teardown(struct domain *d)
> +/* Reset this p2m table to be empty. */
> +void p2m_flush_table(struct p2m_domain *p2m)
>   {
> -    struct p2m_domain *p2m = p2m_get_hostp2m(d);
>       struct page_info *pg;
> +    unsigned int i, j;
> +    lpae_t *table;
> +
> +    if ( p2m->root )
> +    {
> +        /* Clear all concatenated root level pages. */
> +        for ( i = 0; i < P2M_ROOT_PAGES; i++ )
> +        {
> +            table = __map_domain_page(p2m->root + i);
> +
> +            for ( j = 0; j < LPAE_ENTRIES; j++ )
> +            {
> +                lpae_t *entry = table + j;
> +
> +                /*
> +                 * Individual altp2m views can be flushed, whilst altp2m is
> +                 * active. To avoid inconsistencies on CPUs that continue to
> +                 * use the views to be flushed (e.g., see altp2m_reset), we
> +                 * must remove every p2m entry atomically.

When I read this comment, I wonder how this is safe to clear without any 
locking? At least to prevent multiple instance to modify the P2M at the 
same time. If you expect the caller to do it for you, then an 
ASSERT(...) is necessary at the beginning of the function.

Likely you want this function do the locking.

Also that comment is more suitable on top of the for loop rather than here.

> +                 */
> +                p2m_remove_pte(entry, p2m->clean_pte);
> +            }
> +        }

You still haven't address my comment regarding the overhead you 
introduce whilst tearing down a P2M (e.g when the domain is destroyed).

> +    }
> +
> +    /*
> +     * Flush TLBs before releasing remaining intermediate p2m page tables to
> +     * prevent illegal access to stalled TLB entries.
> +     */
> +    p2m_flush_tlb(p2m);

Again, this is called by p2m_flush_table where the P2M may not have been 
allocated because the initialization failed. So trying to flush TLB may 
lead to a panic in Xen (the vttbr is invalid).

Furthermore we already flush the TLBs when creating the domain (see 
p2m_alloc_table). So you add yet another overhead.

>   
> +    /* Free the rest of the trie pages back to the paging pool. */
>       while ( (pg = page_list_remove_head(&p2m->pages)) )
>           free_domheap_page(pg);
>   
> +    p2m->lowest_mapped_gfn = INVALID_GFN;
> +    p2m->max_mapped_gfn = _gfn(0);
> +}
> +
> +void p2m_teardown_one(struct p2m_domain *p2m)
> +{
> +    p2m_flush_table(p2m);
> +
>       if ( p2m->root )
>           free_domheap_pages(p2m->root, P2M_ROOT_ORDER);
>   
>       p2m->root = NULL;
>   
> -    p2m_free_vmid(d);
> +    p2m_free_vmid(p2m->domain);

This is a bit odd to read given the VMID is per P2M. Likely you want 
your patches #9 and #10 before this patch.

>   
>       radix_tree_destroy(&p2m->mem_access_settings, NULL);
>   }
>   
> -int p2m_init(struct domain *d)
> +int p2m_init_one(struct domain *d, struct p2m_domain *p2m)
>   {
> -    struct p2m_domain *p2m = p2m_get_hostp2m(d);
>       int rc = 0;
>       unsigned int cpu;
>   
> @@ -1268,6 +1306,32 @@ int p2m_init(struct domain *d)
>       return rc;
>   }
>   
> +static void p2m_teardown_hostp2m(struct domain *d)
> +{
> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +
> +    p2m_teardown_one(p2m);
> +}
> +
> +void p2m_teardown(struct domain *d)
> +{
> +    p2m_teardown_hostp2m(d);
> +}
> +
> +static int p2m_init_hostp2m(struct domain *d)
> +{
> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +
> +    p2m->p2m_class = p2m_host;
> +
> +    return p2m_init_one(d, p2m);
> +}
> +
> +int p2m_init(struct domain *d)
> +{
> +    return p2m_init_hostp2m(d);
> +}

Please explain in the commit message why you need to introduce 
p2m_init/p2m_teardown that just call p2m_init_one/p2m_teardown_hostp2m.

> +
>   /*
>    * The function will go through the p2m and remove page reference when it
>    * is required. The mapping will be removed from the p2m.
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index 332d74f11c..9bb38e689a 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -295,6 +295,15 @@ static inline int guest_physmap_add_page(struct domain *d,
>   
>   mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn);
>   
> +/* Flushes the page table held by the p2m. */
> +void p2m_flush_table(struct p2m_domain *p2m);
> +
> +/* Initialize the p2m structure. */
> +int p2m_init_one(struct domain *d, struct p2m_domain *p2m);
> +
> +/* Release resources held by the p2m structure. */
> +void p2m_teardown_one(struct p2m_domain *p2m);
> +
>   /*
>    * Populate-on-demand
>    */
> 

Cheers,

-- 
Julien Grall

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

Re: [PATCH v4 07/39] arm/p2m: Move hostp2m init/teardown to individual functions

[Sergej Proskurin]

Hi Julien,


On 10/09/2017 07:15 PM, Julien Grall wrote:
> Hi Sergej,
>

[...]

>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>> index 5e86368010..3a1a38e7af 100644
>> --- a/xen/arch/arm/p2m.c
>> +++ b/xen/arch/arm/p2m.c
>> @@ -1203,27 +1203,65 @@ static void p2m_free_vmid(struct domain *d)
>>       spin_unlock(&vmid_alloc_lock);
>>   }
>>   -void p2m_teardown(struct domain *d)
>> +/* Reset this p2m table to be empty. */
>> +void p2m_flush_table(struct p2m_domain *p2m)
>>   {
>> -    struct p2m_domain *p2m = p2m_get_hostp2m(d);
>>       struct page_info *pg;
>> +    unsigned int i, j;
>> +    lpae_t *table;
>> +
>> +    if ( p2m->root )
>> +    {
>> +        /* Clear all concatenated root level pages. */
>> +        for ( i = 0; i < P2M_ROOT_PAGES; i++ )
>> +        {
>> +            table = __map_domain_page(p2m->root + i);
>> +
>> +            for ( j = 0; j < LPAE_ENTRIES; j++ )
>> +            {
>> +                lpae_t *entry = table + j;
>> +
>> +                /*
>> +                 * Individual altp2m views can be flushed, whilst
>> altp2m is
>> +                 * active. To avoid inconsistencies on CPUs that
>> continue to
>> +                 * use the views to be flushed (e.g., see
>> altp2m_reset), we
>> +                 * must remove every p2m entry atomically.
>
> When I read this comment, I wonder how this is safe to clear without
> any locking? At least to prevent multiple instance to modify the P2M
> at the same time. If you expect the caller to do it for you, then an
> ASSERT(...) is necessary at the beginning of the function.
>
> Likely you want this function do the locking.

You are correct, I was assuming the caller to lock the associated p2m.
Because of the fact that it is only the function "altp2m_reset" in
altp2m.c that flushes the p2m's without the purpose of subsequently
destroying it, I believe we should leave locking to the caller and (as
you mentioned above) use an ASSERT to ensure this behavior. Otherwise,
we would lock the p2m's everytime even if they are about to be destroyed
(as p2m_flush_table is called by p2m_teardown_one). I remember you
saying (and I agree at this point) that we should not do that as this
would be wasted cycles. Yet, if you should insist on locking the p2m
inside of p2m_flush_table, I will gladly incorporate the locking into
the upper function.

>
> Also that comment is more suitable on top of the for loop rather than
> here.

Ok, thanks.

>
>> +                 */
>> +                p2m_remove_pte(entry, p2m->clean_pte);
>> +            }
>> +        }
>
> You still haven't address my comment regarding the overhead you
> introduce whilst tearing down a P2M (e.g when the domain is destroyed).
>

I believe you are referring to the following comments:

> Also, this adds a small overhead when tearing down a p2m because the
> clear is not necessary. 

and

> You seem to forget the p2m teardown is also called during domain
> destruction.

Sorry, I must have missed it. To increase performance, at this point, we
could either (i) differentiate between altp2m views and the hostp2m (as
the hostp2m does not have to be cleared before it gets freed) or (ii) we
could introduce another function (e.g. p2m_clear_table) that is
responsible for doing just that. In this way, we could call
p2m_clear_table from altp2m_reset (the only place where we actually need
to clear altp2m views without subsequently destroying them) would not
need to clear the p2m's everytime we destroy one. Also, we sould need to
lock the p2m only in p2m_clear_table and not inside of p2m_flush_table
or one of its caller. What would you prefer?

>> +    }
>> +
>> +    /*
>> +     * Flush TLBs before releasing remaining intermediate p2m page
>> tables to
>> +     * prevent illegal access to stalled TLB entries.
>> +     */
>> +    p2m_flush_tlb(p2m);
>
> Again, this is called by p2m_flush_table where the P2M may not have
> been allocated because the initialization failed. So trying to flush
> TLB may lead to a panic in Xen (the vttbr is invalid).
>
> Furthermore we already flush the TLBs when creating the domain (see
> p2m_alloc_table). So you add yet another overhead.
>

Right. Yet, we must flush the TLBs after clearing (resetting the altp2m
views through altp2m_reset) the views. That is, if you should be ok with
my suggestion above (introducing p2m_clear_table), we could limit
flushing the TLBs only to the code that actually resets the views.

>>   +    /* Free the rest of the trie pages back to the paging pool. */
>>       while ( (pg = page_list_remove_head(&p2m->pages)) )
>>           free_domheap_page(pg);
>>   +    p2m->lowest_mapped_gfn = INVALID_GFN;
>> +    p2m->max_mapped_gfn = _gfn(0);
>> +}
>> +
>> +void p2m_teardown_one(struct p2m_domain *p2m)
>> +{
>> +    p2m_flush_table(p2m);
>> +
>>       if ( p2m->root )
>>           free_domheap_pages(p2m->root, P2M_ROOT_ORDER);
>>         p2m->root = NULL;
>>   -    p2m_free_vmid(d);
>> +    p2m_free_vmid(p2m->domain);
>
> This is a bit odd to read given the VMID is per P2M. Likely you want
> your patches #9 and #10 before this patch.

Ok, thank you.

>
>>         radix_tree_destroy(&p2m->mem_access_settings, NULL);
>>   }
>>   -int p2m_init(struct domain *d)
>> +int p2m_init_one(struct domain *d, struct p2m_domain *p2m)
>>   {
>> -    struct p2m_domain *p2m = p2m_get_hostp2m(d);
>>       int rc = 0;
>>       unsigned int cpu;
>>   @@ -1268,6 +1306,32 @@ int p2m_init(struct domain *d)
>>       return rc;
>>   }
>>   +static void p2m_teardown_hostp2m(struct domain *d)
>> +{
>> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
>> +
>> +    p2m_teardown_one(p2m);
>> +}
>> +
>> +void p2m_teardown(struct domain *d)
>> +{
>> +    p2m_teardown_hostp2m(d);
>> +}
>> +
>> +static int p2m_init_hostp2m(struct domain *d)
>> +{
>> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
>> +
>> +    p2m->p2m_class = p2m_host;
>> +
>> +    return p2m_init_one(d, p2m);
>> +}
>> +
>> +int p2m_init(struct domain *d)
>> +{
>> +    return p2m_init_hostp2m(d);
>> +}
>
> Please explain in the commit message why you need to introduce
> p2m_init/p2m_teardown that just call p2m_init_one/p2m_teardown_hostp2m.

I will extend the commit message by stating that we will fill p2m_init
in one of the future commits by adding an altp2m initialization routine.
Thanks.

>
>> +
>>   /*
>>    * The function will go through the p2m and remove page reference
>> when it
>>    * is required. The mapping will be removed from the p2m.
>> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
>> index 332d74f11c..9bb38e689a 100644
>> --- a/xen/include/asm-arm/p2m.h
>> +++ b/xen/include/asm-arm/p2m.h
>> @@ -295,6 +295,15 @@ static inline int guest_physmap_add_page(struct
>> domain *d,
>>     mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn);
>>   +/* Flushes the page table held by the p2m. */
>> +void p2m_flush_table(struct p2m_domain *p2m);
>> +
>> +/* Initialize the p2m structure. */
>> +int p2m_init_one(struct domain *d, struct p2m_domain *p2m);
>> +
>> +/* Release resources held by the p2m structure. */
>> +void p2m_teardown_one(struct p2m_domain *p2m);
>> +
>>   /*
>>    * Populate-on-demand
>>    */
>>

Thanks,
~Sergej


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

[PATCH v4 08/39] arm/p2m: Cosmetic fix - function prototype of p2m_alloc_table

[Sergej Proskurin]

The function "p2m_alloc_table" should be able to allocate 2nd stage
translation tables not only for the host's p2m but also for alternate
p2m's.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v2: Removed altp2m table initialization from "p2m_table_init".

v3: Removed initialization of the field d->arch.altp2m_active in
    "p2m_table_init" to avoid altp2m initialization throughout different
    files.

    Merged the function "p2m_alloc_table" and "p2m_table_init".
---
 xen/arch/arm/p2m.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 3a1a38e7af..65dd2772bf 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1112,9 +1112,8 @@ int guest_physmap_remove_page(struct domain *d, gfn_t gfn, mfn_t mfn,
     return p2m_remove_mapping(d, gfn, (1 << page_order), mfn);
 }
 
-static int p2m_alloc_table(struct domain *d)
+static int p2m_alloc_table(struct p2m_domain *p2m)
 {
-    struct p2m_domain *p2m = p2m_get_hostp2m(d);
     struct page_info *page;
     unsigned int i;
 
@@ -1290,7 +1289,7 @@ int p2m_init_one(struct domain *d, struct p2m_domain *p2m)
     p2m->clean_pte = iommu_enabled &&
         !iommu_has_feature(d, IOMMU_FEAT_COHERENT_WALK);
 
-    rc = p2m_alloc_table(d);
+    rc = p2m_alloc_table(p2m);
 
     /*
      * Make sure that the type chosen to is able to store the an vCPU ID
-- 
2.13.3


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

[PATCH v4 09/39] arm/p2m: Rename parameter in p2m_alloc_vmid

[Sergej Proskurin]

This commit does not change or introduce any additional functionality
but rather is a part of the following commit that alters the
functionality of the function "p2m_alloc_vmid".

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/p2m.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 65dd2772bf..808d99e1e9 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1166,24 +1166,24 @@ static int p2m_alloc_vmid(struct domain *d)
 {
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
 
-    int rc, nr;
+    int rc, vmid;
 
     spin_lock(&vmid_alloc_lock);
 
-    nr = find_first_zero_bit(vmid_mask, MAX_VMID);
+    vmid = find_first_zero_bit(vmid_mask, MAX_VMID);
 
-    ASSERT(nr != INVALID_VMID);
+    ASSERT(vmid != INVALID_VMID);
 
-    if ( nr == MAX_VMID )
+    if ( vmid == MAX_VMID )
     {
         rc = -EBUSY;
         printk(XENLOG_ERR "p2m.c: dom%d: VMID pool exhausted\n", d->domain_id);
         goto out;
     }
 
-    set_bit(nr, vmid_mask);
+    set_bit(vmid, vmid_mask);
 
-    p2m->vmid = nr;
+    p2m->vmid = vmid;
 
     rc = 0;
 
-- 
2.13.3


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

[PATCH v4 10/39] arm/p2m: Change func prototype and impl of p2m_(alloc|free)_vmid

[Sergej Proskurin]

This commit changes the prototype and implementation of the functions
"p2m_alloc_vmid" and "p2m_free_vmid". The function "p2m_alloc_vmid" does
not expect the struct domain as argument anymore and returns an
allocated vmid. The function "p2m_free_vmid" takes only the vmid that is
to be freed as argument.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v3: Changed function prototypes and implementation of the functions
    "p2m_alloc_vmid" and "p2m_free_vmid".

    Changes in "p2m_alloc_vmid":
    This function does not expect any arguments. Also, in this commit,
    the function "p2m_alloc_vmid" returns either the successfully
    allocated vmid or the value INVALID_VMID. Thus, it is now the
    responsibility of the caller to set the returned vmid in the
    associated fields.

    Changes in "p2m_free_vmid":
    This function expects now only the vmid of type uint8_t.
---
 xen/arch/arm/p2m.c | 33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 808d99e1e9..ec855341b9 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1162,11 +1162,9 @@ static void p2m_vmid_allocator_init(void)
     set_bit(INVALID_VMID, vmid_mask);
 }
 
-static int p2m_alloc_vmid(struct domain *d)
+static uint8_t p2m_alloc_vmid(void)
 {
-    struct p2m_domain *p2m = p2m_get_hostp2m(d);
-
-    int rc, vmid;
+    uint8_t vmid;
 
     spin_lock(&vmid_alloc_lock);
 
@@ -1176,28 +1174,23 @@ static int p2m_alloc_vmid(struct domain *d)
 
     if ( vmid == MAX_VMID )
     {
-        rc = -EBUSY;
-        printk(XENLOG_ERR "p2m.c: dom%d: VMID pool exhausted\n", d->domain_id);
+        vmid = INVALID_VMID;
+        printk(XENLOG_ERR "p2m.c: VMID pool exhausted\n");
         goto out;
     }
 
     set_bit(vmid, vmid_mask);
 
-    p2m->vmid = vmid;
-
-    rc = 0;
-
 out:
     spin_unlock(&vmid_alloc_lock);
-    return rc;
+    return vmid;
 }
 
-static void p2m_free_vmid(struct domain *d)
+static void p2m_free_vmid(uint8_t vmid)
 {
-    struct p2m_domain *p2m = p2m_get_hostp2m(d);
     spin_lock(&vmid_alloc_lock);
-    if ( p2m->vmid != INVALID_VMID )
-        clear_bit(p2m->vmid, vmid_mask);
+    if ( vmid != INVALID_VMID )
+        clear_bit(vmid, vmid_mask);
 
     spin_unlock(&vmid_alloc_lock);
 }
@@ -1254,7 +1247,7 @@ void p2m_teardown_one(struct p2m_domain *p2m)
 
     p2m->root = NULL;
 
-    p2m_free_vmid(p2m->domain);
+    p2m_free_vmid(p2m->vmid);
 
     radix_tree_destroy(&p2m->mem_access_settings, NULL);
 }
@@ -1267,11 +1260,9 @@ int p2m_init_one(struct domain *d, struct p2m_domain *p2m)
     rwlock_init(&p2m->lock);
     INIT_PAGE_LIST_HEAD(&p2m->pages);
 
-    p2m->vmid = INVALID_VMID;
-
-    rc = p2m_alloc_vmid(d);
-    if ( rc != 0 )
-        return rc;
+    p2m->vmid = p2m_alloc_vmid();
+    if ( p2m->vmid == INVALID_VMID )
+        return -EBUSY;
 
     p2m->domain = d;
     p2m->max_mapped_gfn = _gfn(0);
-- 
2.13.3


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

[PATCH v4 11/39] altp2m: Move (MAX|INVALID)_ALTP2M to xen/p2m-common.h

[Sergej Proskurin]

We move the macros (MAX|INVALID)_ALTP2M out of x86-related code to
common code, as the following patches will make use of them on ARM.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Julien Grall <julien.grall@arm.com>
---
v4: We have introduced this patch to our patch series.
---
 xen/include/asm-arm/altp2m.h    | 1 +
 xen/include/asm-x86/domain.h    | 3 +--
 xen/include/xen/altp2m-common.h | 8 ++++++++
 3 files changed, 10 insertions(+), 2 deletions(-)
 create mode 100644 xen/include/xen/altp2m-common.h

diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index 0711796123..66afa959f6 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -20,6 +20,7 @@
 #ifndef __ASM_ARM_ALTP2M_H
 #define __ASM_ARM_ALTP2M_H
 
+#include <xen/altp2m-common.h>
 #include <xen/sched.h>
 
 /* Alternate p2m on/off per domain */
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index fb8bf17458..1d10f4b59f 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -1,6 +1,7 @@
 #ifndef __ASM_DOMAIN_H__
 #define __ASM_DOMAIN_H__
 
+#include <xen/altp2m-common.h>
 #include <xen/mm.h>
 #include <xen/radix-tree.h>
 #include <asm/hvm/vcpu.h>
@@ -234,8 +235,6 @@ struct paging_vcpu {
 
 #define MAX_NESTEDP2M 10
 
-#define MAX_ALTP2M      10 /* arbitrary */
-#define INVALID_ALTP2M  0xffff
 #define MAX_EPTP        (PAGE_SIZE / sizeof(uint64_t))
 struct p2m_domain;
 struct time_scale {
diff --git a/xen/include/xen/altp2m-common.h b/xen/include/xen/altp2m-common.h
new file mode 100644
index 0000000000..670fb42292
--- /dev/null
+++ b/xen/include/xen/altp2m-common.h
@@ -0,0 +1,8 @@
+#ifndef __XEN_ALTP2M_COMMON_H__
+#define __XEN_ALTP2M_COMMON_H__
+
+#define MAX_ALTP2M      10      /* The system may contain an arbitrary number
+                                   of altp2m views. */
+#define INVALID_ALTP2M  0xffff
+
+#endif /* __XEN_ALTP2M_COMMON_H__ */
-- 
2.13.3


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

Re: [PATCH v4 11/39] altp2m: Move (MAX|INVALID)_ALTP2M to xen/p2m-common.h

[Jan Beulich]

>>> On 30.08.17 at 20:32, <proskurin@sec.in.tum.de> wrote:
> We move the macros (MAX|INVALID)_ALTP2M out of x86-related code to
> common code, as the following patches will make use of them on ARM.

But both seem not impossible to be require arch-specific values.

Jan


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

Re: [PATCH v4 11/39] altp2m: Move (MAX|INVALID)_ALTP2M to xen/p2m-common.h

[Sergej Proskurin]

Hi Jan,


On 08/31/2017 10:04 AM, Jan Beulich wrote:
>>>> On 30.08.17 at 20:32, <proskurin@sec.in.tum.de> wrote:
>> We move the macros (MAX|INVALID)_ALTP2M out of x86-related code to
>> common code, as the following patches will make use of them on ARM.
> But both seem not impossible to be require arch-specific values.

Right. The general idea at this point is to move as much of altp2m
functionality/configuration as possible into a common place. Yet, if you
believe that, e.g., the number of altp2m views could/should diverge
between both architectures, I will gladly move the defines back into
arch-related parts. However, we need to consider that while x86/Intel
supports up to 512 entries for EPT pointers as part of the VMCS, we are
quite flexible on ARM: we manage the views entirely in software and
hence on ARM we can easily keep up with Intel's specification. This
allows us to hold parts of the altp2m configuration in a unified place.
Or do you believe this is not the right way to go?

Thanks,
~Sergej


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

Re: [PATCH v4 11/39] altp2m: Move (MAX|INVALID)_ALTP2M to xen/p2m-common.h

[Jan Beulich]

>>> On 31.08.17 at 11:49, <proskurin@sec.in.tum.de> wrote:
> On 08/31/2017 10:04 AM, Jan Beulich wrote:
>>>>> On 30.08.17 at 20:32, <proskurin@sec.in.tum.de> wrote:
>>> We move the macros (MAX|INVALID)_ALTP2M out of x86-related code to
>>> common code, as the following patches will make use of them on ARM.
>> But both seem not impossible to be require arch-specific values.
> 
> Right. The general idea at this point is to move as much of altp2m
> functionality/configuration as possible into a common place. Yet, if you
> believe that, e.g., the number of altp2m views could/should diverge
> between both architectures, I will gladly move the defines back into
> arch-related parts. However, we need to consider that while x86/Intel
> supports up to 512 entries for EPT pointers as part of the VMCS, we are
> quite flexible on ARM: we manage the views entirely in software and
> hence on ARM we can easily keep up with Intel's specification. This
> allows us to hold parts of the altp2m configuration in a unified place.
> Or do you believe this is not the right way to go?

Well, you've basically answered this yourself: Why would you
want to constrain ARM just because of VMX restrictions? Requiring
all architectures to surface the same constants (regardless of
actual values) is all you need to be able to commonize code.

Jan


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

Re: [PATCH v4 11/39] altp2m: Move (MAX|INVALID)_ALTP2M to xen/p2m-common.h

[Sergej Proskurin]

Hi Jan,


On 08/31/2017 12:19 PM, Jan Beulich wrote:
>>>> On 31.08.17 at 11:49, <proskurin@sec.in.tum.de> wrote:
>> On 08/31/2017 10:04 AM, Jan Beulich wrote:
>>>>>> On 30.08.17 at 20:32, <proskurin@sec.in.tum.de> wrote:
>>>> We move the macros (MAX|INVALID)_ALTP2M out of x86-related code to
>>>> common code, as the following patches will make use of them on ARM.
>>> But both seem not impossible to be require arch-specific values.
>> Right. The general idea at this point is to move as much of altp2m
>> functionality/configuration as possible into a common place. Yet, if you
>> believe that, e.g., the number of altp2m views could/should diverge
>> between both architectures, I will gladly move the defines back into
>> arch-related parts. However, we need to consider that while x86/Intel
>> supports up to 512 entries for EPT pointers as part of the VMCS, we are
>> quite flexible on ARM: we manage the views entirely in software and
>> hence on ARM we can easily keep up with Intel's specification. This
>> allows us to hold parts of the altp2m configuration in a unified place.
>> Or do you believe this is not the right way to go?
> Well, you've basically answered this yourself: Why would you
> want to constrain ARM just because of VMX restrictions? Requiring
> all architectures to surface the same constants (regardless of
> actual values) is all you need to be able to commonize code.

Alright, I will remove the upper constants from common code in v5.

Thanks,
~Sergej

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

[PATCH v4 12/39] arm/p2m: Add altp2m init/teardown routines

[Sergej Proskurin]

The p2m initialization now invokes initialization routines responsible
for the allocation and initialization of altp2m structures. The same
applies to teardown routines. The functionality has been adopted from
the x86 altp2m implementation.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v2: Shared code between host/altp2m init/teardown functions.
    Added conditional init/teardown of altp2m.
    Altp2m related functions are moved to altp2m.c

v3: Removed locking the altp2m_lock in altp2m_teardown. Locking this
    lock at this point is unnecessary.

    Removed re-setting altp2m_vttbr, altp2m_p2m, and altp2m_active
    values in the function "altp2m_teardown". Re-setting these values is
    unnecessary as the entire domain will be destroyed right afterwards.

    Removed check for "altp2m_enabled" in "p2m_init" as altp2m has not yet
    been enabled by libxl at this point.

    Removed check for "altp2m_enabled" before tearing down altp2m within
    the function "p2m_teardown" so that altp2m gets destroyed even if
    the HVM_PARAM_ALTP2M gets reset before "p2m_teardown" is called.

    Added initialization of the field d->arch.altp2m_active in
    "altp2m_init".

    Removed check for already initialized vmid's in "altp2m_init_one",
    as "altp2m_init_one" is now called always with an uninitialized p2m.

    Removed the array altp2m_vttbr[] in struct arch_domain.

v4: Removed initialization of altp2m_p2m[] to NULL in altp2m_init, as
    the "struct arch_domain" is already initialized to zero.

    We moved the definition of the macro MAX_ALTP2M to a common place in
    a separate commit.
---
 xen/arch/arm/Makefile        |  1 +
 xen/arch/arm/altp2m.c        | 56 ++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/p2m.c           | 15 +++++++++++-
 xen/include/asm-arm/altp2m.h |  6 +++++
 xen/include/asm-arm/domain.h | 11 ++++++++-
 5 files changed, 87 insertions(+), 2 deletions(-)
 create mode 100644 xen/arch/arm/altp2m.c

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 282d2c2949..a08683335d 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -5,6 +5,7 @@ subdir-$(CONFIG_ARM_64) += efi
 subdir-$(CONFIG_ACPI) += acpi
 
 obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
+obj-y += altp2m.o
 obj-y += bootfdt.init.o
 obj-y += cpu.o
 obj-y += cpuerrata.o
diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
new file mode 100644
index 0000000000..e73b69d99d
--- /dev/null
+++ b/xen/arch/arm/altp2m.c
@@ -0,0 +1,56 @@
+/*
+ * arch/arm/altp2m.c
+ *
+ * Alternate p2m
+ * Copyright (c) 2016 Sergej Proskurin <proskurin@sec.in.tum.de>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License, version 2,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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 <http://www.gnu.org/licenses/>.
+ */
+
+#include <asm/p2m.h>
+#include <asm/altp2m.h>
+
+int altp2m_init(struct domain *d)
+{
+    spin_lock_init(&d->arch.altp2m_lock);
+    d->arch.altp2m_active = false;
+
+    return 0;
+}
+
+void altp2m_teardown(struct domain *d)
+{
+    unsigned int i;
+    struct p2m_domain *p2m;
+
+    for ( i = 0; i < MAX_ALTP2M; i++ )
+    {
+        p2m = d->arch.altp2m_p2m[i];
+
+        if ( !p2m )
+            continue;
+
+        p2m_teardown_one(p2m);
+        xfree(p2m);
+    }
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index ec855341b9..e017e2972e 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -14,6 +14,7 @@
 #include <asm/event.h>
 #include <asm/hardirq.h>
 #include <asm/page.h>
+#include <asm/altp2m.h>
 
 #define MAX_VMID_8_BIT  (1UL << 8)
 #define MAX_VMID_16_BIT (1UL << 16)
@@ -1305,6 +1306,12 @@ static void p2m_teardown_hostp2m(struct domain *d)
 
 void p2m_teardown(struct domain *d)
 {
+    /*
+     * Teardown altp2m unconditionally so that altp2m gets always destroyed --
+     * even if HVM_PARAM_ALTP2M gets reset before teardown.
+     */
+    altp2m_teardown(d);
+
     p2m_teardown_hostp2m(d);
 }
 
@@ -1319,7 +1326,13 @@ static int p2m_init_hostp2m(struct domain *d)
 
 int p2m_init(struct domain *d)
 {
-    return p2m_init_hostp2m(d);
+    int rc;
+
+    rc = p2m_init_hostp2m(d);
+    if ( rc )
+        return rc;
+
+    return altp2m_init(d);
 }
 
 /*
diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index 66afa959f6..1706f61f0c 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -23,6 +23,9 @@
 #include <xen/altp2m-common.h>
 #include <xen/sched.h>
 
+#define altp2m_lock(d)    spin_lock(&(d)->arch.altp2m_lock)
+#define altp2m_unlock(d)  spin_unlock(&(d)->arch.altp2m_lock)
+
 /* Alternate p2m on/off per domain */
 static inline bool_t altp2m_active(const struct domain *d)
 {
@@ -37,4 +40,7 @@ static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
     return 0;
 }
 
+int altp2m_init(struct domain *d);
+void altp2m_teardown(struct domain *d);
+
 #endif /* __ASM_ARM_ALTP2M_H */
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 0991a0a79d..668f398dbf 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -1,6 +1,7 @@
 #ifndef __ASM_DOMAIN_H__
 #define __ASM_DOMAIN_H__
 
+#include <xen/altp2m-common.h>
 #include <xen/cache.h>
 #include <xen/sched.h>
 #include <asm/page.h>
@@ -146,8 +147,16 @@ struct arch_domain
         uint8_t privileged_call_enabled : 1;
     } monitor;
 
-    /* altp2m: allow multiple copies of the host's p2m */
+    /*
+     * Lock that protects critical altp2m operations that must not be performed
+     * concurrently.
+     */
+    spinlock_t altp2m_lock;
+
+    /* Altp2m: allow multiple copies of the host's p2m. */
     bool altp2m_active;
+
+    struct p2m_domain *altp2m_p2m[MAX_ALTP2M];
 }  __cacheline_aligned;
 
 struct arch_vcpu
-- 
2.13.3


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

[PATCH v4 13/39] arm/p2m: Add altp2m table flushing routine

[Sergej Proskurin]

The current implementation differentiates between flushing and
destroying altp2m views. This commit adds the function altp2m_flush,
which allows to release all of the alternate p2m views. To make sure
that we flush alternate p2m's only if they are not used by any vCPU, we
introduce a counter that tracks the vCPUs that are currently using the
particular p2m.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v2: Pages in p2m->pages are not cleared in p2m_flush_table anymore.
    VMID is freed in p2m_free_one.
    Cosmetic fixes.

v3: Changed the locking mechanism to "p2m_write_lock" inside the
    function "altp2m_flush".

    Do not flush but rather teardown the altp2m in the function
    "altp2m_flush".

    Exchanged the check "altp2m_vttbr[idx] == INVALID_VTTBR" for
    "altp2m_p2m[idx] == NULL" in "altp2m_flush".

v4: Removed the p2m locking instructions in "altp2m_flush", as they are
    not needed at this point. At this point, altp2m should be inactive and
    thus the particular p2m should not be used by a vCPU. Therefore, we
    added an ASSERT statement to ensure this.

    We introduce the counter active_vcpus as part of this patch.

    Rename the function altp2m_flush to altp2m_flush_complete.
---
 xen/arch/arm/altp2m.c        | 32 ++++++++++++++++++++++++++++++++
 xen/include/asm-arm/altp2m.h |  3 +++
 xen/include/asm-arm/p2m.h    |  5 +++++
 3 files changed, 40 insertions(+)

diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
index e73b69d99d..9c06055a94 100644
--- a/xen/arch/arm/altp2m.c
+++ b/xen/arch/arm/altp2m.c
@@ -28,6 +28,38 @@ int altp2m_init(struct domain *d)
     return 0;
 }
 
+void altp2m_flush_complete(struct domain *d)
+{
+    unsigned int i;
+    struct p2m_domain *p2m;
+
+    /*
+     * If altp2m is active, we are not allowed to flush altp2m[0]. This special
+     * view is considered as the hostp2m as long as altp2m is active.
+     */
+    ASSERT(!altp2m_active(d));
+
+    altp2m_lock(d);
+
+    for ( i = 0; i < MAX_ALTP2M; i++ )
+    {
+        p2m = d->arch.altp2m_p2m[i];
+
+        if ( p2m == NULL )
+            continue;
+
+        ASSERT(!atomic_read(&p2m->active_vcpus));
+
+        /* We do not need to lock the p2m, as altp2m is inactive. */
+        p2m_teardown_one(p2m);
+
+        xfree(p2m);
+        d->arch.altp2m_p2m[i] = NULL;
+    }
+
+    altp2m_unlock(d);
+}
+
 void altp2m_teardown(struct domain *d)
 {
     unsigned int i;
diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index 1706f61f0c..e116cce25f 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -43,4 +43,7 @@ static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
 int altp2m_init(struct domain *d);
 void altp2m_teardown(struct domain *d);
 
+/* Flush all the alternate p2m's for a domain. */
+void altp2m_flush_complete(struct domain *d);
+
 #endif /* __ASM_ARM_ALTP2M_H */
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 9bb38e689a..e8a2116081 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -10,6 +10,8 @@
 #include <xen/p2m-common.h>
 #include <public/memory.h>
 
+#include <asm/atomic.h>
+
 #define paddr_bits PADDR_BITS
 
 #define p2m_switch_vttbr_and_get_flags(ovttbr, nvttbr, flags)       \
@@ -132,6 +134,9 @@ struct p2m_domain {
     /* Keeping track on which CPU this p2m was used and for which vCPU */
     uint8_t last_vcpu_ran[NR_CPUS];
 
+    /* Alternate p2m: count of vcpu's currently using this p2m. */
+    atomic_t active_vcpus;
+
     /* Choose between: host/alternate. */
     p2m_class_t p2m_class;
 };
-- 
2.13.3


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

[PATCH v4 14/39] arm/p2m: Add HVMOP_altp2m_set_domain_state

[Sergej Proskurin]

The HVMOP_altp2m_set_domain_state allows to activate altp2m on a
specific domain. This commit adopts the x86
HVMOP_altp2m_set_domain_state implementation.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v2: Dynamically allocate memory for altp2m views only when needed.
    Move altp2m related helpers to altp2m.c.
    p2m_flush_tlb is made publicly accessible.

v3: Cosmetic fixes.

    Removed call to "p2m_alloc_table" in "altp2m_init_helper" as the
    entire p2m allocation is now done within the function
    "p2m_init_one". The same applies to the call of the function
    "p2m_flush_tlb" from "p2m_init_one".

    Removed the "altp2m_enabled" check in HVMOP_altp2m_set_domain_state
    case as it has been moved in front of the switch statement in
    "do_altp2m_op".

    Changed the order of setting the new altp2m state (depending on
    setting/resetting the state) in HVMOP_altp2m_set_domain_state case.

    Removed the call to altp2m_vcpu_reset from altp2m_vcpu_initialize,
    as the p2midx is set right after the call to 0, representing the
    default view.

    Moved the define "vcpu_altp2m" from domain.h to altp2m.h to avoid
    defining altp2m-related functionality in multiple files. Also renamed
    "vcpu_altp2m" to "altp2m_vcpu".

    Declared the function "p2m_flush_tlb" as static, as it is not called
    from altp2m.h anymore.

    Exported the function "altp2m_get_altp2m" in altp2m.h.

    Exchanged the check "altp2m_vttbr[idx] == INVALID_VTTBR" for
    "altp2m_p2m[idx] == NULL" in "altp2m_init_by_id".

    Set the field p2m->access_required to false by default.

v4: Removed unnecessary initialization in "altp2m_init_helper".

    Move the field "active_vcpus" in "struct p2m_domain" out of this
    commit.

    Set "d->arch.altp2m_active" to the provided new state only once instead of
    for each vCPU.

    Move the definition of the macro INVALID_ALTP2M to a common place in
    a separate commit.

    ARM supports an external-only interface to the altp2m subsystem,
    i.e., the guest does not have access to the altp2m subsystem. Thus,
    we remove the check for the current vcpu in the function
    altp2m_vcpu_initialize; there is no scenario in which a guest is
    allowed to initialize the altp2m subsystem for itself.

    Cosmetic fixes.
---
 xen/arch/arm/altp2m.c        | 97 ++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/hvm.c           | 30 +++++++++++++-
 xen/include/asm-arm/altp2m.h | 10 +++++
 xen/include/asm-arm/domain.h |  3 ++
 4 files changed, 139 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
index 9c06055a94..43e95c5681 100644
--- a/xen/arch/arm/altp2m.c
+++ b/xen/arch/arm/altp2m.c
@@ -20,6 +20,103 @@
 #include <asm/p2m.h>
 #include <asm/altp2m.h>
 
+struct p2m_domain *altp2m_get_altp2m(struct vcpu *v)
+{
+    unsigned int idx = v->arch.ap2m_idx;
+
+    if ( idx == INVALID_ALTP2M )
+        return NULL;
+
+    BUG_ON(idx >= MAX_ALTP2M);
+
+    return v->domain->arch.altp2m_p2m[idx];
+}
+
+static void altp2m_vcpu_reset(struct vcpu *v)
+{
+    v->arch.ap2m_idx = INVALID_ALTP2M;
+}
+
+void altp2m_vcpu_initialize(struct vcpu *v)
+{
+    /*
+     * ARM supports an external-only interface to the altp2m subsystem, i.e.,
+     * the guest does not have access to the altp2m subsystem. Thus, we can
+     * simply pause the vcpu, as there is no scenario in which we initialize
+     * altp2m on the current vcpu. That is, the vcpu must be paused every time
+     * we initialize altp2m.
+     */
+    vcpu_pause(v);
+
+    v->arch.ap2m_idx = 0;
+    atomic_inc(&altp2m_get_altp2m(v)->active_vcpus);
+
+    vcpu_unpause(v);
+}
+
+void altp2m_vcpu_destroy(struct vcpu *v)
+{
+    struct p2m_domain *p2m;
+
+    if ( v != current )
+        vcpu_pause(v);
+
+    if ( (p2m = altp2m_get_altp2m(v)) )
+        atomic_dec(&p2m->active_vcpus);
+
+    altp2m_vcpu_reset(v);
+
+    if ( v != current )
+        vcpu_unpause(v);
+}
+
+static int altp2m_init_helper(struct domain *d, unsigned int idx)
+{
+    int rc;
+    struct p2m_domain *p2m = d->arch.altp2m_p2m[idx];
+
+    ASSERT(p2m == NULL);
+
+    /* Allocate a new, zeroed altp2m view. */
+    p2m = xzalloc(struct p2m_domain);
+    if ( p2m == NULL)
+        return -ENOMEM;
+
+    p2m->p2m_class = p2m_alternate;
+
+    /* Initialize the new altp2m view. */
+    rc = p2m_init_one(d, p2m);
+    if ( rc )
+        goto err;
+
+    d->arch.altp2m_p2m[idx] = p2m;
+
+    return rc;
+
+err:
+    xfree(p2m);
+    d->arch.altp2m_p2m[idx] = NULL;
+
+    return rc;
+}
+
+int altp2m_init_by_id(struct domain *d, unsigned int idx)
+{
+    int rc = -EINVAL;
+
+    if ( idx >= MAX_ALTP2M )
+        return rc;
+
+    altp2m_lock(d);
+
+    if ( d->arch.altp2m_p2m[idx] == NULL )
+        rc = altp2m_init_helper(d, idx);
+
+    altp2m_unlock(d);
+
+    return rc;
+}
+
 int altp2m_init(struct domain *d)
 {
     spin_lock_init(&d->arch.altp2m_lock);
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index 43b8352cb7..ec8e259797 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -90,8 +90,36 @@ static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
 
     case HVMOP_altp2m_set_domain_state:
-        rc = -EOPNOTSUPP;
+    {
+        struct vcpu *v;
+        bool ostate, nstate;
+
+        ostate = d->arch.altp2m_active;
+        nstate = !!a.u.domain_state.state;
+
+        /* If the alternate p2m state has changed, handle appropriately */
+        if ( (nstate != ostate) &&
+             (ostate || !(rc = altp2m_init_by_id(d, 0))) )
+        {
+            d->arch.altp2m_active = nstate;
+
+            for_each_vcpu( d, v )
+            {
+                if ( !ostate )
+                    altp2m_vcpu_initialize(v);
+                else
+                    altp2m_vcpu_destroy(v);
+            }
+
+            /*
+             * The altp2m_active state has been deactivated. It is now safe to
+             * flush all altp2m views -- including altp2m[0].
+             */
+            if ( ostate )
+                altp2m_flush_complete(d);
+        }
         break;
+    }
 
     case HVMOP_altp2m_vcpu_enable_notify:
         rc = -EOPNOTSUPP;
diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index e116cce25f..2ef88cec35 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -43,6 +43,16 @@ static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
 int altp2m_init(struct domain *d);
 void altp2m_teardown(struct domain *d);
 
+void altp2m_vcpu_initialize(struct vcpu *v);
+void altp2m_vcpu_destroy(struct vcpu *v);
+
+/* Get current alternate p2m table. */
+struct p2m_domain *altp2m_get_altp2m(struct vcpu *v);
+
+/* Make a specific alternate p2m valid. */
+int altp2m_init_by_id(struct domain *d,
+                      unsigned int idx);
+
 /* Flush all the alternate p2m's for a domain. */
 void altp2m_flush_complete(struct domain *d);
 
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 668f398dbf..fc16cb81fd 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -293,6 +293,9 @@ struct arch_vcpu
     struct vtimer phys_timer;
     struct vtimer virt_timer;
     bool_t vtimer_initialized;
+
+    /* Alternate p2m index */
+    uint16_t ap2m_idx;
 }  __cacheline_aligned;
 
 void vcpu_show_execution_state(struct vcpu *);
-- 
2.13.3


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

[PATCH v4 15/39] arm/p2m: Add HVMOP_altp2m_create_p2m

[Sergej Proskurin]

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v2: Cosmetic fixes.

v3: Cosmetic fixes.

    Renamed the function "altp2m_init_next" to
    "altp2m_init_next_available".

    Exchanged the check "altp2m_vttbr[idx] == INVALID_VTTBR" for
    "altp2m_p2m[idx] == NULL" in "altp2m_init_next_available".
---
 xen/arch/arm/altp2m.c        | 23 +++++++++++++++++++++++
 xen/arch/arm/hvm.c           |  3 ++-
 xen/include/asm-arm/altp2m.h |  4 ++++
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
index 43e95c5681..6b1e34709f 100644
--- a/xen/arch/arm/altp2m.c
+++ b/xen/arch/arm/altp2m.c
@@ -117,6 +117,29 @@ int altp2m_init_by_id(struct domain *d, unsigned int idx)
     return rc;
 }
 
+int altp2m_init_next_available(struct domain *d, uint16_t *idx)
+{
+    int rc = -EINVAL;
+    uint16_t i;
+
+    altp2m_lock(d);
+
+    for ( i = 0; i < MAX_ALTP2M; i++ )
+    {
+        if ( d->arch.altp2m_p2m[i] != NULL )
+            continue;
+
+        rc = altp2m_init_helper(d, i);
+        *idx = i;
+
+        break;
+    }
+
+    altp2m_unlock(d);
+
+    return rc;
+}
+
 int altp2m_init(struct domain *d)
 {
     spin_lock_init(&d->arch.altp2m_lock);
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index ec8e259797..caa2e1b516 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -126,7 +126,8 @@ static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
 
     case HVMOP_altp2m_create_p2m:
-        rc = -EOPNOTSUPP;
+        if ( !(rc = altp2m_init_next_available(d, &a.u.view.view)) )
+            rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
         break;
 
     case HVMOP_altp2m_destroy_p2m:
diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index 2ef88cec35..b9719f9d5b 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -53,6 +53,10 @@ struct p2m_domain *altp2m_get_altp2m(struct vcpu *v);
 int altp2m_init_by_id(struct domain *d,
                       unsigned int idx);
 
+/* Find and initialize the next available alternate p2m. */
+int altp2m_init_next_available(struct domain *d,
+                               uint16_t *idx);
+
 /* Flush all the alternate p2m's for a domain. */
 void altp2m_flush_complete(struct domain *d);
 
-- 
2.13.3


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

[PATCH v4 16/39] arm/p2m: Add HVMOP_altp2m_destroy_p2m

[Sergej Proskurin]

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v2: Substituted the call to tlb_flush for p2m_flush_table.
    Added comments.
    Cosmetic fixes.

v3: Changed the locking mechanism to "p2m_write_lock" inside the
    function "altp2m_destroy_by_id".

    Do not flush but rather teardown the altp2m in the function
    "altp2m_destroy_by_id".

    Exchanged the check "altp2m_vttbr[idx] == INVALID_VTTBR" for
    "altp2m_p2m[idx] == NULL" in "altp2m_destroy_by_id".

v4: Removed locking the p2m in "altp2m_destroy_by_id" as the p2m is not
    used by anyone else at this point.
---
 xen/arch/arm/altp2m.c        | 39 +++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/hvm.c           |  2 +-
 xen/include/asm-arm/altp2m.h |  4 ++++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
index 6b1e34709f..1128e1af16 100644
--- a/xen/arch/arm/altp2m.c
+++ b/xen/arch/arm/altp2m.c
@@ -180,6 +180,45 @@ void altp2m_flush_complete(struct domain *d)
     altp2m_unlock(d);
 }
 
+int altp2m_destroy_by_id(struct domain *d, unsigned int idx)
+{
+    struct p2m_domain *p2m;
+    int rc = -EBUSY;
+
+    /*
+     * The altp2m[0] is considered as the hostp2m and is used as a safe harbor
+     * to which you can switch as long as altp2m is active. After deactivating
+     * altp2m, the system switches back to the original hostp2m view. That is,
+     * altp2m[0] should only be destroyed/flushed/freed, when altp2m is
+     * deactivated.
+     */
+    if ( !idx || idx >= MAX_ALTP2M )
+        return rc;
+
+    domain_pause_except_self(d);
+
+    altp2m_lock(d);
+
+    if ( d->arch.altp2m_p2m[idx] != NULL )
+    {
+        p2m = d->arch.altp2m_p2m[idx];
+
+        if ( !_atomic_read(p2m->active_vcpus) )
+        {
+            p2m_teardown_one(p2m);
+            xfree(p2m);
+            d->arch.altp2m_p2m[idx] = NULL;
+            rc = 0;
+        }
+    }
+
+    altp2m_unlock(d);
+
+    domain_unpause_except_self(d);
+
+    return rc;
+}
+
 void altp2m_teardown(struct domain *d)
 {
     unsigned int i;
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index caa2e1b516..4bf2f28a1a 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -131,7 +131,7 @@ static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
 
     case HVMOP_altp2m_destroy_p2m:
-        rc = -EOPNOTSUPP;
+        rc = altp2m_destroy_by_id(d, a.u.view.view);
         break;
 
     case HVMOP_altp2m_switch_p2m:
diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index b9719f9d5b..778c6c4f12 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -60,4 +60,8 @@ int altp2m_init_next_available(struct domain *d,
 /* Flush all the alternate p2m's for a domain. */
 void altp2m_flush_complete(struct domain *d);
 
+/* Make a specific alternate p2m invalid */
+int altp2m_destroy_by_id(struct domain *d,
+                         unsigned int idx);
+
 #endif /* __ASM_ARM_ALTP2M_H */
-- 
2.13.3


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

[PATCH v4 17/39] arm/p2m: Add HVMOP_altp2m_switch_p2m

[Sergej Proskurin]

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v3: Extended the function "altp2m_switch_domain_altp2m_by_id" so that if
    the guest domain indirectly calles this function, the current vcpu also
    changes the altp2m view without performing an explicit context switch.

    Exchanged the check "altp2m_vttbr[idx] == INVALID_VTTBR" for
    "altp2m_p2m[idx] == NULL" in "altp2m_switch_domain_altp2m_by_id".

v4: ARM supports an external-only interface to the altp2m subsystem,
    i.e, the guest does not have access to altp2m. Thus, we don't have
    to consider that the current vcpu will not switch its context in the
    function "p2m_restore_state". For this reason, we do not check for
    whether we are working on the current vcpu in the function
    altp2m_switch_domain_altp2m_by_id. If the current guest access
    restriction to the altp2m subsystem should change in the future, we
    have to update VTTBR_EL2 directly.

    Cosmetic fixes.
---
 xen/arch/arm/altp2m.c        | 45 ++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/hvm.c           |  2 +-
 xen/include/asm-arm/altp2m.h |  4 ++++
 3 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
index 1128e1af16..9a2cf5a018 100644
--- a/xen/arch/arm/altp2m.c
+++ b/xen/arch/arm/altp2m.c
@@ -32,6 +32,51 @@ struct p2m_domain *altp2m_get_altp2m(struct vcpu *v)
     return v->domain->arch.altp2m_p2m[idx];
 }
 
+int altp2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx)
+{
+    struct vcpu *v;
+    int rc = -EINVAL;
+
+    if ( idx >= MAX_ALTP2M )
+        return rc;
+
+    domain_pause_except_self(d);
+
+    altp2m_lock(d);
+
+    if ( d->arch.altp2m_p2m[idx] != NULL )
+    {
+        for_each_vcpu( d, v )
+        {
+            if ( idx == v->arch.ap2m_idx )
+                continue;
+
+            atomic_dec(&altp2m_get_altp2m(v)->active_vcpus);
+            v->arch.ap2m_idx = idx;
+            atomic_inc(&altp2m_get_altp2m(v)->active_vcpus);
+
+            /*
+             * ARM supports an external-only interface to the altp2m subsystem,
+             * i.e, the guest does not have access to altp2m. Thus, we don't
+             * have to consider that the current vcpu will not switch its
+             * context in the function "p2m_restore_state".
+             *
+             * XXX: If the current guest access restriction to the altp2m
+             * subsystem should change in the future, we have to update
+             * VTTBR_EL2 directly.
+             */
+        }
+
+        rc = 0;
+    }
+
+    altp2m_unlock(d);
+
+    domain_unpause_except_self(d);
+
+    return rc;
+}
+
 static void altp2m_vcpu_reset(struct vcpu *v)
 {
     v->arch.ap2m_idx = INVALID_ALTP2M;
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index 4bf2f28a1a..9bddc7e17e 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -135,7 +135,7 @@ static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
 
     case HVMOP_altp2m_switch_p2m:
-        rc = -EOPNOTSUPP;
+        rc = altp2m_switch_domain_altp2m_by_id(d, a.u.view.view);
         break;
 
     case HVMOP_altp2m_set_mem_access:
diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index 778c6c4f12..d59f704489 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -49,6 +49,10 @@ void altp2m_vcpu_destroy(struct vcpu *v);
 /* Get current alternate p2m table. */
 struct p2m_domain *altp2m_get_altp2m(struct vcpu *v);
 
+/* Switch alternate p2m for entire domain */
+int altp2m_switch_domain_altp2m_by_id(struct domain *d,
+                                      unsigned int idx);
+
 /* Make a specific alternate p2m valid. */
 int altp2m_init_by_id(struct domain *d,
                       unsigned int idx);
-- 
2.13.3


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

[PATCH v4 18/39] arm/p2m: Add p2m_get_active_p2m macro

[Sergej Proskurin]

This commit introduces the macro "p2m_get_active_p2m" returning the
currently active (alt)p2m. The need for this macro will be shown in the
following commits.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v4: Moved the introduced macro from ./xen/arch/arm/p2m.c to
    ./xen/include/asm-arm/p2m.h as it will be used in multiple files in the
    following commits.
---
 xen/include/asm-arm/p2m.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index e8a2116081..d3467daacf 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -14,6 +14,9 @@
 
 #define paddr_bits PADDR_BITS
 
+#define p2m_get_active_p2m(v) unlikely(altp2m_active(v->domain)) ?  \
+                              altp2m_get_altp2m(v) : p2m_get_hostp2m(v->domain);
+
 #define p2m_switch_vttbr_and_get_flags(ovttbr, nvttbr, flags)       \
 ({                                                                  \
      ovttbr = READ_SYSREG64(VTTBR_EL2);                             \
-- 
2.13.3


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

[PATCH v4 19/39] arm/p2m: Make p2m_restore_state ready for altp2m

[Sergej Proskurin]

This commit adapts the function "p2m_restore_state" in a way that the
currently active altp2m table is considered during state restoration.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v3: Moved declaration of "altp2m_switch_domain_altp2m_by_id" out of this
    patch.

v4: Moved the variable "p2m", as to satisfy compiler warnings, prohibiting
    mixing declarations and code.
---
 xen/arch/arm/p2m.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index e017e2972e..16c7585ffa 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -112,8 +112,8 @@ void p2m_save_state(struct vcpu *p)
 
 void p2m_restore_state(struct vcpu *n)
 {
-    struct p2m_domain *p2m = p2m_get_hostp2m(n->domain);
     uint8_t *last_vcpu_ran;
+    struct p2m_domain *p2m = p2m_get_active_p2m(n);
 
     if ( is_idle_vcpu(n) )
         return;
-- 
2.13.3


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

[PATCH v4 20/39] arm/p2m: Make get_page_from_gva ready for altp2m

[Sergej Proskurin]

The function get_page_from_gva uses ARM's hardware support to translate
gva's to machine addresses. This function is used, among others, for
memory regulation purposes, e.g, within the context of memory ballooning.
To ensure correct behavior while altp2m is in use, we use the host's p2m
table for the associated gva to ma translation. This is required at this
point, as altp2m lazily copies pages from the host's p2m and even might
be flushed because of changes to the host's p2m (as it is done within
the context of memory ballooning).

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v3: Cosmetic fixes.

    Make use of the p2m_(switch|restore)_vttbr_and_(g|s)et_flags macros
    to avoid code duplication.

v4: Remove initialization of the old vttbr outside of the macro
    "p2m_switch_vttbr_and_get_flags".
---
 xen/arch/arm/p2m.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 16c7585ffa..20d7784708 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1470,7 +1470,24 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
 
     p2m_read_lock(p2m);
 
-    rc = gvirt_to_maddr(va, &maddr, flags);
+    /*
+     * If altp2m is active, we need to translate the gva upon the hostp2m's
+     * vttbr, as it contains all valid mappings while the currently active
+     * altp2m view might not have the required gva mapping yet.
+     */
+    if ( unlikely(altp2m_active(d)) )
+    {
+        unsigned long flags = 0;
+        uint64_t ovttbr;
+
+        p2m_switch_vttbr_and_get_flags(ovttbr, p2m->vttbr, flags);
+
+        rc = gvirt_to_maddr(va, &maddr, flags);
+
+        p2m_restore_vttbr_and_set_flags(ovttbr, flags);
+    }
+    else
+        rc = gvirt_to_maddr(va, &maddr, flags);
 
     if ( rc )
         goto err;
-- 
2.13.3


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

[PATCH v4 21/39] arm/p2m: Cosmetic fix - __p2m_get_mem_access

[Sergej Proskurin]

In this commit, we extend the function prototype of "__p2m_get_mem_access" to
hold an argument of type "struct p2m_domain*", as we need to distinguish
between the host's p2m and different altp2m views.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v3: Changed the parameter of "p2m_mem_access_check_and_get_page"
    from "struct p2m_domain*" to "struct vcpu*".

v4: We don't need to adjust the function "p2m_mem_access_check_and_get_page"
    any more, as its change is already part of another patch.
---
 xen/arch/arm/mem_access.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
index 3e2bb4088a..5bc28db8ff 100644
--- a/xen/arch/arm/mem_access.c
+++ b/xen/arch/arm/mem_access.c
@@ -24,10 +24,9 @@
 #include <asm/event.h>
 #include <asm/guest_walk.h>
 
-static int __p2m_get_mem_access(struct domain *d, gfn_t gfn,
+static int __p2m_get_mem_access(struct p2m_domain *p2m, gfn_t gfn,
                                 xenmem_access_t *access)
 {
-    struct p2m_domain *p2m = p2m_get_hostp2m(d);
     void *i;
     unsigned int index;
 
@@ -148,7 +147,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag,
      * We do this first as this is faster in the default case when no
      * permission is set on the page.
      */
-    rc = __p2m_get_mem_access(v->domain, gfn, &xma);
+    rc = __p2m_get_mem_access(p2m, gfn, &xma);
     if ( rc < 0 )
         goto err;
 
@@ -443,7 +442,7 @@ int p2m_get_mem_access(struct domain *d, gfn_t gfn,
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
 
     p2m_read_lock(p2m);
-    ret = __p2m_get_mem_access(d, gfn, access);
+    ret = __p2m_get_mem_access(p2m, gfn, access);
     p2m_read_unlock(p2m);
 
     return ret;
-- 
2.13.3


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

[PATCH v4 22/39] arm/p2m: Make p2m_mem_access_check ready for altp2m

[Sergej Proskurin]

This commit extends the function "p2m_mem_access_check" and
"p2m_mem_access_check_and_get_page" to consider altp2m. The function
"p2m_mem_access_check_and_get_page" needs to translate the gva upon the
hostp2m's vttbr, as it contains all valid mappings while the currently
active altp2m view might not have the required gva mapping yet.

Also, the new implementation fills the request buffer to hold
altp2m-related information.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v3: Extended the function "p2m_mem_access_check_and_get_page" to
    consider altp2m. Similar to "get_page_from_gva", the function
    "p2m_mem_access_check_and_get_page" needs to translate the gva upon
    the hostp2m's vttbr. Although, the function "gva_to_ipa" (called in
    "p2m_mem_access_check_and_get_page") performs a stage 1 table walk,
    it will access page tables residing in memory. Accesses to this
    memory are controlled by the underlying 2nd stage translation table
    and hence require the original mappings of the hostp2m.

v4: Cosmetic fixes.

    Initialized the variable "ipa" in the function
    "p2m_mem_access_check_and_get_page" to satisfy compiler warnings.
---
 xen/arch/arm/mem_access.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
index 5bc28db8ff..ebc3a86af3 100644
--- a/xen/arch/arm/mem_access.c
+++ b/xen/arch/arm/mem_access.c
@@ -21,6 +21,7 @@
 #include <xen/sched.h>
 #include <xen/vm_event.h>
 #include <public/vm_event.h>
+#include <asm/altp2m.h>
 #include <asm/event.h>
 #include <asm/guest_walk.h>
 
@@ -108,9 +109,31 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag,
     xenmem_access_t xma;
     p2m_type_t t;
     struct page_info *page = NULL;
-    struct p2m_domain *p2m = p2m_get_hostp2m(v->domain);
+    struct domain *d = v->domain;
+    struct p2m_domain *p2m = p2m_get_hostp2m(d);
+
+    /*
+     * If altp2m is active, we need to translate the gva upon the hostp2m's
+     * vttbr, as it contains all valid mappings while the currently active
+     * altp2m view might not have the required gva mapping yet. Although, the
+     * function gva_to_ipa performs a stage 1 table walk, it will access page
+     * tables residing in memory. Accesses to this memory are controlled by the
+     * underlying 2nd stage translation table and hence require the original
+     * mappings of the hostp2m.
+     */
+    if ( unlikely(altp2m_active(d)) )
+    {
+        unsigned long flags = 0;
+        uint64_t ovttbr = READ_SYSREG64(VTTBR_EL2);
+
+        p2m_switch_vttbr_and_get_flags(ovttbr, p2m->vttbr, flags);
 
-    rc = gva_to_ipa(gva, &ipa, flag);
+        rc = gva_to_ipa(gva, &ipa, flag);
+
+        p2m_restore_vttbr_and_set_flags(ovttbr, flags);
+    }
+    else
+        rc = gva_to_ipa(gva, &ipa, flag);
 
     /*
      * In case mem_access is active, hardware-based gva_to_ipa translation
@@ -225,13 +248,15 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec)
     xenmem_access_t xma;
     vm_event_request_t *req;
     struct vcpu *v = current;
-    struct p2m_domain *p2m = p2m_get_hostp2m(v->domain);
+    struct p2m_domain *p2m = p2m_get_active_p2m(v);
 
     /* Mem_access is not in use. */
     if ( !p2m->mem_access_enabled )
         return true;
 
-    rc = p2m_get_mem_access(v->domain, gaddr_to_gfn(gpa), &xma);
+    p2m_read_lock(p2m);
+    rc = __p2m_get_mem_access(p2m, _gfn(paddr_to_pfn(gpa)), &xma);
+    p2m_read_unlock(p2m);
     if ( rc )
         return true;
 
-- 
2.13.3


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

[PATCH v4 23/39] arm/p2m: Cosmetic fix - function prototypes

[Sergej Proskurin]

This commit changes the prototypes of the following functions:
- p2m_insert_mapping
- p2m_remove_mapping

These changes are required as our implementation reuses most of the
existing ARM p2m implementation to set page table attributes of the
individual altp2m views. Therefore, exiting function prototypes have
been extended to hold another argument (of type struct p2m_domain *).
This allows to specify the p2m/altp2m domain that should be processed by
the individual function -- instead of accessing the host's default p2m
domain.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v2: Adoption of the functions "__p2m_lookup" and "__p2m_get_mem_access"
    have been moved out of this commit.
---
 xen/arch/arm/p2m.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 20d7784708..c5bf64aee0 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1012,13 +1012,12 @@ int p2m_set_entry(struct p2m_domain *p2m,
     return rc;
 }
 
-static inline int p2m_insert_mapping(struct domain *d,
+static inline int p2m_insert_mapping(struct p2m_domain *p2m,
                                      gfn_t start_gfn,
                                      unsigned long nr,
                                      mfn_t mfn,
                                      p2m_type_t t)
 {
-    struct p2m_domain *p2m = p2m_get_hostp2m(d);
     int rc;
 
     p2m_write_lock(p2m);
@@ -1028,12 +1027,11 @@ static inline int p2m_insert_mapping(struct domain *d,
     return rc;
 }
 
-static inline int p2m_remove_mapping(struct domain *d,
+static inline int p2m_remove_mapping(struct p2m_domain *p2m,
                                      gfn_t start_gfn,
                                      unsigned long nr,
                                      mfn_t mfn)
 {
-    struct p2m_domain *p2m = p2m_get_hostp2m(d);
     int rc;
 
     p2m_write_lock(p2m);
@@ -1050,7 +1048,7 @@ int map_regions_p2mt(struct domain *d,
                      mfn_t mfn,
                      p2m_type_t p2mt)
 {
-    return p2m_insert_mapping(d, gfn, nr, mfn, p2mt);
+    return p2m_insert_mapping(p2m_get_hostp2m(d), gfn, nr, mfn, p2mt);
 }
 
 int unmap_regions_p2mt(struct domain *d,
@@ -1058,7 +1056,7 @@ int unmap_regions_p2mt(struct domain *d,
                        unsigned long nr,
                        mfn_t mfn)
 {
-    return p2m_remove_mapping(d, gfn, nr, mfn);
+    return p2m_remove_mapping(p2m_get_hostp2m(d), gfn, nr, mfn);
 }
 
 int map_mmio_regions(struct domain *d,
@@ -1066,7 +1064,7 @@ int map_mmio_regions(struct domain *d,
                      unsigned long nr,
                      mfn_t mfn)
 {
-    return p2m_insert_mapping(d, start_gfn, nr, mfn, p2m_mmio_direct_dev);
+    return p2m_insert_mapping(p2m_get_hostp2m(d), start_gfn, nr, mfn, p2m_mmio_direct_dev);
 }
 
 int unmap_mmio_regions(struct domain *d,
@@ -1074,7 +1072,7 @@ int unmap_mmio_regions(struct domain *d,
                        unsigned long nr,
                        mfn_t mfn)
 {
-    return p2m_remove_mapping(d, start_gfn, nr, mfn);
+    return p2m_remove_mapping(p2m_get_hostp2m(d), start_gfn, nr, mfn);
 }
 
 int map_dev_mmio_region(struct domain *d,
@@ -1087,7 +1085,7 @@ int map_dev_mmio_region(struct domain *d,
     if ( !(nr && iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + nr - 1)) )
         return 0;
 
-    res = p2m_insert_mapping(d, gfn, nr, mfn, p2m_mmio_direct_c);
+    res = p2m_insert_mapping(p2m_get_hostp2m(d), gfn, nr, mfn, p2m_mmio_direct_c);
     if ( res < 0 )
     {
         printk(XENLOG_G_ERR "Unable to map MFNs [%#"PRI_mfn" - %#"PRI_mfn" in Dom%d\n",
@@ -1104,13 +1102,13 @@ int guest_physmap_add_entry(struct domain *d,
                             unsigned long page_order,
                             p2m_type_t t)
 {
-    return p2m_insert_mapping(d, gfn, (1 << page_order), mfn, t);
+    return p2m_insert_mapping(p2m_get_hostp2m(d), gfn, (1 << page_order), mfn, t);
 }
 
 int guest_physmap_remove_page(struct domain *d, gfn_t gfn, mfn_t mfn,
                               unsigned int page_order)
 {
-    return p2m_remove_mapping(d, gfn, (1 << page_order), mfn);
+    return p2m_remove_mapping(p2m_get_hostp2m(d), gfn, (1 << page_order), mfn);
 }
 
 static int p2m_alloc_table(struct p2m_domain *p2m)
-- 
2.13.3


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

[PATCH v4 24/39] arm/p2m: Make p2m_put_l3_page ready for altp2m

[Sergej Proskurin]

This commit extends the prototype of the function "p2m_put_l3_page" by
an additional function parameter of type "struct p2m_domain*". This is
needed as a future commit will extend the function "p2m_put_l3_page" so
that we can call "put_page" only if the p2m being modified is the
hostp2m.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/p2m.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index c5bf64aee0..246250d8c6 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -606,7 +606,7 @@ static int p2m_mem_access_radix_set(struct p2m_domain *p2m, gfn_t gfn,
  * TODO: Handle superpages, for now we only take special references for leaf
  * pages (specifically foreign ones, which can't be super mapped today).
  */
-static void p2m_put_l3_page(const lpae_t pte)
+static void p2m_put_l3_page(struct p2m_domain *p2m, const lpae_t pte)
 {
     ASSERT(lpae_valid(pte));
 
@@ -649,7 +649,7 @@ static void p2m_free_entry(struct p2m_domain *p2m,
     if ( level == 3 )
     {
         p2m->stats.mappings[level]--;
-        p2m_put_l3_page(entry);
+        p2m_put_l3_page(p2m, entry);
         return;
     }
 
-- 
2.13.3


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

[PATCH v4 25/39] arm/p2m: Modify reference count only if hostp2m active

[Sergej Proskurin]

This commit makes sure that the page reference count is updated through
the function "p2m_put_l3_page" only when the entries have been freed
from the host's p2m.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>

v4: Moved the check for the host's p2m from "p2m_free_entry" to
    "p2m_put_l3_page".
---
 xen/arch/arm/p2m.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 246250d8c6..e9274c74a8 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -617,7 +617,7 @@ static void p2m_put_l3_page(struct p2m_domain *p2m, const lpae_t pte)
      * flush the TLBs if the page is reallocated before the end of
      * this loop.
      */
-    if ( p2m_is_foreign(pte.p2m.type) )
+    if ( p2m_is_foreign(pte.p2m.type) && p2m_is_hostp2m(p2m) )
     {
         mfn_t mfn = _mfn(pte.p2m.base);
 
-- 
2.13.3


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

[PATCH v4 26/39] arm/p2m: Add HVMOP_altp2m_set_mem_access

[Sergej Proskurin]

The HVMOP_altp2m_set_mem_access allows to set gfn permissions of
(currently one page at a time) of a specific altp2m view. In case the
view does not hold the requested gfn entry, it will be first copied from
the host's p2m table and then modified as requested.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Tamas K Lengyel <tamas@tklengyel.com>
---
v2: Prevent the page reference count from being falsely updated on
    altp2m modification. Therefore, we add a check determining whether
    the target p2m is a hostp2m before p2m_put_l3_page is called.

v3: Cosmetic fixes.

    Added the functionality to set/get the default_access also in/from
    the requested altp2m view.

    Read-locked hp2m in "altp2m_set_mem_access".

    Moved the functions "p2m_is_(hostp2m|altp2m)" out of this commit.

    Moved the funtion "modify_altp2m_entry" out of this commit.

    Moved the function "p2m_lookup_attr" out of this commit.

    Moved guards for "p2m_put_l3_page" out of this commit.

v4: Cosmetic fixes.

    Removed locking altp2m_lock, as it unnecessarily serializes accesses
    to "altp2m_set_mem_access".

    Use the functions "p2m_(set|get)_entry" instead of the helpers
    "p2m_lookup_attr" and "modify_altp2m_entry".

    Removes the restriction enforcing changing the memory access of
    p2m_ram_(rw|ro). Instead, we allow to set memory permissions of all
    pages of the particular altp2m view.

    Move the functionality locking ap2m and hp2m out of "altp2m_set_mem_access"
    into "p2m_set_mem_access".

    Comment the need for the default access in altp2m views.
---
 xen/arch/arm/altp2m.c        | 46 ++++++++++++++++++++++++++++
 xen/arch/arm/hvm.c           |  7 ++++-
 xen/arch/arm/mem_access.c    | 72 +++++++++++++++++++++++++++++++++++---------
 xen/include/asm-arm/altp2m.h | 12 ++++++++
 4 files changed, 122 insertions(+), 15 deletions(-)

diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
index 9a2cf5a018..8c3212780a 100644
--- a/xen/arch/arm/altp2m.c
+++ b/xen/arch/arm/altp2m.c
@@ -77,6 +77,52 @@ int altp2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx)
     return rc;
 }
 
+int altp2m_set_mem_access(struct domain *d,
+                          struct p2m_domain *hp2m,
+                          struct p2m_domain *ap2m,
+                          p2m_access_t a,
+                          gfn_t gfn)
+{
+    p2m_type_t p2mt;
+    p2m_access_t old_a;
+    mfn_t mfn, mfn_sp;
+    gfn_t gfn_sp;
+    unsigned int order;
+    int rc;
+
+    /* Check if entry is part of the altp2m view. */
+    mfn = p2m_get_entry(ap2m, gfn, &p2mt, NULL, &order);
+
+    /* Check host p2m if no valid entry in ap2m. */
+    if ( mfn_eq(mfn, INVALID_MFN) )
+    {
+        /* Check if entry is part of the host p2m view. */
+        mfn = p2m_get_entry(hp2m, gfn, &p2mt, &old_a, &order);
+        if ( mfn_eq(mfn, INVALID_MFN) )
+            return -ESRCH;
+
+        /* If this is a superpage, copy that first. */
+        if ( order != THIRD_ORDER )
+        {
+            /* Align the gfn and mfn to the given pager order. */
+            gfn_sp = _gfn(gfn_x(gfn) & ~((1UL << order) - 1));
+            mfn_sp = _mfn(mfn_x(mfn) & ~((1UL << order) - 1));
+
+            rc = p2m_set_entry(ap2m, gfn_sp, (1UL << order), mfn_sp, p2mt, old_a);
+            if ( rc )
+                return rc;
+        }
+    }
+
+    /* Align the gfn and mfn to the given pager order. */
+    gfn = _gfn(gfn_x(gfn) & ~((1UL << THIRD_ORDER) - 1));
+    mfn = _mfn(mfn_x(mfn) & ~((1UL << THIRD_ORDER) - 1));
+
+    rc = p2m_set_entry(ap2m, gfn, (1UL << THIRD_ORDER), mfn, p2mt, a);
+
+    return rc;
+}
+
 static void altp2m_vcpu_reset(struct vcpu *v)
 {
     v->arch.ap2m_idx = INVALID_ALTP2M;
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index 9bddc7e17e..7e91f2436d 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -139,7 +139,12 @@ static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
 
     case HVMOP_altp2m_set_mem_access:
-        rc = -EOPNOTSUPP;
+        if ( a.u.set_mem_access.pad )
+            rc = -EINVAL;
+        else
+            rc = p2m_set_mem_access(d, _gfn(a.u.set_mem_access.gfn), 1, 0, 0,
+                                    a.u.set_mem_access.hvmmem_access,
+                                    a.u.set_mem_access.view);
         break;
 
     case HVMOP_altp2m_change_gfn:
diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
index ebc3a86af3..ee2a43fc6e 100644
--- a/xen/arch/arm/mem_access.c
+++ b/xen/arch/arm/mem_access.c
@@ -374,7 +374,7 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
                         uint32_t start, uint32_t mask, xenmem_access_t access,
                         unsigned int altp2m_idx)
 {
-    struct p2m_domain *p2m = p2m_get_hostp2m(d);
+    struct p2m_domain *hp2m = p2m_get_hostp2m(d), *ap2m = NULL; 
     p2m_access_t a;
     unsigned int order;
     long rc = 0;
@@ -394,13 +394,26 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
 #undef ACCESS
     };
 
+    /* altp2m view 0 is treated as the hostp2m */
+    if ( altp2m_idx )
+    {
+        if ( altp2m_idx >= MAX_ALTP2M ||
+                d->arch.altp2m_p2m[altp2m_idx] == NULL )
+            return -EINVAL;
+
+        ap2m = d->arch.altp2m_p2m[altp2m_idx];
+    }
+
     switch ( access )
     {
     case 0 ... ARRAY_SIZE(memaccess) - 1:
         a = memaccess[access];
         break;
     case XENMEM_access_default:
-        a = p2m->default_access;
+        if ( ap2m )
+            a = ap2m->default_access;
+        else
+            a = hp2m->default_access;
         break;
     default:
         return -EINVAL;
@@ -410,31 +423,60 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
      * Flip mem_access_enabled to true when a permission is set, as to prevent
      * allocating or inserting super-pages.
      */
-    p2m->mem_access_enabled = true;
+    if ( ap2m )
+        ap2m->mem_access_enabled = true;
+    else
+        hp2m->mem_access_enabled = true;
 
     /* If request to set default access. */
     if ( gfn_eq(gfn, INVALID_GFN) )
     {
-        p2m->default_access = a;
+        if ( ap2m )
+            /*
+             * XXX: Currently, we allow to set the default access of individual
+             * altp2m views.  The default access is required, e.g, when
+             * splitting a superpage belonging to an altp2m view. By setting
+             * the default access, we can limit the access to the split pages
+             * without excplicitely accessing these.
+             */
+            ap2m->default_access = a;
+        else
+            hp2m->default_access = a;
+
         return 0;
     }
 
-    p2m_write_lock(p2m);
+    p2m_write_lock(hp2m);
+    if ( ap2m )
+        p2m_write_lock(ap2m);
 
     for ( gfn = gfn_add(gfn, start); nr > start;
           gfn = gfn_next_boundary(gfn, order) )
     {
-        p2m_type_t t;
-        mfn_t mfn = p2m_get_entry(p2m, gfn, &t, NULL, &order);
-
-
-        if ( !mfn_eq(mfn, INVALID_MFN) )
+        if ( ap2m )
         {
-            order = 0;
-            rc = p2m_set_entry(p2m, gfn, 1, mfn, t, a);
-            if ( rc )
+            /*
+             * TODO: ARM altp2m currently supports only setting of memory
+             * access rights of only one (4K) page at a time.
+             */
+
+            rc = altp2m_set_mem_access(d, hp2m, ap2m, a, gfn);
+            if ( rc && rc != -ESRCH )
                 break;
         }
+        else
+        {
+            p2m_type_t t;
+            mfn_t mfn = p2m_get_entry(hp2m, gfn, &t, NULL, &order);
+
+            if ( !mfn_eq(mfn, INVALID_MFN) )
+            {
+                order = 0;
+                rc = p2m_set_entry(hp2m, gfn, 1, mfn, t, a);
+                if ( rc )
+                    break;
+            }
+        }
 
         start += gfn_x(gfn_next_boundary(gfn, order)) - gfn_x(gfn);
         /* Check for continuation if it is not the last iteration */
@@ -445,7 +487,9 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
         }
     }
 
-    p2m_write_unlock(p2m);
+    if ( ap2m )
+        p2m_write_unlock(ap2m);
+    p2m_write_unlock(hp2m);
 
     return rc;
 }
diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index d59f704489..f8e772f120 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -68,4 +68,16 @@ void altp2m_flush_complete(struct domain *d);
 int altp2m_destroy_by_id(struct domain *d,
                          unsigned int idx);
 
+/*
+ * Set memory access attributes of the gfn in the altp2m view. If the altp2m
+ * view does not contain the particular entry, copy it first from the hostp2m.
+ *
+ * Currently supports memory attribute adoptions of only one (4K) page.
+ */
+int altp2m_set_mem_access(struct domain *d,
+                          struct p2m_domain *hp2m,
+                          struct p2m_domain *ap2m,
+                          p2m_access_t a,
+                          gfn_t gfn);
+
 #endif /* __ASM_ARM_ALTP2M_H */
-- 
2.13.3


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

[PATCH v4 27/39] arm/p2m: Add altp2m_propagate_change

[Sergej Proskurin]

This commit introduces the function "altp2m_propagate_change" that is
responsible to propagate changes applied to the host's p2m to a specific
or even all altp2m views. In this way, Xen can in-/decrease the guest's
physmem at run-time without leaving the altp2m views with
stalled/invalid entries.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v3: Cosmetic fixes.

    Changed the locking mechanism to "p2m_write_lock" inside the
    function "altp2m_reset".

    Removed TLB flushing and resetting of the max_mapped_gfn
    lowest_mapped_gfn fields within the function "altp2m_reset". These
    operations are performed in the function "p2m_flush_table".

    Protected altp2m_active(d) check in "altp2m_propagate_change".

    The function "altp2m_propagate_change" now decides whether an entry
    needs to be dropped out of the altp2m view only if the smfn value
    equals INVALID_MFN.

    Extended the function "altp2m_propagate_change" so that it returns
    an int value to the caller. Also, the function "apply_p2m_changes"
    checks the return value and fails the entire operation on error.

    Moved the funtion "modify_altp2m_range" out of this commit.

v4: Use the functions "p2m_(set|get)_entry" instead of the helpers
    "p2m_lookup_attr" and "modify_altp2m_entry".
---
 xen/arch/arm/altp2m.c        | 84 ++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/p2m.c           |  4 +++
 xen/include/asm-arm/altp2m.h |  8 +++++
 3 files changed, 96 insertions(+)

diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
index 8c3212780a..4883b1323b 100644
--- a/xen/arch/arm/altp2m.c
+++ b/xen/arch/arm/altp2m.c
@@ -123,6 +123,90 @@ int altp2m_set_mem_access(struct domain *d,
     return rc;
 }
 
+static inline void altp2m_reset(struct p2m_domain *p2m)
+{
+    p2m_write_lock(p2m);
+    p2m_flush_table(p2m);
+    p2m_write_unlock(p2m);
+}
+
+int altp2m_propagate_change(struct domain *d,
+                            gfn_t sgfn,
+                            unsigned int page_order,
+                            mfn_t smfn,
+                            p2m_type_t p2mt,
+                            p2m_access_t p2ma)
+{
+    int rc = 0;
+    unsigned int i;
+    unsigned int reset_count = 0;
+    unsigned int last_reset_idx = ~0;
+    struct p2m_domain *p2m;
+    mfn_t m;
+
+    altp2m_lock(d);
+
+    if ( !altp2m_active(d) )
+        goto out;
+
+    for ( i = 0; i < MAX_ALTP2M; i++ )
+    {
+        p2m = d->arch.altp2m_p2m[i];
+
+        if ( p2m == NULL )
+            continue;
+
+        /*
+         * Get the altp2m mapping. If the smfn has not been dropped, a valid
+         * altp2m mapping needs to be changed/modified accordingly.
+         */
+        p2m_read_lock(p2m);
+        m = p2m_get_entry(p2m, sgfn, NULL, NULL, NULL);
+        p2m_read_unlock(p2m);
+
+        /* Check for a dropped page that may impact this altp2m. */
+        if ( mfn_eq(smfn, INVALID_MFN) &&
+             (gfn_x(sgfn) >= gfn_x(p2m->lowest_mapped_gfn)) &&
+             (gfn_x(sgfn) <= gfn_x(p2m->max_mapped_gfn)) )
+        {
+            if ( !reset_count++ )
+            {
+                altp2m_reset(p2m);
+                last_reset_idx = i;
+            }
+            else
+            {
+                /* At least 2 altp2m's impacted, so reset everything. */
+                for ( i = 0; i < MAX_ALTP2M; i++ )
+                {
+                    p2m = d->arch.altp2m_p2m[i];
+
+                    if ( i == last_reset_idx || p2m == NULL )
+                        continue;
+
+                    altp2m_reset(p2m);
+                }
+                goto out;
+            }
+        }
+        else if ( !mfn_eq(m, INVALID_MFN) )
+        {
+            /* Align the gfn and mfn to the given pager order. */
+            sgfn = _gfn(gfn_x(sgfn) & ~((1UL << page_order) - 1));
+            smfn = _mfn(mfn_x(smfn) & ~((1UL << page_order) - 1));
+
+            p2m_write_lock(p2m);
+            rc = p2m_set_entry(p2m, sgfn, (1UL << page_order), smfn, p2mt, p2ma);
+            p2m_write_unlock(p2m);
+        }
+    }
+
+out:
+    altp2m_unlock(d);
+
+    return rc;
+}
+
 static void altp2m_vcpu_reset(struct vcpu *v)
 {
     v->arch.ap2m_idx = INVALID_ALTP2M;
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index e9274c74a8..dcf7be6439 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -958,6 +958,10 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
     else
         rc = 0;
 
+    /* Update all affected altp2m views if necessary. */
+    if ( p2m_is_hostp2m(p2m) )
+        rc = altp2m_propagate_change(p2m->domain, sgfn, page_order, smfn, t, a);
+
 out:
     unmap_domain_page(table);
 
diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index f8e772f120..3e418cb0f0 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -80,4 +80,12 @@ int altp2m_set_mem_access(struct domain *d,
                           p2m_access_t a,
                           gfn_t gfn);
 
+/* Propagates changes made to hostp2m to affected altp2m views. */
+int altp2m_propagate_change(struct domain *d,
+                            gfn_t sgfn,
+                            unsigned int page_order,
+                            mfn_t smfn,
+                            p2m_type_t p2mt,
+                            p2m_access_t p2ma);
+
 #endif /* __ASM_ARM_ALTP2M_H */
-- 
2.13.3


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

[PATCH v4 28/39] altp2m: Rename p2m_altp2m_check to altp2m_check

[Sergej Proskurin]

In this commit, we rename the function "p2m_altp2m_check" to
"altp2m_check".  This is a preparation measure for the following commit
which moves the renamed function "altp2m_check" from p2m.c to altp2m.c
in order to group all altp2m-related functions to one spot (which is
altp2m.c). The reason for modifying the function's name is due the
association of the function with the associated .c file.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Tamas K Lengyel <tamas@tklengyel.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v4: In this commit, we have pulled the renaming of the function
    "p2m_altp2m_check" out of the previous commit "altp2m: Introduce
    altp2m_switch_vcpu_altp2m_by_id"
---
 xen/arch/x86/mm/p2m.c     | 2 +-
 xen/common/vm_event.c     | 2 +-
 xen/include/asm-arm/p2m.h | 2 +-
 xen/include/asm-x86/p2m.h | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index e8a57d118c..d5038ed66b 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1687,7 +1687,7 @@ void p2m_mem_paging_resume(struct domain *d, vm_event_response_t *rsp)
     }
 }
 
-void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
+void altp2m_check(struct vcpu *v, uint16_t idx)
 {
     if ( altp2m_active(v->domain) )
         p2m_switch_vcpu_altp2m_by_id(v, idx);
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 9291db61c5..42e6f09029 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -418,7 +418,7 @@ void vm_event_resume(struct domain *d, struct vm_event_domain *ved)
 
             /* Check for altp2m switch */
             if ( rsp.flags & VM_EVENT_FLAG_ALTERNATE_P2M )
-                p2m_altp2m_check(v, rsp.altp2m_idx);
+                altp2m_check(v, rsp.altp2m_idx);
 
             if ( rsp.flags & VM_EVENT_FLAG_SET_REGISTERS )
                 vm_event_set_registers(v, &rsp);
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index d3467daacf..5564473e26 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -186,7 +186,7 @@ typedef enum {
                              p2m_to_mask(p2m_map_foreign)))
 
 static inline
-void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
+void altp2m_check(struct vcpu *v, uint16_t idx)
 {
     /* Not supported on ARM. */
 }
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 6395e8fd1d..d1cc65f86d 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -804,7 +804,7 @@ static inline struct p2m_domain *p2m_get_altp2m(struct vcpu *v)
 bool_t p2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned int idx);
 
 /* Check to see if vcpu should be switched to a different p2m. */
-void p2m_altp2m_check(struct vcpu *v, uint16_t idx);
+void altp2m_check(struct vcpu *v, uint16_t idx);
 
 /* Flush all the alternate p2m's for a domain */
 void p2m_flush_altp2m(struct domain *d);
-- 
2.13.3


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

[PATCH v4 29/39] x86/altp2m: Move altp2m_check to altp2m.c

[Sergej Proskurin]

In this commit, we move the function "altp2m_check" from p2m.c to altp2m.c in
order to group all altp2m-related functions in one point, namely in
altp2m.{c|h}. This commit moves solely the x86 code.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Tamas K Lengyel <tamas@tklengyel.com>
---
v4: This commit has been pulled out of the previous commit "altp2m: Introduce
    altp2m_switch_vcpu_altp2m_by_id".
---
 xen/arch/x86/mm/altp2m.c     | 6 ++++++
 xen/arch/x86/mm/p2m.c        | 6 ------
 xen/common/vm_event.c        | 1 +
 xen/include/asm-x86/altp2m.h | 3 +++
 xen/include/asm-x86/p2m.h    | 3 ---
 5 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/mm/altp2m.c b/xen/arch/x86/mm/altp2m.c
index 930bdc2669..00abb5a5bb 100644
--- a/xen/arch/x86/mm/altp2m.c
+++ b/xen/arch/x86/mm/altp2m.c
@@ -65,6 +65,12 @@ altp2m_vcpu_destroy(struct vcpu *v)
         vcpu_unpause(v);
 }
 
+void altp2m_check(struct vcpu *v, uint16_t idx)
+{
+    if ( altp2m_active(v->domain) )
+        p2m_switch_vcpu_altp2m_by_id(v, idx);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index d5038ed66b..3feb6315c2 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1687,12 +1687,6 @@ void p2m_mem_paging_resume(struct domain *d, vm_event_response_t *rsp)
     }
 }
 
-void altp2m_check(struct vcpu *v, uint16_t idx)
-{
-    if ( altp2m_active(v->domain) )
-        p2m_switch_vcpu_altp2m_by_id(v, idx);
-}
-
 static struct p2m_domain *
 p2m_getlru_nestedp2m(struct domain *d, struct p2m_domain *p2m)
 {
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 42e6f09029..66f1d83d84 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -29,6 +29,7 @@
 #include <asm/monitor.h>
 #include <asm/vm_event.h>
 #include <xsm/xsm.h>
+#include <asm/altp2m.h>
 
 /* for public/io/ring.h macros */
 #define xen_mb()   smp_mb()
diff --git a/xen/include/asm-x86/altp2m.h b/xen/include/asm-x86/altp2m.h
index 64c761873e..67d0205612 100644
--- a/xen/include/asm-x86/altp2m.h
+++ b/xen/include/asm-x86/altp2m.h
@@ -38,4 +38,7 @@ static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
     return vcpu_altp2m(v).p2midx;
 }
 
+/* Check to see if vcpu should be switched to a different p2m. */
+void altp2m_check(struct vcpu *v, uint16_t idx);
+
 #endif /* __ASM_X86_ALTP2M_H */
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index d1cc65f86d..863d7559cb 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -803,9 +803,6 @@ static inline struct p2m_domain *p2m_get_altp2m(struct vcpu *v)
 /* Switch alternate p2m for a single vcpu */
 bool_t p2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned int idx);
 
-/* Check to see if vcpu should be switched to a different p2m. */
-void altp2m_check(struct vcpu *v, uint16_t idx);
-
 /* Flush all the alternate p2m's for a domain */
 void p2m_flush_altp2m(struct domain *d);
 
-- 
2.13.3


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

Re: [PATCH v4 29/39] x86/altp2m: Move altp2m_check to altp2m.c

[Razvan Cojocaru]

On 08/30/2017 09:32 PM, Sergej Proskurin wrote:
> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
> index 42e6f09029..66f1d83d84 100644
> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -29,6 +29,7 @@
>  #include <asm/monitor.h>
>  #include <asm/vm_event.h>
>  #include <xsm/xsm.h>
> +#include <asm/altp2m.h>

Any reason why this include has not happened alphabetically (it belongs
to the <asm/...> group)?


Thanks,
Razvan

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

Re: [PATCH v4 29/39] x86/altp2m: Move altp2m_check to altp2m.c

[Sergej Proskurin]

Hi Razvan,


On 08/30/2017 08:42 PM, Razvan Cojocaru wrote:
> On 08/30/2017 09:32 PM, Sergej Proskurin wrote:
>> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
>> index 42e6f09029..66f1d83d84 100644
>> --- a/xen/common/vm_event.c
>> +++ b/xen/common/vm_event.c
>> @@ -29,6 +29,7 @@
>>  #include <asm/monitor.h>
>>  #include <asm/vm_event.h>
>>  #include <xsm/xsm.h>
>> +#include <asm/altp2m.h>
> Any reason why this include has not happened alphabetically (it belongs
> to the <asm/...> group)?

I must have missed that, thank you. I am going to fix this in v5.

Cheers,
~Sergej

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

[PATCH v4 30/39] arm/altp2m: Move altp2m_check to altp2m.h

[Sergej Proskurin]

In this commit, we move the function "altp2m_check" from p2m.h to altp2m.h in
order to group all altp2m-related functions in one point, namely in
altp2m.{c|h}. This commit moves solely the arm code.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v4: This commit has been pulled out of the previous commit "altp2m: Introduce
    altp2m_switch_vcpu_altp2m_by_id".
---
 xen/include/asm-arm/altp2m.h | 7 +++++++
 xen/include/asm-arm/p2m.h    | 6 ------
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index 3e418cb0f0..5a2444e8f8 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -49,6 +49,13 @@ void altp2m_vcpu_destroy(struct vcpu *v);
 /* Get current alternate p2m table. */
 struct p2m_domain *altp2m_get_altp2m(struct vcpu *v);
 
+/* Check to see if vcpu should be switched to a different p2m. */
+static inline
+void altp2m_check(struct vcpu *v, uint16_t idx)
+{
+    /* Not supported on ARM. */
+}
+
 /* Switch alternate p2m for entire domain */
 int altp2m_switch_domain_altp2m_by_id(struct domain *d,
                                       unsigned int idx);
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 5564473e26..5a000d2f67 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -185,12 +185,6 @@ typedef enum {
                             (P2M_RAM_TYPES | P2M_GRANT_TYPES |  \
                              p2m_to_mask(p2m_map_foreign)))
 
-static inline
-void altp2m_check(struct vcpu *v, uint16_t idx)
-{
-    /* Not supported on ARM. */
-}
-
 /* Second stage paging setup, to be called on all CPUs */
 void setup_virt_paging(void);
 
-- 
2.13.3


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

[PATCH v4 31/39] arm/altp2m: Introduce altp2m_switch_vcpu_altp2m_by_id

[Sergej Proskurin]

This commit adds the function "altp2m_switch_vcpu_altp2m_by_id" that is
executed after checking whether the vcpu should be switched to a different
altp2m within the function "altp2m_check".

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v3: This commit has been moved out of the commit "arm/p2m: Add altp2m
    paging mechanism".

    Moved the function "p2m_altp2m_check" from p2m.c to altp2m.c and
    renamed it to "altp2m_check". This change required the adoption of
    the complementary function in the x86 architecture.

v4: Moved code renaming and movement of ARM and x86 related code out of
    this commit.

    While parts of this commit have been Acked-by Razvan Cojocaru and
    George Dunlap in v3, we have removed the Acks as the previous patch
    has been distributed across multiple smaller patches and now needs
    to be reviewed again.
---
 xen/arch/arm/altp2m.c        | 32 ++++++++++++++++++++++++++++++++
 xen/include/asm-arm/altp2m.h |  6 +-----
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
index 4883b1323b..9c9876c932 100644
--- a/xen/arch/arm/altp2m.c
+++ b/xen/arch/arm/altp2m.c
@@ -32,6 +32,38 @@ struct p2m_domain *altp2m_get_altp2m(struct vcpu *v)
     return v->domain->arch.altp2m_p2m[idx];
 }
 
+static bool altp2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned int idx)
+{
+    struct domain *d = v->domain;
+    bool rc = false;
+
+    if ( unlikely(idx >= MAX_ALTP2M) )
+        return rc;
+
+    altp2m_lock(d);
+
+    if ( d->arch.altp2m_p2m[idx] != NULL )
+    {
+        if ( idx != v->arch.ap2m_idx )
+        {
+            atomic_dec(&altp2m_get_altp2m(v)->active_vcpus);
+            v->arch.ap2m_idx = idx;
+            atomic_inc(&altp2m_get_altp2m(v)->active_vcpus);
+        }
+        rc = true;
+    }
+
+    altp2m_unlock(d);
+
+    return rc;
+}
+
+void altp2m_check(struct vcpu *v, uint16_t idx)
+{
+    if ( altp2m_active(v->domain) )
+        altp2m_switch_vcpu_altp2m_by_id(v, idx);
+}
+
 int altp2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx)
 {
     struct vcpu *v;
diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index 5a2444e8f8..f9e14ab1dc 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -50,11 +50,7 @@ void altp2m_vcpu_destroy(struct vcpu *v);
 struct p2m_domain *altp2m_get_altp2m(struct vcpu *v);
 
 /* Check to see if vcpu should be switched to a different p2m. */
-static inline
-void altp2m_check(struct vcpu *v, uint16_t idx)
-{
-    /* Not supported on ARM. */
-}
+void altp2m_check(struct vcpu *v, uint16_t idx);
 
 /* Switch alternate p2m for entire domain */
 int altp2m_switch_domain_altp2m_by_id(struct domain *d,
-- 
2.13.3


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

[PATCH v4 32/39] arm/altp2m: Make altp2m_vcpu_idx ready for altp2m

[Sergej Proskurin]

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
 xen/include/asm-arm/altp2m.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index f9e14ab1dc..eff6bd5a38 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -35,9 +35,7 @@ static inline bool_t altp2m_active(const struct domain *d)
 /* Alternate p2m VCPU */
 static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
 {
-    /* Not implemented on ARM, should not be reached. */
-    BUG();
-    return 0;
+    return v->arch.ap2m_idx;
 }
 
 int altp2m_init(struct domain *d);
-- 
2.13.3


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

[PATCH v4 33/39] arm/p2m: Add altp2m paging mechanism

[Sergej Proskurin]

This commit adds the function "altp2m_lazy_copy" implementing the altp2m
paging mechanism. The function "altp2m_lazy_copy" lazily copies the
hostp2m's mapping into the currently active altp2m view on 2nd stage
translation faults on instruction or data access.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v3: Cosmetic fixes.

    Locked hostp2m in the function "altp2m_lazy_copy" to avoid a mapping
    being changed in hostp2m before it has been inserted into the
    valtp2m view.

    Removed unnecessary calls to "p2m_mem_access_check" in the functions
    "do_trap_instr_abort_guest" and "do_trap_data_abort_guest" after a
    translation fault has been handled by the function
    "altp2m_lazy_copy".

    Adapted "altp2m_lazy_copy" to return the value "true" if the
    encountered translation fault encounters a valid entry inside of the
    currently active altp2m view. If multiple vcpus are using the same
    altp2m, it is likely that both generate a translation fault, whereas
    the first one will be already handled by "altp2m_lazy_copy". With
    this change the 2nd vcpu will retry accessing the faulting address.

    Changed order of altp2m checking and MMIO emulation within the
    function "do_trap_data_abort_guest".  Now, altp2m is checked and
    handled only if the MMIO does not have to be emulated.

    Changed the function prototype of "altp2m_lazy_copy".  This commit
    removes the unnecessary struct p2m_domain* from the previous
    function prototype.  Also, this commit removes the unnecessary
    argument gva.  Finally, this commit changes the address of the
    function parameter gpa from paddr_t to gfn_t and renames it to gfn.

    Moved the altp2m handling mechanism into a separate function
    "try_handle_altp2m".

    Moved the functions "p2m_altp2m_check" and
    "altp2m_switch_vcpu_altp2m_by_id" out of this patch.

    Moved applied code movement into a separate patch.

v4: Cosmetic fixes.

    Changed the function prototype of "altp2m_lazy_copy" and
    "try_handle_altp2m" by removing the unused function parameter of
    type "struct npfec".

    Removed the function "try_handle_altp2m".

    Please note that we cannot reorder the calls to "altp2m_lazy_copy"
    and "gfn_to_mfn" as to deprioritize altp2m. If the call to
    "gfn_to_mfn" would be performed before "altp2m_lazy_copy", the
    system would likely stall if altp2m was active. This is because the
    "p2m_lookup" routine in "gfn_to_mfn" considers only the host's p2m,
    which will most likely return a mfn != INVALID_MFN and thus entirely
    skip the call to "altp2m_lazy_copy".

    Use the functions "p2m_(set|get)_entry" instead of the helpers
    "p2m_lookup_attr" and "modify_altp2m_entry" in the function
    "altp2m_lazy_copy". Therefore, we write-lock the altp2m view
    throughout the entire function.

    Moved read-locking of hp2m to the beginning of the function
    "altp2m_lazy_copy".
---
 xen/arch/arm/altp2m.c        | 66 ++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/traps.c         | 17 ++++++++++++
 xen/include/asm-arm/altp2m.h |  4 +++
 3 files changed, 87 insertions(+)

diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
index 9c9876c932..fd455bdbfc 100644
--- a/xen/arch/arm/altp2m.c
+++ b/xen/arch/arm/altp2m.c
@@ -155,6 +155,72 @@ int altp2m_set_mem_access(struct domain *d,
     return rc;
 }
 
+/*
+ * The function altp2m_lazy_copy returns "false" on error.  The return value
+ * "true" signals that either the mapping has been successfully lazy-copied
+ * from the hostp2m to the currently active altp2m view or that the altp2m view
+ * holds already a valid mapping. The latter is the case if multiple vcpus
+ * using the same altp2m view generate a translation fault that is led back in
+ * both cases to the same mapping and the first fault has been already handled.
+ */
+bool altp2m_lazy_copy(struct vcpu *v, gfn_t gfn)
+{
+    struct domain *d = v->domain;
+    struct p2m_domain *hp2m = p2m_get_hostp2m(d), *ap2m = NULL;
+    p2m_type_t p2mt;
+    p2m_access_t p2ma;
+    mfn_t mfn;
+    unsigned int page_order;
+    int rc;
+
+    ap2m = altp2m_get_altp2m(v);
+    if ( unlikely(!ap2m) )
+        return false;
+
+    /*
+     * Lock hp2m to prevent the hostp2m to change a mapping before it is added
+     * to the altp2m view.
+     */
+    p2m_read_lock(hp2m);
+    p2m_write_lock(ap2m);
+
+    /* Check if entry is part of the altp2m view. */
+    mfn = p2m_get_entry(ap2m, gfn, NULL, NULL, NULL);
+
+    /*
+     * If multiple vcpus are using the same altp2m, it is likely that both
+     * generate a translation fault, whereas the first one will be handled
+     * successfully and the second will encounter a valid mapping that has
+     * already been added as a result of the previous translation fault. In
+     * this case, the 2nd vcpu needs to retry accessing the faulting address.
+     */
+    if ( !mfn_eq(mfn, INVALID_MFN) )
+        goto out;
+
+    /* Check if entry is part of the host p2m view. */
+    mfn = p2m_get_entry(hp2m, gfn, &p2mt, &p2ma, &page_order);
+    if ( mfn_eq(mfn, INVALID_MFN) )
+        goto out;
+
+    /* Align the gfn and mfn to the given pager order. */
+    gfn = _gfn(gfn_x(gfn) & ~((1UL << page_order) - 1));
+    mfn = _mfn(mfn_x(mfn) & ~((1UL << page_order) - 1));
+
+    rc = p2m_set_entry(ap2m, gfn, (1UL << page_order), mfn, p2mt, p2ma);
+    if ( rc )
+    {
+        gdprintk(XENLOG_ERR, "altp2m[%u] failed to set entry for %#"PRI_gfn" -> %#"PRI_mfn"\n",
+                 v->arch.ap2m_idx, gfn_x(gfn), mfn_x(mfn));
+        domain_crash(d);
+    }
+
+out:
+    p2m_write_unlock(ap2m);
+    p2m_read_unlock(hp2m);
+
+    return true;
+}
+
 static inline void altp2m_reset(struct p2m_domain *p2m)
 {
     p2m_write_lock(p2m);
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index aa838e8e77..3ef15d3100 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -52,6 +52,8 @@
 #include <asm/cpuerrata.h>
 #include <asm/acpi.h>
 
+#include <asm/altp2m.h>
+
 /* The base of the stack must always be double-word aligned, which means
  * that both the kernel half of struct cpu_user_regs (which is pushed in
  * entry.S) and struct cpu_info (which lives at the bottom of a Xen
@@ -2634,6 +2636,14 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
     }
     case FSC_FLT_TRANS:
         /*
+         * The guest shall retry accessing the page if the altp2m handler
+         * succeeds. Otherwise, we continue injecting an instruction abort
+         * exception.
+         */
+        if ( altp2m_lazy_copy(current, _gfn(paddr_to_pfn(gpa))) )
+            return;
+
+        /*
          * The PT walk may have failed because someone was playing
          * with the Stage-2 page table. Walk the Stage-2 PT to check
          * if the entry exists. If it's the case, return to the guest
@@ -2774,6 +2784,13 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
         }
 
         /*
+         * The guest shall retry accessing the page if the altp2m handler
+         * succeeds. Otherwise, we continue injecting a data abort exception.
+         */
+        if ( altp2m_lazy_copy(current, _gfn(paddr_to_pfn(info.gpa))) )
+            return;
+
+        /*
          * The PT walk may have failed because someone was playing
          * with the Stage-2 page table. Walk the Stage-2 PT to check
          * if the entry exists. If it's the case, return to the guest
diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index eff6bd5a38..4cdca63f01 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -81,6 +81,10 @@ int altp2m_set_mem_access(struct domain *d,
                           p2m_access_t a,
                           gfn_t gfn);
 
+/* Alternate p2m paging mechanism. */
+bool altp2m_lazy_copy(struct vcpu *v,
+                      gfn_t gfn);
+
 /* Propagates changes made to hostp2m to affected altp2m views. */
 int altp2m_propagate_change(struct domain *d,
                             gfn_t sgfn,
-- 
2.13.3


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

[PATCH v4 34/39] arm/p2m: Add HVMOP_altp2m_change_gfn

[Sergej Proskurin]

This commit adds the functionality to change mfn mappings for specified
gfn's in altp2m views. This mechanism can be used within the context of
VMI, e.g., to establish stealthy debugging.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v3: Moved the altp2m_lock to guard access to d->arch.altp2m_vttbr[idx]
    in altp2m_change_gfn.

    Locked hp2m to prevent hp2m entries from being modified while the
    function "altp2m_change_gfn" is active.

    Removed setting ap2m->mem_access_enabled in "altp2m_change_gfn", as
    we do not need explicitly splitting pages at this point.

    Extended checks allowing to change gfn's in p2m_ram_(rw|ro) memory
    only.

    Moved the funtion "remove_altp2m_entry" out of this commit.

v4: Cosmetic fixes.

    Moved the initialization of the ap2m pointer after having checked
    that the altp2m index and the associated altp2m view are valid.

    Use the functions "p2m_(set|get)_entry" instead of the helpers
    "p2m_lookup_attr", "remove_altp2m_entry", and "modify_altp2m_entry".

    Removed the call to altp2m_lock in "altp2m_change_gfn" as it is
    sufficient to read lock the host's p2m and write lock the indexed
    altp2m.

    We make sure that we do not remove a superpage by mistake if the
    user requests a specific gfn.

    Removed memaccess-related comment as (i) memaccess is handled by
    "p2m_set_entry" and (ii) we map always only one page and
    "p2m_set_entry" can handle splitting superpages if required.
---
 xen/arch/arm/altp2m.c        | 81 ++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/hvm.c           |  7 +++-
 xen/include/asm-arm/altp2m.h |  6 ++++
 3 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
index fd455bdbfc..37820e7b2a 100644
--- a/xen/arch/arm/altp2m.c
+++ b/xen/arch/arm/altp2m.c
@@ -305,6 +305,87 @@ out:
     return rc;
 }
 
+int altp2m_change_gfn(struct domain *d,
+                      unsigned int idx,
+                      gfn_t old_gfn,
+                      gfn_t new_gfn)
+{
+    struct p2m_domain *hp2m, *ap2m;
+    mfn_t mfn;
+    p2m_access_t p2ma;
+    p2m_type_t p2mt;
+    unsigned int page_order;
+    int rc = -EINVAL;
+
+    hp2m = p2m_get_hostp2m(d);
+
+    if ( idx >= MAX_ALTP2M || d->arch.altp2m_p2m[idx] == NULL )
+        return rc;
+
+    ap2m = d->arch.altp2m_p2m[idx];
+
+    p2m_read_lock(hp2m);
+    p2m_write_lock(ap2m);
+
+    mfn = p2m_get_entry(ap2m, old_gfn, &p2mt, NULL, NULL);
+
+    /* Check whether the page needs to be reset. */
+    if ( gfn_eq(new_gfn, INVALID_GFN) )
+    {
+        /* If mfn is mapped by old_gfn, remove old_gfn from the altp2m table. */
+        if ( !mfn_eq(mfn, INVALID_MFN) )
+            rc = p2m_set_entry(ap2m, old_gfn, (1UL << THIRD_ORDER), INVALID_MFN,
+                               p2m_invalid, p2m_access_rwx);
+
+        goto out;
+    }
+
+    /* Check hostp2m if no valid entry in altp2m present. */
+    if ( mfn_eq(mfn, INVALID_MFN) )
+    {
+        mfn = p2m_get_entry(hp2m, old_gfn, &p2mt, &p2ma, &page_order);
+
+        if ( mfn_eq(mfn, INVALID_MFN) ||
+             /* Allow changing gfn's in p2m_ram_(rw|ro) memory only. */
+             ((p2mt != p2m_ram_rw) && (p2mt != p2m_ram_ro)) )
+            goto out;
+
+        /* If this is a superpage, copy that first. */
+        if ( page_order != THIRD_ORDER )
+        {
+            /* Align the old_gfn and mfn to the given pager order. */
+            old_gfn = _gfn(gfn_x(old_gfn) & ~((1UL << page_order) - 1));
+            mfn = _mfn(mfn_x(mfn) & ~((1UL << page_order) - 1));
+
+            if ( p2m_set_entry(ap2m, old_gfn, (1UL << page_order), mfn, p2mt, p2ma) )
+                goto out;
+        }
+    }
+
+    mfn = p2m_get_entry(ap2m, new_gfn, &p2mt, &p2ma, NULL);
+
+    /* If new_gfn is not part of altp2m, get the mapping information from hp2m */
+    if ( mfn_eq(mfn, INVALID_MFN) )
+        mfn = p2m_get_entry(hp2m, new_gfn, &p2mt, &p2ma, NULL);
+
+    if ( mfn_eq(mfn, INVALID_MFN) ||
+         /* Allow changing gfn's in p2m_ram_(rw|ro) memory only. */
+         ((p2mt != p2m_ram_rw) && (p2mt != p2m_ram_ro)) )
+        goto out;
+
+    if ( p2m_set_entry(ap2m, old_gfn, (1UL << THIRD_ORDER), mfn, p2mt, p2ma) )
+        goto out;
+
+    rc = 0;
+
+out:
+    p2m_write_unlock(ap2m);
+    p2m_read_unlock(hp2m);
+
+    return rc;
+}
+
+
 static void altp2m_vcpu_reset(struct vcpu *v)
 {
     v->arch.ap2m_idx = INVALID_ALTP2M;
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index 7e91f2436d..8cf6db24a6 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -148,7 +148,12 @@ static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
 
     case HVMOP_altp2m_change_gfn:
-        rc = -EOPNOTSUPP;
+        if ( a.u.change_gfn.pad1 || a.u.change_gfn.pad2 )
+            rc = -EINVAL;
+        else
+            rc = altp2m_change_gfn(d, a.u.change_gfn.view,
+                                   _gfn(a.u.change_gfn.old_gfn),
+                                   _gfn(a.u.change_gfn.new_gfn));
         break;
     }
 
diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index 4cdca63f01..f5cf560371 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -93,4 +93,10 @@ int altp2m_propagate_change(struct domain *d,
                             p2m_type_t p2mt,
                             p2m_access_t p2ma);
 
+/* Change a gfn->mfn mapping */
+int altp2m_change_gfn(struct domain *d,
+                      unsigned int idx,
+                      gfn_t old_gfn,
+                      gfn_t new_gfn);
+
 #endif /* __ASM_ARM_ALTP2M_H */
-- 
2.13.3


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

[PATCH v4 35/39] arm/p2m: Adjust debug information to altp2m

[Sergej Proskurin]

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
v2: Dump p2m information of the hostp2m and all altp2m views.

v4: Adjust printk format.
---
 xen/arch/arm/p2m.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index dcf7be6439..db213bea20 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -103,6 +103,26 @@ void dump_p2m_lookup(struct domain *d, paddr_t addr)
 
     dump_pt_walk(page_to_maddr(p2m->root), addr,
                  P2M_ROOT_LEVEL, P2M_ROOT_PAGES);
+    printk("\n");
+
+    if ( altp2m_active(d) )
+    {
+        unsigned int i;
+
+        for ( i = 0; i < MAX_ALTP2M; i++ )
+        {
+            if ( d->arch.altp2m_p2m[i] == NULL )
+                continue;
+
+            p2m = d->arch.altp2m_p2m[i];
+
+            printk("AP2M[%u] @ %p mfn:%lx\n",
+                    i, p2m->root, __page_to_mfn(p2m->root));
+
+            dump_pt_walk(page_to_maddr(p2m->root), addr, P2M_ROOT_LEVEL, P2M_ROOT_PAGES);
+            printk("\n");
+        }
+    }
 }
 
 void p2m_save_state(struct vcpu *p)
-- 
2.13.3


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

[PATCH v4 36/39] altp2m: Document external-only use on ARM

[Sergej Proskurin]

From: Tamas K Lengyel <tamas.lengyel@zentific.com>

Currently, the altp2m feature has been used and thus documented for the
x86 architecture. As we aim to introduce altp2m to ARM, in this commit,
we adjust the documentation by pointing out x86 only parts and thus make
clear that the modes XEN_ALTP2M_external and XEN_ALTP2M_disabled are
also valid for the ARM architecture.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
v4: We added this patch to our patch series.
---
 docs/man/xl.cfg.pod.5.in | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index 79cb2eaea7..259cf18ea6 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -1380,7 +1380,7 @@ guest Operating Systems.
 
 =item B<altp2m="MODE">
 
-B<(x86 only)> Specifies the access mode to the alternate-p2m capability.
+Specifies the access mode to the alternate-p2m capability.
 Alternate-p2m allows a guest to manage multiple p2m guest physical "memory
 views" (as opposed to a single p2m).
 You may want this option if you want to access-control/isolate
@@ -1398,8 +1398,8 @@ Altp2m is disabled for the domain (default).
 
 =item B<mixed>
 
-The mixed mode allows access to the altp2m interface for both in-guest
-and external tools as well.
+B<(x86 only)> The mixed mode allows access to the altp2m interface for both
+in-guest and external tools as well.
 
 =item B<external>
 
@@ -1407,7 +1407,7 @@ Enables access to the alternate-p2m capability by external privileged tools.
 
 =item B<limited>
 
-Enables limited access to the alternate-p2m capability,
+B<(x86 only)> Enables limited access to the alternate-p2m capability,
 ie. giving the guest access only to enable/disable the VMFUNC and #VE features.
 
 =back
-- 
2.13.3


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

Re: [PATCH v4 36/39] altp2m: Document external-only use on ARM

[Wei Liu]

On Wed, Aug 30, 2017 at 08:32:55PM +0200, Sergej Proskurin wrote:
> From: Tamas K Lengyel <tamas.lengyel@zentific.com>
> 
> Currently, the altp2m feature has been used and thus documented for the
> x86 architecture. As we aim to introduce altp2m to ARM, in this commit,
> we adjust the documentation by pointing out x86 only parts and thus make
> clear that the modes XEN_ALTP2M_external and XEN_ALTP2M_disabled are
> also valid for the ARM architecture.
> 
> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

[PATCH v4 37/39] altp2m: Allow activating altp2m on ARM domains

[Sergej Proskurin]

The previous libxl implemention limited the use of altp2m to x86 HVM domains.
This commit extends libxl by introducing the altp2m switch to ARM domains.

Additionally, we introduce the macro LIBXL_HAVE_ARM_ALTP2M in parallel to the
former LIBXL_HAVE_ALTP2M to differentiate between altp2m for x86 and and altp2m
for ARM architectures. We also extend the documentation of the option "altp2m"
in ./docs/man/xl.cfg.pod.5.in.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.h         | 10 +++++++++-
 tools/libxl/libxl_dom.c     | 16 ++++++++++++++--
 tools/libxl/libxl_types.idl |  2 +-
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 17045253ab..e7af15bc45 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -872,11 +872,19 @@ typedef struct libxl__ctx libxl_ctx;
 
 /*
  * LIBXL_HAVE_ALTP2M
- * If this is defined, then libxl supports alternate p2m functionality.
+ * If this is defined, then libxl supports alternate p2m functionality for
+ * x86 HVM guests.
  */
 #define LIBXL_HAVE_ALTP2M 1
 
 /*
+ * LIBXL_HAVE_ARM_ALTP2M
+ * If this is defined, then libxl supports alternate p2m functionality for
+ * ARM guests.
+ */
+#define LIBXL_HAVE_ARM_ALTP2M 1
+
+/*
  * LIBXL_HAVE_REMUS
  * If this is defined, then libxl supports remus.
  */
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index f54fd49a73..db77c95a7e 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -314,6 +314,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
     libxl_domain_build_info *const info = &d_config->b_info;
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char *xs_domid, *con_domid;
+    bool altp2m_support = false;
     int rc;
     uint64_t size;
 
@@ -458,18 +459,29 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
 #endif
     }
 
+#if defined(__i386__) || defined(__x86_64__)
     /* Alternate p2m support on x86 is available only for HVM guests. */
-    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM)
+        altp2m_support = true;
+#elif defined(__arm__) || defined(__aarch64__)
+    /* Alternate p2m support on ARM is available for all guests. */
+    altp2m_support = true;
+#endif
+
+    if (altp2m_support) {
         /* The config parameter "altp2m" replaces the parameter "altp2mhvm". For
-         * legacy reasons, both parameters are accepted on x86 HVM guests.
+         * legacy reasons, both parameters are accepted on x86 HVM guests (only
+         * "altp2m" is accepted on ARM guests).
          *
          * If the legacy field info->u.hvm.altp2m is set, activate altp2m.
          * Otherwise set altp2m based on the field info->altp2m. */
+#if defined(__i386__) || defined(__x86_64__)
         if (info->altp2m == LIBXL_ALTP2M_MODE_DISABLED &&
             libxl_defbool_val(info->u.hvm.altp2m))
             xc_hvm_param_set(ctx->xch, domid, HVM_PARAM_ALTP2M,
                              libxl_defbool_val(info->u.hvm.altp2m));
         else
+#endif
             xc_hvm_param_set(ctx->xch, domid, HVM_PARAM_ALTP2M,
                              info->altp2m);
     }
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 6e80d36256..412a0b6129 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -583,7 +583,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
                               ])),
     # Alternate p2m is not bound to any architecture or guest type, as it is
-    # supported by x86 HVM and ARM support is planned.
+    # supported by x86 HVM and ARM domains.
     ("altp2m", libxl_altp2m_mode),
 
     ], dir=DIR_IN
-- 
2.13.3


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

Re: [PATCH v4 37/39] altp2m: Allow activating altp2m on ARM domains

[Wei Liu]

On Wed, Aug 30, 2017 at 08:32:56PM +0200, Sergej Proskurin wrote:
> The previous libxl implemention limited the use of altp2m to x86 HVM domains.
> This commit extends libxl by introducing the altp2m switch to ARM domains.
> 
> Additionally, we introduce the macro LIBXL_HAVE_ARM_ALTP2M in parallel to the
> former LIBXL_HAVE_ALTP2M to differentiate between altp2m for x86 and and altp2m
> for ARM architectures. We also extend the documentation of the option "altp2m"
> in ./docs/man/xl.cfg.pod.5.in.
> 
> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

[PATCH v4 38/39] arm/xen-access: Extend xen-access for altp2m on ARM

[Sergej Proskurin]

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Tamas K Lengyel <tamas@tklengyel.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/tests/xen-access/xen-access.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index 1e69e25a16..481337cacd 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -362,10 +362,11 @@ void usage(char* progname)
 {
     fprintf(stderr, "Usage: %s [-m] <domain_id> write|exec", progname);
 #if defined(__i386__) || defined(__x86_64__)
-            fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug|cpuid|desc_access|write_ctrlreg_cr4");
+            fprintf(stderr, "|breakpoint|debug|cpuid|desc_access|write_ctrlreg_cr4");
 #elif defined(__arm__) || defined(__aarch64__)
             fprintf(stderr, "|privcall");
 #endif
+            fprintf(stderr, "|altp2m_write|altp2m_exec");
             fprintf(stderr,
             "\n"
             "Logs first page writes, execs, or breakpoint traps that occur on the domain.\n"
@@ -441,18 +442,6 @@ int main(int argc, char *argv[])
     {
         breakpoint = 1;
     }
-    else if ( !strcmp(argv[0], "altp2m_write") )
-    {
-        default_access = XENMEM_access_rx;
-        altp2m = 1;
-        memaccess = 1;
-    }
-    else if ( !strcmp(argv[0], "altp2m_exec") )
-    {
-        default_access = XENMEM_access_rw;
-        altp2m = 1;
-        memaccess = 1;
-    }
     else if ( !strcmp(argv[0], "debug") )
     {
         debug = 1;
@@ -475,6 +464,18 @@ int main(int argc, char *argv[])
         privcall = 1;
     }
 #endif
+    else if ( !strcmp(argv[0], "altp2m_write") )
+    {
+        default_access = XENMEM_access_rx;
+        altp2m = 1;
+        memaccess = 1;
+    }
+    else if ( !strcmp(argv[0], "altp2m_exec") )
+    {
+        default_access = XENMEM_access_rw;
+        altp2m = 1;
+        memaccess = 1;
+    }
     else
     {
         usage(argv[0]);
@@ -547,12 +548,14 @@ int main(int argc, char *argv[])
             goto exit;
         }
 
+#if defined(__i386__) || defined(__x86_64__)
         rc = xc_monitor_singlestep( xch, domain_id, 1 );
         if ( rc < 0 )
         {
             ERROR("Error %d failed to enable singlestep monitoring!\n", rc);
             goto exit;
         }
+#endif
     }
 
     if ( memaccess && !altp2m )
@@ -663,7 +666,9 @@ int main(int argc, char *argv[])
                 rc = xc_altp2m_switch_to_view( xch, domain_id, 0 );
                 rc = xc_altp2m_destroy_view(xch, domain_id, altp2m_view_id);
                 rc = xc_altp2m_set_domain_state(xch, domain_id, 0);
+#if defined(__i386__) || defined(__x86_64__)
                 rc = xc_monitor_singlestep(xch, domain_id, 0);
+#endif
             } else {
                 rc = xc_set_mem_access(xch, domain_id, XENMEM_access_rwx, ~0ull, 0);
                 rc = xc_set_mem_access(xch, domain_id, XENMEM_access_rwx, START_PFN,
@@ -883,9 +888,11 @@ int main(int argc, char *argv[])
 exit:
     if ( altp2m )
     {
+#if defined(__i386__) || defined(__x86_64__)
         uint32_t vcpu_id;
         for ( vcpu_id = 0; vcpu_id<XEN_LEGACY_MAX_VCPUS; vcpu_id++)
             rc = control_singlestep(xch, domain_id, vcpu_id, 0);
+#endif
     }
 
     /* Tear down domain xenaccess */
-- 
2.13.3


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

[PATCH v4 39/39] arm/xen-access: Add test of xc_altp2m_change_gfn

[Sergej Proskurin]

This commit extends xen-access by a simple test of the functionality
provided by "xc_altp2m_change_gfn". The idea is to dynamically remap a
trapping gfn to another mfn, which holds the same content as the
original mfn. On success, the guest will continue to run. Subsequent
altp2m access violations will trap into Xen and be forced by xen-access
to switch to the default view (altp2m[0]) as before. The introduced test
can be invoked by providing the argument "altp2m_remap".

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Tamas K Lengyel <tamas@tklengyel.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
v3: Cosmetic fixes in "xenaccess_copy_gfn" and "xenaccess_change_gfn".

    Added munmap in "copy_gfn" in the second error case.

    Added option "altp2m_remap" selecting the altp2m-remap test.

v4: Dropped the COMPAT API for mapping foreign memory. Instead, we use the
    stable library xenforeignmemory.

    Dropped the use of xc_domain_increase_reservation_exact as we do not
    need to increase the domain's physical memory. Otherwise, remapping
    a page via altp2m would become visible to the guest itself. As long
    as we have additional shadow-memory for the guest domain, we do not
    need to reserve any additional memory.
---
 tools/tests/xen-access/Makefile     |   2 +-
 tools/tests/xen-access/xen-access.c | 182 +++++++++++++++++++++++++++++++++++-
 2 files changed, 179 insertions(+), 5 deletions(-)

diff --git a/tools/tests/xen-access/Makefile b/tools/tests/xen-access/Makefile
index e11f639ccf..ab195e233f 100644
--- a/tools/tests/xen-access/Makefile
+++ b/tools/tests/xen-access/Makefile
@@ -26,6 +26,6 @@ clean:
 distclean: clean
 
 xen-access: xen-access.o Makefile
-	$(CC) -o $@ $< $(LDFLAGS) $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenevtchn)
+	$(CC) -o $@ $< $(LDFLAGS) $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenevtchn) $(LDLIBS_libxenforeignmemory)
 
 -include $(DEPS)
diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index 481337cacd..f9b9fb6bbf 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -41,6 +41,7 @@
 #include <xenctrl.h>
 #include <xenevtchn.h>
 #include <xen/vm_event.h>
+#include <xenforeignmemory.h>
 
 #if defined(__arm__) || defined(__aarch64__)
 #include <xen/arch-arm.h>
@@ -49,6 +50,8 @@
 #define START_PFN 0ULL
 #endif
 
+#define INVALID_GFN ~(0UL)
+
 #define DPRINTF(a, b...) fprintf(stderr, a, ## b)
 #define ERROR(a, b...) fprintf(stderr, a "\n", ## b)
 #define PERROR(a, b...) fprintf(stderr, a ": %s\n", ## b, strerror(errno))
@@ -76,12 +79,19 @@ typedef struct vm_event {
 typedef struct xenaccess {
     xc_interface *xc_handle;
 
+    xenforeignmemory_handle *fmem;
+
     xen_pfn_t max_gpfn;
 
     vm_event_t vm_event;
+
+    unsigned int ap2m_idx;
+    xen_pfn_t gfn_old;
+    xen_pfn_t gfn_new;
 } xenaccess_t;
 
 static int interrupted;
+static int gfn_changed = 0;
 bool evtchn_bind = 0, evtchn_open = 0, mem_access_enable = 0;
 
 static void close_handler(int sig)
@@ -89,6 +99,104 @@ static void close_handler(int sig)
     interrupted = sig;
 }
 
+static int xenaccess_copy_gfn(xenaccess_t *xenaccess,
+                              domid_t domain_id,
+                              xen_pfn_t dst_gfn,
+                              xen_pfn_t src_gfn)
+{
+    void *src_vaddr = NULL;
+    void *dst_vaddr = NULL;
+
+    src_vaddr = xenforeignmemory_map(xenaccess->fmem, domain_id, PROT_READ,
+                                     1, &src_gfn, NULL);
+    if ( src_vaddr == NULL )
+        return -1;
+
+    dst_vaddr = xenforeignmemory_map(xenaccess->fmem, domain_id, PROT_WRITE,
+                                     1, &dst_gfn, NULL);
+    if ( dst_vaddr == NULL )
+    {
+        munmap(src_vaddr, XC_PAGE_SIZE);
+        return -1;
+    }
+
+    memcpy(dst_vaddr, src_vaddr, XC_PAGE_SIZE);
+
+    xenforeignmemory_unmap(xenaccess->fmem, src_vaddr, 1);
+    xenforeignmemory_unmap(xenaccess->fmem, dst_vaddr, 1);
+
+    return 0;
+}
+
+/*
+ * This function allocates and populates a page in the guest's physmap that is
+ * subsequently filled with contents of the trapping address. Finally, through
+ * the invocation of xc_altp2m_change_gfn, the altp2m subsystem changes the gfn
+ * to mfn mapping of the target altp2m view.
+ */
+static int xenaccess_change_gfn(xenaccess_t *xenaccess,
+                                domid_t domain_id,
+                                unsigned int ap2m_idx,
+                                xen_pfn_t gfn_old,
+                                xen_pfn_t *gfn_new)
+{
+    int rc;
+
+    /*
+     * We perform this function only once as it is intended to be used for
+     * testing and demonstration purposes. Thus, we signalize that further
+     * altp2m-related traps will not change trapping gfn's.
+     */
+    gfn_changed = 1;
+
+    *gfn_new = ++(xenaccess->max_gpfn);
+
+    rc = xc_domain_populate_physmap_exact(xenaccess->xc_handle, domain_id, 1, 0, 0, gfn_new);
+    if ( rc < 0 )
+        goto err;
+
+    /* Copy content of the old gfn into the newly allocated gfn */
+    rc = xenaccess_copy_gfn(xenaccess, domain_id, *gfn_new, gfn_old);
+    if ( rc < 0 )
+        goto err;
+
+    rc = xc_altp2m_change_gfn(xenaccess->xc_handle, domain_id, ap2m_idx, gfn_old, *gfn_new);
+    if ( rc < 0 )
+        goto err;
+
+    return 0;
+
+err:
+    xc_domain_decrease_reservation_exact(xenaccess->xc_handle, domain_id, 1, 0, gfn_new);
+
+    (xenaccess->max_gpfn)--;
+
+    return -1;
+}
+
+static int xenaccess_reset_gfn(xc_interface *xch,
+                               domid_t domain_id,
+                               unsigned int ap2m_idx,
+                               xen_pfn_t gfn_old,
+                               xen_pfn_t gfn_new)
+{
+    int rc;
+
+    /* Reset previous state */
+    xc_altp2m_change_gfn(xch, domain_id, ap2m_idx, gfn_old, INVALID_GFN);
+
+    /* Invalidate the new gfn */
+    xc_altp2m_change_gfn(xch, domain_id, ap2m_idx, gfn_new, INVALID_GFN);
+
+    rc = xc_domain_decrease_reservation_exact(xch, domain_id, 1, 0, &gfn_new);
+    if ( rc < 0 )
+        return -1;
+
+    (xenaccess->max_gpfn)--;
+
+    return 0;
+}
+
 int xc_wait_for_event_or_timeout(xc_interface *xch, xenevtchn_handle *xce, unsigned long ms)
 {
     struct pollfd fd = { .fd = xenevtchn_fd(xce), .events = POLLIN | POLLERR };
@@ -175,6 +283,13 @@ int xenaccess_teardown(xc_interface *xch, xenaccess_t *xenaccess)
         }
     }
 
+    /* Close Xen foreign memory interface */
+    rc = xenforeignmemory_close(xenaccess->fmem);
+    if ( rc != 0 )
+    {
+        ERROR("Error closing Xen foreign memory interface");
+    }
+
     /* Close connection to Xen */
     rc = xc_interface_close(xenaccess->xc_handle);
     if ( rc != 0 )
@@ -234,6 +349,10 @@ xenaccess_t *xenaccess_init(xc_interface **xch_r, domid_t domain_id)
     }
     mem_access_enable = 1;
 
+    xenaccess->ap2m_idx = ~(0);
+    xenaccess->gfn_old = INVALID_GFN;
+    xenaccess->gfn_new = INVALID_GFN;
+
     /* Open event channel */
     xenaccess->vm_event.xce_handle = xenevtchn_open(NULL, 0);
     if ( xenaccess->vm_event.xce_handle == NULL )
@@ -274,6 +393,13 @@ xenaccess_t *xenaccess_init(xc_interface **xch_r, domid_t domain_id)
 
     DPRINTF("max_gpfn = %"PRI_xen_pfn"\n", xenaccess->max_gpfn);
 
+    xenaccess->fmem = xenforeignmemory_open(0, 0);
+    if ( xenaccess->fmem == NULL )
+    {
+        ERROR("Failed to open xen foreign memory interface");
+        goto err;
+    }
+
     return xenaccess;
 
  err:
@@ -366,7 +492,7 @@ void usage(char* progname)
 #elif defined(__arm__) || defined(__aarch64__)
             fprintf(stderr, "|privcall");
 #endif
-            fprintf(stderr, "|altp2m_write|altp2m_exec");
+            fprintf(stderr, "|altp2m_write|altp2m_exec|altp2m_remap");
             fprintf(stderr,
             "\n"
             "Logs first page writes, execs, or breakpoint traps that occur on the domain.\n"
@@ -392,6 +518,7 @@ int main(int argc, char *argv[])
     int shutting_down = 0;
     int privcall = 0;
     int altp2m = 0;
+    int altp2m_remap = 0;
     int debug = 0;
     int cpuid = 0;
     int desc_access = 0;
@@ -476,6 +603,13 @@ int main(int argc, char *argv[])
         altp2m = 1;
         memaccess = 1;
     }
+    else if ( !strcmp(argv[0], "altp2m_remap") )
+    {
+        default_access = XENMEM_access_rw;
+        altp2m = 1;
+        altp2m_remap = 1;
+        memaccess = 1;
+    }
     else
     {
         usage(argv[0]);
@@ -669,6 +803,14 @@ int main(int argc, char *argv[])
 #if defined(__i386__) || defined(__x86_64__)
                 rc = xc_monitor_singlestep(xch, domain_id, 0);
 #endif
+
+                /* Reset remapped gfn. */
+                if ( altp2m_remap && xenaccess->gfn_new != INVALID_GFN )
+                    rc = xenaccess_reset_gfn(xenaccess->xc_handle,
+                                             xenaccess->vm_event.domain_id,
+                                             xenaccess->ap2m_idx,
+                                             xenaccess->gfn_old,
+                                             xenaccess->gfn_new);
             } else {
                 rc = xc_set_mem_access(xch, domain_id, XENMEM_access_rwx, ~0ull, 0);
                 rc = xc_set_mem_access(xch, domain_id, XENMEM_access_rwx, START_PFN,
@@ -742,10 +884,42 @@ int main(int argc, char *argv[])
 
                 if ( altp2m && req.flags & VM_EVENT_FLAG_ALTERNATE_P2M)
                 {
-                    DPRINTF("\tSwitching back to default view!\n");
-
                     rsp.flags |= (VM_EVENT_FLAG_ALTERNATE_P2M | VM_EVENT_FLAG_TOGGLE_SINGLESTEP);
-                    rsp.altp2m_idx = 0;
+
+                    if ( altp2m_remap )
+                    {
+                        if ( !gfn_changed )
+                        {
+                            /* Store trapping gfn and ap2m index for cleanup. */
+                            xenaccess->gfn_old = req.u.mem_access.gfn;
+                            xenaccess->ap2m_idx = req.altp2m_idx;
+
+                            /* Note that this function is called only once. */
+                            rc = xenaccess_change_gfn(xenaccess, domain_id, req.altp2m_idx,
+                                                      xenaccess->gfn_old, &xenaccess->gfn_new);
+                            if (rc < 0)
+                            {
+                                ERROR("Error remapping gfn=%"PRIx64"\n", xenaccess->gfn_old);
+                                interrupted = -1;
+                                continue;
+                            }
+
+                            /* Do not change the currently active altp2m view, yet. */
+                            rsp.altp2m_idx = req.altp2m_idx;
+                        }
+                        else
+                        {
+                            DPRINTF("\tSwitching back to default view!\n");
+
+                            rsp.altp2m_idx = 0;
+                        }
+                    }
+                    else
+                    {
+                        DPRINTF("\tSwitching back to default view!\n");
+
+                        rsp.altp2m_idx = 0;
+                    }
                 }
                 else if ( default_access != after_first_access )
                 {
-- 
2.13.3


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

Re: [PATCH v4 39/39] arm/xen-access: Add test of xc_altp2m_change_gfn

[Razvan Cojocaru]

On 08/30/2017 09:32 PM, Sergej Proskurin wrote:
> This commit extends xen-access by a simple test of the functionality
> provided by "xc_altp2m_change_gfn". The idea is to dynamically remap a
> trapping gfn to another mfn, which holds the same content as the
> original mfn. On success, the guest will continue to run. Subsequent
> altp2m access violations will trap into Xen and be forced by xen-access
> to switch to the default view (altp2m[0]) as before. The introduced test
> can be invoked by providing the argument "altp2m_remap".
> 
> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
> ---
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Cc: Tamas K Lengyel <tamas@tklengyel.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
> v3: Cosmetic fixes in "xenaccess_copy_gfn" and "xenaccess_change_gfn".
> 
>     Added munmap in "copy_gfn" in the second error case.
> 
>     Added option "altp2m_remap" selecting the altp2m-remap test.
> 
> v4: Dropped the COMPAT API for mapping foreign memory. Instead, we use the
>     stable library xenforeignmemory.
> 
>     Dropped the use of xc_domain_increase_reservation_exact as we do not
>     need to increase the domain's physical memory. Otherwise, remapping
>     a page via altp2m would become visible to the guest itself. As long
>     as we have additional shadow-memory for the guest domain, we do not
>     need to reserve any additional memory.
> ---
>  tools/tests/xen-access/Makefile     |   2 +-
>  tools/tests/xen-access/xen-access.c | 182 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 179 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/tests/xen-access/Makefile b/tools/tests/xen-access/Makefile
> index e11f639ccf..ab195e233f 100644
> --- a/tools/tests/xen-access/Makefile
> +++ b/tools/tests/xen-access/Makefile
> @@ -26,6 +26,6 @@ clean:
>  distclean: clean
>  
>  xen-access: xen-access.o Makefile
> -	$(CC) -o $@ $< $(LDFLAGS) $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenevtchn)
> +	$(CC) -o $@ $< $(LDFLAGS) $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenevtchn) $(LDLIBS_libxenforeignmemory)
>  
>  -include $(DEPS)
> diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
> index 481337cacd..f9b9fb6bbf 100644
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -41,6 +41,7 @@
>  #include <xenctrl.h>
>  #include <xenevtchn.h>
>  #include <xen/vm_event.h>
> +#include <xenforeignmemory.h>
>  
>  #if defined(__arm__) || defined(__aarch64__)
>  #include <xen/arch-arm.h>
> @@ -49,6 +50,8 @@
>  #define START_PFN 0ULL
>  #endif
>  
> +#define INVALID_GFN ~(0UL)
> +
>  #define DPRINTF(a, b...) fprintf(stderr, a, ## b)
>  #define ERROR(a, b...) fprintf(stderr, a "\n", ## b)
>  #define PERROR(a, b...) fprintf(stderr, a ": %s\n", ## b, strerror(errno))
> @@ -76,12 +79,19 @@ typedef struct vm_event {
>  typedef struct xenaccess {
>      xc_interface *xc_handle;
>  
> +    xenforeignmemory_handle *fmem;
> +
>      xen_pfn_t max_gpfn;
>  
>      vm_event_t vm_event;
> +
> +    unsigned int ap2m_idx;
> +    xen_pfn_t gfn_old;
> +    xen_pfn_t gfn_new;
>  } xenaccess_t;
>  
>  static int interrupted;
> +static int gfn_changed = 0;
>  bool evtchn_bind = 0, evtchn_open = 0, mem_access_enable = 0;
>  
>  static void close_handler(int sig)
> @@ -89,6 +99,104 @@ static void close_handler(int sig)
>      interrupted = sig;
>  }
>  
> +static int xenaccess_copy_gfn(xenaccess_t *xenaccess,
> +                              domid_t domain_id,
> +                              xen_pfn_t dst_gfn,
> +                              xen_pfn_t src_gfn)
> +{
> +    void *src_vaddr = NULL;
> +    void *dst_vaddr = NULL;
> +
> +    src_vaddr = xenforeignmemory_map(xenaccess->fmem, domain_id, PROT_READ,
> +                                     1, &src_gfn, NULL);
> +    if ( src_vaddr == NULL )
> +        return -1;
> +
> +    dst_vaddr = xenforeignmemory_map(xenaccess->fmem, domain_id, PROT_WRITE,
> +                                     1, &dst_gfn, NULL> +    if ( dst_vaddr == NULL )
> +    {
> +        munmap(src_vaddr, XC_PAGE_SIZE);
> +        return -1;
> +    }
> +
> +    memcpy(dst_vaddr, src_vaddr, XC_PAGE_SIZE);
> +
> +    xenforeignmemory_unmap(xenaccess->fmem, src_vaddr, 1);
> +    xenforeignmemory_unmap(xenaccess->fmem, dst_vaddr, 1);
> +
> +    return 0;
> +}
> +
> +/*
> + * This function allocates and populates a page in the guest's physmap that is
> + * subsequently filled with contents of the trapping address. Finally, through
> + * the invocation of xc_altp2m_change_gfn, the altp2m subsystem changes the gfn
> + * to mfn mapping of the target altp2m view.
> + */
> +static int xenaccess_change_gfn(xenaccess_t *xenaccess,
> +                                domid_t domain_id,
> +                                unsigned int ap2m_idx,
> +                                xen_pfn_t gfn_old,
> +                                xen_pfn_t *gfn_new)
> +{
> +    int rc;
> +
> +    /*
> +     * We perform this function only once as it is intended to be used for
> +     * testing and demonstration purposes. Thus, we signalize that further
> +     * altp2m-related traps will not change trapping gfn's.
> +     */
> +    gfn_changed = 1;
> +
> +    *gfn_new = ++(xenaccess->max_gpfn);

Unnecessary parentheses.

> +    rc = xc_domain_populate_physmap_exact(xenaccess->xc_handle, domain_id, 1, 0, 0, gfn_new);
> +    if ( rc < 0 )
> +        goto err;
> +
> +    /* Copy content of the old gfn into the newly allocated gfn */
> +    rc = xenaccess_copy_gfn(xenaccess, domain_id, *gfn_new, gfn_old);
> +    if ( rc < 0 )
> +        goto err;
> +
> +    rc = xc_altp2m_change_gfn(xenaccess->xc_handle, domain_id, ap2m_idx, gfn_old, *gfn_new);
> +    if ( rc < 0 )
> +        goto err;
> +
> +    return 0;
> +
> +err:
> +    xc_domain_decrease_reservation_exact(xenaccess->xc_handle, domain_id, 1, 0, gfn_new);
> +
> +    (xenaccess->max_gpfn)--;

Here too.

> +
> +    return -1;
> +}
> +
> +static int xenaccess_reset_gfn(xc_interface *xch,
> +                               domid_t domain_id,
> +                               unsigned int ap2m_idx,
> +                               xen_pfn_t gfn_old,
> +                               xen_pfn_t gfn_new)
> +{
> +    int rc;
> +
> +    /* Reset previous state */
> +    xc_altp2m_change_gfn(xch, domain_id, ap2m_idx, gfn_old, INVALID_GFN);
> +
> +    /* Invalidate the new gfn */
> +    xc_altp2m_change_gfn(xch, domain_id, ap2m_idx, gfn_new, INVALID_GFN);

Do these two xc_altp2m_change_gfn() calls not require error checking?

> +
> +    rc = xc_domain_decrease_reservation_exact(xch, domain_id, 1, 0, &gfn_new);
> +    if ( rc < 0 )
> +        return -1;
> +
> +    (xenaccess->max_gpfn)--;

Again, please remove the parentheses.


Thanks,
Razvan

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

Re: [PATCH v4 39/39] arm/xen-access: Add test of xc_altp2m_change_gfn

[Sergej Proskurin]

Hi Razvan,


[...]

>> +
>> +    *gfn_new = ++(xenaccess->max_gpfn);
> Unnecessary parentheses.
>

Thanks.

>> +    rc = xc_domain_populate_physmap_exact(xenaccess->xc_handle, domain_id, 1, 0, 0, gfn_new);
>> +    if ( rc < 0 )
>> +        goto err;
>> +
>> +    /* Copy content of the old gfn into the newly allocated gfn */
>> +    rc = xenaccess_copy_gfn(xenaccess, domain_id, *gfn_new, gfn_old);
>> +    if ( rc < 0 )
>> +        goto err;
>> +
>> +    rc = xc_altp2m_change_gfn(xenaccess->xc_handle, domain_id, ap2m_idx, gfn_old, *gfn_new);
>> +    if ( rc < 0 )
>> +        goto err;
>> +
>> +    return 0;
>> +
>> +err:
>> +    xc_domain_decrease_reservation_exact(xenaccess->xc_handle, domain_id, 1, 0, gfn_new);
>> +
>> +    (xenaccess->max_gpfn)--;
> Here too.
>
>> +
>> +    return -1;
>> +}
>> +
>> +static int xenaccess_reset_gfn(xc_interface *xch,
>> +                               domid_t domain_id,
>> +                               unsigned int ap2m_idx,
>> +                               xen_pfn_t gfn_old,
>> +                               xen_pfn_t gfn_new)
>> +{
>> +    int rc;
>> +
>> +    /* Reset previous state */
>> +    xc_altp2m_change_gfn(xch, domain_id, ap2m_idx, gfn_old, INVALID_GFN);
>> +
>> +    /* Invalidate the new gfn */
>> +    xc_altp2m_change_gfn(xch, domain_id, ap2m_idx, gfn_new, INVALID_GFN);
> Do these two xc_altp2m_change_gfn() calls not require error checking?
>
>> +
>> +    rc = xc_domain_decrease_reservation_exact(xch, domain_id, 1, 0, &gfn_new);
>> +    if ( rc < 0 )
>> +        return -1;
>> +
>> +    (xenaccess->max_gpfn)--;
> Again, please remove the parentheses.
>

Thanks again. I will adjust the implementation for v5.

Cheers,
~Sergej

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

Re: [PATCH v4 00/39] arm/altp2m: Introducing altp2m to ARM

[Sergej Proskurin]

Hi all,

just wanted to friendly remind you about the next altp2m on ARM patch
series, since it has been submitted for over a month now and got
somewhat lost on xen-devel.

I understand that it is too late to get this patch series into 4.10.
Yet, I would like to queue the series for 4.11. Please let me know if I
should wait for reviews until the end of the extended code freeze deadline.

Thanks,
~Sergej

On 08/30/2017 08:32 PM, Sergej Proskurin wrote:
> Hi all,
> 
> The following patch series can be found on Github[0] and is part of my
> contribution to last year's Google Summer of Code (GSoC)[1]. My project is
> managed by the organization The Honeynet Project. As part of GSoC, I was being
> supervised by the Xen maintainer Tamas K. Lengyel <tamas@tklengyel.com>, George
> D. Webster, and Steven Maresca.
> 
> In this patch series, we provide an implementation of the altp2m subsystem for
> ARM. Our implementation is based on the altp2m subsystem for x86, providing
> additional --alternate-- views on the guest's physical memory by means of the
> ARM 2nd stage translation mechanism. The patches introduce new HVMOPs and
> extend the p2m subsystem. Also, we extend libxl to support altp2m on ARM and
> modify xen-access to test the suggested functionality.
> 
> To be more precise, altp2m allows to create and switch to additional p2m views
> (i.e. gfn to mfn mappings). These views can be manipulated and activated as
> will through the provided HVMOPs. In this way, the active guest instance in
> question can seamlessly proceed execution without noticing that anything has
> changed. The prime scope of application of altp2m is Virtual Machine
> Introspection, where guest systems are analyzed from the outside of the VM.
> 
> Altp2m can be activated by means of the guest control parameter "altp2m" on x86
> and ARM architectures. For use-cases requiring purely external access to
> altp2m, this patch allows to specify if the altp2m interface should be external
> only.
> 
> This version is a revised version of v3 that has been submitted in 2016. It
> incorporates the comments of the previous patch series. Although the previous
> version has been submitted last year, I have kept the comments of the
> individual patches. Both the purpose and changes from v3 to v4 are stated
> inside the individual commits.
> 
> Best regards,
> ~Sergej
> 
> [0] https://github.com/sergej-proskurin/xen (branch arm-altp2m-v4)
> [1] https://summerofcode.withgoogle.com/projects/#4970052843470848
> 
> Sergej Proskurin (38):
>   arm/p2m: Introduce p2m_(switch|restore)_vttbr_and_(g|s)et_flags
>   arm/p2m: Add first altp2m HVMOP stubs
>   arm/p2m: Add hvm_allow_(set|get)_param
>   arm/p2m: Add HVMOP_altp2m_get_domain_state
>   arm/p2m: Introduce p2m_is_(hostp2m|altp2m)
>   arm/p2m: Cosmetic fix - substitute _gfn(ULONG_MAX) for INVALID_GFN
>   arm/p2m: Move hostp2m init/teardown to individual functions
>   arm/p2m: Cosmetic fix - function prototype of p2m_alloc_table
>   arm/p2m: Rename parameter in p2m_alloc_vmid
>   arm/p2m: Change func prototype and impl of p2m_(alloc|free)_vmid
>   altp2m: Move (MAX|INVALID)_ALTP2M to xen/p2m-common.h
>   arm/p2m: Add altp2m init/teardown routines
>   arm/p2m: Add altp2m table flushing routine
>   arm/p2m: Add HVMOP_altp2m_set_domain_state
>   arm/p2m: Add HVMOP_altp2m_create_p2m
>   arm/p2m: Add HVMOP_altp2m_destroy_p2m
>   arm/p2m: Add HVMOP_altp2m_switch_p2m
>   arm/p2m: Add p2m_get_active_p2m macro
>   arm/p2m: Make p2m_restore_state ready for altp2m
>   arm/p2m: Make get_page_from_gva ready for altp2m
>   arm/p2m: Cosmetic fix - __p2m_get_mem_access
>   arm/p2m: Make p2m_mem_access_check ready for altp2m
>   arm/p2m: Cosmetic fix - function prototypes
>   arm/p2m: Make p2m_put_l3_page ready for altp2m
>   arm/p2m: Modify reference count only if hostp2m active
>   arm/p2m: Add HVMOP_altp2m_set_mem_access
>   arm/p2m: Add altp2m_propagate_change
>   altp2m: Rename p2m_altp2m_check to altp2m_check
>   x86/altp2m: Move altp2m_check to altp2m.c
>   arm/altp2m: Move altp2m_check to altp2m.h
>   arm/altp2m: Introduce altp2m_switch_vcpu_altp2m_by_id
>   arm/altp2m: Make altp2m_vcpu_idx ready for altp2m
>   arm/p2m: Add altp2m paging mechanism
>   arm/p2m: Add HVMOP_altp2m_change_gfn
>   arm/p2m: Adjust debug information to altp2m
>   altp2m: Allow activating altp2m on ARM domains
>   arm/xen-access: Extend xen-access for altp2m on ARM
>   arm/xen-access: Add test of xc_altp2m_change_gfn
> 
> Tamas K Lengyel (1):
>   altp2m: Document external-only use on ARM
> 
>  docs/man/xl.cfg.pod.5.in            |   8 +-
>  tools/libxl/libxl.h                 |  10 +-
>  tools/libxl/libxl_dom.c             |  16 +-
>  tools/libxl/libxl_types.idl         |   2 +-
>  tools/tests/xen-access/Makefile     |   2 +-
>  tools/tests/xen-access/xen-access.c | 213 ++++++++++++-
>  xen/arch/arm/Makefile               |   1 +
>  xen/arch/arm/altp2m.c               | 601 ++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/hvm.c                  | 202 +++++++++++-
>  xen/arch/arm/mem_access.c           | 112 +++++--
>  xen/arch/arm/p2m.c                  | 219 +++++++++----
>  xen/arch/arm/traps.c                |  17 +
>  xen/arch/x86/mm/altp2m.c            |   6 +
>  xen/arch/x86/mm/p2m.c               |   6 -
>  xen/common/vm_event.c               |   3 +-
>  xen/include/asm-arm/altp2m.h        |  73 ++++-
>  xen/include/asm-arm/domain.h        |  15 +
>  xen/include/asm-arm/p2m.h           |  62 +++-
>  xen/include/asm-x86/altp2m.h        |   3 +
>  xen/include/asm-x86/domain.h        |   3 +-
>  xen/include/asm-x86/p2m.h           |   3 -
>  xen/include/xen/altp2m-common.h     |   8 +
>  22 files changed, 1444 insertions(+), 141 deletions(-)
>  create mode 100644 xen/arch/arm/altp2m.c
>  create mode 100644 xen/include/xen/altp2m-common.h
> 

-- 
Sergej Proskurin, M.Sc.
Wissenschaftlicher Mitarbeiter

Technische Universität München
Fakultät für Informatik
Lehrstuhl für Sicherheit in der Informatik

Boltzmannstraße 3
85748 Garching (bei München)

Tel. +49 (0)89 289-18592
Fax +49 (0)89 289-18579

proskurin@sec.in.tum.de
www.sec.in.tum.de

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

Re: [PATCH v4 00/39] arm/altp2m: Introducing altp2m to ARM

[Julien Grall]

On 07/10/2017 11:18, Sergej Proskurin wrote:
> Hi all,

Hello Sergej,

>
> just wanted to friendly remind you about the next altp2m on ARM patch
> series, since it has been submitted for over a month now and got
> somewhat lost on xen-devel.
>
> I understand that it is too late to get this patch series into 4.10.
> Yet, I would like to queue the series for 4.11. Please let me know if I
> should wait for reviews until the end of the extended code freeze deadline.

This is in my queue, I will have a look once I am done with 4.10 patches.

Cheers,

-- 
Julien Grall

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

Re: [PATCH v4 00/39] arm/altp2m: Introducing altp2m to ARM

[Sergej Proskurin]

Hi Julien,

On 10/07/2017 12:29 PM, Julien Grall wrote:
> 
> 
> On 07/10/2017 11:18, Sergej Proskurin wrote:
>> Hi all,
> 
> Hello Sergej,
> 
>>
>> just wanted to friendly remind you about the next altp2m on ARM patch
>> series, since it has been submitted for over a month now and got
>> somewhat lost on xen-devel.
>>
>> I understand that it is too late to get this patch series into 4.10.
>> Yet, I would like to queue the series for 4.11. Please let me know if I
>> should wait for reviews until the end of the extended code freeze
>> deadline.
> 
> This is in my queue, I will have a look once I am done with 4.10 patches.
> 

Alright, thank you.

Cheers,
~Sergej

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

Re: [PATCH v4 00/39] arm/altp2m: Introducing altp2m to ARM

[Julien Grall]

On 07/10/17 11:54, Sergej Proskurin wrote:
> Hi Julien,

Hello,

> 
> On 10/07/2017 12:29 PM, Julien Grall wrote:
>>
>>
>> On 07/10/2017 11:18, Sergej Proskurin wrote:
>>> Hi all,
>>
>> Hello Sergej,
>>
>>>
>>> just wanted to friendly remind you about the next altp2m on ARM patch
>>> series, since it has been submitted for over a month now and got
>>> somewhat lost on xen-devel.
>>>
>>> I understand that it is too late to get this patch series into 4.10.
>>> Yet, I would like to queue the series for 4.11. Please let me know if I
>>> should wait for reviews until the end of the extended code freeze
>>> deadline.
>>
>> This is in my queue, I will have a look once I am done with 4.10 patches.
>>
> 
> Alright, thank you.

I started to look at it today and then stopped when I saw my main 
comments on the version sent a year ago were not addressed.

The major one is having do_altp2m_op common between x86 and Arm. There 
are no reason to keep them separated and will be a source of problem in 
the future.

I will look the rest of the series once this is fixed.

Cheers,

-- 
Julien Grall

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