|
On 06/09/16 18:44, George Dunlap wrote:
On Tue, Sep 6, 2016 at 11:12 AM, Andrew Cooper
<andrew.cooper3@xxxxxxxxxx> wrote:
In a similar style to hvm_do_hypercall(). The C version is far easier to
understand and edit than the assembly versions.
There are a few small differences however. The register clobbering values
have changed (to match the HVM side), and in particular clobber the upper
32bits of 64bit arguments. The hypercall and performance counter record are
reordered to increase code sharing between the 32bit and 64bit cases.
The sole callers of __trace_hypercall_entry() were the assembly code. Given
the new C layout, it is more convenient to fold __trace_hypercall_entry() into
pv_hypercall(), and call __trace_hypercall() directly.
Finally, pv_hypercall() will treat a NULL hypercall function pointer as
-ENOSYS, allowing further cleanup.
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
v2:
* Use guest_mode_kernel() rather than TF_kernel_mode
* Consistent use of 32bit stores
* Don't truncate rax for 64bit domains
* Move eax return assignment into C
---
xen/arch/x86/hypercall.c | 120 +++++++++++++++++++++++++++++++++++++
xen/arch/x86/trace.c | 27 ---------
xen/arch/x86/x86_64/asm-offsets.c | 1 -
xen/arch/x86/x86_64/compat/entry.S | 69 +--------------------
xen/arch/x86/x86_64/entry.S | 61 +------------------
5 files changed, 124 insertions(+), 154 deletions(-)
diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/hypercall.c
index 4b42f86..13a89a0 100644
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -17,7 +17,12 @@
* Copyright (c) 2015,2016 Citrix Systems Ltd.
*/
+#include <xen/compiler.h>
#include <xen/hypercall.h>
+#include <xen/trace.h>
+
+extern hypercall_fn_t *const hypercall_table[NR_hypercalls],
+ *const compat_hypercall_table[NR_hypercalls];
#define ARGS(x, n) \
[ __HYPERVISOR_ ## x ] = (n)
@@ -111,6 +116,121 @@ const uint8_t compat_hypercall_args_table[NR_hypercalls] =
#undef ARGS
+void pv_hypercall(struct cpu_user_regs *regs)
+{
+ struct vcpu *curr = current;
+#ifndef NDEBUG
+ unsigned long old_rip = regs->rip;
+#endif
+ unsigned long eax;
+
+ ASSERT(guest_kernel_mode(curr, regs));
+
+ eax = is_pv_32bit_vcpu(curr) ? regs->_eax : regs->eax;
+
+ if ( (eax >= NR_hypercalls) || !hypercall_table[eax] )
+ {
+ regs->eax = -ENOSYS;
+ return;
+ }
+
+ if ( !is_pv_32bit_vcpu(curr) )
+ {
+ unsigned long rdi = regs->rdi;
+ unsigned long rsi = regs->rsi;
+ unsigned long rdx = regs->rdx;
+ unsigned long r10 = regs->r10;
+ unsigned long r8 = regs->r8;
+ unsigned long r9 = regs->r9;
+
+#ifndef NDEBUG
+ /* Deliberately corrupt parameter regs not used by this hypercall. */
+ switch ( hypercall_args_table[eax] )
+ {
+ case 0: rdi = 0xdeadbeefdeadf00dUL;
+ case 1: rsi = 0xdeadbeefdeadf00dUL;
+ case 2: rdx = 0xdeadbeefdeadf00dUL;
+ case 3: r10 = 0xdeadbeefdeadf00dUL;
+ case 4: r8 = 0xdeadbeefdeadf00dUL;
+ case 5: r9 = 0xdeadbeefdeadf00dUL;
+ }
Out of curiosity, is Coverity going to complain about the lack of /*
FALLTHROUGH */ in these stanzas, or is there some magic that lets it
know this is intended? (Or does it ignore DEBUG code?)
From the docs:
Because there are many reasons why a case intentionally does not
end with a break statement, the MISSING_BREAK checker does not
report defects in cases that:
-
Are followed by a case that starts with a break.
-
End with a comment. The checker assumes that this comment
is acknowledging a fallthrough. The comment can start
anywhere on the last line, or be a multi-line C comment.
-
Are empty.
-
Have no control flow path because, for example, there is a
return statement.
-
Have at least one conditional statement that contains a
break statement.
-
Start and end on the same line.
-
Have a top-level statement that is a call to a function
that can end the program.
-
Fall through to another case that has a similar numeric
value when interpreted as ASCII. Values are considered
similar when both are whitespace values (such as space, tab,
or newline), or the two values are different cases
(uppercase or lowercase) of the same letter.
I suspect we are hitting the "start and end on the same line"
exclusion. It is worth noting that the HVM code is already like
this, and passing fine.
~Andrew
|
