* [PATCH] x86: mark hypercall argument regs clobbering for intended fall-through
@ 2021-06-09 10:34 Jan Beulich
2021-06-09 12:49 ` Andrew Cooper
0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2021-06-09 10:34 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Roger Pau Monné
The CIDs below are all for the PV side of things, but also take care of
the HVM side.
Coverity-ID: 1485896, 1485901, 1485906, 1485910, 1485911,
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Let's see whether Coverity actually understands the (relatively) new
pseudo-keyword.
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -246,11 +246,11 @@ int hvm_hypercall(struct cpu_user_regs *
/* Deliberately corrupt parameter regs not used by this hypercall. */
switch ( hypercall_args_table[eax].native )
{
- case 0: rdi = 0xdeadbeefdeadf00dUL;
- case 1: rsi = 0xdeadbeefdeadf00dUL;
- case 2: rdx = 0xdeadbeefdeadf00dUL;
- case 3: r10 = 0xdeadbeefdeadf00dUL;
- case 4: r8 = 0xdeadbeefdeadf00dUL;
+ case 0: rdi = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 1: rsi = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 2: rdx = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 3: r10 = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 4: r8 = 0xdeadbeefdeadf00dUL; fallthrough;
case 5: r9 = 0xdeadbeefdeadf00dUL;
}
#endif
@@ -264,11 +264,11 @@ int hvm_hypercall(struct cpu_user_regs *
/* Deliberately corrupt parameter regs used by this hypercall. */
switch ( hypercall_args_table[eax].native )
{
- case 6: regs->r9 = 0xdeadbeefdeadf00dUL;
- case 5: regs->r8 = 0xdeadbeefdeadf00dUL;
- case 4: regs->r10 = 0xdeadbeefdeadf00dUL;
- case 3: regs->rdx = 0xdeadbeefdeadf00dUL;
- case 2: regs->rsi = 0xdeadbeefdeadf00dUL;
+ case 6: regs->r9 = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 5: regs->r8 = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 4: regs->r10 = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 3: regs->rdx = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 2: regs->rsi = 0xdeadbeefdeadf00dUL; fallthrough;
case 1: regs->rdi = 0xdeadbeefdeadf00dUL;
}
}
--- a/xen/arch/x86/pv/hypercall.c
+++ b/xen/arch/x86/pv/hypercall.c
@@ -149,11 +149,11 @@ void pv_hypercall(struct cpu_user_regs *
/* Deliberately corrupt parameter regs not used by this hypercall. */
switch ( hypercall_args_table[eax].native )
{
- case 0: rdi = 0xdeadbeefdeadf00dUL;
- case 1: rsi = 0xdeadbeefdeadf00dUL;
- case 2: rdx = 0xdeadbeefdeadf00dUL;
- case 3: r10 = 0xdeadbeefdeadf00dUL;
- case 4: r8 = 0xdeadbeefdeadf00dUL;
+ case 0: rdi = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 1: rsi = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 2: rdx = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 3: r10 = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 4: r8 = 0xdeadbeefdeadf00dUL; fallthrough;
case 5: r9 = 0xdeadbeefdeadf00dUL;
}
#endif
@@ -172,11 +172,11 @@ void pv_hypercall(struct cpu_user_regs *
/* Deliberately corrupt parameter regs used by this hypercall. */
switch ( hypercall_args_table[eax].native )
{
- case 6: regs->r9 = 0xdeadbeefdeadf00dUL;
- case 5: regs->r8 = 0xdeadbeefdeadf00dUL;
- case 4: regs->r10 = 0xdeadbeefdeadf00dUL;
- case 3: regs->rdx = 0xdeadbeefdeadf00dUL;
- case 2: regs->rsi = 0xdeadbeefdeadf00dUL;
+ case 6: regs->r9 = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 5: regs->r8 = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 4: regs->r10 = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 3: regs->rdx = 0xdeadbeefdeadf00dUL; fallthrough;
+ case 2: regs->rsi = 0xdeadbeefdeadf00dUL; fallthrough;
case 1: regs->rdi = 0xdeadbeefdeadf00dUL;
}
}
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] x86: mark hypercall argument regs clobbering for intended fall-through
2021-06-09 10:34 [PATCH] x86: mark hypercall argument regs clobbering for intended fall-through Jan Beulich
@ 2021-06-09 12:49 ` Andrew Cooper
2021-06-09 12:55 ` Jan Beulich
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Andrew Cooper @ 2021-06-09 12:49 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Wei Liu, Roger Pau Monné
On 09/06/2021 11:34, Jan Beulich wrote:
> The CIDs below are all for the PV side of things, but also take care of
> the HVM side.
>
> Coverity-ID: 1485896, 1485901, 1485906, 1485910, 1485911,
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Let's see whether Coverity actually understands the (relatively) new
> pseudo-keyword.
This is exceedingly disappointing. Coverity used to have the only
sensible rule for not causing spurious fallthrough warnings, but this
has apparently regressed.
Coverity works on the AST, so ought to be after GCC has interpreted
__attribute__((__fallthrough__)) if applicable.
However, I doubt it will work in the fallback case, because #define
fallthrough looks dubious. To trigger the older logic, the /*
fallthrough */ comment needs to be the final thing before the next case
label, and it isn't with the added semicolon.
Given that this pseudo-keyword is restricted to the SMMU driver for now,
we don't actually know if Coverity likes it or not.
~Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] x86: mark hypercall argument regs clobbering for intended fall-through
2021-06-09 12:49 ` Andrew Cooper
@ 2021-06-09 12:55 ` Jan Beulich
2021-06-30 6:47 ` Ping: " Jan Beulich
2021-06-30 6:50 ` Jan Beulich
2 siblings, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2021-06-09 12:55 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Roger Pau Monné, xen-devel
On 09.06.2021 14:49, Andrew Cooper wrote:
> On 09/06/2021 11:34, Jan Beulich wrote:
>> The CIDs below are all for the PV side of things, but also take care of
>> the HVM side.
>>
>> Coverity-ID: 1485896, 1485901, 1485906, 1485910, 1485911,
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Let's see whether Coverity actually understands the (relatively) new
>> pseudo-keyword.
>
> This is exceedingly disappointing. Coverity used to have the only
> sensible rule for not causing spurious fallthrough warnings, but this
> has apparently regressed.
>
> Coverity works on the AST, so ought to be after GCC has interpreted
> __attribute__((__fallthrough__)) if applicable.
>
> However, I doubt it will work in the fallback case, because #define
> fallthrough looks dubious. To trigger the older logic, the /*
> fallthrough */ comment needs to be the final thing before the next case
> label, and it isn't with the added semicolon.
>
> Given that this pseudo-keyword is restricted to the SMMU driver for now,
> we don't actually know if Coverity likes it or not.
When it was introduced, I did specifically ask whether it pleases
Coverity. I was told it would, and I had no proof to the contrary, so
I had to accept what I was told. My asking at the time was precisely
to avoid having to have two forms of annotation on every single
legitimate / intentional fall-through case.
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Ping: [PATCH] x86: mark hypercall argument regs clobbering for intended fall-through
2021-06-09 12:49 ` Andrew Cooper
2021-06-09 12:55 ` Jan Beulich
@ 2021-06-30 6:47 ` Jan Beulich
2021-06-30 6:50 ` Jan Beulich
2 siblings, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2021-06-30 6:47 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Roger Pau Monné, xen-devel
On 09.06.2021 14:49, Andrew Cooper wrote:
> On 09/06/2021 11:34, Jan Beulich wrote:
>> The CIDs below are all for the PV side of things, but also take care of
>> the HVM side.
>>
>> Coverity-ID: 1485896, 1485901, 1485906, 1485910, 1485911,
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Let's see whether Coverity actually understands the (relatively) new
>> pseudo-keyword.
>
> This is exceedingly disappointing. Coverity used to have the only
> sensible rule for not causing spurious fallthrough warnings, but this
> has apparently regressed.
>
> Coverity works on the AST, so ought to be after GCC has interpreted
> __attribute__((__fallthrough__)) if applicable.
>
> However, I doubt it will work in the fallback case, because #define
> fallthrough looks dubious. To trigger the older logic, the /*
> fallthrough */ comment needs to be the final thing before the next case
> label, and it isn't with the added semicolon.
>
> Given that this pseudo-keyword is restricted to the SMMU driver for now,
> we don't actually know if Coverity likes it or not.
My reply from the 9th had no further reaction, so let me ask more
directly: Besides leaving the Coverity issues open, what alternatives
do you see? IOW I'm missing from your reply any indication what it
would rework of the patch you want me to do, if any. Or if none, what
it is that stands in the way of getting this change in.
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86: mark hypercall argument regs clobbering for intended fall-through
2021-06-09 12:49 ` Andrew Cooper
2021-06-09 12:55 ` Jan Beulich
2021-06-30 6:47 ` Ping: " Jan Beulich
@ 2021-06-30 6:50 ` Jan Beulich
2 siblings, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2021-06-30 6:50 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Roger Pau Monné, xen-devel
On 09.06.2021 14:49, Andrew Cooper wrote:
> On 09/06/2021 11:34, Jan Beulich wrote:
>> The CIDs below are all for the PV side of things, but also take care of
>> the HVM side.
>>
>> Coverity-ID: 1485896, 1485901, 1485906, 1485910, 1485911,
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Let's see whether Coverity actually understands the (relatively) new
>> pseudo-keyword.
>
> This is exceedingly disappointing. Coverity used to have the only
> sensible rule for not causing spurious fallthrough warnings, but this
> has apparently regressed.
Actually - where do you see a regression here? These cases of fall-through
have been entirely un-annotated so far, so I'm instead surprised that
apparently there were no warnings so far. Or maybe they had been marked
false-positive in some database, and some unrelated code change made
Coverity think this was new / changed code.
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-30 6:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-09 10:34 [PATCH] x86: mark hypercall argument regs clobbering for intended fall-through Jan Beulich
2021-06-09 12:49 ` Andrew Cooper
2021-06-09 12:55 ` Jan Beulich
2021-06-30 6:47 ` Ping: " Jan Beulich
2021-06-30 6:50 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).