ACRN is a flexible, lightweight reference hypervisor, built with real-time
and safety-criticality in mind, optimized to streamline embedded development
through an open source platform. It is built for embedded IOT with small
footprint and real-time features. More details can be found
in https://projectacrn.org/
This is the patch set that allows the Linux to work on ACRN hypervisor and it can
work with the following patch set to manage the Linux guest on acrn-hypervisor. It
includes the detection of acrn_hypervisor, upcall notification vector from
hypervisor, hypercall. The hypervisor detection is similar to Xen/VMWARE/Hyperv.
ACRN also uses the upcall notification mechanism similar to that in Xen/Microsoft
HyperV when it needs to send the notification to Linux OS. The hypercall provides
the mechanism that can be used to query/configure the acrn-hypervisor by Linux guest.
Following this patch set, we will send acrn driver part, which provides the interface
that can be used to manage the virtualized CPU/memory/device/interrupt for other guest
OS after the acrn hypervisor is detected.
v1->v2: Change the CONFIG_ACRN to CONFIG_ACRN_GUEST, which makes it easy to
understand.
Remove the export of x86_hyper_acrn.
Remove the unused API definition of acrn_setup_intr_handler and
acrn_remove_intr_handler.
Adjust the order of header file
Add the declaration of acrn_hv_vector_handler and tracing
definition of acrn_hv_callback_vector.
Refine the comments for the function of acrn_hypercall0/1/2
Zhao Yakui (3):
arch/x86: add the support of ACRN guest
arch/x86/acrn: Use HYPERVISOR_CALLBACK_VECTOR for Acrn upcall vector
arch/x86/acrn: add hypercall for acrn_guest
arch/x86/Kconfig | 8 ++++
arch/x86/entry/entry_64.S | 5 +++
arch/x86/include/asm/acrn_hypercall.h | 84 +++++++++++++++++++++++++++++++++++
arch/x86/include/asm/acrnhyper.h | 19 ++++++++
arch/x86/include/asm/hardirq.h | 3 +-
arch/x86/include/asm/hypervisor.h | 1 +
arch/x86/kernel/cpu/Makefile | 1 +
arch/x86/kernel/cpu/acrn.c | 57 ++++++++++++++++++++++++
arch/x86/kernel/cpu/hypervisor.c | 4 ++
arch/x86/kernel/irq.c | 3 +-
10 files changed, 183 insertions(+), 2 deletions(-)
create mode 100644 arch/x86/include/asm/acrn_hypercall.h
create mode 100644 arch/x86/include/asm/acrnhyper.h
create mode 100644 arch/x86/kernel/cpu/acrn.c
--
2.7.4
When acrn_hypervisor is detected, the hypercall is needed so that the
acrn guest can query/config some settings. For example: it can be used
to query the resources in hypervisor and manage the CPU/memory/device/
interrupt for Guest system.
So the hypercall is added so that the kernel can communicate with the
low-level acrn-hypervisor. On x86 it is implemented by using vmacll when
the acrn hypervisor is enabled.
V1->V2: Refine the comments for the function of acrn_hypercall0/1/2
Co-developed-by: Jason Chen CJ <[email protected]>
Signed-off-by: Jason Chen CJ <[email protected]>
Signed-off-by: Zhao Yakui <[email protected]>
---
arch/x86/include/asm/acrn_hypercall.h | 84 +++++++++++++++++++++++++++++++++++
1 file changed, 84 insertions(+)
create mode 100644 arch/x86/include/asm/acrn_hypercall.h
diff --git a/arch/x86/include/asm/acrn_hypercall.h b/arch/x86/include/asm/acrn_hypercall.h
new file mode 100644
index 0000000..3262935
--- /dev/null
+++ b/arch/x86/include/asm/acrn_hypercall.h
@@ -0,0 +1,84 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * acrn_hypercall.h : acrn hypervisor call API
+ */
+
+#ifndef __ACRN_HYPERCALL_H__
+#define __ACRN_HYPERCALL_H__
+
+#include <linux/errno.h>
+
+#ifdef CONFIG_ACRN_GUEST
+
+/*
+ * Hypercalls for ACRN Guest
+ *
+ * Hypercall number is passed in r8 register.
+ * Return value will be placed in rax.
+ * Up to 2 arguments are passed in rdi, rsi.
+ */
+
+static inline long acrn_hypercall0(unsigned long hcall_id)
+{
+
+ register long result asm("rax");
+ register unsigned long r8 asm("r8") = hcall_id;
+
+ asm volatile(".byte 0x0F,0x01,0xC1\n"
+ : "=r"(result)
+ : "r"(r8));
+
+ return result;
+}
+
+static inline long acrn_hypercall1(unsigned long hcall_id,
+ unsigned long param1)
+{
+
+ register long result asm("rax");
+ register unsigned long r8 asm("r8") = hcall_id;
+
+ asm volatile(".byte 0x0F,0x01,0xC1\n"
+ : "=r"(result)
+ : "D"(param1), "r"(r8));
+
+ return result;
+}
+
+static inline long acrn_hypercall2(unsigned long hcall_id,
+ unsigned long param1,
+ unsigned long param2)
+{
+
+ register long result asm("rax");
+ register unsigned long r8 asm("r8") = hcall_id;
+
+ asm volatile(".byte 0x0F,0x01,0xC1\n"
+ : "=r"(result)
+ : "D"(param1), "S"(param2), "r"(r8));
+
+ return result;
+}
+
+#else
+
+static inline long acrn_hypercall0(unsigned long hcall_id)
+{
+ return -ENOTSUPP;
+}
+
+static inline long acrn_hypercall1(unsigned long hcall_id,
+ unsigned long param1)
+{
+ return -ENOTSUPP;
+}
+
+static inline long acrn_hypercall2(unsigned long hcall_id,
+ unsigned long param1,
+ unsigned long param2)
+{
+ return -ENOTSUPP;
+}
+#endif
+
+#endif /* __ACRN_HYPERCALL_H__ */
--
2.7.4
ACRN is one open-source hypervisour, which is maintained by Linux
foundation. This is to add the para-virtualization support so that
it allows the Linux guest to run on acrn-hypervisor.
This adds x86_hyper_acrn into supported hypervisors array, which enables
Linux ACRN guest running on ACRN hypervisor. It is restricted to X86_64.
v1->v2: Change the CONFIG_ACRN to CONFIG_ACRN_GUEST, which makes it easy to
understand.
Remove the export of x86_hyper_acrn.
Co-developed-by: Jason Chen CJ <[email protected]>
Signed-off-by: Jason Chen CJ <[email protected]>
Signed-off-by: Zhao Yakui <[email protected]>
---
arch/x86/Kconfig | 8 ++++++++
arch/x86/include/asm/hypervisor.h | 1 +
arch/x86/kernel/cpu/Makefile | 1 +
arch/x86/kernel/cpu/acrn.c | 35 +++++++++++++++++++++++++++++++++++
arch/x86/kernel/cpu/hypervisor.c | 4 ++++
5 files changed, 49 insertions(+)
create mode 100644 arch/x86/kernel/cpu/acrn.c
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c1f9b3c..d73225e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -842,6 +842,14 @@ config JAILHOUSE_GUEST
cell. You can leave this option disabled if you only want to start
Jailhouse and run Linux afterwards in the root cell.
+config ACRN_GUEST
+ bool "ACRN Guest support"
+ depends on X86_64 && PARAVIRT
+ help
+ This option allows to run Linux as guest in ACRN hypervisor. Enabling
+ this will allow the kernel to boot in a paravirtualized environment
+ under the ACRN hypervisor.
+
endif #HYPERVISOR_GUEST
source "arch/x86/Kconfig.cpu"
diff --git a/arch/x86/include/asm/hypervisor.h b/arch/x86/include/asm/hypervisor.h
index 8c5aaba..50a30f6 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -29,6 +29,7 @@ enum x86_hypervisor_type {
X86_HYPER_XEN_HVM,
X86_HYPER_KVM,
X86_HYPER_JAILHOUSE,
+ X86_HYPER_ACRN,
};
#ifdef CONFIG_HYPERVISOR_GUEST
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index cfd24f9..17a7cdf 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_X86_CPU_RESCTRL) += resctrl/
obj-$(CONFIG_X86_LOCAL_APIC) += perfctr-watchdog.o
obj-$(CONFIG_HYPERVISOR_GUEST) += vmware.o hypervisor.o mshyperv.o
+obj-$(CONFIG_ACRN_GUEST) += acrn.o
ifdef CONFIG_X86_FEATURE_NAMES
quiet_cmd_mkcapflags = MKCAP $@
diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
new file mode 100644
index 0000000..3956567
--- /dev/null
+++ b/arch/x86/kernel/cpu/acrn.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ACRN detection support
+ *
+ * Copyright (C) 2019 Intel Corporation. All rights reserved.
+ *
+ * Jason Chen CJ <[email protected]>
+ * Zhao Yakui <[email protected]>
+ *
+ */
+
+#include <asm/hypervisor.h>
+
+static uint32_t __init acrn_detect(void)
+{
+ return hypervisor_cpuid_base("ACRNACRNACRN\0\0", 0);
+}
+
+static void __init acrn_init_platform(void)
+{
+}
+
+static bool acrn_x2apic_available(void)
+{
+ /* do not support x2apic */
+ return false;
+}
+
+const __initconst struct hypervisor_x86 x86_hyper_acrn = {
+ .name = "ACRN",
+ .detect = acrn_detect,
+ .type = X86_HYPER_ACRN,
+ .init.init_platform = acrn_init_platform,
+ .init.x2apic_available = acrn_x2apic_available,
+};
diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
index 479ca47..87e39ad 100644
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -32,6 +32,7 @@ extern const struct hypervisor_x86 x86_hyper_xen_pv;
extern const struct hypervisor_x86 x86_hyper_xen_hvm;
extern const struct hypervisor_x86 x86_hyper_kvm;
extern const struct hypervisor_x86 x86_hyper_jailhouse;
+extern const struct hypervisor_x86 x86_hyper_acrn;
static const __initconst struct hypervisor_x86 * const hypervisors[] =
{
@@ -49,6 +50,9 @@ static const __initconst struct hypervisor_x86 * const hypervisors[] =
#ifdef CONFIG_JAILHOUSE_GUEST
&x86_hyper_jailhouse,
#endif
+#ifdef CONFIG_ACRN_GUEST
+ &x86_hyper_acrn,
+#endif
};
enum x86_hypervisor_type x86_hyper_type;
--
2.7.4
Linux kernel uses the HYPERVISOR_CALLBACK_VECTOR for hypervisor upcall
vector. And it is already used for Xen and HyperV.
After Acrn hypervisor is detected, it will also use this defined vector
to notify kernel.
V1->V2: Remove the unused API definition of acrn_setup_intr_handler and
acrn_remove_intr_handler.
Adjust the order of header file
Add the declaration of acrn_hv_vector_handler and tracing
definition of acrn_hv_callback_vector.
Co-developed-by: Jason Chen CJ <[email protected]>
Signed-off-by: Jason Chen CJ <[email protected]>
Signed-off-by: Zhao Yakui <[email protected]>
---
arch/x86/entry/entry_64.S | 5 +++++
arch/x86/include/asm/acrnhyper.h | 19 +++++++++++++++++++
arch/x86/include/asm/hardirq.h | 3 ++-
arch/x86/kernel/cpu/acrn.c | 22 ++++++++++++++++++++++
arch/x86/kernel/irq.c | 3 ++-
5 files changed, 50 insertions(+), 2 deletions(-)
create mode 100644 arch/x86/include/asm/acrnhyper.h
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 1f0efdb..d1b8ad3 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1129,6 +1129,11 @@ apicinterrupt3 HYPERV_STIMER0_VECTOR \
hv_stimer0_callback_vector hv_stimer0_vector_handler
#endif /* CONFIG_HYPERV */
+#if IS_ENABLED(CONFIG_ACRN_GUEST)
+apicinterrupt3 HYPERVISOR_CALLBACK_VECTOR \
+ acrn_hv_callback_vector acrn_hv_vector_handler
+#endif
+
idtentry debug do_debug has_error_code=0 paranoid=1 shift_ist=DEBUG_STACK
idtentry int3 do_int3 has_error_code=0
idtentry stack_segment do_stack_segment has_error_code=1
diff --git a/arch/x86/include/asm/acrnhyper.h b/arch/x86/include/asm/acrnhyper.h
new file mode 100644
index 0000000..9f9c239
--- /dev/null
+++ b/arch/x86/include/asm/acrnhyper.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_ACRNHYPER_H
+#define _ASM_X86_ACRNHYPER_H
+
+#include <linux/types.h>
+#include <linux/atomic.h>
+#include <linux/nmi.h>
+#include <asm/io.h>
+
+#ifdef CONFIG_ACRN_GUEST
+void acrn_hv_callback_vector(void);
+#ifdef CONFIG_TRACING
+#define trace_acrn_hv_callback_vector acrn_hv_callback_vector
+#endif
+
+void acrn_hv_vector_handler(struct pt_regs *regs);
+#endif
+
+#endif
diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h
index d9069bb..a8f4d08 100644
--- a/arch/x86/include/asm/hardirq.h
+++ b/arch/x86/include/asm/hardirq.h
@@ -37,7 +37,8 @@ typedef struct {
#ifdef CONFIG_X86_MCE_AMD
unsigned int irq_deferred_error_count;
#endif
-#if IS_ENABLED(CONFIG_HYPERV) || defined(CONFIG_XEN)
+#if IS_ENABLED(CONFIG_HYPERV) || defined(CONFIG_XEN) ||\
+ defined(CONFIG_ACRN_GUEST)
unsigned int irq_hv_callback_count;
#endif
#if IS_ENABLED(CONFIG_HYPERV)
diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
index 3956567..7a233b5 100644
--- a/arch/x86/kernel/cpu/acrn.c
+++ b/arch/x86/kernel/cpu/acrn.c
@@ -9,7 +9,11 @@
*
*/
+#include <linux/interrupt.h>
#include <asm/hypervisor.h>
+#include <asm/acrnhyper.h>
+#include <asm/desc.h>
+#include <asm/irq_regs.h>
static uint32_t __init acrn_detect(void)
{
@@ -18,6 +22,8 @@ static uint32_t __init acrn_detect(void)
static void __init acrn_init_platform(void)
{
+ alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR,
+ acrn_hv_callback_vector);
}
static bool acrn_x2apic_available(void)
@@ -26,6 +32,22 @@ static bool acrn_x2apic_available(void)
return false;
}
+static void (*acrn_intr_handler)(void);
+
+__visible void __irq_entry acrn_hv_vector_handler(struct pt_regs *regs)
+{
+ struct pt_regs *old_regs = set_irq_regs(regs);
+
+ entering_ack_irq();
+ inc_irq_stat(irq_hv_callback_count);
+
+ if (acrn_intr_handler)
+ acrn_intr_handler();
+
+ exiting_irq();
+ set_irq_regs(old_regs);
+}
+
const __initconst struct hypervisor_x86 x86_hyper_acrn = {
.name = "ACRN",
.detect = acrn_detect,
diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index 59b5f2e..02c6ff6 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -134,7 +134,8 @@ int arch_show_interrupts(struct seq_file *p, int prec)
seq_printf(p, "%10u ", per_cpu(mce_poll_count, j));
seq_puts(p, " Machine check polls\n");
#endif
-#if IS_ENABLED(CONFIG_HYPERV) || defined(CONFIG_XEN)
+#if IS_ENABLED(CONFIG_HYPERV) || defined(CONFIG_XEN) ||\
+ defined(CONFIG_ACRN_GUEST)
if (test_bit(HYPERVISOR_CALLBACK_VECTOR, system_vectors)) {
seq_printf(p, "%*s: ", prec, "HYP");
for_each_online_cpu(j)
--
2.7.4
Zhao,
On Tue, 26 Mar 2019, Zhao Yakui wrote:
Vs. the Subject line: arch/x86: add the support of ACRN guest
The proper prefix for x86 is surprisingly 'x86:' not 'arch/x86:'. Also
please start the first word after the colon with an upper case letter.
> ACRN is one open-source hypervisour, which is maintained by Linux
s/one/an/
> foundation.
by the Linuxfoundation.
> This is to add the para-virtualization support so that
> it allows the Linux guest to run on acrn-hypervisor.
>
> This adds x86_hyper_acrn into supported hypervisors array, which enables
> Linux ACRN guest running on ACRN hypervisor. It is restricted to X86_64.
Please do not use 'This is to add' or 'This adds'. Just say:
Add ....
> v1->v2: Change the CONFIG_ACRN to CONFIG_ACRN_GUEST, which makes it easy to
> understand.
> Remove the export of x86_hyper_acrn.
Thanks for having the version changes documented, but please put them after
the '---' line below and add another '---' before the diffstat. These
changes are not part of the final change log and if they are below then I
don't have to strip them manually.
> Co-developed-by: Jason Chen CJ <[email protected]>
> Signed-off-by: Jason Chen CJ <[email protected]>
> Signed-off-by: Zhao Yakui <[email protected]>
> ---
> arch/x86/Kconfig | 8 ++++++++
> arch/x86/include/asm/hypervisor.h | 1 +
> arch/x86/kernel/cpu/Makefile | 1 +
> arch/x86/kernel/cpu/acrn.c | 35 +++++++++++++++++++++++++++++++++++
> arch/x86/kernel/cpu/hypervisor.c | 4 ++++
> 5 files changed, 49 insertions(+)
> create mode 100644 arch/x86/kernel/cpu/acrn.c
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index c1f9b3c..d73225e 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -842,6 +842,14 @@ config JAILHOUSE_GUEST
> cell. You can leave this option disabled if you only want to start
> Jailhouse and run Linux afterwards in the root cell.
>
> +config ACRN_GUEST
> + bool "ACRN Guest support"
> + depends on X86_64 && PARAVIRT
Why does this select PARAVIRT? The current patches are not implementing
anything of the paravirt functionality. Which part of paravirtualization
are you going to provide?
Thanks,
tglx
On Tue, 26 Mar 2019, Zhao Yakui wrote:
> diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h
> index d9069bb..a8f4d08 100644
> --- a/arch/x86/include/asm/hardirq.h
> +++ b/arch/x86/include/asm/hardirq.h
> @@ -37,7 +37,8 @@ typedef struct {
> #ifdef CONFIG_X86_MCE_AMD
> unsigned int irq_deferred_error_count;
> #endif
> -#if IS_ENABLED(CONFIG_HYPERV) || defined(CONFIG_XEN)
> +#if IS_ENABLED(CONFIG_HYPERV) || defined(CONFIG_XEN) ||\
> + defined(CONFIG_ACRN_GUEST)
..
> @@ -134,7 +134,8 @@ int arch_show_interrupts(struct seq_file *p, int prec)
> seq_printf(p, "%10u ", per_cpu(mce_poll_count, j));
> seq_puts(p, " Machine check polls\n");
> #endif
> -#if IS_ENABLED(CONFIG_HYPERV) || defined(CONFIG_XEN)
> +#if IS_ENABLED(CONFIG_HYPERV) || defined(CONFIG_XEN) ||\
> + defined(CONFIG_ACRN_GUEST)
> if (test_bit(HYPERVISOR_CALLBACK_VECTOR, system_vectors)) {
> seq_printf(p, "%*s: ", prec, "HYP");
> for_each_online_cpu(j)
This starts to become unreadable. Please create a new config symbol:
config X86_HV_CALLBACK_VECTOR
def_bool
and select if from HYPERV, XEN and your new thing. That wants to be a
separate preparatory patch at the beginning of the series please.
Thanks,
tglx
On Tue, 26 Mar 2019, Zhao Yakui wrote:
> When acrn_hypervisor is detected, the hypercall is needed so that the
> acrn guest can query/config some settings. For example: it can be used
> to query the resources in hypervisor and manage the CPU/memory/device/
> interrupt for Guest system.
>
> So the hypercall is added so that the kernel can communicate with the
> low-level acrn-hypervisor. On x86 it is implemented by using vmacll when
is implemented with the VMCALL instruction
> the acrn hypervisor is enabled.
>
> +++ b/arch/x86/include/asm/acrn_hypercall.h
> @@ -0,0 +1,84 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * acrn_hypercall.h : acrn hypervisor call API
No file names in headers please. They are pointless and get out of sync
when files are renamed.
> + */
> +
> +#ifndef __ACRN_HYPERCALL_H__
> +#define __ACRN_HYPERCALL_H__
asm headers start with
__ASM_X86_....
> +
> +#include <linux/errno.h>
> +
> +#ifdef CONFIG_ACRN_GUEST
> +
> +/*
> + * Hypercalls for ACRN Guest
> + *
> + * Hypercall number is passed in r8 register.
> + * Return value will be placed in rax.
> + * Up to 2 arguments are passed in rdi, rsi.
> + */
> +
> +static inline long acrn_hypercall0(unsigned long hcall_id)
> +{
> +
Remove the empty new line please.
> + register long result asm("rax");
> + register unsigned long r8 asm("r8") = hcall_id;
Please order them the other way around, like a reverse christmas tree:
register unsigned long r8 asm("r8") = hcall_id;
register long result asm("rax");
That's way simpler to read.
> + asm volatile(".byte 0x0F,0x01,0xC1\n"
> + : "=r"(result)
> + : "r"(r8));
Please mention in the changelog why this is implemented with bytes and not
with the proper mnemonic. I assume it depends on binutils, so please
document which version of binutils supports the mnemonic.
And in the first function, i.e. hypercall0, add a comment above the asm
volatile() to that effect as well.
Thanks,
tglx
On 2019年04月06日 03:07, Thomas Gleixner wrote:
> On Tue, 26 Mar 2019, Zhao Yakui wrote:
>> diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h
>> index d9069bb..a8f4d08 100644
>> --- a/arch/x86/include/asm/hardirq.h
>> +++ b/arch/x86/include/asm/hardirq.h
>> @@ -37,7 +37,8 @@ typedef struct {
>> #ifdef CONFIG_X86_MCE_AMD
>> unsigned int irq_deferred_error_count;
>> #endif
>> -#if IS_ENABLED(CONFIG_HYPERV) || defined(CONFIG_XEN)
>> +#if IS_ENABLED(CONFIG_HYPERV) || defined(CONFIG_XEN) ||\
>> + defined(CONFIG_ACRN_GUEST)
>
> ..
>
>> @@ -134,7 +134,8 @@ int arch_show_interrupts(struct seq_file *p, int prec)
>> seq_printf(p, "%10u ", per_cpu(mce_poll_count, j));
>> seq_puts(p, " Machine check polls\n");
>> #endif
>> -#if IS_ENABLED(CONFIG_HYPERV) || defined(CONFIG_XEN)
>> +#if IS_ENABLED(CONFIG_HYPERV) || defined(CONFIG_XEN) ||\
>> + defined(CONFIG_ACRN_GUEST)
>> if (test_bit(HYPERVISOR_CALLBACK_VECTOR, system_vectors)) {
>> seq_printf(p, "%*s: ", prec, "HYP");
>> for_each_online_cpu(j)
>
> This starts to become unreadable. Please create a new config symbol:
>
> config X86_HV_CALLBACK_VECTOR
> def_bool
>
> and select if from HYPERV, XEN and your new thing. That wants to be a
> separate preparatory patch at the beginning of the series please.
Thanks for the suggestion.
Sure. One new config will be added in next version.
>
> Thanks,
>
> tglx
>
On 2019年04月06日 03:19, Thomas Gleixner wrote:
> On Tue, 26 Mar 2019, Zhao Yakui wrote:
>
>> When acrn_hypervisor is detected, the hypercall is needed so that the
>> acrn guest can query/config some settings. For example: it can be used
>> to query the resources in hypervisor and manage the CPU/memory/device/
>> interrupt for Guest system.
>>
>> So the hypercall is added so that the kernel can communicate with the
>> low-level acrn-hypervisor. On x86 it is implemented by using vmacll when
>
> is implemented with the VMCALL instruction
>
>> the acrn hypervisor is enabled.
>>
>> +++ b/arch/x86/include/asm/acrn_hypercall.h
>> @@ -0,0 +1,84 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * acrn_hypercall.h : acrn hypervisor call API
>
> No file names in headers please. They are pointless and get out of sync
> when files are renamed.
Sure. It will be removed in next vesion.
>
>> + */
>> +
>> +#ifndef __ACRN_HYPERCALL_H__
>> +#define __ACRN_HYPERCALL_H__
>
> asm headers start with
>
> __ASM_X86_....
>
This will be fixed.
>> +
>> +#include <linux/errno.h>
>> +
>> +#ifdef CONFIG_ACRN_GUEST
>> +
>> +/*
>> + * Hypercalls for ACRN Guest
>> + *
>> + * Hypercall number is passed in r8 register.
>> + * Return value will be placed in rax.
>> + * Up to 2 arguments are passed in rdi, rsi.
>> + */
>> +
>> +static inline long acrn_hypercall0(unsigned long hcall_id)
>> +{
>> +
>
> Remove the empty new line please.
>
>> + register long result asm("rax");
>> + register unsigned long r8 asm("r8") = hcall_id;
>
> Please order them the other way around, like a reverse christmas tree:
>
> register unsigned long r8 asm("r8") = hcall_id;
> register long result asm("rax");
>
> That's way simpler to read.
This will be fixed.
>
>> + asm volatile(".byte 0x0F,0x01,0xC1\n"
>> + : "=r"(result)
>> + : "r"(r8));
>
> Please mention in the changelog why this is implemented with bytes and not
> with the proper mnemonic. I assume it depends on binutils, so please
> document which version of binutils supports the mnemonic.
I check the binutils version mentioned in Document/changes.
(binutils, 2.20)
It seems that the "vmcall" is already supported.
So the "vmcall" mnemonic will be used to make it readable.
>
> And in the first function, i.e. hypercall0, add a comment above the asm
> volatile() to that effect as well.
Sure.
>
> Thanks,
>
> tglx
>
On 2019年04月06日 03:01, Thomas Gleixner wrote:
> Zhao,
>
> On Tue, 26 Mar 2019, Zhao Yakui wrote:
>
> Vs. the Subject line: arch/x86: add the support of ACRN guest
>
> The proper prefix for x86 is surprisingly 'x86:' not 'arch/x86:'. Also
> please start the first word after the colon with an upper case letter.
>
>> ACRN is one open-source hypervisour, which is maintained by Linux
>
> s/one/an/
>
>> foundation.
>
> by the Linuxfoundation.
>
>> This is to add the para-virtualization support so that
>> it allows the Linux guest to run on acrn-hypervisor.
>>
>> This adds x86_hyper_acrn into supported hypervisors array, which enables
>> Linux ACRN guest running on ACRN hypervisor. It is restricted to X86_64.
>
> Please do not use 'This is to add' or 'This adds'. Just say:
>
> Add ....
>
>> v1->v2: Change the CONFIG_ACRN to CONFIG_ACRN_GUEST, which makes it easy to
>> understand.
>> Remove the export of x86_hyper_acrn.
>
> Thanks for having the version changes documented, but please put them after
> the '---' line below and add another '---' before the diffstat. These
> changes are not part of the final change log and if they are below then I
> don't have to strip them manually.
>
Sure.
It will be updated.
>> Co-developed-by: Jason Chen CJ <[email protected]>
>> Signed-off-by: Jason Chen CJ <[email protected]>
>> Signed-off-by: Zhao Yakui <[email protected]>
>> ---
>> arch/x86/Kconfig | 8 ++++++++
>> arch/x86/include/asm/hypervisor.h | 1 +
>> arch/x86/kernel/cpu/Makefile | 1 +
>> arch/x86/kernel/cpu/acrn.c | 35 +++++++++++++++++++++++++++++++++++
>> arch/x86/kernel/cpu/hypervisor.c | 4 ++++
>> 5 files changed, 49 insertions(+)
>> create mode 100644 arch/x86/kernel/cpu/acrn.c
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index c1f9b3c..d73225e 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -842,6 +842,14 @@ config JAILHOUSE_GUEST
>> cell. You can leave this option disabled if you only want to start
>> Jailhouse and run Linux afterwards in the root cell.
>>
>> +config ACRN_GUEST
>> + bool "ACRN Guest support"
>> + depends on X86_64 && PARAVIRT
>
> Why does this select PARAVIRT? The current patches are not implementing
> anything of the paravirt functionality. Which part of paravirtualization
> are you going to provide?
Thanks for your nice and careful review.
Yes. The CONFIG_PARAVIRT is not required.
It will be removed.
>
> Thanks,
>
> tglx
>