On Thu, Mar 31, 2022 at 09:44:06AM +0100, Julien Grall wrote:
>
> On 31/03/2022 02:15, Elliott Mitchell wrote:
> > On Tue, Mar 29, 2022 at 08:27:24AM +0200, Jan Beulich wrote:
> >> On 29.03.2022 00:25, Stefano Stabellini wrote:
> >>> On Sat, 26 Mar 2022, Elliott Mitchell wrote:
> >>>> The hypercalls implementation for Linux and FreeBSD have two key headers,
> >>>> hypercall.h and hypervisor.h. I'm curious why the implementations for
> >>>> x86 and ARM* are so distinct.
> >>>>
> >>>> I found it fairly straightforward to implement ARM* versions of the x86
> >>>> _hypercall#() macros. Once that is done, most of the wrappers in the x86
> >>>> hypercall.h can be moved to a shared hypervisor.h header.
> >>>>
> >>>> Why does Xen/ARM on Linux still have hypercall.S when merging the
> >>>> headers should reduce maintainance?
> >>>>
> >>>> Was GCC extended inline assembly language for ARM* thought too awful?
> >>>>
> >>>> I'm also curious why these headers are part of the Linux kernel, instead
> >>>> of being maintained by the Xen Project?
> >>>
> >>> I would have to dig through ancient archives to give you a full answer
> >>> but the reason was that the asm inline on ARM didn't provide enough
> >>> guarantees on ordering and registers it would use and clobber.
> >>
> >> While there may be ordering issues (albeit most ought to be taken care
> >> of by marking the asm() volatile and having it have a memory clobber),
> >> registers used / clobbered ought to always be expressable by asm() -
> >> if not by constraints covering just a single register (such frequently
> >> simply don't exist), then by using register variables tied to a
> >> particular register. Of course in all of this there's an assumption of
> >> no bugs in this area in the compilers claimed as being supported ...
> >
> > I'm merely been working with recent versions of Clang on FreeBSD, but
> > I've got something which appears to work.
> >
> > I would be somewhat hopeful GCC might have fewer bugs on ARM as
> > registers aren't so precious. Yet that could well be a minefield.
>
> Linux and Xen have already some code heavily using inline assembly for
> the SMCCC v1.1 helpers. So, in theory, it should be possible to convert
> the hypercall to use inline assembly helpers.
>
> Unlike SMCCC v1.1, the hypercalls are following the AAPCS. So by using
> the assembly wrapper we don't have to worry on what to save as the
> compiler will automatically know what to do. Looking at
> xen/include/public/arch-arm.h, we may be able to reduce the numbers of
> registers to preserve. So it would be more interesting to switch to
> inline assembly to reduce the number of instructions.
>
> That said, we also need to be mindful of straigh-line speculation with
> HVC instruction. With the inline version, the speculation barrier (sb or
> dsb/isb) would need to be architecturally executed. With the assembly
> wrapper, I believe we could only speculatively execute it by adding
> after the ret.
>
> Note that Linux doesn't have the speculation barrier yet. In in two mind
> whether it should be added. In one hand, it would be good to make the
> hypercalls safe by default (IOW the caller doesn't need to check for
> gadget after). On the other hand, AFAIK the path would only be reachable
> with root privileges.
>
> That said, the security posture may be different on other OS. So if we
> intend to share the header with other OS, then the current approach
> might be better.
Guess I should simply send out what I've got right now.
The idea is all the HYPERVISOR_*() wrappers which both Linux and FreeBSD
have in the architecture hypercall.h could be moved to hypervisor.h.
This generates _hypercall#() wrapper macros which match the x86
_hypercall#() inline functions. The actual underlying inline functions
are __hypercall_xen_#().
The underlying inline functions place the arguments first and op code
last. The reason is ARM places the arguments in r0-r4/x0-x4 and the
inline function could end up emitting a bunch of instructions shuffling
these. With the op code last the compiler may emit a move instruction
for the op code, but the arguments will pass straight through.
The attached patch is marked as FreeBSD's license, because I've been
trying to get FreeBSD running on Xen/ARM. I'm fine with it under Xen's
license or GPL 2.0+. This has been tested and appears to work, but I
won't be surprised if it doesn't match what people want.
--
(\___(\___(\______ --=> 8-) EHM <=-- ______/)___/)___/)
\BS ( | ehem+sig...@m5p.com PGP 87145445 | ) /
\_CS\ | _____ -O #include <stddisclaimer.h> O- _____ | / _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
>From 8ad5602f26a0f7b9a04cbda33330ba8004d8fcc1 Mon Sep 17 00:00:00 2001
From: Elliott Mitchell <ehem+...@m5p.com>
Date: Mon, 7 Mar 2022 18:26:31 -0800
Subject: [PATCH] xen/arm64: implement aarch64 Xen hypercall header
To: xen-devel@lists.xenproject.org
Match the i386/AMD64 headers in doing inline functions for the Xen
hypervisor calls. This has been heavily inspired by work done by
Julien Grall and Stefano Stabellini.
---
sys/arm64/include/xen/hypercall.h | 148 ++++++++++++++++++++++++++++++
1 file changed, 148 insertions(+)
create mode 100644 sys/arm64/include/xen/hypercall.h
diff --git a/sys/arm64/include/xen/hypercall.h b/sys/arm64/include/xen/hypercall.h
new file mode 100644
index 00000000000..b8e51611f26
--- /dev/null
+++ b/arm64/include/xen/hypercall.h
@@ -0,0 +1,148 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0+ OR BSD-2-Clause-FreeBSD
+ *
+ * Copyright (c) 2022 Elliott Mitchell
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#ifndef __MACHINE_ARM64_XEN_HYPERCALL_H__
+#define __MACHINE_ARM64_XEN_HYPERCALL_H__
+__FBSDID("$FreeBSD$");
+
+#ifndef __XEN_HYPERVISOR_H__
+# error "please don't include this file directly"
+#endif
+
+/*
+ * See the Xen contrib header: xen/arch-arm.h for details on Xen's
+ * hypercall calling conventions.
+ *
+ * The hypercall number is passed in r12/x16.
+ *
+ * Input parameters are in r0-r4/x0-x4 as appropriate to the number of
+ * arguments. Input registers are clobbered.
+ *
+ * Return is in r0/x0.
+ *
+ * The hypercall tag for Xen is 0x0EA1.
+ */
+
+#define hypercallf(NUM, ARGS, REGVAR, REGASM) \
+ static inline long \
+ __hypercall_xen_##NUM(ARGS long op) \
+ { \
+ register long _op __asm__(OPREG) = op; \
+ REGVAR \
+ __asm__ volatile ( \
+ "hvc #0x0EA1;\n" \
+ : "+r" (_op)REGASM \
+ : /* clobbered inputs, are outputs, really */ \
+ : "memory" \
+ ); \
+ return (_arg0); \
+ }
+
+#ifndef __ILP32__
+#define OPREG "x16"
+#define REGPRE "x"
+#else
+#define OPREG "r12"
+#define REGPRE "r"
+#endif
+
+#define COMMAS(...) __VA_ARGS__
+#define ARG(n) long arg##n,
+#define VAR(n) register long _arg##n __asm__(REGPRE __STRING(n)) = arg##n;
+#define REG(n) , "+r" (_arg##n)
+
+
+#define hypercall0(NUM, ARGS, REGVAR, REGASM) \
+ hypercallf(NUM,, register long _arg0 __asm__(REGPRE"0");, \
+ COMMAS(, "=r" (_arg0)))
+
+#define hypercall1(NUM, ARGS, REGVAR, REGASM) \
+ hypercallf(NUM, COMMAS(ARG(0)ARGS), VAR(0)REGVAR, COMMAS(REG(0)REGASM))
+
+#define hypercall2(NUM, ARGS, REGVAR, REGASM) \
+ hypercall1(NUM, COMMAS(ARG(1)ARGS), VAR(1)REGVAR, COMMAS(REG(1)REGASM))
+
+#define hypercall3(NUM, ARGS, REGVAR, REGASM) \
+ hypercall2(NUM, COMMAS(ARG(2)ARGS), VAR(2)REGVAR, COMMAS(REG(2)REGASM))
+
+#define hypercall4(NUM, ARGS, REGVAR, REGASM) \
+ hypercall3(NUM, COMMAS(ARG(3)ARGS), VAR(3)REGVAR, COMMAS(REG(3)REGASM))
+
+#define hypercall5(NUM, ARGS, REGVAR, REGASM) \
+ hypercall4(NUM, COMMAS(ARG(4)ARGS), VAR(4)REGVAR, COMMAS(REG(4)REGASM))
+
+#define hypercall(NUM) hypercall##NUM(NUM,,,)
+
+/* the actual inline function definitions */
+
+hypercall(0)
+hypercall(1)
+hypercall(2)
+hypercall(3)
+hypercall(4)
+hypercall(5)
+
+/* cleanup */
+
+#undef hypercallf
+#undef OPREG
+#undef REGPRE
+#undef COMMAS
+#undef ARG
+#undef VAR
+#undef REG
+
+#undef hypercall0
+#undef hypercall1
+#undef hypercall2
+#undef hypercall3
+#undef hypercall4
+#undef hypercall
+
+/* the wrappers expected by hypervisor.h */
+
+#define _hypercall0(type, name) \
+ (type)__hypercall_xen_0(__HYPERVISOR_##name)
+#define _hypercall1(type, name, arg0) \
+ (type)__hypercall_xen_1((long)(arg0), __HYPERVISOR_##name)
+#define _hypercall2(type, name, arg0, arg1) \
+ (type)__hypercall_xen_2((long)(arg0), (long)(arg1), \
+ __HYPERVISOR_##name)
+#define _hypercall3(type, name, arg0, arg1, arg2) \
+ (type)__hypercall_xen_3((long)(arg0), (long)(arg1), \
+ (long)(arg2), __HYPERVISOR_##name)
+#define _hypercall4(type, name, arg0, arg1, arg2, arg3) \
+ (type)__hypercall_xen_4((long)(arg0), (long)(arg1), \
+ (long)(arg2), (long)(arg3), __HYPERVISOR_##name)
+#define _hypercall5(type, name, arg0, arg1, arg2, arg3, arg4) \
+ (type)__hypercall_xen_5((long)(arg0), (long)(arg1), \
+ (long)(arg2), (long)(arg3), (long)(arg4), __HYPERVISOR_##name)
+
+#define privcmd_hypercall(op, arg0, arg1, arg2, arg3, arg4) \
+ (int)__hypercall_xen_5((arg0), (arg1), (arg2), (arg3), (arg4), (op))
+
+#endif /* __MACHINE_ARM64_XEN_HYPERCALL_H__ */
--
2.30.2
