Hello Julien,
On Wed, 2020-07-01 at 18:46 +0100, Julien Grall wrote:
Title: s/hypervizor/hypervisor/
On 01/07/2020 17:29, Anastasiia Lukianenko wrote:
From: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com
Port hypervizor related code from mini-os. Update essential
Ditto.
But I would be quite cautious to import code from mini-OS in order to support Arm. The port has always been broken and from a look below needs to be refined for Arm.
arch code to support required bit operations, memory barriers etc.
Copyright for the bits ported belong to at least the following authors, please see related files for details:
Copyright (c) 2002-2003, K A Fraser Copyright (c) 2005, Grzegorz Milos, gm281@cam.ac.uk,Intel Research Cambridge Copyright (c) 2014, Karim Allah Ahmed karim.allah.ahmed@gmail.com
Signed-off-by: Oleksandr Andrushchenko < oleksandr_andrushchenko@epam.com> Signed-off-by: Anastasiia Lukianenko < anastasiia_lukianenko@epam.com>
arch/arm/include/asm/xen/system.h | 96 +++++++++++ common/board_r.c | 11 ++ drivers/Makefile | 1 + drivers/xen/Makefile | 5 + drivers/xen/hypervisor.c | 277 ++++++++++++++++++++++++++++++ include/xen.h | 11 ++ include/xen/hvm.h | 30 ++++ 7 files changed, 431 insertions(+) create mode 100644 arch/arm/include/asm/xen/system.h create mode 100644 drivers/xen/Makefile create mode 100644 drivers/xen/hypervisor.c create mode 100644 include/xen.h create mode 100644 include/xen/hvm.h
diff --git a/arch/arm/include/asm/xen/system.h b/arch/arm/include/asm/xen/system.h new file mode 100644 index 0000000000..81ab90160e --- /dev/null +++ b/arch/arm/include/asm/xen/system.h @@ -0,0 +1,96 @@ +/*
- SPDX-License-Identifier: GPL-2.0
- (C) 2014 Karim Allah Ahmed karim.allah.ahmed@gmail.com
- (C) 2020, EPAM Systems Inc.
- */
+#ifndef _ASM_ARM_XEN_SYSTEM_H +#define _ASM_ARM_XEN_SYSTEM_H
+#include <compiler.h> +#include <asm/bitops.h>
+/* If *ptr == old, then store new there (and return new).
- Otherwise, return the old value.
- Atomic.
- */
+#define synch_cmpxchg(ptr, old, new) \ +({ __typeof__(*ptr) stored = old; \
- __atomic_compare_exchange_n(ptr, &stored, new, 0,
__ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST) ? new : old; \ +})
+/* As test_and_clear_bit, but using __ATOMIC_SEQ_CST */ +static inline int synch_test_and_clear_bit(int nr, volatile void *addr) +{
- u8 *byte = ((u8 *)addr) + (nr >> 3);
- u8 bit = 1 << (nr & 7);
- u8 orig;
- orig = __atomic_fetch_and(byte, ~bit, __ATOMIC_SEQ_CST);
- return (orig & bit) != 0;
+}
+/* As test_and_set_bit, but using __ATOMIC_SEQ_CST */ +static inline int synch_test_and_set_bit(int nr, volatile void *base) +{
- u8 *byte = ((u8 *)base) + (nr >> 3);
- u8 bit = 1 << (nr & 7);
- u8 orig;
- orig = __atomic_fetch_or(byte, bit, __ATOMIC_SEQ_CST);
- return (orig & bit) != 0;
+}
+/* As set_bit, but using __ATOMIC_SEQ_CST */ +static inline void synch_set_bit(int nr, volatile void *addr) +{
- synch_test_and_set_bit(nr, addr);
+}
+/* As clear_bit, but using __ATOMIC_SEQ_CST */ +static inline void synch_clear_bit(int nr, volatile void *addr) +{
- synch_test_and_clear_bit(nr, addr);
+}
+/* As test_bit, but with a following memory barrier. */ +//static inline int synch_test_bit(int nr, volatile void *addr) +static inline int synch_test_bit(int nr, const void *addr) +{
- int result;
- result = test_bit(nr, addr);
- barrier();
- return result;
+}
I can understand why we implement sync_* helpers as AFAICT the generic helpers are not SMP safe. However...
+#define xchg(ptr, v) __atomic_exchange_n(ptr, v, __ATOMIC_SEQ_CST) +#define xchg(ptr, v) __atomic_exchange_n(ptr, v, __ATOMIC_SEQ_CST)
+#define mb() dsb() +#define rmb() dsb() +#define wmb() dsb() +#define __iormb() dmb() +#define __iowmb() dmb()
Why do you need to re-implement the barriers?
+#define xen_mb() mb() +#define xen_rmb() rmb() +#define xen_wmb() wmb()
+#define smp_processor_id() 0
Shouldn't this be common?
+#define to_phys(x) ((unsigned long)(x)) +#define to_virt(x) ((void *)(x))
+#define PFN_UP(x) (unsigned long)(((x) + PAGE_SIZE - 1)
PAGE_SHIFT)
+#define PFN_DOWN(x) (unsigned long)((x) >> PAGE_SHIFT) +#define PFN_PHYS(x) ((unsigned long)(x) << PAGE_SHIFT) +#define PHYS_PFN(x) (unsigned long)((x) >> PAGE_SHIFT)
+#define virt_to_pfn(_virt) (PFN_DOWN(to_phys(_virt))) +#define virt_to_mfn(_virt) (PFN_DOWN(to_phys(_virt))) +#define mfn_to_virt(_mfn) (to_virt(PFN_PHYS(_mfn))) +#define pfn_to_virt(_pfn) (to_virt(PFN_PHYS(_pfn)))
There is already generic phys <-> virt helpers (see include/asm-generic/io.h). So why do you need to create a new version?
AFAIU, we need to use phys_to_virt and virt_to_phys functions from include/asm-generic/io.h instead of to_phys and to_virt defines. For the rest of the definitions, we think they should be left as we work with frames, not addresses.
+#endif diff --git a/common/board_r.c b/common/board_r.c index fa57fa9b69..fd36edb4e5 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -56,6 +56,7 @@ #include <timer.h> #include <trace.h> #include <watchdog.h> +#include <xen.h>
Do we want to include it for other boards?
#ifdef CONFIG_ADDR_MAP #include <asm/mmu.h> #endif @@ -462,6 +463,13 @@ static int initr_mmc(void) } #endif
+#ifdef CONFIG_XEN +static int initr_xen(void) +{
- xen_init();
- return 0;
+} +#endif /*
- Tell if it's OK to load the environment early in boot.
@@ -769,6 +777,9 @@ static init_fnc_t init_sequence_r[] = { #endif #ifdef CONFIG_MMC initr_mmc, +#endif +#ifdef CONFIG_XEN
- initr_xen, #endif initr_env, #ifdef CONFIG_SYS_BOOTPARAMS_LEN
diff --git a/drivers/Makefile b/drivers/Makefile index 94e8c5da17..0dd8891e76 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_$(SPL_)REMOTEPROC) += remoteproc/ obj-$(CONFIG_$(SPL_TPL_)TPM) += tpm/ obj-$(CONFIG_$(SPL_TPL_)ACPI_PMC) += power/acpi_pmc/ obj-$(CONFIG_$(SPL_)BOARD) += board/ +obj-$(CONFIG_XEN) += xen/
ifndef CONFIG_TPL_BUILD ifdef CONFIG_SPL_BUILD diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile new file mode 100644 index 0000000000..1211bf2386 --- /dev/null +++ b/drivers/xen/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# (C) Copyright 2020 EPAM Systems Inc.
+obj-y += hypervisor.o diff --git a/drivers/xen/hypervisor.c b/drivers/xen/hypervisor.c new file mode 100644 index 0000000000..5883285142 --- /dev/null +++ b/drivers/xen/hypervisor.c @@ -0,0 +1,277 @@ +/*****************************************************************
- hypervisor.c
- Communication to/from hypervisor.
- Copyright (c) 2002-2003, K A Fraser
- Copyright (c) 2005, Grzegorz Milos, gm281@cam.ac.uk,Intel
Research Cambridge
- Copyright (c) 2020, EPAM Systems Inc.
- Permission is hereby granted, free of charge, to any person
obtaining a copy
- of this software and associated documentation files (the
"Software"), to
- deal in the Software without restriction, including without
limitation the
- rights to use, copy, modify, merge, publish, distribute,
sublicense, and/or
- sell copies of the Software, and to permit persons to whom the
Software is
- furnished to do so, subject to the following conditions:
- The above copyright notice and this permission notice shall be
included in
- all copies or substantial portions of the Software.
- THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
EXPRESS OR
- IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
MERCHANTABILITY,
- FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
EVENT SHALL THE
- AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
OR OTHER
- LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
ARISING
- FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
OTHER
- DEALINGS IN THE SOFTWARE.
- */
+#include <common.h> +#include <cpu_func.h> +#include <log.h> +#include <memalign.h>
+#include <asm/io.h> +#include <asm/armv8/mmu.h> +#include <asm/xen/system.h>
+#include <linux/bug.h>
+#include <xen/hvm.h> +#include <xen/interface/memory.h>
+#define active_evtchns(cpu, sh, idx) \
- ((sh)->evtchn_pending[idx] & \
~(sh)->evtchn_mask[idx])+int in_callback;
+/*
- Shared page for communicating with the hypervisor.
- Events flags go here, for example.
- */
+struct shared_info *HYPERVISOR_shared_info;
+#ifndef CONFIG_PARAVIRT
Is there any plan to support this on x86?
+static const char *param_name(int op) +{ +#define PARAM(x)[HVM_PARAM_##x] = #x
- static const char *const names[] = {
PARAM(CALLBACK_IRQ),PARAM(STORE_PFN),PARAM(STORE_EVTCHN),PARAM(PAE_ENABLED),PARAM(IOREQ_PFN),PARAM(TIMER_MODE),PARAM(HPET_ENABLED),PARAM(IDENT_PT),PARAM(ACPI_S_STATE),PARAM(VM86_TSS),PARAM(VPT_ALIGN),PARAM(CONSOLE_PFN),PARAM(CONSOLE_EVTCHN),Most of those parameters are never going to be used on Arm. So could this be clobberred?
- };
+#undef PARAM
- if (op >= ARRAY_SIZE(names))
return "unknown";- if (!names[op])
return "reserved";- return names[op];
+}
+int hvm_get_parameter_maintain_dcache(int idx, uint64_t *value)
I would recommend to add some comments explaining when this function is meant to be used and what it is doing in regards of the cache.
+{
- struct xen_hvm_param xhv;
- int ret;
I don't think there is a guarantee that your cache is going to be clean when writing xhv. So you likely want to add a invalidate_dcache_range() before writing it.
- xhv.domid = DOMID_SELF;
- xhv.index = idx;
- invalidate_dcache_range((unsigned long)&xhv,
(unsigned long)&xhv + sizeof(xhv));- ret = HYPERVISOR_hvm_op(HVMOP_get_param, &xhv);
- if (ret < 0) {
pr_err("Cannot get hvm parameter %s (%d): %d!\n",param_name(idx), idx, ret);BUG();- }
- invalidate_dcache_range((unsigned long)&xhv,
(unsigned long)&xhv + sizeof(xhv));- *value = xhv.value;
- return ret;
+}
+int hvm_get_parameter(int idx, uint64_t *value) +{
- struct xen_hvm_param xhv;
- int ret;
- xhv.domid = DOMID_SELF;
- xhv.index = idx;
- ret = HYPERVISOR_hvm_op(HVMOP_get_param, &xhv);
- if (ret < 0) {
pr_err("Cannot get hvm parameter %s (%d): %d!\n",param_name(idx), idx, ret);BUG();- }
- *value = xhv.value;
- return ret;
+}
+int hvm_set_parameter(int idx, uint64_t value) +{
- struct xen_hvm_param xhv;
- int ret;
- xhv.domid = DOMID_SELF;
- xhv.index = idx;
- xhv.value = value;
- ret = HYPERVISOR_hvm_op(HVMOP_set_param, &xhv);
- if (ret < 0) {
pr_err("Cannot get hvm parameter %s (%d): %d!\n",param_name(idx), idx, ret);BUG();- }
- return ret;
+}
+struct shared_info *map_shared_info(void *p) +{
- struct xen_add_to_physmap xatp;
- HYPERVISOR_shared_info = (struct shared_info
*)memalign(PAGE_SIZE,
PAGE_SIZE);
- if (HYPERVISOR_shared_info == NULL)
BUG();- xatp.domid = DOMID_SELF;
- xatp.idx = 0;
- xatp.space = XENMAPSPACE_shared_info;
- xatp.gpfn = virt_to_pfn(HYPERVISOR_shared_info);
- if (HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp) != 0)
BUG();- return HYPERVISOR_shared_info;
+}
+void unmap_shared_info(void) +{
- struct xen_remove_from_physmap xrtp;
- xrtp.domid = DOMID_SELF;
- xrtp.gpfn = virt_to_pfn(HYPERVISOR_shared_info);
- if (HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrtp) !=
BUG();+} +#endif
+void do_hypervisor_callback(struct pt_regs *regs) +{
- unsigned long l1, l2, l1i, l2i;
- unsigned int port;
- int cpu = 0;
- struct shared_info *s = HYPERVISOR_shared_info;
- struct vcpu_info *vcpu_info = &s->vcpu_info[cpu];
- in_callback = 1;
- vcpu_info->evtchn_upcall_pending = 0;
- /* NB x86. No need for a barrier here -- XCHG is a barrier on
x86. */ +#if !defined(__i386__) && !defined(__x86_64__)
- /* Clear master flag /before/ clearing selector flag. */
- wmb();
+#endif
- l1 = xchg(&vcpu_info->evtchn_pending_sel, 0);
- while (l1 != 0) {
l1i = __ffs(l1);l1 &= ~(1UL << l1i);while ((l2 = active_evtchns(cpu, s, l1i)) != 0) {l2i = __ffs(l2);l2 &= ~(1UL << l2i);port = (l1i * (sizeof(unsigned long) * 8)) +l2i;
/* TODO: handle new event: do_event(port,regs); */
/* Suppress -Wunused-but-set-variable */(void)(port);}- }
You likely want a memory barrier here as otherwise in_callback could be written/seen before the loop end.
- in_callback = 0;
+}
+void force_evtchn_callback(void) +{ +#ifdef XEN_HAVE_PV_UPCALL_MASK
- int save;
+#endif
- struct vcpu_info *vcpu;
- vcpu = &HYPERVISOR_shared_info->vcpu_info[smp_processor_id()];
On Arm, this is only valid for vCPU0. For all the other vCPUs, you will want to register a vCPU shared info.
+#ifdef XEN_HAVE_PV_UPCALL_MASK
- save = vcpu->evtchn_upcall_mask;
+#endif
- while (vcpu->evtchn_upcall_pending) {
+#ifdef XEN_HAVE_PV_UPCALL_MASK
vcpu->evtchn_upcall_mask = 1;+#endif
barrier();What are you trying to prevent with this barrier? In particular why would the compiler be an issue but not the processor?
do_hypervisor_callback(NULL);barrier();+#ifdef XEN_HAVE_PV_UPCALL_MASK
vcpu->evtchn_upcall_mask = save;barrier();Same here.
+#endif
- };
+}
+void mask_evtchn(uint32_t port) +{
- struct shared_info *s = HYPERVISOR_shared_info;
- synch_set_bit(port, &s->evtchn_mask[0]);
+}
+void unmask_evtchn(uint32_t port) +{
- struct shared_info *s = HYPERVISOR_shared_info;
- struct vcpu_info *vcpu_info = &s-
vcpu_info[smp_processor_id()];
- synch_clear_bit(port, &s->evtchn_mask[0]);
- /*
* The following is basically the equivalent of'hw_resend_irq'. Just like
* a real IO-APIC we 'lose the interrupt edge' if the channelis masked.
*/This seems to be out-of-context now, you might want to update it.
- if (synch_test_bit(port, &s->evtchn_pending[0]) &&
!synch_test_and_set_bit(port / (sizeof(unsigned long) * 8),&vcpu_info->evtchn_pending_sel)) {vcpu_info->evtchn_upcall_pending = 1;+#ifdef XEN_HAVE_PV_UPCALL_MASK
if (!vcpu_info->evtchn_upcall_mask)+#endif
force_evtchn_callback();- }
+}
+void clear_evtchn(uint32_t port) +{
- struct shared_info *s = HYPERVISOR_shared_info;
- synch_clear_bit(port, &s->evtchn_pending[0]);
+}
+void xen_init(void) +{
- debug("%s\n", __func__);
Is this a left-over?
- map_shared_info(NULL);
+}
diff --git a/include/xen.h b/include/xen.h new file mode 100644 index 0000000000..1d6f74cc92 --- /dev/null +++ b/include/xen.h @@ -0,0 +1,11 @@ +/*
- SPDX-License-Identifier: GPL-2.0
- (C) 2020, EPAM Systems Inc.
- */
+#ifndef __XEN_H__ +#define __XEN_H__
+void xen_init(void);
+#endif /* __XEN_H__ */ diff --git a/include/xen/hvm.h b/include/xen/hvm.h new file mode 100644 index 0000000000..89de9625ca --- /dev/null +++ b/include/xen/hvm.h @@ -0,0 +1,30 @@ +/*
- SPDX-License-Identifier: GPL-2.0
- Simple wrappers around HVM functions
- Copyright (c) 2002-2003, K A Fraser
- Copyright (c) 2005, Grzegorz Milos, gm281@cam.ac.uk,Intel
Research Cambridge
- Copyright (c) 2020, EPAM Systems Inc.
- */
+#ifndef XEN_HVM_H__ +#define XEN_HVM_H__
+#include <asm/xen/hypercall.h> +#include <xen/interface/hvm/params.h> +#include <xen/interface/xen.h>
+extern struct shared_info *HYPERVISOR_shared_info;
+int hvm_get_parameter(int idx, uint64_t *value); +int hvm_get_parameter_maintain_dcache(int idx, uint64_t *value); +int hvm_set_parameter(int idx, uint64_t value);
+struct shared_info *map_shared_info(void *p); +void unmap_shared_info(void); +void do_hypervisor_callback(struct pt_regs *regs); +void mask_evtchn(uint32_t port); +void unmask_evtchn(uint32_t port); +void clear_evtchn(uint32_t port);
+#endif /* XEN_HVM_H__ */
Cheers,
Regards, Anastasiia