Re: [Xen-devel] [PATCH 01/18] arm/altp2m: Add cmd-line support for altp2m on ARM.

[Julien Grall <julien.grall@xxxxxxx>]

Hello Sergej,

On 04/07/16 12:45, Sergej Proskurin wrote:
> The Xen altp2m subsystem is currently supported only on x86-64 based
> architectures. By utilizing ARM's virtualization extensions, we intend
> to implement altp2m support for ARM architectures and thus further
> extend current Virtual Machine Introspection (VMI) capabilities on ARM.
>
> With this commit, Xen is now able to activate altp2m support on ARM by
> means of the command-line argument 'altp2m' (bool).
>
> Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
> ---
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> ---
>   xen/arch/arm/hvm.c            | 22 ++++++++++++++++++++
>   xen/include/asm-arm/hvm/hvm.h | 47 +++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 69 insertions(+)
>   create mode 100644 xen/include/asm-arm/hvm/hvm.h
>
> diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
> index d999bde..3615036 100644
> --- a/xen/arch/arm/hvm.c
> +++ b/xen/arch/arm/hvm.c
> @@ -32,6 +32,28 @@
>
>   #include <asm/hypercall.h>
>
> +#include <asm/hvm/hvm.h>
> +
> +/* Xen command-line option enabling altp2m */
> +static bool_t __initdata opt_altp2m_enabled = 0;
> +boolean_param("altp2m", opt_altp2m_enabled);
> +
> +struct hvm_function_table hvm_funcs __read_mostly = {
> +    .name = "ARM_HVM",
> +};
I don't see any reason to introduce hvm_function_table on ARM. This 
structure is used to know whether the hardware will support altp2m. 
However, based on your implementation, this feature will this will not 
depend on the hardware for ARM.
> +
> +/* Initcall enabling hvm functionality. */
> +static int __init hvm_enable(void)
> +{
> +    if ( opt_altp2m_enabled )
> +        hvm_funcs.altp2m_supported = 1;
> +    else
> +        hvm_funcs.altp2m_supported = 0;
> +
> +    return 0;
> +}
> +presmp_initcall(hvm_enable);
> +
>   long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
>   {
>       long rc = 0;
> diff --git a/xen/include/asm-arm/hvm/hvm.h b/xen/include/asm-arm/hvm/hvm.h
> new file mode 100644
> index 0000000..96c455c
> --- /dev/null
> +++ b/xen/include/asm-arm/hvm/hvm.h
> @@ -0,0 +1,47 @@
> +/*
> + * include/asm-arm/hvm/hvm.h
> + *
> + * Copyright (c) 2016, Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
> + *
> + * 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/>.
> + */
> +
> +#ifndef __ASM_ARM_HVM_HVM_H__
> +#define __ASM_ARM_HVM_HVM_H__
> +
> +struct hvm_function_table {
> +    char *name;
> +
> +    /* Necessary hardware support for alternate p2m's. */
> +    bool_t altp2m_supported;
> +};
> +
> +extern struct hvm_function_table hvm_funcs;
> +
> +/* Returns true if hardware supports alternate p2m's */
This comment is not true. The feature does not depend on the hardware 
for ARM.
> +static inline bool_t hvm_altp2m_supported(void)
> +{
> +    return hvm_funcs.altp2m_supported;
You could directly use opt_altp2m_enabled here.

> +}
> +
> +#endif /* __ASM_ARM_HVM_HVM_H__ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
Regards,

--
Julien Grall

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