Re: [Xen-devel] [PATCH v2 01/25] arm/altp2m: Add first altp2m HVMOP stubs.

[Julien Grall <julien.grall@xxxxxxx>]

Hello Sergej,

On 01/08/16 18:10, Sergej Proskurin wrote:
> This commit moves 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.
>
> 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@xxxxxxxxxxxxx>
> ---
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> ---
> v2: Removed altp2m command-line option: Guard through HVM_PARAM_ALTP2M.
>     Removed not used altp2m helper stubs in altp2m.h.
> ---
>  xen/arch/arm/hvm.c           | 79 ++++++++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/altp2m.h |  4 +--
>  xen/include/asm-arm/domain.h |  3 ++
>  3 files changed, 84 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
> index d999bde..eb524ae 100644
> --- a/xen/arch/arm/hvm.c
> +++ b/xen/arch/arm/hvm.c
> @@ -32,6 +32,81 @@
>
>  #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;
> +    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;
> +
> +    if ( (a.cmd != HVMOP_altp2m_get_domain_state) &&
> +         (a.cmd != HVMOP_altp2m_set_domain_state) &&
> +         !d->arch.altp2m_active )
Why not using altp2m_active(d) here?

Also this check looks quite racy. What does prevent another CPU to 
disable altp2m at the same time? How the code would behave?
Regards,

--
Julien Grall

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