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

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

[Subbaraya Sundeep]

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>
---
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..e1b6227 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 %d provided."
+                         "Maximum frame size is %d\n",
+                         __func__, value & FRAMESZ_MASK, FRAMESZ_MAX);
+            break;
+        }
         s->regs[R_SPI_DFSIZE] = value;
         break;
 
-- 
2.5.0

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

[no-reply]

Hi,

This series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 1508678882-4327-1-git-send-email-sundeep.lkml@gmail.com
Subject: [Qemu-devel] [Qemu devel v3 PATCH] msf2: Remove dead code reported by Coverity

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
time make docker-test-build@min-glib
time make docker-test-mingw@fedora
time make docker-test-block@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/1508678882-4327-1-git-send-email-sundeep.lkml@gmail.com -> patchew/1508678882-4327-1-git-send-email-sundeep.lkml@gmail.com
Switched to a new branch 'test'
a1c66b66e7 msf2: Remove dead code reported by Coverity

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc'...
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule path '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc' failed
Failed to clone 'dtc'. Retry scheduled
Cloning into '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc'...
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule path '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc' failed
Failed to clone 'dtc' a second time, aborting
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

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

[Darren Kenny]

On Sun, Oct 22, 2017 at 06:58:02PM +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>
>---
>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..e1b6227 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 %d provided."
>+                         "Maximum frame size is %d\n",
>+                         __func__, value & FRAMESZ_MASK, FRAMESZ_MAX);

Nitpicking here really, but shouldn't the %d be %u since value is a uint32_t?

Thanks,

Darren.

>+            break;
>+        }
>         s->regs[R_SPI_DFSIZE] = value;
>         break;
>
>-- 
>2.5.0
>
>

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

[sundeep subbaraya]

Hi Darren,

On Mon, Oct 23, 2017 at 12:18 AM, Darren Kenny <darren.kenny@oracle.com>
wrote:

> On Sun, Oct 22, 2017 at 06:58:02PM +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>
>> ---
>> 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..e1b6227 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 %d
>> provided."
>> +                         "Maximum frame size is %d\n",
>> +                         __func__, value & FRAMESZ_MASK, FRAMESZ_MAX);
>>
>
> Nitpicking here really, but shouldn't the %d be %u since value is a
> uint32_t?
>
> Ok I will change to %u.

Thanks,
Sundeep


> Thanks,
>
> Darren.
>
>
> +            break;
>> +        }
>>         s->regs[R_SPI_DFSIZE] = value;
>>         break;
>>
>> --
>> 2.5.0
>>
>>
>>

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

[sundeep subbaraya]

Hi Peter,

On Sun, Oct 22, 2017 at 7:35 PM, <no-reply@patchew.org> wrote:

> Hi,
>
> This series failed automatic build test. Please find the testing commands
> and
> their output below. If you have docker installed, you can probably
> reproduce it
> locally.
>
> Type: series
> Message-id: 1508678882-4327-1-git-send-email-sundeep.lkml@gmail.com
> Subject: [Qemu-devel] [Qemu devel v3 PATCH] msf2: Remove dead code
> reported by Coverity
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> set -e
> git submodule update --init dtc
> # Let docker tests dump environment info
> export SHOW_ENV=1
> export J=8
> time make docker-test-quick@centos6
> time make docker-test-build@min-glib
> time make docker-test-mingw@fedora
> time make docker-test-block@fedora
> === TEST SCRIPT END ===
>
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>  * [new tag]               patchew/1508678882-4327-1-git-
> send-email-sundeep.lkml@gmail.com -> patchew/1508678882-4327-1-git-
> send-email-sundeep.lkml@gmail.com
> Switched to a new branch 'test'
> a1c66b66e7 msf2: Remove dead code reported by Coverity
>
> === OUTPUT BEGIN ===
> Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path
> 'dtc'
> Cloning into '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc'...
> fatal: Could not read from remote repository.
>
> Please make sure you have the correct access rights
> and the repository exists.
> fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule path
> '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc' failed
> Failed to clone 'dtc'. Retry scheduled
> Cloning into '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc'...
> fatal: Could not read from remote repository.
>
> Please make sure you have the correct access rights
> and the repository exists.
> fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule path
> '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc' failed
> Failed to clone 'dtc' a second time, aborting
> === OUTPUT END ===
>
> Test command exited with code: 1
>

I did not understand. Is the error related to my patch?

Thanks,
Sundeep


>
>
> ---
> Email generated automatically by Patchew [http://patchew.org/].
> Please send your feedback to patchew-devel@freelists.org
>

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

[Fam Zheng]

On Mon, 10/23 08:59, sundeep subbaraya wrote:
> > Cloning into '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc'...
> > fatal: Could not read from remote repository.
> >
> > Please make sure you have the correct access rights
> > and the repository exists.
> > fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule path
> > '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc' failed
> > Failed to clone 'dtc' a second time, aborting
> > === OUTPUT END ===
> >
> > Test command exited with code: 1
> >
> 
> I did not understand. Is the error related to my patch?

Nope, there is something wrong with the testing bot.

Fam

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

[sundeep subbaraya]

Hi Fam,

On Mon, Oct 23, 2017 at 10:48 AM, Fam Zheng <famz@redhat.com> wrote:

> On Mon, 10/23 08:59, sundeep subbaraya wrote:
> > > Cloning into '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc'...
> > > fatal: Could not read from remote repository.
> > >
> > > Please make sure you have the correct access rights
> > > and the repository exists.
> > > fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule
> path
> > > '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc' failed
> > > Failed to clone 'dtc' a second time, aborting
> > > === OUTPUT END ===
> > >
> > > Test command exited with code: 1
> > >
> >
> > I did not understand. Is the error related to my patch?
>
> Nope, there is something wrong with the testing bot.
>

Ok.

Thanks,
Sundeep

>
> Fam
>