[Qemu-devel] [Qemu devel V4 PATCH] msf2: Remove dead code reported by Coverity

Subbaraya Sundeep posted 1 patch 6 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1508898544-10307-1-git-send-email-sundeep.lkml@gmail.com
Test checkpatch passed
Test docker passed
Test s390x passed
hw/ssi/mss-spi.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
[Qemu-devel] [Qemu devel V4 PATCH] msf2: Remove dead code reported by Coverity
Posted by Subbaraya Sundeep 6 years, 5 months ago
Fixed incorrect frame size mask, validated maximum frame
size in spi_write and removed dead code.

Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
---
v4:
	changed %d to %u while logging frame size error.
v3:
	Added comment that [31:6] bits are reserved in R_SPI_DFSIZE
	register and logged incorrect value too in guest error(suggested
	by Darren).
v2:
	else if -> else in set_fifodepth
	log guest error when frame size is more than 32

 hw/ssi/mss-spi.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c
index 5a8e308..d60daba 100644
--- a/hw/ssi/mss-spi.c
+++ b/hw/ssi/mss-spi.c
@@ -76,9 +76,10 @@
 #define C_BIGFIFO            (1 << 29)
 #define C_RESET              (1 << 31)
 
-#define FRAMESZ_MASK         0x1F
+#define FRAMESZ_MASK         0x3F
 #define FMCOUNT_MASK         0x00FFFF00
 #define FMCOUNT_SHIFT        8
+#define FRAMESZ_MAX          32
 
 static void txfifo_reset(MSSSpiState *s)
 {
@@ -104,10 +105,8 @@ static void set_fifodepth(MSSSpiState *s)
         s->fifo_depth = 32;
     } else if (size <= 16) {
         s->fifo_depth = 16;
-    } else if (size <= 32) {
-        s->fifo_depth = 8;
     } else {
-        s->fifo_depth = 4;
+        s->fifo_depth = 8;
     }
 }
 
@@ -301,6 +300,17 @@ static void spi_write(void *opaque, hwaddr addr,
         if (s->enabled) {
             break;
         }
+        /*
+         * [31:6] bits are reserved bits and for future use.
+         * [5:0] are for frame size. Only [5:0] bits are validated
+         * during write, [31:6] bits are untouched.
+         */
+        if ((value & FRAMESZ_MASK) > FRAMESZ_MAX) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: Incorrect size %u provided."
+                         "Maximum frame size is %u\n",
+                         __func__, value & FRAMESZ_MASK, FRAMESZ_MAX);
+            break;
+        }
         s->regs[R_SPI_DFSIZE] = value;
         break;
 
-- 
2.5.0


Re: [Qemu-devel] [Qemu devel V4 PATCH] msf2: Remove dead code reported by Coverity
Posted by Darren Kenny 6 years, 5 months ago
LGTM now, thanks.

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

On Wed, Oct 25, 2017 at 07:59:04AM +0530, Subbaraya Sundeep wrote:
>Fixed incorrect frame size mask, validated maximum frame
>size in spi_write and removed dead code.
>
>Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
>---
>v4:
>	changed %d to %u while logging frame size error.
>v3:
>	Added comment that [31:6] bits are reserved in R_SPI_DFSIZE
>	register and logged incorrect value too in guest error(suggested
>	by Darren).
>v2:
>	else if -> else in set_fifodepth
>	log guest error when frame size is more than 32
>
> hw/ssi/mss-spi.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
>diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c
>index 5a8e308..d60daba 100644
>--- a/hw/ssi/mss-spi.c
>+++ b/hw/ssi/mss-spi.c
>@@ -76,9 +76,10 @@
> #define C_BIGFIFO            (1 << 29)
> #define C_RESET              (1 << 31)
>
>-#define FRAMESZ_MASK         0x1F
>+#define FRAMESZ_MASK         0x3F
> #define FMCOUNT_MASK         0x00FFFF00
> #define FMCOUNT_SHIFT        8
>+#define FRAMESZ_MAX          32
>
> static void txfifo_reset(MSSSpiState *s)
> {
>@@ -104,10 +105,8 @@ static void set_fifodepth(MSSSpiState *s)
>         s->fifo_depth = 32;
>     } else if (size <= 16) {
>         s->fifo_depth = 16;
>-    } else if (size <= 32) {
>-        s->fifo_depth = 8;
>     } else {
>-        s->fifo_depth = 4;
>+        s->fifo_depth = 8;
>     }
> }
>
>@@ -301,6 +300,17 @@ static void spi_write(void *opaque, hwaddr addr,
>         if (s->enabled) {
>             break;
>         }
>+        /*
>+         * [31:6] bits are reserved bits and for future use.
>+         * [5:0] are for frame size. Only [5:0] bits are validated
>+         * during write, [31:6] bits are untouched.
>+         */
>+        if ((value & FRAMESZ_MASK) > FRAMESZ_MAX) {
>+            qemu_log_mask(LOG_GUEST_ERROR, "%s: Incorrect size %u provided."
>+                         "Maximum frame size is %u\n",
>+                         __func__, value & FRAMESZ_MASK, FRAMESZ_MAX);
>+            break;
>+        }
>         s->regs[R_SPI_DFSIZE] = value;
>         break;
>
>-- 
>2.5.0
>
>

Re: [Qemu-devel] [Qemu devel V4 PATCH] msf2: Remove dead code reported by Coverity
Posted by sundeep subbaraya 6 years, 5 months ago
On Wed, Oct 25, 2017 at 2:15 PM, Darren Kenny <darren.kenny@oracle.com>
wrote:

> LGTM now, thanks.
>
> Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
>

Thanks Darren,

Sundeep

>
> Thanks,
>
> Darren.
>
>
> On Wed, Oct 25, 2017 at 07:59:04AM +0530, Subbaraya Sundeep wrote:
>
>> Fixed incorrect frame size mask, validated maximum frame
>> size in spi_write and removed dead code.
>>
>> Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
>> ---
>> v4:
>>         changed %d to %u while logging frame size error.
>> v3:
>>         Added comment that [31:6] bits are reserved in R_SPI_DFSIZE
>>         register and logged incorrect value too in guest error(suggested
>>         by Darren).
>> v2:
>>         else if -> else in set_fifodepth
>>         log guest error when frame size is more than 32
>>
>> hw/ssi/mss-spi.c | 18 ++++++++++++++----
>> 1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c
>> index 5a8e308..d60daba 100644
>> --- a/hw/ssi/mss-spi.c
>> +++ b/hw/ssi/mss-spi.c
>> @@ -76,9 +76,10 @@
>> #define C_BIGFIFO            (1 << 29)
>> #define C_RESET              (1 << 31)
>>
>> -#define FRAMESZ_MASK         0x1F
>> +#define FRAMESZ_MASK         0x3F
>> #define FMCOUNT_MASK         0x00FFFF00
>> #define FMCOUNT_SHIFT        8
>> +#define FRAMESZ_MAX          32
>>
>> static void txfifo_reset(MSSSpiState *s)
>> {
>> @@ -104,10 +105,8 @@ static void set_fifodepth(MSSSpiState *s)
>>         s->fifo_depth = 32;
>>     } else if (size <= 16) {
>>         s->fifo_depth = 16;
>> -    } else if (size <= 32) {
>> -        s->fifo_depth = 8;
>>     } else {
>> -        s->fifo_depth = 4;
>> +        s->fifo_depth = 8;
>>     }
>> }
>>
>> @@ -301,6 +300,17 @@ static void spi_write(void *opaque, hwaddr addr,
>>         if (s->enabled) {
>>             break;
>>         }
>> +        /*
>> +         * [31:6] bits are reserved bits and for future use.
>> +         * [5:0] are for frame size. Only [5:0] bits are validated
>> +         * during write, [31:6] bits are untouched.
>> +         */
>> +        if ((value & FRAMESZ_MASK) > FRAMESZ_MAX) {
>> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: Incorrect size %u
>> provided."
>> +                         "Maximum frame size is %u\n",
>> +                         __func__, value & FRAMESZ_MASK, FRAMESZ_MAX);
>> +            break;
>> +        }
>>         s->regs[R_SPI_DFSIZE] = value;
>>         break;
>>
>> --
>> 2.5.0
>>
>>
>>
Re: [Qemu-devel] [Qemu devel V4 PATCH] msf2: Remove dead code reported by Coverity
Posted by Alistair Francis 6 years, 5 months ago
On Wed, Oct 25, 2017 at 4:29 AM, Subbaraya Sundeep
<sundeep.lkml@gmail.com> wrote:
> Fixed incorrect frame size mask, validated maximum frame
> size in spi_write and removed dead code.
>
> Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

> ---
> v4:
>         changed %d to %u while logging frame size error.
> v3:
>         Added comment that [31:6] bits are reserved in R_SPI_DFSIZE
>         register and logged incorrect value too in guest error(suggested
>         by Darren).
> v2:
>         else if -> else in set_fifodepth
>         log guest error when frame size is more than 32
>
>  hw/ssi/mss-spi.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c
> index 5a8e308..d60daba 100644
> --- a/hw/ssi/mss-spi.c
> +++ b/hw/ssi/mss-spi.c
> @@ -76,9 +76,10 @@
>  #define C_BIGFIFO            (1 << 29)
>  #define C_RESET              (1 << 31)
>
> -#define FRAMESZ_MASK         0x1F
> +#define FRAMESZ_MASK         0x3F
>  #define FMCOUNT_MASK         0x00FFFF00
>  #define FMCOUNT_SHIFT        8
> +#define FRAMESZ_MAX          32
>
>  static void txfifo_reset(MSSSpiState *s)
>  {
> @@ -104,10 +105,8 @@ static void set_fifodepth(MSSSpiState *s)
>          s->fifo_depth = 32;
>      } else if (size <= 16) {
>          s->fifo_depth = 16;
> -    } else if (size <= 32) {
> -        s->fifo_depth = 8;
>      } else {
> -        s->fifo_depth = 4;
> +        s->fifo_depth = 8;
>      }
>  }
>
> @@ -301,6 +300,17 @@ static void spi_write(void *opaque, hwaddr addr,
>          if (s->enabled) {
>              break;
>          }
> +        /*
> +         * [31:6] bits are reserved bits and for future use.
> +         * [5:0] are for frame size. Only [5:0] bits are validated
> +         * during write, [31:6] bits are untouched.
> +         */
> +        if ((value & FRAMESZ_MASK) > FRAMESZ_MAX) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: Incorrect size %u provided."
> +                         "Maximum frame size is %u\n",
> +                         __func__, value & FRAMESZ_MASK, FRAMESZ_MAX);
> +            break;
> +        }
>          s->regs[R_SPI_DFSIZE] = value;
>          break;
>
> --
> 2.5.0
>

Re: [Qemu-devel] [Qemu devel V4 PATCH] msf2: Remove dead code reported by Coverity
Posted by Peter Maydell 6 years, 4 months ago
On 25 October 2017 at 03:29, Subbaraya Sundeep <sundeep.lkml@gmail.com> wrote:
> Fixed incorrect frame size mask, validated maximum frame
> size in spi_write and removed dead code.
>
> Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
> ---
> v4:
>         changed %d to %u while logging frame size error.
> v3:
>         Added comment that [31:6] bits are reserved in R_SPI_DFSIZE
>         register and logged incorrect value too in guest error(suggested
>         by Darren).
> v2:
>         else if -> else in set_fifodepth
>         log guest error when frame size is more than 32
>
>  hw/ssi/mss-spi.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>



Applied to target-arm.next, thanks.

-- PMM