1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
|
From cb6c3cfc5f8aa8bd8aae1abffea0574b02a04840 Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Tue, 24 Sep 2024 14:36:25 +0200
Subject: [PATCH 21/35] x86/pv: Address Coverity complaint in
check_guest_io_breakpoint()
Commit 08aacc392d86 ("x86/emul: Fix misaligned IO breakpoint behaviour in PV
guests") caused a Coverity INTEGER_OVERFLOW complaint based on the reasoning
that width could be 0.
It can't, but digging into the code generation, GCC 8 and later (bisected on
godbolt) choose to emit a CSWITCH lookup table, and because the range (bottom
2 bits clear), it's a 16-entry lookup table.
So Coverity is understandable, given that GCC did emit a (dead) logic path
where width stayed 0.
Rewrite the logic. Introduce x86_bp_width() which compiles to a single basic
block, which replaces the switch() statement. Take the opportunity to also
make start and width be loop-scope variables.
No practical change, but it should compile better and placate Coverity.
Fixes: 08aacc392d86 ("x86/emul: Fix misaligned IO breakpoint behaviour in PV guests")
Coverity-ID: 1616152
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 6d41a9d8a12ff89adabdc286e63e9391a0481699
master date: 2024-08-21 23:59:19 +0100
---
xen/arch/x86/include/asm/debugreg.h | 25 +++++++++++++++++++++++++
xen/arch/x86/pv/emul-priv-op.c | 21 ++++++---------------
2 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/xen/arch/x86/include/asm/debugreg.h b/xen/arch/x86/include/asm/debugreg.h
index 6baa725441..23aa592e40 100644
--- a/xen/arch/x86/include/asm/debugreg.h
+++ b/xen/arch/x86/include/asm/debugreg.h
@@ -115,4 +115,29 @@ unsigned int x86_adj_dr7_rsvd(const struct cpu_policy *p, unsigned int dr7);
unsigned int x86_merge_dr6(const struct cpu_policy *p, unsigned int dr6,
unsigned int new);
+/*
+ * Calculate the width of a breakpoint from its dr7 encoding.
+ *
+ * The LEN encoding in dr7 is 2 bits wide per breakpoint and encoded as a X-1
+ * (0, 1 and 3) for widths of 1, 2 and 4 respectively in the 32bit days.
+ *
+ * In 64bit, the unused value (2) was given a meaning of width 8, which is
+ * great for efficiency but less great for nicely calculating the width.
+ */
+static inline unsigned int x86_bp_width(unsigned int dr7, unsigned int bp)
+{
+ unsigned int raw = (dr7 >> (DR_CONTROL_SHIFT +
+ DR_CONTROL_SIZE * bp + 2)) & 3;
+
+ /*
+ * If the top bit is set (i.e. we've got an 4 or 8 byte wide breakpoint),
+ * flip the bottom to reverse their order, making them sorted properly.
+ * Then it's a simple shift to calculate the width.
+ */
+ if ( raw & 2 )
+ raw ^= 1;
+
+ return 1U << raw;
+}
+
#endif /* _X86_DEBUGREG_H */
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 15c83b9d23..b90f745c75 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -323,30 +323,21 @@ static unsigned int check_guest_io_breakpoint(struct vcpu *v,
unsigned int port,
unsigned int len)
{
- unsigned int width, i, match = 0;
- unsigned long start;
+ unsigned int i, match = 0;
if ( !v->arch.pv.dr7_emul || !(v->arch.pv.ctrlreg[4] & X86_CR4_DE) )
return 0;
for ( i = 0; i < 4; i++ )
{
+ unsigned long start;
+ unsigned int width;
+
if ( !(v->arch.pv.dr7_emul & (3 << (i * DR_ENABLE_SIZE))) )
continue;
- start = v->arch.dr[i];
- width = 0;
-
- switch ( (v->arch.dr7 >>
- (DR_CONTROL_SHIFT + i * DR_CONTROL_SIZE)) & 0xc )
- {
- case DR_LEN_1: width = 1; break;
- case DR_LEN_2: width = 2; break;
- case DR_LEN_4: width = 4; break;
- case DR_LEN_8: width = 8; break;
- }
-
- start &= ~(width - 1UL);
+ width = x86_bp_width(v->arch.dr7, i);
+ start = v->arch.dr[i] & ~(width - 1UL);
if ( (start < (port + len)) && ((start + width) > port) )
match |= 1u << i;
--
2.46.1
|
GitWeb