diff mbox

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

Message ID 20160704114605.10086-2-proskurin@sec.in.tum.de (mailing list archive)
State New, archived
Headers show

Commit Message

Sergej Proskurin July 4, 2016, 11:45 a.m. UTC
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@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
 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

Comments

Andrew Cooper July 4, 2016, 12:15 p.m. UTC | #1
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@sec.in.tum.de>

In addition, please patch docs/misc/xen-command-line.markdown to
indicate that the altp2m option is available for x86 Intel and ARM.

~Andrew
Sergej Proskurin July 4, 2016, 1:02 p.m. UTC | #2
On 07/04/2016 02:15 PM, Andrew Cooper wrote:
> 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@sec.in.tum.de>
> 
> In addition, please patch docs/misc/xen-command-line.markdown to
> indicate that the altp2m option is available for x86 Intel and ARM.
> 
> ~Andrew
> 

Will do, thank you.

Cheers,
Sergej
Julien Grall July 4, 2016, 1:25 p.m. UTC | #3
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@sec.in.tum.de>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> ---
>   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@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/>.
> + */
> +
> +#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,
Sergej Proskurin July 4, 2016, 1:43 p.m. UTC | #4
Hello Julien,

On 07/04/2016 03:25 PM, Julien Grall wrote:
> 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@sec.in.tum.de>
>> ---
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Cc: Julien Grall <julien.grall@arm.com>
>> ---
>>   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.
> 

This is true: hvm_function_table is not of crucial importance. During
the implementation, we decided to pull out arch-independent parts out
the x86 and ARM implementation (still needs to be done) and hence reused
as much code as possible. However, this struct can be left out.

>> +
>> +/* 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@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/>.
>> + */
>> +
>> +#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.
> 

True. I will change that.

>> +static inline bool_t hvm_altp2m_supported(void)
>> +{
>> +    return hvm_funcs.altp2m_supported;
> 
> You could directly use opt_altp2m_enabled here.
> 

Ok, thank you.

>> +}
>> +
>> +#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,
> 

Thank you.

Cheers, Sergej
Julien Grall July 4, 2016, 5:42 p.m. UTC | #5
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).

Thinking a bit more about this patch. Why do we need an option to enable 
altp2m? All the code could be gated by the value of HVM_PARAM_ALTP2M.

Regards,
Tamas K Lengyel July 4, 2016, 5:56 p.m. UTC | #6
On Mon, Jul 4, 2016 at 11:42 AM, Julien Grall <julien.grall@arm.com> wrote:
>
>
> 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).
>
>
> Thinking a bit more about this patch. Why do we need an option to enable
> altp2m? All the code could be gated by the value of HVM_PARAM_ALTP2M.
>

I think the reasoning for having the altp2m boot parameter when it was
introduced for x86 was that altp2m is considered experimental. IMHO it
is stable enough so that we can forgo the boot param altogether.

Tamas
Sergej Proskurin July 4, 2016, 9:08 p.m. UTC | #7
On 07/04/2016 07:56 PM, Tamas K Lengyel wrote:
> On Mon, Jul 4, 2016 at 11:42 AM, Julien Grall <julien.grall@arm.com> wrote:
>>
>>
>> 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).
>>
>>
>> Thinking a bit more about this patch. Why do we need an option to enable
>> altp2m? All the code could be gated by the value of HVM_PARAM_ALTP2M.
>>
> 
> I think the reasoning for having the altp2m boot parameter when it was
> introduced for x86 was that altp2m is considered experimental. IMHO it
> is stable enough so that we can forgo the boot param altogether.
> 
> Tamas
> 

That would definitely remove the need for the altp2m initcall mechanism.
diff mbox

Patch

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",
+};
+
+/* 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@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/>.
+ */
+
+#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 */
+static inline bool_t hvm_altp2m_supported(void)
+{
+    return hvm_funcs.altp2m_supported;
+}
+
+#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:
+ */