x86/pv: Address Coverity complaint in check_guest_io_breakpoint()

Commit 08aacc392d ("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: 08aacc392d ("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>
This commit is contained in:
Andrew Cooper committed 2024-08-21 23:59:19 +01:00
1 parent db39fa4b27
commit 6d41a9d8a1
2 files changed
+31 -15

No files matched your search

+25
View File
@@ -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 */
+6 -15
View File
@@ -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;
Reference in new issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.