On 2023-11-21 16:36, Jan Beulich wrote:
On 25.10.2023 15:22, Nicola Vetrini wrote:
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -1034,10 +1034,10 @@ int guest_wrmsr_x2apic(struct vcpu *v,
uint32_t msr, uint64_t val)
case APIC_EOI:
case APIC_ESR:
if ( val )
- {
- default:
return X86EMUL_EXCEPTION;
- }
+ break;
+ default:
+ return X86EMUL_EXCEPTION;
}
vlapic_reg_write(v, array_index_nospec(offset, PAGE_SIZE), val);
Considering the plan to confine applicability of the rule, one style
aspect
which would need to be taken into account is that the entire rest of
this
switch() has blank lines between case blocks.
The other is that imo the overall result would be closer to what we
have
right now if the new code was
case APIC_EOI:
case APIC_ESR:
if ( !val )
break;
fallthrough;
default:
return X86EMUL_EXCEPTION;
}
at which point the need for the blank line would also disappear.
This is also a fine solution. I'll keep this in mind when this patch
will be revisited.
As to the description - isn't this change (whichever way done) also
addressing another violation, requiring "break" (or alike according to
our interpretation) at the end of each case block?
Correct. It's probably a good idea to mention that, but the
"fallthrough" is also a candidate for a deviation from R16.3, so we'll
see about that.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
