1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] spapr: add missing break in h_get_cpu_characteristics()

Greg Kurz posted 1 patch 6 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/151751446151.11348.15065690714406381610.stgit@bahia.lan
Test checkpatch passed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test ppc passed
Test s390x passed
hw/ppc/spapr_hcall.c |    1 +
1 file changed, 1 insertion(+)
[Qemu-devel] [PATCH] spapr: add missing break in h_get_cpu_characteristics()
Posted by Greg Kurz 6 years, 2 months ago 
Detected by Coverity (CID 1385702). This fixes the recently added hypercall
to let guests properly apply Spectre and Meltdown workarounds.

Fixes: c59704b25473 "target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS"
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_hcall.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 4d0e6eb0cf1d..596f58378a40 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1697,6 +1697,7 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
     switch (safe_indirect_branch) {
     case SPAPR_CAP_FIXED:
         characteristics |= H_CPU_CHAR_BCCTRL_SERIALISED;
+        break;
     default: /* broken */
         assert(safe_indirect_branch == SPAPR_CAP_BROKEN);
         break;
Re: [Qemu-devel] [PATCH] spapr: add missing break in h_get_cpu_characteristics()
Posted by Suraj Jitindar Singh 6 years, 2 months ago 
On Thu, 2018-02-01 at 20:47 +0100, Greg Kurz wrote:
> Detected by Coverity (CID 1385702). This fixes the recently added
> hypercall
> to let guests properly apply Spectre and Meltdown workarounds.
> 
> Fixes: c59704b25473 "target/ppc/spapr: Add H-Call
> H_GET_CPU_CHARACTERISTICS"
> Signed-off-by: Greg Kurz <groug@kaod.org>

Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

> ---
>  hw/ppc/spapr_hcall.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 4d0e6eb0cf1d..596f58378a40 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1697,6 +1697,7 @@ static target_ulong
> h_get_cpu_characteristics(PowerPCCPU *cpu,
>      switch (safe_indirect_branch) {
>      case SPAPR_CAP_FIXED:
>          characteristics |= H_CPU_CHAR_BCCTRL_SERIALISED;
> +        break;
>      default: /* broken */
>          assert(safe_indirect_branch == SPAPR_CAP_BROKEN);
>          break;
>
Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr: add missing break in h_get_cpu_characteristics()
Posted by Daniel Henrique Barboza 6 years, 2 months ago 

On 02/01/2018 05:47 PM, Greg Kurz wrote:
> Detected by Coverity (CID 1385702). This fixes the recently added hypercall
> to let guests properly apply Spectre and Meltdown workarounds.

Paolo Bonzini reported this error in a reply to the pull request that
added the patch:

"Re: [Qemu-ppc] [Qemu-devel] [PULL 12/12] target/ppc/spapr: Add H-Call 
H_GET_CPU_CHARACTERISTICS

On 28/01/2018 22:28, David Gibson wrote:

> +    switch (safe_indirect_branch) {
> +    case SPAPR_CAP_FIXED:
> +        characteristics |= H_CPU_CHAR_BCCTRL_SERIALISED;

Missing "break;" here.

Paolo

"

I think it is nice to mention in the commit msg that Paolo also detected 
this same error,
specially given that his email was sent before this patch.


Thanks,


Daniel


>
> Fixes: c59704b25473 "target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS"
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>   hw/ppc/spapr_hcall.c |    1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 4d0e6eb0cf1d..596f58378a40 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1697,6 +1697,7 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
>       switch (safe_indirect_branch) {
>       case SPAPR_CAP_FIXED:
>           characteristics |= H_CPU_CHAR_BCCTRL_SERIALISED;
> +        break;
>       default: /* broken */
>           assert(safe_indirect_branch == SPAPR_CAP_BROKEN);
>           break;
>
>
Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr: add missing break in h_get_cpu_characteristics()
Posted by Greg Kurz 6 years, 2 months ago 
On Fri, 2 Feb 2018 07:11:08 -0200
Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> wrote:

> On 02/01/2018 05:47 PM, Greg Kurz wrote:
> > Detected by Coverity (CID 1385702). This fixes the recently added hypercall
> > to let guests properly apply Spectre and Meltdown workarounds.  
> 
> Paolo Bonzini reported this error in a reply to the pull request that
> added the patch:
> 
> "Re: [Qemu-ppc] [Qemu-devel] [PULL 12/12] target/ppc/spapr: Add H-Call 
> H_GET_CPU_CHARACTERISTICS
> 
> On 28/01/2018 22:28, David Gibson wrote:
> 
> > +    switch (safe_indirect_branch) {
> > +    case SPAPR_CAP_FIXED:
> > +        characteristics |= H_CPU_CHAR_BCCTRL_SERIALISED;  
> 
> Missing "break;" here.
> 
> Paolo
> 
> "
> 
> I think it is nice to mention in the commit msg that Paolo also detected 
> this same error,
> specially given that his email was sent before this patch.
> 

Heh, Paolo's mail landed in the pull req thread in my mail client and I saw
it after sending the patch :P ... also I'm pretty sure Paolo was made aware
of this issue by Coverity, just as I was :)

From: scan-admin@coverity.com
To: groug@kaod.org
Subject: New Defects reported by Coverity Scan for QEMU
Date: Thu, 01 Feb 2018 18:11:33 +0000 (UTC)

Hi,

Please find the latest report on new defect(s) introduced to QEMU found with
Coverity Scan.

...

*** CID 1385702:  Control flow issues  (MISSING_BREAK)
/hw/ppc/spapr_hcall.c: 1700 in h_get_cpu_characteristics()
1694             break;
1695         }
1696     
1697         switch (safe_indirect_branch) {
1698         case SPAPR_CAP_FIXED:
1699             characteristics |= H_CPU_CHAR_BCCTRL_SERIALISED;
>>>     CID 1385702:  Control flow issues  (MISSING_BREAK)
>>>     The above case falls through to this one.  
1700         default: /* broken */
1701             assert(safe_indirect_branch == SPAPR_CAP_BROKEN);
1702             break;
1703         }
1704     
1705         args[0] = characteristics;


No big deal I guess :)

> 
> Thanks,
> 
> 
> Daniel
> 
> 
> >
> > Fixes: c59704b25473 "target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS"
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> >   hw/ppc/spapr_hcall.c |    1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index 4d0e6eb0cf1d..596f58378a40 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -1697,6 +1697,7 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
> >       switch (safe_indirect_branch) {
> >       case SPAPR_CAP_FIXED:
> >           characteristics |= H_CPU_CHAR_BCCTRL_SERIALISED;
> > +        break;
> >       default: /* broken */
> >           assert(safe_indirect_branch == SPAPR_CAP_BROKEN);
> >           break;
> >
> >  
>
Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr: add missing break in h_get_cpu_characteristics()
Posted by Daniel Henrique Barboza 6 years, 2 months ago 

On 02/02/2018 08:00 AM, Greg Kurz wrote:
> On Fri, 2 Feb 2018 07:11:08 -0200
> Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> wrote:
>
>> On 02/01/2018 05:47 PM, Greg Kurz wrote:
>>> Detected by Coverity (CID 1385702). This fixes the recently added hypercall
>>> to let guests properly apply Spectre and Meltdown workarounds.
>> Paolo Bonzini reported this error in a reply to the pull request that
>> added the patch:
>>
>> "Re: [Qemu-ppc] [Qemu-devel] [PULL 12/12] target/ppc/spapr: Add H-Call
>> H_GET_CPU_CHARACTERISTICS
>>
>> On 28/01/2018 22:28, David Gibson wrote:
>>
>>> +    switch (safe_indirect_branch) {
>>> +    case SPAPR_CAP_FIXED:
>>> +        characteristics |= H_CPU_CHAR_BCCTRL_SERIALISED;
>> Missing "break;" here.
>>
>> Paolo
>>
>> "
>>
>> I think it is nice to mention in the commit msg that Paolo also detected
>> this same error,
>> specially given that his email was sent before this patch.
>>
> Heh, Paolo's mail landed in the pull req thread in my mail client and I saw
> it after sending the patch :P ... also I'm pretty sure Paolo was made aware
> of this issue by Coverity, just as I was :)
>
> From: scan-admin@coverity.com
> To: groug@kaod.org
> Subject: New Defects reported by Coverity Scan for QEMU
> Date: Thu, 01 Feb 2018 18:11:33 +0000 (UTC)
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to QEMU found with
> Coverity Scan.
>
> ...
>
> *** CID 1385702:  Control flow issues  (MISSING_BREAK)
> /hw/ppc/spapr_hcall.c: 1700 in h_get_cpu_characteristics()
> 1694             break;
> 1695         }
> 1696
> 1697         switch (safe_indirect_branch) {
> 1698         case SPAPR_CAP_FIXED:
> 1699             characteristics |= H_CPU_CHAR_BCCTRL_SERIALISED;
>>>>      CID 1385702:  Control flow issues  (MISSING_BREAK)
>>>>      The above case falls through to this one.
> 1700         default: /* broken */
> 1701             assert(safe_indirect_branch == SPAPR_CAP_BROKEN);
> 1702             break;
> 1703         }
> 1704
> 1705         args[0] = characteristics;
>
>
> No big deal I guess :)

Roger that!

>
>> Thanks,
>>
>>
>> Daniel
>>
>>
>>> Fixes: c59704b25473 "target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS"
>>> Signed-off-by: Greg Kurz <groug@kaod.org>
>>> ---
>>>    hw/ppc/spapr_hcall.c |    1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>>> index 4d0e6eb0cf1d..596f58378a40 100644
>>> --- a/hw/ppc/spapr_hcall.c
>>> +++ b/hw/ppc/spapr_hcall.c
>>> @@ -1697,6 +1697,7 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
>>>        switch (safe_indirect_branch) {
>>>        case SPAPR_CAP_FIXED:
>>>            characteristics |= H_CPU_CHAR_BCCTRL_SERIALISED;
>>> +        break;
>>>        default: /* broken */
>>>            assert(safe_indirect_branch == SPAPR_CAP_BROKEN);
>>>            break;
>>>
>>>
Re: [Qemu-devel] [PATCH] spapr: add missing break in h_get_cpu_characteristics()
Posted by David Gibson 6 years, 2 months ago 
On Thu, Feb 01, 2018 at 08:47:41PM +0100, Greg Kurz wrote:
> Detected by Coverity (CID 1385702). This fixes the recently added hypercall
> to let guests properly apply Spectre and Meltdown workarounds.
> 
> Fixes: c59704b25473 "target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS"
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-2.12, thanks.

> ---
>  hw/ppc/spapr_hcall.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 4d0e6eb0cf1d..596f58378a40 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1697,6 +1697,7 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
>      switch (safe_indirect_branch) {
>      case SPAPR_CAP_FIXED:
>          characteristics |= H_CPU_CHAR_BCCTRL_SERIALISED;
> +        break;
>      default: /* broken */
>          assert(safe_indirect_branch == SPAPR_CAP_BROKEN);
>          break;
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.