hw/ssi/mss-spi.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
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
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
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 >
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
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 >
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
>
>
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
>>
>>
>>
© 2016 - 2024 Red Hat, Inc.